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

EL-1756: Create new field in the completed user journeys table to record EE result #1591

Closed

Conversation

MazOneTwoOne
Copy link
Contributor

Jira ticket

What changed and why

  • amend completed user journey table for early_result_type

Guidance to review

  • n/a

Checklist

Before you ask people to review this PR:

  • Tests and rubocop should be passing
  • Branch is generally up to date with main Github - definitely no conflicts
  • No unnecessary whitespace changes. These make diffs harder to read and conflicts more likely.
  • PR description says what changed and why, with a link to the JIRA story.
  • Diff has been checked for unexpected changes being included.
  • Commit messages say why the change was made.

@MazOneTwoOne MazOneTwoOne marked this pull request as ready for review October 9, 2024 12:01
@MazOneTwoOne MazOneTwoOne requested a review from a team as a code owner October 9, 2024 12:01
Copy link
Contributor

@psweeting1 psweeting1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MazOneTwoOne MazOneTwoOne added Approved UAT Keep PR open to keep ephemeral UAT URL alive and removed Ready For Review labels Oct 9, 2024
@@ -0,0 +1,11 @@
class AmmenedCompletedUserJourneyForEarlyResultType < ActiveRecord::Migration[7.1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you've got a typo here in the class name - but on another point, normally with migration file naming, the standard is to use the present tense, and use words like 'add', 'create', 'remove' etc (you can take a look at the older migrations for some good examples - something like AddEarlyResultTypeToCompletedUserJourneys)

Also you are probably already aware of this but just in case - to generate these files you can run some commands, which give you a nice file template to work with i.e. rails generate migration ChangeCompletedUserJourneys.

Other than the naming, looks good!

@MazOneTwoOne
Copy link
Contributor Author

Closing this as having issues with pods and migrations.

@MazOneTwoOne MazOneTwoOne deleted the EL-1756-new-field-in-completed-user-journeys branch October 10, 2024 14:02
@MazOneTwoOne MazOneTwoOne restored the EL-1756-new-field-in-completed-user-journeys branch October 10, 2024 14:21
@MazOneTwoOne MazOneTwoOne deleted the EL-1756-new-field-in-completed-user-journeys branch October 10, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved UAT Keep PR open to keep ephemeral UAT URL alive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants