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

[Workplace Search] Fix bug where error was behind modal stuck in loading state #104360

Merged
merged 2 commits into from
Jul 7, 2021
Merged

[Workplace Search] Fix bug where error was behind modal stuck in loading state #104360

merged 2 commits into from
Jul 7, 2021

Conversation

scottybollinger
Copy link
Contributor

fixes https://github.com/elastic/workplace-search-team/issues/1831

Summary

This PR accomplishes 2 things:

  1. It fixes a bug where the error was behind modal stuck in loading state.

Before
schema-bug

After
after

  1. It also fixes an issue from a previous PR where the wrong property was being used for the error message.

The issue is that this is the shape of the server error response:

{
    "statusCode": 400,
    "error": "Bad Request",
    "message": "Fields cannot be blank.",
    "attributes": {
        "errors": ["Fields cannot be blank."]
    }
}

I mistakingly used the message prop, which is a string, when the UI expects an array of strings, which attributes.errors is.

Checklist

@scottybollinger
Fix an issue from previous PR
3295dc4 
In elastic#104024, the error handling incorrectly used the `message` property on the response, when it should have been the attributes.errors array.
@scottybollinger
Use inline error for duplicate name
4e7d3ba
@scottybollinger scottybollinger requested a review from a team July 5, 2021 22:09
@scottybollinger scottybollinger added auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.14.0 labels Jul 5, 2021
@scottybollinger scottybollinger enabled auto-merge (squash) July 5, 2021 22:09
@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 2.1MB 2.1MB +113.0B

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

yakhinvadim
yakhinvadim approved these changes Jul 7, 2021
View reviewed changes
Copy link
Contributor

@yakhinvadim yakhinvadim left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@scottybollinger scottybollinger merged commit 874882e into elastic:master Jul 7, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 7, 2021
@scottybollinger @kibanamachine
[Workplace Search] Fix bug where error was behind modal stuck in load…
7593064 
…ing state (elastic#104360)

* Fix an issue from previous PR

In elastic#104024, the error handling incorrectly used the `message` property on the response, when it should have been the attributes.errors array.

* Use inline error for duplicate name
@kibanamachine kibanamachine mentioned this pull request Jul 7, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.14

This backport PR will be merged automatically after passing CI.

darnautov pushed a commit to darnautov/kibana that referenced this pull request Jul 7, 2021
@scottybollinger @darnautov
[Workplace Search] Fix bug where error was behind modal stuck in load…
0c2198d 
…ing state (elastic#104360)

* Fix an issue from previous PR

In elastic#104024, the error handling incorrectly used the `message` property on the response, when it should have been the attributes.errors array.

* Use inline error for duplicate name
kibanamachine added a commit that referenced this pull request Jul 7, 2021
@kibanamachine @scottybollinger
[Workplace Search] Fix bug where error was behind modal stuck in load…
701fb53 
…ing state (#104360) (#104636)

* Fix an issue from previous PR

In #104024, the error handling incorrectly used the `message` property on the response, when it should have been the attributes.errors array.

* Use inline error for duplicate name

Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
@tylersmalley
Copy link
Contributor

I see this PR was merged into master and 7.14. Since 7.14 is past feature freeze it's no longer being tracked in the 7.x branch. I assume you intended to include this PR in 7.15+. If so, can you please add the 7.15 label and ensure it's backported.

@spalger spalger added the v7.15.0 label Jul 7, 2021
@spalger
Copy link
Contributor

spalger commented Jul 7, 2021

Backporting this as part of a dependent backport in #104702

spalger pushed a commit to spalger/kibana that referenced this pull request Jul 7, 2021
@scottybollinger @spalger
[Workplace Search] Fix bug where error was behind modal stuck in load…
18834cf 
…ing state (elastic#104360)

* Fix an issue from previous PR

In elastic#104024, the error handling incorrectly used the `message` property on the response, when it should have been the attributes.errors array.

* Use inline error for duplicate name
@spalger spalger mentioned this pull request Jul 7, 2021
Merged
spalger pushed a commit that referenced this pull request Jul 7, 2021
@scottybollinger @byronhulcher @kibanamachine
[7.x] [Enterprise Search] Fix bug in Add Schema modal (#104024) (#104702
de121c2 
)

* [Enterprise Search] Fix bug in Add Schema modal (#104024)

* [Enterprise Search] Fix bug in Add Schema modal

This PR fixes a bug where an error passed from the server was not rendering and was causing the UI to hang. The problem is that the `body` property was missed in the error object.

* Use default message when error response for actions.setServerField has no message

* i18n-ize default error message

* Remove TODO

Co-authored-by: Byron Hulcher <byronhulcher@gmail.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* [Workplace Search] Fix bug where error was behind modal stuck in loading state (#104360)

* Fix an issue from previous PR

In #104024, the error handling incorrectly used the `message` property on the response, when it should have been the attributes.errors array.

* Use inline error for duplicate name

Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
Co-authored-by: Byron Hulcher <byronhulcher@gmail.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@scottybollinger scottybollinger deleted the scottybollinger/schema-dup-error branch March 15, 2022 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yakhinvadim yakhinvadim yakhinvadim 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.14.0 v7.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@scottybollinger @kibanamachine @tylersmalley @spalger @yakhinvadim