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

[Discover] Hide time picker when an indexpattern without timefield is selected #62134

Merged
merged 13 commits into from
Apr 9, 2020
Merged

[Discover] Hide time picker when an indexpattern without timefield is selected #62134

merged 13 commits into from
Apr 9, 2020

Conversation

kertal
Copy link
Member

@kertal kertal commented Apr 1, 2020 •
edited
Loading

Summary

After selecting an indexpattern without timefield, the timepicker shouldn't be displayed. This PR implements this expected behavior

Fixes: #62417

Checklist

@kertal kertal requested a review from a team April 1, 2020 11:20
@kertal kertal added Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v7.7.0 :KibanaApp/fix-it-week labels Apr 1, 2020
flash1293
flash1293 approved these changes Apr 2, 2020
View reviewed changes
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested in Chrome and works, LGTM 👍
Couldn't find enableTimeRangeSelector anywhere else in the discover code, is this a leftover of the migration?

Also, while testing for this PR I noticed that the error handling is broken when the last used index pattern doesn't exist (unrelated to this PR) - will create a separate issue for that.

@kertal
Copy link
Member Author

kertal commented Apr 2, 2020

@flash1293 yes, seems to be a leftover, I've added a functional test for that

@kertal kertal requested a review from lizozom April 2, 2020 09:37
@kertal kertal self-assigned this Apr 2, 2020
flash1293
flash1293 reviewed Apr 2, 2020
View reviewed changes
test/functional/apps/discover/_indexpattern_without_timefield.js Outdated
await esArchiver.unload('index_pattern_without_timefield');
});

it('should not display a timepicker', async function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: When we have this test we should only check whether it's shown correctly when there is a timefield

Copy link
Member Author

Choose a reason for hiding this comment

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

you mean: we should also check if it's displayed correctly when there is a timefield, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's what I meant

Copy link
Member Author

Choose a reason for hiding this comment

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

totally, agree, will do!

Copy link
Member Author

Choose a reason for hiding this comment

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

implemented this test, and finally Jenkins is green :)

@kertal
Copy link
Member Author

kertal commented Apr 6, 2020

@elasticmachine merge upstream

@kertal
Copy link
Member Author

kertal commented Apr 6, 2020

@elasticmachine merge upstream

@kertal
Copy link
Member Author

kertal commented Apr 7, 2020

@elasticmachine merge upstream

@kertal
Copy link
Member Author

kertal commented Apr 7, 2020

@elasticmachine merge upstream

elasticmachine and others added 4 commits April 7, 2020 16:56
@elasticmachine
Merge branch 'master' into kertal-pr-2020-04-01-fix-discover-index-pa…
56705de 
…ttern-without-timefield
@kertal
Improve functional tests
cf974a9
@kertal
Merge branch 'kertal-pr-2020-04-01-fix-discover-index-pattern-without…
1b17036 
…-timefield' of github.com:kertal/kibana into kertal-pr-2020-04-01-fix-discover-index-pattern-without-timefield
@kertal
Restore accidentally deleted files
dbb82c3
@kertal
Copy link
Member Author

kertal commented Apr 9, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@kertal kertal merged commit 2a1c8d8 into elastic:master Apr 9, 2020
@kertal kertal mentioned this pull request Apr 9, 2020
Merged
kertal added a commit to kertal/kibana that referenced this pull request Apr 9, 2020
@kertal
[Discover] Hide time picker when an indexpattern without timefield is…
9ced784 
… selected (elastic#62134)

* Assign valid value whether the timepicker should be displayed

* Add functional tests
@kertal kertal mentioned this pull request Apr 9, 2020
Merged
kertal added a commit to kertal/kibana that referenced this pull request Apr 9, 2020
@kertal
[Discover] Hide time picker when an indexpattern without timefield is…
517a263 
… selected (elastic#62134)

* Assign valid value whether the timepicker should be displayed

* Add functional tests
kertal added a commit that referenced this pull request Apr 9, 2020
@kertal
[Discover] Hide time picker when an indexpattern without timefield is…
4aef4c7 
… selected (#62134) (#63106)

* Assign valid value whether the timepicker should be displayed

* Add functional tests
kertal added a commit that referenced this pull request Apr 9, 2020
@kertal
[Discover] Hide time picker when an indexpattern without timefield is…
211d28a 
… selected (#62134) (#63107)

* Assign valid value whether the timepicker should be displayed

* Add functional tests
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 9, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into es_ui_shared/…
bf57be0 
…chore/put-all-xjson-together

* 'master' of github.com:elastic/kibana:
  [EPM] Update UI copy to use `integration` (elastic#63077)
  [NP] Inline buildPointSeriesData and buildHierarchicalData dependencies (elastic#61575)
  [Maps] create NOT EXISTS filter for tooltip property with no value (elastic#62849)
  [Endpoint] Add link to Logs UI to the Host Details view (elastic#62852)
  [UI COPY] Fixes typo in max_shingle_size for search_as_you_type (elastic#63071)
  [APM] docs: add alerting examples for APM (elastic#62864)
  [EPM] Change PACKAGES_SAVED_OBJECT_TYPE id (elastic#62818)
  docs: fix rendering of bulleted list (elastic#62855)
  Exposed AddMessageVariables as separate component (elastic#63007)
  Add Data - Adding cloud reset password link to cloud instructions (elastic#62835)
  [ML] DF Analytics:  update memory estimate after adding exclude fields (elastic#62850)
  [Table Vis] Fix visualization overflow (elastic#62630)
  [Endpoint][EPM] Endpoint depending on ingest manager to initialize (elastic#62871)
  [Remote clusters] Fix flaky jest tests (elastic#58768)
  [Discover] Hide time picker when an indexpattern without timefield is selected (elastic#62134)
  Move search source parsing and serializing to data (elastic#59919)
  [ML] Functional tests - stabilize typing in mml input (elastic#63091)
  [data.search.aggs]: Clean up TimeBuckets implementation (elastic#62123)
  [ML] Functional transform tests - stabilize source selection (elastic#63087)
  add embed flag to saved object url as well (elastic#62926)

# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/es_index.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flash1293 flash1293 flash1293 approved these changes

@lizozom lizozom Awaiting requested review from lizozom

Assignees

@kertal kertal

Labels
Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Discover] Timepicker is displayed using an indexpattern without timefield
4 participants
@kertal @kibanamachine @flash1293 @elasticmachine