Problem/Motivation

Condition plugins were added to core before PluginFormInterface existed.
Its implementation of getFormId() is hacky and unnecessary.
Conditions have no real need to be configured on their own, the configuration has to be stored somewhere else.

Additionally, conditions are configurable, and should implement ConfigurablePluginInterface accordingly.

Proposed resolution

Update the interfaces

Remaining tasks

N/A

User interface changes

N/A

API changes

ConditionInterface implements PluginFormInterface instead of FormInterface, and also ConfigurablePluginInterface.

CommentFileSizeAuthor
#5 interdiff.txt2.37 KBtim.plunkett
#5 condition-plugin-forms-2268801-5.patch8.96 KBtim.plunkett
#3 condition-plugin-forms-2268801-3.patch7.96 KBtim.plunkett
#1 condition-plugin-forms-2268801-1.patch6.33 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Active » Needs review
FileSize
condition-plugin-forms-2268801-1.patch6.33 KB

Status: Needs review » Needs work

The last submitted patch, 1: condition-plugin-forms-2268801-1.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
condition-plugin-forms-2268801-3.patch7.96 KB

Ah, there's that test coverage!

EclipseGc’s picture

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

This seems straight forward and obvious.

Eclipse

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Title: Condition plugins should use PluginFormInterface instead of FormInterface » Update ConditionInterface to use recently added plugin interfaces
Issue summary: View changes
Status: Reviewed & tested by the community » Needs review
FileSize
condition-plugin-forms-2268801-5.patch8.96 KB
interdiff.txt2.37 KB

Hmm, as much as it pains me to expand the scope, I'd rather do that than have another tiny issue.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit e27542b on 8.x by webchick: 
    Issue #2268801 by tim.plunkett: Update ConditionInterface to use...
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Issue tags: +Page Manager

Status: Fixed » Closed (fixed)

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