equinor / fusion-workspace

MIT License
9 stars 2 forks source link

🎁: PBI wrapper - Custom Access denied message #551

Closed atlewee closed 6 months ago

atlewee commented 9 months ago

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

ken-mellem commented 7 months ago

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 commented 7 months ago

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

Gustav-Eikaas commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

Htonning commented 7 months ago

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 commented 7 months ago

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

Htonning commented 7 months ago

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

ken-mellem commented 7 months ago

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

kjellhaaland commented 7 months ago

Some links that may be useful:

kjellhaaland commented 7 months ago

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

lindayd commented 7 months ago

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

lindayd commented 7 months ago

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

kjellhaaland commented 7 months ago

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 commented 7 months ago

I have access - but got help.

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

lindayd commented 7 months ago

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

kjellhaaland commented 6 months ago

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