Merge upstream commits #534

sea-kelp · 2025-02-27T09:35:51Z

Description of Changes

Pulling in the following changes:
#533 (Needed so unit tests don't fail)
lucyparsons#1159
lucyparsons#1157
lucyparsons#1155
lucyparsons#1151
lucyparsons#1150
lucyparsons#1143
lucyparsons#1141
lucyparsons#1140
lucyparsons#1133
lucyparsons#1129
lucyparsons#1126
lucyparsons#1124

Notes for Deployment

None!

Screenshots (if appropriate)

Testing instructions

Ran tests and visited all pages

Checks

I have rebased my changes on main
just lint passes
just test passes

There were a handful of routes where we did not validate the initial ID parameters. Additionally, I standardized how object querying was done within the `main.views` file. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.

lucyparsons#1125 Removed the `mockdata` parameter from test functions that don't require it. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.

lucyparsons#1004 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. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment. - [x] The `flask db upgrade` and `flask db downgrade` functions work for both migrations. ```console 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$ ```

## Fixes issue Addresses TO-DO in code. ## Description of Changes The `/find` route was using an unsorted list of departments unlike the rest of the application. Before: <img width="1334" alt="Screenshot 2024-11-06 at 10 13 13 PM" src="https://github.com/user-attachments/assets/8c6b832a-109e-465a-a6ec-c102524eb43b"> After: <img width="1324" alt="Screenshot 2024-11-06 at 10 13 36 PM" src="https://github.com/user-attachments/assets/1f639518-a750-498e-8a26-6f5784ad052b"> ## Tests and Linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.

## Description of Changes Addressed inconsistencies with the model `__repr__` functions. Mostly making this because this PR got a bit out of control: lucyparsons#1138 ## Tests and Linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.

Sets the groundwork for this issue: lucyparsons#385 Simplifying the import logic for the `auth` and `main` routes of the application. This also sets up the work for this PR: lucyparsons#1138 - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.

Adds a generic `__repr__` function and and a generic `to_dict` function so that objects can be serialized in the future. The reason I didn't use the SQLAlchemy class like we talked about in the previous PR was because that would require a complete rework of most object instantiations and how properties are ordered. For a greater view into that, you can look at this PR: lucyparsons#1142 - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.

Moved `Department`-related methods to the `Department` object. - [x] This branch is up-to-date with the `develop` branch. - [x] Ran `make create_db_diagram` command. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.

## Description of Changes Removed audit fields from the `Description` download function. They leak user information and provide data that isn't particularly useful outside of internal auditing. ## Tests and Linting - [x] This branch is up-to-date with the `develop` branch. - [x] Ran `make create_db_diagram` command. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.

## Description of Changes _Changes carried over from https://github.com/OrcaCollective/OpenOversight/pull/533_ Fix bug where submit image page department selector was not submitting images to the correct department. It looks like the current implementation was using `selectedIndex` (the index of the selected `<option>` element in the `<select>`) to determine the initial department id, which assumes the departments are loaded in order: https://github.com/OrcaCollective/OpenOversight/blob/2c5ad9a74687943eac5e71a874c0f4ceb4784fa0/OpenOversight/app/templates/submit_image.html#L79-L82 ## Notes for Deployment None! ## Screenshots (if appropriate) N/A ## Testing instructions 1. Log in as admin user. 2. Create a new department "Peoria Police Department (PPD)" at http://localhost:3000/departments/new. 3. Update the user's preferred department to PPD at http://localhost:3000/auth/change-dept/. 4. Visit the Submit Image page (http://localhost:3000/submit) and confirm that the populated department is BPD. 5. Upload an image. 6. Verify in devtools that an image was submitted to http://localhost:3000/upload/departments/4. 7. Update the department selector to Springfield Police Department. 8. Upload an image. 9. Verify in devtools that an image was submitted to http://localhost:3000/upload/departments/1. 10. Change the preferred department to Springfield Police Department at http://localhost:3000/auth/change-dept/. 11. Visit the Submit Image page (http://localhost:3000/submit) and confirm that the populated department is now SPD. 12. Upload an image. 13. Verify in devtools that an image was submitted to http://localhost:3000/upload/departments/1. ## Checks - [x] I have rebased my changes on `main` - [x] `just lint` passes - [x] `just test` passes

## Description of Changes Default to "N/A" if department state is invalid In [the migration](https://github.com/lucyparsons/OpenOversight/blob/develop/OpenOversight/migrations/versions/2023-07-26-1551_18f43ac4622f_add_state_column_to_departments.py) where we introduced the department state column, we defaulted to empty string for existing departments. We think this may have broken the browse page when trying to load the browse page. ## Notes for Deployment None! ## Screenshots (if appropriate) N/A ## Tests and Linting Reproduced the internal server error by setting the department state to empty string ``` openoversight-dev=# update departments set state='' where id = 1; ``` Checked that the error was no longer displayed once this change was implemented - [x] This branch is up-to-date with the `develop` branch. - [ ] Ran `make create_db_diagram` command. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.

