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

Intellectual property enum for Person #336

Merged
merged 7 commits into from
May 6, 2024
Merged

Intellectual property enum for Person #336

merged 7 commits into from
May 6, 2024

Conversation

damisul
Copy link
Collaborator

@damisul damisul commented May 2, 2024

Updated person model and people table: replaced public_domain column with intellectual_property enum.

Fixed incongruous copyright report.

Also this PR contains changes to speedup ManifestationIdex rebuilding: I've added preloading of all involved persons and tags, and modified methods used to fetch authors/translators etc to be able to use preloaded data.

It allowed to reduce chewy:reset time for ManifestationIndex on my machine from 23 minutes to 11 minutes.

I propose to use 'Rebase and merge' policy for this PR to keep those changes separated in commit history.

@damisul damisul marked this pull request as draft May 2, 2024 18:50
@damisul damisul marked this pull request as ready for review May 3, 2024 14:18
@damisul damisul requested a review from abartov May 3, 2024 14:18
@damisul damisul linked an issue May 3, 2024 that may be closed by this pull request
@abartov abartov merged commit c70e4d6 into master May 6, 2024
7 checks passed
@abartov abartov deleted the copyright_statuses branch May 6, 2024 14:23
@abartov
Copy link
Owner

abartov commented May 6, 2024

@damisul - I am testing this, and noticed what I missed in a previous PR: the manifestation/edit_metadata form has not been updated to offer the new intellectual property modes, and still has a boolean radio button. Please fix this.

@abartov
Copy link
Owner

abartov commented May 6, 2024

also: Manifestation.copyright? yields a NoMethodError


app/models/manifestation.rb:78:in `copyright?'
app/views/shared/_anth_panel.html.haml:96
app/views/shared/_anth_panel.html.haml:57:in `each'
app/views/shared/_anth_panel.html.haml:57
app/views/shared/_anthology.html.haml:52
app/views/shared/_anth_stuff.html.haml:2
app/views/manifestation/browse.html.haml:21
app/controllers/manifestation_controller.rb:45:in `browse'
app/controllers/manifestation_controller.rb:317:in `genre' 

@damisul
Copy link
Collaborator Author

damisul commented May 6, 2024

@damisul - I am testing this, and noticed what I missed in a previous PR: the manifestation/edit_metadata form has not been updated to offer the new intellectual property modes, and still has a boolean radio button. Please fix this.

Hm... this is strange. I see this on my machine using current master (after this PR was merged):
Снимок экрана в 2024-05-07 01-14-33

@abartov , are you sure you're using latest version of sources?

@damisul
Copy link
Collaborator Author

damisul commented May 6, 2024

And

also: Manifestation.copyright? yields a NoMethodError

Just checked and currently on 96th line of _anth_panel.html.haml we don't check copyrighted? field. Instead we render intellectual_property partial there:

- intellectual_property = text.manifestation.expression.intellectual_property

@abartov
Copy link
Owner

abartov commented May 7, 2024

Pardon me! Both are my mistake. I accidentally looked at the production branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Add more copyright statuses
2 participants