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

Document how to override notifications messages #1193

Closed
dimitrovs opened this issue Nov 1, 2017 · 20 comments
Closed

Document how to override notifications messages #1193

dimitrovs opened this issue Nov 1, 2017 · 20 comments

Comments

@dimitrovs
Copy link
Contributor

This has already been asked before: #180 but the answer provided was to basically not use the Admin component. I believe this could be a useful feature. Can we discuss some alternative approaches to provide an easier to use and user friendly server messaging?

I propose to add support for a mapping file, similar to the way Translation messages are implemented, such that every error (or even every response) received from the server will be looked up in this server messages mapping file by the Admin on rest saga.

@fzaninotto
Copy link
Member

And do what? That means baking in the saga some predefined behaviors. Which ones?

@dimitrovs
Copy link
Contributor Author

The saga already shows the error message as a notification. What I am proposing is to support a map of resource + error code to a custom error message instead of the default one. This will allow us to give more meaningful notification messages instead of generic "Internal Server Error".

@fzaninotto
Copy link
Member

I think the solution for that is that you provide a custom saga. But we need to provide a way to disable aor's default error messages. So marking this as an enhancement.

@fzaninotto fzaninotto changed the title Feature Request: Server Message mapping Feature Request: Override Notification Messages Saga Nov 3, 2017
@fzaninotto fzaninotto changed the title Feature Request: Override Notification Messages Saga Override Notification Messages Saga Nov 3, 2017
@dimitrovs
Copy link
Contributor Author

For anybody else who needs this: if you are in control of your backend you can just return {message:'Text'} and Admin-on-rest will show the message text in the notification.

@alauper
Copy link
Contributor

alauper commented Jan 19, 2018

@dimitrovs thaaaaank you. I was scratching my head for hours trying to figure out how to override the generic error.

@djhi
Copy link
Contributor

djhi commented Feb 16, 2018

@dimitrovs, can we close this then ?

@djhi djhi changed the title Override Notification Messages Saga Document how to override notifications messages Feb 20, 2018
@djhi
Copy link
Contributor

djhi commented Feb 20, 2018

This is documented actually: See https://marmelab.com/admin-on-rest/RestClients.html#writing-your-own-rest-client (at the end, Error format).

How can we make this easier to find ?

@joe-at-startupmedia
Copy link

Just stumbled into this issue today. It's documented in this issue as well.
https://github.com/marmelab/admin-on-rest/issues/843

@MROFerreiro
Copy link

Even with the possibility to show a custom error message, translation of it is not possible. React-Admin assumes the message coming in {error.message} from server. If the server message is "Couldn't find resource" it's that message that will be shown in notification.

I think it would be better to error message be based on a code number/string. Something like, if server returns error object like the following {error: {code: 405}} it goes to dictionary.error[*code from server error*] and show that message in notification. If server has no code number/string show a default error message.

@djhi
Copy link
Contributor

djhi commented Feb 8, 2019

Yes we can do a better job on documenting this. In the mean time, you should know that the notifications will always try to translate their texts (https://github.com/marmelab/react-admin/blob/master/packages/ra-ui-materialui/src/layout/Notification.js#L83).

So, in order to translate your API errors, just make the error's message a translation key.

@wmwart
Copy link

wmwart commented Apr 3, 2019

@djhi , how do you translate your Errors?

@djhi
Copy link
Contributor

djhi commented Apr 3, 2019

Please ask this on StackOverflow

@wmwart
Copy link

wmwart commented Apr 4, 2019

@imekinox
Copy link

I just posted an answer on your stackoverflow question @wmwart

@jesseshieh
Copy link
Contributor

As @fzaninotto mentioned in a comment above, are there any plans to support disabling notifications? Should I open a new issue?

I see that overriding the error message is possible, but in my case, I'm looking for a way to disable the notification entirely. I posted a stackoverflow question and looked at the source code, but I can't seem to find a way to do this. Anybody know a way to disable the notification?

The relevant part of the source I found is the notification saga which always shows either the error, the error.message or the notification.body with no option to skip or ignore. Thanks for an awesome framework, btw!

@fzaninotto
Copy link
Member

No, we have no such plans. But you can achieve it in userland by overriding the Notification component in the layout.

@jesseshieh
Copy link
Contributor

Thanks @fzaninotto! I'll give that a try.

@vikimaf
Copy link

vikimaf commented Dec 18, 2019

{message: 'Text'}
Can you give an example of what the error from the server should look like?
2019-12-18_13-54

@JulienMattiussi JulienMattiussi self-assigned this Mar 5, 2020
@JulienMattiussi
Copy link
Contributor

JulienMattiussi commented Mar 5, 2020

@pavel-nano Can you give more detail of the situation illustrated by your screenshot ?
Like which kind of button have been clicked ? is it custom or not ? maybe the type of APi call and the error code ?

I tried to reproduce your notifcation "failing" message but without succes.
The network preview displayed is exactely what you need to set a custom notification from the API. And for now, I don't understand why it didn't.

Note : Only way of possible bug I founded is in the Exporter if your csv export fails.

@JulienMattiussi
Copy link
Contributor

As we have no more information to progress on this issue since a long, I close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests