Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show police department on the image and tag detail pages #307

Merged
merged 5 commits into from
Sep 30, 2017

Conversation

redshiftzero
Copy link
Member

Status

Ready for review

Description of Changes

Fixes #305.

Changes proposed in this pull request:

  • Adds foreign key constraint (and migration) between images and departments tables
  • Shows the police department on the image page:

screen shot 2017-09-21 at 10 05 51 pm

Testing / Deployment

Tests and linting

  • I have rebased my changes on current develop

  • pytests pass in the development environment on my local machine

  • flake8 checks pass

Copy link
Contributor

@r4v5 r4v5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may just be a thing that's done magically by sqlalchemy but doesn't the raw_images.department_id column need to be populated (presumably with '1' for the first department for existing images) in order to not violate the constraint you'll be imposing on it?

Also, did I miss the place where uploaded raw_images will have the department ID set on them (possibly in another PR)? Because an insert without a department_id will cause a FK constraint violation.

@r4v5
Copy link
Contributor

r4v5 commented Sep 26, 2017

Ah: for the first one, there's a manage.py task added in #310 (f686248) to address it, which I think probably should be pulled into this branch, since it's technically part of this change ('image-related things'). And the image upload route is part of the previous changeset. Okay cool.

@r4v5 r4v5 merged commit 533dbcf into develop Sep 30, 2017
@redshiftzero
Copy link
Member Author

You probably figured this out hence the merge, but writing out here for posterity:

doesn't the raw_images.department_id column need to be populated (presumably with '1' for the first department for existing images) in order to not violate the constraint you'll be imposing on it?

Yep! The manage.py link_images_to_department command was added in #303 to handle this (a bit hacky maybe, but 🤷‍♀️). I added a similar command in #310 for doing the same thing with two other tables that should store department_id (in prep for adding a foriegn key constraint later on).

@redshiftzero
Copy link
Member Author

Ohh I see ya moved that bit o' code over here. That's 💯

@r4v5 r4v5 deleted the add-department-images branch July 1, 2018 20:27
AetherUnbound pushed a commit that referenced this pull request Apr 23, 2023
Bumps [pre-commit](https://github.com/pre-commit/pre-commit) from 3.2.1 to 3.2.2.
- [Release notes](https://github.com/pre-commit/pre-commit/releases)
- [Changelog](https://github.com/pre-commit/pre-commit/blob/main/CHANGELOG.md)
- [Commits](pre-commit/pre-commit@v3.2.1...v3.2.2)

---
updated-dependencies:
- dependency-name: pre-commit
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants