Move models to its own package (#963)

## Fixes issue
#959

## Description of Changes
Move `models` to its own package to prevent circular dependencies and
address straggling relative imports.

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

OpenOversight/app/__init__.py

@@ -17,8 +17,9 @@
 from flask_wtf.csrf import CSRFProtect
 from markupsafe import Markup
 
-from OpenOversight.app.config import config
 from OpenOversight.app.email_client import EmailClient
+from OpenOversight.app.models.config import config
+from OpenOversight.app.models.database import db
 from OpenOversight.app.utils.constants import MEGABYTE, SERVICE_ACCOUNT_FILE
 
 
@@ -40,7 +41,6 @@ def create_app(config_name="default"):
     app = Flask(__name__)
     # Creates and adds the Config object of the correct type to app.config
     app.config.from_object(config[config_name])
-    from .models import db
 
     bootstrap.init_app(app)
     csrf.init_app(app)
@@ -99,11 +99,19 @@ def _handler_method(e):
         return _handler_method
 
     error_handlers = [
-        (HTTPStatus.FORBIDDEN, "Forbidden", "403.html"),
-        (HTTPStatus.NOT_FOUND, "Not found", "404.html"),
-        (HTTPStatus.REQUEST_ENTITY_TOO_LARGE, "File too large", "413.html"),
-        (HTTPStatus.TOO_MANY_REQUESTS, "Too many requests", "429.html"),
-        (HTTPStatus.INTERNAL_SERVER_ERROR, "Internal Server Error", "500.html"),
+        (HTTPStatus.FORBIDDEN, HTTPStatus.FORBIDDEN.phrase, "403.html"),
+        (HTTPStatus.NOT_FOUND, HTTPStatus.NOT_FOUND.phrase, "404.html"),
+        (
+            HTTPStatus.REQUEST_ENTITY_TOO_LARGE,
+            HTTPStatus.REQUEST_ENTITY_TOO_LARGE.phrase,
+            "413.html",
+        ),
+        (HTTPStatus.TOO_MANY_REQUESTS, HTTPStatus.TOO_MANY_REQUESTS.phrase, "429.html"),
+        (
+            HTTPStatus.INTERNAL_SERVER_ERROR,
+            HTTPStatus.INTERNAL_SERVER_ERROR.phrase,
+            "500.html",
+        ),
     ]
     for code, error, template in error_handlers:
         # Pass generated errorhandler function to @app.errorhandler decorator

OpenOversight/app/auth/forms.py

@@ -9,8 +9,8 @@
 from wtforms.validators import DataRequired, Email, EqualTo, Length, Optional, Regexp
 from wtforms_sqlalchemy.fields import QuerySelectField
 
-from ..models import User
-from ..utils.db import dept_choices
+from OpenOversight.app.models.database import User
+from OpenOversight.app.utils.db import dept_choices
 
 
 class LoginForm(Form):

OpenOversight/app/auth/views.py

@@ -11,21 +11,9 @@
 )
 from flask_login import current_user, login_required, login_user, logout_user
 
