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

Bring back removal of translatable columns #3828

Merged
merged 1 commit into from
Nov 8, 2019

Conversation

javierm
Copy link
Member

@javierm javierm commented Nov 7, 2019

References

Objectives

  • Remove unused columns

Notes

Globalize does not support having translatable columns with the same name in the original table and the translations table. We were planning to migrate to Mobility, but we aren't doing so before releasing version 1.1.

We've also found a gotcha regarding having both columns: if we use the update_column method, which we use in rake tasks to speed up the process and in tests where we want to skip validations and callbacks, we update the column in the original table and no exception is raised. If we remove the column in the original table, when we use update_column we get an exception, which is what we want since our intention is to update the column in the translations table.

There are also fields we've added to translation tables but not the original tables, like milestones_summary and homepage in legislation_process_translations or description in active_polls. Having some fields in both tables and some fields only in translation tables is not very consistent.

With this change we're following the advice given by the Mobility lead developer: "If you don't need the columns, I think it would make sense to just remove them to avoid any edge case issues."

@javierm javierm self-assigned this Nov 7, 2019
@javierm javierm added this to Reviewing in Roadmap via automation Nov 7, 2019
Globalize does not support having translatable columns with the same
name in the original table and the translations table. We were planning
to migrate to Mobility, but we aren't doing so before releasing version
1.1.

We've also found a gotcha regarding having both columns: if we use the
`update_column` method, which we use in rake tasks to speed up the
process and in tests where we want to skip validations and callbacks, we
update the column in the original table and no exception is raised. If
we remove the column in the original table, we get an exception, which
is what we want since our intention is to update the column in the
translations table.

With this change we're following the advice given by the Mobility lead
developer: "If you don't need the columns, I think it would make sense
to just remove them to avoid any edge case issues."

This commit reverts commit 251326e.
@javierm javierm changed the base branch from remove_obsolete_report_columns to master November 7, 2019 21:01
@javierm javierm force-pushed the remove_obsolete_translatable_columns branch from f71d609 to 7a78776 Compare November 7, 2019 21:02
@javierm javierm merged commit 0f646c6 into master Nov 8, 2019
Roadmap automation moved this from Reviewing to Release 1.1.0 Nov 8, 2019
@javierm javierm deleted the remove_obsolete_translatable_columns branch November 8, 2019 17:20
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
…ranslatable_columns

Bring back removal of translatable columns
@taitus taitus mentioned this pull request Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Roadmap
  
Release 1.1.0
Development

Successfully merging this pull request may close these issues.

None yet

1 participant