Problem/Motivation

When preparing the options for parent field in the term form (TermForm), the children of the current terms are excluded from the hierarchy because a term cannot set as parent a child term. When adding a new term there are no child terms, neither parent terms but the form callback still computes them.

Proposed resolution

Don't compute children and parents of a new term on TermForm.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#9 2845361-9.patch1.35 KBclaudiu.cristea
#7 2845361-7.patch1.35 KBclaudiu.cristea
#4 2845361-4.patch1.2 KBMunavijayalakshmi
#2 2845361-2.patch1.35 KBclaudiu.cristea

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Active » Needs review
StatusFileSize
new 2845361-2.patch1.35 KB

patch.

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.

Munavijayalakshmi’s picture

Munavijayalakshmi commented
StatusFileSize
new 2845361-4.patch1.2 KB

Rerolled the patch.

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.

pfrenssen’s picture

pfrenssen
Location Sofia
commented

#4 is not a straight reroll of #2 but changes a line which seems to included in the patch for a good reason. In the patch of #4 the term ID is being added to the $exclude blacklist even if the term is new. If a term is new it does not have an ID yet so this will always return an empty value. This doesn't seem correct to me.

To me it seems that #2 is the correct patch.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
StatusFileSize
new 2845361-7.patch1.35 KB

Ok, then reposting #2 in order to be reviewed.

Status: Needs review » Needs work

The last submitted patch, 7: 2845361-7.patch, failed testing. View results

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review
StatusFileSize
new 2845361-9.patch1.35 KB

Reroll.

borisson_’s picture

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

I don't think we should add a new test for this, because it's already tested. This looks like it makes sense.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2845361-9.patch, failed testing. View results

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Reviewed & tested by the community

APCU failure

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2845361-9.patch, failed testing. View results

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Reviewed & tested by the community

Again.

  • larowlan committed abebbfb on 8.5.x 
    Issue #2845361 by claudiu.cristea, Munavijayalakshmi: Don't compute...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed as abebbfb and pushed to 8.5.x.

Thank you

Status: Fixed » Closed (fixed)

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