Remove transaction.rollback() and use session fixture (#1117)

## Fixes issue
#1054
- This is the last PR for this issue. ❗❗❗❗❗❗ 

## Description of Changes
Removed the redundant `transaction.rollback()` function call in the
`session` fixture and changed all references to `db.session` to
`session` so that tests are using the correct scoping.

Addressed this warning:
```console
=============================================================================================== warnings summary ===============================================================================================
OpenOversight/tests/test_commands.py::test_add_department__duplicate
OpenOversight/tests/test_models.py::test__uuid_uniqueness_constraint
OpenOversight/tests/routes/test_singular_redirects.py::test_redirect_add_salary
  /usr/src/app/OpenOversight/tests/conftest.py:325: SAWarning: transaction already deassociated from connection
    transaction.rollback()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

---------- coverage: platform linux, python 3.11.9-final-0 -----------
```

## 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.
- [x] Validate that no more warnings show up in the `make test` command.

```console
---------- coverage: platform linux, python 3.11.9-final-0 -----------
Name                                                                                                         Stmts   Miss  Cover
--------------------------------------------------------------------------------------------------------------------------------
OpenOversight/__init__.py                                                                                        0      0   100%
OpenOversight/app/__init__.py                                                                                   74      1    99%
```

OpenOversight/tests/conftest.py

@@ -314,17 +314,16 @@ def db(app):
 @pytest.fixture(scope="function")
 def session(db):
     """Creates a new database session for a test."""
-    connection = db.engine.connect()
-    transaction = connection.begin()
-
-    session = scoped_session(session_factory=sessionmaker(bind=connection))
-    db.session = session
-
-    yield session
-
-    transaction.rollback()
-    connection.close()
-    session.remove()
+    with db.engine.connect() as connection:
+        with connection.begin():
+            session = scoped_session(session_factory=sessionmaker(bind=connection))
+            db.session = session
+
+            try:
+                yield session
+            finally:
+                session.remove()
+                connection.close()
 
 
 @pytest.fixture

OpenOversight/tests/routes/test_descriptions.py

@@ -5,7 +5,7 @@
 from flask import current_app, url_for
 
 from OpenOversight.app.main.forms import EditTextForm, TextForm
-from OpenOversight.app.models.database import Description, Officer, User, db
+from OpenOversight.app.models.database import Description, Officer, User
 from OpenOversight.app.utils.constants import ENCODING_UTF_8
 from OpenOversight.tests.conftest import AC_DEPT
 from OpenOversight.tests.constants import ADMIN_USER_EMAIL
@@ -134,8 +134,8 @@ def test_admins_can_edit_descriptions(mockdata, client, session):
             created_by=admin.id,
             last_updated_by=admin.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
 
         form = EditTextForm(
             text_contents=new_description,
@@ -175,8 +175,8 @@ def test_ac_can_edit_their_descriptions_in_their_department(mockdata, client, se
             created_by=user.id,
             last_updated_by=user.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
 
         form = EditTextForm(
             text_contents=new_description,
@@ -216,8 +216,8 @@ def test_ac_can_edit_others_descriptions(mockdata, client, session):
             created_by=user.id - 1,
             last_updated_by=user.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
 
         form = EditTextForm(
             text_contents=new_description,
@@ -257,8 +257,8 @@ def test_ac_cannot_edit_descriptions_not_in_their_department(mockdata, client, s
             created_at=original_date,
             last_updated_at=original_date,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
 
         form = EditTextForm(
             text_contents=new_description,
@@ -307,8 +307,8 @@ def test_acs_can_delete_their_descriptions_in_their_department(
             created_at=now,
             last_updated_at=now,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
         description_id = description.id
         rv = client.post(
             url_for(
@@ -338,8 +338,8 @@ def test_acs_cannot_delete_descriptions_not_in_their_department(
             created_at=now,
             last_updated_at=now,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
         description_id = description.id
         rv = client.post(
             url_for(
@@ -363,8 +363,8 @@ def test_acs_can_get_edit_form_for_their_dept(mockdata, client, session):
             text_contents="Hello",
             officer_id=officer.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
         rv = client.get(
             url_for(
                 "main.description_api_edit",
@@ -387,8 +387,8 @@ def test_acs_can_get_others_edit_form(mockdata, client, session):
             created_by=user.id - 1,
             last_updated_by=user.id - 1,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
         rv = client.get(
             url_for(
                 "main.description_api_edit",
@@ -411,8 +411,8 @@ def test_acs_cannot_get_edit_form_for_their_non_dept(mockdata, client, session):
             text_contents="Hello",
             officer_id=officer.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
         rv = client.get(
             url_for(
                 "main.description_api_edit",
@@ -434,8 +434,8 @@ def test_users_can_see_descriptions(mockdata, client, session):
             text_contents=text_contents,
             officer_id=officer.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
         rv = client.get(
             url_for("main.officer_profile", officer_id=officer.id),
             follow_redirects=True,
@@ -455,8 +455,8 @@ def test_admins_can_see_descriptions(mockdata, client, session):
             text_contents=text_contents,
             officer_id=officer.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
         rv = client.get(
             url_for("main.officer_profile", officer_id=officer.id),
             follow_redirects=True,
@@ -475,8 +475,8 @@ def test_acs_can_see_descriptions_in_their_department(mockdata, client, session)
             text_contents=text_contents,
             officer_id=officer.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
         rv = client.get(
             url_for("main.officer_profile", officer_id=officer.id),
             follow_redirects=True,
@@ -498,8 +498,8 @@ def test_acs_can_see_descriptions_not_in_their_department(mockdata, client, sess
             text_contents=text_contents,
             officer_id=officer.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
         rv = client.get(
             url_for("main.officer_profile", officer_id=officer.id),
             follow_redirects=True,
@@ -523,8 +523,8 @@ def test_anonymous_users_cannot_see_description_creators(mockdata, client, sessi
             created_by=ac.id,
             last_updated_by=ac.id,
         )
-        db.session.add(description)
-        db.session.commit()
+        session.add(description)
+        session.commit()
 
         rv = client.get(
             url_for("main.officer_profile", officer_id=officer.id),

OpenOversight/tests/routes/test_notes.py

@@ -5,7 +5,7 @@
 from flask import current_app, url_for
 
 from OpenOversight.app.main.forms import EditTextForm, TextForm
-from OpenOversight.app.models.database import Department, Note, Officer, db
+from OpenOversight.app.models.database import Department, Note, Officer
 from OpenOversight.app.models.database_cache import (
     has_database_cache_entry,
     put_database_cache_entry,
@@ -123,8 +123,8 @@ def test_admins_can_edit_notes(mockdata, client, session, faker):
             created_at=original_date,
             last_updated_at=original_date,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
 
         form = EditTextForm(
             text_contents=new_note,
@@ -159,8 +159,8 @@ def test_ac_can_edit_their_notes_in_their_department(mockdata, client, session,
             created_at=original_date,
             last_updated_at=original_date,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
 
         form = EditTextForm(
             text_contents=new_note,
@@ -193,8 +193,8 @@ def test_ac_can_edit_others_notes(mockdata, client, session):
             last_updated_at=original_date,
             last_updated_by=user.id - 1,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
 
         form = EditTextForm(
             text_contents=new_note,
@@ -230,8 +230,8 @@ def test_ac_cannot_edit_notes_not_in_their_department(mockdata, client, session)
             last_updated_at=original_date,
             last_updated_by=user.id,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
 
         form = EditTextForm(
             text_contents=new_note,
@@ -281,8 +281,8 @@ def test_acs_can_delete_their_notes_in_their_department(mockdata, client, sessio
             last_updated_at=now,
             last_updated_by=user.id,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
         rv = client.post(
             url_for("main.note_api_delete", officer_id=officer.id, obj_id=note.id),
             follow_redirects=True,
@@ -309,8 +309,8 @@ def test_acs_cannot_delete_notes_not_in_their_department(
             last_updated_at=now,
             last_updated_by=2,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
         rv = client.post(
             url_for("main.note_api_delete", officer_id=officer.id, obj_id=note.id),
             follow_redirects=True,
@@ -334,8 +334,8 @@ def test_acs_can_get_edit_form_for_their_dept(mockdata, client, session):
             last_updated_at=now,
             last_updated_by=user.id,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
         rv = client.get(
             url_for("main.note_api_edit", obj_id=note.id, officer_id=officer.id),
             follow_redirects=True,
@@ -357,8 +357,8 @@ def test_acs_can_get_others_edit_form(mockdata, client, session):
             last_updated_at=now,
             last_updated_by=user.id - 1,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
         rv = client.get(
             url_for("main.note_api_edit", obj_id=note.id, officer_id=officer.id),
             follow_redirects=True,
@@ -382,8 +382,8 @@ def test_acs_cannot_get_edit_form_for_their_non_dept(mockdata, client, session):
             last_updated_at=now,
             last_updated_by=2,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
         rv = client.get(
             url_for("main.note_api_edit", obj_id=note.id, officer_id=officer.id),
             follow_redirects=True,
@@ -404,8 +404,8 @@ def test_users_cannot_see_notes(mockdata, client, session, faker):
             last_updated_at=now,
             last_updated_by=1,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
         rv = client.get(
             url_for("main.officer_profile", officer_id=officer.id),
             follow_redirects=True,
@@ -430,8 +430,8 @@ def test_admins_can_see_notes(mockdata, client, session):
             last_updated_at=now,
             last_updated_by=user.id,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
         rv = client.get(
             url_for("main.officer_profile", officer_id=officer.id),
             follow_redirects=True,
@@ -455,8 +455,8 @@ def test_acs_can_see_notes_in_their_department(mockdata, client, session):
             last_updated_at=now,
             last_updated_by=user.id,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
         rv = client.get(
             url_for("main.officer_profile", officer_id=officer.id),
             follow_redirects=True,
@@ -482,8 +482,8 @@ def test_acs_cannot_see_notes_not_in_their_department(mockdata, client, session)
             last_updated_at=now,
             last_updated_by=1,
         )
-        db.session.add(note)
-        db.session.commit()
+        session.add(note)
+        session.commit()
         rv = client.get(
             url_for("main.officer_profile", officer_id=officer.id),
             follow_redirects=True,

OpenOversight/tests/routes/test_user_api.py

@@ -4,7 +4,7 @@
 from flask import current_app, url_for
 
 from OpenOversight.app.auth.forms import EditUserForm, LoginForm, RegistrationForm
-from OpenOversight.app.models.database import User, db
+from OpenOversight.app.models.database import User
 from OpenOversight.app.utils.constants import ENCODING_UTF_8
 from OpenOversight.tests.conftest import AC_DEPT
 from OpenOversight.tests.constants import (
@@ -208,7 +208,7 @@ def test_admin_can_enable_user(mockdata, client, session):
 
         user = User.query.filter_by(email=GENERAL_USER_EMAIL).one()
         user.is_disabled = True
-        db.session.commit()
+        session.commit()
 
         user = session.get(User, user.id)
         assert user.is_disabled
@@ -291,7 +291,7 @@ def test_admin_can_approve_user(mockdata, client, session):
 
         user = User.query.filter_by(email=GENERAL_USER_EMAIL).first()
         user.approved = False
-        db.session.commit()
+        session.commit()
 
         user = session.get(User, user.id)
         assert not user.approved
@@ -344,7 +344,7 @@ def test_admin_approval_sends_confirmation_email(
         user = User.query.filter_by(is_administrator=False).first()
         user.approved = currently_approved
         user.confirmed = currently_confirmed
-        db.session.commit()
+        session.commit()
 
         user = session.get(User, user.id)
         assert user.approved == currently_approved