-
Notifications
You must be signed in to change notification settings - Fork 4
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
UML-3136: Update viewer journey templates to support combined LPAs #2903
Merged
cooperaj
merged 34 commits into
main
from
UML-3136-update-viewer-templates-combined-lpa
Nov 27, 2024
Merged
UML-3136: Update viewer journey templates to support combined LPAs #2903
cooperaj
merged 34 commits into
main
from
UML-3136-update-viewer-templates-combined-lpa
Nov 27, 2024
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2903 +/- ##
============================================
+ Coverage 91.25% 91.78% +0.53%
- Complexity 1504 1765 +261
============================================
Files 309 383 +74
Lines 6393 8196 +1803
============================================
+ Hits 5834 7523 +1689
- Misses 542 656 +114
Partials 17 17
|
cooperaj
reviewed
Nov 7, 2024
service-front/app/src/Viewer/templates/viewer/check-code-found-combined-lpa.html.twig
Show resolved
Hide resolved
service-front/app/src/Viewer/templates/viewer/enter-code.html.twig
Outdated
Show resolved
Hide resolved
allenannom
reviewed
Nov 19, 2024
allenannom
approved these changes
Nov 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
docker
Pull requests that update Docker code
php
Pull requests that update Php code
service-api
service-front
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.
Purpose
So, this was a bit of a slog, but from testing the viewer journey for the current LPA format, combined Sirius, and combined LPA-store LPAs, it looks like it's working.
I went with new templates to support the combined LPA rather than adding logic to existing templates as this felt a bit cleaner and should hopefully have less cleanup in the future.
I made an attempt to start the API work that would be required to support this once the mock API response is removed but the PR was already getting huge so I moved it to its own branch. Feel free to disregard if the API work isn't picked up for a while as it will quickly be outdated.
I ran out of time before I could add behat tests but it may be worth waiting to implement them when the data model has solidified and the API side has been implemented.
I made a few additions to the front end unit tests such as using some test helpers rather than relying on JSON files to get an LPA instrumented and ready to test.
Hopefully, this all makes sense and I haven't made any rash assumptions on the format of things but shout up if you want to talk through any of the PR and we can jump on a call.
Fixes UML-3136