-
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
Adds a License
model
#167
base: main
Are you sure you want to change the base?
Adds a License
model
#167
Conversation
Gemfile.lock
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated during test run? Are y'all ok to check these in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think so - but ideally as a separate commit. I guess it's because you're using an intel mac?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eh yeah.
Looks good @rosschapman! I think the data in your PR description could be added to Do you intend to add the UI changes to this PR too? |
@chrislo I will add the UI changes in a follow-up PR in order to reduce the cognitive load per review and avoid things that are done being held up by congruent discussion (a habit from the job). I can add data to the seed! That's certainly an expedient setup method. I gather then it won't mess with any build or deployment process. |
This PR part 1 of N to allow Artists to assign a license to an Album.
Ultimately I felt that a configuration table for the license data was a better option than an enum since it would 1) add a hard relational constraint on the association's data and would consolidate important secondary metadata values like
label
andsource
all in one place. In other words, a License is a richer domain model than something like a the common type of single value enum such asstatus
. In one practical context, it makes the upcoming changes to the form extremely simple by being able to do:Here's the DML for the license data. I'm not sure what's the preference for how to apply this patch.