add all regions to activity report dropdown if user has central office #357
Conversation
thewatermethod
requested review from
AdamAdHocTeam,
jasalisbury,
kryswisnaskas and
SarahJaine
|
Looks good! Thanks for adjusting the width of the region selector to match the mockup. One question/comment on the code. I also verified that the "All regions" option displays data from all regions user has read permissions to. As a side note, there could be a slight inconsistency if a central office user wasn't explicitly given a read access to all 12 regions. This shouldn't really happen if an admin is careful, but I think we could point that out so the customers will understand the behavior and not be surprised. |
| @@ -20,7 +20,7 @@ const DropdownIndicator = (props) => ( | |||
|
|
|||
| const Placeholder = (props) => <components.Placeholder {...props} />; | |||
|
|
|||
| export const getUserOptions = (regions) => regions.map((region) => ({ value: region, label: `Region ${region}` })); | |||
| export const getUserOptions = (regions) => regions.map((region) => ({ value: region, label: `Region ${region}` })).sort((a, b) => a.value - b.value); | |||
There was a problem hiding this comment.
Is the sorting still needed?
There was a problem hiding this comment.
A comment and a question:
This region dropdown has the same problem as in the other PR, needs to be a button. Also will need to close on blur. (which doesn't seem to be an issue this PR created)
Do we need to remove the "Create Report" button when central office is the region selected? I think so because we won't know what region to use for the report (unless we want region 14 reports, which my initial reaction is that would be a mistake)
|
We do have a JIRA I opened on the button/react-select keyboard nav issue when the regional dropdown was originally merged. Since it's fixed in the 167 PR (and already in prod as the react-select component), maybe it's ok to leave alone for this PR? Not sure. Easy enough remove "Create report" if we think that's best. Right now it will create a report in the first actual region (i.e. the region not 14) even if 14 is selected. Also not sure about this, as far as what the expected behavior is. |
Leaving it to be fixed in #332 sounds good to me!
Not having the button seem preferable to clicking it not knowing which region the report is being created in. My understanding is central office users shouldn't be creating activity reports, but maybe check with Patrice at the next standup. |
Description of change
Added "All Regions" to the region select dropdown on the activity-reports page if the user has "central office" as their home region.
How to test
Navigate to the landing page as a user with central office (14) as their home region. Notice that "All regions" is present in the dropdown. Switch between regions. The alerts and reports should change for that region.
Issue(s)
Checklists
Every PR
N/A
Production Deploy
After merge/deploy