Problem/Motivation

In #3252386: Use PHP attributes instead of doctrine annotations we added support for attribute based plugin discovery.
As part of that issue we converted block and action plugins.

This issue is to convert \Drupal\views\Annotation\ViewsArgumentplugins to use Attributes.

Proposed resolution

  1. Add a class to represent the new Attribute - Example
  2. Update the plugin manager constructor to include both the attribute and annotation class names - example
  3. Convert all plugins that use the annotation to use the new attribute - example

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3425359

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Title: Convert ViewsArgument plugin discovery to attributes » Convert ViewsArgumentHanlders plugin discovery to attributes

quietone opened merge request !6894

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Status: Active » Needs work

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

sorlov’s picture

sorlov CreditAttribution: sorlov at Skilld commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

Appears to have test failures and open thread.

sorlov’s picture

sorlov CreditAttribution: sorlov at Skilld commented
Status: Needs work » Needs review

fixed

smustgrave’s picture

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

Appears all 40 instances of @ViewsArgument have been replaced.

Thread has been addressed,

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work

Added some comments to the MR.

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

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer commented
Title: Convert ViewsArgumentHanlders plugin discovery to attributes » Convert ViewsArgumentHandlers plugin discovery to attributes
Component: migration system » views.module
Status: Needs work » Needs review

Setting back to Needs review
Fixed the title and the component of the issue

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Needs review » Needs work

appears to have test failures.

sorlov’s picture

sorlov CreditAttribution: sorlov at Skilld commented
Status: Needs work » Needs review
smustgrave’s picture

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

Believe all feedback has been addressed, test failure was fixed

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work

The changes to ViewsHandlerManager need to support all handlers not just ViewsArguments. I made a suggestion abvout how to implement this and it was not implemented and there's no explanation about why not on the issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs work » Reviewed & tested by the community

#16 is wrong. Somehow looking at out of date changes. Sorry.

alexpott’s picture

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

Committed and pushed d675ece27f to 11.x and d3a13779f9 to 10.3.x. Thanks!

  • alexpott committed d3a13779 on 10.3.x 
    Issue #3425359 by sorlov, quietone, naveenvalecha, smustgrave, alexpott...

  • alexpott committed d675ece2 on 11.x 
    Issue #3425359 by sorlov, quietone, naveenvalecha, smustgrave, alexpott...

Status: Fixed » Closed (fixed)

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