Problem/Motivation

The parent field is a problem currently as it is loaded only explicitly and in hasTranslationChanges when we load original the parent field will be always empty.

Additionally the parent field has the problem that on TermForm submit the root might be selected which means that the first item will have target_id=0 but when we load the parents of the original the target_id=0 item will not be present as TermStorage::loadParents loads only entities.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#5 2843060.patch833 bytesamateescu
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hchonov created an issue. See original summary.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now 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.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now 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.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Active » Needs review
FileSize
2843060.patch833 bytes

Since #2543726: Make $term->parent behave like any other entity reference field, to fix REST and Migrate support and de-customize its Views integration landed and made the parent field behave like a regular field, I think this piece of code is no longer needed.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Since this is just a protected method, we can remove this. The tests agree with removing it.

Deleting code is the best, great work @amateescu!

hchonov’s picture

hchonov
Primary language Bulgarian
Location 🇪🇺🇩🇪🇧🇬
CreditAttribution: hchonov at bio.logis Genetic Information Management GmbH commented

@amateescu++

+1 for the RTBC

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Thunder commented
Status: Reviewed & tested by the community » Fixed

Not surprising that there's a few stragglers like this left in the code. And thinking about it adding tests is a bit funny here because we'd be testing for absence of this code.

Also removing this method is fine because it is implement on the parent class so there is no API changes.

  • alexpott committed 2d0c3eb on 8.6.x 
    Issue #2843060 by amateescu: Term field "parent" could not be handled...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and at Thunder commented

Committed 2d0c3eb and pushed to 8.6.x. Thanks!

Status: Fixed » Closed (fixed)

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