I've noticed in _content_moderation_save_history() the following;

  if(module_exists('trigger')) {
    module_invoke_all('content_moderation', "{$node->type}_{$curstate}_{$nextstate}", $user);
  }
  if(module_exists('rules')) {
    rules_invoke_event("{$node->type}_{$curstate}_{$nextstate}", $node);
    rules_invoke_event("anynode_anystate_{$nextstate}", $node);
  }

In the instance of triggers we're passing through the $user var. Should this be the case?

I'm writing an action that is triggered by a moderations state change and the $object being passed through is this $user and of no use at all. There is no reference to the $node at all too.

The $user that is being passed thorough is also the global $user object.

I'm suspecting that this is in error.

Can I get confirmation that this is indeen an error and that $node should be being passed though instead.

Cheers,
Gold

Comments

eugenmayer’s picture

eugenmayer commented

Well first, i would have said that is by intention. But looking at the code, that does not make sense at all.
I think it would be handy to have both objects there. "Who is doing what on what object".

to answer your question, yes its a bug :)

gold’s picture

gold
they/them
Location 20 minutes in the future
commented

Yeah, given that in this instance $user is the global $user it's already available from everywhere it didn't make a lot of sense to me. :)

Cool. Will get it fixed up.

Eugen, how do you want me to contribute these back? I've been pushing my updates to my fork thinking that you could merge them back into your repo. Have you been doing this or would you like me to push them straight to yours?

eugenmayer’s picture

eugenmayer commented

Well lets do it that way:

You alway contribute directly into the folder, but we just coordinate it that way:
- every patch should be in a queue and marked "patch to be ported", before you / i apply it
- before we release, we have a chat and things like that

is that ok? You already have SCM access and all this. Welcome to the team :)

eugenmayer’s picture

eugenmayer commented
Status: Active » Patch (to be ported)

:)

gold’s picture

gold
they/them
Location 20 minutes in the future
commented
Status: Patch (to be ported) » Needs review

Cool. Thanks. :)

With the time pressure I've currently got I've rolled this out to my fork. I've not actually tried merging from one repo to another before so for the moment (until I get the opportunity to learn how to do it right) I'll keep going the way I am. Better that than bungling it and adding to your workload undoing a potential mess.

The update I just pushed to my repo includes a new trigger and action;
Trigger: State of any node changes
Action: Ensure Content Moderation has unique state

The trigger is self explanatory.

The Action checks to see if the node type is under moderation and that the state we're changing to in neither none or live. If these conditions are met it sets all revisions of the current node with the previous state to none ensuring that there will only ever be one revision with any given non-live/none state.

The usecase where this was required for me;
We're using Views(node_revisions) and VBO to allow the client to filter revisions based on state to bulk publish nodes. We have 3 states (none -> review -> live). We found it was possible to have more than one revision in review. This was undesirable from our approach as using VBO to update nodes under review to a live state wouldn't work if we had more than one revision for a single node in the review state.

Adding this trigger and action allowed us to get around this.

With this being an action and trigger that is not on by default it doesn't impact the workflow unless the user chooses to use this approach.

This update currently resides at https://github.com/Unifex/content_moderation/commit/233b10610fbeb0dbf0be...

eugenmayer’s picture

eugenmayer commented

Iam not expecting you to have any merge-issues actually. The only issue we have is that you did not work with branches, so you rather would have issues seperating commits for specific issues

gold’s picture

gold
they/them
Location 20 minutes in the future
commented

Yeah. Kind of new to git and was under time pressure from the boss. :/

Once this project is down I'll have to sit down and take the time to get up to speed on git tags/branches etc.