sea-kelp · 2025-02-27T09:56:36Z

OpenOversight/app/templates/contact.html

+        <h2>Contact and media information:</h2>
+        <p>
+          For media inquiries about OpenOversight, please email <a href="mailto:[email protected]">[email protected]</a>.
+        </p>
+        <p>
+          For other inquiries about the project, including collaboration, please contact <a href="mailto:[email protected]">[email protected]</a>.
+        </p>


Does this need to be updated?

Yes I think so, I'll make the suggested edit!

sea-kelp · 2025-02-27T09:58:38Z

OpenOversight/tests/routes/test_officer_and_department.py

+@pytest.mark.skip(reason="Last updated dates not incorporated into new design yet")
 def test_browse_displays_last_updated_dates(mockdata, client, session):


Temporarily disabling this test until we can figure out how to incorporate the last updated dates into the new re-design

sea-kelp · 2025-02-27T10:00:41Z

OpenOversight/app/templates/index.html

-              While Seattle Tech Bloc does not accept donations at this time, if you'd like to financially
-              contribute to police accountability work in the Seattle area, please donate to Divest SPD or
-              these local BIPOC who have been doing the work as part of this movement.
+              We're looking for volunteers to help us sort through images. If you're interested in helping out, please sign up!
            </p>
-            <a class="btn btn-lg btn-secondary"
-               href="https://venmo.com/code?user_id=2748347827879936448"
-               target="_blank"
-               rel="noopener noreferrer">Donate to Divest SPD via Venmo</a>
-            <div class="small-horizontal-padding"></div>
-            <a class="btn btn-lg btn-secondary"
-               href="https://docs.google.com/spreadsheets/u/0/d/1BOZr842Qtwx7aWKTb8qnVbKntBNJVvRKFOAw48dk9c4/htmlview"
-               target="_blank"
-               rel="noopener noreferrer">Pay The Fee</a>
-          </div>


Is this still relevant? Is there somewhere on the page that it would make sense to slot this back in?

I think the Divest SPD link is still good to have, and could replace (or be below) the LPL donation link.

This is what the link looks like right now so I'm not sure there's a good way to fit that in unfortunately

sea-kelp · 2025-02-27T10:09:53Z

OpenOversight/app/templates/base.html

+            <p class="font-weight-300">spd.watch is independently maintained by Seattle Tech Bloc.</p>
+            <a href="https://lucyparsonslabs.com/" target="_blank">
+              <img src="{{ url_for('static', filename='images/lpl-logo-white-transparent.png') }}"
+                   class="mt-4 lpl-logo"
+                   alt="Lucy Parsons Labs logo">
+            </a>
+            <p>
+              <a href="https://bsky.app/profile/divestspd.bsky.social"><i class="fa-brands fa-bluesky fa-3x social"></i></a>
+              <a href="https://instagram.com/divestspd"><i class="fa-brands fa-instagram fa-3x social"></i></a>
+              <a href="https://github.com/OrcaCollective/openoversight"><i class="fa-brands fa-github fa-3x social"></i></a>
+              <a href="mailto:[email protected]"><i class="fa fa-envelope fa-3x social"></i></a>
+            </p>


Let me know if there's anything that needs to be updated in this section

I think this is good!

AetherUnbound

I tried running this locally, but I ran into the following when I tried to log into an account that had existed before the redesign:

sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedColumn) column users.confirmed_at does not exist
LINE 1: ...name, users.password_hash AS users_password_hash, users.conf...
                                                             ^
HINT:  Perhaps you meant to reference the column "users.confirmed".

[SQL: SELECT users.id AS users_id, users._uuid AS users__uuid, users.email AS users_email, users.username AS users_username, users.password_hash AS users_password_hash, users.confirmed_at AS users_confirmed_at, users.confirmed_by AS users_confirmed_by, users.approved_at AS users_approved_at, users.approved_by AS users_approved_by, users.is_area_coordinator AS users_is_area_coordinator, users.ac_department_id AS users_ac_department_id, users.is_administrator AS users_is_administrator, users.disabled_at AS users_disabled_at, users.disabled_by AS users_disabled_by, users.dept_pref AS users_dept_pref, users.created_at AS users_created_at 
FROM users 
WHERE lower(users.email) = lower(%(lower_1)s) 
 LIMIT %(param_1)s]
