Skip to content

change filter locales strings from initiatives to proposals#6013

Merged
tramuntanal merged 3 commits intodevelopfrom
fix/filter-locales
Apr 28, 2020
Merged

change filter locales strings from initiatives to proposals#6013
tramuntanal merged 3 commits intodevelopfrom
fix/filter-locales

Conversation

@microstudi
Copy link
Copy Markdown
Contributor

@microstudi microstudi commented Apr 22, 2020

🎩 What? Why?

With the introduction of filters for proposals in the admin, some strings that were in the initiatives module were reused. However, as initiatives is not enable by default when installing Decidim the system cannot find those strings and result in weird menus in production and a total failure when developing.

We should move the reused strings to proposals as this module is always enabled.

NOTE: this should be backported to 0.20 and 0.21

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry

📷 Screenshots (optional)

Without initiatives enabled this is how it looks the filter in the proposals admin (in development mode just fails):
image

Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one question @microstudi

created: Created
discarded: Discarded
published: Published
rejected: Rejected
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this strings have been reused, doesn't it mean that they were already being used before? If we remove them won't it affect the initiatives module?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the initiatives modules will take the string from the proposals module, this string is moved there.
The problem now is that the proposals module is using this string and, if initiatives is not active, doesn't find it.
If we keep it here, it will be repeated in 2 files isn't?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the problem. But I'm referring to the remote (but possible?) case where initiatives are enabled and proposals not. Also, to have the code correctly grouped, each module should be as independent as possible, and it seems to me that this should not be a reason to have this dependency.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact I agree with you. The thing is that is that many strings are created this way, this means that we will should "namespace" all strings in all modules. That's not a quick fix. We should create a issue for that and treat it properly.
I any case, I don't thing it is possible to run decidim without the proposals module.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I any case, I don't thing it is possible to run decidim without the proposals module.

Yes, that's the case right now, it's one of the (too many) modules that we've as a dependency by default. The thing is that shouldn't be, as we're starting to see some instances that have only Initiatives module enabled.

But I also agree on having this discussion on another place, and that for the moment this PR is OK (proposals can be an implicit depedency of initatives; there are cases where initiatives are not enabled)

tramuntanal
tramuntanal previously approved these changes Apr 27, 2020
@tramuntanal tramuntanal merged commit 1f2c424 into develop Apr 28, 2020
@tramuntanal tramuntanal deleted the fix/filter-locales branch April 28, 2020 07:48
faithngetich pushed a commit to faithngetich/decidim that referenced this pull request Apr 28, 2020
…6013)

* position locales in correct modules

* add changelog

Co-authored-by: Oliver Valls <oliver.vh@coditramuntana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants