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

feat(ocpadvisor-123): functional filters with mockdata #629

Merged
merged 6 commits into from
Nov 7, 2023

Conversation

Fewwy
Copy link
Collaborator

@Fewwy Fewwy commented Nov 2, 2023

Working filters for the workloads table

No URL PARAMS YET! Gonna work on them in a separate PR

How to setup mock api:

Clone insights-results-aggregator-mock
make build (takes a few minutes for me, coffee time ☕)
2.1. can speed it up with make build -j 4 to run multiple jobs at once
make run
Test the mocked api curl localhost:8080/api/insights-results-aggregator/v2/namespaces/dvo
Run the app MOCK=true npm run start:proxy:beta
Workloads should load mocked data

How to test with local data:
You can use the mock data provided from the json file - for that you need comment out data from props and use json

@Fewwy Fewwy requested a review from a team as a code owner November 2, 2023 16:15
@Fewwy Fewwy force-pushed the workloads-filters-with-mock branch from 1eff54e to 0bd5112 Compare November 2, 2023 16:21
@Fewwy Fewwy self-assigned this Nov 2, 2023
@Fewwy Fewwy added the enhancement New feature or request label Nov 2, 2023
@codecov-commenter
Copy link

codecov-commenter commented Nov 2, 2023

Codecov Report

Attention: 40 lines in your changes are missing coverage. Please review.

Files Coverage Δ
src/Services/Filters.js 90.90% <ø> (ø)
src/Components/Common/Tables.js 77.07% <7.14%> (-6.85%) ⬇️
src/Utilities/Workloads.js 15.62% <15.62%> (ø)

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@AsToNlele AsToNlele left a comment

Choose a reason for hiding this comment

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

Amazing job @Fewwy! 😄 Filters work as expected, tested with multiple filters at once.
Just a question, what about the filter params in the url?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea with the json!

Comment on lines 293 to 298
const mapping = {
1: 'low',
2: 'moderate',
3: 'important',
4: 'critical',
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Really like this mapping, will propably refactor the severityTypeTotext to use this mapping and just uppercase the first letter.

@Fewwy Fewwy force-pushed the workloads-filters-with-mock branch 2 times, most recently from b53ded6 to 236eb4c Compare November 6, 2023 12:54
@Fewwy Fewwy force-pushed the workloads-filters-with-mock branch from 9d33bb9 to 395a0e1 Compare November 7, 2023 10:16
Copy link
Contributor

@AsToNlele AsToNlele left a comment

Choose a reason for hiding this comment

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

Well done, filters work as expected! 😄

delete cleanedUpParams.sortDirection;
delete cleanedUpParams.offset;
delete cleanedUpParams.limit;
return Object.values(cleanedUpParams).filter((value) => !isEmpty(value));
Copy link
Contributor

Choose a reason for hiding this comment

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

You could return this instead

Suggested change
return Object.values(cleanedUpParams).filter((value) => !isEmpty(value));
return (
Object.values(cleanedUpParams).filter((value) => !isEmpty(value)).length > 0
);

and then remove the filtersApplied state, since it's not used in any effect.

Used like this: showDeleteButton: noFiltersApplied(filters)

@Fewwy
Copy link
Collaborator Author

Fewwy commented Nov 7, 2023

/retest

@Fewwy Fewwy merged commit 6b61535 into RedHatInsights:master Nov 7, 2023
2 checks passed
@gkarat
Copy link
Collaborator

gkarat commented Nov 9, 2023

🎉 This PR is included in version 1.25.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@gkarat gkarat added the released label Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants