- Issue created by @jastraat
- Merge request !41Issue #3522425: Extend retain revision info setting to all bulk operations. β (Merged) created by jastraat
-
jastraat β
committed 349315a2 on 2.0.x
Issue #3522425: Extend retain revision info setting to all bulk...
-
jastraat β
committed 349315a2 on 2.0.x
Automatically closed - issue fixed for 2 weeks with no activity.
- πΊπΈUnited States jastraat
@joseph.olstad I appreciate your initiative, but this was unfortunately a work in progress that I hadn't marked as ready for review. Would you mind reverting that commit? It's not complete unfortunately and it causes a PHP error in its current state.
- πΊπΈUnited States jastraat
Also - I'm not sure it makes sense to extend this to the unpublish/archive actions. In my testing, the publish action did not create a new revision, but the archive/unpublish action DOES create a new revision of the node.
-
joseph.olstad β
committed 980e88c0 on 2.0.x
Revert "Issue #3522425: Extend retain revision info setting to all bulk...
-
joseph.olstad β
committed 980e88c0 on 2.0.x
- πΊπΈUnited States jastraat
Thank you! I'll let you know when I'm ready for a review :)
- π¨π¦Canada joseph.olstad
this was not tagged so there's no need to publish a release to revert.
- Merge request !42Issue #3522425: If retain revision info enabled, append last log message β (Open) created by jastraat
- πΊπΈUnited States jastraat
Alrighty, I created a new merge request that I actually did some testing with.
In looking deeper at the code, only the publish action does not create a new revision but instead updates the latest one.
Given that, the code needs to treat that action slightly differently. With the publish action, we retain all the original revision information (author, date, message) and simply append information about who/when bulk publishing occurred.
For all other operations where there is a new revision created, I'm leaving the new revision information mostly untouched but append information (if it exists) about the last revision of the entity to the bulk message.
This will help our use case and it does remove a bit of code duplication even if someone isn't using the new setting.
But - I'm not sure how widely useful this is for non-publish actions (in other site use cases). Let me know what you think, and please test with and without the new "retain revision info" option enabled. - π¨π¦Canada joseph.olstad
Ok great, thanks for this, it looks very good.
If we're happy with it, put it in the 2.0.x dev , and then let it simmer in there for a while.
- πΊπΈUnited States jastraat
Hey Joseph, this isn't directly related to the functionality of this ticket, but I'm noticing something strange when using the module with media entities.
There's a different process for using this module with a media view ( β¨ Views Bulk Operations Actions for media entities Active ) - which is fine. However, when using that approach, the "Archive Current Revision" action does not appear - just the publish/unpublish current revision.
That's not a big deal, but it looks like the "unpublish" action in this module runs the archive action first - and then runs the draft action on top of it.
You end up with a content entity that is unpublished but in a draft vs archived moderation state. I'm not sure the best approach here, but this issue is highlighted with this patch applied because it's appending both moderation state changes in the revision log message.
Example:
"Bulk operation create draft revision. Last revision message: Bulk operation create archived revision. Last revision message: This content is ready to unpublish"In #3253887: Change revision message on bulk Unpublish β someone else mentioned how the current log message is confusing, and I can definitely see why now. Is there a reason the unpublish revision doesn't just set to the archived state and only fallback to the draft state if no archive state was found?
- πΊπΈUnited States jastraat
I went ahead an made adjustments to only have the archive change happen on the "unpublish" action with the draft as a fallback. It does eliminate quite a bit of duplicate code and results in a log message that I think makes more sense and an end moderation state of whatever the archived one is (if available).
I recognize that the scope is increased though.
- π¨π¦Canada joseph.olstad
ok sure, sounds like a good idea. I'll plan to do some testing on this in the comming weeks . Meanwhile I applaud any others wishing to help review and test this.
- πΊπΈUnited States jasongose
This works as advertised for me (drupal 10.4.7; sorry for the double post, I missed updating the status)
- π¨π¦Canada joseph.olstad
Cool, thanks for testing, this is on my radar 100%, I'd like to test a few of my multilingual setups , we tend to publish and unpublish both of our languages at the same time.