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

Add state to departments table #981

Merged
merged 193 commits into from
Jul 26, 2023
Merged

Add state to departments table #981

merged 193 commits into from
Jul 26, 2023

Conversation

michplunkett
Copy link
Collaborator

@michplunkett michplunkett commented Jul 21, 2023

Fixes issue

Partially addresses this: #797

Description of Changes

Add state column to the departments table and update all relevant tests and features. This is the first step to addressing the Department state searching feature.

While taking care of the .format in models/database.py for Department, I took care of the other ones as well.

Tests and linting

  • This branch is up-to-date with the develop branch.
  • pytest passes on my local development environment.
  • pre-commit passes on my local development environment.
DB Migration Output
$ flask db upgrade
[2023-07-26 15:52:20,432] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$ flask db downgrade
[2023-07-26 15:52:25,802] 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 18f43ac4622f -> 1931b987ce0d, add state column to departments
$ flask db upgrade
[2023-07-26 15:52:30,441] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$ 

@michplunkett
Copy link
Collaborator Author

michplunkett commented Jul 25, 2023

I need to update documentation and some a couple of tests cases (test wide faker and PoliceDepartment class), and I'll merge it tomorrow.

upload_s3_patch = patch(
"OpenOversight.app.utils.cloud.upload_obj_to_s3",
MagicMock(return_value="https://s3-some-bucket/someaddress.jpg"),
)


class PoliceDepartment:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What will this be used for? It seems to be basically the same as the class in database.py - all the same attributes

Copy link
Collaborator Author

@michplunkett michplunkett Jul 26, 2023

Choose a reason for hiding this comment

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

It's used an easier and more standard alternative to code like this:

name = "Added Police Department"
short_name = "APD"
unique_internal_identifier = "30ad0au239eas939asdj"

It doesn't come with all of the baggage that the database.py object does and it exists the test_utils.py as it's just a testing utility.

OpenOversight/tests/test_utils.py Outdated Show resolved Hide resolved
@@ -300,13 +300,20 @@ def add_mockdata(session):
department = Department(
name=DEPARTMENT_NAME,
short_name="SPD",
state=random.sample(US_STATE_ABBREVIATIONS, 1)[0],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Problem with random is that it can lead to flaky tests (sometimes passing, sometimes not) which are much worse to fix than a test that always fails

OpenOversight/app/models/database.py Outdated Show resolved Hide resolved
Comment on lines +570 to +573
@click.argument(
"department-state",
type=click.Choice([state.abbr for state in us.STATES]),
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the next PR, once we can create and edit Departments from the front-end, we can make this required.

short_name = db.Column(db.String(100), unique=False, nullable=False)
state = db.Column(db.String(2), server_default="", nullable=False)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's been set to db.Column(db.String(2), server_default="", nullable=False), @abandoned-prototype.

@michplunkett michplunkett merged commit 0e989eb into develop Jul 26, 2023
2 checks passed
@michplunkett michplunkett deleted the add_state_filtering branch July 26, 2023 16:46
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Sep 24, 2023
Partially addresses this:
lucyparsons#797

Add `state` column to the `departments` table and update all relevant
tests and features. This is the first step to addressing the Department
state searching feature.

While taking care of the `.format` in `models/database.py` for
`Department`, I took care of the other ones as well.

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

<details><summary>DB Migration Output</summary>

```console
$ flask db upgrade
[2023-07-26 15:52:20,432] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$ flask db downgrade
[2023-07-26 15:52:25,802] 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 18f43ac4622f -> 1931b987ce0d, add state column to departments
$ flask db upgrade
[2023-07-26 15:52:30,441] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$
```
</details>
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Sep 25, 2023
Partially addresses this:
lucyparsons#797

Add `state` column to the `departments` table and update all relevant
tests and features. This is the first step to addressing the Department
state searching feature.

While taking care of the `.format` in `models/database.py` for
`Department`, I took care of the other ones as well.

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

<details><summary>DB Migration Output</summary>

```console
$ flask db upgrade
[2023-07-26 15:52:20,432] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$ flask db downgrade
[2023-07-26 15:52:25,802] 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 18f43ac4622f -> 1931b987ce0d, add state column to departments
$ flask db upgrade
[2023-07-26 15:52:30,441] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$
```
</details>
AetherUnbound pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Oct 9, 2023
Partially addresses this:
lucyparsons#797

Add `state` column to the `departments` table and update all relevant
tests and features. This is the first step to addressing the Department
state searching feature.

While taking care of the `.format` in `models/database.py` for
`Department`, I took care of the other ones as well.

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

<details><summary>DB Migration Output</summary>

```console
$ flask db upgrade
[2023-07-26 15:52:20,432] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$ flask db downgrade
[2023-07-26 15:52:25,802] 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 18f43ac4622f -> 1931b987ce0d, add state column to departments
$ flask db upgrade
[2023-07-26 15:52:30,441] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$
```
</details>
AetherUnbound pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Oct 9, 2023
Partially addresses this:
lucyparsons#797

Add `state` column to the `departments` table and update all relevant
tests and features. This is the first step to addressing the Department
state searching feature.

While taking care of the `.format` in `models/database.py` for
`Department`, I took care of the other ones as well.

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

<details><summary>DB Migration Output</summary>

```console
$ flask db upgrade
[2023-07-26 15:52:20,432] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$ flask db downgrade
[2023-07-26 15:52:25,802] 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 18f43ac4622f -> 1931b987ce0d, add state column to departments
$ flask db upgrade
[2023-07-26 15:52:30,441] 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 1931b987ce0d -> 18f43ac4622f, add state column to departments
$
```
</details>
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.

Add states to browse department page or separate department lists by states
3 participants