Refactoring: (step1 of 4) make creator of project clearer #2042
+23
−19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Thw `Project::assigned_to_regional_delivery_officer` is not used. We remove it before refactoring: `Project#regional_delivery_officer` -> `Project#creator` to reflect the reality that transfers and conversions can be created by both RDOs and RCSs.
edavey
force-pushed
the
refactor/step1_make-creator-of-project-clearer
branch
from
f8f6790 to
5347332
Compare
lozette
approved these changes
Dec 18, 2024
The `Project#regional_delivery_officer` association is misleading. It dates from an earlier time when only RGOs could create projects. However, today Regional Casework Services (RCSs) can also create projects and so can have their `user.ids` set in `projects.regional_delivery_officer_id`. That's a recipe for confusion, which has been documented in [The project's "Added by" user is wrong][]. A more accurate name for this association is `Project#creator`. However, renaming a database column (in this case also a foreign key) is best done in steps to avoid errors in code or in referential integrity checks e.g. this scenario at a point during the deployment process: - our migration has run and renamed `projects.regional_delivery_officer_id` to `projects.creator_id` - before our new code which references `projects.creator_id` rather than `projects.regional_delivery_officer_id` has deployed, a user attempts to create a transfer/conversion. BOOM! To avoid these errors and to make the change without downtime we'll stage the refactoring over 4 steps, to be performed **in 4 separate deployments**: 1. (**THIS PR**) introduce new foreign key column --- - create migration to: - add `projects.creator_id` column, allow to be null - add new index on `projects.creator_id` - add new foreign key constraint on `projects.creator_id` - start setting new `projects.creator_id` in addition to existing `projects.regional_delivery_officer_id` (e.g. when a project is created, when the 'added_by' is updated) 2. back-fill new foreign key column --- - run a data migration to populate the new `projects.creator_id` with existing references from `projects.regional_delivery_officer_id` 3. discontinue use of old foreign key column --- - run migration to make `projects.creator_id` non-nullable - Change code to use new `projects.creator_id` in all places. 4. remove old foreign key column --- `remove_foreign_key :projects, regional_delivery_officer_id ` [The project's "Added by" user is wrong]: https://dfe-gov-uk.visualstudio.com/Academies-and-Free-Schools-SIP/_wiki/wikis/Academies-and-Free-Schools-SIP.wiki/8813/Troubleshooting-common-issues-for-support-users-and-support-developers?anchor=the-project%27s-%22added-by%22-user-is-wrong
edavey
force-pushed
the
refactor/step1_make-creator-of-project-clearer
branch
from
5347332 to
ba88c46
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rename project "regional_delivery_officer" to "creator"
The
Project#regional_delivery_officerassociation is misleading. It dates from an earlier time when only RGOs could create projects. However, today Regional Casework Services (RCSs) can also create projects and so can have theiruser.idsset inprojects.regional_delivery_officer_id.That's a recipe for confusion, which Laura has documented in The project's "Added by" user is wrong.
A more accurate name for this association is
Project#creator.However, renaming a database column (in this case also a foreign key) is best done in steps to avoid errors in code or in referential integrity checks e.g. this scenario at a point during the deployment process:
projects.regional_delivery_officer_idtoprojects.creator_idprojects.creator_idrather thanprojects.regional_delivery_officer_idhas deployed, a user attempts to create a transfer/conversion. BOOM!To avoid these errors without taking the service offline we'll perform the refactoring over 4 steps, to be performed in 4 separate deployments:
1. (THIS PR) introduce new foreign key column
projects.creator_idcolumn, allow to be nullprojects.creator_idprojects.creator_idprojects.creator_idin addition to existingprojects.regional_delivery_officer_id(e.g. when a project is created, when the 'added_by' is updated)2. back-fill new foreign key column
projects.creator_idwith existing references fromprojects.regional_delivery_officer_id3. discontinue use of old foreign key column
projects.creator_idnon-nullableprojects.creator_idin all places.4. remove old foreign key column