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 form validation instead of triggering integrity error #643

Merged
merged 16 commits into from
Sep 20, 2022

Conversation

munnsmunns
Copy link
Collaborator

@munnsmunns munnsmunns commented Jul 8, 2022

Create a generic helper method to check unique by assessment validation, when we don't include the assessment in a model form. This helper method is applied to numerous forms via a clean_FOO method.

We initially tried including the assessment field in the form as a disabled hidden field, but ran into some issues with the value being hydrated correctly with a foreign key relation on updates.

  • assessment.DatasetForm
  • lit.SearchForm
  • riskofbias.RoBDomainForm
  • summary.SummaryTableForm
  • summary.VisualForm
  • summary.DataPivotForm

In addition, expose non_field_errors in the react DjangoForm if errors are returned in validation.

@munnsmunns munnsmunns marked this pull request as ready for review July 11, 2022 20:21
@munnsmunns munnsmunns requested a review from shapiromatron July 11, 2022 20:21
@munnsmunns munnsmunns changed the base branch from main to next September 14, 2022 15:15
Copy link
Owner

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

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

This isn't working for me; when I try to update an existing summary.Visual without changing anything, I get a field form error.

I have a few ideas, I'm going to start working on this...

Copy link
Owner

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

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

@munnsmunns ok, made some edits and I think this is a reasonable solution that works well - as a bonus - net negative code! Please give it a review to makes sure it works as expected for creates, updates, etc.

@munnsmunns munnsmunns merged commit e74c291 into next Sep 20, 2022
@munnsmunns munnsmunns deleted the integrity-error branch September 20, 2022 21:15
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.

2 participants