Skip to content

Support empty strings as overrides properly#22

Merged
mficzel merged 1 commit intomainfrom
emptyStringOverrideDebug
Jun 26, 2025
Merged

Support empty strings as overrides properly#22
mficzel merged 1 commit intomainfrom
emptyStringOverrideDebug

Conversation

@hedayati-m
Copy link
Collaborator

By updating a translation in backend module, it is allowed to set empty string as translation. As a result we have a record in sitegeist_csvpo_domain_translationoverride with empty string as the translation. On the other hand when it comes to overrides the code sees empty strings as null and gives the option to "add" a new override with the same keys which are already in the table, which causes a doctrine dbal exception.
We have two options
1- Do not allow empty strings in update action.
2- Reflect the exact state of the override table and update records.

In this branch the second option is implemented.

@mficzel mficzel changed the title check if translation override is realy null Support empty strings as overrides Jun 11, 2025
@mficzel mficzel changed the title Support empty strings as overrides Support empty strings as overrides properly Jun 11, 2025
Copy link
Member

@mficzel mficzel left a comment

Choose a reason for hiding this comment

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

I wonder wether what the actual use case is here. I could set an override to an empty string but this would still yield an fallback string in the fronend.

I think it would make more sense to add a validator to the override that ensures no empty overrides are created. Or do you have a usecase where an empty override would be a feature. In that case we can discuss the expected behavior in the frontend.

@hedayati-m
Copy link
Collaborator Author

hedayati-m commented Jun 11, 2025

The right way would be not allowing empty overrides get into the overrides table in first place. Nevertheless i think the override attribute is supposed to mirror the exact state of the record in the override table. In this case empty strings are not be considered as null.

@mficzel
Copy link
Member

mficzel commented Jun 26, 2025

After discussion we decided to treat empty overrides as a feature and allow to create them.

@mficzel mficzel merged commit cef8838 into main Jun 26, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants