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 ER Diagram, remove unused db models #657

Merged
merged 9 commits into from
Mar 24, 2024

Conversation

Joshua-Douglas
Copy link
Member

This PR makes progress towards #655. Before I update the user models I thought it would be useful to clean out all of the unused data models, and plot the currently used models.

What changes did you make?

  • Delete unused db models
  • Add a model README.md that plots the current ER Diagram. I provide instructions for how to generate the diagram.

Rationale behind the changes?

Testing done for these changes

  • The current test suite covers our endpoints that are actually needed. These tests confirm that all of the removed models are unused.
classDiagram
class host{
   *INTEGER id NOT NULL
   VARCHAR name NOT NULL
}
class housing_program{
   *INTEGER id NOT NULL
   VARCHAR program_name NOT NULL
   INTEGER service_provider NOT NULL
}
class housing_program_service_provider{
   *INTEGER id NOT NULL
   VARCHAR provider_name NOT NULL
}
class user{
   *INTEGER id NOT NULL
   VARCHAR email NOT NULL
}
housing_program_service_provider "1" -- "0..n" housing_program
Loading

@agosmou
Copy link
Member

agosmou commented Feb 18, 2024

I left a comment on the parent issue about labeling this PR so we can find it easily if need be. Lmk your thoughts.

@agosmou agosmou self-requested a review February 18, 2024 22:23
--config-settings="--global-option="-LC:\Program Files\Graphviz\lib" `
pygraphviz
python -m pip install eralchemy2
```
Copy link
Member

Choose a reason for hiding this comment

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

I couldnt get this to work on Windows - tried putting it into my path and everything. Im thinking it has something to do with it being a C program. Not sure.

I jumped into WSL2 and it installed well then.

sudo apt install graphviz libgraphviz-dev pkg-config
python3 -m pip install pygraphviz
python3 -m pip install eralchemy2

then from the api directory I got it to work with:
eralchemy2 -i "sqlite:///./homeuniteus.db" -o "HomeUniteUsDataModel.md"

However, it printed out the old ERD. I think it's because the alembic upgrade head create the sqllite db with the old migration.

Will this PR add a db migration that removes the unused tables?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @agosmou,

Thanks for the added instructions. I agree that the windows install is a pain, so I added the linux install instructions on a7f06ab.

I'll add a comment below to address your other concerns.

Copy link
Member

@mira-kine mira-kine left a comment

Choose a reason for hiding this comment

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

Thanks Josh, this will definitely help visualize #655 changes! The ER diagram successfully printed out for me, though right now it prints out the old ER. Other than that, this PR looks good to me.

@tylerthome
Copy link
Member

Waiting on #648 to merge

@Joshua-Douglas
Copy link
Member Author

Hey @mira-kine & @agosmou,

Thanks for highlighting the lack of a database migration script to account for these changes! I've been relying on the API to autogenerate the db, but this approach requires you to delete the database after each update.

To alter the database more properly I added an alembic migration script to drop the removed tables in 8657569. I also updated the old version's migration script to allow it to process databases that were created by the API without ever running alembic in d76be3d. Before this change the old migration script would throw several "table already exists errors".

Now you should be able to run alembic upgrade head. Once you do that you'll be able to generate the most up-to-date ER diagram, without losing any of your user data.

I also updated the ER diagram to include the alembic version number table in aacbeb4.

classDiagram
class alembic_version{
   *VARCHAR<32> version_num NOT NULL
}
class host{
   *INTEGER id NOT NULL
   VARCHAR name NOT NULL
}
class housing_program{
   *INTEGER id NOT NULL
   VARCHAR program_name NOT NULL
   INTEGER service_provider NOT NULL
}
class housing_program_service_provider{
   *INTEGER id NOT NULL
   VARCHAR provider_name NOT NULL
}
class user{
   *INTEGER id NOT NULL
   VARCHAR email NOT NULL
}
housing_program_service_provider "1" -- "0..n" housing_program
Loading

@Joshua-Douglas
Copy link
Member Author

Hey @tylerthome,

Thanks for waiting to merge. PR #648 has been merged, and I made a handful of improvements. The PR is ready for re-review.

@Joshua-Douglas Joshua-Douglas merged commit fea47a5 into main Mar 24, 2024
4 checks passed
@Joshua-Douglas Joshua-Douglas deleted the 655-remove-unused-db-models branch March 24, 2024 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants