Remove PageRepository #2262
Remove PageRepository #2262
Conversation
Replace it with direct ActiveRecord queries
We can call the `Form#save_draft!` method to take care of the Form state change if the form is live/archived rather than duplicating this logic.
stephencdaly
force-pushed
the
remove-page-repository
branch
2 times, most recently
from
19b4378 to
711011b
Compare
Add a new Page.create_and_update_form! method to replace the PageRepository.create! method. Added this rather than updated the form in an `after_create` callback as it's unhelpful to have the callback run after creating a page as part of tests.
Replace with calling Page#save_and_update_form directly.
Replace with direct model calls
Replace with direct call to Page#move_page. This was the last remaining method in PageRepository, so the class has been deleted.
stephencdaly
force-pushed
the
remove-page-repository
branch
from
711011b to
ef17246
Compare
|
|
🎉 A review copy of this PR has been deployed! You can reach it at: https://pr-2262.admin.review.forms.service.gov.uk/ It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready For the sign in details and more information, see the review apps wiki page. |
| def self.create_and_update_form!(form_id:, | ||
| question_text:, | ||
| hint_text:, | ||
| is_optional:, | ||
| is_repeatable:, | ||
| answer_settings:, | ||
| page_heading:, | ||
| guidance_markdown:, | ||
| answer_type:) | ||
| page = Page.new( | ||
| form_id:, | ||
| question_text:, | ||
| hint_text:, | ||
| is_optional:, | ||
| is_repeatable:, | ||
| answer_settings:, | ||
| page_heading:, | ||
| guidance_markdown:, | ||
| answer_type:, | ||
| ) |
There was a problem hiding this comment.
Rather than listing all the attributes and creating extra work for ourself, we can use the Ruby argument forwarding syntax [1]
| def self.create_and_update_form!(form_id:, | |
| question_text:, | |
| hint_text:, | |
| is_optional:, | |
| is_repeatable:, | |
| answer_settings:, | |
| page_heading:, | |
| guidance_markdown:, | |
| answer_type:) | |
| page = Page.new( | |
| form_id:, | |
| question_text:, | |
| hint_text:, | |
| is_optional:, | |
| is_repeatable:, | |
| answer_settings:, | |
| page_heading:, | |
| guidance_markdown:, | |
| answer_type:, | |
| ) | |
| def self.create_and_update_form!(...) | |
| page = Page.new(...) |
| end | ||
|
|
||
| unless PageRepository.destroy(page) | ||
| unless page.destroy |
There was a problem hiding this comment.
I think we need to call destroy_and_update_form here?
What problem does this pull request solve?
Trello card: https://trello.com/c/KqWovTZq/2506-remove-repository-service-from-forms-admin
The PageRepostory was added for the purpose of migrating from making requests to forms-api using ActiveResource to updating forms in the forms-admin database using ActiveRecord.
Now that the migration is completed, we can remove this service and call methods on the Page ActiveRecord model directly
Things to consider when reviewing