-
-
Notifications
You must be signed in to change notification settings - Fork 736
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(1-3173): clear "removed tags" when you bulk update tags (#8952)
This PR fixes a bug wherein the list of tags to remove from a group of tags wouldn't be correctly updated. ## Repro steps - Add a console log line to `frontend/src/component/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageBulkTagsDialog.tsx`'s `ManagebulkTagsDialog`. Log the value of the`payload` variable. - Pick a flag with no tags. - Add tag A -> before submitting, you should have one added tag and zero removed flags. After submitting, both should be empty. - Now remove tag A -> before submitting, you should have one removed tag and zero added tag. After submitting, both should be empty - Notice that removed flags hasn't been emptied, but still contains tag A. - Now add tab B -> before submitting, you should have tag B in added and nothing in removed. Notice that tag A is still in removed. ## Discussion points This gives us both a `clear` and a `reset` event, which is unfortunate because they sound like they do the same thing. I'd suggest renaming the `clear` event (because it doesn't really clear the state completely), but I'm not sure to what. Happy to do that if you have a suggestion. I have not tested that submission of the form actually resets the state. I spent about 45 minutes looking at it, but couldn't find a way that was sensible and worked (considered spying: couldn't make it work; considered refactoring and extracting components: think that's too much of a change). I think this is benign enough that it can go without a test for that thing actually being called. I did, however, test the different reducer commands.
- Loading branch information
1 parent
37a3ec9
commit 7a43634
Showing
4 changed files
with
120 additions
and
9 deletions.
There are no files selected for viewing
89 changes: 89 additions & 0 deletions
89
...ponent/feature/FeatureView/FeatureOverview/ManageTagsDialog/ManageBulkTagsDialog.test.tsx
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,89 @@ | ||
import { payloadReducer } from './ManageBulkTagsDialog'; | ||
|
||
describe('payloadReducer', () => { | ||
it('should add a tag to addedTags and remove it from removedTags', () => { | ||
const initialState = { | ||
addedTags: [{ type: 'simple', value: 'A' }], | ||
removedTags: [ | ||
{ type: 'simple', value: 'B' }, | ||
{ type: 'simple', value: 'C' }, | ||
], | ||
}; | ||
|
||
const action = { | ||
type: 'add' as const, | ||
payload: { type: 'simple', value: 'B' }, | ||
}; | ||
|
||
const newState = payloadReducer(initialState, action); | ||
|
||
expect(newState).toMatchObject({ | ||
addedTags: [ | ||
{ type: 'simple', value: 'A' }, | ||
{ type: 'simple', value: 'B' }, | ||
], | ||
removedTags: [{ type: 'simple', value: 'C' }], | ||
}); | ||
}); | ||
|
||
it('should remove a tag from addedTags and add it to removedTags', () => { | ||
const initialState = { | ||
addedTags: [ | ||
{ type: 'simple', value: 'A' }, | ||
{ type: 'simple', value: 'B' }, | ||
], | ||
removedTags: [{ type: 'simple', value: 'C' }], | ||
}; | ||
|
||
const action = { | ||
type: 'remove' as const, | ||
payload: { type: 'simple', value: 'B' }, | ||
}; | ||
|
||
const newState = payloadReducer(initialState, action); | ||
|
||
expect(newState).toMatchObject({ | ||
addedTags: [{ type: 'simple', value: 'A' }], | ||
removedTags: [ | ||
{ type: 'simple', value: 'C' }, | ||
{ type: 'simple', value: 'B' }, | ||
], | ||
}); | ||
}); | ||
|
||
it('should empty addedTags and set removedTags to the payload on clear', () => { | ||
const initialState = { | ||
addedTags: [{ type: 'simple', value: 'A' }], | ||
removedTags: [{ type: 'simple', value: 'B' }], | ||
}; | ||
|
||
const action = { | ||
type: 'clear' as const, | ||
payload: [{ type: 'simple', value: 'C' }], | ||
}; | ||
|
||
const newState = payloadReducer(initialState, action); | ||
|
||
expect(newState).toMatchObject({ | ||
addedTags: [], | ||
removedTags: [{ type: 'simple', value: 'C' }], | ||
}); | ||
}); | ||
|
||
it('should empty both addedTags and removedTags on reset', () => { | ||
const initialState = { | ||
addedTags: [{ type: 'simple', value: 'test' }], | ||
removedTags: [{ type: 'simple', value: 'test2' }], | ||
}; | ||
|
||
const action = { | ||
type: 'reset' as const, | ||
}; | ||
|
||
const newState = payloadReducer(initialState, action); | ||
expect(newState).toMatchObject({ | ||
addedTags: [], | ||
removedTags: [], | ||
}); | ||
}); | ||
}); |
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