Account created on 18 February 2015, almost 10 years ago
#

Merge Requests

Recent comments

🇧🇪Belgium L_VanDamme

l_vandamme made their first commit to this issue’s fork.

🇧🇪Belgium L_VanDamme

l_vandamme made their first commit to this issue’s fork.

🇧🇪Belgium L_VanDamme

Looks good to me.

Small note: we might want to lose the default content (initial blog item) now that we have the add content by bundle button.

🇧🇪Belgium L_VanDamme

@phenaproxima I did another pass, properly rebasing, updating the licensing stuff to what is now in 0.x and moving the entity displays to config as we did for blog. Should be good to go now.

🇧🇪Belgium L_VanDamme

@phenaproxima I moved the entity displays back to the config folder (will do so for news as well in that ticket) and fixed the related blog posts view while I was at it.

🇧🇪Belgium L_VanDamme

@phenaproxima I changed the news description everywhere I could find it and added the licensing stuff. Having some troubles with a rebase though, can you have a look?

🇧🇪Belgium L_VanDamme

@phenaproxima As I understood it, we are planning to use layout builder on all content types. 1 thing that is in contention still is whether we'll be using the layout builder overrides for overview pages.

As for the view and form displays, I moved them to the recipe.yml as config actions to allow them to be overridden without causing issues when reapplying the recipe. This was due to a discussion about when we should allow for these kind of changes and when not, and the way I see it, if there's recipes in drupal_cms (or in a future version of it) that would alter the config, it might be better to have it as config actions and so allow overrides.
Let me know if this doesn't make sense.

🇧🇪Belgium L_VanDamme

@phenaproxima Updated the MR. I also took the opportunity to update the blog description and to add news to the CODEOWNERS file.

🇧🇪Belgium L_VanDamme

@phenaproxima Can you review? Build job is currently failing, but local test runs were ok for the news recipe.

🇧🇪Belgium L_VanDamme

@phenaproxima There's some tests failing but I don't think it is blog related. Let me know if there is something to fix.

