-
Notifications
You must be signed in to change notification settings - Fork 2
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
FFS-2466: Rename cbv_flow identifier column for clarity #472
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, but, can you remove the old migration change before merging?
@@ -1,5 +1,5 @@ | |||
class AddEndUserIdColumnToCbvFlows < ActiveRecord::Migration[7.1] | |||
def change | |||
add_column :cbv_flows, :pinwheel_end_user_id, :uuid, default: 'gen_random_uuid()', null: false | |||
add_column :cbv_flows, :end_user_id, :uuid, default: 'gen_random_uuid()', null: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, not a good idea to edit migrations that have already been run, since this would break the future migration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof, that's a mistake. My bad, thanks for catching
# Conflicts: # app/db/schema.rb # docs/app/rendered/database-schema.pdf
0bd5be0
to
91963b6
Compare
# Conflicts: # app/db/schema.rb # docs/app/rendered/database-schema.pdf
Ticket
Resolves FFS-2466.
Changes
Renames cbv_flow identifier column so that it's clear it can be used simply to identify the flow
Context for reviewers
It was confusing how this column was named. We use this column to identify which flow the incoming aggregator account is meant for.
Acceptance testing
:alert: Deploy block! @ffs-eng I just merged PR [#123] and will be doing acceptance testing in demo - please don't deploy until I'm finished!
)