Finish audit fields for `users` table #1129

michplunkett · 2024-10-09T20:11:25Z

Fixes issue

Description of Changes

Added disabled_by, disabled_at, approved_at, approved_by, and confirmed_at, and confirmed_by fields to the users table so that we could keep track of users actions on the platform at a more granular level. I also removed the previously mentioned columns corresponding boolean columns.

Tests and Linting

This branch is up-to-date with the develop branch.
pytest passes on my local development environment.
pre-commit passes on my local development environment.
The flask db upgrade and flask db downgrade functions work for both migrations.

I have no name!@c38d30934aa7:/usr/src/app$ %
(env) michaelp@MacBook-Air-18 OpenOversight % docker exec -it openoversight-web-1 bash
I have no name!@5027495d31c0:/usr/src/app$ flask db downgrade
...
INFO  [alembic.runtime.migration] Running downgrade 99c50fc8d294 -> bf254c0961ca, complete audit field addition to users
I have no name!@5027495d31c0:/usr/src/app$ flask db downgrade
...
INFO  [alembic.runtime.migration] Running downgrade bf254c0961ca -> 5865f488470c, add remaining audit fields for users table
I have no name!@5027495d31c0:/usr/src/app$ flask db downgrade
...
INFO  [alembic.runtime.migration] Running downgrade 5865f488470c -> 939ea0f2b26d, change salary column types
I have no name!@5027495d31c0:/usr/src/app$ flask db upgrade
...
INFO  [alembic.runtime.migration] Running upgrade 939ea0f2b26d -> 5865f488470c, change salary column types
INFO  [alembic.runtime.migration] Running upgrade 5865f488470c -> bf254c0961ca, add remaining audit fields for users table
INFO  [alembic.runtime.migration] Running upgrade bf254c0961ca -> 99c50fc8d294, complete audit field addition to users
I have no name!@5027495d31c0:/usr/src/app$

…rs_.py

…ers_.py

…users.py

michplunkett · 2024-10-11T20:22:50Z

OpenOversight/app/utils/constants.py

@@ -14,6 +14,7 @@

 # Config Key Constants
 KEY_ALLOWED_EXTENSIONS = "ALLOWED_EXTENSIONS"
+KEY_APPROVE_REGISTRATIONS = "APPROVE_REGISTRATIONS"


Added this as a constant because it was being used throughout the tests.

michplunkett · 2024-10-11T22:11:14Z

OpenOversight/tests/routes/test_user_api.py

-        assert not user.is_disabled
+        assert user.disabled_at is None
+        assert user.disabled_by is None


These are the same statements, but they feel more intuitive written like this. I'd be happy to switch them back tho.

michplunkett · 2024-10-11T22:15:12Z

OpenOversight/tests/routes/test_user_api.py

-        _, user = login_admin(client)
+        _, current_user = login_admin(client)


What the user variable is referencing feels more clear with this change.

b-meson · 2024-10-22T17:57:43Z

OpenOversight/tests/routes/test_auth.py

        session.add(user)

        rv, _ = login_modified_disabled_user(client)
        assert b"/user/sam" in rv.data

        # Disable account again and check that login_required redirects user correctly
-        user.is_disabled = True
-        session.add(user)
+        user.disable_user(User.query.filter_by(is_administrator=True).first().id)


why are we doing is_admin here?

The way the disable_user function is setup, it requires a user.id so that we can see who made the action. In this case, I just pulled the first administrator user I could find and took that ID. I could have also just put 1 as the value as well.

That's very helpful and makes perfect sense. Thanks!

michplunkett added 11 commits October 9, 2024 14:14

Update database.py

c2be7e2

Create 2024-10-09-1859_0a7c591e13b9_add_remaining_audit_fields_to_use…

9ab33f5

…rs_.py

Update 2024-10-09-1859_0a7c591e13b9_add_remaining_audit_fields_to_use…

ef4fc79

…rs_.py

Delete 2024-10-09-1859_0a7c591e13b9_add_remaining_audit_fields_to_use…

d0b34fc

…rs_.py

Update database.py

9dcb384

Create 2024-10-09-1927_bf254c0961ca_add_remaining_audit_fields_for_us…

