Problem/Motivation

(This came up in #2860097: Ensure that content translations can be moderated independently.)

Using hook_entity_prepare_form(), instead of relying on routes (and its load_latest_revision flag) to integrate Content Moderation with entity forms. This would have the following advantages over the current approach:

  • it would support nested entity forms
  • it would support multiple unrelated entity forms on the same route

OTOH it would have the drawback of making things less explicit.

Proposed resolution

Use hook_entity_prepare_form().

Remaining tasks

TBD

User interface changes

TBD

API changes

TBD

Data model changes

TBD

CommentFileSizeAuthor
#3 2940907-3.patch6.46 KBsam152

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2865616: Add an option for EntityConverter to load the latest entity revision and fix all entity forms to use this option.

I did some research into this exactly one week ago. I talked to Content Moderation maintainer Tim Millwood.

I asked him:

11:18:13 <WimLeers> The question: why doesn't Content Moderation uses `hook_entity_prepare()` to load the desired revision (the "latest")? Why did it instead choose to change route definitions (for e.g. entity forms)? Introduced in https://www.drupal.org/node/2918286. Knowing the reasoning for that decisions/direction would be super helpful. 
11:19:25 <WimLeers> i.e. relying on a relatively "external/global" change (changing route definitions, to ensure that the upcasted params contain exactly what Content Moderation wants/needs), rather than relying on a fairly "internal" change (Content Moderation alters entity forms, therefore it can also alter which revision it loads)
11:19:50 <WimLeers> (My guess: "because form alter hooks")

He answered:

11:20:21 <timmillwood> I think this is something that actually came from Workbench Moderation.
11:21:12 <timmillwood> see http://cgit.drupalcode.org/workbench_moderation/tree/src/ParamConverter/EntityRevisionConverter.php
11:21:29 <timmillwood> So I guess my answer would be, because no one told me about a better way to do it.

So how did this come to be?

11:22:01 <WimLeers> Does something come to mind that'd be a problem if loading the desired revision actually happened in `hook_entity_prepare()`?
11:22:52 <WimLeers> Also: who would know why things were done this way in Workbench Moderation?
11:23:06 <timmillwood> Not off the top of my head, but that CR you linked to is in core, not content_moderation. So doesn't the whole of core now load the latest revision?
11:23:26 <timmillwood> crell was the goto person for workbench moderation.
11:23:54 <WimLeers> Now `git blame`ing
11:24:40 <timmillwood> becw, josephdpurcell, or larowlan might have some insight.
11:26:14 <timmillwood> plach RTBC'd and catch committed this to core, so I guess it does make sense? ;)
11:28:29 <WimLeers> It *was* a crell change: http://cgit.drupalcode.org/workbench_moderation/commit/?id=e76558ac82d6bf89e3e16dcd3313d3e774e22141 
11:28:32 <WimLeers> without an issue…
11:28:35 <WimLeers> without docs…
11:28:42 <WimLeers> without tests …
11:28:50 <WimLeers> Okay. This is as far as we'll get here.
11:28:55 <WimLeers> Thanks for your time and insight!

My guess is that it could lead to some form alters behaving incorrectly (because based on the route definition, it should be the default revision, not the latest), but that’s just a hypothetical.

Zero mentions of "prepare" in #2865616: Add an option for EntityConverter to load the latest entity revision and fix all entity forms to use this option..

It's fine if there is a good reason for this to be implemented this way, but that reason is not obvious and comes with trade-offs, so it should be documented.

sam152’s picture

sam152 commented
Status: Active » Needs review
StatusFileSize
new 2940907-3.patch6.46 KB

I think the answer is indeed: we've always done it that way! With regards to the converter issue, I suppose the idea was moving the existing utility of the CM converter to core for other modules to use, I don't think that precludes us from stepping back and asking if it's the right approach for entity forms specifically.

This shouldn't be too hard to prototype. If we're sticking with the approach of core being responsible for this (which I'm still convinced should be the case) EntityForm::init invokes entity_prepare_form right after it calls prepareEntity, so loading the latest revision in there should do nicely.

It's probably better to postpone discussing if CM should implement the hook or if core should load it for us, behind #2940575: Document the scope and purpose of pending revisions.

Status: Needs review » Needs work

The last submitted patch, 3: 2940907-3.patch, failed testing. View results

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Needs work » Postponed (maintainer needs more info)

Is this still an issue? I guess it's outdated.

amateescu’s picture

amateescu commented
Status: Postponed (maintainer needs more info) » Closed (outdated)
Related issues: +#2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing

This was fixed in #2942907: Entity system does not provide an API for retrieving an entity variant that is safe for editing.