Skip to content

Commit

Permalink
Address sql alchemy warning (#1036)
Browse files Browse the repository at this point in the history
## Fixes issue
#1023

## Description of Changes
Address the warnings that pop up when populating rows in the DB by
giving the foreign key constraints names. The warning no longer shows up
when running `make dev`. I named the foreign keys according to the motif
that they were given by default:
<img width="368" alt="Screenshot 2023-08-22 at 3 31 50 PM"
src="https://github.com/lucyparsons/OpenOversight/assets/5885605/4682dfaa-7d35-4a9f-9ccb-dac9073a5d99">

```zsh
/usr/src/app/OpenOversight/app/../migrations/env.py:75: SAWarning: Cannot correctly sort tables; there are unresolvable cycles between tables "departments, users", which is usually caused by mutually dependent foreign key constraints.  Foreign key constraints involving these tables will not be considered; this warning may raise an error in a future release.
  context.run_migrations()
```

## 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] Ran `make dev` to create tables and populate data to validate
changes to migrations.
  • Loading branch information
michplunkett authored Aug 22, 2023
1 parent de93328 commit 485a1e3
Show file tree
Hide file tree
Showing 19 changed files with 268 additions and 122 deletions.
121 changes: 98 additions & 23 deletions OpenOversight/app/models/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,18 @@

officer_links = db.Table(
"officer_links",
db.Column("officer_id", db.Integer, db.ForeignKey("officers.id"), primary_key=True),
db.Column("link_id", db.Integer, db.ForeignKey("links.id"), primary_key=True),
db.Column(
"officer_id",
db.Integer,
db.ForeignKey("officers.id", name="officer_links_officer_id_fkey"),
primary_key=True,
),
db.Column(
"link_id",
db.Integer,
db.ForeignKey("links.id", name="officer_links_link_id_fkey"),
primary_key=True,
),
db.Column(
"created_at",
db.DateTime(timezone=True),
Expand All @@ -49,9 +59,17 @@

officer_incidents = db.Table(
"officer_incidents",
db.Column("officer_id", db.Integer, db.ForeignKey("officers.id"), primary_key=True),
db.Column(
"incident_id", db.Integer, db.ForeignKey("incidents.id"), primary_key=True
"officer_id",
db.Integer,
db.ForeignKey("officers.id", name="officer_incidents_officer_id_fkey"),
primary_key=True,
),
db.Column(
"incident_id",
db.Integer,
db.ForeignKey("incidents.id", name="officer_incidents_incident_id_fkey"),
primary_key=True,
),
db.Column(
"created_at",
Expand Down Expand Up @@ -162,7 +180,9 @@ class Job(BaseModel, TrackUpdates):
job_title = db.Column(db.String(255), index=True, unique=False, nullable=False)
is_sworn_officer = db.Column(db.Boolean, index=True, default=True)
order = db.Column(db.Integer, index=True, unique=False, nullable=False)
department_id = db.Column(db.Integer, db.ForeignKey("departments.id"))
department_id = db.Column(
db.Integer, db.ForeignKey("departments.id", name="jobs_department_id_fkey")
)
department = db.relationship("Department", backref="jobs")

__table_args__ = (
Expand Down Expand Up @@ -210,7 +230,9 @@ class Officer(BaseModel, TrackUpdates):
birth_year = db.Column(db.Integer, index=True, unique=False, nullable=True)
assignments = db.relationship("Assignment", back_populates="base_officer")
face = db.relationship("Face", backref="officer")
department_id = db.Column(db.Integer, db.ForeignKey("departments.id"))
department_id = db.Column(
db.Integer, db.ForeignKey("departments.id", name="officers_department_id_fkey")
)
department = db.relationship("Department", backref="officers")
unique_internal_identifier = db.Column(
db.String(50), index=True, unique=True, nullable=True
Expand Down Expand Up @@ -303,7 +325,12 @@ class Salary(BaseModel, TrackUpdates):
__tablename__ = "salaries"

id = db.Column(db.Integer, primary_key=True)
officer_id = db.Column(db.Integer, db.ForeignKey("officers.id", ondelete="CASCADE"))
officer_id = db.Column(
db.Integer,
db.ForeignKey(
"officers.id", name="salaries_officer_id_fkey", ondelete="CASCADE"
),
)
officer = db.relationship("Officer", back_populates="salaries")
salary = db.Column(db.Numeric, index=True, unique=False, nullable=False)
overtime_pay = db.Column(db.Numeric, index=True, unique=False, nullable=True)
Expand All @@ -318,12 +345,25 @@ class Assignment(BaseModel, TrackUpdates):
__tablename__ = "assignments"

id = db.Column(db.Integer, primary_key=True)
officer_id = db.Column(db.Integer, db.ForeignKey("officers.id", ondelete="CASCADE"))
officer_id = db.Column(
db.Integer,
db.ForeignKey(
"officers.id", name="assignments_officer_id_fkey", ondelete="CASCADE"
),
)
base_officer = db.relationship("Officer", back_populates="assignments")
star_no = db.Column(db.String(120), index=True, unique=False, nullable=True)
job_id = db.Column(db.Integer, db.ForeignKey("jobs.id"), nullable=False)
job_id = db.Column(
db.Integer,
db.ForeignKey("jobs.id", name="assignments_job_id_fkey"),
nullable=False,
)
job = db.relationship("Job")
unit_id = db.Column(db.Integer, db.ForeignKey("unit_types.id"), nullable=True)
unit_id = db.Column(
db.Integer,
db.ForeignKey("unit_types.id", name="assignments_unit_id_fkey"),
nullable=True,
)
unit = db.relationship("Unit")
start_date = db.Column(db.Date, index=True, unique=False, nullable=True)
resign_date = db.Column(db.Date, index=True, unique=False, nullable=True)
Expand All @@ -337,7 +377,10 @@ class Unit(BaseModel, TrackUpdates):

id = db.Column(db.Integer, primary_key=True)
description = db.Column(db.String(120), index=True, unique=False)
department_id = db.Column(db.Integer, db.ForeignKey("departments.id"))
department_id = db.Column(
db.Integer,
db.ForeignKey("departments.id", name="unit_types_department_id_fkey"),
)
department = db.relationship(
"Department", backref="unit_types", order_by="Unit.description.asc()"
)
Expand All @@ -350,7 +393,9 @@ class Face(BaseModel, TrackUpdates):
__tablename__ = "faces"

id = db.Column(db.Integer, primary_key=True)
officer_id = db.Column(db.Integer, db.ForeignKey("officers.id"))
officer_id = db.Column(
db.Integer, db.ForeignKey("officers.id", name="faces_officer_id_fkey")
)
img_id = db.Column(
db.Integer,
db.ForeignKey(
Expand Down Expand Up @@ -404,7 +449,10 @@ class Image(BaseModel, TrackUpdates):

is_tagged = db.Column(db.Boolean, default=False, unique=False, nullable=True)

department_id = db.Column(db.Integer, db.ForeignKey("departments.id"))
department_id = db.Column(
db.Integer,
db.ForeignKey("departments.id", name="raw_images_department_id_fkey"),
)
department = db.relationship("Department", backref="raw_images")

def __repr__(self):
Expand All @@ -414,9 +462,17 @@ def __repr__(self):
incident_links = db.Table(
"incident_links",
db.Column(
"incident_id", db.Integer, db.ForeignKey("incidents.id"), primary_key=True
"incident_id",
db.Integer,
db.ForeignKey("incidents.id", name="incident_links_incident_id_fkey"),
primary_key=True,
),
db.Column(
"link_id",
db.Integer,
db.ForeignKey("links.id", name="incident_links_link_id_fkey"),
primary_key=True,
),
db.Column("link_id", db.Integer, db.ForeignKey("links.id"), primary_key=True),
db.Column(
"created_at",
db.DateTime(timezone=True),
Expand All @@ -429,12 +485,17 @@ def __repr__(self):
incident_license_plates = db.Table(
"incident_license_plates",
db.Column(
"incident_id", db.Integer, db.ForeignKey("incidents.id"), primary_key=True
"incident_id",
db.Integer,
db.ForeignKey("incidents.id", name="incident_license_plates_incident_id_fkey"),
primary_key=True,
),
db.Column(
"license_plate_id",
db.Integer,
db.ForeignKey("license_plates.id"),
db.ForeignKey(
"license_plates.id", name="incident_license_plates_license_plate_id_fkey"
),
primary_key=True,
),
db.Column(
Expand All @@ -449,10 +510,16 @@ def __repr__(self):
incident_officers = db.Table(
"incident_officers",
db.Column(
"incident_id", db.Integer, db.ForeignKey("incidents.id"), primary_key=True
"incident_id",
db.Integer,
db.ForeignKey("incidents.id", name="incident_officers_incident_id_fkey"),
primary_key=True,
),
db.Column(
"officers_id", db.Integer, db.ForeignKey("officers.id"), primary_key=True
"officers_id",
db.Integer,
db.ForeignKey("officers.id", name="incident_officers_officers_id_fkey"),
primary_key=True,
),
db.Column(
"created_at",
Expand Down Expand Up @@ -545,7 +612,9 @@ class Incident(BaseModel, TrackUpdates):
time = db.Column(db.Time, unique=False, index=True)
report_number = db.Column(db.String(50), index=True)
description = db.Column(db.Text(), nullable=True)
address_id = db.Column(db.Integer, db.ForeignKey("locations.id"))
address_id = db.Column(
db.Integer, db.ForeignKey("locations.id", name="incidents_address_id_fkey")
)
address = db.relationship("Location", backref="incidents")
license_plates = db.relationship(
"LicensePlate",
Expand All @@ -565,7 +634,9 @@ class Incident(BaseModel, TrackUpdates):
lazy="subquery",
backref=db.backref("incidents"),
)
department_id = db.Column(db.Integer, db.ForeignKey("departments.id"))
department_id = db.Column(
db.Integer, db.ForeignKey("departments.id", name="incidents_department_id_fkey")
)
department = db.relationship("Department", backref="incidents", lazy=True)


Expand All @@ -578,13 +649,17 @@ class User(UserMixin, BaseModel):
confirmed = db.Column(db.Boolean, default=False)
approved = db.Column(db.Boolean, default=False)
is_area_coordinator = db.Column(db.Boolean, default=False)
ac_department_id = db.Column(db.Integer, db.ForeignKey("departments.id"))
ac_department_id = db.Column(
db.Integer, db.ForeignKey("departments.id", name="users_ac_department_id_fkey")
)
ac_department = db.relationship(
"Department", backref="coordinators", foreign_keys=[ac_department_id]
)
is_administrator = db.Column(db.Boolean, default=False)
is_disabled = db.Column(db.Boolean, default=False)
dept_pref = db.Column(db.Integer, db.ForeignKey("departments.id"))
dept_pref = db.Column(
db.Integer, db.ForeignKey("departments.id", name="users_dept_pref_fkey")
)
dept_pref_rel = db.relationship("Department", foreign_keys=[dept_pref])

# creator backlinks
Expand Down
80 changes: 40 additions & 40 deletions OpenOversight/app/utils/forms.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import datetime
from typing import Any, Union
from typing import Union

from sqlalchemy import or_
from sqlalchemy.orm import selectinload
Expand Down Expand Up @@ -28,14 +28,16 @@
db,
)
from OpenOversight.app.utils.choices import GENDER_CHOICES, RACE_CHOICES
from OpenOversight.app.utils.general import get_or_create


def add_new_assignment(officer_id: int, form: AssignmentForm, current_user: User):
if form.unit.data:
unit_id = form.unit.data.id
else:
unit_id = None
def if_exists_or_none(val: Union[str, None]) -> Union[str, None]:
return val if val else None


def add_new_assignment(
officer_id: int, form: AssignmentForm, current_user: User
) -> None:
unit_id = form.unit.data.id if form.unit.data else None

job = Job.query.filter_by(
department_id=form.job_title.data.department_id,
Expand All @@ -54,7 +56,7 @@ def add_new_assignment(officer_id: int, form: AssignmentForm, current_user: User
db.session.commit()


def add_officer_profile(form: AddOfficerForm, current_user: User):
def add_officer_profile(form: AddOfficerForm, current_user: User) -> Officer:
officer = Officer(
first_name=form.first_name.data,
last_name=form.last_name.data,
Expand All @@ -69,10 +71,7 @@ def add_officer_profile(form: AddOfficerForm, current_user: User):
db.session.add(officer)
db.session.commit()

if form.unit.data:
officer_unit = form.unit.data
else:
officer_unit = None
officer_unit = form.unit.data if form.unit.data else None

assignment = Assignment(
base_officer=officer,
Expand All @@ -84,11 +83,9 @@ def add_officer_profile(form: AddOfficerForm, current_user: User):
db.session.add(assignment)
if form.links.data:
for link in form.data["links"]:
# don't try to create with a blank string
if link["url"]:
li, _ = get_or_create(db.session, Link, **link)
if li:
officer.links.append(li)
li = get_or_create_link_from_form(link, current_user)
officer.links.append(li)
if form.notes.data:
for note in form.data["notes"]:
# don't try to create with a blank string
Expand Down Expand Up @@ -126,17 +123,14 @@ def add_officer_profile(form: AddOfficerForm, current_user: User):
return officer


def create_description(self, form: TextForm, current_user: User):
def create_description(self, form: TextForm, current_user: User) -> Description:
return Description(
text_contents=form.text_contents.data,
officer_id=form.officer_id.data,
)


def create_incident(self, form: IncidentForm, current_user: User):
def if_exists_or_none(val: Union[str, Any]):
return val if val else None

def create_incident(self, form: IncidentForm, current_user: User) -> Incident:
fields = {
"date": form.date_field.data,
"time": form.time_field.data,
Expand Down Expand Up @@ -191,23 +185,8 @@ def if_exists_or_none(val: Union[str, Any]):

if "links" in form.data:
for link in form.data["links"]:
# don't try to create with a blank string
if link["url"]:
li = Link.query.filter_by(
author=if_exists_or_none(link["author"]),
link_type=if_exists_or_none(link["link_type"]),
title=if_exists_or_none(link["title"]),
url=if_exists_or_none(link["url"]),
).first()
if not li:
li = Link(
author=if_exists_or_none(link["author"]),
description=if_exists_or_none(link["description"]),
link_type=if_exists_or_none(link["link_type"]),
title=if_exists_or_none(link["title"]),
url=if_exists_or_none(link["url"]),
)
db.session.add(li)
li = get_or_create_link_from_form(link, current_user)
fields["links"].append(li)

return Incident(
Expand All @@ -223,14 +202,14 @@ def if_exists_or_none(val: Union[str, Any]):
)


def create_note(self, form: TextForm, current_user: User):
def create_note(self, form: TextForm, current_user: User) -> Note:
return Note(
text_contents=form.text_contents.data,
officer_id=form.officer_id.data,
)


def edit_existing_assignment(assignment, form: AssignmentForm):
def edit_existing_assignment(assignment, form: AssignmentForm) -> Assignment:
assignment.star_no = form.star_no.data

job = form.job_title.data
Expand All @@ -249,7 +228,28 @@ def edit_existing_assignment(assignment, form: AssignmentForm):
return assignment


def edit_officer_profile(officer, form: EditOfficerForm):
def get_or_create_link_from_form(link_form, current_user: User) -> Union[Link, None]:
link = None
if link_form["url"]:
link = Link.query.filter_by(
author=if_exists_or_none(link_form["author"]),
link_type=if_exists_or_none(link_form["link_type"]),
title=if_exists_or_none(link_form["title"]),
url=if_exists_or_none(link_form["url"]),
).first()
if not link:
link = Link(
author=if_exists_or_none(link_form["author"]),
description=if_exists_or_none(link_form["description"]),
link_type=if_exists_or_none(link_form["link_type"]),
title=if_exists_or_none(link_form["title"]),
url=if_exists_or_none(link_form["url"]),
)
db.session.add(link)
return link


def edit_officer_profile(officer, form: EditOfficerForm) -> Officer:
for field, data in form.data.items():
setattr(officer, field, data)

Expand Down
Loading

0 comments on commit 485a1e3

Please sign in to comment.