From d1e2825b951c19e706148d88b0fc0bd834d0920e Mon Sep 17 00:00:00 2001 From: Jonathon Date: Tue, 22 Oct 2024 21:21:08 +0100 Subject: [PATCH 1/6] Add rake task to assign access permissions for an organisation Four tasks are added that allow us to: - Assign and remove permissions on a case by case basis for specific artefacts - Remove all permission flags from an artefact, essentially resetting it to open permission - Assign an org permission to a group of artefacts from a CSV file containing rows of URLs There is no easy way to find the OrgID from an org name so OrgID must be provided by us when using it in the rake tasks. --- lib/tasks/access_and_permissions.rake | 60 +++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 lib/tasks/access_and_permissions.rake diff --git a/lib/tasks/access_and_permissions.rake b/lib/tasks/access_and_permissions.rake new file mode 100644 index 000000000..c9cb12291 --- /dev/null +++ b/lib/tasks/access_and_permissions.rake @@ -0,0 +1,60 @@ +namespace :permissions do + desc "Add an organisation to an document's access permissions list" + task :add_organisation_access, %i[document_content_id org_content_id] => :environment do |_, args| + document = Artefact.where(id: args[document_content_id]) + if document.nil? + puts "Document not found, no permissions changed." + elsif document.latest_edition.owning_org_content_ids.include? args[org_content_id] + puts "Organisation already has permission to access this document" + else + Edition.where(panopticon_id: document.id).each do |edition| + edition.owning_org_content_ids << args[org_content_id] + end + document.save_as_task("PermissionsAddition") + puts "Access permission successfully assigned" + end + end + + desc "Remove an organisation from an document's access permissions list" + task :remove_organisation_access, %i[document_content_id org_content_id] => :environment do |_, args| + document = Artefact.where(id: args[document_content_id]) + if document.exists? + Edition.where(panopticon_id: document.id).each do |edition| + edition.owning_org_content_ids.delete(args[org_content_id]) + end + document.save_as_task("PermissionsRemoval") + puts "Access removed from organisation" + else + puts "Document not found, no permissions changed." + end + end + + desc "Remove all access permissions from an document" + task :remove_all_access_flags, %i[document_content_id] => :environment do |_, args| + document = Artefact.where(id: args[document_content_id]) + if document.exists? + Edition.where(panopticon_id: document.id).each do |edition| + edition.owning_org_content_ids.clear + end + document.save_as_task("PermissionsClear") + puts "All access permissions removed" + else + puts "Document not found, no permissions changed." + end + end + + desc "Bulk process access permissions from CSV of URLs - See doc" + task :bulk_process_access_flags, %i[csv_filename organisation_id] => :environment do |_, args| + csv = CSV.new(args[csv_filename]) + csv.each do |row| + path = row[0] + path.slice! "https://www.gov.uk/" + document = Artefact.where(slug: path) + + next if document.nil? + + Rake::Task["permissions:add_organisation_access"].reenable # I prefer to do this first but can be done after if cleaner + Rake::Task["permissions:add_organisation_access"].invoke(document.id, organisation_id) + end + end +end From 2279393832002756cee9a311e61b6936604eaf93 Mon Sep 17 00:00:00 2001 From: Helen Pickavance Date: Mon, 18 Nov 2024 16:19:05 +0000 Subject: [PATCH 2/6] Change to find to fetch singular Artefact --- lib/tasks/access_and_permissions.rake | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/tasks/access_and_permissions.rake b/lib/tasks/access_and_permissions.rake index c9cb12291..09a6fd123 100644 --- a/lib/tasks/access_and_permissions.rake +++ b/lib/tasks/access_and_permissions.rake @@ -1,14 +1,14 @@ namespace :permissions do desc "Add an organisation to an document's access permissions list" task :add_organisation_access, %i[document_content_id org_content_id] => :environment do |_, args| - document = Artefact.where(id: args[document_content_id]) + document = Artefact.find_by(id: args[:document_content_id]) if document.nil? puts "Document not found, no permissions changed." - elsif document.latest_edition.owning_org_content_ids.include? args[org_content_id] + elsif document.latest_edition.owning_org_content_ids.include? args[:org_content_id] puts "Organisation already has permission to access this document" else Edition.where(panopticon_id: document.id).each do |edition| - edition.owning_org_content_ids << args[org_content_id] + edition.owning_org_content_ids << args[:org_content_id] end document.save_as_task("PermissionsAddition") puts "Access permission successfully assigned" From 24337a4d0b60aab1c29cf59d8a55bf12bb0fb261 Mon Sep 17 00:00:00 2001 From: Barbara Slawinska Date: Tue, 19 Nov 2024 08:35:26 +0000 Subject: [PATCH 3/6] omit validation to apply permissions to all editions, additionally correcting save_as_task! syntax --- lib/tasks/access_and_permissions.rake | 42 +++++---------------------- 1 file changed, 8 insertions(+), 34 deletions(-) diff --git a/lib/tasks/access_and_permissions.rake b/lib/tasks/access_and_permissions.rake index 09a6fd123..7e21ef552 100644 --- a/lib/tasks/access_and_permissions.rake +++ b/lib/tasks/access_and_permissions.rake @@ -1,3 +1,5 @@ +require "csv" + namespace :permissions do desc "Add an organisation to an document's access permissions list" task :add_organisation_access, %i[document_content_id org_content_id] => :environment do |_, args| @@ -9,52 +11,24 @@ namespace :permissions do else Edition.where(panopticon_id: document.id).each do |edition| edition.owning_org_content_ids << args[:org_content_id] + edition.save!(validate: false) end - document.save_as_task("PermissionsAddition") + document.save_as_task!("PermissionsAddition") puts "Access permission successfully assigned" end end - desc "Remove an organisation from an document's access permissions list" - task :remove_organisation_access, %i[document_content_id org_content_id] => :environment do |_, args| - document = Artefact.where(id: args[document_content_id]) - if document.exists? - Edition.where(panopticon_id: document.id).each do |edition| - edition.owning_org_content_ids.delete(args[org_content_id]) - end - document.save_as_task("PermissionsRemoval") - puts "Access removed from organisation" - else - puts "Document not found, no permissions changed." - end - end - - desc "Remove all access permissions from an document" - task :remove_all_access_flags, %i[document_content_id] => :environment do |_, args| - document = Artefact.where(id: args[document_content_id]) - if document.exists? - Edition.where(panopticon_id: document.id).each do |edition| - edition.owning_org_content_ids.clear - end - document.save_as_task("PermissionsClear") - puts "All access permissions removed" - else - puts "Document not found, no permissions changed." - end - end - desc "Bulk process access permissions from CSV of URLs - See doc" task :bulk_process_access_flags, %i[csv_filename organisation_id] => :environment do |_, args| - csv = CSV.new(args[csv_filename]) - csv.each do |row| - path = row[0] + CSV.foreach(args[:csv_filename], headers: true) do |row| + path = row[1] path.slice! "https://www.gov.uk/" - document = Artefact.where(slug: path) + document = Artefact.find_by(slug: path) next if document.nil? Rake::Task["permissions:add_organisation_access"].reenable # I prefer to do this first but can be done after if cleaner - Rake::Task["permissions:add_organisation_access"].invoke(document.id, organisation_id) + Rake::Task["permissions:add_organisation_access"].invoke(document.id, args[:organisation_id]) end end end From 40d9f35843e354d25154d6779615747fb62630d0 Mon Sep 17 00:00:00 2001 From: Barbara Slawinska Date: Thu, 21 Nov 2024 09:27:15 +0000 Subject: [PATCH 4/6] Add tests --- .../unit/tasks/access_and_permissions_test.rb | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 test/unit/tasks/access_and_permissions_test.rb diff --git a/test/unit/tasks/access_and_permissions_test.rb b/test/unit/tasks/access_and_permissions_test.rb new file mode 100644 index 000000000..22fb5586f --- /dev/null +++ b/test/unit/tasks/access_and_permissions_test.rb @@ -0,0 +1,55 @@ +require "test_helper" +require "rake" + +class AccessAndPermissionsTaskTest < ActiveSupport::TestCase + setup do + @add_organisation_access_task = Rake::Task["permissions:add_organisation_access"] + @bulk_process_task = Rake::Task["permissions:bulk_process_access_flags"] + + @add_organisation_access_task.reenable + @bulk_process_task.reenable + + @artefact1 = FactoryBot.create(:artefact, slug: "example-slug-1") + @artefact2 = FactoryBot.create(:artefact, slug: "example-slug-2") + @edition1 = FactoryBot.create(:edition, panopticon_id: @artefact1.id, owning_org_content_ids: []) + @edition2 = FactoryBot.create(:edition, panopticon_id: @artefact2.id, owning_org_content_ids: []) + @csv_file_path = Rails.root.join("tmp/test_bulk_access.csv") + CSV.open(@csv_file_path, "w") do |csv| + csv << %w[Header1 URL] + csv << ["Row1", "https://www.gov.uk/example-slug-1"] + csv << ["Row2", "https://www.gov.uk/example-slug-2"] + end + end + + test "add_organisation_access assigns permissions correctly" do + organisation_id = "test-org-id" + + @add_organisation_access_task.invoke(@artefact1.id, organisation_id) + @edition1.reload + + assert_includes @edition1.owning_org_content_ids, organisation_id + assert_not_includes @edition2.owning_org_content_ids, organisation_id + end + + test "add_organisation_access does not duplicate access" do + organisation_id = "test-org-id" + @edition1.update!(owning_org_content_ids: [organisation_id]) + + @add_organisation_access_task.invoke(@artefact1.id, organisation_id) + @edition1.reload + + assert_equal(1, @edition1.owning_org_content_ids.count { |id| id == organisation_id }) + end + + test "bulk_process_access_flags processes all rows in CSV" do + organisation_id = "test-org-id" + + @bulk_process_task.invoke(@csv_file_path.to_s, organisation_id) + + @edition1.reload + @edition2.reload + + assert_includes @edition1.owning_org_content_ids, organisation_id + assert_includes @edition2.owning_org_content_ids, organisation_id + end +end From 26a3320c5b37900cf8cf32c36df1bfbd2e757fd9 Mon Sep 17 00:00:00 2001 From: Helen Pickavance Date: Thu, 21 Nov 2024 11:57:37 +0000 Subject: [PATCH 5/6] Add logging to file and exception handling - Logging allows us to gather a list of documents that cannot be found - Failure to find is typically because the path/slug has changed. --- lib/tasks/access_and_permissions.rake | 44 +++++++++++++++++++-------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/lib/tasks/access_and_permissions.rake b/lib/tasks/access_and_permissions.rake index 7e21ef552..b72dcd3a3 100644 --- a/lib/tasks/access_and_permissions.rake +++ b/lib/tasks/access_and_permissions.rake @@ -1,34 +1,52 @@ require "csv" namespace :permissions do - desc "Add an organisation to an document's access permissions list" - task :add_organisation_access, %i[document_content_id org_content_id] => :environment do |_, args| + desc "Add an organisation to a document's access permissions list" + task :add_organisation_access, %i[document_content_id org_content_id log_file] => :environment do |_, args| document = Artefact.find_by(id: args[:document_content_id]) + if document.nil? - puts "Document not found, no permissions changed." - elsif document.latest_edition.owning_org_content_ids.include? args[:org_content_id] - puts "Organisation already has permission to access this document" + message = "Document ID #{args[:document_content_id]} not found, no permissions changed." + elsif document.latest_edition.owning_org_content_ids.include?(args[:org_content_id]) + message = "Organisation already has permission to access the document with ID - #{document.id}" else Edition.where(panopticon_id: document.id).each do |edition| edition.owning_org_content_ids << args[:org_content_id] edition.save!(validate: false) end document.save_as_task!("PermissionsAddition") - puts "Access permission successfully assigned" + message = "Access permission successfully assigned to document with ID - #{document.id}" end + args[:log_file] ? args[:log_file].puts(message) : puts(message) + rescue StandardError => e + error_message = "An error occurred while processing document ID #{args[:document_content_id]}: #{e.message}" + args[:log_file] ? args[:log_file].puts(error_message) : puts(error_message) end desc "Bulk process access permissions from CSV of URLs - See doc" task :bulk_process_access_flags, %i[csv_filename organisation_id] => :environment do |_, args| - CSV.foreach(args[:csv_filename], headers: true) do |row| - path = row[1] - path.slice! "https://www.gov.uk/" - document = Artefact.find_by(slug: path) + log_file = File.new("/tmp/permissions_rake_log.txt", "w") + + begin + CSV.foreach(args[:csv_filename], headers: true) do |row| + path = row[1] + path&.slice!("https://www.gov.uk/") + document = Artefact.find_by(slug: path) - next if document.nil? + if document.nil? + log_file.puts "Document with slug '#{path}' not found. Skipping..." + next + end - Rake::Task["permissions:add_organisation_access"].reenable # I prefer to do this first but can be done after if cleaner - Rake::Task["permissions:add_organisation_access"].invoke(document.id, args[:organisation_id]) + Rake::Task["permissions:add_organisation_access"].reenable + Rake::Task["permissions:add_organisation_access"].invoke(document.id, args[:organisation_id], log_file) + rescue Mongoid::Errors::DocumentNotFound => e + log_file.puts "--- Document not found error ---" + log_file.puts e.detailed_message + log_file.puts "------" + end + ensure + log_file.close end end end From 06a376e1035348031880539a80e49d4b3868bbb8 Mon Sep 17 00:00:00 2001 From: Helen Pickavance Date: Thu, 21 Nov 2024 17:28:04 +0000 Subject: [PATCH 6/6] Log org id with log messages and fix rescue blocks --- lib/tasks/access_and_permissions.rake | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/tasks/access_and_permissions.rake b/lib/tasks/access_and_permissions.rake index b72dcd3a3..3895cff88 100644 --- a/lib/tasks/access_and_permissions.rake +++ b/lib/tasks/access_and_permissions.rake @@ -6,26 +6,27 @@ namespace :permissions do document = Artefact.find_by(id: args[:document_content_id]) if document.nil? - message = "Document ID #{args[:document_content_id]} not found, no permissions changed." + message = "Document ID #{args[:document_content_id]} not found, no permissions added for organisation with ID: #{args[:org_content_id]}" elsif document.latest_edition.owning_org_content_ids.include?(args[:org_content_id]) - message = "Organisation already has permission to access the document with ID - #{document.id}" + message = "Organisation with ID: #{args[:org_content_id]} already has permission to access the document with ID: #{document.id}" else Edition.where(panopticon_id: document.id).each do |edition| edition.owning_org_content_ids << args[:org_content_id] edition.save!(validate: false) end document.save_as_task!("PermissionsAddition") - message = "Access permission successfully assigned to document with ID - #{document.id}" + message = "Access permission for organisation ID: #{args[:org_content_id]}, successfully assigned to document with ID: #{document.id}" end args[:log_file] ? args[:log_file].puts(message) : puts(message) - rescue StandardError => e + rescue Mongoid::Errors::DocumentNotFound => e error_message = "An error occurred while processing document ID #{args[:document_content_id]}: #{e.message}" args[:log_file] ? args[:log_file].puts(error_message) : puts(error_message) end - desc "Bulk process access permissions from CSV of URLs - See doc" + desc "Bulk process access permissions from CSV of URLs" task :bulk_process_access_flags, %i[csv_filename organisation_id] => :environment do |_, args| - log_file = File.new("/tmp/permissions_rake_log.txt", "w") + log_file = File.open("/tmp/permissions_rake_log.txt", "w") + log_file.puts("Adding access permissions for the organisation with ID - #{args[:organisation_id]}") begin CSV.foreach(args[:csv_filename], headers: true) do |row| @@ -40,8 +41,8 @@ namespace :permissions do Rake::Task["permissions:add_organisation_access"].reenable Rake::Task["permissions:add_organisation_access"].invoke(document.id, args[:organisation_id], log_file) - rescue Mongoid::Errors::DocumentNotFound => e - log_file.puts "--- Document not found error ---" + rescue StandardError => e + log_file.puts "--- Error occurred ---" log_file.puts e.detailed_message log_file.puts "------" end