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] Formatting fixes for CSV export in Discover, CSV download from Dashboard panel #67027

Merged

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented May 19, 2020

Summary

Part of #68718.

Addresses bugs and inconsistencies in CSV exports via more code re-use and better interfaces to the reusable code.

  • Fixes the BOM issue for Download CSV
  • Fixes Download CSV showing raw data instead of formatted per the index pattern
  • Fixes the date + timezone formatting issues for Download CSV
  • Makes timezone formatting work if Advanced Settings dateFormat:tz is set to browser
  • Completes export when the index pattern contains an alias to a closed index

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch 2 times, most recently from 4d6eb14 to 892e804 Compare May 19, 2020 16:51
@tsullivan tsullivan changed the title Reporting/csv date format consistency [Reporting] Code cleanup for CSV export for Dashboard May 19, 2020
@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch 3 times, most recently from af43ff0 to e147e1f Compare May 21, 2020 15:53
@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch 4 times, most recently from 24ea1a2 to 086a42a Compare May 27, 2020 20:16
@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch 6 times, most recently from e9211d3 to 18f33e1 Compare June 9, 2020 23:54
@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch from 18f33e1 to 3d4d0d5 Compare June 10, 2020 22:10
'xpack.reporting.exportTypes.csv_from_savedobject.executeJob.failedToAccessPanel',
{ defaultMessage: 'Failed to access panel metadata for job execution' }
);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This check didn't make sense because panel is constructed in the create_job

@tsullivan tsullivan changed the title [Reporting] Code cleanup for CSV export for Dashboard [Reporting] Code cleanup for CSV Export Types Jun 11, 2020
@tsullivan
Copy link
Member Author

Related: #60737 (comment)

@tsullivan tsullivan added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0 labels Jun 11, 2020
@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch 4 times, most recently from 0e2a133 to 2dea8ed Compare June 16, 2020 20:35
(ref: SavedObjectReference) => ref.type === 'index-pattern'
);
if (!indexPatternMeta) {
throw new Error('Could not find index pattern for the saved search!');
Copy link
Member Author

Choose a reason for hiding this comment

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

Error checking was moved here from create_job_search.ts which has been factored out

@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch from 2dea8ed to b08ac11 Compare June 16, 2020 20:41
@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch from 97141cb to 69090a2 Compare July 6, 2020 23:35
@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch 3 times, most recently from 400e2ff to c7840ae Compare July 13, 2020 17:25
@tsullivan tsullivan changed the title [Reporting] Various fixes for CSV export in Discover, CSV download from Dashboard panel [Reporting] Formatting fixes for CSV export in Discover, CSV download from Dashboard panel Jul 13, 2020
@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch from c7840ae to 14a7170 Compare July 13, 2020 19:52
…nload from Dashboard panel

commit e195964deaa3e7e8d94704d6514e01498c913a81
Author: Timothy Sullivan <[email protected]>
Date:   Mon Jul 13 10:17:36 2020 -0700

    Squashed commit of the following:

    commit 87c9c496a6cccaf7a60a44b496f7c0c0423cd2ea
    Merge: d531101ab3 ed749eb
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 10:17:02 2020 -0700

        Merge branch 'data/allow-custom-formatting' into reporting/csv-date-format-consistency

    commit d531101ab3c2f12628287bd5ad4a02bbf8b5c990
    Merge: 400e2ffba4 17dc043
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 10:15:38 2020 -0700

        Merge branch 'master' into reporting/csv-date-format-consistency

    commit ed749eb
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 10:12:28 2020 -0700

        move shared code to common

    commit 4e5eebd
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 09:07:32 2020 -0700

        3td time api doc chagens

    commit 34df331
    Merge: 54fa2fe 17dc043
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 08:50:21 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 400e2ffba4546cf78c53ce96b45a59878f0df076
    Author: Timothy Sullivan <[email protected]>
    Date:   Sun Jul 12 21:29:34 2020 -0700

        [Reporting] Data formatting fixes for CSV export in Discover, CSV download from Dashboard panel

    commit 54fa2fe
    Merge: 1b6e9e8 e1253ed
    Author: Elastic Machine <[email protected]>
    Date:   Sun Jul 12 22:18:38 2020 -0600

        Merge branch 'master' into data/allow-custom-formatting

    commit 1b6e9e8
    Author: Timothy Sullivan <[email protected]>
    Date:   Fri Jul 10 15:03:08 2020 -0700

        weird api change needed but no real diff

    commit fc9ff7b
    Merge: 736e9ee 66c531d
    Author: Timothy Sullivan <[email protected]>
    Date:   Fri Jul 10 14:51:51 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 736e9ee
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 17:43:10 2020 -0700

        fix path for tests

    commit 1bebcc8
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 17:25:09 2020 -0700

        re-use public code in server, add test

    commit 1e1d3c5
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 16:35:30 2020 -0700

        rerun api changes

    commit 231f793
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 16:31:55 2020 -0700

        fix src/plugins/data/public/field_formats/constants.ts

    commit d42275c
    Merge: 206aed6 8e2277a
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 16:01:40 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 206aed6
    Merge: 5aa2d80 09da110
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 15:03:12 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 5aa2d80
    Author: Timothy Sullivan <[email protected]>
    Date:   Wed Jul 8 12:12:31 2020 -0700

        api doc changes

    commit 76e2c30
    Merge: 1789afc 595e9c2
    Author: Timothy Sullivan <[email protected]>
    Date:   Wed Jul 8 12:04:12 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 1789afc
    Author: Timothy Sullivan <[email protected]>
    Date:   Fri Jul 3 11:23:03 2020 -0700

        simplify changes

    commit 6428455
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 2 16:05:57 2020 -0700

        add more to tests - need help though

    commit 6aacfbd
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 2 12:04:28 2020 -0700

        [Data Plugin] Allow server-side date formatters to accept custom timezone

        When Advanced Settings shows the date format timezone to be "Browser,"
        this means nothing to field formatters in the server-side context. The
        field formatters need a way to accept custom format parameters. This
        allows a server-side module that creates a FieldFormatMap to set a
        timezone as a custom parameter. When custom formatting parameters exist,
        they get combined with the defaults.
@tsullivan tsullivan force-pushed the reporting/csv-date-format-consistency branch from 14a7170 to 15b21a1 Compare July 13, 2020 22:07
@@ -95,22 +101,3 @@ export interface CsvResultFromSearch {
type: string;
result: SavedSearchGeneratorResult;
}

export interface GenerateCsvParams {
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to server/export_types/csv/server/generate_csv/index.ts since it's only used for that module

logger.debug('executing scroll request');
return parseResponse(
callEndpoint('scroll', {
await callEndpoint('scroll', {
Copy link
Member Author

Choose a reason for hiding this comment

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

The previous code was passing through a promise, for it to be resolved in some other context

@@ -10,8 +10,10 @@ import { CancellationToken } from '../../../../../common';
import { LevelLogger } from '../../../../lib';
import { ScrollConfig } from '../../../../types';

async function parseResponse(request: SearchResponse<any>) {
const response = await request;
Copy link
Member Author

Choose a reason for hiding this comment

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

This await was cheating against the TS type of request

Copy link
Contributor

@joelgriffith joelgriffith left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@tsullivan tsullivan marked this pull request as ready for review July 13, 2020 23:14
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@tsullivan tsullivan merged commit 2340f8a into elastic:master Jul 14, 2020
@tsullivan tsullivan deleted the reporting/csv-date-format-consistency branch July 14, 2020 00:22
tsullivan added a commit to tsullivan/kibana that referenced this pull request Jul 14, 2020
… from Dashboard panel (elastic#67027)

* [Reporting] Data formatting fixes for CSV export in Discover, CSV download from Dashboard panel

commit e195964deaa3e7e8d94704d6514e01498c913a81
Author: Timothy Sullivan <[email protected]>
Date:   Mon Jul 13 10:17:36 2020 -0700

    Squashed commit of the following:

    commit 87c9c496a6cccaf7a60a44b496f7c0c0423cd2ea
    Merge: d531101ab3 ed749eb
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 10:17:02 2020 -0700

        Merge branch 'data/allow-custom-formatting' into reporting/csv-date-format-consistency

    commit d531101ab3c2f12628287bd5ad4a02bbf8b5c990
    Merge: 400e2ffba4 17dc043
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 10:15:38 2020 -0700

        Merge branch 'master' into reporting/csv-date-format-consistency

    commit ed749eb
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 10:12:28 2020 -0700

        move shared code to common

    commit 4e5eebd
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 09:07:32 2020 -0700

        3td time api doc chagens

    commit 34df331
    Merge: 54fa2fe 17dc043
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 08:50:21 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 400e2ffba4546cf78c53ce96b45a59878f0df076
    Author: Timothy Sullivan <[email protected]>
    Date:   Sun Jul 12 21:29:34 2020 -0700

        [Reporting] Data formatting fixes for CSV export in Discover, CSV download from Dashboard panel

    commit 54fa2fe
    Merge: 1b6e9e8 e1253ed
    Author: Elastic Machine <[email protected]>
    Date:   Sun Jul 12 22:18:38 2020 -0600

        Merge branch 'master' into data/allow-custom-formatting

    commit 1b6e9e8
    Author: Timothy Sullivan <[email protected]>
    Date:   Fri Jul 10 15:03:08 2020 -0700

        weird api change needed but no real diff

    commit fc9ff7b
    Merge: 736e9ee 66c531d
    Author: Timothy Sullivan <[email protected]>
    Date:   Fri Jul 10 14:51:51 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 736e9ee
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 17:43:10 2020 -0700

        fix path for tests

    commit 1bebcc8
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 17:25:09 2020 -0700

        re-use public code in server, add test

    commit 1e1d3c5
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 16:35:30 2020 -0700

        rerun api changes

    commit 231f793
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 16:31:55 2020 -0700

        fix src/plugins/data/public/field_formats/constants.ts

    commit d42275c
    Merge: 206aed6 8e2277a
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 16:01:40 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 206aed6
    Merge: 5aa2d80 09da110
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 15:03:12 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 5aa2d80
    Author: Timothy Sullivan <[email protected]>
    Date:   Wed Jul 8 12:12:31 2020 -0700

        api doc changes

    commit 76e2c30
    Merge: 1789afc 595e9c2
    Author: Timothy Sullivan <[email protected]>
    Date:   Wed Jul 8 12:04:12 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 1789afc
    Author: Timothy Sullivan <[email protected]>
    Date:   Fri Jul 3 11:23:03 2020 -0700

        simplify changes

    commit 6428455
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 2 16:05:57 2020 -0700

        add more to tests - need help though

    commit 6aacfbd
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 2 12:04:28 2020 -0700

        [Data Plugin] Allow server-side date formatters to accept custom timezone

        When Advanced Settings shows the date format timezone to be "Browser,"
        this means nothing to field formatters in the server-side context. The
        field formatters need a way to accept custom format parameters. This
        allows a server-side module that creates a FieldFormatMap to set a
        timezone as a custom parameter. When custom formatting parameters exist,
        they get combined with the defaults.

* comments
tsullivan added a commit that referenced this pull request Jul 14, 2020
… from Dashboard panel (#67027) (#71576)

* [Reporting] Data formatting fixes for CSV export in Discover, CSV download from Dashboard panel

commit e195964deaa3e7e8d94704d6514e01498c913a81
Author: Timothy Sullivan <[email protected]>
Date:   Mon Jul 13 10:17:36 2020 -0700

    Squashed commit of the following:

    commit 87c9c496a6cccaf7a60a44b496f7c0c0423cd2ea
    Merge: d531101ab3 ed749eb
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 10:17:02 2020 -0700

        Merge branch 'data/allow-custom-formatting' into reporting/csv-date-format-consistency

    commit d531101ab3c2f12628287bd5ad4a02bbf8b5c990
    Merge: 400e2ffba4 17dc043
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 10:15:38 2020 -0700

        Merge branch 'master' into reporting/csv-date-format-consistency

    commit ed749eb
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 10:12:28 2020 -0700

        move shared code to common

    commit 4e5eebd
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 09:07:32 2020 -0700

        3td time api doc chagens

    commit 34df331
    Merge: 54fa2fe 17dc043
    Author: Timothy Sullivan <[email protected]>
    Date:   Mon Jul 13 08:50:21 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 400e2ffba4546cf78c53ce96b45a59878f0df076
    Author: Timothy Sullivan <[email protected]>
    Date:   Sun Jul 12 21:29:34 2020 -0700

        [Reporting] Data formatting fixes for CSV export in Discover, CSV download from Dashboard panel

    commit 54fa2fe
    Merge: 1b6e9e8 e1253ed
    Author: Elastic Machine <[email protected]>
    Date:   Sun Jul 12 22:18:38 2020 -0600

        Merge branch 'master' into data/allow-custom-formatting

    commit 1b6e9e8
    Author: Timothy Sullivan <[email protected]>
    Date:   Fri Jul 10 15:03:08 2020 -0700

        weird api change needed but no real diff

    commit fc9ff7b
    Merge: 736e9ee 66c531d
    Author: Timothy Sullivan <[email protected]>
    Date:   Fri Jul 10 14:51:51 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 736e9ee
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 17:43:10 2020 -0700

        fix path for tests

    commit 1bebcc8
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 17:25:09 2020 -0700

        re-use public code in server, add test

    commit 1e1d3c5
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 16:35:30 2020 -0700

        rerun api changes

    commit 231f793
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 16:31:55 2020 -0700

        fix src/plugins/data/public/field_formats/constants.ts

    commit d42275c
    Merge: 206aed6 8e2277a
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 16:01:40 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 206aed6
    Merge: 5aa2d80 09da110
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 9 15:03:12 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 5aa2d80
    Author: Timothy Sullivan <[email protected]>
    Date:   Wed Jul 8 12:12:31 2020 -0700

        api doc changes

    commit 76e2c30
    Merge: 1789afc 595e9c2
    Author: Timothy Sullivan <[email protected]>
    Date:   Wed Jul 8 12:04:12 2020 -0700

        Merge branch 'master' into data/allow-custom-formatting

    commit 1789afc
    Author: Timothy Sullivan <[email protected]>
    Date:   Fri Jul 3 11:23:03 2020 -0700

        simplify changes

    commit 6428455
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 2 16:05:57 2020 -0700

        add more to tests - need help though

    commit 6aacfbd
    Author: Timothy Sullivan <[email protected]>
    Date:   Thu Jul 2 12:04:28 2020 -0700

        [Data Plugin] Allow server-side date formatters to accept custom timezone

        When Advanced Settings shows the date format timezone to be "Browser,"
        this means nothing to field formatters in the server-side context. The
        field formatters need a way to accept custom format parameters. This
        allows a server-side module that creates a FieldFormatMap to set a
        timezone as a custom parameter. When custom formatting parameters exist,
        they get combined with the defaults.

* comments
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 14, 2020
* master: (314 commits)
  [APM] Use status_code field to calculate error rate (elastic#71109)
  [Observability] Change appLink passing the date range (elastic#71259)
  [Security] Add Timeline improvements (elastic#71506)
  adjust vislib bar opacity (elastic#71421)
  Fix ScopedHistory mock and adapt usages (elastic#71404)
  [Security Solution] Add hook for reading/writing resolver query params (elastic#70809)
  [APM] Bug fixes from ML integration testing (elastic#71564)
  [Discover] Add caused_by.type and caused_by.reason to error toast modal (elastic#70404)
  [Security Solution] Add 3rd level breadcrumb to admin page (elastic#71275)
  [Security Solution][Exceptions] Exception modal bulk close alerts that match exception attributes (elastic#71321)
  Change signal.rule.risk score mapping from keyword to float (elastic#71126)
  Added help text where needed on connectors and alert actions UI (elastic#69601)
  [SIEM][Detections] Value Lists Management Modal (elastic#67068)
  [test] Skips test preventing promotion of ES snapshot elastic#71582
  [test] Skips test preventing promotion of ES snapshot elastic#71555
  [ILM] Fix alignment of the timing field (elastic#71273)
  [SIEM][Detection Engine][Lists] Adds the ability for exception lists to be multi-list queried. (elastic#71540)
  initial telemetry setup (elastic#69330)
  [Reporting] Formatting fixes for CSV export in Discover, CSV download from Dashboard panel (elastic#67027)
  Search across spaces (elastic#67644)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jul 14, 2020
…t-apps-page-titles

* 'master' of github.com:elastic/kibana: (88 commits)
  [ML] Functional tests - disable DFA creation and cloning tests
  [APM] Use status_code field to calculate error rate (elastic#71109)
  [Observability] Change appLink passing the date range (elastic#71259)
  [Security] Add Timeline improvements (elastic#71506)
  adjust vislib bar opacity (elastic#71421)
  Fix ScopedHistory mock and adapt usages (elastic#71404)
  [Security Solution] Add hook for reading/writing resolver query params (elastic#70809)
  [APM] Bug fixes from ML integration testing (elastic#71564)
  [Discover] Add caused_by.type and caused_by.reason to error toast modal (elastic#70404)
  [Security Solution] Add 3rd level breadcrumb to admin page (elastic#71275)
  [Security Solution][Exceptions] Exception modal bulk close alerts that match exception attributes (elastic#71321)
  Change signal.rule.risk score mapping from keyword to float (elastic#71126)
  Added help text where needed on connectors and alert actions UI (elastic#69601)
  [SIEM][Detections] Value Lists Management Modal (elastic#67068)
  [test] Skips test preventing promotion of ES snapshot elastic#71582
  [test] Skips test preventing promotion of ES snapshot elastic#71555
  [ILM] Fix alignment of the timing field (elastic#71273)
  [SIEM][Detection Engine][Lists] Adds the ability for exception lists to be multi-list queried. (elastic#71540)
  initial telemetry setup (elastic#69330)
  [Reporting] Formatting fixes for CSV export in Discover, CSV download from Dashboard panel (elastic#67027)
  ...

# Conflicts:
#	x-pack/plugins/index_management/public/application/index.tsx
}

return { headers: decryptedHeaders } as KibanaRequest;
};
Copy link
Member Author

Choose a reason for hiding this comment

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

Oh snap - this was added from a bad merge. This file should have been removed as of #71031 but I accidentally added it back in this PR somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants