From 6a2739382fdad53b98ec9dc6594638a1fd94bc9f Mon Sep 17 00:00:00 2001 From: Michael Plunkett <5885605+michplunkett@users.noreply.github.com> Date: Thu, 3 Aug 2023 14:20:06 -0500 Subject: [PATCH] Add `created_at` and `created_by` columns (#1002) ## Fixes issue https://github.com/lucyparsons/OpenOversight/issues/928 ## Description of Changes Add `created_by` and `created_at` columns to tables so that we can better audit things happening in the application. ## 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.
DB migration output ```console $ flask db stamp head [2023-08-01 18:18:34,782] INFO in __init__: OpenOversight startup INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running stamp_revision -> 18f43ac4622f $ flask db upgrade [2023-08-01 18:18:49,052] INFO in __init__: OpenOversight startup INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running upgrade 18f43ac4622f -> b429700e2dd2, add created_by and created_at columns $ flask db downgrade [2023-08-01 18:18:54,540] INFO in __init__: OpenOversight startup INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running downgrade b429700e2dd2 -> 18f43ac4622f, add created_by and created_at columns $ flask db upgrade [2023-08-01 18:19:04,407] INFO in __init__: OpenOversight startup INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running upgrade 18f43ac4622f -> b429700e2dd2, add created_by and created_at columns $ ```
--- OpenOversight/app/csv_imports.py | 8 +- OpenOversight/app/main/downloads.py | 4 +- OpenOversight/app/main/forms.py | 67 +- OpenOversight/app/main/model_view.py | 29 +- OpenOversight/app/main/views.py | 44 +- OpenOversight/app/models/config.py | 3 +- OpenOversight/app/models/database.py | 187 +++++- OpenOversight/app/models/database_imports.py | 31 +- OpenOversight/app/models/emails.py | 22 +- .../app/templates/incident_detail.html | 36 +- OpenOversight/app/utils/cloud.py | 2 +- OpenOversight/app/utils/constants.py | 6 + OpenOversight/app/utils/db.py | 12 +- OpenOversight/app/utils/forms.py | 15 +- ...c_add_created_by_and_created_at_columns.py | 597 ++++++++++++++++++ OpenOversight/tests/conftest.py | 227 ++++--- OpenOversight/tests/routes/route_helpers.py | 23 +- OpenOversight/tests/routes/test_auth.py | 85 +-- .../tests/routes/test_descriptions.py | 66 +- .../tests/routes/test_image_tagging.py | 16 +- OpenOversight/tests/routes/test_incidents.py | 131 ++-- OpenOversight/tests/routes/test_notes.py | 50 +- .../routes/test_officer_and_department.py | 166 +++-- OpenOversight/tests/routes/test_user_api.py | 76 +-- OpenOversight/tests/test_commands.py | 69 +- OpenOversight/tests/test_csvs/incidents.csv | 2 +- OpenOversight/tests/test_csvs/links.csv | 2 +- OpenOversight/tests/test_models.py | 4 +- docs/advanced_csv_import.rst | 8 +- 29 files changed, 1518 insertions(+), 470 deletions(-) create mode 100644 OpenOversight/migrations/versions/2023-08-01-1905_b38c133bed3c_add_created_by_and_created_at_columns.py diff --git a/OpenOversight/app/csv_imports.py b/OpenOversight/app/csv_imports.py index 7fc86a828..8b365dcc1 100644 --- a/OpenOversight/app/csv_imports.py +++ b/OpenOversight/app/csv_imports.py @@ -234,7 +234,7 @@ def _handle_assignments_csv( csv_reader = rows else: existing_assignments = ( - Assignment.query.join(Assignment.baseofficer) + Assignment.query.join(Assignment.base_officer) .filter(Officer.department_id == department_id) .all() ) @@ -376,8 +376,8 @@ def _handle_incidents_csv( "city", "state", "zip_code", - "creator_id", - "last_updated_id", + "created_by", + "last_updated_by", "officer_ids", "license_plates", ], @@ -442,7 +442,7 @@ def _handle_links_csv( "link_type", "description", "author", - "creator_id", + "created_by", "officer_ids", "incident_ids", ], diff --git a/OpenOversight/app/main/downloads.py b/OpenOversight/app/main/downloads.py index 599ebfe28..b925e0bf6 100644 --- a/OpenOversight/app/main/downloads.py +++ b/OpenOversight/app/main/downloads.py @@ -114,7 +114,7 @@ def officer_record_maker(officer: Officer) -> _Record: def assignment_record_maker(assignment: Assignment) -> _Record: - officer = assignment.baseofficer + officer = assignment.base_officer return { "id": assignment.id, "officer id": assignment.officer_id, @@ -159,7 +159,7 @@ def descriptions_record_maker(description: Description) -> _Record: return { "id": description.id, "text_contents": description.text_contents, - "creator_id": description.creator_id, + "created_by": description.created_by, "officer_id": description.officer_id, "created_at": description.created_at, "updated_at": description.updated_at, diff --git a/OpenOversight/app/main/forms.py b/OpenOversight/app/main/forms.py index 10a9648ee..f6a6b2eb2 100644 --- a/OpenOversight/app/main/forms.py +++ b/OpenOversight/app/main/forms.py @@ -138,6 +138,11 @@ class FaceTag(Form): dataY = IntegerField("dataY", validators=[InputRequired()]) dataWidth = IntegerField("dataWidth", validators=[InputRequired()]) dataHeight = IntegerField("dataHeight", validators=[InputRequired()]) + created_by = HiddenField( + validators=[ + DataRequired(message="Face Tags must have a valid user ID for creating.") + ] + ) class AssignmentForm(Form): @@ -162,6 +167,11 @@ class AssignmentForm(Form): resign_date = DateField( "Assignment end date", validators=[Optional(), validate_end_date] ) + created_by = HiddenField( + validators=[ + DataRequired(message="Assignments must have a valid user ID for creating.") + ] + ) class SalaryForm(Form): @@ -177,6 +187,11 @@ class SalaryForm(Form): validators=[NumberRange(min=1900, max=2100)], ) is_fiscal_year = BooleanField("Is fiscal year?", default=False) + created_by = HiddenField( + validators=[ + DataRequired(message="Salaries must have a valid user ID for creating.") + ] + ) def validate(self, extra_validators=None): if not self.data.get("salary") and not self.data.get("overtime_pay"): @@ -207,6 +222,11 @@ class DepartmentForm(Form): jobs = FieldList( StringField("Job", default="", validators=[Regexp(r"\w*")]), label="Ranks" ) + created_by = HiddenField( + validators=[ + DataRequired(message="Departments must have a valid user ID for creating.") + ] + ) submit = SubmitField(label="Add") @@ -236,7 +256,11 @@ class LinkForm(Form): default="", validators=[AnyOf(allowed_values(LINK_CHOICES))], ) - creator_id = HiddenField(validators=[DataRequired(message="Not a valid user ID")]) + created_by = HiddenField( + validators=[ + DataRequired(message="Links must have a valid user ID for creating.") + ] + ) def validate(self, extra_validators=None): success = super(LinkForm, self).validate(extra_validators=extra_validators) @@ -270,7 +294,11 @@ class TextForm(EditTextForm): officer_id = HiddenField( validators=[DataRequired(message="Not a valid officer ID")] ) - creator_id = HiddenField(validators=[DataRequired(message="Not a valid user ID")]) + created_by = HiddenField( + validators=[ + DataRequired(message="Text fields must have a valid user ID for creating.") + ] + ) class AddOfficerForm(Form): @@ -356,6 +384,11 @@ class AddOfficerForm(Form): min_entries=1, widget=BootstrapListWidget(), ) + created_by = HiddenField( + validators=[ + DataRequired(message="Officers must have a valid user ID for creating.") + ] + ) submit = SubmitField(label="Add") @@ -417,6 +450,11 @@ class AddUnitForm(Form): query_factory=dept_choices, get_label="name", ) + created_by = HiddenField( + validators=[ + DataRequired(message="Units must have a valid user ID for creating.") + ] + ) submit = SubmitField(label="Add") @@ -467,6 +505,11 @@ class LocationForm(Form): Regexp(r"^\d{5}$", message="Zip codes must have 5 digits."), ], ) + created_by = HiddenField( + validators=[ + DataRequired(message="Locations must have a valid user ID for creating.") + ] + ) class LicensePlateForm(Form): @@ -476,6 +519,13 @@ class LicensePlateForm(Form): choices=STATE_CHOICES, validators=[AnyOf(allowed_values(STATE_CHOICES))], ) + created_by = HiddenField( + validators=[ + DataRequired( + message="License Plates must have a valid user ID for creating." + ) + ] + ) def validate_state(self, field): if self.number.data != "" and field.data == "": @@ -545,11 +595,16 @@ class IncidentForm(DateFieldForm): min_entries=1, widget=BootstrapListWidget(), ) - creator_id = HiddenField( - validators=[DataRequired(message="Incidents must have a creator id.")] + created_by = HiddenField( + validators=[DataRequired(message="Incidents must have a user id for creating.")] ) - last_updated_id = HiddenField( - validators=[DataRequired(message="Incidents must have a user id for editing.")] + last_updated_by = HiddenField( + validators=[DataRequired(message="Incidents must have a user ID for editing.")] + ) + last_updated_at = HiddenField( + validators=[ + DataRequired(message="Incidents must have a timestamp for editing.") + ] ) submit = SubmitField(label="Submit") diff --git a/OpenOversight/app/main/model_view.py b/OpenOversight/app/main/model_view.py index 15b351d4f..02a64842f 100644 --- a/OpenOversight/app/main/model_view.py +++ b/OpenOversight/app/main/model_view.py @@ -63,10 +63,12 @@ def new(self, form=None): add_department_query(form, current_user) if getattr(current_user, "dept_pref_rel", None): set_dynamic_default(form.department, current_user.dept_pref_rel) - if hasattr(form, "creator_id") and not form.creator_id.data: - form.creator_id.data = current_user.get_id() - if hasattr(form, "last_updated_id"): - form.last_updated_id.data = current_user.get_id() + if hasattr(form, "created_by") and not form.created_by.data: + form.created_by.data = current_user.get_id() + # TODO: Determine whether creating counts as updating, seems redundant + if hasattr(form, "last_updated_by"): + form.last_updated_by.data = current_user.get_id() + form.last_updated_at.data = datetime.datetime.now() if form.validate_on_submit(): new_obj = self.create_function(form) @@ -92,19 +94,20 @@ def edit(self, obj_id, form=None): if not form: form = self.get_edit_form(obj) - # if the object doesn't have a creator id set, st it to current user + # if the object doesn't have a creator id set it to current user if ( - hasattr(obj, "creator_id") - and hasattr(form, "creator_id") - and getattr(obj, "creator_id") + hasattr(obj, "created_by") + and hasattr(form, "created_by") + and getattr(obj, "created_by") ): - form.creator_id.data = obj.creator_id - elif hasattr(form, "creator_id"): - form.creator_id.data = current_user.get_id() + form.created_by.data = obj.created_by + elif hasattr(form, "created_by"): + form.created_by.data = current_user.get_id() # if the object keeps track of who updated it last, set to current user - if hasattr(form, "last_updated_id"): - form.last_updated_id.data = current_user.get_id() + if hasattr(obj, "last_updated_by") and hasattr(form, "last_updated_by"): + form.last_updated_by.data = current_user.get_id() + form.last_updated_at.data = datetime.datetime.now() if hasattr(form, "department"): add_department_query(form, current_user) diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index 6ab07df34..908e48f0b 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -324,6 +324,7 @@ def sitemap_officers(): @ac_or_admin_required def add_assignment(officer_id): form = AssignmentForm() + form.created_by.data = current_user.get_id() officer = Officer.query.filter_by(id=officer_id).first() form.job_title.query = ( Job.query.filter_by(department_id=officer.department_id) @@ -402,6 +403,7 @@ def edit_assignment(officer_id, assignment_id): @ac_or_admin_required def add_salary(officer_id): form = SalaryForm() + form.created_by.data = current_user.get_id() officer = Officer.query.filter_by(id=officer_id).first() if not officer: flash("Officer not found") @@ -498,7 +500,7 @@ def classify_submission(image_id, contains_cops): if image.contains_cops is not None and not current_user.is_administrator: flash("Only administrator can re-classify image") return redirect(redirect_url()) - image.user_id = current_user.get_id() + image.created_by = current_user.get_id() if contains_cops == 1: image.contains_cops = True elif contains_cops == 0: @@ -519,6 +521,7 @@ def classify_submission(image_id, contains_cops): @admin_required def add_department(): form = DepartmentForm() + form.created_by = current_user.get_id() if form.validate_on_submit(): department_does_not_exist = ( Department.query.filter_by( @@ -532,6 +535,7 @@ def add_department(): name=form.name.data, short_name=form.short_name.data, state=form.state.data, + created_by=current_user.get_id(), ) db.session.add(department) db.session.flush() @@ -578,6 +582,7 @@ def edit_department(department_id): previous_name = department.name form = EditDepartmentForm(obj=department) original_ranks = department.jobs + form.created_by.data = department.created_by if form.validate_on_submit(): if form.name.data != previous_name: does_already_department_exist = ( @@ -898,10 +903,10 @@ def get_dept_units(department_id=None): @login_required @ac_or_admin_required def add_officer(): - jsloads = ["js/dynamic_lists.js", "js/add_officer.js"] form = AddOfficerForm() + form.created_by.data = current_user.get_id() for link in form.links: - link.creator_id.data = current_user.id + link.created_by.data = current_user.id add_unit_query(form, current_user) add_department_query(form, current_user) set_dynamic_default(form.department, current_user.dept_pref_rel) @@ -924,14 +929,17 @@ def add_officer(): return redirect(url_for("main.submit_officer_images", officer_id=officer.id)) else: current_app.logger.info(form.errors) - return render_template("add_officer.html", form=form, jsloads=jsloads) + return render_template( + "add_officer.html", + form=form, + jsloads=["js/dynamic_lists.js", "js/add_officer.js"], + ) @main.route("/officer//edit", methods=[HTTPMethod.GET, HTTPMethod.POST]) @login_required @ac_or_admin_required def edit_officer(officer_id): - jsloads = ["js/dynamic_lists.js"] officer = Officer.query.filter_by(id=officer_id).one() form = EditOfficerForm(obj=officer) @@ -953,7 +961,9 @@ def edit_officer(officer_id): return redirect(url_for("main.officer_profile", officer_id=officer.id)) else: current_app.logger.info(form.errors) - return render_template("edit_officer.html", form=form, jsloads=jsloads) + return render_template( + "edit_officer.html", form=form, jsloads=["js/dynamic_lists.js"] + ) @main.route("/unit/new", methods=[HTTPMethod.GET, HTTPMethod.POST]) @@ -1129,7 +1139,7 @@ def label_data(department_id=None, image_id=None): face_position_y=upper, face_width=form.dataWidth.data, face_height=form.dataHeight.data, - user_id=current_user.get_id(), + created_by=current_user.get_id(), ) db.session.add(new_tag) db.session.commit() @@ -1245,9 +1255,9 @@ def download_dept_officers_csv(department_id): def download_dept_assignments_csv(department_id): assignments = ( db.session.query(Assignment) - .join(Assignment.baseofficer) + .join(Assignment.base_officer) .filter(Officer.department_id == department_id) - .options(contains_eager(Assignment.baseofficer)) + .options(contains_eager(Assignment.base_officer)) .options(joinedload(Assignment.unit)) .options(joinedload(Assignment.job)) ) @@ -1365,7 +1375,7 @@ def download_dept_descriptions_csv(department_id): field_names = [ "id", "text_contents", - "creator_id", + "created_by", "officer_id", "created_at", "updated_at", @@ -1443,7 +1453,7 @@ def upload(department_id, officer_id=None): # we set both images to the uploaded one img_id=image.id, original_image_id=image.id, - user_id=current_user.get_id(), + created_by=current_user.get_id(), ) db.session.add(face) db.session.commit() @@ -1561,7 +1571,7 @@ def get_new_form(self): form.officers[0].oo_id.data = request.args.get("officer_id") for link in form.links: - link.creator_id.data = current_user.id + link.created_by.data = current_user.id return form def get_edit_form(self, obj): @@ -1571,10 +1581,10 @@ def get_edit_form(self, obj): no_links = len(obj.links) no_officers = len(obj.officers) for link in form.links: - if link.creator_id.data: + if link.created_by.data: continue else: - link.creator_id.data = current_user.id + link.created_by.data = current_user.id for officer_idx, officer in enumerate(obj.officers): form.officers[officer_idx].oo_id.data = officer.id @@ -1791,8 +1801,8 @@ def new(self, form=None): abort(HTTPStatus.FORBIDDEN) if not form: form = self.get_new_form() - if hasattr(form, "creator_id") and not form.creator_id.data: - form.creator_id.data = current_user.get_id() + if hasattr(form, "created_by") and not form.created_by.data: + form.created_by.data = current_user.get_id() if form.validate_on_submit(): link = Link( @@ -1801,7 +1811,7 @@ def new(self, form=None): link_type=form.link_type.data, description=form.description.data, author=form.author.data, - creator_id=form.creator_id.data, + created_by=form.created_by.data, ) self.officer.links.append(link) db.session.add(link) diff --git a/OpenOversight/app/models/config.py b/OpenOversight/app/models/config.py index 3ea9ecade..ac58a7518 100644 --- a/OpenOversight/app/models/config.py +++ b/OpenOversight/app/models/config.py @@ -7,6 +7,7 @@ KEY_ENV_PROD, KEY_ENV_TESTING, KEY_OFFICERS_PER_PAGE, + KEY_OO_MAIL_SUBJECT_PREFIX, KEY_TIMEZONE, MEGABYTE, ) @@ -44,7 +45,7 @@ def __init__(self): # Mail Settings self.OO_MAIL_SUBJECT_PREFIX = os.environ.get( - "OO_MAIL_SUBJECT_PREFIX", "[OpenOversight]" + KEY_OO_MAIL_SUBJECT_PREFIX, "[OpenOversight]" ) self.OO_SERVICE_EMAIL = os.environ.get("OO_SERVICE_EMAIL") # TODO: Remove the default once we are able to update the production .env file diff --git a/OpenOversight/app/models/database.py b/OpenOversight/app/models/database.py index c775f8177..6c56872a8 100644 --- a/OpenOversight/app/models/database.py +++ b/OpenOversight/app/models/database.py @@ -33,6 +33,13 @@ "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( + "created_at", + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ), ) officer_incidents = db.Table( @@ -41,9 +48,15 @@ db.Column( "incident_id", db.Integer, db.ForeignKey("incidents.id"), primary_key=True ), + db.Column( + "created_at", + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ), ) - # This is a last recently used cache that also utilizes a time-to-live function for each # value saved in it (12 hours). # TODO: Change this into a singleton so that we can clear values when updates happen @@ -75,6 +88,15 @@ class Department(BaseModel): name = db.Column(db.String(255), index=False, unique=False, nullable=False) short_name = db.Column(db.String(100), unique=False, nullable=False) state = db.Column(db.String(2), server_default="", nullable=False) + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) + created_by = db.Column( + db.Integer, db.ForeignKey("users.id", ondelete="SET NULL"), unique=False + ) # See https://github.com/lucyparsons/OpenOversight/issues/462 unique_internal_identifier_label = db.Column( @@ -126,6 +148,15 @@ class Job(BaseModel): order = db.Column(db.Integer, index=True, unique=False, nullable=False) department_id = db.Column(db.Integer, db.ForeignKey("departments.id")) department = db.relationship("Department", backref="jobs") + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) + created_by = db.Column( + db.Integer, db.ForeignKey("users.id", ondelete="SET NULL"), unique=False + ) __table_args__ = ( UniqueConstraint( @@ -145,7 +176,9 @@ class Note(BaseModel): id = db.Column(db.Integer, primary_key=True) text_contents = db.Column(db.Text()) - creator_id = db.Column(db.Integer, db.ForeignKey("users.id", ondelete="SET NULL")) + created_by = db.Column( + db.Integer, db.ForeignKey("users.id", ondelete="SET NULL"), unique=False + ) creator = db.relationship("User", backref="notes") officer_id = db.Column(db.Integer, db.ForeignKey("officers.id", ondelete="CASCADE")) officer = db.relationship("Officer", back_populates="notes") @@ -165,7 +198,9 @@ class Description(BaseModel): officer = db.relationship("Officer", back_populates="descriptions") id = db.Column(db.Integer, primary_key=True) text_contents = db.Column(db.Text()) - creator_id = db.Column(db.Integer, db.ForeignKey("users.id", ondelete="SET NULL")) + created_by = db.Column( + db.Integer, db.ForeignKey("users.id", ondelete="SET NULL"), unique=False + ) officer_id = db.Column(db.Integer, db.ForeignKey("officers.id", ondelete="CASCADE")) created_at = db.Column( db.DateTime(timezone=True), @@ -196,6 +231,15 @@ class Officer(BaseModel): unique_internal_identifier = db.Column( db.String(50), index=True, unique=True, nullable=True ) + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) + created_by = db.Column( + db.Integer, db.ForeignKey("users.id", ondelete="SET NULL"), unique=False + ) links = db.relationship( "Link", secondary=officer_links, backref=db.backref("officers", lazy=True) @@ -296,6 +340,15 @@ class Salary(BaseModel): overtime_pay = db.Column(db.Numeric, index=True, unique=False, nullable=True) year = db.Column(db.Integer, index=True, unique=False, nullable=False) is_fiscal_year = db.Column(db.Boolean, index=False, unique=False, nullable=False) + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) + created_by = db.Column( + db.Integer, db.ForeignKey("users.id", ondelete="SET NULL"), unique=False + ) def __repr__(self): return f"" @@ -328,6 +390,15 @@ class Unit(BaseModel): department = db.relationship( "Department", backref="unit_types", order_by="Unit.description.asc()" ) + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) + created_by = db.Column( + db.Integer, db.ForeignKey("users.id", ondelete="SET NULL"), unique=False + ) def __repr__(self): return f"Unit: {self.description}" @@ -366,11 +437,17 @@ class Face(BaseModel): original_image = db.relationship( "Image", backref="tags", foreign_keys=[original_image_id], lazy=True ) - user_id = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=True) + created_by = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=True) user = db.relationship("User", backref="faces") featured = db.Column( db.Boolean, nullable=False, default=False, server_default="false" ) + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) __table_args__ = (UniqueConstraint("officer_id", "img_id", name="unique_faces"),) @@ -385,7 +462,6 @@ class Image(BaseModel): filepath = db.Column(db.String(255), unique=False) hash_img = db.Column(db.String(120), unique=False, nullable=True) - # Track when the image was put into our database created_at = db.Column( db.DateTime(timezone=True), index=True, @@ -398,7 +474,12 @@ class Image(BaseModel): db.DateTime(timezone=True), index=True, unique=False, nullable=True ) contains_cops = db.Column(db.Boolean, nullable=True) - user_id = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=True) + created_by = db.Column( + db.Integer, + db.ForeignKey("users.id", ondelete="SET NULL"), + nullable=True, + unique=False, + ) user = db.relationship("User", backref="raw_images") is_tagged = db.Column(db.Boolean, default=False, unique=False, nullable=True) @@ -416,6 +497,13 @@ def __repr__(self): "incident_id", db.Integer, db.ForeignKey("incidents.id"), primary_key=True ), db.Column("link_id", db.Integer, db.ForeignKey("links.id"), primary_key=True), + db.Column( + "created_at", + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ), ) incident_license_plates = db.Table( @@ -429,6 +517,13 @@ def __repr__(self): db.ForeignKey("license_plates.id"), primary_key=True, ), + db.Column( + "created_at", + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ), ) incident_officers = db.Table( @@ -439,6 +534,13 @@ def __repr__(self): db.Column( "officers_id", db.Integer, db.ForeignKey("officers.id"), primary_key=True ), + db.Column( + "created_at", + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ), ) @@ -452,6 +554,18 @@ class Location(BaseModel): city = db.Column(db.String(100), unique=False, index=True) state = db.Column(db.String(2), unique=False, index=True) zip_code = db.Column(db.String(5), unique=False, index=True) + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) + created_by = db.Column( + db.Integer, + db.ForeignKey("users.id", ondelete="SET NULL"), + nullable=True, + unique=False, + ) @validates("zip_code") def validate_zip_code(self, key, zip_code): @@ -491,6 +605,18 @@ class LicensePlate(BaseModel): id = db.Column(db.Integer, primary_key=True) number = db.Column(db.String(8), nullable=False, index=True) state = db.Column(db.String(2), index=True) + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) + created_by = db.Column( + db.Integer, + db.ForeignKey("users.id", ondelete="SET NULL"), + nullable=True, + unique=False, + ) # for use if car is federal, diplomat, or other non-state # non_state_identifier = db.Column(db.String(20), index=True) @@ -509,8 +635,19 @@ class Link(BaseModel): link_type = db.Column(db.String(100), index=True) description = db.Column(db.Text(), nullable=True) author = db.Column(db.String(255), nullable=True) - creator_id = db.Column(db.Integer, db.ForeignKey("users.id", ondelete="SET NULL")) + created_by = db.Column( + db.Integer, + db.ForeignKey("users.id", ondelete="SET NULL"), + nullable=True, + unique=False, + ) creator = db.relationship("User", backref="links", lazy=True) + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) @validates("url") def validate_url(self, key, url): @@ -547,13 +684,31 @@ class Incident(BaseModel): ) department_id = db.Column(db.Integer, db.ForeignKey("departments.id")) department = db.relationship("Department", backref="incidents", lazy=True) - creator_id = db.Column(db.Integer, db.ForeignKey("users.id")) + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) + created_by = db.Column( + db.Integer, + db.ForeignKey("users.id", ondelete="SET NULL"), + nullable=True, + unique=False, + ) creator = db.relationship( - "User", backref="incidents_created", lazy=True, foreign_keys=[creator_id] + "User", backref="incidents_created", lazy=True, foreign_keys=[created_by] + ) + last_updated_by = db.Column( + db.Integer, + db.ForeignKey("users.id", ondelete="SET NULL"), + nullable=True, + unique=False, ) - last_updated_id = db.Column(db.Integer, db.ForeignKey("users.id")) - last_updated_by = db.relationship( - "User", backref="incidents_updated", lazy=True, foreign_keys=[last_updated_id] + last_updated_at = db.Column( + db.DateTime(timezone=True), + nullable=True, + unique=False, ) @@ -576,6 +731,12 @@ class User(UserMixin, BaseModel): dept_pref_rel = db.relationship("Department", foreign_keys=[dept_pref]) classifications = db.relationship("Image", backref="users") tags = db.relationship("Face", backref="users") + created_at = db.Column( + db.DateTime(timezone=True), + nullable=False, + server_default=sql_func.now(), + unique=False, + ) def _jwt_encode(self, payload, expiration): secret = current_app.config["SECRET_KEY"] diff --git a/OpenOversight/app/models/database_imports.py b/OpenOversight/app/models/database_imports.py index 4d2b6987f..7aeb57029 100644 --- a/OpenOversight/app/models/database_imports.py +++ b/OpenOversight/app/models/database_imports.py @@ -1,4 +1,5 @@ -from typing import TYPE_CHECKING, Any, Dict, Optional, Sequence, Tuple, Union +import datetime +from typing import Any, Dict, Optional, Sequence, Tuple, Union import dateutil.parser @@ -24,10 +25,6 @@ from OpenOversight.app.validators import state_validator, url_validator -if TYPE_CHECKING: - import datetime - - def validate_choice( value: Optional[str], given_choices: Sequence[Tuple[str, str]] ) -> Optional[str]: @@ -39,13 +36,13 @@ def validate_choice( return None -def parse_date(date_str: Optional[str]) -> Optional["datetime.date"]: +def parse_date(date_str: Optional[str]) -> Optional[datetime.date]: if date_str: return dateutil.parser.parse(date_str).date() return None -def parse_time(time_str: Optional[str]) -> Optional["datetime.time"]: +def parse_time(time_str: Optional[str]) -> Optional[datetime.time]: if time_str: return dateutil.parser.parse(time_str).time() return None @@ -201,7 +198,7 @@ def create_link_from_dict(data: Dict[str, Any], force_id: bool = False) -> Link: link_type=validate_choice(data.get("link_type"), LINK_CHOICES), description=parse_str(data.get("description"), None), author=parse_str(data.get("author"), None), - creator_id=parse_int(data.get("creator_id")), + created_by=parse_int(data.get("created_by")), ) if force_id and data.get("id"): @@ -226,8 +223,8 @@ def update_link_from_dict(data: Dict[str, Any], link: Link) -> Link: link.description = parse_str(data.get("description"), None) if "author" in data: link.author = parse_str(data.get("author"), None) - if "creator_id" in data: - link.creator_id = parse_int(data.get("creator_id")) + if "created_by" in data: + link.created_by = parse_int(data.get("created_by")) if "officers" in data: link.officers = data.get("officers") or [] if "incidents" in data: @@ -285,8 +282,9 @@ def create_incident_from_dict(data: Dict[str, Any], force_id: bool = False) -> I description=parse_str(data.get("description"), None), address_id=data.get("address_id"), department_id=parse_int(data.get("department_id")), - creator_id=parse_int(data.get("creator_id")), - last_updated_id=parse_int(data.get("last_updated_id")), + created_by=parse_int(data.get("created_by")), + last_updated_by=parse_int(data.get("last_updated_by")), + last_updated_at=datetime.datetime.now(), ) incident.officers = data.get("officers", []) @@ -313,10 +311,11 @@ def update_incident_from_dict(data: Dict[str, Any], incident: Incident) -> Incid incident.address_id = data.get("address_id") if "department_id" in data: incident.department_id = parse_int(data.get("department_id")) - if "creator_id" in data: - incident.creator_id = parse_int(data.get("creator_id")) - if "last_updated_id" in data: - incident.last_updated_id = parse_int(data.get("last_updated_id")) + if "created_by" in data: + incident.created_by = parse_int(data.get("created_by")) + if "last_updated_by" in data: + incident.last_updated_by = parse_int(data.get("last_updated_by")) + incident.last_updated_at = datetime.datetime.now() if "officers" in data: incident.officers = data["officers"] or [] if "license_plate_objects" in data: diff --git a/OpenOversight/app/models/emails.py b/OpenOversight/app/models/emails.py index 6ddc711e8..90efc8672 100644 --- a/OpenOversight/app/models/emails.py +++ b/OpenOversight/app/models/emails.py @@ -3,6 +3,8 @@ from flask import current_app, render_template +from OpenOversight.app.utils.constants import KEY_OO_MAIL_SUBJECT_PREFIX + class Email: """Base class for all emails.""" @@ -22,7 +24,9 @@ def create_message(self): class AdministratorApprovalEmail(Email): def __init__(self, receiver: str, user, admin): - subject = f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} New User Registered" + subject = ( + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} New User Registered" + ) body = render_template( "auth/email/new_registration.html", user=user, admin=admin ) @@ -32,7 +36,7 @@ def __init__(self, receiver: str, user, admin): class ChangeEmailAddressEmail(Email): def __init__(self, receiver: str, user, token: str): subject = ( - f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} Confirm Your Email " + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Confirm Your Email " f"Address" ) body = render_template("auth/email/change_email.html", user=user, token=token) @@ -41,9 +45,7 @@ def __init__(self, receiver: str, user, token: str): class ChangePasswordEmail(Email): def __init__(self, receiver: str, user): - subject = ( - f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} Your Password Has Changed" - ) + subject = f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Your Password Has Changed" body = render_template( "auth/email/change_password.html", user=user, @@ -54,14 +56,16 @@ def __init__(self, receiver: str, user): class ConfirmAccountEmail(Email): def __init__(self, receiver: str, user, token: str): - subject = f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} Confirm Your Account" + subject = ( + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Confirm Your Account" + ) body = render_template("auth/email/confirm.html", user=user, token=token) super().__init__(body, subject, receiver) class ConfirmedUserEmail(Email): def __init__(self, receiver: str, user, admin): - subject = f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} New User Confirmed" + subject = f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} New User Confirmed" body = render_template( "auth/email/new_confirmation.html", user=user, admin=admin ) @@ -70,6 +74,8 @@ def __init__(self, receiver: str, user, admin): class ResetPasswordEmail(Email): def __init__(self, receiver: str, user, token: str): - subject = f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} Reset Your Password" + subject = ( + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Reset Your Password" + ) body = render_template("auth/email/reset_password.html", user=user, token=token) super().__init__(body, subject, receiver) diff --git a/OpenOversight/app/templates/incident_detail.html b/OpenOversight/app/templates/incident_detail.html index 4c855611b..56c0ab492 100644 --- a/OpenOversight/app/templates/incident_detail.html +++ b/OpenOversight/app/templates/incident_detail.html @@ -41,24 +41,26 @@ incident.department.name }}

{% endif %} -
-

- Incident - {% if incident.report_number %}{{ incident.report_number }}{% endif %} -

-
- - - {% with detail=True %} - {% include "partials/incident_fields.html" %} - {% endwith %} - -
+
+
+

+ Incident + {% if incident.report_number %}{{ incident.report_number }}{% endif %} +

+
+ + + {% with detail=True %} + {% include "partials/incident_fields.html" %} + {% endwith %} + +
+
+
+
+

Incident Description

+ {{ incident.description | markdown }}
-
-
-

Incident Description

- {{ incident.description | markdown }}
{% include "partials/links_and_videos_row.html" %} {% if current_user.is_administrator diff --git a/OpenOversight/app/utils/cloud.py b/OpenOversight/app/utils/cloud.py index d96a73394..2e1d1c9d4 100644 --- a/OpenOversight/app/utils/cloud.py +++ b/OpenOversight/app/utils/cloud.py @@ -139,7 +139,7 @@ def upload_image_to_s3_and_store_in_db(image_buf, user_id, department_id=None): created_at=datetime.datetime.now(), department_id=department_id, taken_at=date_taken, - user_id=user_id, + created_by=user_id, ) db.session.add(new_image) db.session.commit() diff --git a/OpenOversight/app/utils/constants.py b/OpenOversight/app/utils/constants.py index ab80dda49..ba1b33e20 100644 --- a/OpenOversight/app/utils/constants.py +++ b/OpenOversight/app/utils/constants.py @@ -12,7 +12,9 @@ KEY_ENV_DEV = "development" KEY_ENV_TESTING = "testing" KEY_ENV_PROD = "production" +KEY_NUM_OFFICERS = "NUM_OFFICERS" KEY_OFFICERS_PER_PAGE = "OFFICERS_PER_PAGE" +KEY_OO_MAIL_SUBJECT_PREFIX = "OO_MAIL_SUBJECT_PREFIX" KEY_TIMEZONE = "TIMEZONE" # File Handling Constants @@ -28,3 +30,7 @@ MEGABYTE = 1024 * KILOBYTE MINUTE = 60 HOUR = 60 * MINUTE + +# Test Constants +ADMIN_EMAIL = "test@example.org" +ADMIN_PASSWORD = "testtest" diff --git a/OpenOversight/app/utils/db.py b/OpenOversight/app/utils/db.py index 87c2402bb..febc96590 100644 --- a/OpenOversight/app/utils/db.py +++ b/OpenOversight/app/utils/db.py @@ -33,20 +33,20 @@ def add_unit_query(form, current_user): def compute_leaderboard_stats(select_top=25): top_sorters = ( - db.session.query(User, func.count(Image.user_id)) + db.session.query(User, func.count(Image.created_by)) .select_from(Image) .join(User) .group_by(User) - .order_by(func.count(Image.user_id).desc()) + .order_by(func.count(Image.created_by).desc()) .limit(select_top) .all() ) top_taggers = ( - db.session.query(User, func.count(Face.user_id)) + db.session.query(User, func.count(Face.created_by)) .select_from(Face) .join(User) .group_by(User) - .order_by(func.count(Face.user_id).desc()) + .order_by(func.count(Face.created_by).desc()) .limit(select_top) .all() ) @@ -72,8 +72,8 @@ def get_officer(department_id, star_no, first_name, last_name): else: star_no = str(star_no) for assignment in Assignment.query.filter_by(star_no=star_no).all(): - if assignment.baseofficer in officers: - return assignment.baseofficer + if assignment.base_officer in officers: + return assignment.base_officer return None diff --git a/OpenOversight/app/utils/forms.py b/OpenOversight/app/utils/forms.py index b99e43577..821c30bc3 100644 --- a/OpenOversight/app/utils/forms.py +++ b/OpenOversight/app/utils/forms.py @@ -67,7 +67,7 @@ def add_officer_profile(form, current_user): officer_unit = None assignment = Assignment( - baseofficer=officer, + base_officer=officer, star_no=form.star_no.data, job_id=form.job_id.data, unit=officer_unit, @@ -125,7 +125,7 @@ def add_officer_profile(form, current_user): def create_description(self, form): return Description( text_contents=form.text_contents.data, - creator_id=form.creator_id.data, + created_by=form.created_by.data, officer_id=form.officer_id.data, created_at=datetime.datetime.now(), updated_at=datetime.datetime.now(), @@ -140,8 +140,8 @@ def create_incident(self, form): "license_plates": [], "links": [], "address": "", - "creator_id": form.creator_id.data, - "last_updated_id": form.last_updated_id.data, + "created_by": form.created_by.data, + "last_updated_by": form.last_updated_by.data, } if "address" in form.data: @@ -180,15 +180,16 @@ def create_incident(self, form): report_number=form.data["report_number"], license_plates=fields["license_plates"], links=fields["links"], - creator_id=fields["creator_id"], - last_updated_id=fields["last_updated_id"], + created_by=fields["created_by"], + last_updated_by=fields["last_updated_by"], + last_updated_at=datetime.datetime.now(), ) def create_note(self, form): return Note( text_contents=form.text_contents.data, - creator_id=form.creator_id.data, + created_by=form.created_by.data, officer_id=form.officer_id.data, created_at=datetime.datetime.now(), updated_at=datetime.datetime.now(), diff --git a/OpenOversight/migrations/versions/2023-08-01-1905_b38c133bed3c_add_created_by_and_created_at_columns.py b/OpenOversight/migrations/versions/2023-08-01-1905_b38c133bed3c_add_created_by_and_created_at_columns.py new file mode 100644 index 000000000..81ab0b288 --- /dev/null +++ b/OpenOversight/migrations/versions/2023-08-01-1905_b38c133bed3c_add_created_by_and_created_at_columns.py @@ -0,0 +1,597 @@ +"""add created_by and created_at columns + +Revision ID: b38c133bed3c +Revises: 18f43ac4622f +Create Date: 2023-08-01 19:05:34.745077 + +""" +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "b38c133bed3c" +down_revision = "18f43ac4622f" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("assignments", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.create_foreign_key( + "assignments_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + with op.batch_alter_table("departments", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.create_foreign_key( + "departments_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + with op.batch_alter_table("descriptions", schema=None) as batch_op: + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.drop_constraint("descriptions_creator_id_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "descriptions_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + op.execute( + """ + UPDATE descriptions + SET created_by = creator_id + WHERE created_by IS NULL + """ + ) + + with op.batch_alter_table("descriptions", schema=None) as batch_op: + batch_op.drop_column("creator_id") + + with op.batch_alter_table("faces", schema=None) as batch_op: + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.drop_constraint("faces_user_id_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "faces_created_by_fkey", "users", ["created_by"], ["id"] + ) + + op.execute( + """ + UPDATE faces + SET created_by = user_id + WHERE created_by IS NULL + """ + ) + + with op.batch_alter_table("faces", schema=None) as batch_op: + batch_op.drop_column("user_id") + + with op.batch_alter_table("incident_license_plates", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + + with op.batch_alter_table("incident_links", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + + with op.batch_alter_table("incident_officers", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + + with op.batch_alter_table("incidents", schema=None) as batch_op: + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.add_column(sa.Column("last_updated_by", sa.Integer(), nullable=True)) + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.add_column( + sa.Column( + "last_updated_at", + sa.DateTime(timezone=True), + nullable=True, + ) + ) + batch_op.drop_constraint("incidents_last_updated_id_fkey", type_="foreignkey") + batch_op.drop_constraint("incidents_creator_id_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "incidents_last_updated_by_fkey", + "users", + ["last_updated_by"], + ["id"], + ondelete="SET NULL", + ) + batch_op.create_foreign_key( + "incidents_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + op.execute( + """ + UPDATE incidents + SET created_by = creator_id + WHERE created_by IS NULL + """ + ) + + with op.batch_alter_table("incidents", schema=None) as batch_op: + batch_op.drop_column("creator_id") + batch_op.drop_column("last_updated_id") + + with op.batch_alter_table("jobs", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.create_foreign_key( + "jobs_created_by_fkey", "users", ["created_by"], ["id"], ondelete="SET NULL" + ) + + with op.batch_alter_table("license_plates", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.create_foreign_key( + "license_plates_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + with op.batch_alter_table("links", schema=None) as batch_op: + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.drop_constraint("links_creator_id_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "links_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + op.execute( + """ + UPDATE links + SET created_by = creator_id + WHERE created_by IS NULL + """ + ) + + with op.batch_alter_table("links", schema=None) as batch_op: + batch_op.drop_column("creator_id") + + with op.batch_alter_table("locations", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.create_foreign_key( + "locations_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + with op.batch_alter_table("notes", schema=None) as batch_op: + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.drop_constraint("notes_creator_id_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "notes_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + op.execute( + """ + UPDATE notes + SET created_by = creator_id + WHERE created_by IS NULL + """ + ) + + with op.batch_alter_table("notes", schema=None) as batch_op: + batch_op.drop_column("creator_id") + + with op.batch_alter_table("officer_incidents", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + + with op.batch_alter_table("officer_links", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + + with op.batch_alter_table("officers", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.create_foreign_key( + "officers_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + with op.batch_alter_table("raw_images", schema=None) as batch_op: + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.drop_constraint("raw_images_user_id_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "raw_images_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + op.execute( + """ + UPDATE raw_images + SET created_by = user_id + WHERE created_by IS NULL + """ + ) + + with op.batch_alter_table("raw_images", schema=None) as batch_op: + batch_op.drop_column("user_id") + + with op.batch_alter_table("salaries", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.create_foreign_key( + "salaries_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + with op.batch_alter_table("unit_types", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + batch_op.add_column(sa.Column("created_by", sa.Integer(), nullable=True)) + batch_op.create_foreign_key( + "unit_types_created_by_fkey", + "users", + ["created_by"], + ["id"], + ondelete="SET NULL", + ) + + with op.batch_alter_table("users", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ) + ) + + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table("users", schema=None) as batch_op: + batch_op.drop_column("created_at") + + with op.batch_alter_table("unit_types", schema=None) as batch_op: + batch_op.drop_constraint("unit_types_created_by_fkey", type_="foreignkey") + batch_op.drop_column("created_by") + batch_op.drop_column("created_at") + + with op.batch_alter_table("salaries", schema=None) as batch_op: + batch_op.drop_constraint("salaries_created_by_fkey", type_="foreignkey") + batch_op.drop_column("created_by") + batch_op.drop_column("created_at") + + with op.batch_alter_table("raw_images", schema=None) as batch_op: + batch_op.add_column( + sa.Column("user_id", sa.INTEGER(), autoincrement=False, nullable=True) + ) + batch_op.drop_constraint("raw_images_created_by_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "raw_images_user_id_fkey", "users", ["user_id"], ["id"] + ) + + op.execute( + """ + UPDATE raw_images + SET user_id = created_by + WHERE user_id IS NULL + """ + ) + + with op.batch_alter_table("raw_images", schema=None) as batch_op: + batch_op.drop_column("created_by") + + with op.batch_alter_table("officers", schema=None) as batch_op: + batch_op.drop_constraint("officers_created_by_fkey", type_="foreignkey") + batch_op.drop_column("created_by") + batch_op.drop_column("created_at") + + with op.batch_alter_table("officer_links", schema=None) as batch_op: + batch_op.drop_column("created_at") + + with op.batch_alter_table("officer_incidents", schema=None) as batch_op: + batch_op.drop_column("created_at") + + with op.batch_alter_table("notes", schema=None) as batch_op: + batch_op.add_column( + sa.Column("creator_id", sa.INTEGER(), autoincrement=False, nullable=True) + ) + batch_op.drop_constraint("notes_created_by_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "notes_creator_id_fkey", + "users", + ["creator_id"], + ["id"], + ondelete="SET NULL", + ) + + op.execute( + """ + UPDATE notes + SET creator_id = created_by + WHERE creator_id IS NULL + """ + ) + + with op.batch_alter_table("notes", schema=None) as batch_op: + batch_op.drop_column("created_by") + + with op.batch_alter_table("locations", schema=None) as batch_op: + batch_op.drop_constraint("locations_created_by_fkey", type_="foreignkey") + batch_op.drop_column("created_by") + batch_op.drop_column("created_at") + + with op.batch_alter_table("links", schema=None) as batch_op: + batch_op.add_column( + sa.Column("creator_id", sa.INTEGER(), autoincrement=False, nullable=True) + ) + batch_op.drop_constraint("links_created_by_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "links_creator_id_fkey", + "users", + ["creator_id"], + ["id"], + ondelete="SET NULL", + ) + + op.execute( + """ + UPDATE links + SET creator_id = created_by + WHERE creator_id IS NULL + """ + ) + + with op.batch_alter_table("links", schema=None) as batch_op: + batch_op.drop_column("created_at") + batch_op.drop_column("created_by") + + with op.batch_alter_table("license_plates", schema=None) as batch_op: + batch_op.drop_constraint("license_plates_created_by_fkey", type_="foreignkey") + batch_op.drop_column("created_by") + batch_op.drop_column("created_at") + + with op.batch_alter_table("jobs", schema=None) as batch_op: + batch_op.drop_constraint("jobs_created_by_fkey", type_="foreignkey") + batch_op.drop_column("created_by") + batch_op.drop_column("created_at") + + with op.batch_alter_table("incidents", schema=None) as batch_op: + batch_op.add_column( + sa.Column( + "last_updated_id", sa.INTEGER(), autoincrement=False, nullable=True + ) + ) + batch_op.add_column( + sa.Column("creator_id", sa.INTEGER(), autoincrement=False, nullable=True) + ) + batch_op.drop_constraint("incidents_created_by_fkey", type_="foreignkey") + batch_op.drop_constraint("incidents_last_updated_by_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "incidents_creator_id_fkey", "users", ["creator_id"], ["id"] + ) + batch_op.create_foreign_key( + "incidents_last_updated_id_fkey", "users", ["last_updated_id"], ["id"] + ) + + op.execute( + """ + UPDATE incidents + SET creator_id = created_by + WHERE creator_id IS NULL + """ + ) + + with op.batch_alter_table("incidents", schema=None) as batch_op: + batch_op.drop_column("created_at") + batch_op.drop_column("last_updated_by") + batch_op.drop_column("last_updated_at") + batch_op.drop_column("created_by") + + with op.batch_alter_table("incident_officers", schema=None) as batch_op: + batch_op.drop_column("created_at") + + with op.batch_alter_table("incident_links", schema=None) as batch_op: + batch_op.drop_column("created_at") + + with op.batch_alter_table("incident_license_plates", schema=None) as batch_op: + batch_op.drop_column("created_at") + + with op.batch_alter_table("faces", schema=None) as batch_op: + batch_op.add_column( + sa.Column("user_id", sa.INTEGER(), autoincrement=False, nullable=True) + ) + batch_op.drop_constraint("faces_created_by_fkey", type_="foreignkey") + batch_op.create_foreign_key("faces_user_id_fkey", "users", ["user_id"], ["id"]) + + op.execute( + """ + UPDATE faces + SET user_id = created_by + WHERE user_id IS NULL + """ + ) + + with op.batch_alter_table("faces", schema=None) as batch_op: + batch_op.drop_column("created_at") + batch_op.drop_column("created_by") + + with op.batch_alter_table("descriptions", schema=None) as batch_op: + batch_op.add_column( + sa.Column("creator_id", sa.INTEGER(), autoincrement=False, nullable=True) + ) + batch_op.drop_constraint("descriptions_created_by_fkey", type_="foreignkey") + batch_op.create_foreign_key( + "descriptions_creator_id_fkey", + "users", + ["creator_id"], + ["id"], + ondelete="SET NULL", + ) + + op.execute( + """ + UPDATE descriptions + SET creator_id = created_by + WHERE creator_id IS NULL + """ + ) + + with op.batch_alter_table("descriptions", schema=None) as batch_op: + batch_op.drop_column("created_by") + + with op.batch_alter_table("departments", schema=None) as batch_op: + batch_op.drop_constraint("departments_created_by_fkey", type_="foreignkey") + batch_op.drop_column("created_by") + batch_op.drop_column("created_at") + + with op.batch_alter_table("assignments", schema=None) as batch_op: + batch_op.drop_constraint("assignments_created_by_fkey", type_="foreignkey") + batch_op.drop_column("created_by") + batch_op.drop_column("created_at") + + # ### end Alembic commands ### diff --git a/OpenOversight/tests/conftest.py b/OpenOversight/tests/conftest.py index 1b7846d1c..90f685a72 100644 --- a/OpenOversight/tests/conftest.py +++ b/OpenOversight/tests/conftest.py @@ -3,7 +3,6 @@ import math import os import random -import string import sys import threading import time @@ -40,14 +39,23 @@ ) from OpenOversight.app.models.database import db as _db from OpenOversight.app.utils.choices import DEPARTMENT_STATE_CHOICES -from OpenOversight.app.utils.constants import ENCODING_UTF_8, KEY_ENV_TESTING +from OpenOversight.app.utils.constants import ( + ADMIN_EMAIL, + ADMIN_PASSWORD, + ENCODING_UTF_8, + KEY_ENV_TESTING, + KEY_NUM_OFFICERS, +) from OpenOversight.app.utils.general import merge_dicts -from OpenOversight.tests.routes.route_helpers import ADMIN_EMAIL, ADMIN_PASSWORD factory = Faker() +def pick_uid(): + return str(uuid.uuid4()) + + class PoliceDepartment: """Base Police Department class.""" @@ -71,7 +79,7 @@ def __init__( self.unique_internal_identifier_label = ( unique_internal_identifier_label if unique_internal_identifier_label - else "".join(random.choices(string.ascii_uppercase + string.digits, k=20)) + else pick_uid() ) @@ -158,15 +166,11 @@ def pick_department(): return random.choice(departments) -def pick_uid(): - return str(uuid.uuid4()) - - def pick_salary(): return random.randint(100, 100000000) / 100 -def generate_officer(department): +def generate_officer(department: Department, user: User) -> Officer: year_born = pick_birth_date() f_name, m_initial, l_name = pick_name() return Officer( @@ -179,10 +183,13 @@ def generate_officer(department): employment_date=datetime.datetime(year_born + 20, 4, 4, 1, 1, 1), department_id=department.id, unique_internal_identifier=pick_uid(), + created_by=user.id, ) -def build_assignment(officer: Officer, units: List[Optional[Unit]], jobs: Job): +def build_assignment( + officer: Officer, units: List[Optional[Unit]], jobs: Job, user: User +) -> Assignment: unit = random.choice(units) unit_id = unit.id if unit else None return Assignment( @@ -192,46 +199,48 @@ def build_assignment(officer: Officer, units: List[Optional[Unit]], jobs: Job): unit_id=unit_id, start_date=pick_date(officer.full_name().encode(ENCODING_UTF_8)), resign_date=pick_date(officer.full_name().encode(ENCODING_UTF_8)), + created_by=user.id, ) -def build_note(officer, user, content=None): +def build_note(officer: Officer, user: User, content=None) -> Note: date = factory.date_time_this_year() if content is None: content = factory.text() return Note( text_contents=content, officer_id=officer.id, - creator_id=user.id, + created_by=user.id, created_at=date, updated_at=date, ) -def build_description(officer, user, content=None): +def build_description(officer: Officer, user: User, content=None) -> Description: date = factory.date_time_this_year() if content is None: content = factory.text() return Description( text_contents=content, officer_id=officer.id, - creator_id=user.id, + created_by=user.id, created_at=date, updated_at=date, ) -def build_salary(officer): +def build_salary(officer: Officer, user: User) -> Salary: return Salary( officer_id=officer.id, salary=pick_salary(), overtime_pay=pick_salary(), year=random.randint(2000, 2019), is_fiscal_year=True if random.randint(0, 1) else False, + created_by=user.id, ) -def assign_faces(officer, images): +def assign_faces(officer: Officer, images: Image, user: User): if random.uniform(0, 1) >= 0.5: img_id = random.choice(images).id return Face( @@ -239,6 +248,7 @@ def assign_faces(officer, images): img_id=img_id, original_image_id=img_id, featured=False, + created_by=user.id, ) else: return False @@ -324,28 +334,82 @@ def test_csv_dir(): def add_mockdata(session): - assert current_app.config["NUM_OFFICERS"] >= 5 + assert current_app.config[KEY_NUM_OFFICERS] >= 5 + + test_user = User( + email="jen@example.org", username="test_user", password="dog", confirmed=True + ) + session.add(test_user) + + test_admin = User( + email=ADMIN_EMAIL, + username="test_admin", + password=ADMIN_PASSWORD, + confirmed=True, + is_administrator=True, + ) + session.add(test_admin) + + test_unconfirmed_user = User( + email="freddy@example.org", username="b_meson", password="dog", confirmed=False + ) + session.add(test_unconfirmed_user) + session.commit() + + test_disabled_user = User( + email="may@example.org", + username="may", + password="yam", + confirmed=True, + is_disabled=True, + ) + session.add(test_disabled_user) + session.commit() + + test_modified_disabled_user = User( + email="sam@example.org", + username="sam", + password="the yam", + confirmed=True, + is_disabled=True, + ) + session.add(test_modified_disabled_user) + session.commit() + department = Department( name=SPRINGFIELD_PD.name, short_name=SPRINGFIELD_PD.short_name, state=SPRINGFIELD_PD.state, unique_internal_identifier_label=SPRINGFIELD_PD.unique_internal_identifier_label, + created_by=test_admin.id, ) session.add(department) department2 = Department( name=OTHER_PD.name, short_name=OTHER_PD.short_name, state=OTHER_PD.state, + created_by=test_admin.id, ) session.add(department2) empty_department = Department( name=NO_OFFICER_PD.name, short_name=NO_OFFICER_PD.short_name, state=NO_OFFICER_PD.state, + created_by=test_admin.id, ) session.add(empty_department) session.commit() + test_area_coordinator = User( + email="raq929@example.org", + username="test_ac", + password="horse", + confirmed=True, + is_area_coordinator=True, + ac_department_id=AC_DEPT, + ) + session.add(test_area_coordinator) + for i, rank in enumerate(RANK_CHOICES_1): session.add( Job( @@ -353,6 +417,7 @@ def add_mockdata(session): order=i, is_sworn_officer=True, department_id=department.id, + created_by=test_admin.id, ) ) session.add( @@ -361,6 +426,7 @@ def add_mockdata(session): order=i, is_sworn_officer=True, department_id=empty_department.id, + created_by=test_admin.id, ) ) @@ -371,6 +437,7 @@ def add_mockdata(session): order=i, is_sworn_officer=True, department_id=department2.id, + created_by=test_admin.id, ) ) session.commit() @@ -379,11 +446,19 @@ def add_mockdata(session): random.seed(current_app.config["SEED"]) test_units = [ - Unit(description="test", department_id=1), - Unit(description="District 13", department_id=1), - Unit(description="Donut Devourers", department_id=1), - Unit(description="Bureau of Organized Crime", department_id=2), - Unit(description="Porky's BBQ: Rub Division", department_id=2), + Unit(description="test", department_id=1, created_by=test_admin.id), + Unit(description="District 13", department_id=1, created_by=test_admin.id), + Unit(description="Donut Devourers", department_id=1, created_by=test_admin.id), + Unit( + description="Bureau of Organized Crime", + department_id=2, + created_by=test_admin.id, + ), + Unit( + description="Porky's BBQ: Rub Division", + department_id=2, + created_by=test_admin.id, + ), ] session.add_all(test_units) session.commit() @@ -393,12 +468,14 @@ def add_mockdata(session): Image( filepath=f"/static/images/test_cop{x + 1}.png", department_id=department.id, + created_by=test_admin.id, ) for x in range(5) ] + [ Image( filepath=f"/static/images/test_cop{x + 1}.png", department_id=department2.id, + created_by=test_admin.id, ) for x in range(5) ] @@ -410,19 +487,22 @@ def add_mockdata(session): link_type="link", title="OpenOversight", description="A public, searchable database of law enforcement officers.", + created_by=test_admin.id, ), Link( url="http://www.youtube.com/?v=help", link_type="video", title="Youtube", author="the internet", + created_by=test_admin.id, ), ] officers = [] for d in [department, department2]: officers += [ - generate_officer(d) for _ in range(current_app.config["NUM_OFFICERS"]) + generate_officer(d, test_admin) + for _ in range(current_app.config[KEY_NUM_OFFICERS]) ] officers[0].links = test_officer_links session.add_all(officers) @@ -444,21 +524,23 @@ def add_mockdata(session): assignment_ratio = 0.9 # 90% num_officers_with_assignments_1 = math.ceil(len(officers_dept1) * assignment_ratio) assignments_dept1 = [ - build_assignment(officer, test_units, jobs_dept1) + build_assignment(officer, test_units, jobs_dept1, test_admin) for officer in officers_dept1[:num_officers_with_assignments_1] ] num_officers_with_assignments_2 = math.ceil(len(officers_dept2) * assignment_ratio) assignments_dept2 = [ - build_assignment(officer, test_units, jobs_dept2) + build_assignment(officer, test_units, jobs_dept2, test_admin) for officer in officers_dept2[:num_officers_with_assignments_2] ] - salaries = [build_salary(officer) for officer in all_officers] + salaries = [build_salary(officer, test_admin) for officer in all_officers] faces_dept1 = [ - assign_faces(officer, assigned_images_dept1) for officer in officers_dept1 + assign_faces(officer, assigned_images_dept1, test_admin) + for officer in officers_dept1 ] faces_dept2 = [ - assign_faces(officer, assigned_images_dept2) for officer in officers_dept2 + assign_faces(officer, assigned_images_dept2, test_admin) + for officer in officers_dept2 ] faces1 = [f for f in faces_dept1 if f] faces2 = [f for f in faces_dept2 if f] @@ -469,56 +551,6 @@ def add_mockdata(session): session.add_all(faces1) session.add_all(faces2) - test_user = User( - email="jen@example.org", username="test_user", password="dog", confirmed=True - ) - session.add(test_user) - - test_admin = User( - email=ADMIN_EMAIL, - username="test_admin", - password=ADMIN_PASSWORD, - confirmed=True, - is_administrator=True, - ) - session.add(test_admin) - - test_area_coordinator = User( - email="raq929@example.org", - username="test_ac", - password="horse", - confirmed=True, - is_area_coordinator=True, - ac_department_id=AC_DEPT, - ) - session.add(test_area_coordinator) - - test_unconfirmed_user = User( - email="freddy@example.org", username="b_meson", password="dog", confirmed=False - ) - session.add(test_unconfirmed_user) - session.commit() - - test_disabled_user = User( - email="may@example.org", - username="may", - password="yam", - confirmed=True, - is_disabled=True, - ) - session.add(test_disabled_user) - session.commit() - - test_modified_disabled_user = User( - email="sam@example.org", - username="sam", - password="the yam", - confirmed=True, - is_disabled=True, - ) - session.add(test_modified_disabled_user) - session.commit() - test_addresses = [ Location( street_name="Test St", @@ -527,6 +559,7 @@ def add_mockdata(session): city="My City", state="AZ", zip_code="23456", + created_by=test_admin.id, ), Location( street_name="Testing St", @@ -535,6 +568,7 @@ def add_mockdata(session): city="Another City", state="ME", zip_code="23456", + created_by=test_admin.id, ), ] @@ -542,8 +576,8 @@ def add_mockdata(session): session.commit() test_license_plates = [ - LicensePlate(number="603EEE", state="MA"), - LicensePlate(number="404301", state="WA"), + LicensePlate(number="603EEE", state="MA", created_by=test_admin.id), + LicensePlate(number="404301", state="WA", created_by=test_admin.id), ] session.add_all(test_license_plates) @@ -554,13 +588,13 @@ def add_mockdata(session): url="https://stackoverflow.com/", link_type="link", creator=test_admin, - creator_id=test_admin.id, + created_by=test_admin.id, ), Link( url="http://www.youtube.com/?v=help", link_type="video", creator=test_admin, - creator_id=test_admin.id, + created_by=test_admin.id, ), ] @@ -578,8 +612,9 @@ def add_mockdata(session): license_plates=test_license_plates, links=test_incident_links, officers=[all_officers[o] for o in range(4)], - creator_id=1, - last_updated_id=1, + created_by=test_admin.id, + last_updated_by=test_admin.id, + last_updated_at=datetime.datetime.now(), ), Incident( date=datetime.date(2017, 12, 11), @@ -591,8 +626,9 @@ def add_mockdata(session): license_plates=[test_license_plates[0]], links=test_incident_links, officers=[all_officers[o] for o in range(3)], - creator_id=2, - last_updated_id=1, + created_by=test_admin.id, + last_updated_by=test_admin.id, + last_updated_at=datetime.datetime.now(), ), Incident( date=datetime.datetime(2019, 1, 15), @@ -605,8 +641,9 @@ def add_mockdata(session): license_plates=[test_license_plates[0]], links=test_incident_links, officers=[all_officers[o] for o in range(1)], - creator_id=2, - last_updated_id=1, + created_by=test_admin.id, + last_updated_by=test_admin.id, + last_updated_at=datetime.datetime.now(), ), ] session.add_all(test_incidents) @@ -721,11 +758,11 @@ def teardown(): officers_dept1 = Officer.query.filter_by(department_id=1).all() if sys.version_info.major == 2: - csvf = open(str(csv_path), "w") + csv_file = open(str(csv_path), "w") else: - csvf = open(str(csv_path), "w", newline="") + csv_file = open(str(csv_path), "w", newline="") try: - writer = csv.DictWriter(csvf, fieldnames=fieldnames, extrasaction="ignore") + writer = csv.DictWriter(csv_file, fieldnames=fieldnames, extrasaction="ignore") writer.writeheader() for officer in officers_dept1: if not officer.unique_internal_identifier: @@ -758,7 +795,7 @@ def teardown(): ) writer.writerow(towrite) finally: - csvf.close() + csv_file.close() request.addfinalizer(teardown) return str(csv_path) @@ -795,8 +832,8 @@ def browser(app, server_port): time.sleep(10) # start headless webdriver - vdisplay = Xvfb() - vdisplay.start() + visual_display = Xvfb() + visual_display.start() driver = webdriver.Firefox(log_path="/tmp/geckodriver.log") # wait for browser to start up time.sleep(3) @@ -804,4 +841,4 @@ def browser(app, server_port): # shutdown headless webdriver driver.quit() - vdisplay.stop() + visual_display.stop() diff --git a/OpenOversight/tests/routes/route_helpers.py b/OpenOversight/tests/routes/route_helpers.py index b6f6f08be..5069dea27 100644 --- a/OpenOversight/tests/routes/route_helpers.py +++ b/OpenOversight/tests/routes/route_helpers.py @@ -1,20 +1,21 @@ from flask import url_for from OpenOversight.app.auth.forms import LoginForm - - -ADMIN_EMAIL = "test@example.org" -ADMIN_PASSWORD = "testtest" +from OpenOversight.app.models.database import User +from OpenOversight.app.utils.constants import ADMIN_PASSWORD +from OpenOversight.tests.conftest import AC_DEPT def login_user(client): - form = LoginForm(email="jen@example.org", password="dog", remember_me=True) + user = User.query.filter_by(id=1).first() + form = LoginForm(email=user.email, password="dog", remember_me=True) rv = client.post(url_for("auth.login"), data=form.data, follow_redirects=False) return rv def login_unconfirmed_user(client): - form = LoginForm(email="freddy@example.org", password="dog", remember_me=True) + user = User.query.filter_by(confirmed=False).first() + form = LoginForm(email=user.email, password="dog", 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 @@ -33,21 +34,21 @@ def login_modified_disabled_user(client): def login_admin(client): - form = LoginForm(email=ADMIN_EMAIL, password=ADMIN_PASSWORD, remember_me=True) + user = User.query.filter_by(is_administrator=True).first() + form = LoginForm(email=user.email, password=ADMIN_PASSWORD, remember_me=True) rv = client.post(url_for("auth.login"), data=form.data, follow_redirects=False) return rv def login_ac(client): - form = LoginForm(email="raq929@example.org", password="horse", remember_me=True) + user = User.query.filter_by(ac_department_id=AC_DEPT).first() + form = LoginForm(email=user.email, password="horse", remember_me=True) rv = client.post(url_for("auth.login"), data=form.data, follow_redirects=False) return rv def process_form_data(form_dict): - """Takes the dict from a form with embedded formd and flattens it - - in the way that it is flattened in the browser""" + """Mock the browser-flattening of a form containing embedded data.""" new_dict = {} for key, value in form_dict.items(): if type(value) == list: diff --git a/OpenOversight/tests/routes/test_auth.py b/OpenOversight/tests/routes/test_auth.py index a6f5e0d82..9950ac40a 100644 --- a/OpenOversight/tests/routes/test_auth.py +++ b/OpenOversight/tests/routes/test_auth.py @@ -1,4 +1,3 @@ -# Routing and view tests from http import HTTPStatus from unittest import TestCase from urllib.parse import urlparse @@ -16,8 +15,9 @@ RegistrationForm, ) from OpenOversight.app.models.database import User - -from .route_helpers import ( +from OpenOversight.app.utils.constants import KEY_OO_MAIL_SUBJECT_PREFIX +from OpenOversight.tests.conftest import AC_DEPT +from OpenOversight.tests.routes.route_helpers import ( login_disabled_user, login_modified_disabled_user, login_unconfirmed_user, @@ -90,9 +90,10 @@ def test_user_can_logout(mockdata, client, session): def test_user_cannot_register_with_existing_email(mockdata, client, session): with current_app.test_request_context(): + user = User.query.filter_by(is_administrator=True).first() form = RegistrationForm( - email="jen@example.org", - username="redshiftzero", + email=user.email, + username=user.username, password="dog", password2="dog", ) @@ -109,9 +110,10 @@ def test_user_cannot_register_with_existing_email_differently_cased( mockdata, client, session ): with current_app.test_request_context(): + user = User.query.filter_by(is_administrator=True).first() form = RegistrationForm( - email="JEN@EXAMPLE.ORG", - username="redshiftzero", + email=user.email.upper(), + username=user.username, password="dog", password2="dog", ) @@ -126,9 +128,10 @@ def test_user_cannot_register_with_existing_email_differently_cased( def test_user_cannot_register_if_passwords_dont_match(mockdata, client, session): with current_app.test_request_context(): + user = User.query.filter_by(is_administrator=True).first() form = RegistrationForm( - email="freddy@example.org", - username="b_meson", + email=user.email, + username=user.username, password="dog", password2="cat", ) @@ -141,14 +144,14 @@ def test_user_cannot_register_if_passwords_dont_match(mockdata, client, session) assert b"Passwords must match" in rv.data -def test_user_can_register_with_legit_credentials(mockdata, client, session): +def test_user_can_register_with_legit_credentials(mockdata, client, session, faker): with current_app.test_request_context(), TestCase.assertLogs( current_app.logger ) as log: diceware_password = "operative hamster persevere verbalize curling" form = RegistrationForm( - email="jen@example.com", - username="redshiftzero", + email=faker.ascii_email(), + username="generic_username", password=diceware_password, password2=diceware_password, ) @@ -158,16 +161,17 @@ def test_user_can_register_with_legit_credentials(mockdata, client, session): assert b"A confirmation email has been sent to you." in rv.data assert ( - f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} Confirm Your Account" + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Confirm Your Account" in str(log.output) ) def test_user_cannot_register_with_weak_password(mockdata, client, session): with current_app.test_request_context(): + user = User.query.filter_by(is_administrator=True).first() form = RegistrationForm( - email="jen@example.com", - username="redshiftzero", + email=user.email, + username=user.username, password="weak", password2="weak", ) @@ -188,7 +192,7 @@ def test_user_can_get_a_confirmation_token_resent(mockdata, client, session): assert b"A new confirmation email has been sent to you." in rv.data assert ( - f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} Confirm Your Account" + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Confirm Your Account" in str(log.output) ) @@ -197,7 +201,8 @@ def test_user_can_get_password_reset_token_sent(mockdata, client, session): with current_app.test_request_context(), TestCase.assertLogs( current_app.logger ) as log: - form = PasswordResetRequestForm(email="jen@example.org") + user = User.query.filter_by(is_administrator=True).first() + form = PasswordResetRequestForm(email=user.email) rv = client.post( url_for("auth.password_reset_request"), @@ -207,7 +212,7 @@ def test_user_can_get_password_reset_token_sent(mockdata, client, session): assert b"An email with instructions to reset your password" in rv.data assert ( - f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} Reset Your Password" + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Reset Your Password" in str(log.output) ) @@ -218,7 +223,8 @@ def test_user_can_get_password_reset_token_sent_with_differently_cased_email( with current_app.test_request_context(), TestCase.assertLogs( current_app.logger ) as log: - form = PasswordResetRequestForm(email="JEN@EXAMPLE.ORG") + user = User.query.filter_by(is_administrator=True).first() + form = PasswordResetRequestForm(email=user.email.upper()) rv = client.post( url_for("auth.password_reset_request"), @@ -228,17 +234,17 @@ def test_user_can_get_password_reset_token_sent_with_differently_cased_email( assert b"An email with instructions to reset your password" in rv.data assert ( - f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} Reset Your Password" + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Reset Your Password" in str(log.output) ) def test_user_can_get_reset_password_with_valid_token(mockdata, client, session): with current_app.test_request_context(): + user = User.query.filter_by(is_administrator=True).first() form = PasswordResetForm( - email="jen@example.org", password="catdog", password2="catdog" + email=user.email, password="catdog", password2="catdog" ) - user = User.query.filter_by(email="jen@example.org").one() token = user.generate_reset_token() rv = client.post( @@ -254,10 +260,10 @@ def test_user_can_get_reset_password_with_valid_token_differently_cased( mockdata, client, session ): with current_app.test_request_context(): + user = User.query.filter_by(is_administrator=True).first() form = PasswordResetForm( - email="JEN@EXAMPLE.ORG", password="catdog", password2="catdog" + email=user.email.upper(), password="catdog", password2="catdog" ) - user = User.query.filter_by(email="jen@example.org").one() token = user.generate_reset_token() rv = client.post( @@ -271,8 +277,9 @@ def test_user_can_get_reset_password_with_valid_token_differently_cased( def test_user_cannot_reset_password_with_invalid_token(mockdata, client, session): with current_app.test_request_context(): + user = User.query.filter_by(is_administrator=True).first() form = PasswordResetForm( - email="jen@example.org", password="catdog", password2="catdog" + email=user.email, password="catdog", password2="catdog" ) token = "beepboopbeep" @@ -290,7 +297,8 @@ def test_user_cannot_get_email_reset_token_sent_without_valid_password( ): with current_app.test_request_context(): login_user(client) - form = ChangeEmailForm(email="jen@example.org", password="dogdogdogdog") + user = User.query.filter_by(is_administrator=True).first() + form = ChangeEmailForm(email=user.email, password="dogdogdogdog") rv = client.post( url_for("auth.change_email_request"), data=form.data, follow_redirects=True @@ -304,7 +312,8 @@ def test_user_cannot_get_email_reset_token_sent_to_existing_email( ): with current_app.test_request_context(): login_user(client) - form = ChangeEmailForm(email="freddy@example.org", password="dogdogdogdog") + user = User.query.filter_by(is_administrator=True).first() + form = ChangeEmailForm(email=user.email, password="dogdogdogdog") rv = client.post( url_for("auth.change_email_request"), data=form.data, follow_redirects=True @@ -318,7 +327,8 @@ def test_user_cannot_get_email_reset_token_sent_to_existing_email_differently_ca ): with current_app.test_request_context(): login_user(client) - form = ChangeEmailForm(email="FREDDY@EXAMPLE.ORG", password="dogdogdogdog") + user = User.query.filter_by(is_administrator=True).first() + form = ChangeEmailForm(email=user.email.upper(), password="dogdogdogdog") rv = client.post( url_for("auth.change_email_request"), data=form.data, follow_redirects=True @@ -327,10 +337,12 @@ def test_user_cannot_get_email_reset_token_sent_to_existing_email_differently_ca assert b"An email with instructions to confirm your new email" not in rv.data -def test_user_can_get_email_reset_token_sent_with_password(mockdata, client, session): +def test_user_can_get_email_reset_token_sent_with_password( + mockdata, client, session, faker +): with current_app.test_request_context(): login_user(client) - form = ChangeEmailForm(email="alice@example.org", password="dog") + form = ChangeEmailForm(email=faker.ascii_email(), password="dog") rv = client.post( url_for("auth.change_email_request"), data=form.data, follow_redirects=True @@ -342,7 +354,7 @@ def test_user_can_get_email_reset_token_sent_with_password(mockdata, client, ses def test_user_can_change_email_with_valid_reset_token(mockdata, client, session): with current_app.test_request_context(): login_user(client) - user = User.query.filter_by(email="jen@example.org").one() + user = User.query.filter_by(is_administrator=False, is_disabled=False).first() token = user.generate_email_change_token("alice@example.org") rv = client.get( @@ -367,7 +379,7 @@ def test_user_cannot_change_email_with_invalid_reset_token(mockdata, client, ses def test_user_can_confirm_account_with_valid_token(mockdata, client, session): with current_app.test_request_context(): login_unconfirmed_user(client) - user = User.query.filter_by(email="freddy@example.org").one() + user = User.query.filter_by(confirmed=False).first() token = user.generate_confirmation_token() rv = client.get(url_for("auth.confirm", token=token), follow_redirects=True) @@ -400,7 +412,7 @@ def test_user_can_change_password_if_they_match(mockdata, client, session): assert b"Your password has been updated." in rv.data assert ( - f"{current_app.config['OO_MAIL_SUBJECT_PREFIX']} Your Password Has Changed" + f"{current_app.config[KEY_OO_MAIL_SUBJECT_PREFIX]} Your Password Has Changed" in str(log.output) ) @@ -457,10 +469,7 @@ def test_user_cannot_change_password_if_they_dont_match(mockdata, client, sessio def test_user_can_change_dept_pref(mockdata, client, session): with current_app.test_request_context(): login_user(client) - - test_department_id = 1 - - form = ChangeDefaultDepartmentForm(dept_pref=test_department_id) + form = ChangeDefaultDepartmentForm(dept_pref=AC_DEPT) rv = client.post( url_for("auth.change_dept"), data=form.data, follow_redirects=True @@ -469,4 +478,4 @@ def test_user_can_change_dept_pref(mockdata, client, session): assert b"Updated!" in rv.data user = User.query.filter_by(email="jen@example.org").one() - assert user.dept_pref == test_department_id + assert user.dept_pref == AC_DEPT diff --git a/OpenOversight/tests/routes/test_descriptions.py b/OpenOversight/tests/routes/test_descriptions.py index 6a6929b89..3a71d5343 100644 --- a/OpenOversight/tests/routes/test_descriptions.py +++ b/OpenOversight/tests/routes/test_descriptions.py @@ -45,7 +45,6 @@ def test_officer_descriptions_markdown(mockdata, client, session): rv = client.get(url_for("main.officer_profile", officer_id=1)) assert rv.status_code == HTTPStatus.OK html = rv.data.decode() - print(html) assert "

A markdown description

" in html assert "

A test description!

" in html @@ -55,9 +54,9 @@ def test_admins_cannot_inject_unsafe_html(mockdata, client, session): login_admin(client) officer = Officer.query.first() text_contents = "New description\n" - admin = User.query.filter_by(email="jen@example.org").first() + admin = User.query.filter_by(is_administrator=True).first() form = TextForm( - text_contents=text_contents, officer_id=officer.id, creator_id=admin.id + text_contents=text_contents, officer_id=officer.id, created_by=admin.id ) rv = client.post( @@ -77,9 +76,9 @@ def test_admins_can_create_descriptions(mockdata, client, session): login_admin(client) officer = Officer.query.first() text_contents = "I can haz descriptionz" - admin = User.query.filter_by(email="jen@example.org").first() + admin = User.query.filter_by(is_administrator=True).first() form = TextForm( - text_contents=text_contents, officer_id=officer.id, creator_id=admin.id + text_contents=text_contents, officer_id=officer.id, created_by=admin.id ) rv = client.post( @@ -103,9 +102,9 @@ def test_acs_can_create_descriptions(mockdata, client, session): login_ac(client) officer = Officer.query.first() description = "A description" - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() form = TextForm( - text_contents=description, officer_id=officer.id, creator_id=ac.id + text_contents=description, officer_id=officer.id, created_by=ac.id ) rv = client.post( @@ -127,6 +126,7 @@ def test_acs_can_create_descriptions(mockdata, client, session): def test_admins_can_edit_descriptions(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() officer = Officer.query.first() old_description = "meow" new_description = "I can haz editing descriptionz" @@ -134,7 +134,7 @@ def test_admins_can_edit_descriptions(mockdata, client, session): description = Description( text_contents=old_description, officer_id=officer.id, - creator_id=1, + created_by=user.id, created_at=original_date, updated_at=original_date, ) @@ -163,7 +163,7 @@ def test_admins_can_edit_descriptions(mockdata, client, session): def test_ac_can_edit_their_descriptions_in_their_department(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.filter_by(department_id=AC_DEPT).first() old_description = "meow" new_description = "I can haz editing descriptionz" @@ -171,7 +171,7 @@ def test_ac_can_edit_their_descriptions_in_their_department(mockdata, client, se description = Description( text_contents=old_description, officer_id=officer.id, - creator_id=ac.id, + created_by=ac.id, created_at=original_date, updated_at=original_date, ) @@ -200,7 +200,7 @@ def test_ac_can_edit_their_descriptions_in_their_department(mockdata, client, se def test_ac_can_edit_others_descriptions(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.filter_by(department_id=AC_DEPT).first() old_description = "meow" new_description = "I can haz editing descriptionz" @@ -208,7 +208,7 @@ def test_ac_can_edit_others_descriptions(mockdata, client, session): description = Description( text_contents=old_description, officer_id=officer.id, - creator_id=ac.id - 1, + created_by=ac.id, created_at=original_date, updated_at=original_date, ) @@ -237,18 +237,19 @@ def test_ac_can_edit_others_descriptions(mockdata, client, session): def test_ac_cannot_edit_descriptions_not_in_their_department(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.except_( Officer.query.filter_by(department_id=AC_DEPT) ).first() - ac = User.query.filter_by(email="raq929@example.org").first() + old_description = "meow" new_description = "I can haz editing descriptionz" original_date = datetime.now() description = Description( text_contents=old_description, officer_id=officer.id, - creator_id=ac.id, + created_by=ac.id, created_at=original_date, updated_at=original_date, ) @@ -294,12 +295,12 @@ def test_acs_can_delete_their_descriptions_in_their_department( ): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.filter_by(department_id=AC_DEPT).first() description = Description( text_contents="Hello", officer_id=officer.id, - creator_id=ac.id, + created_by=ac.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -323,13 +324,14 @@ def test_acs_cannot_delete_descriptions_not_in_their_department( ): with current_app.test_request_context(): login_ac(client) + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.except_( Officer.query.filter_by(department_id=AC_DEPT) ).first() description = Description( text_contents="Hello", officer_id=officer.id, - creator_id=2, + created_by=ac.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -353,11 +355,11 @@ def test_acs_can_get_edit_form_for_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) officer = Officer.query.filter_by(department_id=AC_DEPT).first() - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() description = Description( text_contents="Hello", officer_id=officer.id, - creator_id=ac.id, + created_by=ac.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -378,11 +380,11 @@ def test_acs_can_get_others_edit_form(mockdata, client, session): with current_app.test_request_context(): login_ac(client) officer = Officer.query.filter_by(department_id=AC_DEPT).first() - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() description = Description( text_contents="Hello", officer_id=officer.id, - creator_id=ac.id - 1, + created_by=ac.id - 1, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -402,13 +404,14 @@ def test_acs_can_get_others_edit_form(mockdata, client, session): def test_acs_cannot_get_edit_form_for_their_non_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.except_( Officer.query.filter_by(department_id=AC_DEPT) ).first() description = Description( text_contents="Hello", officer_id=officer.id, - creator_id=2, + created_by=ac.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -426,12 +429,13 @@ def test_acs_cannot_get_edit_form_for_their_non_dept(mockdata, client, session): def test_users_can_see_descriptions(mockdata, client, session): with current_app.test_request_context(): + admin = User.query.filter_by(is_administrator=True).first() officer = Officer.query.first() text_contents = "You can see me" description = Description( text_contents=text_contents, officer_id=officer.id, - creator_id=1, + created_by=admin.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -450,12 +454,13 @@ def test_users_can_see_descriptions(mockdata, client, session): def test_admins_can_see_descriptions(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + admin = User.query.filter_by(is_administrator=True).first() officer = Officer.query.first() text_contents = "Kittens see everything" description = Description( text_contents=text_contents, officer_id=officer.id, - creator_id=1, + created_by=admin.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -473,12 +478,13 @@ def test_admins_can_see_descriptions(mockdata, client, session): def test_acs_can_see_descriptions_in_their_department(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.filter_by(department_id=AC_DEPT).first() text_contents = "I can haz descriptionz" description = Description( text_contents=text_contents, officer_id=officer.id, - creator_id=1, + created_by=ac.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -500,12 +506,12 @@ def test_acs_can_see_descriptions_not_in_their_department(mockdata, client, sess Officer.query.filter_by(department_id=AC_DEPT) ).first() login_ac(client) - creator = User.query.get(1) + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() text_contents = "Hello it me" description = Description( text_contents=text_contents, officer_id=officer.id, - creator_id=creator.id, + created_by=ac.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -520,18 +526,18 @@ def test_acs_can_see_descriptions_not_in_their_department(mockdata, client, sess assert description in officer.descriptions assert rv.status_code == HTTPStatus.OK assert text_contents in response_text - assert creator.username in response_text + assert ac.username in response_text def test_anonymous_users_cannot_see_description_creators(mockdata, client, session): with current_app.test_request_context(): officer = Officer.query.first() - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() text_contents = "All we have is each other" description = Description( text_contents=text_contents, officer_id=officer.id, - creator_id=ac.id, + created_by=ac.id, created_at=datetime.now(), updated_at=datetime.now(), ) diff --git a/OpenOversight/tests/routes/test_image_tagging.py b/OpenOversight/tests/routes/test_image_tagging.py index be224c859..1516cc82c 100644 --- a/OpenOversight/tests/routes/test_image_tagging.py +++ b/OpenOversight/tests/routes/test_image_tagging.py @@ -8,7 +8,7 @@ from OpenOversight.app.main import views from OpenOversight.app.main.forms import FaceTag -from OpenOversight.app.models.database import Department, Face, Image, Officer +from OpenOversight.app.models.database import Department, Face, Image, Officer, User from OpenOversight.app.utils.constants import ENCODING_UTF_8 from OpenOversight.tests.conftest import AC_DEPT from OpenOversight.tests.routes.route_helpers import login_ac, login_admin, login_user @@ -151,6 +151,8 @@ def test_user_can_add_tag(mockdata, client, session): officer = Officer.query.filter_by(department_id=1).first() image = Image.query.filter_by(department_id=1).first() login_user(client) + user = User.query.filter_by(is_administrator=True).first() + form = FaceTag( officer_id=officer.id, image_id=image.id, @@ -158,6 +160,7 @@ def test_user_can_add_tag(mockdata, client, session): dataY=32, dataWidth=3, dataHeight=33, + created_by=user.id, ) rv = client.post( url_for("main.label_data", image_id=image.id), @@ -171,6 +174,8 @@ def test_user_can_add_tag(mockdata, client, session): def test_user_cannot_add_tag_if_it_exists(mockdata, client, session): with current_app.test_request_context(): login_user(client) + user = User.query.filter_by(is_administrator=True).first() + tag = Face.query.first() form = FaceTag( officer_id=tag.officer_id, @@ -179,6 +184,7 @@ def test_user_cannot_add_tag_if_it_exists(mockdata, client, session): dataY=32, dataWidth=3, dataHeight=33, + created_by=user.id, ) rv = client.post( @@ -195,6 +201,8 @@ def test_user_cannot_add_tag_if_it_exists(mockdata, client, session): def test_user_cannot_tag_nonexistent_officer(mockdata, client, session): with current_app.test_request_context(): login_user(client) + user = User.query.filter_by(is_administrator=True).first() + tag = Face.query.first() form = FaceTag( officer_id=999999999999999999, @@ -203,6 +211,7 @@ def test_user_cannot_tag_nonexistent_officer(mockdata, client, session): dataY=32, dataWidth=3, dataHeight=33, + created_by=user.id, ) rv = client.post( @@ -217,6 +226,8 @@ def test_user_cannot_tag_officer_mismatched_with_department(mockdata, client, se with current_app.test_request_context(): login_user(client) tag = Face.query.first() + user = User.query.filter_by(is_administrator=True).first() + form = FaceTag( officer_id=tag.officer_id, image_id=tag.original_image_id, @@ -224,6 +235,7 @@ def test_user_cannot_tag_officer_mismatched_with_department(mockdata, client, se dataY=32, dataWidth=3, dataHeight=33, + created_by=user.id, ) rv = client.post( @@ -338,6 +350,7 @@ def test_featured_tag_replaces_others(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() tag1 = Face.query.first() officer = Officer.query.filter_by(id=tag1.officer_id).one() @@ -357,6 +370,7 @@ def test_featured_tag_replaces_others(mockdata, client, session): dataY=32, dataWidth=3, dataHeight=33, + created_by=user.id, ) rv = client.post( url_for("main.label_data", image_id=second_image.id), diff --git a/OpenOversight/tests/routes/test_incidents.py b/OpenOversight/tests/routes/test_incidents.py index 9df9f62d3..cb587b7a5 100644 --- a/OpenOversight/tests/routes/test_incidents.py +++ b/OpenOversight/tests/routes/test_incidents.py @@ -13,7 +13,7 @@ LocationForm, OOIdForm, ) -from OpenOversight.app.models.database import Department, Incident, Officer +from OpenOversight.app.models.database import Department, Incident, Officer, User from OpenOversight.app.utils.constants import ENCODING_UTF_8 from OpenOversight.tests.conftest import AC_DEPT from OpenOversight.tests.routes.route_helpers import ( @@ -62,6 +62,7 @@ def test_route_admin_or_required(route, client, mockdata): def test_admins_can_create_basic_incidents(report_number, mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() test_date = datetime(2000, 5, 25, 1, 45) address_form = LocationForm( @@ -70,11 +71,12 @@ def test_admins_can_create_basic_incidents(report_number, mockdata, client, sess city="FFFFF", state="IA", zip_code="03435", + created_by=user.id, ) # These have to have a dropdown selected because if not, an empty Unicode # string is sent, which does not mach the '' selector. - link_form = LinkForm(link_type="video") - license_plates_form = LicensePlateForm(state="AZ") + link_form = LinkForm(link_type="video", created_by=user.id) + license_plates_form = LicensePlateForm(state="AZ", created_by=user.id) form = IncidentForm( date_field=str(test_date.date()), time_field=str(test_date.time()), @@ -85,6 +87,9 @@ def test_admins_can_create_basic_incidents(report_number, mockdata, client, sess links=[link_form.data], license_plates=[license_plates_form.data], officers=[], + created_by=user.id, + last_updated_by=user.id, + last_updated_at=datetime.now(), ) data = process_form_data(form.data) @@ -103,7 +108,8 @@ def test_admins_cannot_create_incident_with_invalid_report_number( ): with current_app.test_request_context(): login_admin(client) - date = datetime(2000, 5, 25, 1, 45) + user = User.query.filter_by(is_administrator=True).first() + test_date = datetime(2000, 5, 25, 1, 45) report_number = "Will Not Work! #45" address_form = LocationForm( @@ -112,14 +118,15 @@ def test_admins_cannot_create_incident_with_invalid_report_number( city="FFFFF", state="IA", zip_code="03435", + created_by=user.id, ) # These have to have a dropdown selected because if not, an empty Unicode # string is sent, which does not mach the '' selector. - link_form = LinkForm(link_type="video") - license_plates_form = LicensePlateForm(state="AZ") + link_form = LinkForm(link_type="video", created_by=user.id) + license_plates_form = LicensePlateForm(state="AZ", created_by=user.id) form = IncidentForm( - date_field=str(date.date()), - time_field=str(date.time()), + date_field=str(test_date.date()), + time_field=str(test_date.time()), report_number=report_number, description="Something happened", department="1", @@ -143,14 +150,15 @@ def test_admins_cannot_create_incident_with_invalid_report_number( def test_admins_can_edit_incident_date_and_address(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() inc = Incident.query.options( joinedload(Incident.links), joinedload(Incident.license_plates), joinedload(Incident.officers), ).first() inc_id = inc.id - new_date = date(2017, 6, 25) - new_time = time(1, 45) + test_date = date(2017, 6, 25) + test_time = time(1, 45) street_name = "Newest St" address_form = LocationForm( street_name=street_name, @@ -158,19 +166,21 @@ def test_admins_can_edit_incident_date_and_address(mockdata, client, session): city="Boston", state="NH", zip_code="03435", + created_by=user.id, ) links_forms = [ - LinkForm(url=link.url, link_type=link.link_type).data for link in inc.links + LinkForm(url=link.url, link_type=link.link_type, created_by=user.id).data + for link in inc.links ] license_plates_forms = [ - LicensePlateForm(number=lp.number, state=lp.state).data + LicensePlateForm(number=lp.number, state=lp.state, created_by=user.id).data for lp in inc.license_plates ] ooid_forms = [OOIdForm(ooid=officer.id) for officer in inc.officers] form = IncidentForm( - date_field=str(new_date), - time_field=str(new_time), + date_field=str(test_date), + time_field=str(test_time), report_number=inc.report_number, description=inc.description, department="1", @@ -178,6 +188,7 @@ def test_admins_can_edit_incident_date_and_address(mockdata, client, session): links=links_forms, license_plates=license_plates_forms, officers=ooid_forms, + created_by=user.id, ) data = process_form_data(form.data) @@ -189,14 +200,15 @@ def test_admins_can_edit_incident_date_and_address(mockdata, client, session): assert rv.status_code == HTTPStatus.OK assert "successfully updated" in rv.data.decode(ENCODING_UTF_8) updated = Incident.query.get(inc_id) - assert updated.date == new_date - assert updated.time == new_time + assert updated.date == test_date + assert updated.time == test_time assert updated.address.street_name == street_name def test_admins_can_edit_incident_links_and_licenses(mockdata, client, session, faker): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() inc = Incident.query.options( joinedload(Incident.links), joinedload(Incident.license_plates), @@ -210,16 +222,20 @@ def test_admins_can_edit_incident_links_and_licenses(mockdata, client, session, city=inc.address.city, state=inc.address.state, zip_code=inc.address.zip_code, + created_by=inc.created_by, ) old_links = inc.links old_links_forms = [ - LinkForm(url=link.url, link_type=link.link_type).data for link in inc.links + LinkForm(url=link.url, link_type=link.link_type, created_by=user.id).data + for link in inc.links ] new_url = faker.url() - link_form = LinkForm(url=new_url, link_type="video") + link_form = LinkForm(url=new_url, link_type="video", created_by=user.id) old_license_plates = inc.license_plates new_number = "453893" - license_plates_form = LicensePlateForm(number=new_number, state="IA") + license_plates_form = LicensePlateForm( + number=new_number, state="IA", created_by=user.id + ) ooid_forms = [OOIdForm(ooid=officer.id) for officer in inc.officers] form = IncidentForm( @@ -232,6 +248,7 @@ def test_admins_can_edit_incident_links_and_licenses(mockdata, client, session, links=old_links_forms + [link_form.data], license_plates=[license_plates_form.data], officers=ooid_forms, + created_by=user.id, ) data = process_form_data(form.data) @@ -255,6 +272,7 @@ def test_admins_can_edit_incident_links_and_licenses(mockdata, client, session, def test_admins_cannot_make_ancient_incidents(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() inc = Incident.query.options( joinedload(Incident.links), joinedload(Incident.license_plates), @@ -268,6 +286,7 @@ def test_admins_cannot_make_ancient_incidents(mockdata, client, session): city=inc.address.city, state=inc.address.state, zip_code=inc.address.zip_code, + created_by=inc.created_by, ) ooid_forms = [OOIdForm(ooid=officer.id) for officer in inc.officers] @@ -279,6 +298,7 @@ def test_admins_cannot_make_ancient_incidents(mockdata, client, session): department="1", address=address_form.data, officers=ooid_forms, + created_by=user.id, ) data = process_form_data(form.data) @@ -294,7 +314,8 @@ def test_admins_cannot_make_ancient_incidents(mockdata, client, session): def test_admins_cannot_make_incidents_without_state(mockdata, client, session): with current_app.test_request_context(): login_admin(client) - date = datetime(2000, 5, 25, 1, 45) + user = User.query.filter_by(is_administrator=True).first() + test_date = datetime(2000, 5, 25, 1, 45) report_number = "42" address_form = LocationForm( @@ -303,17 +324,19 @@ def test_admins_cannot_make_incidents_without_state(mockdata, client, session): city="FFFFF", state="", zip_code="03435", + created_by=user.id, ) ooid_forms = [OOIdForm(ooid=officer.id) for officer in Officer.query.all()[:5]] form = IncidentForm( - date_field=str(date.date()), - time_field=str(date.time()), + date_field=str(test_date.date()), + time_field=str(test_date.time()), report_number=report_number, description="Something happened", department="1", address=address_form.data, officers=ooid_forms, + created_by=user.id, ) data = process_form_data(form.data) @@ -331,7 +354,8 @@ def test_admins_cannot_make_incidents_with_multiple_validation_errors( ): with current_app.test_request_context(): login_admin(client) - date = datetime(2000, 5, 25, 1, 45) + user = User.query.filter_by(is_administrator=True).first() + test_date = datetime(2000, 5, 25, 1, 45) report_number = "42" address_form = LocationForm( @@ -342,16 +366,18 @@ def test_admins_cannot_make_incidents_with_multiple_validation_errors( state="NY", # invalid ZIP code => 'Zip codes must have 5 digits.' zip_code="0343", + created_by=user.id, ) - # license plate number given, but no state selected => 'Must also select a state.' + # license plate number given, but no state selected => + # 'Must also select a state.' license_plate_form = LicensePlateForm(number="ABCDE", state="") ooid_forms = [OOIdForm(ooid=officer.id) for officer in Officer.query.all()[:5]] form = IncidentForm( # no date given => 'This field is required.' date_field="", - time_field=str(date.time()), + time_field=str(test_date.time()), report_number=report_number, description="Something happened", # invalid department id => 'This field is required.' @@ -376,6 +402,8 @@ def test_admins_cannot_make_incidents_with_multiple_validation_errors( def test_admins_can_edit_incident_officers(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() + inc = Incident.query.options( joinedload(Incident.links), joinedload(Incident.license_plates), @@ -389,12 +417,14 @@ def test_admins_can_edit_incident_officers(mockdata, client, session): city=inc.address.city, state=inc.address.state, zip_code=inc.address.zip_code, + created_by=inc.created_by, ) links_forms = [ - LinkForm(url=link.url, link_type=link.link_type).data for link in inc.links + LinkForm(url=link.url, link_type=link.link_type, created_by=user.id).data + for link in inc.links ] license_plates_forms = [ - LicensePlateForm(number=lp.number, state=lp.state).data + LicensePlateForm(number=lp.number, state=lp.state, created_by=user.id).data for lp in inc.license_plates ] @@ -435,6 +465,8 @@ def test_admins_can_edit_incident_officers(mockdata, client, session): def test_admins_cannot_edit_nonexisting_officers(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() + inc = Incident.query.options( joinedload(Incident.links), joinedload(Incident.license_plates), @@ -448,9 +480,11 @@ def test_admins_cannot_edit_nonexisting_officers(mockdata, client, session): city=inc.address.city, state=inc.address.state, zip_code=inc.address.zip_code, + created_by=inc.created_by, ) links_forms = [ - LinkForm(url=link.url, link_type=link.link_type).data for link in inc.links + LinkForm(url=link.url, link_type=link.link_type, created_by=user.id).data + for link in inc.links ] license_plates_forms = [ LicensePlateForm(number=lp.number, state=lp.state).data @@ -490,8 +524,10 @@ def test_admins_cannot_edit_nonexisting_officers(mockdata, client, session): def test_ac_can_edit_incidents_in_their_department(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + user = User.query.filter_by(ac_department_id=AC_DEPT).first() + inc = Incident.query.filter_by(department_id=AC_DEPT).first() - new_date = datetime(2017, 6, 25, 1, 45) + test_date = datetime(2017, 6, 25, 1, 45) street_name = "Newest St" address_form = LocationForm( street_name=street_name, @@ -499,19 +535,21 @@ def test_ac_can_edit_incidents_in_their_department(mockdata, client, session): city="Boston", state="NH", zip_code="03435", + created_by=user.id, ) links_forms = [ - LinkForm(url=link.url, link_type=link.link_type).data for link in inc.links + LinkForm(url=link.url, link_type=link.link_type, created_by=user.id).data + for link in inc.links ] license_plates_forms = [ - LicensePlateForm(number=lp.number, state=lp.state).data + LicensePlateForm(number=lp.number, state=lp.state, created_by=user.id).data for lp in inc.license_plates ] ooid_forms = [OOIdForm(ooid=officer.id) for officer in inc.officers] form = IncidentForm( - date_field=str(new_date.date()), - time_field=str(new_date.time()), + date_field=str(test_date.date()), + time_field=str(test_date.time()), report_number=inc.report_number, description=inc.description, department=AC_DEPT, @@ -529,19 +567,22 @@ def test_ac_can_edit_incidents_in_their_department(mockdata, client, session): ) assert rv.status_code == HTTPStatus.OK assert "successfully updated" in rv.data.decode(ENCODING_UTF_8) - assert inc.date == new_date.date() - assert inc.time == new_date.time() + assert inc.date == test_date.date() + assert inc.time == test_date.time() assert inc.address.street_name == street_name def test_ac_cannot_edit_incidents_not_in_their_department(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + user = User.query.filter_by( + ac_department_id=None, is_administrator=False + ).first() inc = Incident.query.except_( Incident.query.filter_by(department_id=AC_DEPT) ).first() - new_date = datetime(2017, 6, 25, 1, 45) + test_date = datetime(2017, 6, 25, 1, 45) street_name = "Not Allowed St" address_form = LocationForm( street_name=street_name, @@ -549,19 +590,21 @@ def test_ac_cannot_edit_incidents_not_in_their_department(mockdata, client, sess city="Boston", state="NH", zip_code="03435", + created_by=user.id, ) links_forms = [ - LinkForm(url=link.url, link_type=link.link_type).data for link in inc.links + LinkForm(url=link.url, link_type=link.link_type, created_by=user.id).data + for link in inc.links ] license_plates_forms = [ - LicensePlateForm(number=lp.number, state=lp.state).data + LicensePlateForm(number=lp.number, state=lp.state, created_by=user.id).data for lp in inc.license_plates ] ooid_forms = [OOIdForm(ooid=officer.id) for officer in inc.officers] form = IncidentForm( - date_field=str(new_date.date()), - time_field=str(new_date.time()), + date_field=str(test_date.date()), + time_field=str(test_date.time()), report_number=inc.report_number, description=inc.description, department=AC_DEPT, @@ -711,6 +754,8 @@ def test_incident_markdown(mockdata, client, session): def test_admins_cannot_inject_unsafe_html(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() + inc = Incident.query.options( joinedload(Incident.links), joinedload(Incident.license_plates), @@ -724,12 +769,14 @@ def test_admins_cannot_inject_unsafe_html(mockdata, client, session): city=inc.address.city, state=inc.address.state, zip_code=inc.address.zip_code, + created_by=inc.created_by, ) links_forms = [ - LinkForm(url=link.url, link_type=link.link_type).data for link in inc.links + LinkForm(url=link.url, link_type=link.link_type, created_by=user.id).data + for link in inc.links ] license_plates_forms = [ - LicensePlateForm(number=lp.number, state=lp.state).data + LicensePlateForm(number=lp.number, state=lp.state, created_by=user.id).data for lp in inc.license_plates ] diff --git a/OpenOversight/tests/routes/test_notes.py b/OpenOversight/tests/routes/test_notes.py index 70d7f15c7..7e5f02ff8 100644 --- a/OpenOversight/tests/routes/test_notes.py +++ b/OpenOversight/tests/routes/test_notes.py @@ -47,9 +47,9 @@ def test_admins_cannot_inject_unsafe_html(mockdata, client, session): login_admin(client) officer = Officer.query.first() text_contents = "New note\n" - admin = User.query.filter_by(email="jen@example.org").first() + admin = User.query.filter_by(is_administrator=True).first() form = TextForm( - text_contents=text_contents, officer_id=officer.id, creator_id=admin.id + text_contents=text_contents, officer_id=officer.id, created_by=admin.id ) rv = client.post( @@ -69,9 +69,9 @@ def test_admins_can_create_notes(mockdata, client, session): login_admin(client) officer = Officer.query.first() text_contents = "I can haz notez" - admin = User.query.filter_by(email="jen@example.org").first() + admin = User.query.filter_by(is_administrator=True).first() form = TextForm( - text_contents=text_contents, officer_id=officer.id, creator_id=admin.id + text_contents=text_contents, officer_id=officer.id, created_by=admin.id ) rv = client.post( @@ -93,8 +93,8 @@ def test_acs_can_create_notes(mockdata, client, session): login_ac(client) officer = Officer.query.first() note = "I can haz notez" - ac = User.query.filter_by(email="raq929@example.org").first() - form = TextForm(text_contents=note, officer_id=officer.id, creator_id=ac.id) + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() + form = TextForm(text_contents=note, officer_id=officer.id, created_by=ac.id) rv = client.post( url_for("main.note_api", officer_id=officer.id), @@ -120,7 +120,7 @@ def test_admins_can_edit_notes(mockdata, client, session): note = Note( text_contents=old_note, officer_id=officer.id, - creator_id=1, + created_by=1, created_at=original_date, updated_at=original_date, ) @@ -146,7 +146,7 @@ def test_admins_can_edit_notes(mockdata, client, session): def test_ac_can_edit_their_notes_in_their_department(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.filter_by(department_id=AC_DEPT).first() old_note = "meow" new_note = "I can haz editing notez" @@ -154,7 +154,7 @@ def test_ac_can_edit_their_notes_in_their_department(mockdata, client, session): note = Note( text_contents=old_note, officer_id=officer.id, - creator_id=ac.id, + created_by=ac.id, created_at=original_date, updated_at=original_date, ) @@ -180,7 +180,7 @@ def test_ac_can_edit_their_notes_in_their_department(mockdata, client, session): def test_ac_can_edit_others_notes(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.filter_by(department_id=AC_DEPT).first() old_note = "meow" new_note = "I can haz editing notez" @@ -188,7 +188,7 @@ def test_ac_can_edit_others_notes(mockdata, client, session): note = Note( text_contents=old_note, officer_id=officer.id, - creator_id=ac.id - 1, + created_by=ac.id - 1, created_at=original_date, updated_at=original_date, ) @@ -218,14 +218,14 @@ def test_ac_cannot_edit_notes_not_in_their_department(mockdata, client, session) officer = Officer.query.except_( Officer.query.filter_by(department_id=AC_DEPT) ).first() - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() old_note = "meow" new_note = "I can haz editing notez" original_date = datetime.now() note = Note( text_contents=old_note, officer_id=officer.id, - creator_id=ac.id, + created_by=ac.id, created_at=original_date, updated_at=original_date, ) @@ -262,12 +262,12 @@ def test_admins_can_delete_notes(mockdata, client, session): def test_acs_can_delete_their_notes_in_their_department(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.filter_by(department_id=AC_DEPT).first() note = Note( text_contents="Hello", officer_id=officer.id, - creator_id=ac.id, + created_by=ac.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -292,7 +292,7 @@ def test_acs_cannot_delete_notes_not_in_their_department(mockdata, client, sessi note = Note( text_contents="Hello", officer_id=officer.id, - creator_id=2, + created_by=2, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -313,11 +313,11 @@ def test_acs_can_get_edit_form_for_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) officer = Officer.query.filter_by(department_id=AC_DEPT).first() - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() note = Note( text_contents="Hello", officer_id=officer.id, - creator_id=ac.id, + created_by=ac.id, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -335,11 +335,11 @@ def test_acs_can_get_others_edit_form(mockdata, client, session): with current_app.test_request_context(): login_ac(client) officer = Officer.query.filter_by(department_id=AC_DEPT).first() - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() note = Note( text_contents="Hello", officer_id=officer.id, - creator_id=ac.id - 1, + created_by=ac.id - 1, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -362,7 +362,7 @@ def test_acs_cannot_get_edit_form_for_their_non_dept(mockdata, client, session): note = Note( text_contents="Hello", officer_id=officer.id, - creator_id=2, + created_by=2, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -382,7 +382,7 @@ def test_users_cannot_see_notes(mockdata, client, session): note = Note( text_contents=text_contents, officer_id=officer.id, - creator_id=1, + created_by=1, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -406,7 +406,7 @@ def test_admins_can_see_notes(mockdata, client, session): note = Note( text_contents=text_contents, officer_id=officer.id, - creator_id=1, + created_by=1, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -429,7 +429,7 @@ def test_acs_can_see_notes_in_their_department(mockdata, client, session): note = Note( text_contents=text_contents, officer_id=officer.id, - creator_id=1, + created_by=1, created_at=datetime.now(), updated_at=datetime.now(), ) @@ -454,7 +454,7 @@ def test_acs_cannot_see_notes_not_in_their_department(mockdata, client, session) note = Note( text_contents=text_contents, officer_id=officer.id, - creator_id=1, + created_by=1, created_at=datetime.now(), updated_at=datetime.now(), ) diff --git a/OpenOversight/tests/routes/test_officer_and_department.py b/OpenOversight/tests/routes/test_officer_and_department.py index b70cd5544..352a3eba3 100644 --- a/OpenOversight/tests/routes/test_officer_and_department.py +++ b/OpenOversight/tests/routes/test_officer_and_department.py @@ -386,6 +386,7 @@ def test_admin_edit_assignment_validation_error( def test_ac_can_edit_officer_in_their_dept_assignment(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + user = User.query.filter_by(ac_department_id=AC_DEPT).first() star_no = "1234" new_star_no = "12345" @@ -398,6 +399,7 @@ def test_ac_can_edit_officer_in_their_dept_assignment(mockdata, client, session) job_title=job.id, start_date=date(2019, 1, 1), resign_date=date(2019, 12, 31), + created_by=user.id, ) # Remove existing assignments @@ -425,6 +427,7 @@ def test_ac_can_edit_officer_in_their_dept_assignment(mockdata, client, session) job_title=job.id, start_date=date(2019, 2, 1), resign_date=date(2019, 11, 30), + created_by=user.id, ) rv = client.post( @@ -499,9 +502,13 @@ def test_ac_cannot_edit_assignment_outside_their_dept(mockdata, client, session) def test_admin_can_add_police_department(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() form = DepartmentForm( - name=TestPD.name, short_name=TestPD.short_name, state=TestPD.state + name=TestPD.name, + short_name=TestPD.short_name, + state=TestPD.state, + created_by=user.id, ) rv = client.post( @@ -522,8 +529,11 @@ def test_admin_can_add_police_department(mockdata, client, session): def test_admin_cannot_add_police_department_without_state(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() - form = DepartmentForm(name=TestPD.name, short_name=TestPD.short_name, state="") + form = DepartmentForm( + name=TestPD.name, short_name=TestPD.short_name, state="", created_by=user.id + ) form.validate() errors = form.errors @@ -535,9 +545,13 @@ def test_admin_cannot_add_police_department_without_state(mockdata, client, sess def test_ac_cannot_add_police_department(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + user = User.query.filter_by(is_administrator=False).first() form = DepartmentForm( - name=TestPD.name, short_name=TestPD.short_name, state=TestPD.state + name=TestPD.name, + short_name=TestPD.short_name, + state=TestPD.state, + created_by=user.id, ) rv = client.post( @@ -550,9 +564,13 @@ def test_ac_cannot_add_police_department(mockdata, client, session): def test_admin_cannot_add_duplicate_police_department(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() form = DepartmentForm( - name=TestPD.name, short_name=TestPD.short_name, state=TestPD.state + name=TestPD.name, + short_name=TestPD.short_name, + state=TestPD.state, + created_by=user.id, ) rv = client.post( @@ -589,11 +607,13 @@ def test_admin_can_edit_police_department(mockdata, client, session): ) login_admin(client) + user = User.query.filter_by(is_administrator=True).first() misspelled_form = DepartmentForm( name=MisspelledPD.name, short_name=MisspelledPD.short_name, state=MisspelledPD.state, + created_by=user.id, ) misspelled_rv = client.post( @@ -691,11 +711,13 @@ def test_admin_can_edit_police_department(mockdata, client, session): def test_admin_cannot_edit_police_department_without_state(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() add_department_form = DepartmentForm( name=TestPD.name, short_name=TestPD.short_name, state=TestPD.state, + created_by=user.id, ) add_department_rv = client.post( @@ -710,7 +732,7 @@ def test_admin_cannot_edit_police_department_without_state(mockdata, client, ses ) without_state_form = EditDepartmentForm( - name=TestPD.name, short_name=TestPD.short_name, state="" + name=TestPD.name, short_name=TestPD.short_name, state="", created_by=user.id ) without_state_form.validate() @@ -1000,11 +1022,13 @@ def test_admin_can_create_department_with_same_name_in_different_state( ): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(ac_department_id=AC_DEPT).first() existing_form = DepartmentForm( name=ExistingPD.name, short_name=ExistingPD.short_name, state=ExistingPD.state, + created_by=user.id, ) existing_rv = client.post( @@ -1033,6 +1057,7 @@ def test_admin_can_create_department_with_same_name_in_different_state( name=ExistingDiffStatePD.name, short_name=ExistingDiffStatePD.short_name, state=ExistingDiffStatePD.state, + created_by=user.id, ) existing_diff_state_rv = client.post( @@ -1057,6 +1082,7 @@ def test_admin_can_create_department_with_same_name_in_different_state( name=ExistingPD.name, short_name=ExistingPD.short_name, state=ExistingPD.state, + created_by=user.id, ) existing_duplicate_rv = client.post( @@ -1075,11 +1101,13 @@ def test_admin_cannot_duplicate_police_department_during_edit( ): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() existing_dep_form = DepartmentForm( name=ExistingPD.name, short_name=ExistingPD.short_name, state=ExistingPD.state, + created_by=user.id, ) existing_dep_rv = client.post( @@ -1096,7 +1124,10 @@ def test_admin_cannot_duplicate_police_department_during_edit( NewPD = PoliceDepartment("New Police Department", "NPD", ExistingPD.state) new_dep_form = DepartmentForm( - name=NewPD.name, short_name=NewPD.short_name, state=NewPD.state + name=NewPD.name, + short_name=NewPD.short_name, + state=NewPD.state, + created_by=user.id, ) new_dep_rv = client.post( @@ -1146,6 +1177,8 @@ def test_expected_dept_appears_in_submission_dept_selection(mockdata, client, se def test_admin_can_add_new_officer(mockdata, client, session, department, faker): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() + links = [ LinkForm(url=faker.url(), link_type="link").data, LinkForm(url=faker.url(), link_type="video").data, @@ -1162,6 +1195,7 @@ def test_admin_can_add_new_officer(mockdata, client, session, department, faker) department=department.id, birth_year=1990, links=links, + created_by=user.id, ) data = process_form_data(form.data) @@ -1182,6 +1216,8 @@ def test_admin_can_add_new_officer_with_unit( ): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() + unit = random.choice(unit_choices()) links = [ LinkForm(url=faker.url(), link_type="link").data, @@ -1200,6 +1236,7 @@ def test_admin_can_add_new_officer_with_unit( department=department.id, birth_year=1990, links=links, + created_by=user.id, ) data = process_form_data(form.data) @@ -1213,7 +1250,7 @@ def test_admin_can_add_new_officer_with_unit( assert officer.first_name == "Test" assert officer.race == "WHITE" assert officer.gender == "M" - assert Assignment.query.filter_by(baseofficer=officer, unit=unit).one() + assert Assignment.query.filter_by(base_officer=officer, unit=unit).one() def test_ac_can_add_new_officer_in_their_dept(mockdata, client, session): @@ -1300,12 +1337,14 @@ def test_ac_can_add_new_officer_with_unit_in_their_dept(mockdata, client, sessio ) else: assert officer.gender == gender - assert Assignment.query.filter_by(baseofficer=officer, unit=unit).one() + assert Assignment.query.filter_by(base_officer=officer, unit=unit).one() def test_ac_cannot_add_new_officer_not_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + user = User.query.filter_by(is_administrator=True).first() + department = Department.query.except_( Department.query.filter_by(id=AC_DEPT) ).first() @@ -1325,6 +1364,7 @@ def test_ac_cannot_add_new_officer_not_in_their_dept(mockdata, client, session): job_id=job.id, department=department.id, birth_year=1990, + created_by=user.id, ) data = process_form_data(form.data) @@ -1338,12 +1378,14 @@ def test_ac_cannot_add_new_officer_not_in_their_dept(mockdata, client, session): def test_admin_can_edit_existing_officer(mockdata, client, session, department, faker): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() + unit = random.choice(unit_choices()) link_url0 = faker.url() link_url1 = faker.url() links = [ - LinkForm(url=link_url0, link_type="link").data, - LinkForm(url=link_url0, link_type="video").data, + LinkForm(url=link_url0, link_type="link", created_by=user.id).data, + LinkForm(url=link_url0, link_type="video", created_by=user.id).data, ] job = Job.query.filter_by(department_id=department.id).first() form = AddOfficerForm( @@ -1358,10 +1400,11 @@ def test_admin_can_edit_existing_officer(mockdata, client, session, department, unit=unit.id, birth_year=1990, links=links, + created_by=user.id, ) data = process_form_data(form.data) - rv = client.post(url_for("main.add_officer"), data=data, follow_redirects=True) + client.post(url_for("main.add_officer"), data=data, follow_redirects=True) officer = Officer.query.filter_by(last_name="Testerinski").one() @@ -1384,7 +1427,7 @@ def test_ac_cannot_edit_officer_not_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - officer = officer = Officer.query.except_( + officer = Officer.query.except_( Officer.query.filter_by(department_id=AC_DEPT) ).first() old_last_name = officer.last_name @@ -1453,7 +1496,7 @@ def test_ac_can_edit_officer_in_their_dept(mockdata, client, session): data = process_form_data(form.data) - rv = client.post(url_for("main.add_officer"), data=data, follow_redirects=True) + client.post(url_for("main.add_officer"), data=data, follow_redirects=True) officer = Officer.query.filter_by(last_name=last_name).one() @@ -1487,6 +1530,7 @@ def test_admin_adds_officer_without_middle_initial( ): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() job = Job.query.filter_by(department_id=department.id).first() form = AddOfficerForm( @@ -1498,6 +1542,7 @@ def test_admin_adds_officer_without_middle_initial( job_id=job.id, department=department.id, birth_year=1990, + created_by=user.id, ) data = process_form_data(form.data) @@ -1518,6 +1563,7 @@ def test_admin_adds_officer_with_letter_in_badge_no( ): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() job = Job.query.filter_by(department_id=department.id).first() form = AddOfficerForm( @@ -1530,6 +1576,7 @@ def test_admin_adds_officer_with_letter_in_badge_no( job_id=job.id, department=department.id, birth_year=1990, + created_by=user.id, ) data = process_form_data(form.data) @@ -1548,8 +1595,11 @@ def test_admin_adds_officer_with_letter_in_badge_no( def test_admin_can_add_new_unit(mockdata, client, session, department): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() - form = AddUnitForm(description="Test", department=department.id) + form = AddUnitForm( + description="Test", department=department.id, created_by=user.id + ) rv = client.post( url_for("main.add_unit"), data=form.data, follow_redirects=True @@ -1565,9 +1615,12 @@ def test_admin_can_add_new_unit(mockdata, client, session, department): def test_ac_can_add_new_unit_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + user = User.query.filter_by(ac_department_id=AC_DEPT).first() department = Department.query.filter_by(id=AC_DEPT).first() - form = AddUnitForm(description="Test", department=department.id) + form = AddUnitForm( + description="Test", department=department.id, created_by=user.id + ) rv = client.post( url_for("main.add_unit"), data=form.data, follow_redirects=True @@ -1601,9 +1654,11 @@ def test_admin_can_add_new_officer_with_suffix( ): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() + links = [ - LinkForm(url=faker.url(), link_type="link").data, - LinkForm(url=faker.url(), link_type="video").data, + LinkForm(url=faker.url(), link_type="link", created_by=user.id).data, + LinkForm(url=faker.url(), link_type="video", created_by=user.id).data, ] job = Job.query.filter_by(department_id=department.id).first() form = AddOfficerForm( @@ -1618,6 +1673,7 @@ def test_admin_can_add_new_officer_with_suffix( department=department.id, birth_year=1990, links=links, + created_by=user.id, ) data = process_form_data(form.data) @@ -1743,6 +1799,7 @@ def test_assignments_csv(mockdata, client, session, department): def test_incidents_csv(mockdata, client, session, department, faker): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() # Delete existing incidents for chosen department Incident.query.filter_by(department_id=department.id).delete() @@ -1750,9 +1807,11 @@ def test_incidents_csv(mockdata, client, session, department, faker): incident_date = datetime(2000, 5, 25, 1, 45) report_number = "42" - address_form = LocationForm(street_name="ABCDE", city="FGHI", state="IA") - link_form = LinkForm(url=faker.url(), link_type="video") - license_plates_form = LicensePlateForm(state="AZ") + address_form = LocationForm( + street_name="ABCDE", city="FGHI", state="IA", created_by=user.id + ) + link_form = LinkForm(url=faker.url(), link_type="video", created_by=user.id) + license_plates_form = LicensePlateForm(state="AZ", created_by=user.id) form = IncidentForm( date_field=str(incident_date.date()), time_field=str(incident_date.time()), @@ -1764,6 +1823,7 @@ def test_incidents_csv(mockdata, client, session, department, faker): links=[link_form.data], license_plates=[license_plates_form.data], officers=[], + created_by=user.id, ) # add the incident rv = client.post( @@ -2166,9 +2226,14 @@ def test_edit_officers_with_blank_uids(mockdata, client, session): def test_admin_can_add_salary(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() form = SalaryForm( - salary=123456.78, overtime_pay=666.66, year=2019, is_fiscal_year=False + salary=123456.78, + overtime_pay=666.66, + year=2019, + is_fiscal_year=False, + created_by=user.id, ) rv = client.post( @@ -2187,9 +2252,14 @@ def test_admin_can_add_salary(mockdata, client, session): def test_ac_can_add_salary_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + user = User.query.filter_by(ac_department_id=AC_DEPT).first() form = SalaryForm( - salary=123456.78, overtime_pay=666.66, year=2019, is_fiscal_year=False + salary=123456.78, + overtime_pay=666.66, + year=2019, + is_fiscal_year=False, + created_by=user.id, ) officer = Officer.query.filter_by(department_id=AC_DEPT).first() @@ -2229,12 +2299,17 @@ def test_ac_cannot_add_non_dept_salary(mockdata, client, session): def test_admin_can_edit_salary(mockdata, client, session): with current_app.test_request_context(): login_admin(client) + user = User.query.filter_by(is_administrator=True).first() # Remove existing salaries Salary.query.filter_by(officer_id=1).delete() form = SalaryForm( - salary=123456.78, overtime_pay=666.66, year=2019, is_fiscal_year=False + salary=123456.78, + overtime_pay=666.66, + year=2019, + is_fiscal_year=False, + created_by=user.id, ) rv = client.post( @@ -2246,7 +2321,7 @@ def test_admin_can_edit_salary(mockdata, client, session): assert "Added new salary" in rv.data.decode(ENCODING_UTF_8) assert "$123,456.78" in rv.data.decode(ENCODING_UTF_8) - form = SalaryForm(salary=150000) + form = SalaryForm(salary=150000, created_by=user.id) officer = Officer.query.filter_by(id=1).one() rv = client.post( @@ -2270,6 +2345,7 @@ def test_admin_can_edit_salary(mockdata, client, session): def test_ac_can_edit_salary_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) + user = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.filter_by(department_id=AC_DEPT).first() officer_id = officer.id @@ -2277,7 +2353,11 @@ def test_ac_can_edit_salary_in_their_dept(mockdata, client, session): Salary.query.filter_by(officer_id=officer_id).delete() form = SalaryForm( - salary=123456.78, overtime_pay=666.66, year=2019, is_fiscal_year=False + salary=123456.78, + overtime_pay=666.66, + year=2019, + is_fiscal_year=False, + created_by=user.id, ) rv = client.post( @@ -2289,7 +2369,7 @@ def test_ac_can_edit_salary_in_their_dept(mockdata, client, session): assert "Added new salary" in rv.data.decode(ENCODING_UTF_8) assert "$123,456.78" in rv.data.decode(ENCODING_UTF_8) - form = SalaryForm(salary=150000) + form = SalaryForm(salary=150000, created_by=user.id) officer = Officer.query.filter_by(id=officer_id).one() rv = client.post( @@ -2383,7 +2463,7 @@ def test_get_department_ranks_with_no_department(mockdata, client, session): def test_admin_can_add_link_to_officer_profile(mockdata, client, session): with current_app.test_request_context(): login_admin(client) - admin = User.query.filter_by(email="jen@example.org").first() + admin = User.query.filter_by(is_administrator=True).first() officer = Officer.query.first() form = OfficerLinkForm( @@ -2392,7 +2472,7 @@ def test_admin_can_add_link_to_officer_profile(mockdata, client, session): author="OJB", url="https://bpdwatch.com", link_type="link", - creator_id=admin.id, + created_by=admin.id, officer_id=officer.id, ) @@ -2410,7 +2490,7 @@ def test_admin_can_add_link_to_officer_profile(mockdata, client, session): def test_ac_can_add_link_to_officer_profile_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.filter_by(department_id=AC_DEPT).first() form = OfficerLinkForm( @@ -2419,7 +2499,7 @@ def test_ac_can_add_link_to_officer_profile_in_their_dept(mockdata, client, sess author="OJB", url="https://bpdwatch.com", link_type="link", - creator_id=ac.id, + created_by=ac.id, officer_id=officer.id, ) @@ -2439,7 +2519,7 @@ def test_ac_cannot_add_link_to_officer_profile_not_in_their_dept( ): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() officer = Officer.query.except_( Officer.query.filter_by(department_id=AC_DEPT) ).first() @@ -2450,7 +2530,7 @@ def test_ac_cannot_add_link_to_officer_profile_not_in_their_dept( author="OJB", url="https://bpdwatch.com", link_type="link", - creator_id=ac.id, + created_by=ac.id, officer_id=officer.id, ) @@ -2477,7 +2557,7 @@ def test_admin_can_edit_link_on_officer_profile(mockdata, client, session): author=link.author, url=link.url, link_type=link.link_type, - creator_id=link.creator_id, + created_by=link.created_by, officer_id=officer.id, ) @@ -2495,7 +2575,7 @@ def test_admin_can_edit_link_on_officer_profile(mockdata, client, session): def test_ac_can_edit_link_on_officer_profile_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() # Officer from department with id AC_DEPT and no links officer = ( Officer.query.filter_by(department_id=AC_DEPT) @@ -2512,7 +2592,7 @@ def test_ac_can_edit_link_on_officer_profile_in_their_dept(mockdata, client, ses author="OJB", url="https://bpdwatch.com", link_type="link", - creator_id=ac.id, + created_by=ac.id, officer_id=officer.id, ) @@ -2533,7 +2613,7 @@ def test_ac_can_edit_link_on_officer_profile_in_their_dept(mockdata, client, ses author=link.author, url=link.url, link_type=link.link_type, - creator_id=link.creator_id, + created_by=link.created_by, officer_id=officer.id, ) @@ -2553,7 +2633,7 @@ def test_ac_cannot_edit_link_on_officer_profile_not_in_their_dept( ): with current_app.test_request_context(): login_admin(client) - admin = User.query.filter_by(email="jen@example.org").first() + admin = User.query.filter_by(is_administrator=True).first() # Officer from another department (not id AC_DEPT) and no links officer = ( Officer.query.filter(Officer.department_id != AC_DEPT) @@ -2570,7 +2650,7 @@ def test_ac_cannot_edit_link_on_officer_profile_not_in_their_dept( author="OJB", url="https://bpdwatch.com", link_type="link", - creator_id=admin.id, + created_by=admin.id, officer_id=officer.id, ) @@ -2593,7 +2673,7 @@ def test_ac_cannot_edit_link_on_officer_profile_not_in_their_dept( author=link.author, url=link.url, link_type=link.link_type, - creator_id=link.creator_id, + created_by=link.created_by, officer_id=officer.id, ) @@ -2634,7 +2714,7 @@ def test_ac_can_delete_link_from_officer_profile_in_their_dept( ): with current_app.test_request_context(): login_ac(client) - ac = User.query.filter_by(email="raq929@example.org").first() + ac = User.query.filter_by(ac_department_id=AC_DEPT).first() # Officer from department with id AC_DEPT and no links officer = ( Officer.query.filter_by(department_id=AC_DEPT) @@ -2651,7 +2731,7 @@ def test_ac_can_delete_link_from_officer_profile_in_their_dept( author="OJB", url="https://bpdwatch.com", link_type="link", - creator_id=ac.id, + created_by=ac.id, officer_id=officer.id, ) @@ -2680,7 +2760,7 @@ def test_ac_cannot_delete_link_from_officer_profile_not_in_their_dept( ): with current_app.test_request_context(): login_admin(client) - admin = User.query.filter_by(email="jen@example.org").first() + admin = User.query.filter_by(is_administrator=True).first() # Officer from another department (not id AC_DEPT) and no links officer = ( Officer.query.filter(Officer.department_id != AC_DEPT) @@ -2697,7 +2777,7 @@ def test_ac_cannot_delete_link_from_officer_profile_not_in_their_dept( author="OJB", url="https://bpdwatch.com", link_type="link", - creator_id=admin.id, + created_by=admin.id, officer_id=officer.id, ) diff --git a/OpenOversight/tests/routes/test_user_api.py b/OpenOversight/tests/routes/test_user_api.py index c2bee622f..e591e61c2 100644 --- a/OpenOversight/tests/routes/test_user_api.py +++ b/OpenOversight/tests/routes/test_user_api.py @@ -1,4 +1,3 @@ -# Routing and view tests from http import HTTPMethod, HTTPStatus import pytest @@ -9,12 +8,7 @@ from OpenOversight.app.models.database import User, db from OpenOversight.app.utils.constants import ENCODING_UTF_8 from OpenOversight.tests.conftest import AC_DEPT -from OpenOversight.tests.routes.route_helpers import ( - ADMIN_EMAIL, - login_ac, - login_admin, - login_user, -) +from OpenOversight.tests.routes.route_helpers import login_ac, login_admin, login_user routes_methods = [ @@ -64,14 +58,13 @@ def test_admin_can_update_users_to_ac(mockdata, client, session): login_admin(client) user = User.query.except_(User.query.filter_by(is_administrator=True)).first() - user_id = user.id form = EditUserForm( is_area_coordinator=True, ac_department=AC_DEPT, submit=True ) rv = client.post( - url_for("auth.edit_user", user_id=user_id), + url_for("auth.edit_user", user_id=user.id), data=form.data, follow_redirects=True, ) @@ -85,12 +78,11 @@ def test_admin_cannot_update_to_ac_without_department(mockdata, client, session) login_admin(client) user = User.query.except_(User.query.filter_by(is_administrator=True)).first() - user_id = user.id form = EditUserForm(is_area_coordinator=True, submit=True) rv = client.post( - url_for("auth.edit_user", user_id=user_id), + url_for("auth.edit_user", user_id=user.id), data=form.data, follow_redirects=True, ) @@ -104,14 +96,13 @@ def test_admin_can_update_users_to_admin(mockdata, client, session): login_admin(client) user = User.query.except_(User.query.filter_by(is_administrator=True)).first() - user_id = user.id form = EditUserForm( is_area_coordinator=False, is_administrator=True, submit=True ) rv = client.post( - url_for("auth.edit_user", user_id=user_id), + url_for("auth.edit_user", user_id=user.id), data=form.data, follow_redirects=True, ) @@ -125,21 +116,21 @@ def test_admin_can_delete_user(mockdata, client, session): login_admin(client) user = User.query.first() - user_id = user.id - username = user.username rv = client.get( - url_for("auth.delete_user", user_id=user_id), + url_for("auth.delete_user", user_id=user.id), ) assert b"Are you sure you want to delete this user?" in rv.data rv = client.post( - url_for("auth.delete_user", user_id=user_id), follow_redirects=True + url_for("auth.delete_user", user_id=user.id), follow_redirects=True ) - assert f"User {username} has been deleted!" in rv.data.decode(ENCODING_UTF_8) - assert not User.query.get(user_id) + assert f"User {user.username} has been deleted!" in rv.data.decode( + ENCODING_UTF_8 + ) + assert not User.query.get(user.id) def test_admin_cannot_delete_other_admin(mockdata, client, session): @@ -149,14 +140,13 @@ def test_admin_cannot_delete_other_admin(mockdata, client, session): user = User(is_administrator=True, email="another_user@example.org") session.add(user) session.commit() - user_id = user.id rv = client.post( - url_for("auth.delete_user", user_id=user_id), follow_redirects=True + url_for("auth.delete_user", user_id=user.id), follow_redirects=True ) assert rv.status_code == HTTPStatus.FORBIDDEN - assert User.query.get(user_id) is not None + assert User.query.get(user.id) is not None def test_admin_can_disable_user(mockdata, client, session): @@ -165,7 +155,6 @@ def test_admin_can_disable_user(mockdata, client, session): # just need to make sure to not select the admin user user = User.query.filter_by(is_administrator=False).first() - user_id = user.id assert not user.is_disabled @@ -175,14 +164,14 @@ def test_admin_can_disable_user(mockdata, client, session): ) rv = client.post( - url_for("auth.edit_user", user_id=user_id), + url_for("auth.edit_user", user_id=user.id), data=form.data, follow_redirects=True, ) assert "updated!" in rv.data.decode(ENCODING_UTF_8) - user = User.query.get(user_id) + user = User.query.get(user.id) assert user.is_disabled @@ -190,8 +179,7 @@ def test_admin_cannot_disable_self(mockdata, client, session): with current_app.test_request_context(): login_admin(client) - user = User.query.filter_by(email=ADMIN_EMAIL).first() - user_id = user.id + user = User.query.filter_by(is_administrator=True).first() assert not user.is_disabled @@ -201,14 +189,14 @@ def test_admin_cannot_disable_self(mockdata, client, session): ) rv = client.post( - url_for("auth.edit_user", user_id=user_id), + url_for("auth.edit_user", user_id=user.id), data=form.data, follow_redirects=True, ) assert "You cannot edit your own account!" in rv.data.decode(ENCODING_UTF_8) - user = User.query.get(user_id) + user = User.query.get(user.id) assert not user.is_disabled @@ -217,11 +205,10 @@ def test_admin_can_enable_user(mockdata, client, session): login_admin(client) user = User.query.filter_by(is_administrator=False).first() - user_id = user.id user.is_disabled = True db.session.commit() - user = User.query.get(user_id) + user = User.query.get(user.id) assert user.is_disabled form = EditUserForm( @@ -230,14 +217,14 @@ def test_admin_can_enable_user(mockdata, client, session): ) rv = client.post( - url_for("auth.edit_user", user_id=user_id), + url_for("auth.edit_user", user_id=user.id), data=form.data, follow_redirects=True, ) assert "updated!" in rv.data.decode(ENCODING_UTF_8) - user = User.query.get(user_id) + user = User.query.get(user.id) assert not user.is_disabled @@ -246,21 +233,20 @@ def test_admin_can_resend_user_confirmation_email(mockdata, client, session): login_admin(client) user = User.query.filter_by(confirmed=False).first() - user_id = user.id - email = user.email form = EditUserForm( resend=True, ) rv = client.post( - url_for("auth.edit_user", user_id=user_id), + url_for("auth.edit_user", user_id=user.id), data=form.data, follow_redirects=True, ) - assert f"A new confirmation email has been sent to {email}." in rv.data.decode( - ENCODING_UTF_8 + assert ( + f"A new confirmation email has been sent to {user.email}." + in rv.data.decode(ENCODING_UTF_8) ) @@ -302,11 +288,10 @@ def test_admin_can_approve_user(mockdata, client, session): login_admin(client) user = User.query.filter_by(is_administrator=False).first() - user_id = user.id user.approved = False db.session.commit() - user = User.query.get(user_id) + user = User.query.get(user.id) assert not user.approved form = EditUserForm( @@ -315,14 +300,14 @@ def test_admin_can_approve_user(mockdata, client, session): ) rv = client.post( - url_for("auth.edit_user", user_id=user_id), + url_for("auth.edit_user", user_id=user.id), data=form.data, follow_redirects=True, ) assert "updated!" in rv.data.decode(ENCODING_UTF_8) - user = User.query.get(user_id) + user = User.query.get(user.id) assert user.approved @@ -356,19 +341,18 @@ def test_admin_approval_sends_confirmation_email( login_admin(client) user = User.query.filter_by(is_administrator=False).first() - user_id = user.id user.approved = currently_approved user.confirmed = currently_confirmed db.session.commit() - user = User.query.get(user_id) + user = User.query.get(user.id) assert user.approved == currently_approved assert user.confirmed == currently_confirmed form = EditUserForm(approved=True, submit=True, confirmed=currently_confirmed) rv = client.post( - url_for("auth.edit_user", user_id=user_id), + url_for("auth.edit_user", user_id=user.id), data=form.data, follow_redirects=True, ) @@ -378,5 +362,5 @@ def test_admin_approval_sends_confirmation_email( ) == should_send_email assert "updated!" in rv.data.decode(ENCODING_UTF_8) - user = User.query.get(user_id) + user = User.query.get(user.id) assert user.approved diff --git a/OpenOversight/tests/test_commands.py b/OpenOversight/tests/test_commands.py index 90a1280f6..dc81be39d 100644 --- a/OpenOversight/tests/test_commands.py +++ b/OpenOversight/tests/test_commands.py @@ -27,6 +27,7 @@ Officer, Salary, Unit, + User, ) from OpenOversight.app.utils.choices import DEPARTMENT_STATE_CHOICES from OpenOversight.app.utils.db import get_officer @@ -51,8 +52,6 @@ def run_command_print_output(cli, args=None, **kwargs): """ runner = CliRunner() result = runner.invoke(cli, args=args, **kwargs) - print(result.output) - print(result.stderr_bytes) if result.exception is not None: print(result.exception) print(traceback.print_exception(*result.exc_info)) @@ -497,10 +496,9 @@ def test_bulk_add_officers__success( session, department_without_officers, csv_path, monkeypatch ): monkeypatch.setattr("builtins.input", lambda: "y") + user = User.query.filter_by(is_administrator=False).first() # generate two officers with different names - first_officer = generate_officer(department_without_officers) - print(Job.query.all()) - print(Job.query.filter_by(department=department_without_officers).all()) + first_officer = generate_officer(department_without_officers, user) job = ( Job.query.filter_by(department=department_without_officers).filter_by(order=1) ).first() @@ -509,15 +507,15 @@ def test_bulk_add_officers__success( fo_ln = first_officer.last_name session.add(first_officer) session.commit() - assignment = Assignment(baseofficer=first_officer, job_id=job.id) + assignment = Assignment(base_officer=first_officer, job_id=job.id) session.add(assignment) session.commit() - different_officer = generate_officer(department_without_officers) + different_officer = generate_officer(department_without_officers, user) different_officer.job = job do_fn = different_officer.first_name do_ln = different_officer.last_name session.add(different_officer) - assignment = Assignment(baseofficer=different_officer, job=job) + assignment = Assignment(base_officer=different_officer, job=job, created_by=user.id) session.add(assignment) session.commit() @@ -590,15 +588,16 @@ def test_bulk_add_officers__success( def test_bulk_add_officers__duplicate_name(session, department, csv_path): # two officers with the same name + user = User.query.filter_by(is_administrator=False).first() first_name = "James" last_name = "Smith" - first_officer = generate_officer(department) + first_officer = generate_officer(department, user) first_officer.first_name = first_name first_officer.last_name = last_name session.add(first_officer) session.commit() - different_officer = generate_officer(department) + different_officer = generate_officer(department, user) different_officer.first_name = first_name different_officer.last_name = last_name session.add(different_officer) @@ -637,8 +636,9 @@ def test_bulk_add_officers__write_static_null_field( session, department, csv_path, monkeypatch ): monkeypatch.setattr("builtins.input", lambda: "y") + user = User.query.filter_by(is_administrator=False).first() # start with an officer whose birth_year is missing - officer = generate_officer(department) + officer = generate_officer(department, user) officer.birth_year = None session.add(officer) session.commit() @@ -680,8 +680,9 @@ def test_bulk_add_officers__write_static_null_field( def test_bulk_add_officers__write_static_field_no_flag(session, department, csv_path): + user = User.query.filter_by(is_administrator=False).first() # officer with birth year set - officer = generate_officer(department) + officer = generate_officer(department, user) old_birth_year = 1979 officer.birth_year = old_birth_year session.add(officer) @@ -729,8 +730,9 @@ def test_bulk_add_officers__write_static_field__flag_set( session, department, csv_path, monkeypatch ): monkeypatch.setattr("builtins.input", lambda: "y") + user = User.query.filter_by(is_administrator=False).first() # officer with birth year set - officer = generate_officer(department) + officer = generate_officer(department, user) officer.birth_year = 1979 session.add(officer) session.commit() @@ -778,9 +780,10 @@ def test_bulk_add_officers__no_create_flag( session, department_without_officers, csv_path, monkeypatch ): monkeypatch.setattr("builtins.input", lambda: "y") + user = User.query.filter_by(is_administrator=False).first() # department with one officer department_id = department_without_officers.id - officer = generate_officer(department_without_officers) + officer = generate_officer(department_without_officers, user) officer.gender = None session.add(officer) session.commit() @@ -828,13 +831,13 @@ def test_bulk_add_officers__no_create_flag( assert result.exception is None # confirm that only one officer is in database and information was updated - print(Officer.query.filter_by(department_id=department_id).all()) officer = Officer.query.filter_by(department_id=department_id).one() assert officer.unique_internal_identifier == officer_uuid assert officer.gender == officer_gender_updated def test_advanced_csv_import__success(session, department, test_csv_dir): + user = User.query.filter_by(is_administrator=False).first() # make sure department name aligns with the csv files assert department.name == SPRINGFIELD_PD.name assert department.state == SPRINGFIELD_PD.state @@ -846,6 +849,7 @@ def test_advanced_csv_import__success(session, department, test_csv_dir): first_name="Already", last_name="InDatabase", birth_year=1951, + created_by=user.id, ) session.add(officer) @@ -855,6 +859,7 @@ def test_advanced_csv_import__success(session, department, test_csv_dir): star_no="4567", start_date=datetime.date(2020, 1, 1), job_id=department.jobs[0].id, + created_by=user.id, ) session.add(assignment) @@ -864,6 +869,7 @@ def test_advanced_csv_import__success(session, department, test_csv_dir): officer_id=officer.id, year=2018, is_fiscal_year=False, + created_by=user.id, ) session.add(salary) @@ -873,11 +879,17 @@ def test_advanced_csv_import__success(session, department, test_csv_dir): department_id=1, description="description", time=datetime.time(23, 45, 16), + created_by=user.id, ) incident.officers = [officer] session.add(incident) - link = Link(id=55051, title="Existing Link", url="https://www.example.org") + link = Link( + id=55051, + title="Existing Link", + url="https://www.example.org", + created_by=user.id, + ) session.add(link) officer.links = [link] @@ -904,7 +916,6 @@ def test_advanced_csv_import__success(session, department, test_csv_dir): assert result.exception is None assert result.exit_code == 0 - print(list(Officer.query.all())) all_officers = { officer.unique_internal_identifier: officer for officer in Officer.query.filter_by(department_id=1).all() @@ -1029,12 +1040,14 @@ def _create_csv(data, path, csv_file_name): def test_advanced_csv_import__force_create(session, department, tmp_path): + user = User.query.filter_by(is_administrator=False).first() tmp_path = str(tmp_path) other_department = Department( name="Other department", short_name="OPD", state=random.choice(DEPARTMENT_STATE_CHOICES)[0], + created_by=user.id, ) session.add(other_department) @@ -1043,6 +1056,7 @@ def test_advanced_csv_import__force_create(session, department, tmp_path): department_id=other_department.id, first_name="Already", last_name="InDatabase", + created_by=user.id, ) session.add(officer) session.flush() @@ -1157,11 +1171,13 @@ def test_advanced_csv_import__force_create(session, department, tmp_path): def test_advanced_csv_import__overwrite_assignments(session, department, tmp_path): tmp_path = str(tmp_path) + user = User.query.filter_by(is_administrator=False).first() other_department = Department( name="Other department", short_name="OPD", state=random.choice(DEPARTMENT_STATE_CHOICES)[0], + created_by=user.id, ) session.add(other_department) @@ -1172,12 +1188,14 @@ def test_advanced_csv_import__overwrite_assignments(session, department, tmp_pat department_id=department.id, first_name="Already", last_name="InDatabase", + created_by=user.id, ) officer2 = Officer( id=cop2_id, department_id=department.id, first_name="Also", last_name="InDatabase", + created_by=user.id, ) a1_id = 999101 a2_id = 999102 @@ -1185,11 +1203,13 @@ def test_advanced_csv_import__overwrite_assignments(session, department, tmp_pat id=a1_id, officer_id=officer.id, job_id=Job.query.filter_by(job_title="Police Officer").first().id, + created_by=user.id, ) assignment2 = Assignment( id=a2_id, officer_id=officer2.id, job_id=Job.query.filter_by(job_title="Police Officer").first().id, + created_by=user.id, ) session.add(officer) session.add(assignment) @@ -1312,10 +1332,12 @@ def test_advanced_csv_import__missing_required_field_officers( def test_advanced_csv_import__wrong_department(session, department, tmp_path): + user = User.query.filter_by(is_administrator=False).first() other_department = Department( name="Other department", short_name="OPD", state=random.choice(DEPARTMENT_STATE_CHOICES)[0], + created_by=user.id, ) session.add(other_department) @@ -1346,15 +1368,21 @@ def test_advanced_csv_import__wrong_department(session, department, tmp_path): def test_advanced_csv_import__update_officer_different_department( session, department, tmp_path ): + user = User.query.filter_by(is_administrator=False).first() # set up data other_department = Department( name="Other department", short_name="OPD", state=random.choice(DEPARTMENT_STATE_CHOICES)[0], + created_by=user.id, ) session.add(other_department) officer = Officer( - id=99021, department_id=other_department.id, first_name="Chris", last_name="Doe" + id=99021, + department_id=other_department.id, + first_name="Chris", + last_name="Doe", + created_by=user.id, ) session.add(officer) @@ -1385,13 +1413,14 @@ def test_advanced_csv_import__update_officer_different_department( def test_advanced_csv_import__unit_other_department( session, department, department_without_officers, tmp_path ): + user = User.query.filter_by(is_administrator=False).first() # set up data - officer = generate_officer(department) + officer = generate_officer(department, user) session.add(officer) session.flush() session.add(department_without_officers) session.flush() - unit = Unit(department_id=department_without_officers.id) + unit = Unit(department_id=department_without_officers.id, created_by=user.id) session.add(unit) session.flush() diff --git a/OpenOversight/tests/test_csvs/incidents.csv b/OpenOversight/tests/test_csvs/incidents.csv index 45c9e803a..87b2eef78 100644 --- a/OpenOversight/tests/test_csvs/incidents.csv +++ b/OpenOversight/tests/test_csvs/incidents.csv @@ -1,4 +1,4 @@ -id,department name,department state,date,time,report number,description,street name,cross street1,cross street2,city,state,zip code,license plates,officer_ids,creator id,last updated id +id,department name,department state,date,time,report number,description,street name,cross street1,cross street2,city,state,zip code,license plates,officer_ids,created by,last updated by ,Springfield Police Department,IL,2020-07-20,06:30,CR-1234,Something happened,,East Ave,Main St,Chicago,IL,60603,ABC123_NY|98UMC_IL,#1|49483,, #I1,Springfield Police Department,IL,,,CR-9912,Something happened,Fake Street,Main Street,,Chicago,IL,60603,,#1,, 123456,Springfield Police Department,IL,2020-07-26,,CR-39283,Don't know where it happened,,,,,,,XYZ11,#1,, diff --git a/OpenOversight/tests/test_csvs/links.csv b/OpenOversight/tests/test_csvs/links.csv index b5ed44392..8b1c6fac8 100644 --- a/OpenOversight/tests/test_csvs/links.csv +++ b/OpenOversight/tests/test_csvs/links.csv @@ -1,4 +1,4 @@ -id,title,url,link type,description,author,creator_id,officer ids,incident ids +id,title,url,link type,description,author,created_by,officer ids,incident ids ,A Link,https://www.example.com,link,A link about officers,,,#1|49483, ,Another Link,https://www.example.com/incident,link,A link on an incident,Example Times,,,#I1 55051,Updated Link,https://www.example.org,,,,,,123456 diff --git a/OpenOversight/tests/test_models.py b/OpenOversight/tests/test_models.py index 9d27f5f3c..b7e357b82 100644 --- a/OpenOversight/tests/test_models.py +++ b/OpenOversight/tests/test_models.py @@ -417,9 +417,9 @@ def test_images_added_with_user_id(mockdata, faker): created_at=datetime.datetime.now(), department_id=1, taken_at=datetime.datetime.now(), - user_id=user_id, + created_by=user_id, ) db.session.add(new_image) db.session.commit() - saved = Image.query.filter_by(user_id=user_id).first() + saved = Image.query.filter_by(created_by=user_id).first() assert saved is not None diff --git a/docs/advanced_csv_import.rst b/docs/advanced_csv_import.rst index 665b941c5..52840ed9a 100644 --- a/docs/advanced_csv_import.rst +++ b/docs/advanced_csv_import.rst @@ -150,7 +150,7 @@ Incidents csv ^^^^^^^^^^^^^ - Required: ``id, department_name, department_state`` - Optional: ``date, time, report_number, description, street_name, cross_street1, cross_street2, city, state, zip_code, - creator_id, last_updated_id, officer_ids, license_plates`` + created_by, last_updated_by, officer_ids, license_plates`` Details: ~~~~~~~~ @@ -165,7 +165,7 @@ Details: - ``street_name`` Name of the street the incident occurred, but should not include the street number. - ``cross_street1``, ``cross_street2`` The two closest intersecting streets. - ``city``, ``state``, ``zip_code`` State needs to be in 2 letter abbreviated notation. -- ``creator_id``, ``last_updated_id`` Id of existing user shown as responsible for adding this entry. +- ``created_by``, ``last_updated_by`` Id of existing user shown as responsible for adding this entry. - ``officer_ids`` Ids of officers involved in the incident, separated by ``|``. - Each individual id can either be an integer referring to an existing officer or a string starting with ``#`` referring to a newly created officer. @@ -181,7 +181,7 @@ Details: Links csv ^^^^^^^^^ - Required: ``id, url`` -- Optional: ``title, link_type, description, author, creator_id, officer_ids, incident_ids`` +- Optional: ``title, link_type, description, author, created_by, officer_ids, incident_ids`` Details: ~~~~~~~~ @@ -190,7 +190,7 @@ Details: - ``description`` A short description of the link. - ``link_type`` Choice of ``Link``, ``YouTube Video`` and ``Other Video``. - ``author`` The source or author of the linked article, report, video. -- ``creator_id`` Id of existing user shown as responsible for adding this entry. +- ``created_by`` Id of existing user shown as responsible for adding this entry. - ``officer_ids`` Ids of officer profiles this link should be visible on, separated by ``|``. See same field in incidents above for more details. - ``incidents_ids`` Ids of incidents this link should be associated with, separated by ``|``. Just like ``officer_ids`` this can contain strings starting with ``#`` to refer to an incident created in the incident csv.