Adding in browser info to the report-info drawer #26307

joelgriffith · 2018-11-27T22:54:25Z

Summary

Adds in browser-type to the reporting info side-panel. Will tackle the next TODO once I can figure out a good way to source what kicked off the report.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~[ ] This was checked for cross-browser compatibility, including a check against IE11~~
~~[ ] 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~~
[X ] Unit or functional tests were updated or added to match the most common scenarios
~~[ ] This was checked for keyboard-only and screenreader accessibility~~

joelgriffith · 2018-11-27T22:56:24Z

x-pack/plugins/reporting/server/lib/esqueue/helpers/create_index.js

@@ -41,6 +41,7 @@ const schema = {
      },
    }
  },


Not sure if this makes much a difference since we're likely updating an existing index in current installations. Is there more here that I need to follow up with?

Additions to the data model should be fine as long as it is released in a minor version (not a patch). It'll be worthwhile to test an upgrade scenario: create some reports in 6.5.x and then upgrade to 6.6.x, and check out the browser_type behavior.

elasticmachine · 2018-11-27T23:39:08Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

tsullivan · 2018-11-28T16:53:35Z

x-pack/plugins/reporting/public/components/report_info_button.tsx

@@ -145,6 +144,10 @@ export class ReportInfoButton extends Component<Props, State> {
          title: 'Status',
          description: get(info, 'status', NA),
        },
+        {
+          title: 'Browser Type',
+          description: get(info, 'browser_type', NA),


It's not a huge deal, but it would be nice if this said unknown if the report in the listing is a PDF report that was created in an older version of Kibana where the browser_type isn't available. If the report is a CSV job, it should say n/a

Ah, great point, forgot about our old friend CSV -- will adjust

joelgriffith · 2018-11-29T18:55:35Z

I don't want this to land in a patch release... are patch releases branched off of their respective minor tags?

tsullivan · 2018-11-29T19:05:27Z

I don't want this to land in a patch release... are patch releases branched off of their respective minor tags?

For it to land in a patch release, someone would need to backport it to the 6.5 branch, which we will not do.

After this is merged to master (7.0), I'll ask you to backport it to 6.x, which is versioned 6.6.0.

I'm also adding version labels to this PR for you. I sometimes accidentally overlook the need for version labels in PRs when I review them - sorry about that :)

joelgriffith · 2018-11-29T19:08:41Z

Awesome! I'll try and add these myself as time goes on. :dank:

elasticmachine · 2018-11-29T19:41:10Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

tsullivan

Sent a suggestion. I haven't used this Github feature before, so not sure it's going to work!

x-pack/plugins/reporting/server/lib/enqueue_job.js

joelgriffith · 2018-11-29T21:50:06Z

Nice! That's a pretty cool feature. Saves me from going back and forth between the editor + GH

joelgriffith · 2018-11-29T21:50:51Z

x-pack/plugins/reporting/server/lib/enqueue_job.js

+  const config = server.config();
+  const queueConfig = config.get('xpack.reporting.queue');
+  const browserType = config.get('xpack.reporting.capture.browser.type');
+  const browserType = server.config().get('xpack.reporting.capture.browser.type');


Will fix this dupe

elasticmachine · 2018-11-29T22:19:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-11-30T18:38:15Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-12-03T19:57:25Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

tsullivan

LGTM!

Reviewed the code and tested PDF and CSV reports

* Adding in browser info to the report-info drawer * Conditionalizes the browser-type panel prints + constantizes jobTypes * Fixing config feedback lost in force push

joelgriffith requested review from chrisdavies and tsullivan November 27, 2018 22:54

joelgriffith commented Nov 27, 2018

View reviewed changes

joelgriffith added review (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead labels Nov 27, 2018

tsullivan reviewed Nov 28, 2018

View reviewed changes

tsullivan added v7.0.0 v6.6.0 labels Nov 29, 2018

joelgriffith mentioned this pull request Nov 29, 2018

[Reporting] Check Elasticsearch http.max_content_length against Kibana xpack.reporting.csv.maxSizeBytes #26383

Closed

tsullivan reviewed Nov 29, 2018

View reviewed changes

x-pack/plugins/reporting/server/lib/enqueue_job.js Outdated Show resolved Hide resolved

joelgriffith closed this Nov 29, 2018

joelgriffith reopened this Nov 29, 2018

joelgriffith commented Nov 29, 2018

View reviewed changes

Joel Griffith added 3 commits December 3, 2018 11:10

Adding in browser info to the report-info drawer

00a3edc

Conditionalizes the browser-type panel prints + constantizes jobTypes

66c0ef1

Fixing config feedback lost in force push

e73218f

tsullivan approved these changes Dec 4, 2018

View reviewed changes

joelgriffith merged commit 53f7ed8 into elastic:master Dec 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding in browser info to the report-info drawer #26307

Adding in browser info to the report-info drawer #26307

joelgriffith commented Nov 27, 2018 •

edited

Loading

joelgriffith Nov 27, 2018

tsullivan Nov 28, 2018

elasticmachine commented Nov 27, 2018

tsullivan Nov 28, 2018

joelgriffith Nov 28, 2018

joelgriffith commented Nov 29, 2018

tsullivan commented Nov 29, 2018

joelgriffith commented Nov 29, 2018

elasticmachine commented Nov 29, 2018

tsullivan left a comment

joelgriffith commented Nov 29, 2018

joelgriffith Nov 29, 2018

elasticmachine commented Nov 29, 2018

elasticmachine commented Nov 30, 2018

elasticmachine commented Dec 3, 2018

tsullivan left a comment

@@ @@ -41,6 +41,7 @@ const schema = { @@
                     },
                   }
                 },

Adding in browser info to the report-info drawer #26307

Adding in browser info to the report-info drawer #26307

Conversation

joelgriffith commented Nov 27, 2018 • edited Loading

Summary

Checklist

joelgriffith Nov 27, 2018

Choose a reason for hiding this comment

tsullivan Nov 28, 2018

Choose a reason for hiding this comment

elasticmachine commented Nov 27, 2018

💚 Build Succeeded

tsullivan Nov 28, 2018

Choose a reason for hiding this comment

joelgriffith Nov 28, 2018

Choose a reason for hiding this comment

joelgriffith commented Nov 29, 2018

tsullivan commented Nov 29, 2018

joelgriffith commented Nov 29, 2018

elasticmachine commented Nov 29, 2018

💚 Build Succeeded

tsullivan left a comment

Choose a reason for hiding this comment

joelgriffith commented Nov 29, 2018

joelgriffith Nov 29, 2018

Choose a reason for hiding this comment

elasticmachine commented Nov 29, 2018

💔 Build Failed

elasticmachine commented Nov 30, 2018

💔 Build Failed

elasticmachine commented Dec 3, 2018

💚 Build Succeeded

tsullivan left a comment

Choose a reason for hiding this comment

joelgriffith commented Nov 27, 2018 •

edited

Loading