Motivation

We heavily use paragraphs and features. But now it is not possible to add a new feature with a paragraph without changing the field configuration.

Example

Types:
- Row
- Column
- Teaser
- Image
- .....

Row can only include Column.
Column can include all Types except column and row.

Solution

Add a option in ParagraphSelection for exclude types.

CommentFileSizeAuthor
#45 exclude_types-2856901-45.patch5.53 KBGinovski
#45 exclude_types-2856901-45-test-only.patch3.04 KBGinovski
#44 add_a_possibility_for-2856901-44.patch5.53 KBGinovski
#44 interdiff-2856901-42-44.txt4.15 KBGinovski
#42 exclude_types-2856901-42.patch2.42 KBGinovski
#38 add_a_possibility_for-2856901-38.patch7.4 KBGinovski
#38 interdiff-2856901-36-38.txt1.77 KBGinovski
#36 add_a_possibility_for-2856901-36.patch7.42 KBGinovski
#36 allowed-exclude-types.png22.96 KBGinovski
#35 add_a_possibility_for-2856901-35.patch7.49 KBGinovski
#32 add_a_possibility_for-2856901-32.patch11.37 KBGinovski
#32 interdiff-2856901-31-32.txt6.31 KBGinovski
#31 exclude-include-types.png20.24 KBGinovski
#31 add_a_possibility_for-2856901-31.patch11.4 KBGinovski
#31 interdiff-2856901-18-31.txt4.37 KBGinovski
#2 2856901-provide-exclude-option.patch1.97 KBErik Seifert
#4 2856901-provide-exclude-option.patch4.69 KBErik Seifert
#5 2856901-provide-exclude-option-5.patch4.76 KBErik Seifert
#10 2856901-provide-exclude-option-10.patch4.76 KBErik Seifert
#11 add_a_possibility_for-2856901-11-test-only.patch2.15 KBJeroenT
#11 add_a_possibility_for-2856901-11.patch6.91 KBJeroenT
#12 add_a_possibility_for-2856901-12.patch7.42 KBJeroenT
#12 interdiff-2856901-11-12.txt1.19 KBJeroenT
#18 interdiff-2856901-12-18.txt1.23 KBJeroenT
#18 add_a_possibility_for-2856901-18.patch7.78 KBJeroenT
#18 add_a_possibility_for-2856901-18-test-only.patch2.15 KBJeroenT
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Erik Seifert created an issue. See original summary.

Erik Seifert’s picture

Erik Seifert’s picture

Issue summary: View changes
Erik Seifert’s picture

Erik Seifert’s picture

johnchque’s picture

Not sure what you want to do exactly, but did you try with disabling all, as the help text says, "All disabled is all enabled".

Erik Seifert’s picture

No, i want to exclude some types and allow the remaining..

Erik Seifert’s picture

Issue summary: View changes
Ginovski’s picture

