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

Fixes #315 -- Set _errors when contact is removed #472

Merged
merged 1 commit into from
Jul 20, 2016
Merged

Conversation

oliverroick
Copy link
Member

The error occurs when an invalid form is submitted, and the template tries to render error messages of all form fields. This fails because of a bug inside ContactsForm.full_clean(). `self._errors' is not set when the contact record is removed.

This PR fixes the issue by ensuring self._errors is set in all cases.

@seav
Copy link
Contributor

seav commented Jul 20, 2016

I'm testing this and I can confirm that the website no longer throws an error and the Name field has a required-field error message.

However, I get weird behavior for the contacts form. Here's one:

  1. Go to the add organization form
  2. Add some contact data in the existing single contacts form row
  3. Delete that row
  4. Add a new row (the user can't make up their mind...)
  5. Add some other contact data in the new row
  6. Submit the data (with the Name field empty)
  7. You get the expected Name is required error message, but the contacts form now contains both data that were entered.

You can try various permutations of the above steps to find more weird behavior.

@seav
Copy link
Contributor

seav commented Jul 20, 2016

Note: I think this particular bug is fixed and can be merged. And I can just submit a new issue for the weird contacts form behavior.

@ian-ross ian-ross merged commit 97aeb90 into master Jul 20, 2016
@ian-ross ian-ross deleted the bugfix/#315 branch July 20, 2016 06:17
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.

3 participants