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

[O11y Overview] Add code to display/hide UX section when appropriate #80873

Merged

Conversation

justinkambic
Copy link
Contributor

@justinkambic justinkambic commented Oct 16, 2020

Summary

Resolves #80197.

Resolves #81081.

It was noted that a preview is shown for the UX dashboard even when no data is present, rather than the call to action that is appropriate.

The cause is a bug that doesn't look for the appropriate field. The UX app has a hasData flag that is absent from the other apps in the API response. The correct solution to this problem is updating the API to return the correct value, but I felt as a fix for this bug for 7.10.0 this would be less obtrusive.

We should track this and update the API in 7.11.0 to make UX status follow the same interface as the rest of the o11y apps so we don't have special handling code like this.

NOTE: I created a follow-up issue to track these error suppressions, we should follow-up with a 7.11.0 PR to fix the API.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@justinkambic justinkambic added bug Fixes for quality problems that affect the customer experience v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability Team:Observability Team label for Observability Team (for things that are handled across all of observability) v7.10.0 v7.11.0 labels Oct 16, 2020
@justinkambic justinkambic self-assigned this Oct 16, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@justinkambic justinkambic added the release_note:skip Skip the PR/issue when compiling release notes label Oct 16, 2020
@justinkambic justinkambic force-pushed the overview_display-empty-ux-section branch from e5065f3 to 4c6ff3d Compare October 19, 2020 23:16
@@ -64,7 +64,8 @@ export function DataSections({ bucketSize, hasData, absoluteTime, relativeTime }
/>
</EuiFlexItem>
)}
{hasData?.ux && (
{/* @ts-ignore see https://github.com/elastic/kibana/issues/81081 */}
{hasData?.ux?.hasData && (
Copy link
Contributor

@shahzad31 shahzad31 Oct 20, 2020

Choose a reason for hiding this comment

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

instead of changing the api or suppressing the type , i feel like we can improve the types here. if you notice two lines below it's also (hasData.ux as UXHasDataResponse).serviceName as string used like that.
there is actually already a type declared to handle this specific api use case https://github.com/elastic/kibana/blob/master/x-pack/plugins/observability/public/typings/fetch_overview_data/index.ts#L40

need to check why it's not working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
observability 166.6KB 166.6KB +12.0B

History

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

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

LGTM

@justinkambic justinkambic merged commit c11163d into elastic:master Oct 21, 2020
justinkambic added a commit to justinkambic/kibana that referenced this pull request Oct 21, 2020
…lastic#80873)

* Add code to display/hide UX section when appropriate.

* Suppress errors with link to dedicated issue.

* Fix typo in call to action.

* Remove type error suppression.
justinkambic added a commit to justinkambic/kibana that referenced this pull request Oct 21, 2020
…lastic#80873)

* Add code to display/hide UX section when appropriate.

* Suppress errors with link to dedicated issue.

* Fix typo in call to action.

* Remove type error suppression.
justinkambic added a commit that referenced this pull request Oct 21, 2020
…80873) (#81308)

* Add code to display/hide UX section when appropriate.

* Suppress errors with link to dedicated issue.

* Fix typo in call to action.

* Remove type error suppression.
justinkambic added a commit that referenced this pull request Oct 21, 2020
…80873) (#81307)

* Add code to display/hide UX section when appropriate.

* Suppress errors with link to dedicated issue.

* Fix typo in call to action.

* Remove type error suppression.
@justinkambic justinkambic deleted the overview_display-empty-ux-section branch October 21, 2020 13:17
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 21, 2020
…arm-phase-to-formlib

* 'master' of github.com:elastic/kibana: (55 commits)
  [UX] Fix map color variance and apply proper filter for extended stats (elastic#81106)
  [User Experience] Use EuiSelect for percentiles instead of SuperSelect (elastic#81082)
  [DOCS] Add link for monitoring ssl settings (elastic#81057)
  [test] Await loading indicator in monitoring test (elastic#81279)
  [ILM] Minor copy and link additions to cloud CTA for cold phase (elastic#80512)
  [Mappings editor] Add scaled_float and date_range comp integration tests (elastic#81287)
  [Discover] Deangularize context.app (elastic#80851)
  [O11y Overview] Add code to display/hide UX section when appropriate (elastic#80873)
  [Discover] Extend DiscoverNoResults component to show different message on error (elastic#79671)
  Fix tagcloud word overlapping (elastic#81161)
  [Security Solution] Fixes flaky test rules (elastic#81040)
  Changed the code to avoid tech debt with hacky solutions after receiving comments on EUI issue reported about this problem. (elastic#81183)
  [Security Solution][All] Replace old markdown renderer with the new one (elastic#80301)
  Add namespaced version of the API call (elastic#81278)
  [ML] Data Frame Analytics: Fix race condition and support for feature influence legacy format. (elastic#81123)
  [Fleet] Fix POLICY_CHANGE action creation for new policy (elastic#81236)
  [Security Solution][Endpoint][Admin] Malware user notification checkbox (elastic#78084)
  [SecuritySolution][Unit Tests] - fix flakey unit test (elastic#81239)
  skip flaky suite (elastic#81264)
  [Maps] fix top-level Map page is called 'Kibana' (elastic#81238)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared/forcemerge_field.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Observability Team label for Observability Team (for things that are handled across all of observability) Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.10.0 v7.11.0 v8.0.0
Projects
None yet
5 participants