Skip to content

Comments

Move make live methods into make live form model#1063

Merged
DavidBiddle merged 1 commit intomainfrom
move-make-live-methods
Apr 2, 2024
Merged

Move make live methods into make live form model#1063
DavidBiddle merged 1 commit intomainfrom
move-make-live-methods

Conversation

@DavidBiddle
Copy link
Contributor

@DavidBiddle DavidBiddle commented Apr 2, 2024

What problem does this pull request solve?

Trello card: https://trello.com/c/li50etsD/1432-implement-form-unarchiving (tidying up)

This is a refactor which moves the duplicated user_wants_to_make_form_live and make_form_live methods out of the make_live_controller and the unarchive_controller, and into the make_live_form model.

Things to consider when reviewing

  • Ensure that you consider the wider context.
  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Has all relevant documentation been updated?

@DavidBiddle DavidBiddle marked this pull request as ready for review April 2, 2024 15:12
Copy link
Contributor

@chao-xian chao-xian left a comment

Choose a reason for hiding this comment

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

Thank you for making this change! Can we also reword the commit message with a "why" that the controller shouldn't have such private methods that are really belonging in the domain of the model itself pls?

Moves the user_wants_to_make_form_live and make_form_live methods out of
the make_live_controller and the unarchive_controller and into the
make_live_form model.

This means that the controller no longer holds logic in private methods
that ought to be in the model's domain, reduces duplication between the
two controllers, and makes it easier to unit test these methods.
@DavidBiddle DavidBiddle force-pushed the move-make-live-methods branch from a780054 to 4886dc5 Compare April 2, 2024 15:25
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@DavidBiddle
Copy link
Contributor Author

Thanks @chao-xian, have added that context to the commit message 👍

@DavidBiddle DavidBiddle enabled auto-merge April 2, 2024 15:26
@DavidBiddle DavidBiddle merged commit 72602bb into main Apr 2, 2024
@DavidBiddle DavidBiddle deleted the move-make-live-methods branch April 2, 2024 15:28
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.

2 participants