When looking at the configuration synchronisation pages you will still see certain configuration overrides, even though you have selected those to be ignored with this module. Same happens when using Drush or Console and you get a list of configuration overrides that will change with the import command.

I believe for the sake of better UX this module should either override those outputs or just hide them as it might be confusing for some users since they will expect for those modules to be overridden even though they actually won't be.

I'm happy to work on this, I would just like also someone else's input on this, on how to approach this?

CommentFileSizeAuthor
#10 config_ignore_drush_cim.png96.17 KBtlyngej
#10 should_the_module-2845286-10.patch12.95 KBtlyngej
#9 interdiff-2845286-8-9.txt9.01 KBtlyngej
#9 should_the_module-2845286-9.patch10.34 KBtlyngej
#8 interdiff-2845286-3-8.txt1.43 KBjzavrl
#8 2845286-8.patch4.46 KBjzavrl
#3 should_the_module-2845286-3.patch2.97 KBtlyngej
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jzavrl created an issue. See original summary.

tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented

But you are so absolutely right @jzavrl!

The UX (and DX for that matter) for this module is terrible for the time being.

I don't really have any strong opinions on how to get the user notified about, which config entities that are being ignored, as long as it's done through Drupal Core API's only.

If you put something together that would be lovely. It doesn't have to be the complete work, but a proposal, or a patch with pseudo code would do just fine. Then we can work from there.

tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented
FileSize
should_the_module-2845286-3.patch2.97 KB

Right, as far as I can see, there is no good way to suppress the information displayed, prior to an import. For example, if you visit admin/config/development/configuration or run a drush cim.

But this patch will at least inform the user, running the import, that the configuration has been ignored.

Let me know if you think that this would be a good first step at least.

tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented
Status: Active » Needs review
tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented
tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented

Marking #2850620: How to not list the ignored list during import? as a related issue.

tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented
Related issues: +#2850620: How to not list the ignored list during import?

That didn't go so well. Lets try again...

jzavrl’s picture

jzavrl CreditAttribution: jzavrl at NDP commented
FileSize
2845286-8.patch4.46 KB
interdiff-2845286-3-8.txt1.43 KB

That looks great. The message appears in the UI as well as when using Drush.

I've also wanted to display some of this information right from the start as the messages are only visible upon import. So I've been playing around a little and have altered the configuration overview table so it now displays one more column 'Ignored' and indicators for each row. Let me know what you think.

On a note to the related issue - would it perhaps make more sense to completely hide the rows with config entities that are ignored? For the table that shouldn't be a problem, but not quite sure on the Drush part. I'm afraid we might have to override some classes for that.

tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented
FileSize
should_the_module-2845286-9.patch10.34 KB
interdiff-2845286-8-9.txt9.01 KB

Brilliant approach, @jzavrl!

Definitely better than nothing.

I've made a few changes. Mainly adding missing use statements, and added a few punctuations in the comments here and there.

Further more, I have made a generic function, that we can ask whether a config name should be considered as ignored or not.

Ohh, yes, and added a test to verify that the column is set to the right value.

Hiding the column sounds like something that could cause confusion. For now I think that it's better to be upfront about the fact that there are changes, but the site won't apply them.

Regarding Drush, I might have a friend I can ask about how to alter the output there. If no one else beats me to it :-)

tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented
FileSize
should_the_module-2845286-10.patch12.95 KB
config_ignore_drush_cim.png96.17 KB

I've figured out how to hook into Drush, but unfortunately, it doesn't seem like I can alter the existing table of changes that it prints.

I can, how ever, print something before (see screenshot).

I'm not sure if a message after and before the import is a good idea.

What do you thing?

jzavrl’s picture

jzavrl CreditAttribution: jzavrl at NDP commented
Status: Needs review » Reviewed & tested by the community

Nice! I think having both messages is not a bad idea, I'd leave it at that, to be honest. For now I think it's safe to say that this can be commited.

  • tlyngej committed 52c9908 on 8.x-1.x 
    Issue #2845286 by tlyngej, jzavrl: Should the module suppress...
tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented
Status: Reviewed & tested by the community » Fixed

Cool.

Let's get this bad boy out in the open then!

Status: Fixed » Closed (fixed)

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