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(core/clusterFilter): Use filterModel object to retrieve filter values #8693

Merged
merged 2 commits into from Oct 28, 2020
Merged

fix(core/clusterFilter): Use filterModel object to retrieve filter values #8693

merged 2 commits into from Oct 28, 2020

Conversation

vigneshm
Copy link
Contributor

@vigneshm vigneshm commented Oct 28, 2020 •
edited

In the reactified ClusterFilters component, we end up creating a new sortFilter object for changes to the filter selection (such as status, search, instance). This makes the sortFilter object stale elsewhere.

Fixing the above by referencing ClusterState.filterModel directly.

christopherthielen
christopherthielen approved these changes Oct 28, 2020
View reviewed changes
Copy link
Contributor

@christopherthielen christopherthielen left a comment

Choose a reason for hiding this comment

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

lgtm.

Did you look to see if there are other code that follows the same pattern?

@vigneshm
Copy link
Contributor Author

Did you look to see if there are other code that follows the same pattern?

I performed the following to look for similar patterns. Is there any other form that I'm missing here (not super familiar with *FilterModel structure)?

image

@vigneshm vigneshm added the ready to merge Reviewed and ready for merge label Oct 28, 2020
@caseyhebebrand
Copy link
Contributor

I performed the following to look for similar patterns. Is there any other form that I'm missing here (not super familiar with *FilterModel structure)?

The LoadBalancerFilters and SecurityGroupFilters probably follow this same pattern.

@mergify mergify bot merged commit 62b81bf into spinnaker:master Oct 28, 2020
caseyhebebrand added a commit that referenced this pull request Nov 3, 2020
@caseyhebebrand
chore(core): publish core@0.0.525
f732efe 
ade00f7 fix(core): Remove less override after react refactor (#8695)
deab22f refactor(aws/titus): Reactify instance insights (#8698)
62b81bf fix(core/clusterFilter): Use filterModel object to retrieve filter values (#8693)
4a4f472 fix(core/cronTrigger): Fix regex for weekly cron triggers (#8690)
@caseyhebebrand caseyhebebrand mentioned this pull request Nov 3, 2020
Merged
mergify bot pushed a commit that referenced this pull request Nov 3, 2020
@caseyhebebrand @mergify
chore(core): publish core@0.0.525
5fcb3bf 
ade00f7 fix(core): Remove less override after react refactor (#8695)
deab22f refactor(aws/titus): Reactify instance insights (#8698)
62b81bf fix(core/clusterFilter): Use filterModel object to retrieve filter values (#8693)
4a4f472 fix(core/cronTrigger): Fix regex for weekly cron triggers (#8690)
@vigneshm vigneshm deleted the sync-filter-model branch May 20, 2021 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christopherthielen christopherthielen christopherthielen approved these changes

@caseyhebebrand caseyhebebrand caseyhebebrand approved these changes

Assignees
No one assigned
Labels
ready to merge Reviewed and ready for merge target-release/1.24
Projects
None yet
Milestone
No milestone
4 participants
@vigneshm @caseyhebebrand @christopherthielen @spinnakerbot