Callout breaking changes on integration upgrade

[MichelLosier]

Summary

Resolves: #187481

• Enhances the integration upgrade callout to give special attention to breaking changes in the changelog.
  • Callout includes a CTA to review breaking changes
    • If one breaking change between current and latest version CTA is a direct link the PR
    • If many breaking changes, a flyout is opened listing those breaking changes
  • Includes "I understand" checkbox that must be clicked before upgrade is allowed
    • Upgrade button is disabled until this is confirmed

Local testing

• Go to the integrations view
• Find and click the docker integration
• Change the version of the integration in the url path to 1.2.0
  • Example: <localhost:port>/kibana/app/integrations/detail/docker-1.2.0/overview
• Click the "Add Docker Metrics" CTA
• After install go to the Docker integration settings view

Screenshots

Single breaking change

Multiple breaking changes

Multiple breaking changes with flyout

No breaking changes

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
• Flaky Test Runner was used on any tests changed
• The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

• See some risk examples
• ...

[MichelLosier]

Here is a first pass implementation that adds a separate breaking change warning callout:

Having the upgrade callout, and the breaking change callout be the same kind of "warning" style feels like too much attention grabbing here.

Alternative considerations:

• Combine the breaking change messaging into the single "New version available" callout.
  • The breaking changes portion feels like it needs its own prominence if in a shared callout, maybe give it its own sub-heading in the callout?
• The existing "New version available" callout could be better suited with a "primary" (blue) info callout, and maybe a "cheer" icon to announce it (see example below). Separately the breaking change callout follows the "warning" style due to the risk.
  • That said, I could also read the "New version available" as a warning message since there is risk with not keeping integrations up-to-date

Example:

[MichelLosier]

After consulting with @sileschristian the UX approach we'll take here is:

• Update and breaking changes messaging uses the same callout box
• When there are breaking changes:
  • A Primary CTA is shown to review the breaking changes
    • If there is a single breaking change PR, the CTA links directly the PR. Otherwise, if multiple a flyout is opened with a list of the PR links for review
  • Include a checkbox for acknowledging the impact of the breaking changes. This gates the disabled state of the upgrade button.
• In all scenarios, we provide a secondary CTA to "View changelog" which opens the changelog modal.

[jamiehynds]

@kpollich Instead of linking directly to GitHub, could we expand the "Review breaking changes" section to surface the relevant details of the breaking change directly in-product? This would allow us to communicate the nature of the change, its potential impact, and any recommended actions more clearly. We can still include a link to the PR for reference, but since not every PR will contain all the necessary context, having this information curated in-product would provide a more consistent and helpful experience for users.

[MichelLosier]

The changes in the changelog also include a description field, if that seems to provide sufficient context which should be easy to include in the flyout list.
Example changelog: https://github.com/elastic/integrations/blob/main/packages/o365/changelog.yml

In the case where there is 1 breaking change between the user's current version and the latest version, the CTA doesn't open the flyout and links directly to the PR. In this case we could include the change description in the body of the warning callout

[kpollich]

+1 for using the description property for the additional context.

[MichelLosier]

Here is how this could look including the description field:

Multiple breaking changes, flyout:

One breaking change, in callout body:

Thoughts @jamiehynds ?
@sileschristian any UX improvements you want to callout if we include this?

[jamiehynds]

Looking much better, thanks @MichelLosier.

For the flyout, I think displaying the description field on top and the Github link below might look better. The callout body looks great - it allows us to convey the extent/nature of the breaking change and user acknowledgement.

@chrisberkhout using your o365 breaking change as an example, do you think the description field is enough to communicate the nature of the breaking change and steps to take post-upgrade?

Regarding the acknowledgement, I wonder if it might cause issues for users who use the API to upgrade integrations? Could it potentially break any automations in place?

[kpollich]

Regarding the acknowledgement, I wonder if it might cause issues for users who use the API to upgrade integrations? Could it potentially break any automations in place?

For now I don't believe we're planning to put the same restriction in place for upgrade initiated by the API. The only way I can really think to do this would be to mandate a new query param ?acceptBreakingChanges=true to the upgrade API, but this would itself be a breaking change if we prevent upgrades without that parameter. I think making this a UI-only flow makes the most sense, and API users will continue to be able to upgrade without acknowledgement.

[MichelLosier]

I think displaying the description field on top and the Github link below might look better.

For that choice, I sided with keeping the prominent weight with the links, and description underneath since we want the user clicking and really getting their full understanding of those breaking changes from the PR itself. Since the descriptive text is more of a brief summary, it could deflect that ask for a thorough review if given more presence.

[MichelLosier]

Current design direction from @sileschristian for incorporating the change descriptions

[elasticmachine]

Pinging @elastic/fleet (Team:Fleet)

[nchaulet]

Tested locally and worked as expected LGTM 🚀

[elasticmachine]

💔 Build Failed

• Buildkite Build
• Commit: f2ac70e

Failed CI Steps

• Quick Checks
• Check Types

History

• 💔 Build #293897 failed e2f91d9

[jamiehynds]

Thanks for working through this one! Is there any documentation that I can provide to the integration team, to ensure they understand how to specify a breaking change in an integration update to avail of this new UX?

[MichelLosier]

@jamiehynds They'll just need to make sure they add a change log entry with the breaking-change type using the elastic-package changelog add command as described here:

• https://github.com/elastic/elastic-package?tab=readme-ov-file#elastic-package-changelog-add

The description of that entry is what will show in the UI.

More on the workflow for updating packages:

• https://www.elastic.co/docs/extend/integrations/quick-start#qs-release-new-version

[nchaulet]

@MichelLosier Looks like that change is triggering an error for every custom integration that do not have a changelog,

You can create a custom integration with

POST kbn:/api/fleet/epm/custom_integrations
{
 "integrationName": "test",
 "datasets": []
}

I think we may verify there is a changelog in packageInfo.path before enabling the query to fetch the changelog