a098944

…ers_.py

Update database.py

70f883f

Update 2024-10-09-1927_bf254c0961ca_add_remaining_audit_fields_for_us…

8e065c4

…ers_.py

Update database.py

37472ce

Create 2024-10-09-2000_99c50fc8d294_complete_audit_field_addition_to_…

0e0e2f7

…users.py

Update 2024-10-09-2000_99c50fc8d294_complete_audit_field_addition_to_…

5de0e19

…users.py

michplunkett self-assigned this Oct 9, 2024

michplunkett linked an issue Oct 9, 2024 that may be closed by this pull request

Complete auditing columns for users table #1004

Closed

michplunkett added 17 commits October 9, 2024 15:26

Update database.py

171b657

Update views.py

930c5f3

Update users.html

450e715

Update test_user_api.py

a4af572

Update test_user_api.py

46fd1e4

Update users.html

4842a18

Update test_models.py

be8caa5

Blep

102f222

Update conftest.py

04c4906

Disabling

464d65d

Update test_auth.py

ceda267

Update test_user_api.py

faf125f

Update test_auth.py

3cd4bc9

Update test_user_api.py

c03bb4a

Update sort.html

0d7669d

Update views.py

acf6575

Update views.py

486c418

michplunkett added 6 commits October 9, 2024 23:18

Update test_user_api.py

08d65a9

Update database.py

b446385

Update views.py

7bff35f

Update test_user_api.py

e77393d

Update users.html

0d7dbc1

Add KEY_APPROVE_REGISTRATIONS

3393a51

michplunkett commented Oct 11, 2024

View reviewed changes

michplunkett added 8 commits October 11, 2024 15:37

Update views.py

f022e77

Update views.py

4f0c6c1

Update views.py

9800ebd

Update views.py

d05da42

Update views.py

4588b10

Update views.py

42f7d27

Update views.py

c9ad779

Update views.py

83263fe

michplunkett marked this pull request as ready for review October 11, 2024 22:08

michplunkett requested review from sea-kelp, abandoned-prototype and b-meson October 11, 2024 22:09

michplunkett changed the title ~~Finish adding audit fields to users table~~ Finish audit fields for users table Oct 11, 2024

michplunkett commented Oct 11, 2024

View reviewed changes

Update views.py

fc20b27

michplunkett commented Oct 11, 2024

View reviewed changes

Update test_models.py

709b8b0

b-meson reviewed Oct 22, 2024

View reviewed changes

michplunkett requested a review from b-meson October 22, 2024 20:32

b-meson approved these changes Oct 24, 2024

View reviewed changes

Merge branch 'develop' into finish-audit-fields-users

dadd0b1

michplunkett merged commit 0097663 into develop Oct 24, 2024
3 checks passed

michplunkett deleted the finish-audit-fields-users branch October 24, 2024 19:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Finish audit fields for `users` table #1129

Finish audit fields for `users` table #1129

michplunkett commented Oct 9, 2024 •

edited

Loading

michplunkett Oct 11, 2024

michplunkett Oct 11, 2024

michplunkett Oct 11, 2024 •

edited

Loading

b-meson Oct 22, 2024

michplunkett Oct 22, 2024 •

edited

Loading

b-meson Oct 24, 2024

		_, user = login_admin(client)
		_, current_user = login_admin(client)

Finish audit fields for users table #1129

Finish audit fields for users table #1129

Conversation

michplunkett commented Oct 9, 2024 • edited Loading

Fixes issue

Description of Changes

Tests and Linting

michplunkett Oct 11, 2024

Choose a reason for hiding this comment

michplunkett Oct 11, 2024

Choose a reason for hiding this comment

michplunkett Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

b-meson Oct 22, 2024

Choose a reason for hiding this comment

michplunkett Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

b-meson Oct 24, 2024

Choose a reason for hiding this comment

Finish audit fields for `users` table #1129

Finish audit fields for `users` table #1129

michplunkett commented Oct 9, 2024 •

edited

Loading

michplunkett Oct 11, 2024 •

edited

Loading

michplunkett Oct 22, 2024 •

edited

Loading