Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/admin/collections_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def show
format.json { render json: @collection.to_json }
format.html {
@groups = { inherited: @collection.inherited_local_read_groups, default: @collection.default_local_read_groups }
@users = { inherited: @collection.inherited_read_users, default: @collection.default_read_users }
@users = { inherited: @collection.inherited_read_users.map(&:downcase), default: @collection.default_read_users.map(&:downcase) }
@virtual_groups = { inherited: @collection.inherited_virtual_read_groups, default: @collection.default_virtual_read_groups }
@ip_groups = { inherited: @collection.inherited_ip_read_groups, default: @collection.default_ip_read_groups }
@visibility = @collection.default_visibility
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/units_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def show
format.json { render json: @unit.to_json }
format.html {
@groups = @unit.default_local_read_groups
@users = @unit.default_read_users
@users = @unit.default_read_users.map(&:downcase)
@virtual_groups = @unit.default_virtual_read_groups
@ip_groups = @unit.default_ip_read_groups

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/media_objects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ def custom_edit
if 'access-control' == @active_step
@groups = { base: @media_object.local_read_groups, inherited: @media_object.inherited_local_read_groups }
@group_leases = @media_object.leases('local')
@users = { base: @media_object.read_users, inherited: @media_object.inherited_read_users }
@users = { base: @media_object.read_users.map(&:downcase), inherited: @media_object.inherited_read_users.map(&:downcase) }
@user_leases = @media_object.leases('user')
@virtual_groups = { base: @media_object.virtual_read_groups, inherited: @media_object.inherited_virtual_read_groups }
@virtual_leases = @media_object.leases('external')
Expand Down
2 changes: 1 addition & 1 deletion app/models/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ def is_editor_of_unit?(unit)
end

def is_exclusively_inherited_from_parent?(media_object)
(!@user.in?(media_object.read_users) && @user.in?(media_object.inherited_read_users)) ||
(!@user.in?(media_object.read_users.map(&:downcase)) && @user.in?(media_object.inherited_read_users.map(&:downcase))) ||
((@user_groups & media_object.read_groups).empty? && !(@user_groups & media_object.inherited_read_groups).empty?)
end

Expand Down
5 changes: 5 additions & 0 deletions app/models/admin/collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class Admin::Collection < ActiveFedora::Base
around_save :reindex_members, if: Proc.new { |c| c.name_changed? or c.unit_changed? }
around_save :return_checkouts, if: Proc.new { |c| c.cdl_enabled_changed? && c.cdl_enabled == false }
before_create :create_dropbox_directory!
before_save :normalize_read_users, if: proc { |c| c.default_read_users_changed? }

before_destroy :destroy_dropbox_directory!

Expand Down Expand Up @@ -388,4 +389,8 @@ def published_count
def unpublished_count
media_objects.count - published_count
end

def normalize_read_users
self.default_read_users = default_read_users.map(&:downcase)
end
end
5 changes: 5 additions & 0 deletions app/models/admin/unit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class Admin::Unit < ActiveFedora::Base

has_subresource 'poster', class_name: 'IndexedFile'

before_save :normalize_read_users, if: proc { |u| u.default_read_users_changed? }
around_save :reindex_members, if: proc { |u| u.name_changed? }

def created_at
Expand Down Expand Up @@ -228,4 +229,8 @@ def remove_edit_user(name)
def add_edit_user(name)
self.default_permissions.build({ name: name, type: 'person', access: 'edit' })
end

def normalize_read_users
self.default_read_users = default_read_users.map(&:downcase)
end
end
2 changes: 1 addition & 1 deletion app/models/concerns/admin_collection_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def inherited_depositors
end

def inherited_read_users
unit.default_read_users
unit.default_read_users.map(&:downcase)
end

def inherited_read_groups
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/media_object_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def inherited_virtual_read_groups
def inherited_read_users
return [] unless collection
users = collection.default_read_users.to_a + collection.inherited_read_users.to_a
users.uniq
users.map(&:downcase).uniq
end

def inherited_read_groups
Expand Down
4 changes: 4 additions & 0 deletions app/models/media_object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class MediaObject < ActiveFedora::Base
before_save :update_dependent_properties!, prepend: true
before_save :update_permalink, if: Proc.new { |mo| mo.persisted? && mo.published? }, prepend: true
before_save :assign_id!, prepend: true
before_save :normalize_read_users

after_find do
# Force loading of section_ids from list_source
Expand Down Expand Up @@ -504,4 +505,7 @@ def sections_with_rendering_files?(tags)
tags.any? { |t| sections_with_files(tag: t).present? }
end

def normalize_read_users
self.read_users = read_users.map(&:downcase)
end
end
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def self.find_for_lti(auth_hash, signed_in_resource=nil)
end

def self.autocomplete(query, _id = nil)
self.where("username LIKE :q OR email LIKE :q", q: "%#{query}%").collect { |user|
self.where("LOWER(username) LIKE LOWER(:q) OR LOWER(email) LIKE LOWER(:q)", q: "%#{query}%").collect { |user|
{ id: user.user_key, display: user.user_key }
}
end
Expand Down
14 changes: 14 additions & 0 deletions lib/tasks/avalon_migrations.rake
Original file line number Diff line number Diff line change
Expand Up @@ -168,5 +168,19 @@ namespace :avalon do
ReindexJob.perform_later(collection.media_object_ids)
end
end
desc "Downcase existing special access user entries"
task special_access_users: :environment do
ids = ActiveFedora::SolrService.query('inheritable_read_access_person_ssim:/.*[A-Z].*/ OR read_access_person_ssim:/.*[A-Z].*/', fl: 'id', rows: 100_000)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think 100,000 rows is necessarily always going to be large enough. Maybe this needs to be converted into a loop where it paginates while start + rows < numFound? It could use a smaller batch size then (1000 is a default in ActiveFedora).

I like how this can be run as many times as needed until everything has been migrated. Another idea is adding a logging line about number of rows found and number migrated in the end to help give some feedback that things worked and fully completed. In this vein, maybe the save! could be changed into a save and the error logged if the save failed? Then the migration could keep running and do the most it can do instead of getting hung up on a broken item.

ids.each do |i|
object = ActiveFedora::Base.find(i[:id])
case object.class
when Admin::Unit, Admin::Collection
object.default_read_users = object.default_read_users.map(&:downcase)
when MediaObject
object.read_users = object.read_users.map(&:downcase)
end
object.save!
end
end
end
end
11 changes: 9 additions & 2 deletions spec/controllers/admin_collections_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,15 @@
end

context "add new special access" do
it "user" do
expect{ put 'update', params: { id: collection.id, submit_add_user: "Add", add_user: "test1@example.com", add_user_display: "test1" }}.to change{ collection.reload.default_read_users.size }.by(1)
context "user" do
it "adds user" do
expect { put 'update', params: { id: collection.id, submit_add_user: "Add", add_user: "test1@example.com", add_user_display: "test1" } }.to change { collection.reload.default_read_users.size }.by(1)
end

it "is case insensitive" do
put 'update', params: { id: collection.id, submit_add_user: "Add", add_user: "Test2@example.com", add_user_display: "Test2" }
expect(collection.reload.default_read_users).to include("test2@example.com")
end
end

it "group" do
Expand Down
11 changes: 9 additions & 2 deletions spec/controllers/admin_units_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,15 @@
end

context "add new special access" do
it "user" do
expect { put 'update', params: { id: unit.id, submit_add_user: "Add", add_user: "test1@example.com", add_user_display: "test1" } }.to change { unit.reload.default_read_users.size }.by(1)
context "user" do
it "adds user" do
expect { put 'update', params: { id: unit.id, submit_add_user: "Add", add_user: "test1@example.com", add_user_display: "test1" } }.to change { unit.reload.default_read_users.size }.by(1)
end

it "is case insensitive" do
put 'update', params: { id: unit.id, submit_add_user: "Add", add_user: "Test2@example.com", add_user_display: "Test2" }
expect(unit.reload.default_read_users).to include("test2@example.com")
end
end

it "group" do
Expand Down
12 changes: 9 additions & 3 deletions spec/controllers/media_objects_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1996,9 +1996,15 @@
before(:each) { login_user media_object.collection.managers.first }

context "grant and revoke special read access" do
it "grants and revokes special read access to users" do
expect { put :update, params: { id: media_object.id, step: 'access-control', donot_advance: 'true', add_user: user, submit_add_user: 'Add' } }.to change { media_object.reload.read_users }.from([]).to([user])
expect {put :update, params: { id: media_object.id, step: 'access-control', donot_advance: 'true', remove_user: user, submit_remove_user: 'Remove' } }.to change { media_object.reload.read_users }.from([user]).to([])
context "users" do
it "grants and revokes special read access to users" do
expect { put :update, params: { id: media_object.id, step: 'access-control', donot_advance: 'true', add_user: user, submit_add_user: 'Add' } }.to change { media_object.reload.read_users }.from([]).to([user])
expect {put :update, params: { id: media_object.id, step: 'access-control', donot_advance: 'true', remove_user: user, submit_remove_user: 'Remove' } }.to change { media_object.reload.read_users }.from([user]).to([])
end
it "is case insensitive" do
put :update, params: { id: media_object.id, step: 'access-control', donot_advance: 'true', add_user: "Test1@example.com", submit_add_user: 'Add' }
expect(media_object.reload.read_users).to include("test1@example.com")
end
end
it "grants and revokes special read access to groups" do
expect { put :update, params: { id: media_object.id, step: 'access-control', donot_advance: 'true', add_group: group, submit_add_group: 'Add' } }.to change { media_object.reload.read_groups }.from([]).to([group])
Expand Down