From 7282d88e4f99933578cfdb664173765421f79ef8 Mon Sep 17 00:00:00 2001 From: Jamie Stamp Date: Thu, 7 May 2026 11:25:28 +0100 Subject: [PATCH 1/3] Convert access_limited column to be an integer As a boolean in MySQL is set up as a tinyint(1), we can set up the new options to extend what was already there. ------------------------------------- | DB | Before | After | | Value | migration | migration | |-----------------------------------| | 0 | False | disabled | | 1 | True | organisations | | 2 | | named_users | ------------------------------------- Taking this approach means we maintain the correct options when converting the column. Updates any references to the old values to `disabled` and `organisations` as needed. # Conflicts: # db/schema.rb --- app/models/concerns/edition/limited_access.rb | 16 ++++++++++---- app/services/edition_publisher.rb | 2 +- .../editions/_access_limiting_fields.html.erb | 6 ++--- ..._migrate_access_limited_to_integer_enum.rb | 13 +++++++++++ db/schema.rb | 18 ++++++++++++--- .../admin_statistics_announcements_steps.rb | 2 +- .../most_recent_editions_steps.rb | 2 +- .../admin/editions/tags_component_test.rb | 2 +- test/factories/editions.rb | 2 +- .../edition_access_limited_controller_test.rb | 22 +++++++++---------- .../admin/editions_controller_test.rb | 12 +++++----- .../editorial_remarks_controller_test.rb | 2 +- .../external_attachments_controller_test.rb | 2 +- .../admin/html_attachments_controller_test.rb | 2 +- .../admin/publications_controller_test.rb | 2 +- .../asset_access_options_integration_test.rb | 6 ++--- test/integration/asset_manager_test.rb | 2 +- .../admin_edition_controller_test_helpers.rb | 12 +++++----- .../app/models/admin/edition_filter_test.rb | 6 ++--- .../models/attachment_data_visibility_test.rb | 8 +++---- .../edition_taggable_organisations_test.rb | 6 ++--- .../app/models/edition/limited_access_test.rb | 6 ++--- test/unit/app/models/publication_test.rb | 8 +++---- .../app/models/statistical_data_set_test.rb | 4 ++-- .../services/draft_edition_updater_test.rb | 4 ++-- .../asset_manager_create_asset_job_test.rb | 6 ++--- .../authority/authority_test_helper.rb | 2 +- 27 files changed, 104 insertions(+), 71 deletions(-) create mode 100644 db/migrate/20260506100009_migrate_access_limited_to_integer_enum.rb diff --git a/app/models/concerns/edition/limited_access.rb b/app/models/concerns/edition/limited_access.rb index cc9ca03f3dc..fe92ce5e552 100644 --- a/app/models/concerns/edition/limited_access.rb +++ b/app/models/concerns/edition/limited_access.rb @@ -2,6 +2,7 @@ module Edition::LimitedAccess extend ActiveSupport::Concern included do + enum :access_limited, { disabled: 0, organisations: 1, named_users: 2 } after_initialize :set_access_limited end @@ -16,15 +17,22 @@ def access_limited_object end def access_limited? - self[:access_limited] + organisations? || named_users? end delegate :access_limited_by_default?, to: :class + def access_limited=(value) + @_access_limited_explicitly_set = true + super + end + def set_access_limited - if new_record? && access_limited.nil? - self.access_limited = access_limited_by_default? - end + return unless new_record? + return if @_access_limited_explicitly_set + + self.access_limited = access_limited_by_default? ? :organisations : :disabled + @_access_limited_explicitly_set = false end def accessible_to?(user) diff --git a/app/services/edition_publisher.rb b/app/services/edition_publisher.rb index a358a429d62..300590dd29b 100644 --- a/app/services/edition_publisher.rb +++ b/app/services/edition_publisher.rb @@ -26,7 +26,7 @@ def verb def prepare_edition flag_if_political_content! - edition.access_limited = false + edition.access_limited = :disabled edition.major_change_published_at = Time.zone.now unless edition.minor_change? edition.make_public_at(edition.major_change_published_at) edition.increment_version_number diff --git a/app/views/admin/editions/_access_limiting_fields.html.erb b/app/views/admin/editions/_access_limiting_fields.html.erb index fdbf0eec33a..3f350420370 100644 --- a/app/views/admin/editions/_access_limiting_fields.html.erb +++ b/app/views/admin/editions/_access_limiting_fields.html.erb @@ -4,7 +4,7 @@ heading_level: 3, heading_size: "m", } do %> - <%= form.hidden_field :access_limited, value: "0" %> + <%= form.hidden_field :access_limited, value: "disabled" %> <%= render "govuk_publishing_components/components/checkboxes", { name: "edition[access_limited]", @@ -13,8 +13,8 @@ items: [ { label: "Limit access to publishers from organisations associated with this document before you publish", - value: 1, - checked: edition.access_limited, + value: "organisations", + checked: edition.access_limited?, }, ], } %> diff --git a/db/migrate/20260506100009_migrate_access_limited_to_integer_enum.rb b/db/migrate/20260506100009_migrate_access_limited_to_integer_enum.rb new file mode 100644 index 00000000000..0647d3dc8b5 --- /dev/null +++ b/db/migrate/20260506100009_migrate_access_limited_to_integer_enum.rb @@ -0,0 +1,13 @@ +class MigrateAccessLimitedToIntegerEnum < ActiveRecord::Migration[8.0] + def up + safety_assured do + change_column :editions, :access_limited, :integer, null: false, default: 0 + end + end + + def down + safety_assured do + change_column :editions, :access_limited, :boolean, null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 7bc07467943..0d9168f02f8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_05_03_161913) do +ActiveRecord::Schema[8.1].define(version: 2026_05_07_120000) do create_table "assets", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.string "asset_manager_id", null: false t.bigint "assetable_id" @@ -327,6 +327,15 @@ t.index ["locale"], name: "index_edition_translations_on_locale" end + create_table "edition_user_access_grants", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.datetime "created_at", null: false + t.integer "edition_id" + t.datetime "updated_at", null: false + t.integer "user_id" + t.index ["edition_id"], name: "index_edition_user_access_grants_on_edition_id" + t.index ["user_id"], name: "index_edition_user_access_grants_on_user_id" + end + create_table "edition_world_locations", id: :integer, charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.datetime "created_at", precision: nil t.integer "edition_id" @@ -347,7 +356,8 @@ end create_table "editions", id: :integer, charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| - t.boolean "access_limited", null: false + t.integer "access_limited", default: 0, null: false + t.integer "accessible_by", default: 0, null: false t.string "additional_related_mainstream_content_title" t.string "additional_related_mainstream_content_url" t.boolean "all_nation_applicability", default: true @@ -615,7 +625,7 @@ t.integer "edition_id" t.integer "image_data_id" t.datetime "updated_at", precision: nil - t.string "usage", null: false + t.string "usage" t.index ["edition_id"], name: "index_images_on_edition_id" t.index ["image_data_id"], name: "index_images_on_image_data_id" end @@ -1228,6 +1238,8 @@ add_foreign_key "documents", "editions", column: "latest_edition_id", on_update: :cascade, on_delete: :nullify add_foreign_key "documents", "editions", column: "live_edition_id", on_update: :cascade, on_delete: :nullify + add_foreign_key "edition_user_access_grants", "editions" + add_foreign_key "edition_user_access_grants", "users" add_foreign_key "editions", "governments", on_delete: :nullify add_foreign_key "link_checker_api_report_links", "link_checker_api_reports" add_foreign_key "link_checker_api_reports", "editions" diff --git a/features/step_definitions/admin_statistics_announcements_steps.rb b/features/step_definitions/admin_statistics_announcements_steps.rb index fa65ffba101..d9a7a250e0e 100644 --- a/features/step_definitions/admin_statistics_announcements_steps.rb +++ b/features/step_definitions/admin_statistics_announcements_steps.rb @@ -6,7 +6,7 @@ @statistics_publication = create( :publication, :draft, - access_limited: false, + access_limited: :disabled, publication_type_id: PublicationType::OfficialStatistics.id, title:, ) diff --git a/features/step_definitions/most_recent_editions_steps.rb b/features/step_definitions/most_recent_editions_steps.rb index 52741185365..d44f0cfa80e 100644 --- a/features/step_definitions/most_recent_editions_steps.rb +++ b/features/step_definitions/most_recent_editions_steps.rb @@ -10,7 +10,7 @@ current = Edition.find_by(title:).document.latest_edition new_draft = current.create_draft(random_editor) new_draft.organisations << org - new_draft.access_limited = true + new_draft.access_limited = :organisations new_draft.change_note = "Limited to #{org.name}" new_draft.save! end diff --git a/test/components/admin/editions/tags_component_test.rb b/test/components/admin/editions/tags_component_test.rb index 481cf601ddf..eca2bbfda44 100644 --- a/test/components/admin/editions/tags_component_test.rb +++ b/test/components/admin/editions/tags_component_test.rb @@ -66,7 +66,7 @@ class Admin::Editions::TagsComponentTest < ViewComponent::TestCase end test "adds an access limited tag if edition has limited access" do - edition = build(:edition, access_limited: true) + edition = build(:edition, access_limited: 1) expected_output = "Draft " \ "Limited access" diff --git a/test/factories/editions.rb b/test/factories/editions.rb index c68e0f4aea9..c1843a86107 100644 --- a/test/factories/editions.rb +++ b/test/factories/editions.rb @@ -120,7 +120,7 @@ scheduled_publication { 7.days.from_now } end - trait(:access_limited) { access_limited { true } } + trait(:access_limited) { access_limited { :organisations } } trait(:with_alternative_format_provider) do association :alternative_format_provider, factory: :organisation_with_alternative_format_contact_email diff --git a/test/functional/admin/edition_access_limited_controller_test.rb b/test/functional/admin/edition_access_limited_controller_test.rb index 955f3330a34..2b9b66a6413 100644 --- a/test/functional/admin/edition_access_limited_controller_test.rb +++ b/test/functional/admin/edition_access_limited_controller_test.rb @@ -19,7 +19,7 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase edition = create( :consultation, - access_limited: true, + access_limited: :organisations, create_default_organisation: false, lead_organisations: [organisation], ) @@ -58,7 +58,7 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase edition = create( :consultation, - access_limited: true, + access_limited: :organisations, create_default_organisation: false, lead_organisations: [first_organisation], supporting_organisations: [second_organisation], @@ -75,7 +75,7 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase lead_organisation_ids: [second_organisation.id], supporting_organisation_ids: [third_organisation.id], editorial_remark:, - access_limited: "1", + access_limited: :organisations, }, } @@ -92,7 +92,7 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase edition = create( :consultation, - access_limited: true, + access_limited: :organisations, create_default_organisation: false, lead_organisations: [first_organisation], supporting_organisations: [second_organisation], @@ -107,13 +107,13 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase lead_organisation_ids: [first_organisation.id], supporting_organisation_ids: [second_organisation.id], editorial_remark:, - access_limited: "0", + access_limited: :disabled, }, } assert_equal [first_organisation], edition.reload.lead_organisations assert_equal [second_organisation], edition.supporting_organisations - assert_not edition.access_limited + assert_not edition.reload.access_limited? assert_redirected_to admin_editions_path assert_equal "Access updated for #{edition.title}", flash[:notice] assert_equal "Access options updated by GDS Admin: #{editorial_remark}", edition.editorial_remarks.last.body @@ -125,7 +125,7 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase edition = create( :consultation, - access_limited: true, + access_limited: :organisations, create_default_organisation: false, lead_organisations: [first_organisation], supporting_organisations: [second_organisation], @@ -138,13 +138,13 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase lead_organisation_ids: [first_organisation.id], supporting_organisation_ids: [second_organisation.id], editorial_remark: "", - access_limited: "0", + access_limited: :disabled, }, } assert_template :edit assert_equal ["Editorial remark cannot be blank"], assigns(:edition).errors.full_messages - assert edition.reload.access_limited + assert edition.reload.access_limited? end test "PATCH :update doesn't create an editorial remark or re-render with an error when nothing has changed" do @@ -153,7 +153,7 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase edition = create( :consultation, - access_limited: true, + access_limited: :organisations, create_default_organisation: false, lead_organisations: [first_organisation], supporting_organisations: [second_organisation], @@ -166,7 +166,7 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase lead_organisation_ids: [first_organisation.id], supporting_organisation_ids: [second_organisation.id], editorial_remark: "", - access_limited: "1", + access_limited: :organisations, }, } diff --git a/test/functional/admin/editions_controller_test.rb b/test/functional/admin/editions_controller_test.rb index 4121947927f..8391141a2a8 100644 --- a/test/functional/admin/editions_controller_test.rb +++ b/test/functional/admin/editions_controller_test.rb @@ -286,10 +286,10 @@ class Admin::EditionsControllerTest < ActionController::TestCase login_as create(:writer, organisation: my_organisation) accessible = [ create(:draft_publication), - create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: true, organisations: [my_organisation]), - create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: false, organisations: [other_organisation]), + create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: :organisations, organisations: [my_organisation]), + create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: :disabled, organisations: [other_organisation]), ] - inaccessible = create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: true, organisations: [other_organisation]) + inaccessible = create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: :organisations, organisations: [other_organisation]) get :index, params: { state: :active } @@ -302,7 +302,7 @@ class Admin::EditionsControllerTest < ActionController::TestCase view_test "index should indicate the protected status of limited access editions which I do have access to" do my_organisation = create(:organisation) login_as create(:writer, organisation: my_organisation) - publication = create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: true, organisations: [my_organisation]) + publication = create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: :organisations, organisations: [my_organisation]) get :index, params: { state: :active } @@ -314,7 +314,7 @@ class Admin::EditionsControllerTest < ActionController::TestCase view_test "GET :index admin do not see a view link, but are given a link to edit acccess controls when an access limited edition doesn't belong to their org" do login_as create(:gds_admin) - publication = create(:draft_publication, access_limited: true) + publication = create(:draft_publication, access_limited: :organisations) get :index, params: { state: :active } @@ -330,7 +330,7 @@ class Admin::EditionsControllerTest < ActionController::TestCase my_organisation = create(:organisation) other_organisation = create(:organisation) login_as create(:writer, organisation: my_organisation) - inaccessible = create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: true, organisations: [other_organisation]) + inaccessible = create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: :organisations, organisations: [other_organisation]) post :revise, params: { id: inaccessible } assert_response :forbidden diff --git a/test/functional/admin/editorial_remarks_controller_test.rb b/test/functional/admin/editorial_remarks_controller_test.rb index 3c18f239d8f..2152b2fd10c 100644 --- a/test/functional/admin/editorial_remarks_controller_test.rb +++ b/test/functional/admin/editorial_remarks_controller_test.rb @@ -52,7 +52,7 @@ class Admin::EditorialRemarksControllerTest < ActionController::TestCase end test "should prevent access to inaccessible editions" do - protected_edition = create(:submitted_publication, access_limited: true) + protected_edition = create(:submitted_publication, access_limited: :organisations) get :new, params: { edition_id: protected_edition.id } assert_response :forbidden diff --git a/test/functional/admin/external_attachments_controller_test.rb b/test/functional/admin/external_attachments_controller_test.rb index a4279841e6c..483a6321d22 100644 --- a/test/functional/admin/external_attachments_controller_test.rb +++ b/test/functional/admin/external_attachments_controller_test.rb @@ -26,7 +26,7 @@ def valid_external_attachment_params end test "POST :create ignores external attachments when attachable does not allow them" do - attachable = create(:statistical_data_set, access_limited: false) + attachable = create(:statistical_data_set, access_limited: :disabled) post :create, params: { edition_id: attachable, attachment: valid_external_attachment_params } diff --git a/test/functional/admin/html_attachments_controller_test.rb b/test/functional/admin/html_attachments_controller_test.rb index e187fba9bf0..c1c1d5cf3c8 100644 --- a/test/functional/admin/html_attachments_controller_test.rb +++ b/test/functional/admin/html_attachments_controller_test.rb @@ -54,7 +54,7 @@ def valid_html_attachment_params end test "POST :create ignores html attachments when attachable does not allow them" do - attachable = create(:statistical_data_set, access_limited: false) + attachable = create(:statistical_data_set, access_limited: :disabled) post :create, params: { edition_id: attachable, attachment: valid_html_attachment_params } diff --git a/test/functional/admin/publications_controller_test.rb b/test/functional/admin/publications_controller_test.rb index 2f96896af7e..31ad5bc553e 100644 --- a/test/functional/admin/publications_controller_test.rb +++ b/test/functional/admin/publications_controller_test.rb @@ -138,7 +138,7 @@ class Admin::PublicationsControllerTest < ActionController::TestCase my_organisation = create(:organisation) other_organisation = create(:organisation) @user = login_as(create(:user, organisation: my_organisation)) - inaccessible = create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: true, organisations: [other_organisation]) + inaccessible = create(:draft_publication, publication_type: PublicationType::NationalStatistics, access_limited: :organisations, organisations: [other_organisation]) get :show, params: { id: inaccessible } assert_response :forbidden diff --git a/test/integration/asset_access_options_integration_test.rb b/test/integration/asset_access_options_integration_test.rb index 70183282f2a..ba2eff6df07 100644 --- a/test/integration/asset_access_options_integration_test.rb +++ b/test/integration/asset_access_options_integration_test.rb @@ -71,7 +71,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest end context "given an access-limited draft document" do - let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) } + let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: :organisations) } context "when an attachment is added to the draft document" do before do @@ -153,7 +153,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest end context "given an access-limited draft document and a file attachment" do - let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) } + let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: :organisations) } before do add_file_attachment_with_asset("sample.docx", to: edition) @@ -201,7 +201,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest context "given a draft access-limited consultation" do # the edition has to have same organisation as logged in user, otherwise it's not visible when access_limited = true - let(:edition) { create(:consultation, organisations: [organisation], access_limited: true) } + let(:edition) { create(:consultation, organisations: [organisation], access_limited: :organisations) } let(:outcome_attributes) { FactoryBot.attributes_for(:consultation_outcome) } let!(:outcome) { edition.create_outcome!(outcome_attributes) } diff --git a/test/integration/asset_manager_test.rb b/test/integration/asset_manager_test.rb index c6646fc48f2..c9efe2a967a 100644 --- a/test/integration/asset_manager_test.rb +++ b/test/integration/asset_manager_test.rb @@ -34,7 +34,7 @@ class CreatingAFileAttachment < ActiveSupport::TestCase test "sends the user ids of authorised users to Asset Manager" do organisation = FactoryBot.create(:organisation) - consultation = FactoryBot.create(:consultation, access_limited: true, organisations: [organisation]) + consultation = FactoryBot.create(:consultation, access_limited: :organisations, organisations: [organisation]) @attachment.attachable = consultation @attachment.attachment_data.attachable = consultation @attachment.save! diff --git a/test/support/admin_edition_controller_test_helpers.rb b/test/support/admin_edition_controller_test_helpers.rb index 01f60d36f57..4cf4f5ac2b8 100644 --- a/test/support/admin_edition_controller_test_helpers.rb +++ b/test/support/admin_edition_controller_test_helpers.rb @@ -952,7 +952,7 @@ def should_allow_access_limiting_of(edition_type) params: { edition: controller_attributes_for(edition_type).merge( first_published_at: Date.parse("2010-10-21"), - access_limited: "1", + access_limited: :organisations, lead_organisation_ids: [organisation.id], ), } @@ -963,7 +963,7 @@ def should_allow_access_limiting_of(edition_type) end view_test "edit displays persisted access_limited flag" do - publication = create(edition_type, access_limited: false) + publication = create(edition_type, access_limited: :disabled) get :edit, params: { id: publication } @@ -976,13 +976,13 @@ def should_allow_access_limiting_of(edition_type) test "update records new value of access_limited flag" do controller.current_user.organisation = create(:organisation) controller.current_user.save! - publication = create(edition_type, access_limited: false, organisations: [controller.current_user.organisation]) + publication = create(edition_type, access_limited: :disabled, organisations: [controller.current_user.organisation]) put :update, params: { id: publication, edition: { - access_limited: "1", + access_limited: :organisations, }, } @@ -993,13 +993,13 @@ def should_allow_access_limiting_of(edition_type) controller.current_user.organisation = create(:organisation) controller.current_user.save! organisation = create(:organisation) - edition = create(edition_type, access_limited: false, organisations: [organisation]) + edition = create(edition_type, access_limited: :disabled, organisations: [organisation]) put :update, params: { id: edition, edition: { - access_limited: "1", + access_limited: :organisations, }, } diff --git a/test/unit/app/models/admin/edition_filter_test.rb b/test/unit/app/models/admin/edition_filter_test.rb index ac17cd62644..d4d152102c6 100644 --- a/test/unit/app/models/admin/edition_filter_test.rb +++ b/test/unit/app/models/admin/edition_filter_test.rb @@ -9,7 +9,7 @@ class Admin::EditionFilterTest < ActiveSupport::TestCase user = create(:user) edition = create( :publication, - access_limited: true, + access_limited: :organisations, ) edition.organisations.first.users << user @@ -18,14 +18,14 @@ class Admin::EditionFilterTest < ActiveSupport::TestCase end test "should not return editions which have limited access for other orgs for non-gds admins" do - create(:publication, access_limited: true) + create(:publication, access_limited: :organisations) filter = Admin::EditionFilter.new(Edition, build(:user)) assert_equal 0, filter.editions.count end test "should return limited access editions for GDS admins" do - edition = create(:publication, access_limited: true) + edition = create(:publication, access_limited: :organisations) filter = Admin::EditionFilter.new(Edition, build(:gds_admin)) assert_equal edition, filter.editions.first diff --git a/test/unit/app/models/attachment_data_visibility_test.rb b/test/unit/app/models/attachment_data_visibility_test.rb index 6d3ba7c2387..0697f068335 100644 --- a/test/unit/app/models/attachment_data_visibility_test.rb +++ b/test/unit/app/models/attachment_data_visibility_test.rb @@ -53,7 +53,7 @@ class AttachmentDataVisibilityTest < ActiveSupport::TestCase context "edition is access-limited" do before do - edition.access_limited = true + edition.access_limited = :organisations edition.save! end @@ -85,7 +85,7 @@ class AttachmentDataVisibilityTest < ActiveSupport::TestCase context "new edition is access-limited" do before do new_edition.change_note = "change-note" - new_edition.access_limited = true + new_edition.access_limited = :organisations new_edition.save! end @@ -221,7 +221,7 @@ class AttachmentDataVisibilityTest < ActiveSupport::TestCase context "new edition is access-limited" do before do new_edition.change_note = "change-note" - new_edition.access_limited = true + new_edition.access_limited = :organisations new_edition.save! end @@ -359,7 +359,7 @@ class AttachmentDataVisibilityTest < ActiveSupport::TestCase context "consultation is access-limited" do before do - consultation.update!(access_limited: true) + consultation.update!(access_limited: :organisations) end it "is not accessible to anonymous user" do diff --git a/test/unit/app/models/edition/edition_taggable_organisations_test.rb b/test/unit/app/models/edition/edition_taggable_organisations_test.rb index e28f65023d0..e21f6e3323d 100644 --- a/test/unit/app/models/edition/edition_taggable_organisations_test.rb +++ b/test/unit/app/models/edition/edition_taggable_organisations_test.rb @@ -11,7 +11,7 @@ def setup edition = create( :publication, :draft, - access_limited: false, + access_limited: :disabled, publication_type_id: PublicationType::Guidance.id, organisations: [@lead_org], ) @@ -23,7 +23,7 @@ def setup edition = create( :publication, :draft, - access_limited: false, + access_limited: :disabled, publication_type_id: PublicationType::Form.id, organisations: [@lead_org], ) @@ -42,7 +42,7 @@ def setup :publication, :draft, title: "Title #{index}", - access_limited: false, + access_limited: :disabled, publication_type_id: publication_type.id, organisations: [@lead_org], ) diff --git a/test/unit/app/models/edition/limited_access_test.rb b/test/unit/app/models/edition/limited_access_test.rb index c90088eea5c..f1b7361cc81 100644 --- a/test/unit/app/models/edition/limited_access_test.rb +++ b/test/unit/app/models/edition/limited_access_test.rb @@ -26,10 +26,10 @@ def self.access_limited_by_default? test "can persist limited access flag (regardless of .access_limited_by_default?)" do e = build(:limited_by_default_edition) - e.access_limited = true + e.access_limited = :organisations e.save! assert e.reload.access_limited? - e.access_limited = false + e.access_limited = :disabled e.save! assert_not e.reload.access_limited? end @@ -49,7 +49,7 @@ def self.access_limited_by_default? test "is not accessible if edition is not accessible to user" do user = build(:user) edition_id = 123 - edition = LimitedAccessEdition.new(id: edition_id, access_limited: true) + edition = LimitedAccessEdition.new(id: edition_id, access_limited: :organisations) assert_not edition.accessible_to?(user) end diff --git a/test/unit/app/models/publication_test.rb b/test/unit/app/models/publication_test.rb index 434e13d34e3..31d0e99d328 100644 --- a/test/unit/app/models/publication_test.rb +++ b/test/unit/app/models/publication_test.rb @@ -122,18 +122,18 @@ class PublicationTest < ActiveSupport::TestCase test "new instances respect local access_limited over their publication_type" do limit_by_default, dont_limit_by_default = PublicationType.all.partition(&:access_limited_by_default?).map(&:first) - e = build(:draft_publication, publication_type: limit_by_default, access_limited: false) + e = build(:draft_publication, publication_type: limit_by_default, access_limited: :disabled) assert_not e.access_limited? - e = build(:draft_publication, publication_type: dont_limit_by_default, access_limited: true) + e = build(:draft_publication, publication_type: dont_limit_by_default, access_limited: :organisations) assert e.access_limited? end test "existing instances don't change access_limit when their publication_type does" do limit_by_default, dont_limit_by_default = PublicationType.all.partition(&:access_limited_by_default?).map(&:first) - e = create(:draft_publication, access_limited: false) + e = create(:draft_publication, access_limited: :disabled) e.publication_type = limit_by_default assert_not e.access_limited? - e = create(:draft_publication, access_limited: true) + e = create(:draft_publication, access_limited: :organisations) e.publication_type = dont_limit_by_default assert e.access_limited? end diff --git a/test/unit/app/models/statistical_data_set_test.rb b/test/unit/app/models/statistical_data_set_test.rb index 0c25a7fa1e8..84c5c9a3403 100644 --- a/test/unit/app/models/statistical_data_set_test.rb +++ b/test/unit/app/models/statistical_data_set_test.rb @@ -8,12 +8,12 @@ class StatisticalDataSetTest < ActiveSupport::TestCase end test "specifically limit access" do - data_set = build(:statistical_data_set, access_limited: true) + data_set = build(:statistical_data_set, access_limited: :organisations) assert data_set.access_limited? end test "specifically do not limit access" do - data_set = build(:statistical_data_set, access_limited: false) + data_set = build(:statistical_data_set, access_limited: :disabled) assert_not data_set.access_limited? end diff --git a/test/unit/app/services/draft_edition_updater_test.rb b/test/unit/app/services/draft_edition_updater_test.rb index 91e6c4dfaeb..d6468bf2922 100644 --- a/test/unit/app/services/draft_edition_updater_test.rb +++ b/test/unit/app/services/draft_edition_updater_test.rb @@ -30,7 +30,7 @@ class DraftEditionUpdaterTest < ActiveSupport::TestCase end test "cannot perform if user is limiting their own access" do - edition = create(:draft_publication, access_limited: true, organisations: [create(:organisation)]) + edition = create(:draft_publication, access_limited: :organisations, organisations: [create(:organisation)]) updater = DraftEditionUpdater.new(edition, { current_user: create(:user, organisation: create(:organisation)) }) updater.expects(:notify!).never updater.expects(:update_publishing_api!).never @@ -40,7 +40,7 @@ class DraftEditionUpdaterTest < ActiveSupport::TestCase test "updates editions that cannot be tagged to organisations" do organisation = create(:organisation) - edition = create(:draft_corporate_information_page, organisation:, access_limited: true) + edition = create(:draft_corporate_information_page, organisation:, access_limited: :organisations) updater = DraftEditionUpdater.new(edition, { current_user: create(:user, organisation:) }) updater.expects(:update_publishing_api!).once updater.expects(:notify!).once diff --git a/test/unit/app/sidekiq/asset_manager_create_asset_job_test.rb b/test/unit/app/sidekiq/asset_manager_create_asset_job_test.rb index ddeff1a700c..edaaf647f97 100644 --- a/test/unit/app/sidekiq/asset_manager_create_asset_job_test.rb +++ b/test/unit/app/sidekiq/asset_manager_create_asset_job_test.rb @@ -48,7 +48,7 @@ class AssetManagerCreateAssetJobTest < ActiveSupport::TestCase end test "marks attachments belonging to consultations as access limited" do - consultation = FactoryBot.create(:consultation, organisations: [@organisation], access_limited: true) + consultation = FactoryBot.create(:consultation, organisations: [@organisation], access_limited: :organisations) attachment = FactoryBot.create(:file_attachment, attachable: consultation) attachment.attachment_data.attachable = consultation @@ -58,7 +58,7 @@ class AssetManagerCreateAssetJobTest < ActiveSupport::TestCase end test "marks attachments belonging to consultation responses as access limited" do - consultation = FactoryBot.create(:consultation, organisations: [@organisation], access_limited: true) + consultation = FactoryBot.create(:consultation, organisations: [@organisation], access_limited: :organisations) response = FactoryBot.create(:consultation_outcome, consultation:) attachment = FactoryBot.create(:file_attachment, attachable: response) attachment.attachment_data.attachable = consultation @@ -202,7 +202,7 @@ class AssetManagerCreateAssetJobTest < ActiveSupport::TestCase end test "should not process the file if the attachable has been deleted" do - consultation = FactoryBot.create(:consultation, organisations: [@organisation], access_limited: true) + consultation = FactoryBot.create(:consultation, organisations: [@organisation], access_limited: :organisations) consultation.delete consultation.save!(validate: false) diff --git a/test/unit/lib/whitehall/authority/authority_test_helper.rb b/test/unit/lib/whitehall/authority/authority_test_helper.rb index 91fec5f72db..38399a5c2ff 100644 --- a/test/unit/lib/whitehall/authority/authority_test_helper.rb +++ b/test/unit/lib/whitehall/authority/authority_test_helper.rb @@ -30,7 +30,7 @@ def self.define_edition_factory_methods(edition_type) fpe end define_method("limited_#{edition_type}") do |orgs| - le = FactoryBot.build(edition_type, access_limited: true) + le = FactoryBot.build(edition_type, access_limited: :organisations) le.stubs(:organisations).returns(orgs) le end From acd23f198f421c7ae4dcb6d4279a4981fcdee143 Mon Sep 17 00:00:00 2001 From: Jamie Stamp Date: Thu, 7 May 2026 15:11:24 +0100 Subject: [PATCH 2/3] Add NamedAccess model and migration Introduces the `NamedAccess` model and corresponding migration to manage user access per edition. Ensures email presence, uniqueness within an edition, and valid email format. --- app/models/named_access.rb | 8 ++++++++ db/migrate/20260507120000_create_named_accesses.rb | 11 +++++++++++ db/schema.rb | 13 ++++++------- 3 files changed, 25 insertions(+), 7 deletions(-) create mode 100644 app/models/named_access.rb create mode 100644 db/migrate/20260507120000_create_named_accesses.rb diff --git a/app/models/named_access.rb b/app/models/named_access.rb new file mode 100644 index 00000000000..09770b83518 --- /dev/null +++ b/app/models/named_access.rb @@ -0,0 +1,8 @@ +class NamedAccess < ApplicationRecord + belongs_to :edition, inverse_of: :named_accesses + + validates :email, + presence: true, + uniqueness: { scope: :edition_id, case_sensitive: false }, + format: { with: URI::MailTo::EMAIL_REGEXP } +end diff --git a/db/migrate/20260507120000_create_named_accesses.rb b/db/migrate/20260507120000_create_named_accesses.rb new file mode 100644 index 00000000000..a62c2aec6ef --- /dev/null +++ b/db/migrate/20260507120000_create_named_accesses.rb @@ -0,0 +1,11 @@ +class CreateNamedAccesses < ActiveRecord::Migration[8.0] + def change + create_table :named_accesses do |t| + t.references :edition, type: :integer, null: false, foreign_key: true + t.string :email, null: false + t.timestamps + end + + add_index :named_accesses, %i[edition_id email], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 0d9168f02f8..79d75526a17 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -327,13 +327,13 @@ t.index ["locale"], name: "index_edition_translations_on_locale" end - create_table "edition_user_access_grants", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + create_table "named_accesses", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "created_at", null: false - t.integer "edition_id" + t.integer "edition_id", null: false + t.string "email", null: false t.datetime "updated_at", null: false - t.integer "user_id" - t.index ["edition_id"], name: "index_edition_user_access_grants_on_edition_id" - t.index ["user_id"], name: "index_edition_user_access_grants_on_user_id" + t.index ["edition_id", "email"], name: "index_named_accesses_on_edition_id_and_email", unique: true + t.index ["edition_id"], name: "index_named_accesses_on_edition_id" end create_table "edition_world_locations", id: :integer, charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| @@ -1238,8 +1238,7 @@ add_foreign_key "documents", "editions", column: "latest_edition_id", on_update: :cascade, on_delete: :nullify add_foreign_key "documents", "editions", column: "live_edition_id", on_update: :cascade, on_delete: :nullify - add_foreign_key "edition_user_access_grants", "editions" - add_foreign_key "edition_user_access_grants", "users" + add_foreign_key "named_accesses", "editions" add_foreign_key "editions", "governments", on_delete: :nullify add_foreign_key "link_checker_api_report_links", "link_checker_api_reports" add_foreign_key "link_checker_api_reports", "editions" From 2c38d601ae8d214900e6fb49f1166dab112513b7 Mon Sep 17 00:00:00 2001 From: Jamie Stamp Date: Thu, 7 May 2026 15:14:58 +0100 Subject: [PATCH 3/3] Add named-user access limiting Adds the "Limit access to named publishers" option to the access limiting form. Publishers can now restrict access to a draft to a list of email addresses, with the creating publisher always preserved on the list. `Edition::LimitedAccess` manages the list via the `named_accesses` association with autosave: assigning `access_limited_named_users=` builds and marks-for-destruction records eagerly so the persisted list always matches the input. `EditionRules#access_limit_enforced?` and `Admin::EditionFilter` are extended to enforce membership for the `named_users` mode, and `DraftEditionUpdater` skips its organisation-membership check for it. Co-authored-by: Alex Newton --- .../edition_access_limited_controller.rb | 1 + app/controllers/admin/editions_controller.rb | 1 + app/models/concerns/edition/limited_access.rb | 69 +++++++++++++++ app/services/draft_edition_updater.rb | 2 +- .../editions/_access_limiting_fields.html.erb | 45 +++++++--- db/schema.rb | 3 +- .../authority/rules/edition_rules.rb | 4 +- test/factories/named_accesses.rb | 6 ++ .../edition_access_limited_controller_test.rb | 27 +++++- .../asset_access_options_integration_test.rb | 4 +- .../admin_edition_controller_test_helpers.rb | 3 +- .../app/models/admin/edition_filter_test.rb | 21 +++++ .../app/models/edition/limited_access_test.rb | 87 +++++++++++++++++++ .../services/draft_edition_updater_test.rb | 14 +++ .../authority/department_writer_test.rb | 30 +++++++ 15 files changed, 295 insertions(+), 22 deletions(-) create mode 100644 test/factories/named_accesses.rb diff --git a/app/controllers/admin/edition_access_limited_controller.rb b/app/controllers/admin/edition_access_limited_controller.rb index 27b361b38bf..aae900e8585 100644 --- a/app/controllers/admin/edition_access_limited_controller.rb +++ b/app/controllers/admin/edition_access_limited_controller.rb @@ -48,6 +48,7 @@ def edition_params .fetch(:edition, {}) .permit( :access_limited, + :access_limited_named_users, :editorial_remark, { lead_organisation_ids: [], diff --git a/app/controllers/admin/editions_controller.rb b/app/controllers/admin/editions_controller.rb index df07efe1c66..1b70844ded9 100644 --- a/app/controllers/admin/editions_controller.rb +++ b/app/controllers/admin/editions_controller.rb @@ -222,6 +222,7 @@ def permitted_edition_attributes :scheduled_publication, :lock_version, :access_limited, + :access_limited_named_users, :alternative_format_provider_id, :opening_at, :closing_at, diff --git a/app/models/concerns/edition/limited_access.rb b/app/models/concerns/edition/limited_access.rb index fe92ce5e552..0f2113ffeb3 100644 --- a/app/models/concerns/edition/limited_access.rb +++ b/app/models/concerns/edition/limited_access.rb @@ -1,9 +1,22 @@ module Edition::LimitedAccess extend ActiveSupport::Concern + class Trait < Edition::Traits::Trait + def process_associations_after_save(draft) + @edition.named_accesses.each do |na| + draft.named_accesses.create!(email: na.email) + end + end + end + included do enum :access_limited, { disabled: 0, organisations: 1, named_users: 2 } + has_many :named_accesses, dependent: :destroy, inverse_of: :edition, autosave: true after_initialize :set_access_limited + before_save :destroy_named_accesses_unless_named_users + before_save :ensure_creator_in_named_accesses, if: :named_users? + validate :validate_named_users_emails, if: -> { named_users? } + add_trait Trait end module ClassMethods @@ -38,4 +51,60 @@ def set_access_limited def accessible_to?(user) user.present? && Whitehall::Authority::Enforcer.new(user, self).can?(:see) end + + def access_limited_named_users=(value) + @access_limited_named_users_input = value + new_emails = parse_named_user_emails(value).map(&:downcase).uniq + + existing = active_named_accesses.index_by { |na| na.email.downcase } + + existing.each do |email, na| + na.mark_for_destruction unless new_emails.include?(email) + end + + new_emails.each do |email| + named_accesses.build(email:) unless existing.key?(email) + end + end + + def access_limited_named_users + @access_limited_named_users_input || active_named_accesses.map(&:email).join(", ") + end + +private + + def active_named_accesses + named_accesses.reject(&:marked_for_destruction?) + end + + def parse_named_user_emails(value) + (value || "").split(/[\n,]/).map(&:strip).reject(&:blank?) + end + + def validate_named_users_emails + if active_named_accesses.empty? + errors.add(:access_limited_named_users, "must include at least one email address") + end + + active_named_accesses.select(&:new_record?).each do |na| + next if URI::MailTo::EMAIL_REGEXP.match?(na.email) + + errors.add(:access_limited_named_users, "#{na.email} is not a valid email address") + end + end + + def destroy_named_accesses_unless_named_users + return if named_users? + + named_accesses.each(&:mark_for_destruction) + end + + def ensure_creator_in_named_accesses + return if creator&.email.blank? + + email = creator.email.downcase + return if active_named_accesses.any? { |na| na.email.casecmp?(email) } + + named_accesses.build(email:) + end end diff --git a/app/services/draft_edition_updater.rb b/app/services/draft_edition_updater.rb index 04011ecceb6..c0073b81cf5 100644 --- a/app/services/draft_edition_updater.rb +++ b/app/services/draft_edition_updater.rb @@ -24,7 +24,7 @@ def verb private def should_check_current_user_will_retain_access? - @options[:current_user].present? && edition.access_limited? + @options[:current_user].present? && edition.organisations? end def access_limit_excludes_current_user? diff --git a/app/views/admin/editions/_access_limiting_fields.html.erb b/app/views/admin/editions/_access_limiting_fields.html.erb index 3f350420370..16e561ef52f 100644 --- a/app/views/admin/editions/_access_limiting_fields.html.erb +++ b/app/views/admin/editions/_access_limiting_fields.html.erb @@ -1,22 +1,41 @@
- <%= render "govuk_publishing_components/components/fieldset", { - legend_text: "Limit access", - heading_level: 3, - heading_size: "m", - } do %> - <%= form.hidden_field :access_limited, value: "disabled" %> - - <%= render "govuk_publishing_components/components/checkboxes", { + <% named_users_value = edition.named_users? ? edition.access_limited_named_users.presence : nil %> + <% named_users_value ||= current_user.email %> + <% named_users_error = edition.errors[:access_limited_named_users].first %> + <%= render "govuk_publishing_components/components/radio", { + heading: "Limit access", name: "edition[access_limited]", id: "edition_access_limited", - error_items: errors_for(edition.errors, :access_limited), items: [ { - label: "Limit access to publishers from organisations associated with this document before you publish", - value: "organisations", - checked: edition.access_limited?, + value: :disabled, + text: "No – This document should be available to all publishers", + bold: true, + checked: edition.disabled?, + }, + { + value: :organisations, + text: "Limit access to publishers from organisations associated with this document", + bold: true, + checked: edition.organisations?, + }, + { + value: :named_users, + text: "Limit access to named publishers", + bold: true, + checked: edition.named_users?, + conditional: (render "govuk_publishing_components/components/textarea", { + label: { + text: "Add publishers who will have access", + bold: true, + }, + name: "edition[access_limited_named_users]", + textarea_id: "edition_access_limited_named_users", + error_message: named_users_error, + value: named_users_value, + hint: "Add the emails of the publishers who will have access to this document before publishing. After publishing the document will be available to all publishers in the organisation associated with this document.", + }), }, ], } %> - <% end %>
diff --git a/db/schema.rb b/db/schema.rb index 79d75526a17..73046cd52e2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -357,7 +357,6 @@ create_table "editions", id: :integer, charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.integer "access_limited", default: 0, null: false - t.integer "accessible_by", default: 0, null: false t.string "additional_related_mainstream_content_title" t.string "additional_related_mainstream_content_url" t.boolean "all_nation_applicability", default: true @@ -625,7 +624,7 @@ t.integer "edition_id" t.integer "image_data_id" t.datetime "updated_at", precision: nil - t.string "usage" + t.string "usage", null: false t.index ["edition_id"], name: "index_images_on_edition_id" t.index ["image_data_id"], name: "index_images_on_image_data_id" end diff --git a/lib/whitehall/authority/rules/edition_rules.rb b/lib/whitehall/authority/rules/edition_rules.rb index 4e310fbe867..246469355f0 100644 --- a/lib/whitehall/authority/rules/edition_rules.rb +++ b/lib/whitehall/authority/rules/edition_rules.rb @@ -93,10 +93,12 @@ def can_with_a_historic_instance?(action) end def access_limit_enforced? - if subject.access_limited? + if subject.organisations? organisations = subject.organisations organisations += subject.edition_organisations.map(&:organisation) if subject.respond_to?(:edition_organisations) organisations.exclude?(actor.organisation) + elsif subject.named_users? + subject.named_accesses.none? { |na| na.email.casecmp?(actor.email) } else false end diff --git a/test/factories/named_accesses.rb b/test/factories/named_accesses.rb new file mode 100644 index 00000000000..62cce4d6185 --- /dev/null +++ b/test/factories/named_accesses.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :named_access do + association :edition + email { generate(:email) } + end +end diff --git a/test/functional/admin/edition_access_limited_controller_test.rb b/test/functional/admin/edition_access_limited_controller_test.rb index 2b9b66a6413..85fa2744358 100644 --- a/test/functional/admin/edition_access_limited_controller_test.rb +++ b/test/functional/admin/edition_access_limited_controller_test.rb @@ -27,7 +27,7 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase get :edit, params: { id: edition } assert_select "form[action='#{update_access_limited_admin_edition_path(edition.id)}']" do - assert_select "input[name='edition[access_limited]'][type=checkbox][checked=checked]" + assert_select "input[name='edition[access_limited]'][type=radio][value='organisations'][checked=checked]" assert_select "textarea[name='edition[editorial_remark]']" (1..4).each do |i| @@ -147,6 +147,31 @@ class Admin::EditionAccessLimitedControllerTest < ActionController::TestCase assert edition.reload.access_limited? end + test "PATCH :update updates named_users access and creates an editorial remark" do + edition = create( + :consultation, + access_limited: :disabled, + ) + + editorial_remark = "Limiting to named users." + + put :update, + params: { + id: edition, + edition: { + access_limited: :named_users, + access_limited_named_users: "named@example.com", + editorial_remark:, + }, + } + + assert edition.reload.named_users? + assert_includes edition.named_accesses.pluck(:email), "named@example.com" + assert_redirected_to admin_editions_path + assert_equal "Access updated for #{edition.title}", flash[:notice] + assert_equal "Access options updated by GDS Admin: #{editorial_remark}", edition.editorial_remarks.last.body + end + test "PATCH :update doesn't create an editorial remark or re-render with an error when nothing has changed" do first_organisation = create(:organisation) second_organisation = create(:organisation) diff --git a/test/integration/asset_access_options_integration_test.rb b/test/integration/asset_access_options_integration_test.rb index ba2eff6df07..1297927c98f 100644 --- a/test/integration/asset_access_options_integration_test.rb +++ b/test/integration/asset_access_options_integration_test.rb @@ -34,7 +34,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest context "when document is marked as access limited in Whitehall" do before do visit edit_admin_edition_path(edition) - check "Limit access" + choose "Limit access to publishers from organisations associated with this document" click_button "Save" assert_text "Your document has been saved" end @@ -163,7 +163,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest context "when document is unmarked as access limited in Whitehall" do before do visit edit_admin_edition_path(edition) - uncheck "Limit access" + choose "No – This document should be available to all publishers" click_button "Save" assert_text "Your document has been saved" end diff --git a/test/support/admin_edition_controller_test_helpers.rb b/test/support/admin_edition_controller_test_helpers.rb index 4cf4f5ac2b8..25be383d715 100644 --- a/test/support/admin_edition_controller_test_helpers.rb +++ b/test/support/admin_edition_controller_test_helpers.rb @@ -968,8 +968,7 @@ def should_allow_access_limiting_of(edition_type) get :edit, params: { id: publication } assert_select "form#edit_edition" do - assert_select "input[name='edition[access_limited]'][type=checkbox]" - assert_select "input[name='edition[access_limited]'][type=checkbox][checked=checked]", count: 0 + assert_select "input[name='edition[access_limited]'][type=radio][value='disabled'][checked=checked]" end end diff --git a/test/unit/app/models/admin/edition_filter_test.rb b/test/unit/app/models/admin/edition_filter_test.rb index d4d152102c6..a3d5a683283 100644 --- a/test/unit/app/models/admin/edition_filter_test.rb +++ b/test/unit/app/models/admin/edition_filter_test.rb @@ -31,6 +31,27 @@ class Admin::EditionFilterTest < ActiveSupport::TestCase assert_equal edition, filter.editions.first end + test "should return named_users access limited editions when the user is in the access list" do + user = create(:user) + edition = create(:publication) + edition.access_limited = :named_users + edition.named_accesses.create!(email: user.email) + edition.save! + + filter = Admin::EditionFilter.new(Edition, user) + assert_equal edition, filter.editions.first + end + + test "should not return named_users access limited editions when the user is not in the access list" do + edition = create(:publication) + edition.access_limited = :named_users + edition.named_accesses.create!(email: "someone@else.com") + edition.save! + + filter = Admin::EditionFilter.new(Edition, build(:user)) + assert_equal 0, filter.editions.count + end + test "can preload unpublishing data if asked to" do publication = create(:publication) create(:unpublishing, edition: publication) diff --git a/test/unit/app/models/edition/limited_access_test.rb b/test/unit/app/models/edition/limited_access_test.rb index f1b7361cc81..cb4c7de2438 100644 --- a/test/unit/app/models/edition/limited_access_test.rb +++ b/test/unit/app/models/edition/limited_access_test.rb @@ -61,4 +61,91 @@ def self.access_limited_by_default? assert edition.accessible_to?(user) end + + test "access_limited_named_users= stores named emails in named_accesses after save" do + edition = create(:limited_access_edition) + edition.access_limited = :named_users + edition.access_limited_named_users = "named@example.com" + edition.save! + + assert_includes edition.named_accesses.pluck(:email), "named@example.com" + end + + test "access_limited_named_users= parses multiple emails separated by newlines" do + edition = create(:limited_access_edition) + edition.access_limited = :named_users + edition.access_limited_named_users = "a@example.com\nb@example.com" + edition.save! + + emails = edition.named_accesses.pluck(:email) + assert_includes emails, "a@example.com" + assert_includes emails, "b@example.com" + end + + test "access_limited_named_users= always preserves creator email" do + creator = create(:user) + edition = create(:limited_access_edition, creator:) + edition.access_limited = :named_users + edition.access_limited_named_users = "other@example.com" + edition.save! + + emails = edition.named_accesses.pluck(:email) + assert_includes emails, creator.email.downcase + end + + test "access_limited_named_users= removes emails no longer in the list" do + edition = create(:limited_access_edition) + edition.update_column(:access_limited, Edition.access_limiteds[:named_users]) + edition.reload + edition.named_accesses.create!(email: "old@example.com") + edition.named_accesses.create!(email: "keep@example.com") + + edition.access_limited_named_users = "keep@example.com" + edition.save! + + assert_includes edition.named_accesses.pluck(:email), "keep@example.com" + assert_not_includes edition.named_accesses.pluck(:email), "old@example.com" + end + + test "switching from named_users to disabled clears named_accesses" do + edition = create(:limited_access_edition) + edition.update_column(:access_limited, Edition.access_limiteds[:named_users]) + edition.reload + edition.named_accesses.create!(email: "user@example.com") + + edition.access_limited = :disabled + edition.access_limited_named_users = "" + edition.save! + + assert edition.named_accesses.reload.empty? + end + + test "validates at least one email when named_users setter is called with empty value" do + edition = build(:limited_access_edition, access_limited: :named_users) + edition.access_limited_named_users = "" + + assert_not edition.valid? + assert_includes edition.errors[:access_limited_named_users], "must include at least one email address" + end + + test "access_limited_named_users reader returns existing emails joined by comma" do + edition = create(:limited_access_edition) + edition.update_column(:access_limited, Edition.access_limiteds[:named_users]) + edition.reload + edition.named_accesses.create!(email: "a@example.com") + edition.named_accesses.create!(email: "b@example.com") + + assert_equal "a@example.com, b@example.com", edition.access_limited_named_users + end + + test "create_draft copies named_accesses to new draft" do + edition = create(:limited_access_edition) + edition.update_columns(access_limited: Edition.access_limiteds[:named_users], state: "published") + edition.reload + edition.named_accesses.create!(email: "user@example.com") + + draft = edition.create_draft(create(:user)) + + assert_includes draft.named_accesses.pluck(:email), "user@example.com" + end end diff --git a/test/unit/app/services/draft_edition_updater_test.rb b/test/unit/app/services/draft_edition_updater_test.rb index d6468bf2922..ee8b127f2c6 100644 --- a/test/unit/app/services/draft_edition_updater_test.rb +++ b/test/unit/app/services/draft_edition_updater_test.rb @@ -38,6 +38,20 @@ class DraftEditionUpdaterTest < ActiveSupport::TestCase updater.perform! end + test "can perform if edition uses named_users access limiting (skips org membership check)" do + organisation = create(:organisation) + user = create(:user, organisation:) + other_organisation = create(:organisation) + edition = create(:draft_publication, access_limited: :disabled, organisations: [other_organisation]) + edition.update!(access_limited: :named_users, access_limited_named_users: user.email) + + updater = DraftEditionUpdater.new(edition, { current_user: user }) + updater.expects(:update_publishing_api!).once + updater.expects(:notify!).once + + updater.perform! + end + test "updates editions that cannot be tagged to organisations" do organisation = create(:organisation) edition = create(:draft_corporate_information_page, organisation:, access_limited: :organisations) diff --git a/test/unit/lib/whitehall/authority/department_writer_test.rb b/test/unit/lib/whitehall/authority/department_writer_test.rb index 14beabe4437..64c48eae3c0 100644 --- a/test/unit/lib/whitehall/authority/department_writer_test.rb +++ b/test/unit/lib/whitehall/authority/department_writer_test.rb @@ -56,10 +56,40 @@ def department_writer(id = 1) end end + test "can see a named_users access-limited edition if their email is in named_accesses" do + user = create(:writer) + edition = FactoryBot.build(:publication, access_limited: :named_users) + edition.stubs(:named_accesses).returns( + [FactoryBot.build(:named_access, email: user.email)], + ) + + assert enforcer_for(user, edition).can?(:see) + end + + test "cannot see a named_users access-limited edition if their email is not in named_accesses" do + user = create(:writer) + other_user = create(:writer) + edition = FactoryBot.build(:publication, access_limited: :named_users) + edition.stubs(:named_accesses).returns( + [FactoryBot.build(:named_access, email: other_user.email)], + ) + + assert_not enforcer_for(user, edition).can?(:see) + end + test "can create a new edition of a document that is not access limited" do assert enforcer_for(department_writer, normal_edition).can?(:create) end + test "can create a new unsaved edition when access_limited is named_users and creator is in the list" do + user = create(:writer) + edition = FactoryBot.build(:publication, access_limited: :named_users) + edition.access_limited_named_users = user.email + edition.stubs(:creator).returns(user) + + assert enforcer_for(user, edition).can?(:create) + end + test "can make changes to an edition that is not access limited" do assert enforcer_for(department_writer, normal_edition).can?(:update) end