Problem/Motivation

It is not clear that RevisionableInterface::getRevisionId() does not return the revision ID of the translation it is called on. ::getLatestTranslationAffectedRevisionId() must be used for that.

Proposed resolution

Improve the documentation.

CommentFileSizeAuthor
#3 drupal-getRevisionId_docs-3332667-3.patch844 bytesliam morland
#2 drupal-getRevisionId_docs-3332667-2.patch845 bytesliam morland

Issue fork drupal-3332667

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Liam Morland created an issue. See original summary.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Active » Needs review
StatusFileSize
new drupal-getRevisionId_docs-3332667-2.patch845 bytes
liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
StatusFileSize
new drupal-getRevisionId_docs-3332667-3.patch844 bytes

Fix spelling error.

Status: Needs review » Needs work

The last submitted patch, 3: drupal-getRevisionId_docs-3332667-3.patch, failed testing. View results

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Needs work » Needs review

Patch only changes comments, so the failures are not due to the patch.

joel_osc’s picture

joel_osc commented
Status: Needs review » Reviewed & tested by the community

Given then trouble that I have had dealing with this before I think that this change would benefit the community.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Shouldn't this documentation be on \Drupal\Core\Entity\RevisionableStorageInterface::getLatestRevisionId() - this would seem to return the revision ID for the entity revision you have loaded.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

Do you mean to add the documentation to RevisionableStorageInterface::getLatestRevisionId() in addition to the current change or instead of?

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.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

liam morland opened merge request !14565

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Needs work » Needs review
dcam’s picture

dcam commented
Status: Needs review » Needs work

I dug into this to try and answer the question from #8. RevisionableStorageInterface::getLatestRevisionId() is also not translation-aware. I think the text should be added to it too, particularly since it seems to be more closely related to getLatestTranslationAffectedRevisionId(). If further clarification is desired, then I suggest contacting @alexpott on Slack.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Needs work » Needs review

I have added the documentation to RevisionableStorageInterface.

smustgrave’s picture

smustgrave commented

@alexpott thoughts on the change.

dcam’s picture

dcam commented
Status: Needs review » Reviewed & tested by the community

Thank you for copying the info to the new function. I'm sorry I didn't come back to review it sooner.

I'm willing to go ahead and RTBC the MR. Based on my analysis in #13 this is the appropriate course of action.