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

[App Search] Bugfix: Remove row from engine table after engine is deleted #94823

Merged
merged 3 commits into from
Mar 23, 2021
Merged

[App Search] Bugfix: Remove row from engine table after engine is deleted #94823

merged 3 commits into from
Mar 23, 2021

Conversation

byronhulcher
Copy link
Contributor

@byronhulcher byronhulcher commented Mar 17, 2021 •
edited by cee-chen
Loading

Summary

Introduced a silly bug with my recent changes. I misunderstood how typescript enums work, and assuming that by default they used the property name as their value by default instead of an auto-incrementing integer. So while our code was fine with everything internally being integers, when it started getting data from our API, it was failing to refresh the data and remove the row. Sorry I missed this.

QA

  • Boot up Kibana
  • Goto the Engines Overview
  • Add a few new engines
  • Add a few meta-engines
  • Delete an engine, you should see a success message and the row for the engine should be removed from the table
  • Delete a meta-engine, you should see a success message and the row for the meta-engine should be removed from the table

Checklist

@byronhulcher byronhulcher added release_note:skip Skip the PR/issue when compiling release notes v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 17, 2021
@byronhulcher byronhulcher requested a review from a team March 17, 2021 15:25
@byronhulcher byronhulcher changed the title [App Search] Remove row from engine table after engine is deleted [App Search] Bugfix: Remove row from engine table after engine is deleted Mar 17, 2021
@kibanamachine
Copy link
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

History

  • 💔 Build #113678 failed c082ab1aa0fb4254c565a7d2847a394aa6c073be

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

@cee-chen
Copy link
Contributor

cee-chen commented Mar 17, 2021 •
edited
Loading

I checked out this branch and now I don't see the delete icon/actions column at all on the engines table. Am I missing something?

cee-chen
cee-chen approved these changes Mar 18, 2021
View reviewed changes
Copy link
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Ah I grabbed latest again and it's working for me now locally, sorry! QA looks great, thanks Byron. 🙏

As a total aside: while we're here, can also we add a color: 'danger' to the trash icon to match our other tables? No worries if you'd rather handle separately (or if you want I can grab it), feel free to merge this as-is if so!

@cee-chen
Copy link
Contributor

@elasticmachine merge upstream

@byronhulcher
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 1.9MB 1.9MB -45.0B

History

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

@byronhulcher byronhulcher merged commit 4dec429 into elastic:master Mar 23, 2021
@kibanamachine kibanamachine mentioned this pull request Mar 23, 2021
Merged
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Mar 23, 2021
@byronhulcher @kibanamachine
Use string values for EngineTypes instead of int (elastic#94823)
a34ad4b 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

✅ 7.x / #95165

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Mar 23, 2021
@kibanamachine @byronhulcher
Use string values for EngineTypes instead of int (#94823) (#95165)
bc0e4c7 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Byron Hulcher <byronhulcher@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cee-chen cee-chen cee-chen approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.13.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@byronhulcher @kibanamachine @cee-chen