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] Remove monitor states graphql #62853

Merged

Conversation

justinkambic
Copy link
Contributor

@justinkambic justinkambic commented Apr 7, 2020

Summary

Fixes #58020.

Note: don't review this until #59392 is merged.

Remove all remaining references to Apollo/GraphQL from the Uptime solution. This migrates the monitor list's state management to react-redux and updates our functional tests to not reference GraphQL.

There are also numerous changes to our server code, mostly revolved around removing the GQL endpoint.

We are also now maintaining a new set of runtime types for monitor states, and the remaining generated types are deleted.

Testing this PR

Smoke testing this PR should be sufficient from a functionality standpoint. There shouldn't be changes to functionality, and this is notably supported by the absence of any revisions to our functional UI tests.

The API tests on the other hand required a lot of revision, because not only has the API interface changed with GraphQL's removal, but the API responses are different to reflect the new runtime_types.

Run all tests and ensure that typing looks ok.

One special consideration is the usage of timestamp/@timestamp throughout the monitor state and check group typing. There are a number of timestamp fields, and some of them are of type string and some type number. This code might look confusing and questionable, but the typing was chosen to reflect what the server is returning today. I didn't make these choices, the typing was just not reflecting what we were doing before, until we got the runtime safety provided by io-ts. So we may want to have a conversation around these fields and what we want the data to look like. It's probably more appropriate to modify these fields in a dedicated PR though, rather than in a patch with a significant diff like this one.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@justinkambic justinkambic force-pushed the uptime_remove-monitor-states-graphql branch from c2f6d2f to 19bd02e Compare April 13, 2020 20:34
@justinkambic
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Looks great, awesome job removing fixtures, great cleanup overall.

@@ -1,126 +0,0 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

This component is deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved these tests here because they're running tests on MonitorListComponent.

Comment on lines +17 to +18
dateRangeStart: schema.string(),
dateRangeEnd: schema.string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

i guess we can can move to "to", "from" pattern here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer to not dedicate time to that specific task in this unwieldy patch, especially since there weren't many other things we wanted to correct in review. I have created elastic/uptime#173 dedicated to fixing this everywhere in the solution.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@justinkambic justinkambic merged commit 7efe7e8 into elastic:master Apr 17, 2020
justinkambic added a commit to justinkambic/kibana that referenced this pull request Apr 17, 2020
* WIP replacing GQL with redux/rest.

* Finish implementing migration.

* Introduce new connected component for ping list.

* Replace GraphQL type with io-ts.

* Update some broken tests.

* Add test for new helper function.

* Write test snapshots.

* Migrate api tests from graphql to rest.

* Update fixtures that rely on pings.

* Move ping types to runtime_types folder with rest of io-ts files.

* Update Ping type location and imports, type checking.

* Remove reliance on fixtures for ping functional API tests.

* Fix broken unit tests.

* Fix broken types.

* Remove local state storage from parent components.

* Add functional test to cover Ping List functionality.

* Fix monitor page functional test that was broken by merge conflicts.

* Fix broken tests.

* Fix broken API test.

* Replace a test with a describe block that will pre-navigate all tests.

* Delete unused reducer keys.

* Re-introduce loading to ping list reducer.

* Inroduce code that will cause PingList to re-fetch when refresh button is pressed.

* Update expanded rows to support multiple concurrent expanded rows.

* Modify pingList reducer to have singular optional error field.

* Delete unnecessary helper code.

* Delete unused interface.

* Add runtime_type to parse getPings params, fix pagination index.

* Add dedicated monitor type to runtime_types.

* Fix broken tests.

* Fix broken tests.

* Rename '@timestamp' property to 'timestamp' on Ping type.

* Fix broken type and key pings list table on document ID instead of timestamp.

* Fix broken unit tests.

* Fix broken tests and types.

* Fix broken functional test.

* Add REST endpoint for monitor states.

* Add REST route to constants file.

* Introduce io-ts typing for monitor states.

* Remove remaining GraphQL types.

* Update monitor states types to use io-ts types.

* Add state management for monitor states.

