Problem/Motivation

This is a bit of a cleanup task. When you are working with views plugins you sometimes need to see which type of display you are working in.

This is particularly useful when you need a different output format for "data" displays as opposed to "normal" displays.

I've been working on tooling for REST data displays and this is the 5th time i've needed to know if the current display was a "data" display.

I don't see a use case for having a protected getter that doesn't harbor sensitive data.

Proposed resolution

Make the getType method public and add it to the DisplayPluginInterface interface.

Remaining tasks

Create a patch and review.

User interface changes

none

API changes

DisplayPluginInterface will have to implement getType but fallbacks to DisplayPluginBase will already implement getType.

Data model changes

none

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category This is a cleanup task. Leaving getType out of DisplayPluginInterface is an omission and it doesn't need to be protected since it's just a getter.
Issue priority Major because it blocks contrib from being able to determine what type of display a plugin belongs to.

Comments

Jaesin created an issue. See original summary.

Status: Needs review » Needs work

The last submitted patch, gettype.patch, failed testing.

Jaesin’s picture

Status: Needs work » Needs review
StatusFileSize
new2.31 KB
new4.41 KB

Clean up the rest of the display plugins.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

+1 from me

jibran’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record, +Needs beta evaluation

I'm also +1 on the change but this is going to be disruptive for contrib so maybe add a BE with strong argument why we need this and maybe add a change notice as well.

dawehner’s picture

Issue summary: View changes

/me sighs

Jaesin’s picture

Priority: Normal » Major
Issue summary: View changes
Jaesin’s picture

Status: Needs work » Needs review
dawehner’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs change record, -Needs beta evaluation

@jibran
Added as much information as you can get out of this into a change record.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 2562811-3.patch, failed testing.

Status: Needs work » Needs review

dawehner queued 3: 2562811-3.patch for re-testing.

jibran’s picture

Thanks @dawehner. RTBC+1

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

...

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 2562811-3.patch, failed testing.

Status: Needs work » Needs review

dawehner queued 3: 2562811-3.patch for re-testing.

Jaesin queued 3: 2562811-3.patch for re-testing.

Jaesin’s picture

Status: Needs review » Reviewed & tested by the community

Re-tested to make sure patch still applies. Moving back to RTBC.

effulgentsia’s picture

Adding credit for reviewers, and thank you, @jibran, for flagging the need for a change record here.

  • effulgentsia committed 9de3548 on 8.0.x
    Issue #2562811 by Jaesin, dawehner, jibran: Make \Drupal\views\Plugin\...
effulgentsia’s picture

Status: Reviewed & tested by the community » Fixed

Fixing this omission makes a lot of sense. I'm sad to break contrib subclasses, but better before RC than after. Pushed to 8.0.x and published the CR.

Status: Fixed » Closed (fixed)

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