Problem/Motivation

Want to use config readonly module but add eu_cookie_compliance.settings to "allowed" list. Because user.role.* and core.extension config is saved during the main settings form submit (permissions for showing the banner, core weight adjustment via eu_cookie_compliance_module_set_weight() ).

Steps to reproduce

Enable config_readonly, add 'eu_cookie_compliance.settings' to $settings['config_readonly_whitelist_patterns']
Try to save settings form (admin/config/system/eu-cookie-compliance/settings)

Proposed resolution

Provide a checkbox on the form to skip the permission and weight updates.

CommentFileSizeAuthor
#5 allow-config-save-3208780-4.patch2.37 KBben.hamelin

Issue fork eu_cookie_compliance-3208780

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ben.hamelin created an issue. See original summary.

ben.hamelin opened merge request !12

ben.hamelin closed merge request !12

ben.hamelin opened merge request !13

ben.hamelin’s picture

ben.hamelin
he/him
Primary language English
Location Adirondack Mountains, NY
commented
StatusFileSize
new allow-config-save-3208780-4.patch2.37 KB

I'm having trouble getting the Gitlab generated diff to apply to my project, so uploading a patch I generated locally.

svenryen’s picture

svenryen commented
Status: Active » Needs review

Thanks for the patch. Remember to mark the issue "Needs review".

Status: Needs review » Needs work

The last submitted patch, 5: allow-config-save-3208780-4.patch, failed testing. View results

svenryen’s picture

svenryen commented
Status: Needs work » Needs review
svenryen’s picture

svenryen commented
Status: Needs review » Needs work

Hi @ben.hamelin.

I had a look at your patch. It adds a checkbox at the very top of the module configuration form, and I'm pretty sure the 40,000 users of this module will wonder what's the story behind that checkbox if we accept the patch as is, as many of them may not even have heard of config_readonly.

I have two suggestions:

  1. If the config_readonly module is available, simply check if we're allowed to store the user permission and module weight and execute the call to store those configs only if permitted. Display a notification to the user that the permission/module weight couldn't be stored due to config_readonly being enabled.
  2. Display the new checkbox only if config_readonly is enabled.

While I would prefer option 1, I can also accept a patch for option 2.

Let me know if you have any questions about this. I'm moving status to Needs work.

svenryen’s picture

svenryen commented

(Ignore my commits, I was pushing to the wrong branch)

svenryen closed merge request !13

svenryen reopened merge request !12

svenryen’s picture

svenryen commented

I reopened your MR 12 as that's where I could find your code. MR13 seems to have a few blank lines only.

I've made some improvements to your suggested code.

  1. I've removed the checkbox so that it doesn't confuse the end user.
  2. I check if the config is allowlisted before saving.
  3. If the config is not allowlisted, a warning is shown to the user, informing them that the settings variable needs to be added to the allowlist in order to save the settings.

It would be really nice if you could take a look at the suggested changes and RTBC this issue. Thanks again for the contribution :)

svenryen’s picture

svenryen commented
Status: Needs work » Needs review

Neslee Canil Pinto made their first commit to this issue’s fork.

neslee canil pinto’s picture

neslee canil pinto
he/him
Primary language English
commented
Status: Needs review » Fixed
ben.hamelin’s picture

ben.hamelin
he/him
Primary language English
Location Adirondack Mountains, NY
commented

Thanks for working this in folks! Much appreciated!

Status: Fixed » Closed (fixed)

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