- Issue created by @jurgenhaas
- Merge request !152Issue #3483392 by jurgenhaas: Build privacy base recipe β (Merged) created by jurgenhaas
- First commit to issue fork.
- πΊπΈUnited States phenaproxima Massachusetts
The more I look at this, the more I think we should, at least for now, fold it into the base recipe.
This would eliminate the dependency weirdness about the Page content type, and it would decrease overall maintenance burden by reducing the number of components we'll need to split out. I think that it makes sense to lower our initial maintenance needs at release time, and then split this into its own recipe if the popular demand for it exists. I am not convinced this recipe currently provides enough "stuff", on its own, to justify it being its own component. As has been pointed out, its additions are useful but also quite generic, which really puts this more into the "initial site starter kit" category which is covered by the base recipe.
- πΈπ°Slovakia poker10
@jurgenhaas I see the Klaro! consent manager JS-Library is open-source for client-side usage, but paid for another server-side features. Are we certain that the licensing will not change if we make it more popular? These 3rd party services tend to tighten the usage policies like cookiebot and some others have done in the past.
- π©πͺGermany jurgenhaas Gottmadingen
@poker10 yes, we've checked into that. Not only this statement on their website
but also a lot other indicators have been checked, down to the level that we know the guy and his motivation, who built that service. And even if he were bought out by somebody, that would not prevent us from continued usage of the free library which we have forked and provide as a drupal-library from klaro_js β on drupal.org
- πΊπΈUnited States phenaproxima Massachusetts
Hey, look! I found a reasonable compromise that can allow this to move forward.
Although I am still not entirely convinced that this should be its own recipe, that decision can wait until the beta period, when we need to finalize our list of components (which will mean consolidating some of the existing ones into the base recipe). In the meantime, the Page content type, which was a workaround for overzealous strictness checks from core, is removed; we can rely on core's Page content type recipe only, which solves the dependency problem that was making me most hesitant.
I also went and added basic test coverage here. I think this can go back to you, @jurgenhaas, for any further changes that are needed.
- π©πͺGermany jurgenhaas Gottmadingen
This is now ready for review. Note, it's not perfect yet, but we decided to push this out for review and hopefully get it merged soon. We would then like to follow-up in separate issues and MRs with further details as we also learn from other tracks.
Some explanation:
- Default configuration provides basic privacy compliance and a consent management
- Styling of the consent widgets will be addressed in a separate issue
- Default content is under discussion in π Default content for privacy requirements Active
- If other recipes will bring additional requirements for consent management, they will have to enable the relevant settings for it. We'll consult with track leads on that separately.
So, this review focusses on the basic principle of the setup. Further details to follow in child issues.
- πΊπΈUnited States phenaproxima Massachusetts
Only one small nit, and obviously we need to reenable the tests, but no particular objections from me.
Assigning to @pameeela for manual testing and review.
- πΊπΈUnited States phenaproxima Massachusetts
Tests appear to be failing to due a legitimate bug in the Klaro module: https://git.drupalcode.org/project/drupal_cms/-/jobs/3209937#L52
The problem is that, in their config schema, there's an extra period here: https://git.drupalcode.org/project/klaro/-/blob/3.x/config/schema/klaro....
They'll need to fix that upstream, or we'll need to patch the module. Kicking back for that.
- π©πͺGermany jurgenhaas Gottmadingen
This is fixed. There is now only 1 test failure left. It has to do with the footer menu. I can look into that tomorrow
- πΊπΈUnited States phenaproxima Massachusetts
Tests seem to be passing now. Over to Pam for review.
- π¦πΊAustralia pameeela
Happy to merge this and see how it goes. I am not 100% on enabling the consent manager by default but it will be easier to test and get feedback once it's merged, and we can easily change it to make it optional.
-
phenaproxima β
committed 9830a86e on 0.x authored by
jurgenhaas β
Issue #3483392 by jurgenhaas, phenaproxima, poker10: Build privacy base...
-
phenaproxima β
committed 9830a86e on 0.x authored by
jurgenhaas β
- πΊπΈUnited States phenaproxima Massachusetts
And on that note, shipped! Let's see how this goes and adjust as needed down the line. Onward!
- π©πͺGermany rkoller NΓΌrnberg, Germany
jurgenhaas β credited rkoller β .
- π©πͺGermany jurgenhaas Gottmadingen
Thanks @phenaproxima, I've just added two more for credits as they helped with testing in getting this across the finish line.
Automatically closed - issue fixed for 2 weeks with no activity.