Skip to content

0008 - Add Ajax error handling ADR#9

Closed
atomiix wants to merge 2 commits intoPrestaShop:masterfrom
atomiix:ajax-error-handling
Closed

0008 - Add Ajax error handling ADR#9
atomiix wants to merge 2 commits intoPrestaShop:masterfrom
atomiix:ajax-error-handling

Conversation

@atomiix
Copy link
Copy Markdown
Member

@atomiix atomiix commented Mar 23, 2020

No description provided.

Comment on lines +14 to +20
When handled, [growl](http://ksylvest.github.io/jquery-growl/) is often used this way:

```javascript
if (response.responseJSON && response.responseJSON.message) {
$.growl.error({message: response.responseJSON.message});
}
```
Copy link
Copy Markdown

@NeOMakinG NeOMakinG Mar 23, 2020

Choose a reason for hiding this comment

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

What we could do is adding an i18n message on errors not returning JSON, if this is possible


## Context

We need to define a way to handle Ajax calls errors.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't need to define a way to handle ajax calls errors. We need to define ajax calls errors 😅
Since we use jQuery: have a look at https://api.jquery.com/ajaxerror/
With this you can handle all ajax errors by a generic check.

if response has message:
  message = json message;
else:
  message = generic message;
fi

if growl:
  use growl(message)
else:
  console.log?
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That should be done on every calls (or at least on every "main" actions to avoid spamming since sometimes an actions has more than 3 calls)

@matks
Copy link
Copy Markdown
Contributor

matks commented Jul 29, 2020

It looks like this ADR is kindof stale. We need to simply choose one solution and start using it everywhere.

I suggest @eternoendless chooses the solution to use and then we can complete merge this ADR.

@eternoendless eternoendless changed the title Add Ajax error handling ADR 0008 - Add Ajax error handling ADR Aug 21, 2020
@matks
Copy link
Copy Markdown
Contributor

matks commented Dec 17, 2021

@atomiix if you are OK, maybe we should close this? we were not able to manage this topic until a vote

@atomiix atomiix closed this Dec 17, 2021
@atomiix atomiix deleted the ajax-error-handling branch December 17, 2021 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants