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

use error response from backend to display correct error message #27

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SuyashSalampuria
Copy link
Member

Fixes #24

Copy link
Member

@algomaster99 algomaster99 left a comment

Choose a reason for hiding this comment

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

I can't notice any difference on my machine. Can you send a screencast?

@SuyashSalampuria
Copy link
Member Author

I can't notice any difference on my machine. Can you send a screencast?

image

@algomaster99
Copy link
Member

algomaster99 commented Jul 16, 2020

Django has a feature to generate an object of errors like this
Screenshot from 2020-07-17 02-45-04

If you want to read how this happens, refer to Django's full_clean method. Try to check out its source code 😄

You can just get them and save it to a redux store (already happening) and render them like this:
Screenshot from 2020-07-17 02-43-30

This seems more convenient to me because:

  1. We don't have to decide our own error messages on the frontend.
  2. Multiple states are not needed to maintaining the string for an error message.
  3. If we implement this successfully, we will automatically ensure on the back-end that we don't raise a 500: Internal Server Error.

const errors = errorMessage[field];
errors.forEach(err =>
(
<p> {field} : {err} </p>
Copy link
Member Author

Choose a reason for hiding this comment

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

@algomaster99 I see the control is reaching here but this line is not returned to the screen.

Copy link
Member

Choose a reason for hiding this comment

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

@SuyashSalampuria forEach always returns undefined, which is the reason you cannot see any <p> element. You can use map and this will be resolved. :)

Copy link
Member

@algomaster99 algomaster99 left a comment

Choose a reason for hiding this comment

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

There was some errors with your submission

I think the error is still very equivocal. No one can figure out why the error came at the first go.

Anyway, You don't need to hardcode the error message on the frontend. That's what I am saying. You can edit the backend to throw such error messages. Did you refer to full_clean? If not, please do it. I also suggest referring to omniport-frontend-faculty-profile's code to see how it's done.

@algomaster99
Copy link
Member

@SuyashSalampuria sorry I really forgot about this
But anyway, whenever I am updating a profile, the error isn't thrown.

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.

Show appropriate message for wrong handle name
3 participants