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

AP-4575: Add copy case page flows #5932

Merged
merged 32 commits into from
Nov 13, 2023
Merged

Conversation

jsugarman
Copy link
Contributor

@jsugarman jsugarman commented Nov 1, 2023

What

Add copy case feature

Link to story

The ability to copy case details from one application to another
is considered a prerequisite for the linking cases feature.

This branch adds the pages flows but does not do a full clone/copy of the necessary case details - see AP-4577 for that. Neither does it add the CYA page changes - see AP-4601 for that

Checklist

Before you ask people to review this PR:

  • Tests and rubocop should be passing: bundle exec rake
  • Github should not be reporting conflicts; you should have recently run git rebase main.
  • There should be no unnecessary whitespace changes. These make diffs harder to read and conflicts more likely.
  • The PR description should say what you changed and why, with a link to the JIRA story.
  • You should have looked at the diff against main and ensured that nothing unexpected is included in your changes.
  • You should have checked that the commit messages say why the change was made.

@jsugarman jsugarman force-pushed the ap-4575/copy-case-page-flows branch from 9877d46 to 0dee58a Compare November 1, 2023 14:19
@jsugarman jsugarman mentioned this pull request Nov 1, 2023
7 tasks
@jsugarman jsugarman force-pushed the ap-4575/copy-case-page-flows branch 8 times, most recently from a6a5de5 to 26b334b Compare November 6, 2023 14:01
To store answer to question "Copy an application to your application?"
inline with designs.
Add controller form and view to ask the user
if they want to copy an existing application to this
one.
Use deep_clonable. This is one option, amoeba being
another.
Required and used as an example of using
deep clone to copy nested associations.
Cover the conditional flow for copy case
Inline with conventions used for other controllers
Spec the additional validation whereby search only checks for
provider specific case references.
Confirmed with design as users may want to go and lookup
a reference on the application index page and this would allow them to
go back and forth to the correct step.
@jsugarman jsugarman marked this pull request as ready for review November 7, 2023 11:49
@jsugarman jsugarman requested a review from a team as a code owner November 7, 2023 11:49
@jsugarman jsugarman added the ready for review Please review label Nov 7, 2023
@colinbruce
Copy link
Contributor

An observation from outside the team...
Could the copy_case controllers, views, etc be namespaced into a folder to prevent the provider namespace bloating?
e.g. app/controllers/providers/copy_case/searches, etc ?
It helped with Partner work that we could find a section easier as it was in it's own namespaced folder :)

Copy link
Contributor

@colinbruce colinbruce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, a couple of observations from a linked-cases non-expert!

So that we can store the id of the application for copying from
for use in both the search and confirmation pages withtout regard to
session expiry edge cases, plus use the legal aid application reference
in the check your answers page in line with design.
@jsugarman
Copy link
Contributor Author

An observation from outside the team... Could the copy_case controllers, views, etc be namespaced into a folder to prevent the provider namespace bloating? e.g. app/controllers/providers/copy_case/searches, etc ? It helped with Partner work that we could find a section easier as it was in it's own namespaced folder :)

Yeah, I tried namespacing the controllers and views first and hit problems, will have another look.

@jsugarman jsugarman force-pushed the ap-4575/copy-case-page-flows branch from 91f7e49 to eb29620 Compare November 8, 2023 09:32
@jsugarman
Copy link
Contributor Author

An observation from outside the team... Could the copy_case controllers, views, etc be namespaced into a folder to prevent the provider namespace bloating? e.g. app/controllers/providers/copy_case/searches, etc ? It helped with Partner work that we could find a section easier as it was in it's own namespaced folder :)

This is done now. see commit

Copy link
Contributor

@kmahern kmahern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

Copy link
Contributor

@agoldstone93 agoldstone93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment

app/controllers/providers/copy_case/searches_controller.rb Outdated Show resolved Hide resolved
Copy link

sonarqubecloud bot commented Nov 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jsugarman jsugarman added UAT approved Approved by code reviewers and removed ready for review Please review labels Nov 10, 2023
@jsugarman jsugarman merged commit 7691800 into main Nov 13, 2023
7 checks passed
@jsugarman jsugarman deleted the ap-4575/copy-case-page-flows branch November 13, 2023 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved by code reviewers UAT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants