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

setlist and songUpdates #14

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

setlist and songUpdates #14

wants to merge 16 commits into from

Conversation

kstolte
Copy link
Contributor

@kstolte kstolte commented Jan 26, 2023

This is ready for review and merge after #13 is rebase and merged

  • fix(ap): restricting fetching of setlists to only the users created setlist
  • feat(db): adding new fields for song usage
  • test: adding some basic working data in prisma added song in user id 3 and some categories
  • chore(scripts): db:hard-refresh for destroying the database and reseed
  • feat(helpers): bringing over some time formatting
  • build(package.json): adding momentjs for display of local time
  • removing claimedPublicDomain at this time was having too many issues with the form updating and what not. Proabbly have to be a controlled input?
  • updating form for supporting new fields for song editor
  • adding runtime and notes for the song viewer

…he user created for now

Not sure if this is what we were thinking of for the MVP or if it was going to be a free for all and
users could use eachothers songs.
a user should be able to only get their created setlists. more than likely this will be more of an
organization or band hierarchy kinda thing but for mvp this will work.
added song in user id 3 and some categories
faster local dev interations for when the database needs to be hard reset and starting from scratch.
The format time was from our implementation in lumastic-app, the format runtime is for computing the
friendly version of the stored seconds of runtime.
this should be evaluated and removed if necessary, i believe our implementation of the library is
probably covered in standard JS at this time.
was having too many issues with the form updating and what not. Proabbly have to be a controlled input?
@kstolte kstolte marked this pull request as ready for review January 26, 2023 18:07
Copy link
Contributor

@drewlyton drewlyton left a comment

Choose a reason for hiding this comment

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

Is there any way for us to pull the runtime stuff out into a different branch? Just cause I feel like we should talk through a little more what that feature looks like across the app

@@ -10,6 +10,7 @@
"db:deploy": "npx prisma migrate deploy",
"db:push": "npx prisma db push",
"db:reset": "rm -r prisma/data.db && npm run db:push",
"db:hard-refresh": "rm -r prisma/data.db && npm run db:push && npm run db:seed",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm down to just add the seed command to db:reset if you want?

prisma/seed.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,36 @@
import moment from "moment";
Copy link
Contributor

Choose a reason for hiding this comment

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

Moment doesn't recommend using Moment anymore so maybe we should look at other libraries that are smaller (or just use native JS dates). What are we trying to display, just a string that says like "3m" or "4m". I can't imagine Grayson and I ever really putting seconds since our estimations of time aren't that exact.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kstolte
Copy link
Contributor Author

kstolte commented Jan 26, 2023

Sure!

Base automatically changed from mvp to development February 1, 2023 22:52
Base automatically changed from development to main February 13, 2023 18:40
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