Problem/Motivation
We want to allow moderation of an entity without going to its Edit page. That supports both alternate user workflows and user permissions (as you may want someone moderating but not editing a given entity.)
Proposed resolution
Create a free-standing form that we can inject into an entity, conditionally based on its view mode. For now, this form should contain only a select box showing the states that can be moved to and a submit button. (It will be redesigned later.) On submit, load the entity, set its state accordingly, and save it.
On the Moderation configuration form on the bundle, offer a set of checkboxes to pick which view modes the form should be embedded in.
When configuring a view mode, if that view mode has the form configured to show on it, display it as an "extra field" so that its position can be reordered.
The form should never show if the entity being displayed is the default-revision. It should only show for forward-revisions, ever, always.
Remaining tasks
Implement the above.
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | interdiff-2645490-moderation-form.txt | 3.89 KB | Crell |
| #26 | 2645490-moderation-form.patch | 54.7 KB | Crell |
Comments
Comment #2
Crell commentedTagging as release blocker, as this is the last "we're not done unless this exists" issue. (Plenty more work to do, though, but iterate!)
Comment #3
Crell commentedHm. Open question. What do we do for entities that have don't have their own display page, such as Block Content? How's this going to work? I... have no idea.
Comment #4
Crell commentedComment #5
Crell commentedComment #6
becw commentedThis form should only have accessible destination states that are different from the current moderation state.
Comment #7
Crell commentedComment #8
dawehnerWhat about providing an alternative formatter for the
moderation_statefield which renders the form? Once there, you don't need any kind of special purpose configuration UI.Comment #9
Crell commenteddawehner: Bec suggested the same thing, actually. I'm still undecided about it, as it means you have to get your formatter setup right, too. It's another moving part that can break. I don't know if it's more work to make it appear only for the right people, either.
I'm still working on building the form itself, though, so the exact method of attaching the form is still on the table and I'm happy to be convinced one way or another.
Comment #10
dawehnerSeriously, do you really install a workflow module and configure that, but at the same time you are not able to configure formatters and like hiding the author information from pages, which is almost the first thing many people do on their site.
Comment #11
Crell commentedThis is still a WIP, but it gets some code out there for review. Next step is to actually wire the form up and make it do something. :-) So far this is all plumbing.
Comment #15
dawehnerSo just a random point that this is horrible: Sorting.
Comment #16
becw commentedDo these prophesied
$statesnot need->reveal()called on them?Whitespace.
Comment #17
Crell commentedWell, I tried adding a reveal() call and it seems to pass both ways. Which is... odd. Anyway, I left in the extra reveal() calls.
New patch, now has the form showing up and doing stuff! It's butt ugly, but the functionality seems to be there.
Still todo:
* Why is testbot angry about the previous patch?
* I have a gut feeling that I need some cache information bubbling handling stuff in here somewhere, but I'm not sure exactly where. Will hopefully figure that out tomorrow by asking someone.
Comment #21
Crell commentedThe tests were right, I did have a bug! This should be fully passing now.
Comment #22
becw commentedIt was confusing to me that this output disappeared when the
workbench_moderation_controlwas displayed.Any reason to duplicate core docs here? Maybe reference
\Drupal\Core\Entity\EntityFieldManagerInterface::getExtraFields()instead?This whole file is just moved/renamed, right? No changes in here?
Contains class name, please.
"Set" is unsettlingly short; perhaps "moderate", "apply", or "update"?
I'm not really sure why this is here. It's a convenience method, but it's only implemented and used once, so...
Comment #23
becw commentedAre you planning to provide a browser test for the form UI?
Comment #24
Crell commented1) I wasn't sure about that line; with this form, showing the field makes no sense at all but it doesn't make sense elsewhere either. Removed that line; we'll hide that field another way.
2) Mainly to simplify finding the right docs, and to keep PHP Storm happy. I'll trim it down.
3) Correct.
4) Damnit, PHPStorm!
5) Changed to Update.
6) Building a better set of primitives in the domain language. It's a routine that ought to be in core, but isn't. Having the richer primitive set makes the code easier to read elsewhere, and also lets people not think about how they'd go about doing that logic the next time they need it.
7) BTB isn't usable in core yet, so we can't or it would break testbot. :-(
Updated patch attached.
Comment #25
becw commentedBecause this is critical functionality for workbench_moderation, I think that it would be sensible to add a simpletest, even one with a limited or incomplete test case, for this form.
Your other changes are good to go, I think.
Comment #26
Crell commentedNow with 20% more Simpletest!
Comment #28
becw commentedLovely.
Comment #29
dawehnerToo bad, but IMHO this is a local maximum we went into.