Add transaction model #4548
Open
Add transaction model #4548
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
leenagupte
force-pushed
the
create-transaction-model
branch
from
c8dc6e1 to
8f32535
Compare
leenagupte
force-pushed
the
create-transaction-model
branch
from
8f32535 to
6248af7
Compare
leenagupte
force-pushed
the
create-transaction-model
branch
from
6248af7 to
dcd176c
Compare
leenagupte
force-pushed
the
create-transaction-model
branch
from
dcd176c to
9496f3e
Compare
leenagupte
force-pushed
the
create-transaction-model
branch
from
9496f3e to
7f78b62
Compare
leenagupte
force-pushed
the
create-transaction-model
branch
from
7f78b62 to
396c791
Compare
leenagupte
force-pushed
the
create-transaction-model
branch
from
396c791 to
796598d
Compare
leenagupte
force-pushed
the
create-transaction-model
branch
from
796598d to
4e24bd8
Compare
leenagupte
force-pushed
the
create-transaction-model
branch
from
4e24bd8 to
34acded
Compare
leenagupte
force-pushed
the
create-transaction-model
branch
from
34acded to
f73d075
Compare
This value hasn't been needed since the tracking was switched to use the built-in tracking provided by the tab component. See: a030c99#diff-8ca3371a119b5d7075182adfa23a45626a4e44e8a1cd3185ffe335ec1488354bL20
We currently have two ways to model content items and two content item presenters. * ContentItem models the response from content store * ContentItemModelPresenter takes a ContentItem object and adds a presentation layer to it. * ContentItemPresenter does both, it models the response from content store and adds presentation to it. Ideally there would be content item models that inherit from ContentItem and content item presenters that inherit from ContentItemModelPresenter. And when the original ContentItemPresenter is removed, to rename ContentItemModelPresenter to ContentItemPresenter. To achieve this we need to look at the existing presenters one by one and move the logic to the appropriate place. In the [TransactionPresenter] only the "start_button_text" methods should remain in the presenter. All of the other methods, especially the "pass through" methods model items from the content store, and need to be moved to the model. The `tab_count` method from the presenter has been renamed to `section_count` in the model as this sounds less presentational and it matches the naming of the `multiple_more_information_sections?` method. The `section_count` method has been made private as it is no longer being used in the views. A `start_button_text` attribute as been added so that it can be used in the `start_button_text` method in the presenter, the only method that should remain. Transactions can have variants, so it wasn't as simple as adding attributes for every item as these rely on knowing whether a variant was selected and what that variant is. This information is known when the controller is called, but the way models are instantiated means that we can't pass in document type specific extra parameters. Instead the pattern that was established by travel advice parts is being used here. A "set variant" method has been added to set the value of the variant after the model instance has been created. The first time any of the attributes will be used in the view, but which point the variant value should be known to the model instance. The transaction presenter will then be updated to accept an instance of this model as a parameter in the next commit. [TransactionPresenter]: https://github.com/alphagov/frontend/blob/0ff9340c529ccd85072c2d9bd660fdbfb3f0fab1/app/presenters/transaction_presenter.rb Fix up to model
Also removes the modelling code from the presenter, and updates the `start_button_text` method to use attributes from the model.
This modifies the content item object to return values from the variant if requested.
leenagupte
force-pushed
the
create-transaction-model
branch
from
f73d075 to
4d3661b
Compare
Replaces the use of "publication" object in the views and wraps the content item object as a publication. The publication object cannot be removed completely as the application layout uses it to determine whether to show a breadcrumb. Also fixes failing transaction request tests. These tests relied on a publication instance being created, whereas now the publication method just returns the content item instance.
The partial templates have access to the content item object, so locals aren't needed.
leenagupte
force-pushed
the
create-transaction-model
branch
from
4d3661b to
c8cc3ed
Compare
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
Move modelling code from TransactionPresenter to a Transaction model.
Why
We currently have two ways to model content items and two content item presenters.
Ideally there would be content item models that inherit from ContentItem and content item presenters that inherit from ContentItemModelPresenter.
And when the original ContentItemPresenter is finally removed, to rename ContentItemModelPresenter to ContentItemPresenter.
To achieve this we need to look at the existing presenters one by one and move the logic to the appropriate place.
How
The code that was in the original TransactionPresenter has been split between the TransactionPresenter and the new Transaction model.
TransactionPresenter has also been updated so that it's now initialized with an instance of the model.
Screenshots?
N/A - There shouldn't be any visible changes.
Example transaction: https://govuk-frontend-app-pr-4548.herokuapp.com/register-to-vote