Problem/Motivation

Line 94, in the method body, not in a conditional block:

    $entity = !empty($value->getParent()) ? $value->getEntity() : NULL;

Line 126:

      if ($value->getParent() && ($entity = $value->getEntity()) && !$entity->isNew()) {

We've already determined $value->getParent() and assigned $entity in that case. So we can now just do:

 if ($entity && !$entity->isNew())

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3398400-1.patch1.05 KBpradhumanjain2311

Issue fork drupal-3398400

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

joachim created an issue. See original summary.

pradhumanjain2311’s picture

pradhumanjain2311 commented
Status: Active » Needs review
StatusFileSize
new 3398400-1.patch1.05 KB

Made changes as requested.
Please review.

joachim’s picture

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

Looks good. Thanks!

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

sourabhjain made their first commit to this issue’s fork.

sourabhjain opened merge request !5413

sourabhjain’s picture

sourabhjain
Primary language English
commented
Status: Needs work » Needs review
smustgrave’s picture

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

As mentioned in another ticket(s)

Thanks @sourabhjain. Note that converting a patch to a merge request without other contributions to the issue does not receive credit. Also, I wonder if maybe you could work on some more advanced contributions rather than novice ones? Based on your profile, you are ready for that. 🙂 It's best to leave novice-tagged issues for those who are new to the contribution process.

Reroll to MR seems fine and since it was previously RTBC going to restore.

xjm’s picture

xjm
she/her
Primary language English
commented
Title: needless repeated calls in ValidReferenceConstraintValidator » Clean up repeated method calls in ValidReferenceConstraintValidator

Also remember to hide the patch files when converting to an MR. Thanks!

  • xjm committed 42c2676d on 11.x 
    Issue #3398400 by pradhumanjain2311, joachim, smustgrave: Clean up...

  • xjm committed 35b776fe on 10.2.x 
    Issue #3398400 by pradhumanjain2311, joachim, smustgrave: Clean up...

  • xjm committed a9323c4f on 10.1.x 
    Issue #3398400 by pradhumanjain2311, joachim, smustgrave: Clean up...
xjm’s picture

xjm
she/her
Primary language English
commented
Version: 11.x-dev » 10.1.x-dev
Status: Reviewed & tested by the community » Fixed

I had to read the code in the IS a couple times -- well spotted.

Crediting @joachim for the initial issue report, @pradhumanjain2311 for the patch, and @smustgrave for mentoring. Not crediting the MR conversion without other fixes, as per the above.

Committed to 11.x, 10.2.x, and 10.1.x as a code cleanup with no behavior or functionality change. Thanks!

Status: Fixed » Closed (fixed)

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