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

Filter in notifications drawer #2875

Merged
merged 11 commits into from
Jul 4, 2024

Conversation

CodyWMitchell
Copy link
Contributor

RHCLOUD-29014

  • Enable filtering in the notifications drawer
  • Fetch correct filters from /api/notifications/v1/notifications/facets/bundles

@codecov-commenter
Copy link

codecov-commenter commented Jun 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.69%. Comparing base (868fdce) to head (ba452ba).

Current head ba452ba differs from pull request most recent head d22b57b

Please upload reports for the commit d22b57b to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2875      +/-   ##
==========================================
+ Coverage   63.67%   63.69%   +0.01%     
==========================================
  Files         202      202              
  Lines        4650     4658       +8     
  Branches      858      859       +1     
==========================================
+ Hits         2961     2967       +6     
- Misses       1679     1681       +2     
  Partials       10       10              
Files Coverage Δ
...ponents/NotificationsDrawer/DrawerPanelContent.tsx 84.44% <100.00%> (+1.72%) ⬆️
...nts/NotificationsDrawer/notificationDrawerUtils.ts 0.00% <ø> (-100.00%) ⬇️
src/state/atoms/notificationDrawerAtom.ts 77.77% <ø> (ø)

@CodyWMitchell CodyWMitchell marked this pull request as ready for review July 2, 2024 23:02
id: string;
name: string;
displayName: string;
children: null | Bundle[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think children can be optional children?: Bundle[]. Or is there is a specific need for the null value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I think you're right.

@@ -124,7 +136,23 @@ const DrawerPanelBase = ({ innerRef }: DrawerPanelProps) => {
);
}
};
const fetchFilterConfig = async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this function defined in useEffect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was following how the permissions were fetched after mounted became true in the fetchPermissions method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. The pattern used before is wrong. The mounted variable should be in ref so we don't have to do this. Its something for a future refactoring I guess.

@Hyperkid123 Hyperkid123 merged commit 8f4c9f5 into RedHatInsights:master Jul 4, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants