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.

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] Update the custom fields api documentation. #169242

Closed
wants to merge 4 commits into from

Conversation

adcoelho
Copy link
Contributor

Summary

This PR updates the documentation with the custom fields logic that was implemented recently in the custom fields feature branch.

@adcoelho adcoelho added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature labels Oct 18, 2023
@adcoelho adcoelho self-assigned this Oct 18, 2023
@adcoelho adcoelho added release_note:skip Skip the PR/issue when compiling release notes v8.11.0 v8.12.0 labels Oct 18, 2023
@adcoelho adcoelho marked this pull request as ready for review October 18, 2023 13:40
@adcoelho adcoelho requested a review from a team as a code owner October 18, 2023 13:40
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops-cases (Feature:Cases)

@adcoelho adcoelho requested a review from lcawl October 18, 2023 13:40
@adcoelho adcoelho added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Oct 18, 2023
Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

Thanks Antonio for updating the docs. I have some observations/questions (sorry for my lack of knowledge):

  • Should we update the types for the create/update Case APIs?
  • I think we should tell users how we treat optional fields when are missing from the create/update Case APIs?
  • How I can visualize the changes in the docs?

nullable: true
description: >
The value of the custom field. Cannot be explicitly set to null if the field is configured as required.
However, existing cases when the custom field was created will have this value null as default.
Copy link
Member

Choose a reason for hiding this comment

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

This is not entirely true because we do not persist the null value in ES. Though our APIs will return null. I am not sure if we have to be explicit about it. wdyt? cc @lcawl

Copy link
Contributor Author

@adcoelho adcoelho Oct 19, 2023

Choose a reason for hiding this comment

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

How about:

The value of the custom field. Cannot be explicitly set to null if the field is configured as required. However, cases that had been created before the custom field was added will have this value undefined in elastic search and return null as default when trying to access them via API or UI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's what I'm currently proposing in #169327:

The custom field value.
If the custom field is required, it cannot be explicitly set to null.
However, for cases that existed when the required custom field was added, the default value stored in Elasticsearch is undefined.
The value returned in the API and user interface in this case is null.

@adcoelho
Copy link
Contributor Author

Should we update the types for the create/update Case APIs?

That comes from x-pack/plugins/cases/docs/openapi/components/schemas/custom_fields_property.yaml already updated in this PR.

I think we should tell users how we treat optional fields when are missing from the create/update Case APIs?

This is an explanation more about what the user doesn't send so where would that make sense? Maybe here?

How I can visualize the changes in the docs?

@lcawl ? 😅

@adcoelho
Copy link
Contributor Author

I think we should tell users how we treat optional fields when are missing from the create/update Case APIs?

I updated the description here.

@lcawl
Copy link
Contributor

lcawl commented Oct 19, 2023

Looks like we were both working on this yesterday (my PR: #169327). I will compare to see if we can merge our efforts.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Oct 19, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Investigations - Security Solution Cypress Tests #5 / Fields Browser Fields Browser rendering "before each" hook for "displays a search results label with the expected count of fields matching the filter input" "before each" hook for "displays a search results label with the expected count of fields matching the filter input"
  • [job] [logs] Investigations - Security Solution Cypress Tests #7 / Save Timeline Prompts "before each" hook for "Changed & unsaved timeline should prompt when user navigates away within security solution where timelines are disabled eg. admin screen" "before each" hook for "Changed & unsaved timeline should prompt when user navigates away within security solution where timelines are disabled eg. admin screen"

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @adcoelho

@lcawl
Copy link
Contributor

lcawl commented Oct 19, 2023

  • How I can visualize the changes in the docs?

I removed the previews in #168761, so these specs must be viewed in external viewers for now. The first priority in our new docs site is displaying the serverless APIs, so these case APIs are not visible there yet.

@adcoelho adcoelho closed this Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Cases Cases feature release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.11.0 v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants