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

Avoid showing administration-related banner notifications in view-only dashboard #5295

Closed
felixarntz opened this issue May 27, 2022 · 7 comments
Labels
P0 High priority Type: Bug Something isn't working

Comments

@felixarntz
Copy link
Member

felixarntz commented May 27, 2022

Bug Description

The view-only dashboard is currently showing banner notifications related to administration, which should not be shown in such a case of course, since the user can't act on them. See the below screenshot which shows the Idea Hub CTA banner notification to a contributor user.

Screen Shot 2022-05-27 at 4 26 19 PM


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • From all banner notifications part of BannerNotifications or EntityBannerNotifications, the only notification(s) that should ever be shown in a view-only context should be ZeroDataStateNotifications.

Implementation Brief

  • In the BannerNotifications component:
    • Rely on useViewOnly (e.g. const viewOnly = useViewOnly()).
    • Wrap all notifications within the rendered Fragment in a check whether ! viewOnly, except for ZeroDataStateNotifications.
  • EntityBannerNotifications can remain unchanged since it only contains ZeroDataStateNotifications anyway.

Test Coverage

  • No new tests needed.

QA Brief

  • Ensure that only zero-data state notifications display when viewing the view-only Dashboard.
  • Ensure that all types of notification continue to display on the authenticated Dashboard.

Changelog entry

  • Prevent admin-related notifications from appearing on view-only dashboard.
@felixarntz felixarntz added Type: Bug Something isn't working P0 High priority labels May 27, 2022
@felixarntz felixarntz self-assigned this May 27, 2022
@felixarntz
Copy link
Member Author

cc @aaemnnosttv @wpdarren for awareness

@felixarntz felixarntz assigned aaemnnosttv and unassigned felixarntz May 27, 2022
@aaemnnosttv
Copy link
Collaborator

IB ✅

@aaemnnosttv
Copy link
Collaborator

@techanvil would you please add a QA brief?

@techanvil
Copy link
Collaborator

Thanks for pointing this out @aaemnnosttv - I had added it in the Test Coverage section 🤦. Have moved to QAB, cheers.

@techanvil techanvil removed their assignment May 31, 2022
@wpdarren wpdarren self-assigned this May 31, 2022
@wpdarren
Copy link
Collaborator

QA Update: ⚠️

@techanvil the QAB suggests that only zero-data state notifications display when using view only dashboard. I do see the gathering state notifications though, which in my opinion, would make sense to keep being displayed. Just checking though. I tested this with an editor and admin role.

image

@techanvil
Copy link
Collaborator

Hi @wpdarren, thanks for pointing this out - in hindsight the issue could have been a bit clearer. Gathering data notifications are also relevant to non-admin users and are included in the ZeroDataStateNotifications component mentioned in the AC. So, they should still continue to be seen.

@techanvil techanvil removed their assignment May 31, 2022
@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • Only zero-data state notifications display when viewing the view-only Dashboard.
  • All types of notification continue to display on the authenticated Dashboard.
Screenshots

image
image
image
image

@wpdarren wpdarren removed their assignment May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants