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

[Reporting] Remove "download CSV" export type functionality, Part 2 #201010

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Nov 20, 2024

Follows #199033

Summary

This PR further removes logic that uses xpack.reporting.csv.enablePanelActionDownload, and removes references of that setting from our documentation. See #199033 for the release note.

Changes

  1. enablePanelActionDownload exists as an optional setting in packages/kbn-reporting/server/config_schema.ts
  2. The only reference to this setting marks it as unused in x-pack/plugins/reporting/server/config/index.ts
  3. Removes the detection of the setting from the Upgrade Assistant integration

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

  • Documentation was added for features that require explanation or tutorials
  • 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.

@tsullivan tsullivan added release_note:breaking backport:skip This commit does not require backporting Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Feature:Reporting:CSV Reporting issues pertaining to CSV file export labels Nov 20, 2024
Copy link
Contributor

A documentation preview will be available soon.

Request a new doc build by commenting
  • Rebuild this PR: run docs-build
  • Rebuild this PR and all Elastic docs: run docs-build rebuild

run docs-build is much faster than run docs-build rebuild. A rebuild should only be needed in rare situations.

If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here.

@tsullivan tsullivan force-pushed the reporting/remove-csv-download-upgrade-assistant branch from b5ff439 to 60d1cf9 Compare November 20, 2024 17:14
@tsullivan tsullivan force-pushed the reporting/remove-csv-download-upgrade-assistant branch from 60d1cf9 to 392b936 Compare November 20, 2024 19:17
@tsullivan tsullivan marked this pull request as ready for review November 21, 2024 23:47
@tsullivan tsullivan requested a review from a team as a code owner November 21, 2024 23:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@elastic elastic deleted a comment from elasticmachine Nov 21, 2024
@tsullivan
Copy link
Member Author

/ci

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #9 / DeleteConfirmationModal calls onConfirm

Metrics [docs]

✅ unchanged

Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@@ -60,7 +60,7 @@ const CaptureSchema = schema.object({
const CsvSchema = schema.object({
checkForFormulas: schema.boolean({ defaultValue: true }),
escapeFormulaValues: schema.boolean({ defaultValue: false }),
enablePanelActionDownload: schema.boolean({ defaultValue: false }), // unused as of 9.0
enablePanelActionDownload: schema.maybe(schema.boolean({ defaultValue: false })), // unused as of 9.0
Copy link
Contributor

Choose a reason for hiding this comment

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

So we still allow the enablePanelActionDownload setting to be in the yml file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. We do not want to block instances from startup if this setting exists. The extent of the breaking change is just to ignore the setting.

@tsullivan tsullivan merged commit 7ce5069 into elastic:main Nov 22, 2024
25 checks passed
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024
…lastic#201010)

Follows elastic#199033

## Summary

This PR further removes logic that uses
`xpack.reporting.csv.enablePanelActionDownload`, and removes references
of that setting from our documentation. See
elastic#199033 for the **release note**.

### Changes
1. `enablePanelActionDownload` exists as an **optional** setting in
`packages/kbn-reporting/server/config_schema.ts`
2. The only reference to this setting marks it as unused in
`x-pack/plugins/reporting/server/config/index.ts`
3. Removes the detection of the setting from the Upgrade Assistant
integration

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] 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.
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
…lastic#201010)

Follows elastic#199033

## Summary

This PR further removes logic that uses
`xpack.reporting.csv.enablePanelActionDownload`, and removes references
of that setting from our documentation. See
elastic#199033 for the **release note**.

### Changes
1. `enablePanelActionDownload` exists as an **optional** setting in
`packages/kbn-reporting/server/config_schema.ts`
2. The only reference to this setting marks it as unused in
`x-pack/plugins/reporting/server/config/index.ts`
3. Removes the detection of the setting from the Upgrade Assistant
integration

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] 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.
@tsullivan tsullivan deleted the reporting/remove-csv-download-upgrade-assistant branch December 16, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Reporting:CSV Reporting issues pertaining to CSV file export Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants