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

Adds a License model #167

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

Conversation

rosschapman
Copy link
Contributor

@rosschapman rosschapman commented Feb 8, 2024

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 and source 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 as status. In one practical context, it makes the upcoming changes to the form extremely simple by being able to do:

 <%= form.collection_select :license_id, License.all, :id, :label, include_blank: "Select a license" %>

Here's the DML for the license data. I'm not sure what's the preference for how to apply this patch.

insert into licenses (code, source, label, created_at, updated_at)
VALUES('all_rights_reserved', null, 'All Rights Reserved', now(), now()), 
('by', 'https://creativecommons.org/licenses/by/4.0/', 'Attribution', now(), now()), 
('by_sa', 'https://creativecommons.org/licenses/by-sa/4.0/', 'Attribution-ShareAlike 4.0 International', now(), now()), 
('by_nc', 'https://creativecommons.org/licenses/by-nc/4.0/', 'Attribution-NonCommercial', now(), now()), 
('by_nc_sa', 'https://creativecommons.org/licenses/by-nc-sa/4.0/', 'Attribution-NonCommercial-ShareAlike', now(), now()), 
('by_nd', 'https://creativecommons.org/licenses/by-nd/4.0/', 'Attribution-NoDerivs', now(), now()), 
('by_nc_nd', 'https://creativecommons.org/licenses/by-nc-nd/4.0/', 'Attribution-NonCommercial-NoDerivs', now(), now()), 
('cc_0', 'https://creativecommons.org/publicdomain/zero/1.0/', 'No Copyright', now(), now()) 
returning *;

Gemfile.lock Outdated
Copy link
Contributor Author

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?

Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh yeah.

@chrislo
Copy link
Member

chrislo commented Feb 13, 2024

Looks good @rosschapman! I think the data in your PR description could be added to db/seeds.rb under the line that loads the development seeds. We'd be able to run it in production after this PR lands.

Do you intend to add the UI changes to this PR too?

@rosschapman
Copy link
Contributor Author

rosschapman commented Feb 17, 2024

@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.

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