-
Notifications
You must be signed in to change notification settings - Fork 15
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
Host Checks selection saga #1618
Merged
Merged
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
8f397bc
Move cluster check selection action next to the state
nelsonkopliku c8a2e6a
Move cluster check selection saga in dedicated saga file
nelsonkopliku dfef0a6
Use cluster specific action name for checks selection
nelsonkopliku e36884e
Add a new checksSelection slice for host checks selection
nelsonkopliku fee3cff
Add reducer to hosts to update checks selection
nelsonkopliku ad5bc6b
Add host checks selection saga
nelsonkopliku 99298d9
Use redux state in HostSettingsPage
nelsonkopliku 38d2cfa
Improve imports and faked test data
nelsonkopliku d865524
Simplify host check selection state
nelsonkopliku bdfcbca
Add tests for current cluster checks selection saga
nelsonkopliku 1dbc208
Extract callback functions to component functions
nelsonkopliku d89913d
Add targetName prop to ChecksSelection
nelsonkopliku 1faa63c
Rename checksSelection to hostChecksSelection
nelsonkopliku e6481f8
Use useCallback when passing saving function
nelsonkopliku 3cdf479
Improve name for the new check selection emitted by the ChecksSelecti…
nelsonkopliku File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import { createAction, createSlice } from '@reduxjs/toolkit'; | ||
|
||
const initialState = { | ||
saving: false, | ||
}; | ||
|
||
export const checksSelectionSlice = createSlice({ | ||
name: 'checksSelection', | ||
initialState, | ||
reducers: { | ||
startSavingChecksSelection: (state) => { | ||
state.saving = true; | ||
}, | ||
stopSavingChecksSelection: (state) => { | ||
state.saving = false; | ||
}, | ||
}, | ||
}); | ||
|
||
export const HOST_CHECKS_SELECTED = 'HOST_CHECKS_SELECTED'; | ||
export const checksSelected = createAction(HOST_CHECKS_SELECTED); | ||
|
||
export const { startSavingChecksSelection, stopSavingChecksSelection } = | ||
checksSelectionSlice.actions; | ||
|
||
export default checksSelectionSlice.reducer; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import checksSelectionReducer, { | ||
startSavingChecksSelection, | ||
stopSavingChecksSelection, | ||
} from '@state/hostChecksSelection'; | ||
|
||
describe('Checks Selection reducer', () => { | ||
it('should mark a check selection as saving', () => { | ||
const initialState = { | ||
saving: false, | ||
}; | ||
|
||
const action = startSavingChecksSelection(); | ||
|
||
const expectedState = { | ||
saving: true, | ||
}; | ||
|
||
expect(checksSelectionReducer(initialState, action)).toEqual(expectedState); | ||
}); | ||
|
||
it('should mark a check selection as completed', () => { | ||
const initialState = { | ||
saving: true, | ||
}; | ||
|
||
const action = stopSavingChecksSelection(); | ||
|
||
const expectedState = { | ||
saving: false, | ||
}; | ||
|
||
expect(checksSelectionReducer(initialState, action)).toEqual(expectedState); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't be better to pass
selectedChecks
andclusterId
as the function arguments?Using global args looks a bit unsafe.
But talking from my ignorance... XD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need to have different names tho, otherwise it will clash.
And at the end we would be still passing the same variables, with a different name.
I am open to suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really have a suggestion...
Anyway, now that i checked a bit, we should maybe wrap this functions in
useCallback
, otherwise the function is redefined in every render. Maybe a cheap operation, but with theuseCallback
it is avoided.Maybe @dottorblaster has stronger opinions on this