Edit login user functions to return the User (lucyparsons#1013)

lucyparsons#1010

Edit the `login_user` functions so that they return the user (removing
the need for an extra database query) and create constants for the login
values.

 - [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.

OpenOversight/app/models/database.py

@@ -150,7 +150,7 @@ class Job(BaseModel):
     )
 
     def __repr__(self):
-        return f"<Job ID {self.id,}: {self.job_title}>"
+        return f"<Job ID {self.id}: {self.job_title}>"
 
     def __str__(self):
         return self.job_title

OpenOversight/app/utils/constants.py

@@ -48,7 +48,3 @@
 MEGABYTE = 1024 * KILOBYTE
 MINUTE = 60
 HOUR = 60 * MINUTE
-
-# Test Constants
-ADMIN_EMAIL = "[email protected]"
-ADMIN_PASSWORD = "testtest"

OpenOversight/tests/conftest.py

@@ -22,7 +22,7 @@
 from webdriver_manager.firefox import GeckoDriverManager
 from xvfbwrapper import Xvfb
 
-from OpenOversight.app import create_app
+from OpenOversight.app import EmailClient, create_app
 from OpenOversight.app.models.database import (
     Assignment,
     Department,
@@ -42,13 +42,28 @@
 )
 from OpenOversight.app.models.database import db as _db
 from OpenOversight.app.utils.choices import DEPARTMENT_STATE_CHOICES
-from OpenOversight.app.utils.constants import (
-    ADMIN_EMAIL,
-    ADMIN_PASSWORD,
-    ENCODING_UTF_8,
-    KEY_NUM_OFFICERS,
-)
+from OpenOversight.app.utils.constants import ENCODING_UTF_8, KEY_NUM_OFFICERS
 from OpenOversight.app.utils.general import merge_dicts
+from OpenOversight.tests.constants import (
+    AC_USER_EMAIL,
+    AC_USER_PASSWORD,
+    AC_USER_USERNAME,
+    ADMIN_USER_EMAIL,
+    ADMIN_USER_PASSWORD,
+    ADMIN_USER_USER_NAME,
+    DISABLED_USER_EMAIL,
+    DISABLED_USER_PASSWORD,
+    DISABLED_USER_USERNAME,
+    GENERAL_USER_EMAIL,
+    GENERAL_USER_PASSWORD,
+    GENERAL_USER_USERNAME,
+    MOD_DISABLED_USER_EMAIL,
+    MOD_DISABLED_USER_PASSWORD,
+    MOD_DISABLED_USER_USERNAME,
+    UNCONFIRMED_USER_EMAIL,
+    UNCONFIRMED_USER_PASSWORD,
+    UNCONFIRMED_USER_USERNAME,
+)
 
 
 factory = Faker()
@@ -326,39 +341,45 @@ def add_mockdata(session):
     assert current_app.config[KEY_NUM_OFFICERS] >= 5
 
     test_user = User(
-        email="[email protected]", username="test_user", password="dog", confirmed=True
+        email=GENERAL_USER_EMAIL,
+        username=GENERAL_USER_USERNAME,
+        password=GENERAL_USER_PASSWORD,
+        confirmed=True,
     )
     session.add(test_user)
 
     test_admin = User(
-        email=ADMIN_EMAIL,
-        username="test_admin",
-        password=ADMIN_PASSWORD,
+        email=ADMIN_USER_EMAIL,
+        username=ADMIN_USER_USER_NAME,
+        password=ADMIN_USER_PASSWORD,
         confirmed=True,
         is_administrator=True,
     )
     session.add(test_admin)
 
     test_unconfirmed_user = User(
-        email="[email protected]", username="b_meson", password="dog", confirmed=False
+        email=UNCONFIRMED_USER_EMAIL,
+        username=UNCONFIRMED_USER_USERNAME,
+        password=UNCONFIRMED_USER_PASSWORD,
+        confirmed=False,
     )
     session.add(test_unconfirmed_user)
     session.commit()
 
     test_disabled_user = User(
-        email="[email protected]",
-        username="may",
-        password="yam",
+        email=DISABLED_USER_EMAIL,
+        username=DISABLED_USER_USERNAME,
+        password=DISABLED_USER_PASSWORD,
         confirmed=True,
         is_disabled=True,
     )
     session.add(test_disabled_user)
     session.commit()
 
     test_modified_disabled_user = User(
-        email="[email protected]",
-        username="sam",
-        password="the yam",
+        email=MOD_DISABLED_USER_EMAIL,
+        username=MOD_DISABLED_USER_USERNAME,
+        password=MOD_DISABLED_USER_PASSWORD,
         confirmed=True,
         is_disabled=True,
     )
@@ -390,9 +411,9 @@ def add_mockdata(session):
     session.commit()
 
     test_area_coordinator = User(
-        email="[email protected]",
-        username="test_ac",
-        password="horse",
+        email=AC_USER_EMAIL,
+        username=AC_USER_USERNAME,
+        password=AC_USER_PASSWORD,
         confirmed=True,
         is_area_coordinator=True,
         ac_department_id=AC_DEPT,

OpenOversight/tests/constants.py

@@ -0,0 +1,19 @@
+# User Constants
+AC_USER_EMAIL = "[email protected]"
+AC_USER_PASSWORD = "horse"
+AC_USER_USERNAME = "test_ac"
+ADMIN_USER_EMAIL = "[email protected]"
+ADMIN_USER_PASSWORD = "testtest"
+ADMIN_USER_USER_NAME = "test_admin"
+GENERAL_USER_EMAIL = "[email protected]"
+GENERAL_USER_PASSWORD = "dog"
+GENERAL_USER_USERNAME = "test_user"
+DISABLED_USER_EMAIL = "[email protected]"
+DISABLED_USER_PASSWORD = "yam"
+DISABLED_USER_USERNAME = "may"
+MOD_DISABLED_USER_EMAIL = "[email protected]"
+MOD_DISABLED_USER_PASSWORD = "the yam"
+MOD_DISABLED_USER_USERNAME = "sam"
+UNCONFIRMED_USER_EMAIL = "[email protected]"
+UNCONFIRMED_USER_PASSWORD = "dog"
+UNCONFIRMED_USER_USERNAME = "b_meson"

OpenOversight/tests/routes/route_helpers.py

@@ -2,49 +2,69 @@
 
 from OpenOversight.app.auth.forms import LoginForm
 from OpenOversight.app.models.database import User
-from OpenOversight.app.utils.constants import ADMIN_PASSWORD
-from OpenOversight.tests.conftest import AC_DEPT
+from OpenOversight.tests.constants import (
+    AC_USER_EMAIL,
+    AC_USER_PASSWORD,
+    ADMIN_USER_EMAIL,
+    ADMIN_USER_PASSWORD,
+    DISABLED_USER_EMAIL,
+    DISABLED_USER_PASSWORD,
+    GENERAL_USER_EMAIL,
+    GENERAL_USER_PASSWORD,
+    MOD_DISABLED_USER_EMAIL,
+    MOD_DISABLED_USER_PASSWORD,
+    UNCONFIRMED_USER_EMAIL,
+    UNCONFIRMED_USER_PASSWORD,
+)
 
 
 def login_user(client):
-    user = User.query.filter_by(id=1).first()
-    form = LoginForm(email=user.email, password="dog", remember_me=True)
+    user = User.query.filter_by(email=GENERAL_USER_EMAIL).first()
+    form = LoginForm(email=user.email, password=GENERAL_USER_PASSWORD, remember_me=True)
     rv = client.post(url_for("auth.login"), data=form.data, follow_redirects=False)
-    return rv
+    return rv, user
 
 
 def login_unconfirmed_user(client):
-    user = User.query.filter_by(confirmed=False).first()
-    form = LoginForm(email=user.email, password="dog", remember_me=True)
+    user = User.query.filter_by(email=UNCONFIRMED_USER_EMAIL).first()
+    form = LoginForm(
+        email=user.email, password=UNCONFIRMED_USER_PASSWORD, remember_me=True
+    )
     rv = client.post(url_for("auth.login"), data=form.data, follow_redirects=False)
     assert b"Invalid username or password" not in rv.data
-    return rv
+    return rv, user
 
 
 def login_disabled_user(client):
-    form = LoginForm(email="[email protected]", password="yam", remember_me=True)
+    user = User.query.filter_by(email=DISABLED_USER_EMAIL).first()
+    form = LoginForm(
+        email=user.email, password=DISABLED_USER_PASSWORD, remember_me=True
+    )
     rv = client.post(url_for("auth.login"), data=form.data, follow_redirects=True)
-    return rv
+    return rv, user
 
 
 def login_modified_disabled_user(client):
-    form = LoginForm(email="[email protected]", password="the yam", remember_me=True)
+    user = User.query.filter_by(email=MOD_DISABLED_USER_EMAIL).first()
+    form = LoginForm(
+        email=user.email, password=MOD_DISABLED_USER_PASSWORD, remember_me=True
+    )
     rv = client.post(url_for("auth.login"), data=form.data, follow_redirects=True)
-    return rv
+    return rv, user
 
 
 def login_admin(client):
-    user = User.query.filter_by(is_administrator=True).first()
-    form = LoginForm(email=user.email, password=ADMIN_PASSWORD, remember_me=True)
+    user = User.query.filter_by(email=ADMIN_USER_EMAIL).first()
+    form = LoginForm(email=user.email, password=ADMIN_USER_PASSWORD, remember_me=True)
     rv = client.post(url_for("auth.login"), data=form.data, follow_redirects=False)
-    return rv
+    return rv, user
 
 
 def login_ac(client):
-    user = User.query.filter_by(ac_department_id=AC_DEPT).first()
-    form = LoginForm(email=user.email, password="horse", remember_me=True)
+    user = User.query.filter_by(email=AC_USER_EMAIL).first()
+    form = LoginForm(email=user.email, password=AC_USER_PASSWORD, remember_me=True)
     rv = client.post(url_for("auth.login"), data=form.data, follow_redirects=False)
-    return rv
+    return rv, user
 
 
 def process_form_data(form_dict):

OpenOversight/tests/routes/test_auth.py

@@ -17,6 +17,11 @@
 from OpenOversight.app.models.database import User
 from OpenOversight.app.utils.constants import KEY_OO_MAIL_SUBJECT_PREFIX
 from OpenOversight.tests.conftest import AC_DEPT
+from OpenOversight.tests.constants import (
+    GENERAL_USER_EMAIL,
+    MOD_DISABLED_USER_EMAIL,
+    UNCONFIRMED_USER_EMAIL,
+)
 from OpenOversight.tests.routes.route_helpers import (
     login_disabled_user,
     login_modified_disabled_user,
@@ -58,7 +63,7 @@ def test_route_login_required(route, client, mockdata):
 
 def test_valid_user_can_login(mockdata, client, session):
     with current_app.test_request_context():
-        rv = login_user(client)
+        rv, _ = login_user(client)
         assert rv.status_code == HTTPStatus.FOUND
         assert urlparse(rv.location).path == "/index"
 
@@ -74,7 +79,7 @@ def test_valid_user_can_login_with_email_differently_cased(mockdata, client, ses
 def test_invalid_user_cannot_login(mockdata, client, session):
     with current_app.test_request_context():
         form = LoginForm(
-            email="[email protected]", password="bruteforce", remember_me=True
+            email=UNCONFIRMED_USER_EMAIL, password="bruteforce", remember_me=True
         )
         rv = client.post(url_for("auth.login"), data=form.data)
         assert b"Invalid username or password." in rv.data
@@ -428,7 +433,7 @@ def test_unconfirmed_user_redirected_to_confirm_account(mockdata, client, sessio
 
 def test_disabled_user_cannot_login(mockdata, client, session):
     with current_app.test_request_context():
-        rv = login_disabled_user(client)
+        rv, _ = login_disabled_user(client)
         assert b"User has been disabled" in rv.data
 
 
@@ -438,11 +443,11 @@ def test_disabled_user_cannot_visit_pages_requiring_auth(mockdata, client, sessi
     # you'll get unexpected results if both tests run simultaneously.
     with current_app.test_request_context():
         # Temporarily enable account for login
-        user = User.query.filter_by(email="[email protected]").one()
+        user = User.query.filter_by(email=MOD_DISABLED_USER_EMAIL).one()
         user.is_disabled = False
         session.add(user)
 
-        rv = login_modified_disabled_user(client)
+        rv, _ = login_modified_disabled_user(client)
         assert b"/user/sam" in rv.data
 
         # Disable account again and check that login_required redirects user correctly
@@ -477,5 +482,5 @@ def test_user_can_change_dept_pref(mockdata, client, session):
 
         assert b"Updated!" in rv.data
 
-        user = User.query.filter_by(email="[email protected]").one()
+        user = User.query.filter_by(email=GENERAL_USER_EMAIL).one()
         assert user.dept_pref == AC_DEPT

OpenOversight/tests/routes/test_other.py

@@ -5,6 +5,7 @@
 from flask import current_app, url_for
 
 from OpenOversight.app.utils.constants import ENCODING_UTF_8, KEY_TIMEZONE
+from OpenOversight.tests.constants import GENERAL_USER_USERNAME
 from OpenOversight.tests.routes.route_helpers import login_user
 
 
@@ -32,9 +33,10 @@ def test_user_can_access_profile(mockdata, client, session):
         login_user(client)
 
         rv = client.get(
-            url_for("main.profile", username="test_user"), follow_redirects=True
+            url_for("main.profile", username=GENERAL_USER_USERNAME),
+            follow_redirects=True,
         )
-        assert "test_user" in rv.data.decode(ENCODING_UTF_8)
+        assert GENERAL_USER_USERNAME in rv.data.decode(ENCODING_UTF_8)
         # User email should not appear
         assert "User Email" not in rv.data.decode(ENCODING_UTF_8)
         # Toggle button should not appear for this non-admin user
@@ -48,7 +50,7 @@ def test_user_can_access_profile_differently_cased(mockdata, client, session):
         rv = client.get(
             url_for("main.profile", username="TEST_USER"), follow_redirects=True
         )
-        assert "test_user" in rv.data.decode(ENCODING_UTF_8)
+        assert GENERAL_USER_USERNAME in rv.data.decode(ENCODING_UTF_8)
         assert "User Email" not in rv.data.decode(ENCODING_UTF_8)
         assert "Edit User" not in rv.data.decode(ENCODING_UTF_8)