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

Assign categories to expenses #28

Merged
merged 4 commits into from
Jan 11, 2024

Conversation

ChristopherJohnston
Copy link
Contributor

add a categories table
add seed script to populate with some categories (mimics the splitwise category list)
add category relation to expense table,
add ability to select category in the create/edit forms

Copy link

vercel bot commented Jan 8, 2024

@ChristopherJohnston is attempting to deploy a commit to the Sebastien Castiel Team on Vercel.

A member of the Team first needs to authorize it.

@ChristopherJohnston
Copy link
Contributor Author

Re-submitted this implementation, merged with the expenseDate and uneven-split changes.

The intent here is to follow the split wise categories so it's easier to import from a CSV.

Happy to discuss the implementation.

@scastiel
Copy link
Member

scastiel commented Jan 9, 2024

@ChristopherJohnston looks great!

I had some trouble to make the migration work, so I changed it a bit, to insert categories into the database as part of the migration (instead of using a seed). It makes more sense to me, what do you think?

@scastiel scastiel changed the title add categories table Assign categories to expenses Jan 9, 2024
@scastiel scastiel linked an issue Jan 9, 2024 that may be closed by this pull request
@ChristopherJohnston
Copy link
Contributor Author

ChristopherJohnston commented Jan 9, 2024

It looks fine as part of the migration. I'm now wondering if it's easier and better just to have a hard-coded list of categories to save the additional overhead from database queries - particularly if the categories are going to be fixed.

@ChristopherJohnston
Copy link
Contributor Author

@ChristopherJohnston looks great!

I had some trouble to make the migration work

for reference I took the seed script concept from https://planetscale.com/blog/how-to-seed-a-database-with-prisma-and-next-js

@scastiel
Copy link
Member

I'm now wondering if it's easier and better just to have a hard-coded list of categories to save the additional overhead from database queries - particularly if the categories are going to be fixed.

That is a fair point, maybe it’ll be easier to deal with hardcoded categories.

@Max-TheCat
Copy link
Contributor

I'm now wondering if it's easier and better just to have a hard-coded list of categories to save the additional overhead from database queries - particularly if the categories are going to be fixed.

That is a fair point, maybe it’ll be easier to deal with hardcoded categories.

Since the implementation is pretty much done, I'd keep it in the DB as it keeps things extensible. That's how I was about to implement it anyways 😆

@ChristopherJohnston
Copy link
Contributor Author

Yep it's not a bad implementation as it is - just need to be careful of the impact on api calls on Vercel as volume increases. I would think it's fine for now

@scastiel
Copy link
Member

Thanks @Max-TheCat for the input! I’ll merge the PR as is. Worst case scenario we migrate to a hardcoded list, which wouldn’t be too much work 😉

@scastiel scastiel merged commit 45ee9cd into spliit-app:main Jan 11, 2024
1 check failed
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.

Assign a category to expenses
3 participants