Problem/Motivation

In #3227824: Move the linkset functionality from the decoupled menus contributed module to core's system module we introduced a ApiRequestTrait . This trait is used in the LinkSet functionality and JSON:API. This should also be used in \Drupal\Tests\rest\Functional\ResourceTestBase to minimize duplicated code. It is allowed to rename the method when using the trait (as seen in core/modules/jsonapi/tests/src/Functional/JsonApiRequestTestTrait.php.

Postponed on parent issue:
#3227824: Move the linkset functionality from the decoupled menus contributed module to core's system module

Steps to reproduce

na

Proposed resolution

Refactor \Drupal\Tests\rest\Functional\ResourceTestBase to use the test trait instead of a local method.

Remaining tasks

na.

User interface changes

na.

API changes

na.

Data model changes

Release notes snippet

Issue fork drupal-3320670

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

bbrala created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Postponed

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

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Status: Postponed » Active

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

ankitv18’s picture

ankitv18 commented
Title: [PP-1] Refactor rest\Functional\ResourceTestBase to use ApiRequestTrait » Refactor rest\Functional\ResourceTestBase to use ApiRequestTrait

ankitv18 opened merge request !9059

ankitv18’s picture

ankitv18 commented
Status: Active » Needs review

Please review MR!9059

smustgrave’s picture

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

Refactor appears fine.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs review

Do we need a separate trait? Can we just say

  use ApiRequestTrait {
    makeApiRequest as request;
  }

directly in ResourceTestBase?

smustgrave’s picture

smustgrave commented

Assuming since JsonApi and LinksetController do the same that's why it was done that way.

ankitv18’s picture

ankitv18 commented

@longwave Yes we can do that and actually I was planning to do the same but as @smustgrave mentioned I had taken the reference from jsonApi and linksetController.

ankitv18’s picture

ankitv18 commented

More simpler without extra trait file
cc: @longwave @smustgrave

smustgrave’s picture

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

Haven't looked but may be worth a novice follow up to see if the other instances in jsonApi and linksetController could be refactored too?

But change here LGTM.

  • longwave committed e4d0beac on 11.x 
    Issue #3320670 by ankitv18, smustgrave, bbrala: Refactor rest\Functional...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Committed e4d0bea and pushed to 11.x. Thanks!

As it's just a cleanup I don't see the point in backporting any further. Also not sure it's worth the effort of cleaning up the JSON:API trait, and the linkset test uses this trait directly anyway. The only slightly weird case is a CKEditor5 test that uses the JSON:API trait when it could use the base trait directly.

Status: Fixed » Closed (fixed)

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