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

Don't protect deprecated system subjects #3031

Merged
merged 1 commit into from Feb 18, 2020

Conversation

hornc
Copy link
Collaborator

@hornc hornc commented Feb 13, 2020

Helps with, but doesn't c.l.o.s.e #2107

These former 'system' subjects ["Accessible Book", "Lending Library", "In Library", "Protected DAISY"] are no longer used by the system to indicate lending status and have been deprecated as they are not technically subjects. See #2107

This PR allows any user / librarian to clean these from records if they encounter them.

Deletes code => reduces complexity.

Technical

Testing

Evidence

Stakeholders

@hornc hornc force-pushed the featue/2107-no-more-system-subject-protection branch from c4634a0 to d57a63a Compare February 13, 2020 06:13
@cdrini cdrini self-assigned this Feb 18, 2020
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

  • ✅ No dangling refs to SYSTEM_SUBJECTS
  • ✅ No dangling refs to _prevent_system_subjects_deletion
  • ✅ Test before: adding all system subjects allowed for non-admin
  • ✅ Test before: removing any system subjects dis-allowed for non-admin
  • ✅ Test after: adding all system subjects allowed for non-admin
  • ✅ Test after: removing any system subjects allowed for non-admin

(Note: I tested this locally by editing the is_admin fn to return false)

@cdrini cdrini merged commit 0eecf29 into master Feb 18, 2020
@tfmorris
Copy link
Contributor

Yay!

@cdrini cdrini deleted the featue/2107-no-more-system-subject-protection branch March 12, 2020 13:31
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.

None yet

3 participants