mainio / decidim-module-term_customizer

Decidim module that allows customizing the localization terms in the system for specific contexts.
GNU Affero General Public License v3.0
16 stars 21 forks source link

When creating a new term, if the key is a child of another key that has assigned a value, clear cache gives a 500 server error #76

Open martanducas opened 3 years ago

martanducas commented 3 years ago

This issue has been updated, the real problem is as follows:

I have added some new terms with the following keys: custom.pae.services custom.pae.services.1 custom.pae.services.2

I have cleared cache. The server has crashed (error 500 Internal server error).

@microstudi (https://github.com/microstudi) can give more technical details, but the problem is that according to the YAML format, a key (custom.pae.services) cannot have assigned a value if it has child keys (custom.pae.services.1). Since a user like me is not aware of this restriction, it is necessary to have some kind of validation that avoids this behaviour (otherwise, any admin user can crash a Decidim by adding new terms).

microstudi commented 3 years ago

Yep, also updating this comment:

Hi, the error appears in this line: https://github.com/mainio/decidim-module-term_customizer/blob/0e823f4f768f6255de57559c618dcad784c5d655/lib/decidim/term_customizer/loader.rb#L54

It is thrown in the first view rendered, with this message:

ActionView::Template::Error (no implicit conversion of Symbol into Integer)

The problem is that TermCustomizer accepts any string as a key for a translations, without checking if it would be a valid structure if it'd come from a YAML file.

As @marta-platoniq said, you can add these keys in term customizer:

custom.pae.services: something
custom.pae.services.1: another text
custom.pae.services.2: yet another text

Which would correspond to this YAML structure:

custom:
  pae:
    services: Something
      1: another text
      2: yet another text

Which is invalid.

Probably a solution would involve creating an extra validator in the form handleing the controller request to check if all the keys are compatible.

ahukkanen commented 3 years ago

Yep, this is a known issue.

The problem is that if solved properly, it's quite difficult. I think the best solution to this would be to provide the translation interface for these special translation cases, such as lists or zero/one/other or odd/even type of strings. So instead of adding them separately, you could define different types of translations, such as list translation, which would be then customized according to that specified structure.

And how would that apply to the dynamic translation adding. I believe there would need to be some changes there as well to detect the type for each of the imported strings.

I haven't checked myself how many of these different type situations there are.