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.

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

Visits for contacts cannot be saved if users don't have access to the converted case #7434

Closed
kwa20 opened this issue Dec 4, 2021 · 13 comments · Fixed by #7506 or #7556
Closed

Visits for contacts cannot be saved if users don't have access to the converted case #7434

kwa20 opened this issue Dec 4, 2021 · 13 comments · Fixed by #7506 or #7556
Assignees
Labels
backend Affects the web backend bug An error or misbehavior of an existing feature (ticket type) contacts major A functional requirement is incorrect or incomplete, ... (Severity for bugs/defects) qa-verified Issue has been tested and verified by QA

Comments

@kwa20
Copy link

kwa20 commented Dec 4, 2021

Bug Description

When a restricted user tries to enter visits in hindsight for contacts that have already been converted to cases, an error message prevents them from doing so when they don't have access to the converted case.

Steps to Reproduce

  1. Create a contact in jurisdiction 1 and convert that contact to a case in jurisdiction 2 (do that with e.g. admin + national user)
  2. Log in as a user in jurisdiction 1 and try entering a visit for the converted contact
  3. Create a control contact and convert it into a case that is in the same jurisdiction
  4. Try entering a visit again with the user in jurisdiction 1

Expected Behavior

The user should be able to create visits for contacts that they have access to.

Screenshots

changedInTheMeantimeErrorVisitsContacts

System Details

  • Device:
  • SORMAS version: 1.66.0
  • Android version/Browser: chrome
  • Server URL: release-international.sormas.netzlink.com
  • User Role: admin, national user

Additional Information

Development Specifications

  • When a contact has been converted to a case, disable the button to create a new visit (but still display it)
  • Add an info icon next to it with the following description on hover: "This contact has already been converted to a case. Please add new visits to the case instead."
@kwa20 kwa20 added bug An error or misbehavior of an existing feature (ticket type) contacts major A functional requirement is incorrect or incomplete, ... (Severity for bugs/defects) labels Dec 4, 2021
@markusmann-vg markusmann-vg added this to Backlog in SORMAS Team 2 - DEV - Iteration Backlog via automation Dec 5, 2021
@MateStrysewske MateStrysewske added this to the Sprint 110 - 1.67.0 milestone Dec 6, 2021
@MateStrysewske MateStrysewske added the backend Affects the web backend label Dec 6, 2021
@BarnaBartha BarnaBartha self-assigned this Dec 8, 2021
@BarnaBartha BarnaBartha moved this from Backlog to In Progress in SORMAS Team 2 - DEV - Iteration Backlog Dec 8, 2021
@BarnaBartha
Copy link
Contributor

BarnaBartha commented Dec 8, 2021

@kwa20 @bernardsilenou @Chinedar
Are we sure we want to allow adding visits to a contact that has been converted to a case. What happens here is that the backend logic also updates the case and adds the visit to it - which also leads to a case edit and the user cannot edit that case as it is out of he's jurisdiction. I can add some condition to have a shortcut and fix it but shouldn't we disable/hide the add visit button for the contact in this situation - or maybe add a warning that visits should be handled from the case?

@MateStrysewske MateStrysewske added the needs-response Response from the issue creator required label Dec 8, 2021
@MateStrysewske MateStrysewske moved this from In Progress to Waiting in SORMAS Team 2 - DEV - Iteration Backlog Dec 8, 2021
@bernardsilenou
Copy link

@BarnaBartha @kwa20

  1. You have stated the main point here, by allowing the user to add visits to the contacts that would later be transferred to the case is giving the user access to the case, thus contradiction.
  2. I do not also think that disable/hide the icon is good because if the user have full assess to the case and contact, then the user has the liberty to either do the visit through the case or through the contact.
  3. I would vote for a pop up message, something like "This contact has been converted to a case in a responsible district that your user right does not permit you to edit the case"

@MateStrysewske
Copy link
Contributor

@bernardsilenou That would be extremely unintuitive because the user does not intend to update the case, they try to add a visit to the contact. The message "You can't edit the case" does not make sense for them in this scenario.

I agree with Barna here, same for @JaquM - @bernardsilenou, could you please explain why anyone would continue to manage a contact if that contact has been converted to a case? Wouldn't, under any circumstances, the case be the entity that is managed afterwards?

@MateStrysewske MateStrysewske added the discuss Discussion about whether or not the issue should or can be implemented required label Dec 9, 2021
@kwa20
Copy link
Author

kwa20 commented Dec 9, 2021

@MateStrysewske @BarnaBartha @bernardsilenou I agree that further editing might not be intended, the only scenario I can imagine where someone might want to add a visit is when adding data in hindsight. The error message displayed here is definitely a hurdle though.

I think either having an intuitive error message or disabling the button would work. However, when showing an error message, we should show it upon pressing "New visit" directly so users don't have to go through entering the form before getting disappointed upon saving.
When we disable the button, users should also somehow know what's going on. We could either use a hover tool tip or we can display an info text to the left of the "new visit" button

@MateStrysewske
Copy link
Contributor

