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.

Sign up for GitHub

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

Jump to bottom

Decompose add year group #774

Merged
merged 6 commits into from
Jun 18, 2024
Merged

Decompose add year group #774

merged 6 commits into from
Jun 18, 2024

Conversation

Kizr
Copy link
Contributor

@Kizr Kizr commented Jun 17, 2024

Context

The wizard is becoming unmanageable and unmaintainable it, we need to begin refactoring the logic to fix these issues before further development is needed.

Changes proposed in this pull request

  • Add year group step
  • Use the step rather than the placement for selecting a year group
  • Remove redundant logic

Guidance to review

  • Log in as Anne
  • Add a placement
  • Confirm the year group has saved as expected
  • There should be no discernable changes to the journey

Link to Trello card

Decompose the Placements Build model into Wizard Steps

@Kizr Kizr self-assigned this Jun 17, 2024
@Kizr Kizr requested review from a team as code owners June 17, 2024 16:41
@Kizr Kizr changed the base branch from main to ba/decompose_add_mentors June 17, 2024 16:44
ollietreend
ollietreend reviewed Jun 18, 2024
View reviewed changes
Copy link
Contributor

@ollietreend ollietreend left a comment

Choose a reason for hiding this comment

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

This is looking good 😍 just a couple of things 🙏🏻

spec/wizards/placements/add_placement/steps/year_group_spec.rb Outdated Show resolved Hide resolved
app/models/placements/school.rb Outdated Show resolved Hide resolved
@Kizr Kizr force-pushed the ba/decompose_add_year_group branch from df8639b to 54dcb53 Compare June 18, 2024 11:15
ollietreend
ollietreend approved these changes Jun 18, 2024
View reviewed changes
Copy link
Contributor

@ollietreend ollietreend left a comment

Choose a reason for hiding this comment

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

This looks great! 😍 Thanks for making those changes.

I've left one final minor comment, but it's not a blocker. So I'm approving ✅

spec/wizards/placements/add_placement/steps/year_group_spec.rb Outdated Show resolved Hide resolved
Base automatically changed from ba/decompose_add_mentors to main June 18, 2024 15:20
@Kizr Kizr force-pushed the ba/decompose_add_year_group branch from 54dcb53 to ef250f7 Compare June 18, 2024 15:22
@Kizr Kizr merged commit e83c448 into main Jun 18, 2024
8 checks passed
@Kizr Kizr deleted the ba/decompose_add_year_group branch June 18, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ollietreend ollietreend ollietreend approved these changes

Assignees

@Kizr Kizr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Kizr @ollietreend