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

Add a PageRepository and reroute Page model usage through it #1587

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SamJamCul
Copy link
Contributor

@SamJamCul SamJamCul commented Nov 11, 2024

The PageRepository being introduced is used as an interface between the application and any Page method calls that result in an API call through ActiveResource. The intention is to create a layer of separation, so that database requests can be rerouted more easily as part of the Admin/API responsibilities shifting.

What problem does this pull request solve?

Trello card: https://trello.com/c/QuEeu8uI/1939-add-repository-service-to-forms-admin-for-form-page-model

This PR adds the PageRepository, which is intended to act as a buffer between the app and any ActiveResource API usage when using the Page model. I've tried to go through and find all the spots where we use Page model requests, it's a little fiddly!

As well as CRUD ops, it also implements the move_page, as this also hooks into the API.

If you know of anywhere else that we use the API's Page model, please leave a comment and I'll reroute it through the PageRepository.

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?

@SamJamCul SamJamCul force-pushed the 1939-add-repository-service-to-forms-admin-for-form-page-model branch from e20d3bb to ffad1e4 Compare November 11, 2024 16:51
@SamJamCul SamJamCul force-pushed the 1939-add-repository-service-to-forms-admin-for-form-page-model branch 2 times, most recently from 82497b0 to 7f71685 Compare November 12, 2024 16:20
Removing tests that were specifically designed to operate during the implementation for the `Add another answer` feature. These test are no longer useful, as the attribute that they're focused on, `is_repeatable` behaves as any other attribute.
The PageRepository being introduced is used as an interface between the application and any Page method calls that result in an API call through ActiveResource. The intention is to create a layer of separation, so that database requests can be rerouted more easily as part of the Admin/API responsibilities shifting.
The delete confirmation controller handles the deletion process for forms and pages - but the tests only covered the form deletion process. Adds tests for the page versions of the delete and destroy actions
@SamJamCul SamJamCul force-pushed the 1939-add-repository-service-to-forms-admin-for-form-page-model branch from 7f71685 to 9b95b1c Compare November 15, 2024 12:09
Copy link

sonarcloud bot commented Nov 15, 2024

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