[ILM] Show forcemerge in hot when rollover is searchable snapshot is enabled

[jloleysens]

Summary

• Implement stakeholder feedback to show forcemerge in hot even when Searchable Snapshot (SS) is on
• Fix bug with validation on policy that does not have rollover and does not block saving when rollover is enabled with no values

Gif of SS

[elasticmachine]

Pinging @elastic/es-ui (Team:Elasticsearch UI)

[alisonelizabeth]

Code LGTM. Tested locally. Great catch on the validation bug! Is there a test that we could add that might have caught this?

[alisonelizabeth]

nit: might be a little easier to read using if...else instead of a nested ternary.

[cjcenizal]

A switch/case could also work.

[jloleysens]

@cjcenizal in this case the switch statement would look something like:

    switch (true) {
      case 1 === 1:
        return 'ok';
      case 2 === 2:
        return 'ok then';
      default:
        return 'not ok';
    }

Just curious to hear thoughts on that pattern where we have true as the deciding condition and predicate expressions per case. IMO it might be less easy to grok for the majority of readers because it is less common - WDYT?

[jloleysens]

My take is it does look pretty neat though!

[cjcenizal]

@jloleysens I was thinking something more like this unless I'm missing something:

switch (path) {
  case ROLLOVER_FORM_PATHS.maxAge:
    return {
      code: ROLLOVER_EMPTY_VALIDATION,
      message: i18nTexts.editPolicy.errors.maximumAgeRequiredMessage,
    };

  case ROLLOVER_FORM_PATHS.maxDocs:
    return {
      code: ROLLOVER_EMPTY_VALIDATION,
      message: i18nTexts.editPolicy.errors.maximumDocumentsRequiredMessage,
    };

  default:
    return {
      code: ROLLOVER_EMPTY_VALIDATION,
      message: i18nTexts.editPolicy.errors.maximumSizeRequiredMessage,
    };
}

[jloleysens]

Ah OK, I had a different idea. Yours is simpler!

[jloleysens]

@alisonelizabeth thanks for the review! I've addressed your feedback in my latest commit.

Is there a test that we could add that might have caught this?

Yes, but I don't think adding a test at the level of this plugin is the correct place. This plugin does specify a test for the validation on rollover showing under expected conditions:

should show errors when trying to save with no max size, no max age and no max docs

That should theoretically have covered this, but because of how the form lib works with validation on submit, this was not caught. So I pivoted to using the form lib in a way that it does work, we should probably add a test case for this in the form lib - I'll open an issue for it.

[jloleysens]

@alisonelizabeth issue here: #85383

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 1f02f17

Metrics [docs]

Async chunks

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

id	before	after	diff
indexLifecycleManagement	242.8KB	243.1KB	+293.0B

Distributable file count

id	before	after	diff
default	46960	47720	+760

History

• 💚 Build #92771 succeeded a0c382a

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