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.

Sign up for GitHub

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

Jump to bottom

[Uptime] Added date range filter into expanded list query #52609

Merged
merged 20 commits into from
Jan 7, 2020
Merged

[Uptime] Added date range filter into expanded list query #52609

merged 20 commits into from
Jan 7, 2020

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Dec 10, 2019 •
edited
Loading

Summary

Fixes: https://github.com/elastic/uptime/issues/124

Expanded row in Monitor list wasn't following filters in ES query, filters like date range are being added to query in this PR.

Testing

Select date range filter and see how data reflects in expanded list.

Checklist

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

For maintainers

@shahzad31 shahzad31 self-assigned this Dec 10, 2019
@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.6.0 v8.0.0 labels Dec 10, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@shahzad31 shahzad31 added the WIP Work in progress label Dec 10, 2019
@andrewvc andrewvc mentioned this pull request Dec 10, 2019
Closed
7 tasks
@shahzad31 shahzad31 removed the WIP Work in progress label Dec 11, 2019
@shahzad31
Copy link
Contributor Author

@justinkambic this is ready for review

@shahzad31 shahzad31 changed the title [Uptime] added filters into expanded list query [Uptime] added location filter into expanded list query Dec 11, 2019
justinkambic
justinkambic reviewed Dec 12, 2019
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

I'm getting a React warning in my browser console; there's probably some effect that needs to be removed from the main render of FilterGroup.

image

x-pack/legacy/plugins/uptime/public/components/functional/filter_group/filter_group.tsx Outdated Show resolved Hide resolved
@shahzad31
Copy link
Contributor Author

I'm getting a React warning in my browser console; there's probably some effect that needs to be removed from the main render of FilterGroup.

image

@justinkambic i took care of that error in console. as well as your code suggestion.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

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

justinkambic
justinkambic reviewed Dec 19, 2019
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm having a little trouble discerning how I can ensure that the change is working as designed. I've applied a filter on location that impacts other data (like the overall count series at the top of the page), but I'm still seeing both locations. Am I looking for the wrong kind of behavior?

image

x-pack/legacy/plugins/uptime/public/state/actions/ui.ts Show resolved Hide resolved
x-pack/legacy/plugins/uptime/public/components/functional/filter_group/index.ts Outdated Show resolved Hide resolved
@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@shahzad31 shahzad31 force-pushed the fix/monitor-list-expaned-row-should-follow-filters branch from 3b8e88b to dc00a2c Compare January 3, 2020 16:47
@shahzad31
Copy link
Contributor Author

I'm having a little trouble discerning how I can ensure that the change is working as designed. I've applied a filter on location that impacts other data (like the overall count series at the top of the page), but I'm still seeing both locations. Am I looking for the wrong kind of behavior?

image

So right now it's only valid for Most recent error, you are right, it doesn't filter location checks, since that data is coming from Pagination API.

@shahzad31 shahzad31 changed the title [Uptime] added location filter into expanded list query [Uptime] Added date range filter into expanded list query Jan 6, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

justinkambic
justinkambic approved these changes Jan 7, 2020
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

LGTM - tested locally, date range is impacting the locations displayed as I'd expect, and code looks good.

@shahzad31 shahzad31 merged commit 334dff3 into elastic:master Jan 7, 2020
@shahzad31 shahzad31 deleted the fix/monitor-list-expaned-row-should-follow-filters branch January 7, 2020 17:47
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 7, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into feature/conso…
792771e 
…le-saved-objects

* 'master' of github.com:elastic/kibana: (86 commits)
  [Uptime] Added date range filter into expanded list query (elastic#52609)
  [SIEM] Add react/display-name eslint rule (elastic#53107)
  [SIEM] Enable eslint prefer-template rule (elastic#53983)
  Elasticsearch snapshots automation (elastic#53706)
  [SIEM] Enable eslint react/no-children-prop (elastic#53985)
  [DOCS][Reporting] Adds info about using a custom reporting index (elastic#54052)
  Changing background color to align with EUI color (elastic#54060)
  Fix linting issues (elastic#54068)
  NP Migration: Move doc views registry and existing doc views into discover plugin (elastic#53465)
  [ML] DF Analytics job creation: Add 'excludes' input field to form (elastic#53856)
  EMT-issue-65: add endpoint list api (elastic#53861)
  [SIEM] Fix doubled drag handles in Timeline (elastic#52679)
  Functional tests: refactor visualize_page (elastic#53845)
  [Dashboard] Redesign empty screen in readonly mode (elastic#54073)
  [ML] Support search for partitions on Single Metric Viewer (elastic#53879)
  update apm index pattern (elastic#54095)
  Add data ui for envoyproxy Metricbeat Module (elastic#53476)
  [ML] persist the brush when expanded to full width (elastic#54020)
  Skip failing test (elastic#54100)
  [APM] Show errors on the timeline instead of under the transaction (elastic#53756)
  ...

# Conflicts:
#	src/legacy/core_plugins/console/public/np_ready/application/contexts/services_context.tsx
#	src/legacy/core_plugins/console/public/np_ready/application/index.tsx
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 7, 2020
@jloleysens
Merge branch 'feature/console-saved-objects' of github.com:jloleysens…
760d29f 
…/kibana into feature/console-saved-objects

* 'feature/console-saved-objects' of github.com:jloleysens/kibana: (86 commits)
  [Uptime] Added date range filter into expanded list query (elastic#52609)
  [SIEM] Add react/display-name eslint rule (elastic#53107)
  [SIEM] Enable eslint prefer-template rule (elastic#53983)
  Elasticsearch snapshots automation (elastic#53706)
  [SIEM] Enable eslint react/no-children-prop (elastic#53985)
  [DOCS][Reporting] Adds info about using a custom reporting index (elastic#54052)
  Changing background color to align with EUI color (elastic#54060)
  Fix linting issues (elastic#54068)
  NP Migration: Move doc views registry and existing doc views into discover plugin (elastic#53465)
  [ML] DF Analytics job creation: Add 'excludes' input field to form (elastic#53856)
  EMT-issue-65: add endpoint list api (elastic#53861)
  [SIEM] Fix doubled drag handles in Timeline (elastic#52679)
  Functional tests: refactor visualize_page (elastic#53845)
  [Dashboard] Redesign empty screen in readonly mode (elastic#54073)
  [ML] Support search for partitions on Single Metric Viewer (elastic#53879)
  update apm index pattern (elastic#54095)
  Add data ui for envoyproxy Metricbeat Module (elastic#53476)
  [ML] persist the brush when expanded to full width (elastic#54020)
  Skip failing test (elastic#54100)
  [APM] Show errors on the timeline instead of under the transaction (elastic#53756)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 8, 2020
@gmmorris
Merge branch 'master' into task-manager/schema-config
9e0c028 
* master: (55 commits)
  [ui/public/utils] Copy rarely used items to where they are consumed (elastic#53819)
  set AppArch team as an owner of the search endpoints (elastic#54131)
  Don't expose Elasticsearch client as Observable (elastic#53824)
  [SIEM] Cleanup unnecessary use of enzyme-to-json (elastic#53980)
  fix ui exports doc (elastic#54138)
  change markdown element title (elastic#54194)
  [Logs UI] Refactor log position to hooks (elastic#53540)
  [SIEM] Implement NP Plugin Setup (elastic#54030)
  [DOCS] Updates ML links (elastic#53613)
  sort renovate packages in config
  Spaces - fix flakey api tests (elastic#54154)
  Remove dependency that was causing effect to re-execute infinitely. (elastic#54160)
  [dev/run] expose unexpected flags as more than just names (elastic#54080)
  [DOCS] Moves index pattern doc to Discover (elastic#53347)
  [SIEM] Cleanup React imports (elastic#53981)
  Update eslint related packages (elastic#54107)
  [Uptime] Added date range filter into expanded list query (elastic#52609)
  [SIEM] Add react/display-name eslint rule (elastic#53107)
  [SIEM] Enable eslint prefer-template rule (elastic#53983)
  Elasticsearch snapshots automation (elastic#53706)
  ...
@shahzad31 shahzad31 mentioned this pull request Jan 10, 2020
Merged
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Jan 10, 2020
@shahzad31
[Uptime] Added date range filter into expanded list query (elastic#52609
323ee7b 
)

* added filters into expanded list query

* update filters

* update query

* update snap

* update tests

* update filters

* update test

* remove side effect

* ignore typcehck

* update to remove location filter from query

* update filter groups

* remove code

* update test
shahzad31 added a commit that referenced this pull request Jan 10, 2020
@shahzad31
[Uptime] Added date range filter into expanded list query (#52609) (#…
921e933 
…54453)

* added filters into expanded list query

* update filters

* update query

* update snap

* update tests

* update filters

* update test

* remove side effect

* ignore typcehck

* update to remove location filter from query

* update filter groups

* remove code

* update test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinkambic justinkambic justinkambic approved these changes

Assignees

@shahzad31 shahzad31

Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.6.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Uptime] Expanded list row should follow filters
4 participants
@shahzad31 @elasticmachine @kibanamachine @justinkambic