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

Translate Block using a related "TranslatedContent" model #1215

Closed
2 of 4 tasks
Tracked by #1204
drikusroor opened this issue Aug 6, 2024 · 2 comments · Fixed by #1227
Closed
2 of 4 tasks
Tracked by #1204

Translate Block using a related "TranslatedContent" model #1215

drikusroor opened this issue Aug 6, 2024 · 2 comments · Fixed by #1227
Assignees

Comments

@drikusroor
Copy link
Contributor

drikusroor commented Aug 6, 2024

Requirements

  • Create a BlockTranslatedContent model with the following properties
    • block -> FK to Block
    • language -> CharField with the choices=language_choices
    • name
    • description
  • Remove name & description fields from the Block model
    • Perhaps we can also keep this for internal usage by the experimenter/researcher?
  • (Proposal) Enforce relationship to Phase, i.e. don't allow blank=True and null=True in the Block's phase field. (see NB) In the migration, we can create an empty experiment and phase for all blocks that are currently dangling without a phase and experiment.
  • (Cleanup?) Remove url and hashtag properties as these are configured in SocialMediaConfig already
  • (Maybe later?) Remove the language field from Block since it will be rendered irrelevant by this feature. I'm not sure what extra work needs to be done for this.

NB

  • index - which does exist on ExperimentTranslatedContent won't be needed here as the fallback language is determined on the experiment level.
  • As the language of the block will now be determined by its parent (of parent) experiment, a Block that is not related to a Phase (which still seems to be possible currently) will probably result in a crash or by picking the first language content it finds. Should we enforce a relation to a Phase eventually to avoid this problem? I think that was the plan anyway.
  • image, which might be considered to be in tandem with name and description has no textual content so it needs not to be translated. Right? Anyone, please let me know if you disagree.
@BeritJanssen
Copy link
Collaborator

BeritJanssen commented Aug 6, 2024

I agree with almost all of your choices. See also #1211 for your comment on enforcing a phase relationship of Block. After this data migration, that should be guaranteed throughout. Then we can also remove the consent field from Block, as consent will always be defined on the Experiment level. As for the Image, I agree that we probably won't need translations of the alt text in practice, and can set it to the Block's description instead, but in the long run we may want to have a ImageTranslatedContent model in place, too, for other images, so I wouldn't write any code to set Image texts from BlockTranslatedContent just yet.

As for the one thing I'd change: I was actually wondering, what with the removal of the GroupedBlock model, whether we'd want to have the relationship between BlockTranslatedContent and Block to be a ManyToMany field instead. That would mean we could reuse the textual content defined for a block if we embed it in different experiments. E.g., we could run the block as a loose experiment, and within a dashboard, and would only need to duplicate the Block object itself for this, but not the BlockTranslatedContent objects.

@drikusroor
Copy link
Contributor Author

drikusroor commented Aug 8, 2024

As for the one thing I'd change: I was actually wondering, what with the removal of the GroupedBlock model, whether we'd want to have the relationship between BlockTranslatedContent and Block to be a ManyToMany field instead. That would mean we could reuse the textual content defined for a block if we embed it in different experiments. E.g., we could run the block as a loose experiment, and within a dashboard, and would only need to duplicate the Block object itself for this, but not the BlockTranslatedContent objects.

While I'm not against the idea per se, it would mean that, instead of a full nested form with all inputs, the inline form would look like this (or with checkboxes).

image

Also, as the display name of a block in the admin dashboard is based on the name of its fallback translate content, it would become hard to distinguish different blocks that have the same translated content. Unless, we decide not to delete the block's "name" field and use it only in the admin dashboard to distinguish different blocks.

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 a pull request may close this issue.

2 participants