-
Notifications
You must be signed in to change notification settings - Fork 8
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
[Feature] Claim verification model #10434
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10434 +/- ##
=============================================
+ Coverage 38.61% 71.17% +32.55%
- Complexity 1408 1417 +9
=============================================
Files 995 219 -776
Lines 30570 5464 -25106
Branches 6560 0 -6560
=============================================
- Hits 11805 3889 -7916
+ Misses 18731 1575 -17156
+ Partials 34 0 -34
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
'armed_forces_status' => $this->faker->randomElement(ArmedForcesStatus::cases())->name, | ||
'armed_forces_status' => $this->faker->boolean() ? | ||
ArmedForcesStatus::NON_CAF->name | ||
: $this->faker->randomElement(ArmedForcesStatus::cases())->name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before 2/3 of users were current/past armed forces which seemed a bit egregiously high. This small tweak just halves it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this! 💪 Thanks for the phpunit tests and test queries.
I just finished the readthrough and it's looking great but I think it's missing a few things - mostly my fault. 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great!
🤖 Resolves #10417
👋 Introduction
Add the required fields via migration, set values for existing candidates, schema update, a mutation, and factory tweaks
I think I got the spirit of the issue.
🧪 Testing
May need to adjust permissions before testing locally, I found
/graphiql
was acting odd for me📸 Screenshot