@kwa20 Not sure what's the most user-friendly approach here. I'd probably go with disabling the button (so that it's still displayed, but visibly disabled) and adding an info icon next to it that, when hovered, displays the text "This contact has already been converted to a case. Please add new visits to the case instead."

@Chinedar
Copy link

Chinedar commented Dec 10, 2021 via email

@bernardsilenou
Copy link

Replying to #7434 (comment)

  1. If a user has full access to the case and also the contact, then there is no reason to add visit through the contact after it has been converted to case. Thus the suggested solution would be OK, but I do not think it is the optimum solution when the user does not have access to case.
  2. The main point that I am raising which is basically the reason for this issue is if the user has access to the contact but not to the resulting case, thus they would be blocked from documenting symptoms. Whether it a contact or a case, all are linked to the same person, thus users can be free to do visits through any entity of the person as long as they have full access to one of the entities.
  3. This is a similar situation if user1 has full access to contact1 of person1 but no access to contact2 or contact 3 of person1. User1 is able to document visits for contact1 and the system shared the visit across contact 2 and 3. Thus user1 does visit for contact2 and 3 without having access to them. If contact 2 is then converted to case2, in Region of contact2, then user1 would still be able to do visits for contact1 which is then transferred to contact 2 and even case2. I have not tested this but it should be the current implementation, right? ie visits are shared across all entities of the same person when their followup intervals overlap and disease is the same.

@MateStrysewske
Copy link
Contributor

@bernardsilenou Why would a user that has access to the contact, but not to the case still be required to add visits to the contact though? The contact, at this point - at least from my understanding - is not relevant anymore because it's already been converted to a case. Any further management should be done on the case by whoever has access to it. If a user has access to the contact, but not to the case, they're apparently not the user who's responsible for managing the case, so why should they still need to create visits for it? And if they are responsible, the case will probably be in their responsible jurisdiction anyway, so they can just go to the case and create the visits there.

@MateStrysewske MateStrysewske removed discuss Discussion about whether or not the issue should or can be implemented required needs-response Response from the issue creator required labels Dec 13, 2021
@MateStrysewske MateStrysewske moved this from Waiting to In Progress in SORMAS Team 2 - DEV - Iteration Backlog Dec 13, 2021
BarnaBartha added a commit that referenced this issue Dec 13, 2021
…on to create a new visit + add info icon explaining that the user should add the visits to the resulting case
SORMAS Team 2 - DEV - Iteration Backlog automation moved this from In Progress to Testing Dec 13, 2021
MateStrysewske added a commit that referenced this issue Dec 13, 2021
…on to create a new visit + add info icon explaining that the user should add the visits to the resulting case (#7506)

Co-authored-by: Maté Strysewske <mate.strysewske@vitagroup.ag>
@mario-kuehne mario-kuehne self-assigned this Dec 14, 2021
@mario-kuehne
Copy link
Contributor

mario-kuehne commented Dec 14, 2021

Currently the 'follow-up visits' entities are shown in both views 'contacts' & 'cases' (for the specific contact entity that has been converted to a case). If a user has access to both views then the user is able to edit the follow-up visits in both views.

Expected / Intended behaviour? (in the contacts-view the user is not able to add a new visit, but can edit existing visits)

@MateStrysewske
Copy link
Contributor

@mario-kuehne Yes, that's expected behaviour, but editing the visit might actually lead to the same problem than creating a new one. Did you test that by chance?

@mario-kuehne
Copy link
Contributor

mario-kuehne commented Dec 15, 2021

Found an UI issue and clarified with @MateStrysewske and @BarnaBartha to reopen the ticket.

Reproduction:

  1. Create contact A
  2. Convert contact A to case B
  3. Login as CommunityOfficer
  4. Open Contacts page
  5. Open contact A
  6. Open tab 'Follow-up visits'

contact-new-visit-and-info-icon-gap-ui-issue

Tested on https://test.sormas.netzlink.com/

@mario-kuehne mario-kuehne reopened this Dec 15, 2021
SORMAS Team 2 - DEV - Iteration Backlog automation moved this from Testing to In Progress Dec 15, 2021
@mario-kuehne
Copy link
Contributor

@mario-kuehne Yes, that's expected behaviour, but editing the visit might actually lead to the same problem than creating a new one. Did you test that by chance?

@MateStrysewske Yes, it is also possible to change and save an existing visit for a contact (although an error message is shown), that has been converted to a case in another jurisdiction for which the current logged in user has no access to.

@MateStrysewske
Copy link
Contributor

@BarnaBartha That means that we probably have to implement what I'd suggested earlier, i.e. a differentiation in the case saving process whether the saving is done manually or automatically by the system (in which case we have to skip the check whether the current user is allowed to save). I don't think that we can or should prevent users from editing visits when they have access to them (which would be the other option).

BarnaBartha added a commit that referenced this issue Dec 21, 2021
@BarnaBartha BarnaBartha moved this from In Progress to Review in SORMAS Team 2 - DEV - Iteration Backlog Dec 22, 2021
@mario-kuehne mario-kuehne removed their assignment Dec 23, 2021
SORMAS Team 2 - DEV - Iteration Backlog automation moved this from Review to Testing Jan 6, 2022
MateStrysewske added a commit that referenced this issue Jan 6, 2022
* #7434 - do not validate case save when saving from visit for the update case symptoms logic

* #7434 - CR suggestion - add second save case method

* #7434 - CR suggestion - fix internal flag default

Co-authored-by: Maté Strysewske <mate.strysewske@vitagroup.ag>
@AndyBakcsy-she AndyBakcsy-she self-assigned this Jan 7, 2022
@AndyBakcsy-she AndyBakcsy-she added the qa-verified Issue has been tested and verified by QA label Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Affects the web backend bug An error or misbehavior of an existing feature (ticket type) contacts major A functional requirement is incorrect or incomplete, ... (Severity for bugs/defects) qa-verified Issue has been tested and verified by QA
Projects
None yet
7 participants