Problem/Motivation

\Drupal\Tests\workspaces\Kernel\EntityWorkspaceConflictConstraintValidatorTest defines method reloadEntity() that is identical to \Drupal\Tests\EntityTrait::reloadEntity().

To avoid code duplication EntityWorkspaceConflictConstraintValidatorTest should use EntityTrait.

@see https://www.drupal.org/project/drupal/issues/3560718#comment-16526234

Proposed resolution

Use EntityTrait in EntityWorkspaceConflictConstraintValidatorTest and remove method reloadEntity from the test.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3582184

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

stborchert created an issue. See original summary.

stborchert’s picture

stborchert
he/him
Primary language German
commented
Status: Active » Needs review
joachim’s picture

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

This needs an actual MR to be made, but happy to RTBC the changes in the branch.

stborchert opened merge request !15280

stborchert’s picture

stborchert
he/him
Primary language German
commented

The MR is created now (https://git.drupalcode.org/project/drupal/-/merge_requests/15280) but gitlab seems to be very slow right now. It took roughly 15min until I could see it after creation 🤔

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Reviewed & tested by the community » Needs review

one test failed

stborchert’s picture

stborchert
he/him
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

@andypost This has been a random test failure. Restarted the job and it succeeded.

amateescu’s picture

amateescu commented
Category: Feature request » Task
Status: Reviewed & tested by the community » Needs work

I checked locally and that test passes if we remove all the reloadEntity() calls, probably thanks to various improvements that were done in Workspaces since it was added, so let's do that instead.

stborchert’s picture

stborchert
he/him
Primary language German
commented
Status: Needs work » Needs review

As per comment 8 I've removed calls to reloadEntity() completely from the test. Still works so either it isn't required anymore or the test does something wrong 😉

smustgrave’s picture

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

Believe feedback for this one has been addressed

mstrelan’s picture

mstrelan commented
Status: Reviewed & tested by the community » Needs work

Title and issue summary update needed