Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle null database values as null in translations #479

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alipadron
Copy link
Contributor

This pull requests is originated from the conversation on #474 where @vencelkatai and i agreed on finding a way to have an implementation that satisfies our use cases and also keep the library consistent.

Changes

  • Modified the getTranslation method to check if the raw attribute value is null using self::getAttributeFromArray. If the attribute is null, it will now treat the translation as null instead of falling back to an empty string.

Why This Change Is Needed

Previously, null values in the database were treated inconsistently, being returned as empty strings in translations. This behavior could lead to unexpected results in applications that differentiate between null and empty strings. By aligning the translation output with the underlying database value, this change ensures more predictable and intuitive behavior.

Next Steps

Please @freekmurze and @vencelkatai review the changes and share your feedback. If the approach aligns with expectations, I’ll proceed with further documentation or additional tests if necessary.

Thank you!

- Updated the HasTranslations trait to return null for translations when the underlying database attribute is null.
- Adjusted the logic in getTranslation to check if the raw attribute value is null before attempting to resolve translations.
- Modified a test case to validate that null database values are treated as null in translations instead of empty strings.
@freekmurze
Copy link
Member

This seems ok to me. Do you have any thoughts @vencelkatai ?

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