Status: Active » Needs work
Issue tags: +Needs tests
  1. +++ b/src/Plugin/EntityReferenceSelection/ParagraphSelection.php
    @@ -50,6 +51,15 @@ class ParagraphSelection extends DefaultSelection {
    +      '#title' => $this->t('Exclude paragraphs type'),
    

    Maybe "Choose allowed paragraph types" or "Choose paragraph types" would be more proper, since the exclude might mix with the include option and would not make sense.

  2. +++ b/src/Plugin/EntityReferenceSelection/ParagraphSelection.php
    @@ -50,6 +51,15 @@ class ParagraphSelection extends DefaultSelection {
    +    $form['negotate'] = array(
    

    array() -> []

  3. Needs test coverage.
Erik Seifert’s picture

For test i need more time. A little help would be nice ;- )

Add (1) and (2).

JeroenT’s picture

JeroenT’s picture

Added the negotate item to the schema + made some changes to the backend screen. I changed the select to a checkbox. What do you think?

The last submitted patch, 11: add_a_possibility_for-2856901-11-test-only.patch, failed testing.

The last submitted patch, 11: add_a_possibility_for-2856901-11-test-only.patch, failed testing.

The last submitted patch, 11: add_a_possibility_for-2856901-11.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 12: add_a_possibility_for-2856901-12.patch, failed testing.

miro_dietiker’s picture

Please always add screenshots for UI change proposals. It allows much easier UX review.

JeroenT’s picture

Status: Needs review » Needs work

The last submitted patch, 18: add_a_possibility_for-2856901-18-test-only.patch, failed testing.

JeroenT’s picture

Status: Needs work » Needs review

The last submitted patch, 4: 2856901-provide-exclude-option.patch, failed testing.

The last submitted patch, 4: 2856901-provide-exclude-option.patch, failed testing.

The last submitted patch, 2: 2856901-provide-exclude-option.patch, failed testing.

The last submitted patch, 2: 2856901-provide-exclude-option.patch, failed testing.

The last submitted patch, 5: 2856901-provide-exclude-option-5.patch, failed testing.

The last submitted patch, 5: 2856901-provide-exclude-option-5.patch, failed testing.

The last submitted patch, 10: 2856901-provide-exclude-option-10.patch, failed testing.

The last submitted patch, 10: 2856901-provide-exclude-option-10.patch, failed testing.

miro_dietiker’s picture

Before adding more configurability to Paragraphs that is not simply catching up with core patterns, we should define the final goal of how we want to manage paragraph types.

That makes it related to the paragraph type grouping we are also introducing:
#2831762: Add grouping for Paragraph types in "Modal" add mode

People asked to define sets of paragraph types and only allow that set, so adding a new type to a set doesn't require editing every single other paragraph type adding that new type... Others are proposing better tools for batch updates.
See also #2504583: A way to select between predefined sets of paragraphs?

miro_dietiker’s picture

Ginovski’s picture

Added in the experimental and change some descriptions.

Screenshot:

Ginovski’s picture

miro_dietiker’s picture

Status: Needs review » Needs work
  1. +++ b/src/Plugin/EntityReferenceSelection/ParagraphSelection.php
    @@ -154,4 +164,77 @@ class ParagraphSelection extends DefaultSelection {
    +        $query->condition($entity_type->getKey('id'), NULL, '=');
    

    I don't think this is correct.
    (BTW some more comments in the logic would help read it faster.)

    If no paragraph type is selected, thus it seems getAllowedTypes() returns emptyness now, still all should be allowed.

    Is this selection thing well test covered or am i wrong?

  2. +++ b/src/Plugin/Field/FieldWidget/InlineParagraphsWidget.php
    @@ -707,6 +707,10 @@ class InlineParagraphsWidget extends WidgetBase {
    +      if ($this->getSelectionHandlerSetting('negate')) {
    +        $bundles = \Drupal::service('entity_type.bundle.info')->getBundleInfo($target_type);
    +        $bundles = array_diff_key($bundles, $this->getSelectionHandlerSetting('target_bundles'));
    
    +++ b/src/Plugin/Field/FieldWidget/ParagraphsWidget.php
    @@ -684,6 +684,10 @@ class ParagraphsWidget extends WidgetBase {
    +      if ($this->getSelectionHandlerSetting('negate')) {
    +        $bundles = \Drupal::service('entity_type.bundle.info')->getBundleInfo($target_type);
    +        $bundles = array_diff_key($bundles, $this->getSelectionHandlerSetting('target_bundles'));
    

    I'm very unhappy we need to put this type of setting into the widget.

    Instead, a heper method should just interprete the setting and provide us the list of allowed paragraph types. Like getAllowedBundles().

Ginovski’s picture

Created an issue #2881145: Get allowed paragraph types from ParagraphSelection, this can wait for that

Ginovski’s picture

Ginovski’s picture

#2881145: Get allowed paragraph types from ParagraphSelection is committed.
So adding a patch with the negate setting. Works properly atm.
Screenshot of the form:

Primsi’s picture

Status: Needs review » Needs work
  1. +++ b/config/schema/paragraphs_type.schema.yml
    @@ -37,6 +37,8 @@ paragraphs.behavior.settings.*:
    +      type: boolean
    

    Wouldn't this be integer? Because I think if its bool, radios would interprete false as not selected.

  2. +++ b/src/Plugin/EntityReferenceSelection/ParagraphSelection.php
    @@ -50,6 +51,18 @@ class ParagraphSelection extends DefaultSelection {
    +      '#options' => [
    +        1 => 'All except those selected',
    +        0 => 'None except those selected'
    

    Options are not translated.

    Also I am not sure about those options. Maybe like that (not really happy with those either)?
    1 All selected below
    0 All, except the ones selected below

  3. +++ b/src/Plugin/EntityReferenceSelection/ParagraphSelection.php
    @@ -50,6 +51,18 @@ class ParagraphSelection extends DefaultSelection {
    +      '#return_value' => 1,
    

    I am not really familiar with return_value. What does this do?

  4. +++ b/src/Plugin/EntityReferenceSelection/ParagraphSelection.php
    @@ -50,6 +51,18 @@ class ParagraphSelection extends DefaultSelection {
    +      '#default_value' => isset($selection_handler_settings['negate']) ? $selection_handler_settings['negate'] : FALSE,
    

    Hm, if we change the schema to int, we might want to fallback to 0 here.

  5. +++ b/src/Plugin/EntityReferenceSelection/ParagraphSelection.php
    @@ -168,7 +181,12 @@ class ParagraphSelection extends DefaultSelection {
    -      $bundles = array_intersect_key($bundles, $this->configuration['handler_settings']['target_bundles']);
    +      if (isset($this->configuration['handler_settings']['negate']) && $this->configuration['handler_settings']['negate'] == '1') {
    +        $bundles = array_diff_key($bundles, $this->configuration['handler_settings']['target_bundles']);
    

    So if we make this a int, will it still evaluate to '1'?

Ginovski’s picture

Assigned: Unassigned » Ginovski
Status: Needs work » Needs review
FileSize
1.77 KB
7.4 KB

Addressed #37.
For the descriptions I used:
Exclude the selected below
Include the selected below

  • miro_dietiker committed 0a6e032 on 8.x-1.x authored by Ginovski
    Issue #2856901 by Ginovski, JeroenT, Erik Seifert, miro_dietiker: Add a...
miro_dietiker’s picture

Status: Needs review » Needs work

Committed. :-)

But i noticed that the base of ParagraphSelection is DefaultSelection that implements many methods that are not overridden. There are methods like getReferenceableEntities() countReferenceableEntities() buildEntityQuery() and even createNewEntity() that keep the old (at least not negated) behavior. Where are those methods used?
Needs work for discussion and then to be closed.

Berdir’s picture

Yes, this was too early, it's has incomplete test coverage. Validation doesn't work and entities can't be saved.

Not sure if it's worth to revert, @Ginovski is working on improving it.

Ginovski’s picture

Adding buildEntityQuery() and validate.

Status: Needs review » Needs work

The last submitted patch, 42: exclude_types-2856901-42.patch, failed testing.

Ginovski’s picture

Fixed methods and extended tests.

The last submitted patch, 45: exclude_types-2856901-45-test-only.patch, failed testing. View results

The last submitted patch, 45: exclude_types-2856901-45-test-only.patch, failed testing. View results

  • miro_dietiker committed 5559a51 on 8.x-1.x authored by Ginovski
    Issue #2856901 by Ginovski, JeroenT, Erik Seifert, miro_dietiker: Add a...
miro_dietiker’s picture

Committed, thx. :-)

miro_dietiker’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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