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.

Sign up for GitHub

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

Jump to bottom

Fix filters comparison #363

Merged
merged 2 commits into from
Jul 27, 2023
Merged

Fix filters comparison #363

merged 2 commits into from
Jul 27, 2023

Conversation

jotak
Copy link
Member

@jotak jotak commented Jul 27, 2023

Filters comparison was broken in different ways:

  • Same filters with different ordering would be seen as different
  • Same filters with different values ordering would be seen as different
  • Same filters with different display info would be seen as different (when the page is refreshed, the display info on values isn't set)

This commit fixes all of that, and adds tests for each of these scenarios

@jotak
Fix filters comparison
1c75a2e 
Filters comparison was broken in different ways:
- Same filters with different ordering would be seen as different
- Same filters with different *values* ordering would be seen as
  different
- Same filters with different display info would be seen as different
  (when the page is refreshed, the display info on values isn't set)

This commit fixes all of that, and adds tests for each of these
scenarios
@jotak jotak requested review from memodi and jpinsonneau July 27, 2023 07:30
@codecov
Copy link

codecov bot commented Jul 27, 2023 •
edited

Codecov Report

Merging #363 (22050ee) into main (dccb069) will decrease coverage by 0.50%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##             main     #363      +/-   ##
==========================================
- Coverage   57.44%   56.94%   -0.50%     
==========================================
  Files         161      162       +1     
  Lines        7266     7347      +81     
  Branches      902      902              
==========================================
+ Hits         4174     4184      +10     
- Misses       2846     2917      +71     
  Partials      246      246
Flag Coverage Δ
uitests 58.37% <94.44%> (+0.07%) ⬆️
unittests 52.71% <ø> (-2.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
web/src/components/netflow-traffic.tsx 52.14% <ø> (ø)
web/src/model/filters.ts 74.19% <90.90%> (+3.82%) ⬆️
web/src/components/filters/filters-chips.tsx 51.78% <100.00%> (ø)
web/src/components/overflow/links-overflow.tsx 88.88% <100.00%> (ø)
web/src/model/flow-query.ts 97.56% <100.00%> (ø)
web/src/utils/base-compare.ts 57.14% <100.00%> (+7.14%) ⬆️

... and 1 file with indirect coverage changes

jpinsonneau
jpinsonneau previously approved these changes Jul 27, 2023
View reviewed changes
Copy link
Contributor

@jpinsonneau jpinsonneau left a comment

Choose a reason for hiding this comment

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

LGTM thanks @jotak

@openshift-ci openshift-ci bot added the lgtm label Jul 27, 2023
@memodi
Copy link
Contributor

memodi commented Jul 27, 2023 •
edited by openshift-ci bot

NETOBSERV-1218
/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 27, 2023
@github-actions
Copy link

New image:
quay.io/netobserv/network-observability-console-plugin:ec8fb65

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=ec8fb65 make set-plugin-image

@memodi
Copy link
Contributor

memodi commented Jul 27, 2023

/label jira/valid-reference NETOBSERV-1218

@memodi
Copy link
Contributor

memodi commented Jul 27, 2023

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label Jul 27, 2023
@openshift-ci openshift-ci bot removed the lgtm label Jul 27, 2023
@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Jul 27, 2023
jpinsonneau
jpinsonneau approved these changes Jul 27, 2023
View reviewed changes
Copy link
Contributor

@jpinsonneau jpinsonneau left a comment

Choose a reason for hiding this comment

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

👍

@openshift-ci openshift-ci bot added the lgtm label Jul 27, 2023
@jotak
Copy link
Member Author

jotak commented Jul 27, 2023

/approve

@openshift-ci
Copy link

openshift-ci bot commented Jul 27, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jotak

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jotak jotak merged commit 1c39272 into netobserv:main Jul 27, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpinsonneau jpinsonneau jpinsonneau approved these changes

@memodi memodi Awaiting requested review from memodi

Assignees

@jpinsonneau jpinsonneau

Labels
approved lgtm qe-approved QE has approved this pull request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jotak @memodi @jpinsonneau