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.

Comments

Erik Seifert created an issue. See original summary.

erik seifert’s picture

StatusFileSize
new1.97 KB
erik seifert’s picture

Issue summary: View changes
erik seifert’s picture

StatusFileSize
new4.69 KB

Add new validation for DefaultSelection

erik seifert’s picture

StatusFileSize
new4.76 KB

Remove notices.

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

StatusFileSize
new4.76 KB

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

Add (1) and (2).

jeroent’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new2.15 KB
new6.91 KB

Wrote a test for this issue.

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 work » Needs review
StatusFileSize
new1.23 KB
new7.78 KB
new2.15 KB

Undid the UI changes.

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?

ginovski’s picture

Added in the experimental and change some descriptions.

Screenshot:

ginovski’s picture

Renamed config setting negotate -> negate

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

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new22.96 KB
new7.42 KB

#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
StatusFileSize
new1.77 KB
new7.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

Status: Needs work » Needs review
StatusFileSize
new2.42 KB

Adding buildEntityQuery() and validate.

Status: Needs review » Needs work

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

ginovski’s picture

Status: Needs work » Needs review
StatusFileSize
new4.15 KB
new5.53 KB

Fixed methods and extended tests.

ginovski’s picture

Adding test-only patch.

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.