We should re-use the core PluginFormInterface for facet sources instead of defining our own.

CommentFileSizeAuthor
#5 refactor_facet_source-2610084-5.patch10.53 KBborisson_
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

StryKaizer created an issue. See original summary.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented

Because the ::buildConfigurationForm method on the FacetSourceInterface specifies more arguments than the one on PluginFormInterface we can't extend that interface.

StryKaizer’s picture

StryKaizer
Primary language Dutch
Location Belgium
CreditAttribution: StryKaizer commented

As discussed today at drupalcamp gent, we should add the facet in the base class in the constructor. For an example, see searchapi, they do the same in their processor base class.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented
Title: Refactor facet source interface to use the core pluginforminterface » Refactor facet source interface to use the core PluginFormInterface
Version: » 8.x-1.x-dev
Priority: Minor » Normal
Related issues: +#2648260: Add a "display" plugin type to list all known search pages

Very much in favor of this, this makes the interface easier and will make #2648260: Add a "display" plugin type to list all known search pages easier.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented
Status: Active » Needs review
FileSize
refactor_facet_source-2610084-5.patch10.53 KB

Tests should pass with these changes.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented
Status: Needs review » Fixed

  • borisson_ committed 3ac6b33 on 8.x-1.x 
    Issue #2610084 by borisson_, StryKaizer: Refactor facet source interface...

Status: Fixed » Closed (fixed)

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