* Introduce connected monitor list component.

* Fixup runtime types for monitor states.

* Remove all remaining references to apollo graphql.

* Update URL generator function tests to use inline snapshots instead of snapshot files.

* Fix missing imports and small type issues.

* Prefer inline snapshot to object literal comparison.

* Add type check and console log to API response.

* Update README to remove graphql references.

* Fix type error.

* Make monitor list refresh when global refresh button is pressed.

* Fix broken types.

* Rename `@timestamp` field to `timestamp`.

* Change spelling of var.

* Add timestamp map for `@timestamp` field in monitor states fetcher.

* Remove need for `monito_states` fixture.

* Write test code that allows for deletion of the `monitor_states_id_filtered` fixture.

* Rewrite pagination tests to no longer rely on monitor states page fixtures.

* Skip test that is causing other functional tests to fail.

* Remove unused translations.

* Fix broken test snapshots.

* Fix stale error reporting errors.

* Remove runtime validation from REST handler.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
justinkambic added a commit that referenced this pull request Apr 17, 2020
* WIP replacing GQL with redux/rest.

* Finish implementing migration.

* Introduce new connected component for ping list.

* Replace GraphQL type with io-ts.

* Update some broken tests.

* Add test for new helper function.

* Write test snapshots.

* Migrate api tests from graphql to rest.

* Update fixtures that rely on pings.

* Move ping types to runtime_types folder with rest of io-ts files.

* Update Ping type location and imports, type checking.

* Remove reliance on fixtures for ping functional API tests.

* Fix broken unit tests.

* Fix broken types.

* Remove local state storage from parent components.

* Add functional test to cover Ping List functionality.

* Fix monitor page functional test that was broken by merge conflicts.

* Fix broken tests.

* Fix broken API test.

* Replace a test with a describe block that will pre-navigate all tests.

* Delete unused reducer keys.

* Re-introduce loading to ping list reducer.

* Inroduce code that will cause PingList to re-fetch when refresh button is pressed.

* Update expanded rows to support multiple concurrent expanded rows.

* Modify pingList reducer to have singular optional error field.

* Delete unnecessary helper code.

* Delete unused interface.

* Add runtime_type to parse getPings params, fix pagination index.

* Add dedicated monitor type to runtime_types.

* Fix broken tests.

* Fix broken tests.

* Rename '@timestamp' property to 'timestamp' on Ping type.

* Fix broken type and key pings list table on document ID instead of timestamp.

* Fix broken unit tests.

* Fix broken tests and types.

* Fix broken functional test.

* Add REST endpoint for monitor states.

* Add REST route to constants file.

* Introduce io-ts typing for monitor states.

* Remove remaining GraphQL types.

* Update monitor states types to use io-ts types.

* Add state management for monitor states.

* Introduce connected monitor list component.

* Fixup runtime types for monitor states.

* Remove all remaining references to apollo graphql.

* Update URL generator function tests to use inline snapshots instead of snapshot files.

* Fix missing imports and small type issues.

* Prefer inline snapshot to object literal comparison.

* Add type check and console log to API response.

* Update README to remove graphql references.

* Fix type error.

* Make monitor list refresh when global refresh button is pressed.

* Fix broken types.

* Rename `@timestamp` field to `timestamp`.

* Change spelling of var.

* Add timestamp map for `@timestamp` field in monitor states fetcher.

* Remove need for `monito_states` fixture.

* Write test code that allows for deletion of the `monitor_states_id_filtered` fixture.

* Rewrite pagination tests to no longer rely on monitor states page fixtures.

* Skip test that is causing other functional tests to fail.

* Remove unused translations.

* Fix broken test snapshots.

* Fix stale error reporting errors.

* Remove runtime validation from REST handler.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@justinkambic
Copy link
Contributor Author

Backported to:
7.x/7.8.0 d26b213
#63871

@justinkambic justinkambic deleted the uptime_remove-monitor-states-graphql branch April 17, 2020 19:35
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 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability technical debt Improvement of the software architecture and operational architecture v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Uptime] Convert remaining GQL endpoints to REST
4 participants