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

Error messages during organization creation are not cleared #1220

Closed
amplifi opened this issue Mar 7, 2017 · 11 comments
Closed

Error messages during organization creation are not cleared #1220

amplifi opened this issue Mar 7, 2017 · 11 comments

Comments

@amplifi
Copy link
Contributor

amplifi commented Mar 7, 2017

Steps to reproduce the error

Attempt to create an organization with an empty name. It will show you the appropriate empty name error message. Then enter an organization name that already exists.

Actual behavior

Both error messages appear simultaneously. (The same error occurs no matter which order you trigger them in succession.)

screen shot 2017-03-07 at 9 05 42 pm

Expected behavior

Any visible error message should be cleared upon clicking "Save", so that only the applicable error message appears after a new error is found. Also "Name" in the "Organization with this Name already exists" error message should be lowercase.

khantaalaman added a commit to khantaalaman/cadasta-platform that referenced this issue Mar 8, 2017
Bugfix/Cadasta#1220: Clear error messages during create organization.
@khantaalaman
Copy link
Contributor

The bug has been fixed.

Supporting GIF:

optimised

PS: Sorry for buggy GIFs.

@oliverroick
Copy link
Member

Just curious, is that something that happens in other forms as well? Say the project form for example? If yes, we need a generic solution that works across all forms and all fields.

@khantaalaman
Copy link
Contributor

Cool @oliverroick , I will look into this and update the PR accordingly. By the way, I anyway need to update the PR for making the work "Name" lowercase. Sorry to have skipped that by mistake.

Thanks!

@dpalomino dpalomino self-assigned this Mar 29, 2017
@dpalomino
Copy link

To be tested if that is reproducible in other forms (like for projects).

@dpalomino
Copy link

Retested in the forms for creating projects. Interestingly behaviour is slightly different:

  1. Trying to create an empty name project > proper error message shown ("This field is required")
  2. Trying to use an existing name > Error message is cleared and proper message is shown ("Project with this name already exists in this organization")

However inverting the order (first use an existing name and then try to use an empty name) presents both error messages:

screen shot 2017-03-30 at 00 28 32

I think this is a much more corner case.

khantaalaman added a commit to khantaalaman/cadasta-platform that referenced this issue Mar 31, 2017
@khantaalaman
Copy link
Contributor

Hi @dpalomino, the PR has been updated and now takes care of error messages while creating a new project.
Perhaps, as @oliverroick mentioned about having a generic solution taking care of all forms and related fields, I believe we should add a similar logic ( as in PR ) to one of the js files instead of specific .html files. In case we are going for that, I was planning to add necessary piece of code in form_submit_once.js file. Is doing so the best choice or there can be a better way?
Also, I tried but couldn't find the file which is responsible for showing "Name" instead of "name" in the error message. It would be really grateful of someone who can point out that particular file. Once found, we will also need to make necessary changes in test_duplicate_name_error in test_forms.py.

Looking forward to completely fix this issue.

Thanks! :)

@yoshi2095
Copy link
Contributor

yoshi2095 commented Apr 4, 2017

@khantaalaman it's in cadasta/organization/tests/test_forms.py line 68. Please correct me if I am wrong. @dpalomino @oliverroick

@khantaalaman
Copy link
Contributor

khantaalaman commented Apr 4, 2017

@yoshi2095 I have already mentioned it as the location where we would have to make the change and it's a test.

`Once found, we will also need to make necessary changes in test_duplicate_name_error in test_forms.py

I am not getting the file responsible for the feature itself and not the test.

Thanks for searching it btw. :)

@oliverroick
Copy link
Member

We're using Parsley to do client-side form validation. There might be a way to make Parsley use the same element to display error messages that Django does. That way you can have Parsley take care of the problem and it will be added to all forms.

If that's not possible, you can add that functionality to form_submit_once but I would then rename the file to a more generic name because it handles different aspects of form submission.

@dpalomino dpalomino assigned clash99 and unassigned dpalomino Apr 12, 2017
@dpalomino
Copy link

@clash99 volunteered to take a look, thanks!

@clash99
Copy link
Contributor

clash99 commented Apr 14, 2017

This string isn't translated in current format so added explicit validation message. Other languages will be translated once transifex is updated.

screenshot 2017-04-14 13 31 27

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

Successfully merging a pull request may close this issue.

6 participants