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

Show error in targets when user is not associated with the configured place #7346

Closed
ngaruko opened this issue Sep 28, 2021 · 7 comments
Closed
Assignees
Labels
Type: Bug Fix something that isn't working as intended
Milestone

Comments

@ngaruko
Copy link
Contributor

ngaruko commented Sep 28, 2021

Important: This is a public repository. Anyone in the world can see what's posted here. If you are posting screenshots or log files, please carefully examine them for the presence of any kind of protected health information (PHI). Images or logs containing PHI must be posted in fully-redacted form, with no visible PHI.

Describe the bug
The target page shows blank when the user is not associated with health_center.
To Reproduce
Steps to reproduce the behavior:

  1. Start the web app
  2. Load brac config
  3. Create a user associated with a non health-center place, like a branch
  4. Login and go to analytics
  5. The page is blank

Expected behavior
There should be a clear message displayed on the page.

Logs
None

Screenshots
image

Environment

  • Instance: local
  • Browser: (eg: Firefox, Chrome, incognito mode, etc, which worked, which didn't)
  • Client platform: (eg: Windows, MacOS, Linux)
  • App: webapp
  • Version: 3.13

Additional context
In one of the latest PRs, there has been a context: 'user.parent.type === "health_center"' line added to targets. So, the issue shows after that commit.

@ngaruko ngaruko added the Type: Bug Fix something that isn't working as intended label Sep 28, 2021
@dianabarsan
Copy link
Member

I think context: 'user.parent.type === "health_center"' just means that users have no targets. It's not an error, per say.
I agree that we should have a check in the template to display a text like "No targets" instead of a blank page (https://github.com/medic/cht-core/blob/master/webapp/src/ts/modules/analytics/analytics-targets.component.html#L10)

@ngaruko ngaruko added this to Ready for dev in Care Teams Workstream via automation Sep 28, 2021
@ngaruko ngaruko added this to the 3.14.0 milestone Sep 28, 2021
@ngaruko
Copy link
Contributor Author

ngaruko commented Sep 28, 2021

@dianabarsan Agree. All we need is to display th message instead of blank page. Though for this case, @jonathanbataire suggested that "Branch users(district_hospital level) don't have any targets configured only target aggregates", so we might need to have the 2 links displayed as it is for admin users (or at least the target aggregates).
image

@dianabarsan
Copy link
Member

dianabarsan commented Sep 28, 2021

The target-aggregates page is controlled by a permission can_aggregate_targets . Does this role have this permission?

@ngaruko
Copy link
Contributor Author

ngaruko commented Sep 28, 2021

Ah! You are right. When the aggregate permission is enabled for restricted user, we have the right UI, so we just have to fix/analytics/targets display. /analytics/target-aggregates seems to work ok when the permission if off Target aggregates are disabled

@latin-panda latin-panda self-assigned this Oct 7, 2021
@latin-panda latin-panda moved this from Ready for dev to Dev in progress in Care Teams Workstream Oct 7, 2021
@latin-panda latin-panda moved this from Dev in progress to Ready for AT in Care Teams Workstream Oct 15, 2021
@latin-panda
Copy link
Contributor

This is ready for AT in this PR

@ngaruko ngaruko moved this from Ready for AT to AT in progress in Care Teams Workstream Oct 15, 2021
@ngaruko ngaruko moved this from AT in progress to Ready for AT in Care Teams Workstream Oct 15, 2021
@ngaruko
Copy link
Contributor Author

ngaruko commented Nov 1, 2021

Looks good.
'no target found message' displayed on the branch
image.
Thanks @latin-panda for adding e2e tests for this. Please feel free to merge.

@ngaruko ngaruko moved this from Ready for AT to AT in progress in Care Teams Workstream Nov 1, 2021
@ngaruko ngaruko moved this from AT in progress to Ready to merge in Care Teams Workstream Nov 1, 2021
latin-panda added a commit that referenced this issue Nov 2, 2021
Ticket: #7346

This commit adds a message for when an user doesn't have targets in the view.
@latin-panda
Copy link
Contributor

Merged to main branch.

Care Teams Workstream automation moved this from Ready to merge to Done Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Fix something that isn't working as intended
Projects
No open projects
Development

No branches or pull requests

3 participants