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

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
StatusFileSize
new 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
commented
Status: Needs work » Needs review
StatusFileSize
new condition-plugin-forms-2268801-3.patch7.96 KB

Ah, there's that test coverage!

eclipsegc’s picture

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
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
StatusFileSize
new condition-plugin-forms-2268801-5.patch8.96 KB
new 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 Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
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
commented
Issue tags: +Page Manager

Status: Fixed » Closed (fixed)

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