-from OpenOversight.app.email_client import (
-    AdministratorApprovalEmail,
-    ChangeEmailAddressEmail,
-    ConfirmAccountEmail,
-    ConfirmedUserEmail,
-    EmailClient,
-    ResetPasswordEmail,
-)
-from OpenOversight.app.models import User, db
-from OpenOversight.app.utils.forms import set_dynamic_default
-from OpenOversight.app.utils.general import validate_redirect_url
-
-from .. import sitemap
-from . import auth
-from .forms import (
+from OpenOversight.app import sitemap
+from OpenOversight.app.auth import auth
+from OpenOversight.app.auth.forms import (
     ChangeDefaultDepartmentForm,
     ChangeEmailForm,
     ChangePasswordForm,
@@ -35,7 +23,18 @@
     PasswordResetRequestForm,
     RegistrationForm,
 )
-from .utils import admin_required
+from OpenOversight.app.email_client import EmailClient
+from OpenOversight.app.models.database import User, db
+from OpenOversight.app.models.emails import (
+    AdministratorApprovalEmail,
+    ChangeEmailAddressEmail,
+    ConfirmAccountEmail,
+    ConfirmedUserEmail,
+    ResetPasswordEmail,
+)
+from OpenOversight.app.utils.auth import admin_required
+from OpenOversight.app.utils.forms import set_dynamic_default
+from OpenOversight.app.utils.general import validate_redirect_url
 
 
 js_loads = ["js/zxcvbn.js", "js/password.js"]

OpenOversight/app/commands.py

@@ -10,18 +10,27 @@
 from flask import current_app
 from flask.cli import with_appcontext
 
+from OpenOversight.app.csv_imports import import_csv_files
+from OpenOversight.app.models.database import (
+    Assignment,
+    Department,
+    Image,
+    Job,
+    Officer,
+    Salary,
+    Unit,
+    User,
+    db,
+)
 from OpenOversight.app.utils.constants import ENCODING_UTF_8
 from OpenOversight.app.utils.db import get_officer
 from OpenOversight.app.utils.general import normalize_gender, prompt_yes_no, str_is_true
 
-from .csv_imports import import_csv_files
-from .models import Assignment, Department, Job, Officer, Salary, User, db
-
 
 @click.command()
 @with_appcontext
 def make_admin_user():
-    """Add confirmed administrator account"""
+    """Add confirmed administrator account."""
     while True:
         username = input("Username: ")
         user = User.by_username(username).one_or_none()
@@ -64,9 +73,7 @@ def make_admin_user():
 @click.command()
 @with_appcontext
 def link_images_to_department():
-    """Link existing images to first department"""
-    from app.models import Image, db
-
+    """Link existing images to first department."""
     images = Image.query.all()
     print("Linking images to first department:")
     for image in images:
@@ -81,9 +88,7 @@ def link_images_to_department():
 @click.command()
 @with_appcontext
 def link_officers_to_department():
-    """Links officers and unit_ids to first department"""
-    from app.models import Officer, Unit, db
-
+    """Links officers and unit_ids to first department."""
     officers = Officer.query.all()
     units = Unit.query.all()
 
@@ -159,35 +164,40 @@ def row_has_data(row, required_fields, optional_fields):
     return False
 
 
-def set_field_from_row(row, obj, attribute, allow_blank=True, fieldname=None):
-    fieldname = fieldname or attribute
-    if fieldname in row and (row[fieldname] or allow_blank):
+def set_field_from_row(row, obj, attribute, allow_blank=True, field_name=None):
+    field_name = field_name or attribute
+    if field_name in row and (row[field_name] or allow_blank):
         try:
-            val = datetime.strptime(row[fieldname], "%Y-%m-%d").date()
+            val = datetime.strptime(row[field_name], "%Y-%m-%d").date()
         except ValueError:
-            val = row[fieldname]
+            val = row[field_name]
             if attribute == "gender":
                 val = normalize_gender(val)
         setattr(obj, attribute, val)
 
 
 def update_officer_from_row(row, officer, update_static_fields=False):
-    def update_officer_field(fieldname):
-        if fieldname not in row:
+    def update_officer_field(officer_field_name):
+        if officer_field_name not in row:
             return
 
-        if fieldname == "gender":
-            row[fieldname] = normalize_gender(row[fieldname])
+        if officer_field_name == "gender":
+            row[officer_field_name] = normalize_gender(row[officer_field_name])
 
-        if row[fieldname] and getattr(officer, fieldname) != row[fieldname]:
+        if (
+            row[officer_field_name]
+            and getattr(officer, officer_field_name) != row[officer_field_name]
+        ):
 
             ImportLog.log_change(
                 officer,
                 "Updated {}: {} --> {}".format(
-                    fieldname, getattr(officer, fieldname), row[fieldname]
+                    officer_field_name,
+                    getattr(officer, officer_field_name),
+                    row[officer_field_name],
                 ),
             )
-            setattr(officer, fieldname, row[fieldname])
+            setattr(officer, officer_field_name, row[officer_field_name])
 
     # Name and gender are the only potentially changeable fields, so update those
     update_officer_field("last_name")
@@ -204,42 +214,47 @@ def update_officer_field(fieldname):
         "employment_date",
         "birth_year",
     ]
