-
Notifications
You must be signed in to change notification settings - Fork 26
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
[#3524] Ignore family members without BSN #3561
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3561 +/- ##
=======================================
Coverage 95.97% 95.97%
=======================================
Files 679 679
Lines 21840 21844 +4
Branches 2519 2521 +2
=======================================
+ Hits 20960 20964 +4
Misses 610 610
Partials 270 270
☔ View full report in Codecov by Sentry. |
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 fine, so I approve, just a remark and a question:
-
The v2 mocks return a different response for essentially the same call to the same endpoint. If I understand correctly, the real endpoint response would always contain both children and partners. I'd try to avoid these kinds of mocks that assume a certain implementation. In this case it never tries to reference the partners key if they aren't requested. For instance: a refactor of the implementation that always processes everything from the api response and only filters just what's being asked in the answer, would work against the real api, but trigger these tests. (contrived example, but in essence, I like be able to refactor without changing the test suite and have the tests as a canary in the coal mine to detect when I broke something)
-
Is just mentioning the omission of people without BSN is enough information for a form designer to be able to cater for these cases? Do we need to give examples or suggestions, like a repeating group for "missing familiy members"? I guess they already have to deal with submitters that have no BSN themselves and when the BRP services are unreachable for some reason.
They definitely can't cater for these cases, but to be able to do that they're going to have to tell us how they expect to handle that. I think we have too little domain knowledge and attempts to get that knowledge from actual teams building forms have been unproductive. |
My sausage fingers accidentally closed the PR, sorry! |
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.
Please rebase on master to sort out the CI failures
410ed1b
to
4db05df
Compare
Fixes #3524