Problem/Motivation

Follow-up from #3401730: Add default autoconfigure to all *.services.yml and remove event_subscriber tags

jsonapi.resource_response_validator.subscriber sets a priority and was considered not eligible for autoconfiguration.

However, priority on event subscribers is meaningless; priority is configured in the getSubscribedEvents() method.

Steps to reproduce

Proposed resolution

Enable autoconfiguration and remove all event_subscriber tags in jsonapi.services.yml.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3401993

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

kim.pepper created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Personally I think this is the wrong fix and the error is in jsonapi.services.yml; I don't see how the priority of jsonapi.resource_response_validator.subscriber has any effect.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: [PP-1] AutoconfigurationTest should allow 'event_subscriber' tags if a priority is set » AutoconfigurationTest should allow 'event_subscriber' tags if a priority is set
Status: Postponed » Active

longwave opened merge request !6656

longwave’s picture

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

If this passes then this issue needs rescoping as we can simply enable autoconfiguration in JSON:API. RegisterEventSubscribersPass doesn't look at the priority of the tag at all.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: AutoconfigurationTest should allow 'event_subscriber' tags if a priority is set » Enable autoconfiguration for JSON:API
Component: base system » jsonapi.module
Issue summary: View changes
Status: Needs work » Needs review
smustgrave’s picture

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

Verified all { name: event_subscriber } have been replaced in the jsonapi.services file.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

Yeah, I think if the priority isn't actually used,and tests pass without it then we are good. RTBC+1

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Enable autoconfiguration for JSON:API » Enable services autoconfiguration for JSON:API
Issue tags: +clean-up

I thought this was about jsonapi.settings or something 😇 Changed title.

Nice simplification/clean-up! 😄

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 6c9abc44c4 to 11.x and 9fa4a048e5 to 10.3.x. Thanks!

  • alexpott committed 9fa4a048 on 10.3.x 
    Issue #3401993 by longwave: Enable services autoconfiguration for JSON:...

  • alexpott committed 6c9abc44 on 11.x 
    Issue #3401993 by longwave: Enable services autoconfiguration for JSON:...

Status: Fixed » Closed (fixed)

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