Create a common Int Validator and use it in ingest_pipelines and Index_lifecycle_management

[SoniaSanzV]

Closes #110417

Summary

In the Ingest Node Pipelines section, when the users created a new pipeline selecting de Community ID processor the users could set a non-integer number in this field. Then, they received a server side error when tried to create a pipeline. For fixing this, a validation must be added in the client.

We didn't have a reusable validation for this case, but we did have a custom validation for integer values in the Index lifecycle management plugin. We also had the necessary translation in that plugin. So I went forward with:

• I created a new integer validator in the es_ui_shared package as it is a fairly common validation and we could take advantage of it in the future. Also added a couple of unit test there for this validator.

• I reused in the ingest_pipelines plugin the strings that already existed in index_lifecycle_management.

• I added the new validation in the Community ID form in the ingest_pipelines plugin. Also added some test verifying that the processor doesn't create when the seeds validation fails.

• Changed the method in the index_lifecycle_management validator so now it uses the reusable one.

Now the Ingest pipeline forms shows the validation when the number is not an integer:

And the index_lifecycle_management still shows the validations as expected:

[elasticmachine]

Pinging @elastic/kibana-management (Team:Kibana Management)

[ElenaStoeva]

Amazing job @SoniaSanzV! Many thanks for adding unit tests and jest tests for the processor! Tested locally and it works well. I only have a suggestion on the validation logic.

[ElenaStoeva]

I am thinking of a case where this might not work properly - if value is true or false, Number(value) would evaluate to a number 1 or 0, which is an integer.

From what I understand, we only return undefined in these three cases:

• If value is an empty string
• If value is of type number and it is an integer
• If value is of type string and represents an integer
  Otherwise returning the error message.

Following the above rules, I think the logic can be simplified to something like this:

  if (
    value === '' ||
    (typeof value === 'number' && Number.isInteger(value)) || 
    (typeof value === 'string' && Number.isInteger(Number(value)))
  ) {
    return undefined;
  }

  return { message, code: 'ERR_NOT_INT_NUMBER' };

wdyt?

[SoniaSanzV]

Thank for your comments, Elena!

I don't think it would be a problem in the case of a boolean. If value is a boolean it won't do Number(value) because it only do so for typeof string. If type is a boolean the evaluation will be Number.isInteger(false) and then will return { message, code: 'ERR_NOT_INT_NUMBER' } (I now realize that the name of the const numValue is not good because it's not always a number, like this boolean case)

In any case, probably your option is more precise so I can change it. And I'll add the boolean cases in the UTs too, I think they are a good case to be tested.

[ElenaStoeva]

You're right about the booleans, I missed that it will directly evaluate to Number.isInteger(false)!

I guess my suggestion was mainly around simplifying the logic so that the cases where we return undefined are more explicit and readable.

And I'll add the boolean cases in the UTs too, I think they are a good case to be tested.

Yes, that would be great, thanks!

[ElenaStoeva]

Hey @SoniaSanzV, sorry for the delay with my review! Latest changes lgtm, thanks for addressing my feedback!

Before we merge this, I know one more place which would benefit from this new validator! 😄 We just added integer validation for the data stream retention period modal (#196524) - could you please replace the custom integer validation here with the new es_ui_shared validation?

[SoniaSanzV]

As @ElenaStoeva requested, the validator has also been used in the EditDataRetentionModal.

[ElenaStoeva]

Great job @SoniaSanzV! Tested locally again and works as expected 🎉

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 6b33867

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
esUiShared	220	221	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
indexLifecycleManagement	165.9KB	166.0KB	+120.0B
indexManagement	688.4KB	688.3KB	-49.0B
ingestPipelines	404.6KB	404.8KB	+202.0B
total			+273.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
esUiShared	106.7KB	106.9KB	+235.0B

History

• 💚 Build #243739 succeeded 81a7eda3f4970a19ce1da5c0cd39c147754bbb3d
• 💚 Build #243400 succeeded 431f53542adcbf6534769a38079cf468436550d6
• 💔 Build #243031 failed 427762d7a68f648b6bbd126c05dba77f3442c58c

cc @SoniaSanzV

[kibanamachine]

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11503207094

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation