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

[Backport] Make milestones polymorphic #3057

Merged
merged 14 commits into from
Dec 5, 2018

Conversation

javierm
Copy link
Member

@javierm javierm commented Nov 22, 2018

References

Objectives

Make milestones polymorphic, so we can use them in proposals and legislation processes.

Notes

⚠️ ⚠️ For release notes: Run RAILS_ENV=production bin/rake milestones:migrate.

Even though this task should run smoothly and it doesn't delete old milestones data, we always recommend to backup your database before running any tasks dealing with data migration.

@javierm javierm force-pushed the backport-make-milestones-polymorphic branch from de1305e to 37541bd Compare November 29, 2018 19:33
@javierm javierm changed the title [Backport] Make milestones polymorphic [WIP] [Backport] Make milestones polymorphic Nov 29, 2018
@javierm javierm force-pushed the backport-make-milestones-polymorphic branch from 07904ec to e96e222 Compare November 30, 2018 10:29
mlovic and others added 12 commits November 30, 2018 14:15
Generalize the BudgetInvestmentStatus model to Milestone::Status so it
is not specific to budget investments, but can be used for any entity
which has milestones. This is in preparation to make the Milestone
model polymorphic and usable by entities other than budget investments.
Generalize the Budget::Investment::Milestone model to a
polymorphic Milestone model so it can be used for entities
other than Budget::Investment.
We also make the line shorter so rubocop doesn't complain.
We had a line which was too long according to rubocop, and simplifying
the code makes the line shorter.
Calling `load_tasks` in several files made rails load the tasks several
times, and so they were executed several times when called.

Since the milestone migration can't be executed twice in a row (it would
fail with duplicated ID records), loading the tasks several times made
the milestone migrations task specs fail.
We'd rather keep the table so future data migrations work smoothly, so
we change the migration in order to create the translation table without
using models.
When we insert a record in PostgreSQL and we specify the ID, the
internal ID sequence for that table isn't updated.

In order to keep the original IDs so we didn't break any foreign keys,
we specified the IDs when copying the table, resulting in a table having
its ID sequence with a value of an existing record. When trying to
insert a new record, we got a `PG::UniqueViolation` exception.

Updating the sequence after the data migration might not be the most
elegant solution, but it's easy to do and it's already been tested on a
production environment.
@javierm javierm force-pushed the backport-make-milestones-polymorphic branch from e96e222 to 936cd3f Compare November 30, 2018 13:15
Using `Date.today` was making the spec fail around midnight.
@javierm javierm force-pushed the backport-make-milestones-polymorphic branch 2 times, most recently from 2231d70 to 4419599 Compare November 30, 2018 15:49
@javierm javierm changed the title [WIP] [Backport] Make milestones polymorphic [Backport] Make milestones polymorphic Nov 30, 2018
@javierm javierm force-pushed the backport-make-milestones-polymorphic branch from 4419599 to e5e871f Compare November 30, 2018 16:12
We were having problems under certain conditions with Travis and
Knapsack where tasks were still being loaded twice and so they were
being executed twice.
@javierm javierm force-pushed the backport-make-milestones-polymorphic branch from e5e871f to ea95760 Compare November 30, 2018 16:29
@microweb10 microweb10 self-requested a review December 4, 2018 08:27
@javierm javierm merged commit 4f9ca04 into master Dec 5, 2018
@javierm javierm deleted the backport-make-milestones-polymorphic branch December 5, 2018 11:12
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.

None yet

5 participants