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

🎁: PBI wrapper - Custom Access denied message #551

Closed
1 task
atlewee opened this issue Dec 5, 2023 · 17 comments · Fixed by #577 or #585
Closed
1 task

🎁: PBI wrapper - Custom Access denied message #551

atlewee opened this issue Dec 5, 2023 · 17 comments · Fixed by #577 or #585
Assignees
Labels
Core Fusion Core feature

Comments

@atlewee
Copy link

atlewee commented Dec 5, 2023

Expected outcome

The user is presented with how to get access

Business Value / Developer Experience

Today, when you get an access denied error the user does not know how to obtain access.
Telling the user where to apply for access will make the process more efficient and a lot less frustrating

Acceptance Criteria

  • Report-apps can provide a custom "access-denied" error message that the wrapper displays if has problems getting the report token.
@ken-mellem
Copy link
Contributor

Apparently, I have misunderstood something.

Reproduced

When the user does not have access the following is displayed
image

Wanted (as in Fusion)

image

Note

It appears we are not giving correct message to the user when they do not have access for reports when loading workspace reports or report apps. This should come from the report admin in Fusion.

@ken-mellem ken-mellem removed the Core Fusion Core label Jan 25, 2024
@equinor equinor deleted a comment from JoneUE Jan 25, 2024
@ken-mellem
Copy link
Contributor

@Gustav-Eikaas informs me that there is a bug in Fusion Core. They are on it.

@ken-mellem ken-mellem added the Core Fusion Core label Jan 25, 2024
@Gustav-Eikaas
Copy link
Contributor

ADS-engineering-meeting
Troll west electrification
RLS seems to be working now but some bug in frontend makes it show "failed to load report" instead of rls config

@Gustav-Eikaas
Copy link
Contributor

After fix it looks like this. We can build what Fusion has with pictures and everything but we do not have any implementation right now. Should we prioritize making an informative error message?
image

@ken-mellem
Copy link
Contributor

ken-mellem commented Jan 26, 2024

At least it says "No access"... that must be MVP... let's accept that for the time being.
@atlewee @JoneUE @Htonning

@ken-mellem ken-mellem added the MVP label Jan 26, 2024
@Gustav-Eikaas Gustav-Eikaas linked a pull request Jan 31, 2024 that will close this issue
@Htonning
Copy link

Does the closing of this mean that users now see a a text that we can customize per report? If not I would like to keep this issue open and in the backlog, because we really want this in the future. With more reports in the project portal that has different access control, we need to be able to guide users that dont have access on how they should proceed to get access.

@ken-mellem
Copy link
Contributor

@Htonning sure - I'll see what we can accomplish.

@Htonning
Copy link

Htonning commented Feb 1, 2024

We also have some examples of users that are confused regarding the access management, attaching a screenshot of an email to show why it is important that users get good guidance by the project portal instead of having to send emails to Per Kristian to figure out how it works:

image

@Gustav-Eikaas Gustav-Eikaas reopened this Feb 5, 2024
@ken-mellem
Copy link
Contributor

@kjellhaaland I think this one you could take on Monday.

@kjellhaaland
Copy link
Contributor

I have implemented a error message component for displaying this information and made a PR for fusion-workspace.
When the PR is approved, i will make a PR for cc-reports, to include it in reports.

@ken-mellem / @Htonning, can you verify that this looks like you want?

@lindayd, some of the reports i have used for testing is missing a description and accessdescription. This dialog will be most useful if these was present. Is there any future plan on adding such descriptions to the reports?

The dialog shows:

  1. The Access Description for the report
  2. The general Description for the report
  3. The Owner of the report
  4. The raw Row level security requirements

Screenshot:
Image

@kjellhaaland kjellhaaland linked a pull request Feb 13, 2024 that will close this issue
@lindayd
Copy link

lindayd commented Feb 13, 2024

@kjellhaaland I will add some more useful information in the description fields for the reports tomorrow.

@lindayd
Copy link

lindayd commented Feb 14, 2024

@kjellhaaland I have added some information in the description fields in CI for all ADS reports for testing.

@kjellhaaland
Copy link
Contributor

This is now ready for test.
Currently only implemented in cc-reports, but i am working on implementing it in cc-components as well.

Not sure who is the right person to test and verify.

@ken-mellem @atlewee @lindayd @Htonning ? 😃

Link to test

@ken-mellem
Copy link
Contributor

I have access - but got help.

Looks good! Add scrollbar - when expanding the extra information section, you cannot read all without zooming out.

@lindayd
Copy link

lindayd commented Feb 16, 2024

@kjellhaaland @ken-mellem Looks good! No other comments except the one Ken already mentioned regarding the Access control description section.

@kjellhaaland
Copy link
Contributor

Added scrollbar in latest PR.
Waiting for code-review before merging to prod in both cc-applications and cc-reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Fusion Core feature
Projects
None yet
7 participants