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

[Uptime] Fix "last updated" field #28720

Merged

Conversation

justinkambic
Copy link
Contributor

@justinkambic justinkambic commented Jan 14, 2019

Summary

In #28090 we specified that the Last update field on the Monitors Page does not display values consistent with the time since the last update when a monitor is stopped. This happened for a number of reasons.

Firstly - when Apollo Query objects receive the same data, by default it gets cached. Also, we weren't sorting the top hits value to ensure the latest doc would always be returned. To top it off - the component was referencing a key on the query result that is always undefined! To help prevent this from happening in the future I added a reference to the GQL type expected by the query object.

In addition to fixing these problems, I also extracted a large portion of this query component into a functional component. I provided a default/loading state for it as well.

Before:
image
The above image was captured 13 minutes after the last check was received.

After:
image

Testing this PR

To test this PR:

  1. Start a monitor, navigate to the monitor page for it
  2. Ensure you're seeing data, kill Heartbeat
  3. Within a minute you should see the text update

@justinkambic justinkambic added bug Fixes for quality problems that affect the customer experience v7.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v6.7.0 labels Jan 14, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime

@justinkambic justinkambic requested a review from andrewvc January 14, 2019 20:39
@justinkambic justinkambic changed the title [Uptime] Fix last updated field [Uptime] Fix "last updated" field Jan 14, 2019
@justinkambic justinkambic mentioned this pull request Jan 14, 2019
31 tasks
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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

</EuiFlexItem>
</EuiFlexGroup>
</EuiPanel>
<StatusBar
Copy link
Contributor

Choose a reason for hiding this comment

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

So much cleaner :)

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.

On second thought these need aria labels.

@justinkambic
Copy link
Contributor Author

@andrewvc I added aria-labels in f60650b.

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

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@justinkambic justinkambic merged commit c96068c into elastic:master Jan 23, 2019
justinkambic added a commit to justinkambic/kibana that referenced this pull request Jan 24, 2019
* Update MonitorStatusBar to not reference undefined value.

* Add sort to top_hits aggregation.

* Add default and loading state for status bar.

* Change bool check.

* Add aria-labels for monitor status bar.
justinkambic added a commit that referenced this pull request Jan 25, 2019
* [Uptime] Fix "last updated" field (#28720)

* Update MonitorStatusBar to not reference undefined value.

* Add sort to top_hits aggregation.

* Add default and loading state for status bar.

* Change bool check.

* Add aria-labels for monitor status bar.

* Update expected test result

Fix test because of significant digit limitations.

* Remove unused code.

* Fix bug.
@justinkambic
Copy link
Contributor Author

Backported to:
6.x/6.7.0 f3723e8
#29282

@justinkambic justinkambic deleted the uptime_fix-last-updated-field branch January 25, 2019 19:35
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 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v6.7.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants