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.

Sign up for GitHub

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

Jump to bottom

Use database constraints instead of python asserts #1957

Merged
merged 15 commits into from
Jul 3, 2023
Merged

Use database constraints instead of python asserts #1957

merged 15 commits into from
Jul 3, 2023

Conversation

florian-str
Copy link
Collaborator

This implements all reasonable constraints as requested in #1800.

@niklasmohrin niklasmohrin self-requested a review June 12, 2023 17:05
niklasmohrin
niklasmohrin reviewed Jun 12, 2023
View reviewed changes
Copy link
Member

@niklasmohrin niklasmohrin left a comment

Choose a reason for hiding this comment

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

Make sure to run manage.py format :) Secondly, I think the constraints could all use some nicer names, I made a suggestion for one of them below, we can also come up with the others together if you want.

evap/evaluation/migrations/0132_evaluation_check_vote_date_and_more.py Outdated Show resolved Hide resolved
evap/evaluation/models.py Outdated Show resolved Hide resolved
evap/evaluation/models.py Outdated Show resolved Hide resolved
evap/evaluation/models.py Outdated Show resolved Hide resolved
evap/rewards/models.py Outdated Show resolved Hide resolved
@richardebeling richardebeling changed the title Implementation of #1800 Use database constraints instead of python asserts Jun 14, 2023
@florian-str florian-str force-pushed the constraint branch 3 times, most recently from 3794d3c to cad4659 Compare June 26, 2023 16:30
richardebeling
richardebeling approved these changes Jul 3, 2023
View reviewed changes
Copy link
Member

@richardebeling richardebeling left a comment

Choose a reason for hiding this comment

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

Code looks good. As discussed, please check whether the error message on an evaluation edit/create pages in the error case "end date is before start datetime" is unchanged.

evap/rewards/models.py Show resolved Hide resolved
@florian-str
Copy link
Collaborator Author

It does in fact still produce the correct error message.

niklasmohrin
niklasmohrin approved these changes Jul 3, 2023
View reviewed changes
Copy link
Member

@niklasmohrin niklasmohrin left a comment

Choose a reason for hiding this comment

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

Very nice!

evap/evaluation/migrations/0136_use_more_database_constraints.py Outdated Show resolved Hide resolved
@niklasmohrin
Copy link
Member

I'll update the migration file and merge it real quick :)

@niklasmohrin niklasmohrin merged commit 987684b into e-valuation:main Jul 3, 2023
@niklasmohrin niklasmohrin mentioned this pull request Jul 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niklasmohrin niklasmohrin niklasmohrin approved these changes

@richardebeling richardebeling richardebeling approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@florian-str @niklasmohrin @richardebeling