Refactoring to allow SES submission service to load answers from the database #1212
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stephencdaly
changed the title
stephencdaly
force-pushed
the
allow-submission-services-to-load-answers-from-the-database
branch
3 times, most recently
from
e1e6ec9 to
a09df26
Compare
stephencdaly
changed the title
stephencdaly
force-pushed
the
allow-submission-services-to-load-answers-from-the-database
branch
2 times, most recently
from
821d22c to
12b10c4
Compare
jamie-o-wilkinson
force-pushed
the
allow-submission-services-to-load-answers-from-the-database
branch
2 times, most recently
from
2e89943 to
4e66a22
Compare
stephencdaly
force-pushed
the
allow-submission-services-to-load-answers-from-the-database
branch
2 times, most recently
from
6327cae to
f4a0410
Compare
DavidBiddle
approved these changes
Feb 20, 2025
SesEmailFormatter only needs the completed_steps from the Flow::Context, so we can pass just that in. This refactor will enable us to refactor the form submission service to only receive what it needs from the current context. Co-authored-by: Stephen Daly <[email protected]> Co-authored-by: David Biddle <[email protected]>
The CsvGenerator only needs all_steps from the current_context, so we can pass only this in, instead of the entire current context. This will allow us to refactor AwsSesSubmissionService. Co-authored-by: David Biddle <[email protected]> Co-authored-by: Stephen Daly <[email protected]>
Move all methods in `Context` that interact with the StepFactory into the Journey class. The means that the Journey now has responsibility for constructing `Steps` and navigating the form filling journey based on the saved answers. `Context` now becomes a wrapper for `FormContext` and `Journey` methods, tying them together to read from the session for an active form. In order to store submission data in the database and send the email later, we want to be able to construct an instance of `Journey` from the data stored in the database rather than the session, and this refecator gets us closer to being able to do this. Co-authored-by: David Biddle <[email protected]> Co-authored-by: Jamie Wilkinson <[email protected]> Co-authored-by: Stephen Daly <[email protected]>
Add a `ConfirmationDetailsStore` which just deals with storing the submission confirmation details on the session so that they can be displayed back to the user on the confirmation page. We want to separate this from `FormContext` so that this just deals with the answers. The end goal is to rename this to be `AnswerStore` which can be populated from either the session or from submission details we will store in the database. Co-authored-by: Jamie Wilkinson <[email protected]> Co-authored-by: David Biddle <[email protected]>
Improve a test which was confusingly creating a second FormContext to check that we weren't deleting answers for a different form. A single FormContext deals with all forms so this was unnecessary. Rename a variable from `step2` -> `other_form_step` to make it clearer what is used for. Co-authored-by: David Biddle <[email protected]> Co-authored-by: Jamie Wilkinson <[email protected]>
The FormContext now only deals with saving and retrieving answers from the session. Rename this class to SessionAnswerStore to more accurately reflect its purpose. We will subsequently add a DatabaseAnswerStore class which will implement the `get_stored_answer` method. This will allow us to create a `Journey` using either the SessionAnswerStore or DatabaseAnswerStore. Co-authored-by: Jamie Wilkinson <[email protected]> Co-authored-by: David Biddle <[email protected]>
We've added a new Store module to group classes that deal with saving/retrieving data from the session or other data store. Move the SessionAnswerStore to this module. Co-authored-by: David Biddle <[email protected]> Co-authored-by: Jamie Wilkinson <[email protected]>
We've renamed FormContext -> SessionAnswerStore. Rename the methods in the Step classes to reflect this rename. Co-authored-by: Jamie Wilkinson <[email protected]> Co-authored-by: David Biddle <[email protected]>
We want to share these methods with the new class we'll add to retreive answers from the submission data stored in the database, so move to a module for reuse. Co-authored-by: Jamie Wilkinson <[email protected]> Co-authored-by: David Biddle <[email protected]>
In order to construct an instance of `Journey` and build the steps based on the stored answers, it needs an instance of a class that implements the `get_stored_answer` method. The new DatabaseAnswerStore class represents answers loaded from the database, where we will store a hash with the page IDs as keys and the answers as values. Add tests to ensure that the Journey works with either a `SessionAnswerStore` or a `DatabaseAnswerStore`, and a shared example included in the unit tests for these two classes to enforce the shared interface. Co-authored-by: Jamie Wilkinson <[email protected]> Co-authored-by: David Biddle <[email protected]>
We are now able to theoretically construct a Journey from data stored in the database by using a DatabaseAnswerStore. Construct the AwsSesSubmissionService with the Journey rather than the CurrentContext to uncouple sending submission emails from interacting with the session. This gets us closer to being able to store the submission data in the database and send emails using the database record. This will allow us to: * Send emails asynchronously from a background process * Retry bounced emails
Use ActiveSupport::Delegate to delegate methods in the Context to reduce verbosity. We'd be able to delegate more methods if we were to pass the form ID into the SessionAnswerStore and ConfirmationDetailsStore.
Pass the `form_id` into the SessionAnswerStore so we can delegate most of the remaining methods that `Context` had wrapper methods for to reduce verbosity.
Pass the `form_id` into the ConfirmationDetailsStore so we can delegate the remaining methods that `Context` had wrapper methods for to reduce verbosity. Co-authored-by: Stephen Daly <[email protected]>
We have refactored AwsSesSubmissionService to accept a journey and form on initialisation rather than the current context. This commit refactors NotifySubmissionService so that all submission services are consistent. Co-authored-by: David Biddle <[email protected]> Co-authored-by: Stephen Daly <[email protected]>
We have refactored AwsSesSubmissionService to accept a journey and form on initialisation rather than the current context. This commit refactors S3SubmissionService so that all submission services are consistent. Co-authored-by: David Biddle <[email protected]> Co-authored-by: Stephen Daly <[email protected]>
jamie-o-wilkinson
force-pushed
the
allow-submission-services-to-load-answers-from-the-database
branch
from
f4a0410 to
26a7189
Compare
|
stephencdaly
deleted the
allow-submission-services-to-load-answers-from-the-database
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this pull request solve?
Trello card: https://trello.com/c/lW3IqOhY
This PR contains refactoring to uncouple the AwsSesSubmissionService (and NotifySubmissionService S3SubmissionService) from interacting with the form filler's session.
This gets us closer to being able to store the submission data in the database and send emails via SES using the database record. This will allow us to:
Things to consider when reviewing