-
Notifications
You must be signed in to change notification settings - Fork 2
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
Remove people
table completely? ✂️
#118
Comments
Given that the goal of this repo/project is the minimum code, I'm leaning toward removing the table/schema. 💭 |
My preference would also be to remove the |
@SimonLab thank you for suggesting the removal. 👌 |
Successfully removes New ERD: It's almost comically simple compared to the previous version of the MVP: #89 (comment) Everything still works as expected. 📝 ⏳ ✅ |
I haven't looked in details at the commit to remove the Just saw you already removed the seeds: https://github.com/dwyl/app-mvp/blob/phoenix-1.6-2022-refresh-issue-%2389/priv/repo/seeds.exs 👍 |
Having to remove |
@SimonLab made a very valid/insightful point in #115 (comment) ... 💭
At present the
people
table is simply mirroring the contents ofpeople
fromauth
via the
JWT
when the person successfully authenticates ...i.e. we are storing the data in the MVP App and don't actually need to.
Two Questions:
1. Will it be confusing to a complete beginner to
not have the
people
table in the MVP?At present we have the following ERD as part of the update: #89 (comment)
To me this makes it reasonably clear what the link between the tables is.
But if we were to remove
people
, would it still be clear that anitem
"belongs" to aperson
?Removing the
people
table will not alter the functionality of the MVP ... 📱 📝 ⏳ ✅But it will result in less code & tests. 🎉
So I'm very much considering it. 💭
2. Will we have a
people
table in the "real" App?Once we have finished testing the MVP and we port the code over to the "real" App,
do we want to have a
people
table? or can we exclude it completely and have a separation of concerns?i.e. only have the
items.person_id
reference and store all personal data encrypted in theauth
App?The text was updated successfully, but these errors were encountered: