Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Throw warning upon importing rules with unsupported actions or events #159

Open
argiepiano opened this issue Jan 29, 2022 · 8 comments · May be fixed by #181
Open

[WIP] Throw warning upon importing rules with unsupported actions or events #159

argiepiano opened this issue Jan 29, 2022 · 8 comments · May be fixed by #181

Comments

@argiepiano
Copy link
Collaborator

Vocabularies were entities in D7. This means that imported D7 Rules that contain vocabulary entity actions or events will fail in Backdrop. We should figure out a way to catch those and throw a warning, or perhaps even correct/switch those to the newly created vocabulary actions and events.

This also applies programmatic imports, and rules stored in D7 db.

Not quite sure what the best approach is.

@argiepiano argiepiano changed the title Throw warning upon importing rules with unsupported vocabulary entity actions or events Throw warning upon importing rules with unsupported actions or events Feb 8, 2022
@argiepiano
Copy link
Collaborator Author

argiepiano commented Feb 8, 2022

I've changed the title of this to make it more global (not limited to vocabularies). I just tested migrating a simple D7 site with a rule that used the common "Show a message on site". This action has a machine name drupal_message (its equivalent is backdrop_message).

The rule shows an error in the UI, which is good. I'm wondering if this may be enough, or we want throw additional warnings, or somehow "fix" those rules.
Screen Shot 2022-02-07 at 9 18 05 PM

@argiepiano
Copy link
Collaborator Author

argiepiano commented Feb 8, 2022

Alternatively, we could create a parallel set of actions and events that use the D7 machine names but use the Backdrop callbacks, and add a deprecated watchdog message - kind of what core functions like drupal_set_message() currently do in Backdrop.

I believe these are very few.

@bugfolder
Copy link
Collaborator

Alternatively, we could create a parallel set of actions and events that use the D7 machine names but use the Backdrop callbacks, and add a deprecated watchdog message

That certainly sounds the most user-friendly for upgraders.

@argiepiano
Copy link
Collaborator Author

argiepiano commented May 28, 2022

A quick update on this.

  • Setting rules to throw a deprecated message for drupal_message and drupal_watchdog (the only two rule actions whose names are different in Backdrop) is pretty straightroward
  • The second issue is vocabularies:
    • Events are fine, as all vocabulary events are available in Bd with the same names
    • Conditions involving vocabularies, while they exist in D7 (through entity conditions) they don't make much sense (e.g. Vocabularies don't have bundles), so it's very unlikely that a useful condition would be part of a rule
    • The trickiest ones are actions. D7, for example, made creation of vocabularies an option of "Create a new entity". Backdrop has a separate action called "Create a new vocabulary". If an action using the entity creation to create a Voc in D7 were imported in B, Backdrop currently would import it fine, without throwing any error or warning. When run, the action would throw warnings (by Entity Plus specifically) and would not work

So, validating those actions that use Vocabularies as parameters is something I still need to figure out.

argiepiano added a commit to argiepiano/rules that referenced this issue May 28, 2022
@argiepiano argiepiano linked a pull request May 28, 2022 that will close this issue
@argiepiano
Copy link
Collaborator Author

PR #181 is a "work in progress" - so far dealing with drupal_watchdog and drupal_message actions.

@argiepiano
Copy link
Collaborator Author

New commit hides the actions from the select box in the Add new action page. Actions whose info contains a "hidden => TRUE" flag are hidden.

@bugfolder
Copy link
Collaborator

I seem to have overlooked the PR for this issue up until now. @argiepiano, is this still WIP?

@argiepiano
Copy link
Collaborator Author

@bugfolder, what's there works, but it doesn't completely address the issue. I haven't done any work on this lately. The idea here was to catch D7 imported rules that used actions that don't exist in Backdrop. It's probably a good idea to mark this as WIP

@argiepiano argiepiano changed the title Throw warning upon importing rules with unsupported actions or events [WIP] Throw warning upon importing rules with unsupported actions or events Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants