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

Bugfix: Fix TS to avoid assigning filters to undefined #40249

Merged
merged 2 commits into from
Jul 3, 2019
Merged

Bugfix: Fix TS to avoid assigning filters to undefined #40249

merged 2 commits into from
Jul 3, 2019

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Jul 3, 2019 •
edited
Loading

Summary

Angular is responsible for initializing appState asynchronously.
This means that filter_state_manager might sometimes get a null \ undefined AppState.
The class's TypeScript didn't reflect this and had a place that assigned filters to AppState, without checking if it's defined.

Should resolve #40220

Checklist

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

For maintainers

@lizozom lizozom added bug Fixes for quality problems that affect the customer experience v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.3.0 labels Jul 3, 2019
@lizozom lizozom requested review from spalger and ppisljar July 3, 2019 11:23
@lizozom lizozom self-assigned this Jul 3, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

ppisljar
ppisljar approved these changes Jul 3, 2019
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@spalger spalger added the v7.4.0 label Jul 3, 2019
@lizozom lizozom force-pushed the bug/filter-manager/assign-filters-to-undefined branch from eebd1a2 to 4afca7e Compare July 3, 2019 15:32
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@lizozom lizozom merged commit aef6223 into elastic:master Jul 3, 2019
@lizozom lizozom mentioned this pull request Jul 3, 2019
Merged
lizozom pushed a commit to lizozom/kibana that referenced this pull request Jul 3, 2019
Bugfix: Fix TS to avoid assigning filters to undefined (elastic#40249)
5e2ab1a 
* Define getAppState TS correctly, to avoid filter assignment into an undefined state

* Added tests for the case where appState is undefined
spalger pushed a commit that referenced this pull request Jul 4, 2019
Bugfix: Fix TS to avoid assigning filters to undefined (#40249) (#40283)
a491601 
* Define getAppState TS correctly, to avoid filter assignment into an undefined state

* Added tests for the case where appState is undefined
@spalger
Copy link
Contributor

spalger commented Jul 4, 2019 •
edited
Loading

7.x/7.4: a491601
7.3: 0076658

spalger pushed a commit to spalger/kibana that referenced this pull request Jul 12, 2019
Bugfix: Fix TS to avoid assigning filters to undefined (elastic#40249)
6bcaafd 
* Define getAppState TS correctly, to avoid filter assignment into an undefined state

* Added tests for the case where appState is undefined
This was referenced Jul 12, 2019
Merged
Closed
@lizozom lizozom deleted the bug/filter-manager/assign-filters-to-undefined branch November 14, 2019 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppisljar ppisljar ppisljar approved these changes

@spalger spalger Awaiting requested review from spalger

Assignees

@lizozom lizozom

Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v7.3.0 v7.4.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Failing test: Browser Unit Tests."before each" hook."before each" hook
4 participants
@lizozom @elasticmachine @spalger @ppisljar