human-essentials
human-essentials copied to clipboard
rubyforgood
3041: Allow opting out of child/family functionality
Resolves #3041
Description
This adds the capability for partners or banks to opt out of child and family requests.
I ran out of time before I was able to get to the last few couple of specs which tests that the rendered HTML hides the UI elements correctly. Up to the reviewer whether those are blockers to merging this.
Type of change
- New feature (non-breaking change which adds functionality)
How Has This Been Tested?
Local testing
Screenshots
If I read the test results right, it's failing on something related to partner_form -- so might be a legit failure?
In general whenever we find ourselves doing the same thing in more than one place, you'll want to have it run through a single piece of code. Otherwise you have to remember to change it in exactly the same way in both places, and it becomes easy to miss one of them. Also in general for Rails, it's best to make controllers as small as possible because it's harder (and slower) to write and run tests for them - so the more we can move into "plain Ruby" the better.
In general whenever we find ourselves doing the same thing in more than one place, you'll want to have it run through a single piece of code. Otherwise you have to remember to change it in exactly the same way in both places, and it becomes easy to miss one of them. Also in general for Rails, it's best to make controllers as small as possible because it's harder (and slower) to write and run tests for them - so the more we can move into "plain Ruby" the better.
So not something particularly required for this request, but general cleanup.
Correct - I realized that I'd have to make this change in two places, and took the opportunity to consolidate them into one.
@cielf can you specify what you think would make more sense?
- If it doesn't belong under agency_stability, where would it go?
- Does this mean that the checkbox for partners would only ever be turned off if the value is changed for the org, but never on?
1/ I had thought that they should just be with the initial information that is never turned off, but I'm now wondering if maybe the three toggles (because, you know, after you coded these, someone else asked for being able to take out quantity as well), should be in their own section, at the end?
2/ I think that makes sense from a confusion of the partners point of view. The bank is saying "you can't use this" by turning it off, and saying "you may use this" by turning it on. It's not saying "you have to use this." It may be a right royal pain if a bank makes a mistake, though. We could leave it as it is and see if anyone yelps.
@cielf I've moved the settings to their own section / partial:
Let me know if you still want to change the other behavior or if we can/should merge as is.
@dorner I will take the question of how changing the values at the bank level should behave to the partnerbase group next week.
Could we make the new partial controllable by the banks? [Like the other ones] I know there are some banks that will want to just take the decision out of the hands of the partners.
Right now the settings themselves are controlled by the banks. Are you suggesting making yet another bank setting controlling whether this pane even shows up? I'd think that was unnecessary.
@dorner The banks control whether the partners can have child/individual at all. However, for partner simplicity sake, at least some of them would prefer if the partners don't see the option to opt out.
@dorner -- Alas, no one showed for yesterday's partnerbase working group meeting, so no definite answer. Here's what I think we should do: 1/ Go ahead with what we have -- my concern is unlikely to cause much confusion in the next month 2/ raise that issue at the full stakeholders circle, and address it quickly after that (I think it's a small code change) and 3/ put the control of the section as a separate issue. If you're cool with that, I would approve once the latest changes from main are merged in.
