-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Cases] Category fixes #160026
[Cases] Category fixes #160026
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
Pinging @elastic/response-ops-cases (Feature:Cases) |
@@ -258,7 +258,6 @@ export const CasesFindRequestRt = rt.exact( | |||
/** | |||
* The field to use for sorting the found objects. | |||
* |
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.
would be nice to have somewhere documenting which fields are supported though
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.
Yes, we can do it when we apply the limits in 8.10. The types will give us the supported fields.
@@ -72,6 +72,10 @@ describe('validators', () => { | |||
it('returns true if the category is an empty string', () => { | |||
expect(isCategoryFieldInvalidString('')).toBe(true); | |||
}); | |||
|
|||
it('returns true if the string contains only spaces', () => { | |||
expect(isCategoryFieldInvalidString(' ')).toBe(true); |
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.
thanks 😁
expect(screen.getByTestId('edit-category-submit')).toBeDisabled(); | ||
}); | ||
|
||
it('should disabled the save button on when not changing category', async () => { |
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.
nit:
it('should disabled the save button on when not changing category', async () => { | |
it('should disabled the save button when not changing category', async () => { |
@@ -438,6 +438,26 @@ describe('Cases API', () => { | |||
}); | |||
expect(resp).toEqual({ ...allCases }); | |||
}); | |||
|
|||
it('should not send the category field if it an empty array', async () => { |
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.
nit:
it('should not send the category field if it an empty array', async () => { | |
it('should not send the category field if it is an empty array', async () => { |
@@ -456,13 +456,23 @@ export const getCaseToUpdate = ( | |||
{ id: queryCase.id, version: queryCase.version } | |||
); | |||
|
|||
/** |
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.
Is there an issue for this?
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.
Yes, #153726 section "Strict input validation". We will address it there including the unification of the backend & the frontend.
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.
Looks good. Would there be an easy way to allow deselecting a category to "unset" it? That was a little confusing to me because the only way to "unset" it that worked was do hit the backspace/delete
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.
Tested the new "delete category" button, I like it!
Leaving the approval for the rest which I had already reviewed and tested 👍
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.
Verified all bugs locally, works as expected!! 👍
Nice work 👏
it('renders the status column', async () => { | ||
appMockRenderer.render(<AllCasesList />); | ||
|
||
expect(screen.getByTestId('tableHeaderCell_status_8')).toBeInTheDocument(); |
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.
Thanks for doing this 😄
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.
+1 :D
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @cnasikas |
Summary
This PR:
Checklist
Delete any items that are not applicable to this PR.
For maintainers