[parameters: {'lower_1': '[email protected]', 'param_1': 1}]
(Background on this error at: https://sqlalche.me/e/20/f405)

I think you can reproduce this by:

Checking out main
Running just fresh-start and setting up an admin account
Checking out this branch
Build, and attempt to log in as the admin

Do you mind checking to see if you can reproduce?

AetherUnbound · 2025-03-01T07:01:46Z

OpenOversight/app/templates/contact.html

+        <h2>Contact and media information:</h2>
+        <p>
+          For media inquiries about OpenOversight, please email <a href="mailto:[email protected]">[email protected]</a>.
+        </p>
+        <p>
+          For other inquiries about the project, including collaboration, please contact <a href="mailto:[email protected]">[email protected]</a>.
+        </p>


Yes I think so, I'll make the suggested edit!

OpenOversight/app/templates/contact.html

AetherUnbound · 2025-03-01T07:08:26Z

OpenOversight/app/templates/index.html

-              While Seattle Tech Bloc does not accept donations at this time, if you'd like to financially
-              contribute to police accountability work in the Seattle area, please donate to Divest SPD or
-              these local BIPOC who have been doing the work as part of this movement.
+              We're looking for volunteers to help us sort through images. If you're interested in helping out, please sign up!
            </p>
-            <a class="btn btn-lg btn-secondary"
-               href="https://venmo.com/code?user_id=2748347827879936448"
-               target="_blank"
-               rel="noopener noreferrer">Donate to Divest SPD via Venmo</a>
-            <div class="small-horizontal-padding"></div>
-            <a class="btn btn-lg btn-secondary"
-               href="https://docs.google.com/spreadsheets/u/0/d/1BOZr842Qtwx7aWKTb8qnVbKntBNJVvRKFOAw48dk9c4/htmlview"
-               target="_blank"
-               rel="noopener noreferrer">Pay The Fee</a>
-          </div>


I think the Divest SPD link is still good to have, and could replace (or be below) the LPL donation link.

AetherUnbound · 2025-03-01T07:17:06Z

OpenOversight/app/templates/base.html

+            <p class="font-weight-300">spd.watch is independently maintained by Seattle Tech Bloc.</p>
+            <a href="https://lucyparsonslabs.com/" target="_blank">
+              <img src="{{ url_for('static', filename='images/lpl-logo-white-transparent.png') }}"
+                   class="mt-4 lpl-logo"
+                   alt="Lucy Parsons Labs logo">
+            </a>
+            <p>
+              <a href="https://bsky.app/profile/divestspd.bsky.social"><i class="fa-brands fa-bluesky fa-3x social"></i></a>
+              <a href="https://instagram.com/divestspd"><i class="fa-brands fa-instagram fa-3x social"></i></a>
+              <a href="https://github.com/OrcaCollective/openoversight"><i class="fa-brands fa-github fa-3x social"></i></a>
+              <a href="mailto:[email protected]"><i class="fa fa-envelope fa-3x social"></i></a>
+            </p>


I think this is good!

AetherUnbound

Update: I ran just db upgrade and that resolved the issue 😅

Amazing work on this @sea-kelp - it's looking so good!! And all the information (save for the suggestions I've made) looks great. 🚀

michplunkett and others added 14 commits February 24, 2025 22:39

UX Re-design (lucyparsons#1155)

cab67d7

Fix department selector on submit image page

21c6df1

Re-add dropzone

7fe9594

sea-kelp requested a review from a team as a code owner February 27, 2025 09:35

sea-kelp commented Feb 27, 2025

View reviewed changes

AetherUnbound reviewed Mar 1, 2025

View reviewed changes

AetherUnbound approved these changes Mar 1, 2025

View reviewed changes

Update contact email

08677b3

sea-kelp force-pushed the upstream-batch branch from a6c9e00 to 08677b3 Compare March 1, 2025 08:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge upstream commits #534

Merge upstream commits #534

sea-kelp commented Feb 27, 2025

sea-kelp Feb 27, 2025

AetherUnbound Mar 1, 2025

sea-kelp Feb 27, 2025

sea-kelp Feb 27, 2025

AetherUnbound Mar 1, 2025

sea-kelp Mar 1, 2025

sea-kelp Feb 27, 2025

AetherUnbound Mar 1, 2025

AetherUnbound left a comment

AetherUnbound Mar 1, 2025

AetherUnbound Mar 1, 2025

AetherUnbound Mar 1, 2025

AetherUnbound left a comment

		@pytest.mark.skip(reason="Last updated dates not incorporated into new design yet")
		def test_browse_displays_last_updated_dates(mockdata, client, session):

Merge upstream commits #534

Are you sure you want to change the base?

Merge upstream commits #534

Conversation

sea-kelp commented Feb 27, 2025

Description of Changes

Notes for Deployment

Screenshots (if appropriate)

Testing instructions

Checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AetherUnbound left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AetherUnbound left a comment

Choose a reason for hiding this comment