-
Notifications
You must be signed in to change notification settings - Fork 79
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
Address edit Department
error
#1048
Conversation
@@ -675,7 +675,6 @@ def redirect_add_department(): | |||
@admin_required | |||
def add_department(): | |||
form = DepartmentForm() | |||
form.created_by = current_user.get_id() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert department.created_by == user.id | ||
assert department.last_updated_by == user.id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert both fields are populated upon creation.
@@ -692,6 +695,8 @@ def test_admin_can_edit_police_department(mockdata, client, session): | |||
edit_state_department = Department.query.filter_by(name=CorrectedPD.name).one() | |||
assert edit_state_department.short_name == CorrectedPD.short_name | |||
assert edit_state_department.state == CorrectedPD.state | |||
assert edit_state_department.last_updated_by == user.id | |||
assert edit_state_department.last_updated_at > edit_state_department.created_at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert that the last_updated_at value is greater than the created_at value.
Would you mind adding a test for this? I definitely would have expected one of our tests to have picked this up |
Which error specifically? I checked the behavior of the app manually and via tests, and couldn't find the bug. It ends up that's because it wasn't a bug in the current version, but one that was fixed in one of the last 5 PRs. |
Can you recheck this PR when you get a chance, @sea-kelp? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Description of Changes
I removed the created_by field from the from on editing a
Department
.Tests and linting
develop
branch.pytest
passes on my local development environment.pre-commit
passes on my local development environment.