Follow up for #1810370: Entity Translation API improvements

Updated: Comment #0

Problem/Motivation

some docs missing or mismatched, like @param, @return

mis-spelling and whitespace

Proposed resolution

added and matched

Remaining tasks

TBD

User interface changes

No.

API changes

No.

#1810370: Entity Translation API improvements

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YesCT’s picture

Status: Active » Needs review
  1. +++ b/core/includes/entity.api.phpundefined
    @@ -241,6 +241,35 @@ function hook_entity_update(Drupal\Core\Entity\EntityInterface $entity) {
    + * @param \Drupal\Core\Entity\EntityInterface $entity
    + *   The original entity object.
    + */
    +function hook_entity_translation_delete(\Drupal\Core\Entity\EntityInterface $translation) {
    

    @param does not match.

  2. +++ b/core/lib/Drupal/Core/Entity/Entity.phpundefined
    @@ -294,10 +295,13 @@ public function language() {
        * Implements \Drupal\Core\TypedData\TranslatableInterface::getTranslation().
    +   *
    +   * @return \Drupal\Core\Entity\EntityInterface
        */
    -  public function getTranslation($langcode, $strict = TRUE) {
    +  public function getTranslation($langcode) {
    

    the parameter list changed here. but there are no @param's. I checked:

    class Entity implements IteratorAggregate, EntityInterface {

    and

    interface EntityInterface extends ComplexDataInterface, AccessibleInterface, TranslatableInterface {

    and

    the docs on the TranslatableInterface getTranslation() will work. the return is not as specific, but it's still accurate and I think that is what we are doing. (I asked @fubhy)

    changing this to inheritdoc.

  3. +++ b/core/lib/Drupal/Core/Entity/EntityFormController.phpundefined
    @@ -489,11 +492,23 @@ public function buildEntity(array $form, array &$form_state) {
    +    // Ensure that the entity object is a BC entity if the original one is.
    +    return $this->entity instanceof EntityBCDecorator ? $translation->getBCEntity() : $translation;
    

    missing @return. added it.

  4. Some spelling fixes and whitespace fixes.
  5. +++ b/core/lib/Drupal/Core/Entity/EntityNG.phpundefined
    @@ -369,77 +478,182 @@ public function onChange($property_name) {
    +   * @return \Drupal\Core\Entity\EntityInterface
    +   */
    +  public function getTranslation($langcode) {
    +    // Ensure we always use the default language code when dealing with the
    

    corrects the @return, but is missing the @param Added it.

plach’s picture

Status: Needs review » Needs work

Thanks :)

+++ b/core/lib/Drupal/Core/Entity/Entity.php
@@ -294,9 +294,7 @@ public function language() {
-   * Implements \Drupal\Core\TypedData\TranslatableInterface::getTranslation().
-   *
-   * @return \Drupal\Core\Entity\EntityInterface
+   * {@inheritdoc}

Please revert this, it's meant to cast the return type from TranslatableInterface to EntityInterface.

+++ b/core/lib/Drupal/Core/Entity/EntityNG.php
@@ -478,7 +478,12 @@ public function onChange($property_name) {
+   *   The translation.

In other parts of the docs we refer to it as to the "The translation object".

YesCT’s picture

Assigned: Unassigned » YesCT

OK. I was wondering what to do when the docs should be the same, but the type of the return thing is known to be more specific.
Will add the @param then too.

YesCT’s picture

Assigned: YesCT » Unassigned
Status: Needs work » Needs review
FileSize
2.6 KB
6.48 KB

Ah. I had noted 1. in #1 but not done it. It's done now.

Fixed the things from #2.

--

I didn't change missing docs in the BC decorator, since that will be pulled out.

--

One more grammar fix for "Checks translation statuses and invoke the ..." to invokes.

plach’s picture

Status: Needs review » Needs work

Sorry, I had missed some stuff:

+++ b/core/lib/Drupal/Core/Entity/Entity.php
@@ -296,7 +296,14 @@ public function language() {
+   * Casts the return type from TranslatableInterface to EntityInterface.

I am not sure we should write inline comments for the PHP docs themselves :)

+++ b/core/lib/Drupal/Core/Entity/Entity.php
@@ -296,7 +296,14 @@ public function language() {
+   *   The called object for chaining.

This should be something like: "The translation object corresponding to the specified language. A new translation is created if missing."

+++ b/core/lib/Drupal/Core/Entity/EntityFormController.php
@@ -500,6 +500,9 @@ public function getEntity() {
+   *   The translation.

The translation object.

+++ b/core/lib/Drupal/Core/TypedData/TranslatableInterface.php
@@ -79,6 +78,7 @@ public function hasTranslation($langcode);
+   *   The unsaved translated object.

translation object

fgm’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
4.06 KB
1.42 KB

Rerolled on today's HEAD.

Also, the examples in hook_ENTITY_TYPE_translation_delete() and hook_entity_translation_delete() had become incorrect in the meantime, so updated them to work on current HEAD.

Interdiff is to the rerolled version of the patch in #4.

fgm’s picture

Forgot to apply Plach's second suggestion. The three others no longer seem to be relevant due to the changes in core wording.

plach’s picture

Status: Needs review » Needs work
+++ b/core/lib/Drupal/Core/Entity/EntityInterface.php
@@ -41,7 +41,8 @@ public function id();
+   *   The translation object corresponding to the specified language. A new
+   *   translation is created if missing.
    */

This was supposed to document the ::getTranslation() method, but it no longer applies, we can safely ignore it.

fgm’s picture

Actually, language() will really create a new Language (not translation) object if it does not exist, if we look at Entity::language(). So maybe like this ?

   *   The language object corresponding to the langcode for the entity. A new
   *   language is created if none exists yet for the langcode.
Berdir’s picture

That seems irrelevant and is an implementation detail. All you need to know is that you get a language object back.

The create new code there is for weird scenarios when a language doesn't exist anymore or so, to ensure that you always get an object back.

plach’s picture

In the translation case it was important to mention that a new entity translation is created if the langcode specified has not a corresponding translation already. In this case it's just an implementation detail, so I don't think it needs to be documented.

fgm’s picture

Then #6 is RTBC since it includes everything but that unneeded change ?

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.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.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.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.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.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.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.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.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.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.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should 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.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.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: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should 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: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

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

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

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should 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.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should 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.

penyaskito’s picture

Doc pieces from #6 that are still relevant.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Reading the comments and seems agreement in #6 is the way to go. #25 being a reroll of it.

quietone’s picture

I skimmed the issue and then read the patch. I checked that all comments have been addressed and they have been. The changes here are for grammar and coding standards. There are no changes for any of the problems or proposed resolution listed in the Issue Summary, although it was probably true when the issue was created. Other than that I think this can be committed.

I'll wait a day or two for any feedback.

  • quietone committed 72f1f1f8 on 10.1.x
    Issue #2031917 by fgm, YesCT, penyaskito, plach, smustgrave: Docs follow...
quietone’s picture

Status: Reviewed & tested by the community » Fixed

Committed 72f1f1f and pushed to 10.1.x. Thanks!

penyaskito’s picture

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

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.