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.

Sign up for GitHub

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

Jump to bottom

AP-4567: Add non-passported to list of reasons on means report #5926

Merged
merged 3 commits into from
Nov 7, 2023
Merged

AP-4567: Add non-passported to list of reasons on means report #5926

merged 3 commits into from
Nov 7, 2023

Conversation

agoldstone93
Copy link
Contributor

What

Link to story

Add "Non-Passported application" to list of review reasons on means report if the application is non-passported. This is to avoid a situation whereby caseworkers are flagged to review an application but the list of review reasons is empty

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.

@agoldstone93 agoldstone93 force-pushed the AP-4567-means-report-add-non-passported-reason branch from 65f8b15 to 128abc0 Compare November 1, 2023 09:18
@agoldstone93 agoldstone93 marked this pull request as ready for review November 1, 2023 09:30
@agoldstone93 agoldstone93 requested a review from a team as a code owner November 1, 2023 09:30
@agoldstone93 agoldstone93 added the ready for review Please review label Nov 1, 2023
jsugarman
jsugarman approved these changes Nov 1, 2023
View reviewed changes
Copy link
Contributor

@jsugarman jsugarman left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM 👍 and no blockers, however the ManualReviewDeterminer#review_reasons spec does not seem to test for the passported or non-means-tested route not having any review reasons as I would have hoped for. It may be worth adding a context to that effect if you have time

@agoldstone93 agoldstone93 force-pushed the AP-4567-means-report-add-non-passported-reason branch from 128abc0 to 59990df Compare November 2, 2023 08:29
@jsugarman jsugarman added approved Approved by code reviewers and removed ready for review Please review labels Nov 3, 2023
agoldstone93 and others added 3 commits November 7, 2023 07:46
@agoldstone93
AP-4567: Add non-passported to list of reasons on means report
9d2c477
@agoldstone93
AP-4567: add spec for passported applications
a1bf445
@jsugarman @agoldstone93
AP-4567: Refactor manual_review_determiner_spec
1d8eeed 
Remove let!(:cfe_submission) to prevent unexpected actors/behaviour and
move let declarations to their relevant contexts so we can remove
unneeded declarations.
@agoldstone93 agoldstone93 force-pushed the AP-4567-means-report-add-non-passported-reason branch from 3b50811 to 1d8eeed Compare November 7, 2023 07:47
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Nov 7, 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 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@agoldstone93 agoldstone93 merged commit ff20418 into main Nov 7, 2023
7 checks passed
@agoldstone93 agoldstone93 deleted the AP-4567-means-report-add-non-passported-reason branch November 7, 2023 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsugarman jsugarman jsugarman approved these changes

Assignees
No one assigned
Labels
approved Approved by code reviewers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@agoldstone93 @jsugarman