Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
As a way of working around race conditions (#2376819: Race condition on busy site), compatibility problems (#1807460: Field collection doesn't play nice with workbench moderation (patch)), etc, rewrite the module to leverage the new Drafty module which executes the state changes during a single entity update action thanks to its use of hook_entity_presave().
Comment | File | Size | Author |
---|---|---|---|
#129 | workbench_moderation-n2376839-125.patch | 47.29 KB | DamienMcKenna |
#125 | interdiff-2376839-106-125.txt | 3.2 KB | gnucifer |
#125 | workbench_moderation-n2376839-125.patch | 47.29 KB | gnucifer |
#119 | workbench_moderation-n2376839-119.patch | 46.56 KB | gnucifer |
#106 | workbench_moderation-n2376839-106.patch | 46 KB | DamienMcKenna |
Comments
Comment #1
DamienMcKennaBecause WM 2 is using the State Machine v3 module which still uses the dual node_save() process, this would need to either be a fork or be built as WM 3.
Comment #2
DamienMcKennaComment #3
das-peter CreditAttribution: das-peter commentedThis sounds interesting. It would be great if we could focus all efforts of solving those issues (drafts / revisions) in one location.
Comment #4
catchThe main problems I've seen I've seen is with overwriting the current revision somehow. Either saving entities with $entity->revision = FALSE during workflow operations (i.e. publishing a draft revision) and/or using field_attach_update() to workaround core's lack of support for writing revisions without touching base tables etc.
Hope with drafty is to 1. Not have that set of problems since it'll always force a new revision all the time (then at some point deal with revision proliferation) 2. Provide a single low-level module that any workflow creating draft revisions can rely on so if there are issues they can be handled in one place.
I don't have clients using workbench_moderation atm so I won't work on this personally unless that changes, but happy to review if someone else takes this on.
Comment #5
MichelleComment #6
DamienMcKennaFYI the existing module Revisioning already uses the same/similar logic to handle state changes (hook_presave), so maybe effort should be moved over to using it instead?
Comment #7
catchSo I'd missed revisioning module when reviewing other work, but it doesn't look like it solves the various issues completely either.
If you look at revisioning_node_update() it's following a slightly different approach to drafty - it 'resets' the {node} and field_data_*() tables manually to match the previous published revision state, as opposed to drafty which always uses entity_save() everywhere. This means it needs to handle file entities, taxonomy index etc. individually. Also incompatible with alternative field storage.
Also it looks like it would suffer from at least one issue I've seen with CPS and field collections - where the field collection archive state isn't updated correctly for the published revision - this is tracked in field_collection_entity_update() (see current version of CPS module - various issues with field_collection is why I started the drafty project).
http://cgit.drupalcode.org/revisioning/tree/revisioning.module#n599
Comment #8
DamienMcKennaI presume the problems you saw with Field Collections in CPS would also be true of other linked objects, e.g. files, FPPs or ECK via EntityReference, i.e. that there's a need to also control the revisioned state of the linked objects? In EntityReference there's #1837650: Allow referencing a specific revision ID which can help, but I've not looked into Field Collection much.
Comment #9
catchField collection actually marks the field collection item entities as archived or not archived based on the status of the parent entity referencing them, that's not the case for any of the other reference modules so for the specific problem I'm thinking of it's not an issue. File usage is similar though and that can go horribly wrong with the update-in-place method.
The ability to say 'I want this version of this entity to be referenced by this version of this entity' isn't something that CPS deals with - if you have an edit to one entity that references another entity that you also edit at the same time, that can be handled by keeping both edits in the same changeset.
I'm not convinced per-revision entity references is a good idea - in the published version of the site you just want to reference the default revision, anything else bypasses field cache (or entitycache).
Comment #10
DamienMcKenna.. hence the notion of needing to keep a changeset of multiple entities.
If the "published" state of each of the connected entities is kept in sync, why wouldn't it work with caching?
Comment #11
catchIf the entity reference field is referenced by both entity id and revision id, then whenever you load the entity based on that reference you need to specify the revision ID.
Both core field cache and entitycache don't both caching when a specific revision ID is requested, so it'll be a cache miss on the field cache.
See https://api.drupal.org/api/drupal/modules!field!field.attach.inc/functio...
Comment #12
DamienMcKennaBumping this to v2, because it'd be a large architecture change.
States Machine doesn't fix the architectural flaw, WM is just "passing the buck".
Comment #13
das-peter CreditAttribution: das-peter commentedHow about "pass the buck" from State Machine to drafty then?
I'd really like to see this solved in a central place.
Comment #14
DamienMcKenna@das-peter: That'd be my suggestion.
Comment #15
mikeryanAnother pain point with the current architecture: #1445824: Support for Migrate module
Comment #16
das-peter CreditAttribution: das-peter commentedI started working on this locally. But it's not quite there yet
Comment #17
DamienMcKennaWaiting to see what happens with #2376819: Race condition on busy site first.
Comment #18
DamienMcKennaSorry, scrap that, back to Active.
Comment #19
iStryker CreditAttribution: iStryker commentedIf we do a rewrite, will it get rid of the workbench_moderation_store() function added to drupal_register_shutdown_function(). This has cause major problems, with resaving the live revision. This causes undesirable results when other modules that are using hook_node_save() and/or hook_entity_update()/hook_entity_insert().
Comment #20
das-peter CreditAttribution: das-peter commentedUsing drafty makes any
drupal_register_shutdown_function()
obsolete.Comment #21
agentrickardWe've been working on this internally for a project, and here's the patch we have, which includes proper test coverage.
Comment #22
MiroslavBanov CreditAttribution: MiroslavBanov commentedPatch #21 is for the 7.x-1.x version of Workbench moderation.
Just a suggestion - If thehook_entity_presave
is changed back tohook_node_presave
, and if the functionworkbench_moderation_set_node_state
is moved lower into the file, the diff would be easier to review.This is very good news. We have been using Workbench moderation module 1.x, but often there are problems with field collections, CER, programmatic saving of nodes, and others. I will test this patch when I find the time.
Comment #23
agentrickardIIRC, we have to use hook_entity_presave() because of the callback order. Drafty uses it, and if we fire in hook_node_presave() our changes are not respected.
Comment #24
MiroslavBanov CreditAttribution: MiroslavBanov commentedAha, OK :). Ignore what I said.
Comment #25
MiroslavBanov CreditAttribution: MiroslavBanov commentedOK, testing #21, it does appear to work, and I see no regressions. Great job, agentrickard. I think it is better to focus on that patch, rather than on the 2.x branch.
Unfortunately for me, #21 doesn't automatically resolve all of my problems related to field collections, and synchronization of workbench-moderation-enabled nodes on Drupal sites via services.
Comment #26
das-peter CreditAttribution: das-peter commentedWell, the 2.x branch uses state machine - and for state machine there's the 3.x-drafty branch we're testing for a project atm. :)
Comment #27
agentrickardSo is anyone willing to call this RTBC? Are the test cases sufficient?
Comment #28
MiroslavBanov CreditAttribution: MiroslavBanov commentedI haven't reviewed the tests or run the tests. But I did review the code, tested manually, and I am using the patch in a project. I may look at the tests later but this will be after couple of weeks, probably.
Comment #29
MiroslavBanov CreditAttribution: MiroslavBanov commentedNow that I see the test changes, they are not many. But for some reason I am getting failures. Not sure if there is any regression or they were like that initially. If there is no regression, I say this is RTBC, but need second opinion.
Test failures are attached.
Comment #30
MiroslavBanov CreditAttribution: MiroslavBanov commentedNeed testbot to test on 7.x-1.x
Comment #31
das-peter CreditAttribution: das-peter at Cando commentedFYI: I just merged the Drafty integration into the State Machine 7.x-3.x branch which is used by Workbench Moderation 2.x
Comment #32
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedNo idea what the test failures are, but from a 'drafty' / CPS maintainer perspective, this would be RTBC (ready to be committed) - I did not test it, but Miroslav did ...
Hoping that setting RTBC status will re-run the bot on the patch - can be set back afterwards.
Also not sure how 3.x handles the integration.
Comment #33
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedComment #34
colanMaybe it needs to be set to Needs Review? That test is still stuck. If this doesn't work, anyone know how to get it rolling again? We've got both the old & new QA systems enabled on this project.
Comment #35
catchI'd re-upload the patch.
Comment #36
joelpittetreup
Comment #42
Dave ReidDoes anyone else feel really uneasy making this dependency/change on the stable 1.x branch? Also confused about the very low number of test passes returned by the new test bot.
Comment #43
MiroslavBanov CreditAttribution: MiroslavBanov commentedWell you are probably right about uneasiness and dependency things. But the testbot failures don't make sense at all. They look like problems with the testbot.
Comment #44
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commented#42: The new number is just the number of tests, but if you look at the detailed results you see that it ran all the tests.
The old testbot counted the assertions - it is probably broken for good now though ...
Comment #45
MiroslavBanov CreditAttribution: MiroslavBanov commentedShould we open an issue in testbot issue queue? I really don't understand testbot problems, and the testbot FAQ is not very helpful.
Comment #46
MiroslavBanov CreditAttribution: MiroslavBanov commentedReally I don't understand what's going on with the testbot, bu let's try this one more time.
Only changed the dependencies in setUp().
Comment #49
DamienMcKennaOdd. I ran the tests locally and they all passed successfully. I'll look into them.
Comment #50
DamienMcKennaOk, yes, it's because the Drafty module wasn't downloaded, it only grabs core and workbench_moderation - see the console output for details: https://dispatcher.drupalci.org/job/default/83005/console
Comment #51
DamienMcKennaThe way to fix this would be add a small commit to the main module to add 'drafty' as a test requirement, and then it'd be picked up for a follow-on patch test. Something like the attached file.
Comment #52
DamienMcKennaFYI the hint to the root problem is the following failure message:
If it fails to enable a module then confirm in the console output it was able to download it - in this case it did not download drafty therefore it would obviously not be able to enable it.
Comment #53
nicrodgersThe attached patch is based on #46 and adds drafty as a test_dependency in the .info file as per #51, hopefully this will get the test bot to pass.
Comment #56
DamienMcKenna@nicrogers: The problem isn't the test_dependency in the main patch, the dependencies won't be downloaded unless they're first listed in the existing codebase, hence the need for a separate patch (#51) to first add the new dependency.
At this point we need a maintainer to chime in on what to do next.
Comment #58
das-peter CreditAttribution: das-peter at Cando commentedJust patched & pushed 7.x-1.x with #51 - hope the test bot can run now.
Comment #61
MiroslavBanov CreditAttribution: MiroslavBanov commentedLet's try this again. Same as #53, only change is info file.
Comment #62
DamienMcKennaThis is the patch from #36 rerolled.
Comment #63
nicrodgersAfter applying this patch, translating content with more than one revision doesn't work properly. Once there is more than one revision, the translate tab still shows links to the published revision, and there is no apparent way to translate the current draft revision.
Comment #64
MiroslavBanov CreditAttribution: MiroslavBanov commented@nicrodgers
Are you sure this patch is what caused the problem? Can you give some steps, or requirements to reproduce this?
Comment #65
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commented@nicrodgers
I tried to see how it works with i18n_node + translation + workbench_moderation (without patches). It is translating the published version and not the current draft, and there is no change/regression caused by this patch.
Please open a separate issue as it is not related. If you still think it is regression introduced with the patch, please provide steps to reproduce.
Comment #66
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commentedComment #67
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commentedTo me, this is RTBC. I have used the earlier version for a while. And all that's changed is the test dependency to fix the testbot failure.
Comment #68
nicrodgers@MiroslavBanov
This patch doesn't cause the problem specifically, sorry for the confusion.
There is a patch in https://www.drupal.org/node/1707156 that successfully integrates workbench moderation with entity translation, enabling translation of revisions through the workflow process. That issue was marked as postponed, as a number of developers said that this issue (rewriting to use Drafty module) would make that patch unnecessary. I wasn't sure which of the two issues to feedback on, so I added it to both. Sorry for the confusion.
Comment #69
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedRTBC + 1, patch still looks good to me.
Comment #70
colanAssuming the concerns from #42 have been alleviated, does anyone have any reservations about this latest patch, or should we commit it?
I'm assuming we'll have to put something in the release notes about installing Drafty.
Comment #71
catchIt should probably be a new branch given the new dependency?
Comment #72
DamienMcKennaThe problem with using a new branch is that there's already a 7.x-2.x branch that contains a different rewrite.
Comment #73
colanThat's what I was wondering. 2.x is used for the State Machine / State Flow implementation. So a new 3.x then?
Comment #74
hass CreditAttribution: hass commentedIs 2.x dead? Then we can overwrite it...
Comment #75
dgtlmoon CreditAttribution: dgtlmoon commented+1 overwriting 2.x branch. #1316314: Workflow Architecture for Workbench Moderation is what 2.x is about and that issue is verrrry unloved
Comment #76
DamienMcKennaMy concern with overwriting the 7.x-2.x branch is that it would break existing sites that were already using it, whereas publishing a new 7.x-3.x branch and not marking it as a preferred branch would allow those sites to continue as-is. Of course if the 7.x-1.x branch was deprecated would site maintainers know to jump to 7.x-3.x or would the updater jump to 7.x-2.x?
Ultimately, this whole situation is a little awkward.
Comment #77
dgtlmoon CreditAttribution: dgtlmoon commentedWhy not commit to 7.x-1.x ? The other concern is that this work will also get lost, 1.x is VERY active
Comment #78
hass CreditAttribution: hass commentedA 2.x DEV version was never supported. There was also no working upgrade path as I know. They are lost on all ways. If we now use 2.x everyone knows this is the next major version. I hope there is now a working upgrade path and also a upgrade path to D8.
Comment #79
catchSeems simpletest to make sure 7.x-2.x is marked completely unsupported and has no releases, then commit this to 7.x-3.x. Don't think it's that confusing if there's a dead 2.x branch.
Comment #80
dgtlmoon CreditAttribution: dgtlmoon commentedwhat happens in the future with this new 3.x branch then? how does anyone know it's got anything todo with Drafty? why not merge this into 1.x and close 2.x?
Comment #81
catch@dgtlmoon it'll need to be in the release notes and 1.x can move to unsupported.
The advantage of a new branch is less chance of people missing they need to do something extra when updating then getting fatal errors if they don't.
Comment #82
dgtlmoon CreditAttribution: dgtlmoon commented@catch sure, just trying to see that this work does not become another 2.x branch left behind
"The advantage of a new branch is less chance of people missing they need to do something extra when updating then getting fatal errors if they don't."
What are those 'something extras', can you write tests for it?
What fatal errors exactly are you referring to? should the issue be moved be to Needs Review? I'm still voting for 7.x-1.x there
I still see no clear way or plan that the existing 1.x branch and this 'new branch' you propose will ever become the one branch
Comment #83
catchYou need to download drafty module before updating.
1. Create 7.x-3.x branch
2. Commit this patch to it
3. Release 7.x-3.0
4. Move all 7.x-1.x issues to 7.x-3.x
5. Stop committing anything to 7.x-1.x
6. Mark 7.x-1.x unsupported
Comment #84
colanI'm with @catch on this one. Was just about to post that plan myself.
Comment #85
hass CreditAttribution: hass commentedI have a question about the upgrade path. Does this patch not require a single DB update? Just wondering that such a major change does not require an upgrade... Is it missing or not required?
Comment #86
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commentedI also think it is safest to move to 3.x, and like the plan in #83.
Comment #87
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commented@hass
> Correct. It really is not needed.
Comment #88
DamienMcKennaOne quick point - shouldn't there be an update script to enable Drafty?
Comment #89
DamienMcKennaThis is an example of what the update script could look like.
Comment #90
catch@DamienMcKenna that looks right to me.
Comment #91
DamienMcKennaComment #92
stefan.r CreditAttribution: stefan.r commentedNow that this is finalized, can we tag a new 7.x-1.5(-beta?) release?
Comment #93
DamienMcKennaI'd like to see this patch committed somewhere so that we can reroll all the other RTBC issues.
Comment #94
colanDon't we need to include #89 in the patch first though, or do we want to do these separately in the new branch?
Comment #95
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commentedHere's both patches merged.
Comment #96
dgtlmoon CreditAttribution: dgtlmoon commentedFilename scares my build process
Comment #97
Dave ReidI'm still confused why we're adding a major dependency change in a stable 1.x branch. We should be doing this in a 7.x-3.x branch.
Comment #100
Dave ReidI created a new 7.x-3.x branch and reverted the drafty test dependency from the 7.x-1.x branch. I'll assign this issue to the branch once it becomes available.
Comment #101
Dave ReidWill need to be re-rolled because the update hook should be workbench_moderation_update_7300() now.
Comment #102
Dave ReidComment #103
dgtlmoon CreditAttribution: dgtlmoon commented@davereid that's very heavy handed to just move this to 3.x branch without explaining why you don't understand, not really feeling the love here
Comment #104
stefan.r CreditAttribution: stefan.r commentedYeah that's unfortunate, would have been great to have it already widely installed on 1.x. How close is 3.x to being as stable as 1.x?
On the 1.x branch can we make it optional so it's enabled for new installs and disabled (but can be opted in to) for existing installs?
Comment #105
Dave ReidNew dependencies should not just be added in a branch after it has a stable release. That kind of major change warrants new branches, especially if those changes are not optional. It has been discussed several times already in this issue that we should move this to a new branch, so I was acting on that.
Comment #106
DamienMcKennaSo, an update script like this?
Comment #107
dgtlmoon CreditAttribution: dgtlmoon commentedWhilst testing this #2730265: Try to avoid saving a new revision when restoring the published revision
yeah so it seems it's probably well best to keep this code away from the 1.x branch, this could still break things
Comment #108
dgtlmoon CreditAttribution: dgtlmoon commentedComment #109
dgtlmoon CreditAttribution: dgtlmoon commenteddrafty maintainer says that a double save and race conditions are unavoidable as it currently stands with the drafty module.
where does this leave the work here? it does not resolve the original issue as stated by the owner
Comment #110
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commented@dgtlmoon
I don't get what you're saying in #107 and I don't get why you're making this a needs work in #108. Please explain.
With using Drafty, things are a bit better than without Drafty, because we avoid register_shutdown_function(). There are still two entity_update happening. That's true for both branches.
As for the stability of 3.x, it is exactly the same as 1.x because we branch from 1.x, and there is only this change in it. The question is how quick we can deprecate 1.x and document the upgrade, so the community can focus on 3.x branch.
Comment #111
Dave ReidWIthout any benefit around the double entity-saving, I'm not sure this is worth the effort at this point, especially in fragmenting work and efforts.
Comment #112
dgtlmoon CreditAttribution: dgtlmoon commented@Miroslavbanov see the issue linked iny comment above. It is based on what the drafty maintainer says. There is simply nothing for me to explain "This is unavoidable with forward revisions in 7.x. Workbench,the version of cps before drafty, and other modules were doing this logic in hook_exit() or directly updating the node table both of which lead to data integrity issues or race conditions.
If someone can come up with a non-race-condition alternative that'd be great, but you have to save two revisions and core doesn't have an Api for saving only to the revision tables."
if you read this issues original description you can see this issue has wandered somewhat
Comment #113
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commentedDoes this patch cause Race conditions? Does it resolve Race conditions? I can't say for sure. I think it helps resolve some of the linked issues that are about batch/bulk updates, and the migration module specifically. The issues seem to be more about data integrity and consistency than about performance.
Comment #114
stefan.r CreditAttribution: stefan.r commentedThen the plan in #83 sounds great :)
Comment #115
catchThe double entity_save() is unavoidable in Drupal 7. It's fixed in Drupal 8 but that doesn't help 7.x users and it's not backportable.
I first suggested the drafty forward revision approach about six years ago in a Drupal 7 core forward revisions issue (that got derailed). To not do that, you'd need to be able to save an entity without touching the main table, which there is no API for - neither for entities themselves or for field API.
The difference with drafty and workbench currently is not doing the logic in hook_exit(). The hook_exit() causes race conditions and data-loss, and is why busy sites get corrupted data and field_collection gets broken. If you look at drafty and CPS's code, it has zero special casing for field_collection, only a couple of workarounds for entity_translation and title module bugs/limitations. This is what the issue summary identifies and is a valid reason for the rewrite. I wrote drafty specifically to address similar race conditions in CPS for a Tag1 client - it was using the hook_exit() approach before that.
To summarize, there are three ways to do this, all Drupal 7 workflow modules do some version of these:
1. drafty's method - no race condition, but double save.
2. Save in hook_exit() - race condition and double save.
3. Manual queries on the node table - race conditions, breaks other modules worse than #1 and #2 and no workaround because it's bypassing APIs altogether.
I'm not currently working on the project that's using CPS, nor any sites using workbench, so I have limited time to help with this. #2379071: Use drafty module for draft revision handling is the equivalent CPS issue if people want more background.
Comment #116
catchComment #117
catchAlso this is a data loss bug, not a feature request.
Comment #118
nikolabintev CreditAttribution: nikolabintev commentedIt isn't in 7.x-3.x yet, and it doesn't apply anymore to latest 7.x-3.x. Needs re-roll.
Comment #119
gnucifer CreditAttribution: gnucifer commentedI gave it a shot, re-roll against current 7.x-1.x (52ff3ad818c62a4e5bd9d7e8ad420e22f2ff5e74).
Comment #120
gnucifer CreditAttribution: gnucifer commentedComment #121
gnucifer CreditAttribution: gnucifer commentedForgot to include changes in #106, hold on.
Comment #122
gnucifer CreditAttribution: gnucifer commentedComment #123
gnucifer CreditAttribution: gnucifer commentedI just reviewed the re-rolled patch again and it seems like I botched the merge and some changes are missing. Please don't use the new patch! I don't have time to re-roll it again right now, but if not any one else beats me to it I will give it another go on monday.
Comment #124
DamienMcKennaThe patch also needs to be rerolled against 7.x-3.x rather than 7.x-1.x.
Comment #125
gnucifer CreditAttribution: gnucifer commentedOk, rerolled against 7.x-3.x. I think I got it right this time.
Comment #126
gnucifer CreditAttribution: gnucifer commentedComment #127
DamienMcKennaCan one of the project's maintainers please enable automated testing for the 7.x-3.x branch? Thanks.
Comment #128
Dave ReidEnabled, need to have the patch re-uploaded to test.
Comment #129
DamienMcKennaRe-uploading gnucifer's patch fro #125.
Comment #130
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commentedOK, let's get this patch in. I have been using this since #21, and I am now using it on a new project as well.
Comment #131
stefan.r CreditAttribution: stefan.r commentedAfter this patch, would anything be blocking a 7.x-3.0 still?
Comment #132
dgtlmoon CreditAttribution: dgtlmoon commentedTo the maintainers - please put some really informative text on the project page about exactly what is in the 3.x branch once this is merged in
Comment #133
DamienMcKennaBump.
Comment #135
das-peter CreditAttribution: das-peter at Cando commentedAs this was essentially stuck I decided to commit and push it despite the agreement I had to only touch the 2.x branch.
I did my best to give proper credit.
Comment #136
DamienMcKennaIt was @agentrickard who did all the work, he's due all the credit, but thanks.
Comment #137
das-peter CreditAttribution: das-peter at Cando commented@DamienMcKenna Damn, I was afraid that I missed something :( - thanks for clarifying
@agentrickard sorry about that! :|
Comment #138
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commented#137: You can still tick the right checkbox and revert the commit, then redo it with proper credit - if you want to.
That is what we do in core sometimes.
Comment #140
agentrickardThanks, everyone, for pushing this forward.
Comment #141
smustgrave CreditAttribution: smustgrave commentedI'm not sure if this is the right thread but I was wondering what is the benefit of Drafty's forward revisioning? Why does it need to save twice?
Comment #142
MiroslavBanov CreditAttribution: MiroslavBanov as a volunteer commented@smustgrave
It saves twice with or without Drafty.
See also: https://www.drupal.org/node/2376839#comment-11214317 for details.