Problem/Motivation

Paragraphs Library items do not have a status field that makes them published/unpublished.

This is particularly useful in properly supporting content moderation module integration #2807371: META Support Content Moderation module.

Proposed resolution

  • Let LibraryItem inherit EditorialContentEntityBase which provides publishing options.
  • Write an update function to update existing sites with this change

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#10 make-paragraphs-library-items-2943601-10-interdiff.txt1004 bytesBerdir
#10 make-paragraphs-library-items-2943601-10.patch11.35 KBBerdir
#8 make-paragraphs-library-items-2943601-8-interdiff.txt5.34 KBBerdir
#8 make-paragraphs-library-items-2943601-8.patch10.69 KBBerdir
#4 make-paragraphs-library-items-2943601-4-interdiff.txt5.12 KBmbovan
#4 make-paragraphs-library-items-2943601-4.patch6.51 KBmbovan
#2 make-paragraphs-library-items-2943601-2.patch1.84 KBmbovan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mbovan created an issue. See original summary.

mbovan’s picture

mbovan CreditAttribution: mbovan at MD Systems GmbH commented
Status: Active » Needs review
FileSize
make-paragraphs-library-items-2943601-2.patch1.84 KB

A starting point-patch.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented

This was made in the Paragraph entity time ago, there is also an update function for it, think it gonna be really similar. :)

mbovan’s picture

mbovan CreditAttribution: mbovan at MD Systems GmbH commented
FileSize
make-paragraphs-library-items-2943601-4.patch6.51 KB
make-paragraphs-library-items-2943601-4-interdiff.txt5.12 KB

Adding the missing parts.

I am getting confused with the access handler:

    if ($operation === 'view' && !$library_item->isPublished()) {
      if ($account->hasPermission($this->entityType->getAdminPermission())) {
        return AccessResult::allowed();
      }
      return AccessResult::forbidden();
    }

I would except that this snippet disallows access for unpublished library items entities except for users with administer paragraphs library permission. This works fine when I try to access admin/content/paragraphs/{paragraphs_library_item_id} route directly.

However, it does not work in case this library item is embedded.

Steps to reproduce:

  1. Create a sample unpublished library item with a (published) text paragraph in it
  2. Create a node and attach a previosuly created unpublished library item paragraph
  3. Save a node
  4. Check the saved node as a non-admin user. The unpublished library item is visible. (?)
  5. Edit a node and "From library" paragraph. The label of the referenced library item should be displayed as "- Restricted access -" (In case you are using Autocomplete field widget for the "From library" paragraph type)
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

That's because we don't check the permissions in our preprocess function. We need to do that, including keeping cacheablity metadata.

We need tests that create an unpublished library, access as anon, make sure it doesn't show up, then publish it, then access again and making sure that it now shows up. To get that to pass, you will need to add the library as a cacheable dependency to the access result.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Assigned: Unassigned » Berdir
Status: Needs work » Needs review
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
FileSize
make-paragraphs-library-items-2943601-8.patch10.69 KB
make-paragraphs-library-items-2943601-8-interdiff.txt5.34 KB

Added the access check including tests.

Status: Needs review » Needs work

The last submitted patch, 8: make-paragraphs-library-items-2943601-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Assigned: Berdir » Unassigned
Status: Needs work » Needs review
FileSize
make-paragraphs-library-items-2943601-10.patch11.35 KB
make-paragraphs-library-items-2943601-10-interdiff.txt1004 bytes

Ah, fun fail. The assertion was bogus and we forgot to update it properly. It was still testing the case when the paragraph library label was the link. But it actually also matched on the breadcrumb, but *only* the wrong, old label, because the breadcrumb cache wasn't invalidated. Now, because I added the cacheability metadata, it is.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented
Status: Needs review » Fixed 
+++ b/modules/paragraphs_library/paragraphs_library.install
@@ -248,3 +248,55 @@ function paragraphs_library_update_8005() {
+    ->setRevisionable(TRUE)
+    ->setTranslatable(TRUE)
+    ->setDefaultValue(TRUE)

+++ b/modules/paragraphs_library/paragraphs_library.module
--- a/modules/paragraphs_library/src/Entity/LibraryItem.php
+++ b/modules/paragraphs_library/src/Entity/LibraryItem.php

I was confused why this definition is not needed below and couldn't see from where these defaults would originate. However the resulting storage was consistent for updates and new installs.

Committed.

  • miro_dietiker committed 6e1f00a on 8.x-1.x authored by Berdir 
    Issue #2943601 by Berdir, mbovan: Make Paragraphs Library items...

Status: Fixed » Closed (fixed)

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

the_glitch’s picture

the_glitch CreditAttribution: the_glitch commented

So which of these patches work?

harcher’s picture

harcher CreditAttribution: harcher commented

...