Side note, I enabled layout builder and overrides for basic pages because we will need it to set up overviews that way. We can not use it at this point because core causes some issues when trying to import serialized layout builder sections (see https://www.drupal.org/project/drupal/issues/2942975 📌 [PP-1] Expose Layout Builder data to REST and JSON:API Postponed ). Depending on how much we want this way of working in v1, we might need to add some pressure there.

🇧🇪Belgium L_VanDamme

This patch is causing problems when reverting revisions.

This is because of the getActive call that was added to retrieve the latest version of the block when editing in stead of the saved revision. I believe it was added to prevent EntityChangedConstraint errors, but they were fixed in https://www.drupal.org/project/drupal/issues/3053881 .

Steps to reproduce this:

  1. Create a page with a text block with title AAAA.
  2. Update the page and change the text block title to BBBB.
  3. Revert back to the first version.
  4. The block title is now AAAA. But when you open the edit form of the block, it shows BBBB (the latest revision)

Code that is causing this behavior (in InlineBlock::blockForm()):

    if (!$this->isNew && !$block->isNew() && empty($this->configuration['block_serialized'])) {
      // Get the active block for editing purposes.
      $block = $this->entityRepository->getActive('block_content', $block->id());
    }

I think there might also be other places that this getActive function is called to prevent the error where it is no longer needed.

I would be willing to update the patch, but have not been active in creating it, so I'm not sure about what I might be breaking when removing this code.

🇧🇪Belgium L_VanDamme

Currently holding off on this until layout builder has been implemented in blog.

🇧🇪Belgium L_VanDamme

I had some discussions about this and the idempotency is still in tact without changing anything.

There are some changes that could be done like moving the entity displays to config actions, but really it will always be a balancing act, because it is hard to know what might get overridden an what won't. As a current decision, we will only move config to actions if we know for sure it might get overridden (e.g. when another recipe within Drupal CMS would change the config), which is not currently the case for any blog config. If the need arises in the future, separate tickets for the config items would seem to be the best approach.

🇧🇪Belgium L_VanDamme

My 2 cents:

I like most of what I read here, I think it's a very good place to start from.

I do agree with breidert on the configuration and interface translations being included. I don't know how much we would need to have configured by default, but they should be looked at IMO.

As for mandclu's comment about fields like "where" and "when" fields, I believe we should not make any field type translatable unless we're very sure what it does (e.g. only text and text_area and nothing else). IMO it is better to have a field not translatable that should be in stead of the other way around (because of the note in the edit form).

I agree with ignoring content moderation at this point.

I think metatag translations should be something for the SEO recipe. The most important ones will be generated based on fields anyway, and these fields (or their entities) will be translatable (title, description, image).

One other thing that might need to be discussed is the entity translation default settings (showing language select or not, interface language by default vs default language vs a language selected on recipe install).

🇧🇪Belgium L_VanDamme

+1 on using drupal_cms_anti_spam and article_comments as guideline.

Feel free to get started on this keeping those 2 things in mind, try to keep it simple to start to prevent too much refactoring later on.

🇧🇪Belgium L_VanDamme

@phenaproxima I removed the dependency on the content editor role. I guess there is not really a reason for a blog feature to be opinionated on user roles anyway. Also removed a leftover dependency in the drupal_cms content item.

Can you have a look again?

🇧🇪Belgium L_VanDamme

Some thoughts:

  • I agree with giving the cookies module a try. As mentioned, we're currently using EUCC and are not very happy with it. If anyone has experience with getting compliant implementations using the cookies module, do share any caveats or limitations you experienced
  • Including Cookie Content Blocker or a similar module will be required.
  • We currently use some extra modules to do things like adding datalayer events, extending the consent modal and providing ways to add consent based videos etc. I assume we would prefer to make a decision on which modules to use and then try to update these modules to include what we need rather than making an extension module?
  • We might want to do a little bit of research as to what is being used most for implementing tracking scripts etc. so we know what to support. E.g. Google Tag Manager, Matomo, ...
🇧🇪Belgium L_VanDamme

@jurgenhaas I went over the new division again and agree for the most part.

The only unclear item for me would be the "Subscriptions and unsubscription" being within the consent management recipe. I've not had much experience at all with implementing something like subscriptions in Drupal and it seems to me to be an edge-case that might need to be solved by the module supplying the subscription service in the first place. In that case, I would move this to the audit module, because that will make it possible for any subscription module to implement a plugin to have it checked.

Other than that, it looks good to me!

🇧🇪Belgium L_VanDamme

My proposition for dividing the feature set in to recipes:

Base (Consent) recipe:

  • Privacy: do not track without consent
  • Disclose what data you collect and why
  • Do not embed remote assets (fonts, videos, images, iframes, etc.) without consent
  • Consent management (a.k.a. cookie banner): only ask for consent if really required. Don't forget contact and maybe other forms.
  • Default/sample content: imprint, privacy policy, data protection declaration, general terms and conditions, technical organizational measures (TOMs)
  • FAQs and recommendations / documentation

Data protection recipe:

  • Data protection: only collect data that's really necessary
  • Data processing: only process PII with consent and disclose third parties that you share data with
  • If you store user data (or otherwise PII), provide easy mechanisms ...
    • to receive a report of what's stored about each user
    • to delete and confirm the deletion of that user data on request
  • Sanitized export of data, e.g. for developers (db and files), e.g. with Drush or in the UI
  • FAQs and recommendations / documentation

Audit module:

  • Avoid dark patterns, e.g. do NOT seduce the user to consent
  • Initial and regular internal audits on the website
  • External audits, like e.g. Typo3 has done with the German Federal Government
  • FAQs and recommendations / documentation

Other (not sure)

  • If you provide subscriptions, make it as easy to unsubscribe
  • Privacy and compliance API: law firm may hook into those to automatically provide setup, content and updates
🇧🇪Belgium L_VanDamme

I like the initial setup but have 2 remarks / questions that I haven't seen pop up here yet:

  1. Have you considered indexing a full view mode? We have noticed that this became somewhat required when we started using layout builder, as that included content that wasn't easily transferable to a index view mode. This did however cause some issues with indexing certain non-page-specific content (e.g. a "related items" block).
  2. Do we want to set something up to allow excluding certain pages from the index by default? We tend to include the search_api_exclude module in pretty much all of our projects (especially for 404/403 pages and the homepage).
🇧🇪Belgium L_VanDamme

I updated the MR to stop using RouteMatch parameters or request attributes as they are not reliable. Instead, we should use the Route defaults as we can be 100% sure they will be present and correct (as we set them ourselves).

This is needed because we have noticed very rare edge cases where the page entity could not be found in the RouteMatch parameters or request attributes. In our case this was most easily reproduced right after logging in with users that have a certain role. I did not debug what was actually causing this in code.

🇧🇪Belgium L_VanDamme

I added some lines to the existing patch (the part of the access check) that load the page manager page entity if it is not loaded already. This seems to fix the access issues in our usecase (language switch links).

This does feel a lot like a bandaid for a bigger issue though, more investigation would be nice. Especially by someone more familiar with the routing system and the way parameters are being upcast to objects.

🇧🇪Belgium L_VanDamme

Reuploading patch as I missed the file deletions

🇧🇪Belgium L_VanDamme

Separate patch that applies to 1.0.1 (due to packaging information on info.yml)

🇧🇪Belgium L_VanDamme

I set up a MR with the new code (with some additions and improvements to my previous .zip file)

🇧🇪Belgium L_VanDamme

Uploading a patch I used for a project that implements the second possible solution (updating the database schema), because they specifically wanted to be able to use longer titles.

This patch was made on the 6.0.0-alpha38 version of this module, but should work on later versions as well.

🇧🇪Belgium L_VanDamme

Added a patch based on the open MR

🇧🇪Belgium L_VanDamme

Uploading a patch file based on the open MR

Production build 0.71.5 2024