-    for fieldname in static_fields:
-        if fieldname in row:
-            if row[fieldname] == "":
-                row[fieldname] = None
-            old_value = getattr(officer, fieldname)
-            # If we're expecting a date type, attempt to parse row[fieldname] as a datetime
-            # This also normalizes all date formats, ensuring the following comparison works properly
+    for field_name in static_fields:
+        if field_name in row:
+            if row[field_name] == "":
+                row[field_name] = None
+            old_value = getattr(officer, field_name)
+            # If we're expecting a date type, attempt to parse row[field_name] as a
+            # datetime. This normalizes all date formats, ensuring the following
+            # comparison works properly
             if isinstance(old_value, (date, datetime)):
                 try:
-                    new_value = parse(row[fieldname])
+                    new_value = parse(row[field_name])
                     if isinstance(old_value, date):
                         new_value = new_value.date()
                 except Exception as e:
-                    msg = 'Field {} is a date-type, but "{}" was specified for Officer {} {} and cannot be parsed as a date-type.\nError message from dateutil: {}'.format(
-                        fieldname,
-                        row[fieldname],
-                        officer.first_name,
-                        officer.last_name,
-                        e,
+                    msg = (
+                        'Field {} is a date-type, but "{}" was specified for '
+                        "Officer {} {} and cannot be parsed as a date-type.\nError "
+                        "message from dateutil: {}".format(
+                            field_name,
+                            row[field_name],
+                            officer.first_name,
+                            officer.last_name,
+                            e,
+                        )
                     )
                     raise Exception(msg)
             else:
-                new_value = row[fieldname]
+                new_value = row[field_name]
             if old_value is None:
-                update_officer_field(fieldname)
+                update_officer_field(field_name)
             elif str(old_value) != str(new_value):
                 msg = "Officer {} {} has differing {} field. Old: {}, new: {}".format(
                     officer.first_name,
                     officer.last_name,
-                    fieldname,
+                    field_name,
                     old_value,
                     new_value,
                 )
                 if update_static_fields:
                     print(msg)
-                    update_officer_field(fieldname)
+                    update_officer_field(field_name)
                 else:
                     raise Exception(msg)
 
@@ -270,7 +285,8 @@ def create_officer_from_row(row, department_id):
 
 
 def is_equal(a, b):
-    """Run an exhaustive equality check, originally to compare a sqlalchemy result object of various types to a csv string
+    """Run an exhaustive equality check, originally to compare a sqlalchemy result
+    object of various types to a csv string.
     Note: Stringifying covers object cases (as in the datetime example below)
     >>> is_equal("1", 1)  # string == int
     True
@@ -445,7 +461,8 @@ def process_salary(row, officer, compare=False):
 @click.option(
     "--update-by-name",
     is_flag=True,
-    help="update officers by first and last name (useful when star_no or unique_internal_identifier are not available)",
+    help="update officers by first and last name (useful when star_no or "
+    "unique_internal_identifier are not available)",
 )
 @click.option(
     "--update-static-fields",
@@ -484,7 +501,8 @@ def bulk_add_officers(filename, no_create, update_by_name, update_static_fields)
             and "unique_internal_identifier" not in csvfile.fieldnames
         ):
             raise Exception(
-                "CSV file must include either badge numbers or unique identifiers for officers"
+                "CSV file must include either badge numbers or unique identifiers for "
+                "officers"
             )
 
         for row in csvfile:
@@ -498,7 +516,7 @@ def bulk_add_officers(filename, no_create, update_by_name, update_static_fields)
                     raise Exception("Department ID {} not found".format(department_id))
 
             if not update_by_name:
-                # check for existing officer based on unique ID or name/badge
+                # Check for existing officer based on unique ID or name/badge
                 if (
                     "unique_internal_identifier" in csvfile.fieldnames
                     and row["unique_internal_identifier"]

OpenOversight/app/csv_imports.py

@@ -4,7 +4,18 @@
 
 from sqlalchemy.exc import SQLAlchemyError
 
-from .model_imports import (
+from OpenOversight.app.models.database import (
+    Assignment,
+    Department,
+    Incident,
+    Job,
+    Link,
+    Officer,
+    Salary,
+    Unit,
+    db,
+)
+from OpenOversight.app.models.database_imports import (
     create_assignment_from_dict,
     create_incident_from_dict,
     create_link_from_dict,
@@ -18,17 +29,6 @@
     update_officer_from_dict,
     update_salary_from_dict,
 )
-from .models import (
-    Assignment,
-    Department,
-    Incident,
-    Job,
-    Link,
-    Officer,
-    Salary,
-    Unit,
-    db,
-)
 
 
 def _create_or_update_model(