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.

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

Fixed "See full error" on error toast #4119

Merged
merged 4 commits into from
Apr 29, 2022

Conversation

Machi3mfl
Copy link
Member

Hi Team,
This PR resolves and error when user click on "See full error" button on error toast.
Closes #4030

@Machi3mfl Machi3mfl requested a review from a team April 28, 2022 19:38
@Desvelao Desvelao linked an issue Apr 29, 2022 that may be closed by this pull request
* @returns error
*/
static customErroMessage(error, message){
Copy link
Member

@Desvelao Desvelao Apr 29, 2022

Choose a reason for hiding this comment

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

issue: if the method is static to the class, this is not accessible by the class instance.

suggestion: Use the class name instead of this to use the method.

Explanation:

// Define a class
class MyClass{
  // Constructor of the class instances
  constructor(){
    // Initialize the class instance. `this` keyword does reference to the class instance
  }
  // Define a static method for the class
  static staticMethod(){
    // This method is static of the class. This can't be accesed by the class instance.
    // Use:
    // MyClass.staticMethod();
  }
  
  // Define a method accesible by the class instance throught the prototype chain
  methodAccesibleByTheClassInstance(){
    // This method is in the prototype chain of the class instance, so this is accesible.
    // Use:
    // this.methodAccesibleByTheClassInstance();
  }
}

nitpick: The method name has a typo Erro instead of Error.

@Desvelao
Copy link
Member

Desvelao commented Apr 29, 2022

thought: I am not sure that the proposed solution is good. It modified the thrown error in the WzRequest service, so the See the full error button in theory, should work. But, the problem with the button could appear if the error has no the expected interface.

As @Machi3mfl mentioned in this comment #4030 (comment) for the long term solution, we should adopt the same interface for the errors. This will need an analisys of the solution and the changes could break some things.

My proposed solution is sure the parameter used by the service that displays the toast, has the expected interface, regardless if the error is a string or an object valid to use in the toast. With this solution, we secure the button should work with different error interfaces.

@Desvelao Desvelao added the type/bug Bug issue label Apr 29, 2022
@Mayons
Copy link

Mayons commented Apr 29, 2022

Test OK !
Error Toast 1
Error toast2

@Mayons Mayons self-requested a review April 29, 2022 19:05
@github-actions
Copy link
Contributor

Jest Test Coverage % values
Statements 4.04% ( 1477 / 36528 )
Branches 1.62% ( 460 / 28469 )
Functions 2.99% ( 267 / 8928 )
Lines 4.09% ( 1429 / 34924 )

@mauceballos
Copy link
Contributor

Test kibana 7.10 OK

toast-error

@mauceballos mauceballos self-requested a review April 29, 2022 19:38
@Machi3mfl Machi3mfl merged commit 343d2ee into 4.3-7.10 Apr 29, 2022
@Machi3mfl Machi3mfl deleted the bugfix/4030-see-full-error-toast branch April 29, 2022 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Bug issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

See full error toast button exception
4 participants