-
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] Custom fields MVP #167016
[Cases] Custom fields MVP #167016
Conversation
Connected with #160236 **Merging into a Feature Branch** ## Summary This PR defines the `customField` in the Cases API and Domain types. For now, we only allow the custom fields `string`, `boolean`, and `list`. This PR does not include the case configuration types. The default value for the field is `[]`.
## Summary This was missing in my previous PR #165671
Following up on @js-jankisalvi work I changed the `customFields` property in `CasesConfigure` to be the most basic possible. Changes: - We only accept `TextCustomField` and `ToggleCustomField` in the cases configuration api. - The custom field type used in the Cases domain and API types now comes from the enum in `configure/v1`. - Cases configuration POST requests now require the `customFields` property
…toggle field) (#166483) ## Summary Connected to #160236 ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Christos Nasikas <[email protected]>
…ic/kibana into cases-custom-fields-feature-branch
Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Janki Salvi <[email protected]>
## Summary This PR implements the following validation: - **Custom field keys should be unique** - Where: - `CaseConfigureService` - x-pack/plugins/cases/server/services/configure/index.ts - `post` - `patch` - Tests: - x-pack/plugins/cases/server/services/configure/index.test.ts - **Limit custom fields to 5** - Where: - `ConfigurationRequestRt` (POST) - `ConfigurationPatchRequestRt` - `CasePostRequestRt` - done in a previous PR - `CasePatchRequestRt` - done in a previous PR - Tests: - x-pack/plugins/cases/server/client/configure/client.test.ts - x-pack/plugins/cases/common/types/api/configure/v1.test.ts - x-pack/plugins/cases/server/client/cases/create.test.ts - x-pack/plugins/cases/server/client/cases/update.test.ts
## Summary This PR limits the length of the property value for `text` custom fields.
## Summary This PR implements the following validation: - **Custom Field Keys need to exist in configuration when creating or updating cases** - Where: - Cases Client - `create` - `update` - Tests: - x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/patch_cases.ts - x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/post_case.ts --------- Co-authored-by: kibanamachine <[email protected]>
## Summary This PR adds the ability to add custom fields in the create case form. **Testing** Verify that you all custom fields from configuration are visible in create case form Verify that you can set your custom fields Verify that it throws an error based on the configuration (required: true | false) Connected to #160236 **Create case** https://github.com/elastic/kibana/assets/117571355/8b560804-ea1f-4313-a382-34a17c0750b7 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Christos Nasikas <[email protected]>
## Summary Connected to #160236 This PR adds validation in UI to limit maximum number of custom fields to 5 in configuration cases page ![image](https://github.com/elastic/kibana/assets/117571355/7cbb2dfb-bf13-498c-a4a1-1fddeefa4d46) ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <[email protected]>
## Summary This PR checks for missing required custom fields in the create case API. (In my next PR I am thinking of moving `casesClient.configure.get({ owner: casePostRequest.owner });` outside of the validation functions.)
).toThrowErrorMatchingInlineSnapshot(`"Missing required custom fields: first_key"`); | ||
}); | ||
|
||
it('throws if request sends a custom field that is not part of the configuration', () => { |
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 am wondering if this check exceeds the responsibility of the validation function which is to validate if required fields are present. The validateCustomFieldKeysAgainstConfiguration
validates for fields that are not part of the configuration. Though they are not connected in this context and are tested in isolation (test in the create/update routes should ensure that both the validation functions are being called). Wdyt?
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.
you are right, I'll remove it from the validateRequiredCustomFields fn
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
buildkite test this |
@elasticmachine merge upstream |
Blocked by #167745 |
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
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.
@js-jankisalvi @adcoelho @jcger Great work 🚀
Summary
This PR implements an MPV for custom fields in Cases. Specifically:
Text
custom field type is supported. TheToggle
is implemented to test the framework.null
valuesText
custom field typeTesting
main
before switching to the feature branch)Resolves: #160236
PRs:
Checklist
Delete any items that are not applicable to this PR.
For maintainers
Release notes
Allow users to configure custom fields in Cases. Supported field types: Text and Toggle. Coming soon: List, Number, Date, IP, Email, etc.