From 92ddb1a5f0e94c4eb841c8692b2cdf18184b24ca Mon Sep 17 00:00:00 2001 From: Alexa Grey Date: Mon, 15 Jun 2026 11:46:11 -0700 Subject: [PATCH 1/6] feat: allow submissions to be destroyed Resolves TASK-674 --- app/graphql/mutations/submission_destroy.rb | 18 +++ app/graphql/types/mutation_type.rb | 18 +-- .../mutations/contracts/submission_destroy.rb | 17 +++ .../operations/submission_destroy.rb | 20 ++++ app/policies/submission_policy.rb | 5 +- config/locales/en.yml | 1 + .../mutations/submission_destroy_spec.rb | 105 ++++++++++++++++++ 7 files changed, 173 insertions(+), 11 deletions(-) create mode 100644 app/graphql/mutations/submission_destroy.rb create mode 100644 app/operations/mutations/contracts/submission_destroy.rb create mode 100644 app/operations/mutations/operations/submission_destroy.rb create mode 100644 spec/requests/graphql/mutations/submission_destroy_spec.rb diff --git a/app/graphql/mutations/submission_destroy.rb b/app/graphql/mutations/submission_destroy.rb new file mode 100644 index 00000000..2daec482 --- /dev/null +++ b/app/graphql/mutations/submission_destroy.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Mutations + # @see Mutations::Operations::SubmissionDestroy + class SubmissionDestroy < Mutations::BaseMutation + description <<~TEXT + Destroy a single `Submission` record. + TEXT + + argument :submission_id, ID, loads: Types::SubmissionType, required: true do + description <<~TEXT + The submission to destroy. + TEXT + end + + performs_operation! "mutations.operations.submission_destroy", destroy: true + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 9236980f..b82834cc 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -10,6 +10,14 @@ class MutationType < Types::BaseObject field :apply_schema_properties, mutation: Mutations::ApplySchemaProperties + field :contributor_claim, mutation: Mutations::ContributorClaim + + field :contributor_merge, mutation: Mutations::ContributorMerge + + field :contributor_user_link_destroy, mutation: Mutations::ContributorUserLinkDestroy + + field :contributor_user_link_upsert, mutation: Mutations::ContributorUserLinkUpsert + field :controlled_vocabulary_destroy, mutation: Mutations::ControlledVocabularyDestroy field :controlled_vocabulary_source_update, mutation: Mutations::ControlledVocabularySourceUpdate @@ -128,6 +136,8 @@ class MutationType < Types::BaseObject field :submission_create, mutation: Mutations::SubmissionCreate + field :submission_destroy, mutation: Mutations::SubmissionDestroy + field :submission_leave_review, mutation: Mutations::SubmissionLeaveReview field :submission_publish, mutation: Mutations::SubmissionPublish @@ -177,13 +187,5 @@ class MutationType < Types::BaseObject field :upsert_contribution, mutation: Mutations::UpsertContribution field :user_reset_password, mutation: Mutations::UserResetPassword - - field :contributor_user_link_destroy, mutation: Mutations::ContributorUserLinkDestroy - - field :contributor_user_link_upsert, mutation: Mutations::ContributorUserLinkUpsert - - field :contributor_claim, mutation: Mutations::ContributorClaim - - field :contributor_merge, mutation: Mutations::ContributorMerge end end diff --git a/app/operations/mutations/contracts/submission_destroy.rb b/app/operations/mutations/contracts/submission_destroy.rb new file mode 100644 index 00000000..c3c61ce3 --- /dev/null +++ b/app/operations/mutations/contracts/submission_destroy.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Mutations + module Contracts + # @see Mutations::SubmissionDestroy + # @see Mutations::Operations::SubmissionDestroy + class SubmissionDestroy < MutationOperations::Contract + json do + required(:submission).value(:submission) + end + + rule(:submission) do + base.failure(:only_draft_submissions_destructible) unless value.draft? + end + end + end +end diff --git a/app/operations/mutations/operations/submission_destroy.rb b/app/operations/mutations/operations/submission_destroy.rb new file mode 100644 index 00000000..162ae28e --- /dev/null +++ b/app/operations/mutations/operations/submission_destroy.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Mutations + module Operations + # @see Mutations::SubmissionDestroy + class SubmissionDestroy + include MutationOperations::Base + + use_contract! :submission_destroy + + authorizes! :submission, with: :destroy? + + # @param [Submission] submission + # @return [void] + def call(submission:) + destroy_model! submission, auth: true + end + end + end +end diff --git a/app/policies/submission_policy.rb b/app/policies/submission_policy.rb index 59899e30..eb296437 100644 --- a/app/policies/submission_policy.rb +++ b/app/policies/submission_policy.rb @@ -4,7 +4,7 @@ class SubmissionPolicy < ApplicationPolicy pre_check :deny_anonymous! - pre_check :allow_any_admin!, except: :destroy? + pre_check :allow_any_admin! def read? = manage_target? || deposit? || review? || record_owned_by_current_user? @@ -29,8 +29,7 @@ def create? = deposit? && has_accepted_agreement? def update? = record_owned_by_current_user? - # Submissions cannot be destroyed, only rejected. - def destroy? = false + def destroy? = record_owned_by_current_user? private diff --git a/config/locales/en.yml b/config/locales/en.yml index e20cade1..23607206 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -98,6 +98,7 @@ en: contributor_merge_target_merging: "The target contributor is already being merged into another contributor." depositor_agreement_required: "You must accept the depositor agreement to submit." not_yet_implemented: "This mutation is not yet implemented." + only_draft_submissions_destructible: "Only draft submissions can be deleted." revalidation_connection_failed: "The revalidation request failed to connect." revalidation_request_failed: "The revalidation request failed." revalidation_secret_invalid: "The revalidation request failed to authenticate." diff --git a/spec/requests/graphql/mutations/submission_destroy_spec.rb b/spec/requests/graphql/mutations/submission_destroy_spec.rb new file mode 100644 index 00000000..af9eaba5 --- /dev/null +++ b/spec/requests/graphql/mutations/submission_destroy_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +RSpec.describe Mutations::SubmissionDestroy, type: :request, graphql: :mutation do + mutation_query! <<~GRAPHQL + mutation SubmissionDestroy($input: SubmissionDestroyInput!) { + submissionDestroy(input: $input) { + destroyed + destroyedId + ... ErrorFragment + } + } + GRAPHQL + + let_it_be(:existing_submission_attrs) do + {} + end + + let_it_be(:existing_submission, refind: true) { FactoryBot.create(:submission, **existing_submission_attrs) } + + let_mutation_input!(:submission_id) { existing_submission.to_encoded_id } + + let(:valid_mutation_shape) do + gql.mutation(:submission_destroy) do |m| + m[:destroyed] = true + m[:destroyed_id] = be_an_encoded_id.of_a_deleted_model + end + end + + let(:empty_mutation_shape) do + gql.empty_mutation :submission_destroy + end + + let(:only_drafts_error_shape) do + gql.mutation(:submission_destroy, no_only_drafts_errors: false) do |m| + m[:destroyed] = nil + m[:destroyed_id] = nil + + m.global_errors do |ge| + ge.error :only_draft_submissions_destructible + end + end + end + + shared_examples_for "a successful mutation" do + let(:expected_shape) { valid_mutation_shape } + + it "destroys the submission" do + expect_request! do |req| + req.effect! change(Submission, :count).by(-1) + + req.data! expected_shape + end + end + + context "when the submission is not a draft" do + before do + existing_submission.transition_to! :submitted + end + + it "fails to destroy the submission" do + expect_request! do |req| + req.effect! keep_the_same(Submission, :count) + + req.data! only_drafts_error_shape + end + end + end + end + + shared_examples_for "an unauthorized mutation" do + let(:expected_shape) { empty_mutation_shape } + + it "is not authorized" do + expect_request! do |req| + req.effect! keep_the_same(Submission, :count) + + req.unauthorized! + + req.data! expected_shape + end + end + end + + shared_examples_for "an authorized mutation" do + include_examples "a successful mutation" + end + + as_an_admin_user do + include_examples "an authorized mutation" + end + + context "as the owner of the submission" do + let(:current_user) { existing_submission.user } + + include_examples "an authorized mutation" + end + + as_a_regular_user do + include_examples "an unauthorized mutation" + end + + as_an_anonymous_user do + include_examples "an unauthorized mutation" + end +end From 96c4bbdc1fce1b2a19b7a16da8add65bcd732e7c Mon Sep 17 00:00:00 2001 From: Alexa Grey Date: Fri, 26 Jun 2026 19:11:05 -0700 Subject: [PATCH 2/6] refactor: audit policies & add improved test coverage * Resolves TASK-746 * Adds canPreview permission to users --- .rubocop.yml | 1 + .../types/access_grant_subject_type.rb | 1 + app/graphql/types/user_type.rb | 6 +- app/models/access_grant.rb | 12 +- app/models/application_record.rb | 1 + app/models/concerns/access_grant_subject.rb | 45 +- app/models/primary_role_assignment.rb | 2 + app/models/role.rb | 24 ++ app/operations/access/grant.rb | 2 +- app/operations/access/polymorphic_grant.rb | 8 + app/policies/application_policy.rb | 85 +++- app/policies/contributor_user_link_policy.rb | 4 +- app/policies/user_policy.rb | 24 +- app/services/access/accessible_error.rb | 6 + app/services/access/checking.rb | 45 ++ app/services/access/error.rb | 7 + app/services/access/polymorphic_granter.rb | 60 +++ app/services/access/role_applicator.rb | 160 +++++++ app/services/access/subject_error.rb | 6 + app/services/access/types.rb | 44 +- app/services/roles/types.rb | 3 + app/services/utility/types.rb | 3 +- config/application.rb | 2 +- ...e_primary_role_assignments_to_version_2.rb | 5 + db/structure.sql | 5 +- db/views/primary_role_assignments_v02.sql | 7 + lib/support/lib/classy_list/configuration.rb | 51 +++ lib/support/lib/classy_list/dsl.rb | 35 ++ lib/support/lib/classy_list/error.rb | 8 + lib/support/lib/classy_list/implementation.rb | 51 +++ .../classy_list/instance_implementation.rb | 32 ++ .../lib/classy_list/klass_implementation.rb | 58 +++ lib/support/lib/classy_list/list.rb | 77 ++++ lib/support/lib/classy_list/types.rb | 21 + lib/support/lib/inspectable.rb | 10 + lib/support/lib/inspecting/types.rb | 41 ++ lib/support/lib/inspector.rb | 109 +++++ lib/support/lib/message_maps/configuration.rb | 41 ++ lib/support/lib/message_maps/dsl.rb | 25 ++ .../lib/message_maps/implementation.rb | 51 +++ .../message_maps/instance_implementation.rb | 32 ++ .../lib/message_maps/klass_implementation.rb | 69 +++ lib/support/lib/message_maps/map.rb | 65 +++ lib/support/lib/message_maps/types.rb | 32 ++ lib/support/system.rb | 2 + spec/factories/users.rb | 20 +- spec/models/primary_role_assignment_spec.rb | 64 +++ .../contributor_user_link_policy_spec.rb | 2 +- spec/policies/submission_policy_spec.rb | 177 +------- .../submission_publication_policy_spec.rb | 135 +----- .../policies/submission_target_policy_spec.rb | 199 +-------- spec/policies/user_policy_spec.rb | 202 ++------- .../requests/graphql/query/submission_spec.rb | 2 +- .../graphql/query/submissions_spec.rb | 2 +- spec/requests/graphql/query/user_spec.rb | 14 +- spec/requests/graphql/query/viewer_spec.rb | 44 +- spec/requests/graphql/viewer_spec.rb | 28 -- spec/services/users/access_info_spec.rb | 2 +- .../support/contexts/authorization_testing.rb | 74 ++++ spec/support/contexts/policy_scope_setup.rb | 133 ++++++ spec/support/contexts/policy_setup.rb | 131 +++++- spec/support/matchers/record_matching.rb | 404 ++++++++++++++++++ 62 files changed, 2281 insertions(+), 730 deletions(-) create mode 100644 app/operations/access/polymorphic_grant.rb create mode 100644 app/services/access/accessible_error.rb create mode 100644 app/services/access/checking.rb create mode 100644 app/services/access/error.rb create mode 100644 app/services/access/polymorphic_granter.rb create mode 100644 app/services/access/role_applicator.rb create mode 100644 app/services/access/subject_error.rb create mode 100644 db/migrate/20260603165644_update_primary_role_assignments_to_version_2.rb create mode 100644 db/views/primary_role_assignments_v02.sql create mode 100644 lib/support/lib/classy_list/configuration.rb create mode 100644 lib/support/lib/classy_list/dsl.rb create mode 100644 lib/support/lib/classy_list/error.rb create mode 100644 lib/support/lib/classy_list/implementation.rb create mode 100644 lib/support/lib/classy_list/instance_implementation.rb create mode 100644 lib/support/lib/classy_list/klass_implementation.rb create mode 100644 lib/support/lib/classy_list/list.rb create mode 100644 lib/support/lib/classy_list/types.rb create mode 100644 lib/support/lib/inspectable.rb create mode 100644 lib/support/lib/inspecting/types.rb create mode 100644 lib/support/lib/inspector.rb create mode 100644 lib/support/lib/message_maps/configuration.rb create mode 100644 lib/support/lib/message_maps/dsl.rb create mode 100644 lib/support/lib/message_maps/implementation.rb create mode 100644 lib/support/lib/message_maps/instance_implementation.rb create mode 100644 lib/support/lib/message_maps/klass_implementation.rb create mode 100644 lib/support/lib/message_maps/map.rb create mode 100644 lib/support/lib/message_maps/types.rb create mode 100644 spec/models/primary_role_assignment_spec.rb delete mode 100644 spec/requests/graphql/viewer_spec.rb create mode 100644 spec/support/contexts/authorization_testing.rb create mode 100644 spec/support/contexts/policy_scope_setup.rb create mode 100644 spec/support/matchers/record_matching.rb diff --git a/.rubocop.yml b/.rubocop.yml index 1c32fb8b..47b3993f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -349,6 +349,7 @@ Metrics/PerceivedComplexity: - "app/operations/**/*.rb" - "app/services/**/*.rb" - "lib/support/**/*.rb" + - "spec/**/*.rb" Naming/AccessorMethodName: Enabled: false diff --git a/app/graphql/types/access_grant_subject_type.rb b/app/graphql/types/access_grant_subject_type.rb index a1926329..5a728f36 100644 --- a/app/graphql/types/access_grant_subject_type.rb +++ b/app/graphql/types/access_grant_subject_type.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true module Types + # @see AccessGrantSubject module AccessGrantSubjectType include Types::BaseInterface diff --git a/app/graphql/types/user_type.rb b/app/graphql/types/user_type.rb index 2301fdc7..486c1b0a 100644 --- a/app/graphql/types/user_type.rb +++ b/app/graphql/types/user_type.rb @@ -21,7 +21,7 @@ class UserType < Types::BaseModel description "Has this user's email been verified to work through Keycloak?" end - field :email, String, null: true do + field :email, String, null: true, authorize_field: { to: :read_privileged?, raise: false } do description "A user's email. Depending on the upstream provider, this may not be set." end @@ -111,6 +111,10 @@ class UserType < Types::BaseModel and that they do not already have a contributor profile linked to their account. TEXT + expose_authorization_rule :preview?, <<~TEXT + Whether this user has the ability to preview content in the system. + TEXT + expose_authorization_rule :receive_review_requests?, <<~TEXT Whether this user is a reviewer on **any** submission targets, and should see information about potential review requests in the UI. diff --git a/app/models/access_grant.rb b/app/models/access_grant.rb index eac5bf1a..b61a6121 100644 --- a/app/models/access_grant.rb +++ b/app/models/access_grant.rb @@ -171,13 +171,19 @@ def with_allowed_action?(**options) with_allowed_action(**options).exists? end + # Constrain to access grants that allow _any_ asset creation. + # + # @see AccessGrantSubject#has_granted_asset_creation? + # @see .with_asset_creation? # @return [ActiveRecord::Relation] def with_asset_creation matching_permissions "*.assets.create" end - def with_asset_creation? - with_asset_creation.exists? - end + # Determine if the current scope has _any_ permission to create assets. + # + # @see .with_asset_creation + # @see AccessGrantSubject#has_granted_asset_creation? + def with_asset_creation? = with_asset_creation.exists? end end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index f17f0071..b2be86a0 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -20,6 +20,7 @@ class ApplicationRecord < ActiveRecord::Base include PostgresEnums include StoreModelIntrospection include ::Support::CallsCommonOperation + include ::Support::Inspectable include WhereMatches include WithAdvisoryLock::Concern diff --git a/app/models/concerns/access_grant_subject.rb b/app/models/concerns/access_grant_subject.rb index 83d9fd56..180a75df 100644 --- a/app/models/concerns/access_grant_subject.rb +++ b/app/models/concerns/access_grant_subject.rb @@ -1,9 +1,14 @@ # frozen_string_literal: true # This represents something to which an {AccessGrant} can assign a {Role} -# for a given {Accessible} entity, namely a {User}. +# for a given {Accessible} entity, namely a {User}, but intended to allow +# expansion to {UserGroup} in the future. +# +# @see Access::Types::Subject +# @see Types::AccessGrantSubjectType module AccessGrantSubject extend ActiveSupport::Concern + extend DefinesMonadicOperation include AssociationHelpers @@ -18,16 +23,40 @@ module AccessGrantSubject end # @see Access::AssignGlobalPermissions - # @return [void] - def assign_global_permissions! - call_operation("access.assign_global_permissions", self).value! + # @return [Dry::Monads::Success(void)] + monadic_operation! def assign_global_permissions + call_operation("access.assign_global_permissions", self) end - def enforce_assignments! - call_operation("access.enforce_assignments", subject: self).value! + # @see Access::EnforceAssignments + # @return [Dry::Monads::Success(void)] + monadic_operation! def enforce_assignments + call_operation("access.enforce_assignments", subject: self) end - def has_granted_asset_creation? - access_grants.with_asset_creation? + # @param [Hash] options + # @see Access::PolymorphicGrant + # @see Access::PolymorphicGranter + # @return [Dry::Monads::Success(void)] + monadic_operation! def polymorphic_grant(**options) + call_operation("access.polymorphic_grant", self, **options) end + + # @api private + # @note Used in testing to allow for easy granting of permissions + # @param [FactoryBot::Evaluator] evaluator + # @see #polymorphic_grant + # @return [Dry::Monads::Success(void)] + monadic_operation! def polymorphic_grant_from(evaluator) + options = Role::POLYMORPHIC_GRANTABLE_KEYS.index_with do |key| + evaluator.try(key) || [] + end + + polymorphic_grant(**options) + end + + # @note This is used to determine the upload access permission for non-admins. + # @see AccessGrant#with_asset_creation? + # @see User#has_any_upload_access? + def has_granted_asset_creation? = access_grants.with_asset_creation? end diff --git a/app/models/primary_role_assignment.rb b/app/models/primary_role_assignment.rb index a2ca06ea..67ffb53a 100644 --- a/app/models/primary_role_assignment.rb +++ b/app/models/primary_role_assignment.rb @@ -5,6 +5,8 @@ class PrimaryRoleAssignment < ApplicationRecord include GenericAccessible include View + self.primary_key = %i[subject_type subject_id] + belongs_to :subject, polymorphic: true, inverse_of: :primary_role_assignment belongs_to :role, inverse_of: :primary_role_assignments end diff --git a/app/models/role.rb b/app/models/role.rb index b6926ebb..d11d5048 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -4,6 +4,30 @@ class Role < ApplicationRecord include HasEphemeralSystemSlug include TimestampScopes + # A list of polymorphic system roles that can be granted. + # + # @see Access::PolymorphicGrant + # @see Access::PolymorphicGranter + # @return [] + POLYMORPHIC_GRANTABLES = %i[ + manager + editor + reviewer + depositor + author + reader + ].freeze + + # @see Access::PolymorphicGrant + # @see Access::PolymorphicGranter + # @return [{ Symbol => Symbol }] + POLYMORPHIC_GRANTABLE_MAP = POLYMORPHIC_GRANTABLES.index_with { :"#{_1}_on" }.freeze + + # @see Access::PolymorphicGrant + # @see Access::PolymorphicGranter + # @return [] + POLYMORPHIC_GRANTABLE_KEYS = POLYMORPHIC_GRANTABLE_MAP.values.freeze + pg_enum! :identifier, as: "role_identifier", prefix: :identified_as pg_enum! :kind, as: "role_kind", prefix: :for pg_enum! :primacy, as: "role_primacy", prefix: :has, suffix: :primacy diff --git a/app/operations/access/grant.rb b/app/operations/access/grant.rb index ae57b407..1b540252 100644 --- a/app/operations/access/grant.rb +++ b/app/operations/access/grant.rb @@ -10,7 +10,7 @@ class Grant # @param [Role] role # @param [Accessible] on # @param [AccessGrantSubject] to - # @return [Dry::Monads::Result] + # @return [Dry::Monads::Success(void)] def call(role, on:, to:) return Success(nil) if AccessGrant.has_granted?(role, on:, to:) diff --git a/app/operations/access/polymorphic_grant.rb b/app/operations/access/polymorphic_grant.rb new file mode 100644 index 00000000..637ef8e6 --- /dev/null +++ b/app/operations/access/polymorphic_grant.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Access + # @see Access::PolymorphicGranter + class PolymorphicGrant < Support::SimpleServiceOperation + service_klass Access::PolymorphicGranter + end +end diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index a0d6c6f7..3fe77c5a 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -6,6 +6,24 @@ class ApplicationPolicy < ActionPolicy::Base extend Dry::Core::ClassAttributes + include Support::ClassyList::DSL + + STANDARD_READ_PERMISSIONS = %i[read? show? index?].freeze + + # Feeds into the `except:` option for `allow_any_admin!` pre-checks. + has_simple_symbol_list! :admin_pre_check_exceptions + + # Feeds into the `except:` option for `deny_anonymous!` pre-checks. + has_simple_symbol_list! :anonymous_pre_check_exceptions + + has_simple_symbol_list! :read_permissions + + read_permissions! *STANDARD_READ_PERMISSIONS + + has_simple_symbol_list! :write_permissions + + write_permissions! :create?, :update?, :destroy?, :manage? + defines :always_readable, :authenticated_readable, :readable_in_dev, type: Roles::Types::Bool # @!attribute [r] always_readable @@ -27,7 +45,7 @@ class ApplicationPolicy < ActionPolicy::Base # @return [Boolean] readable_in_dev false - pre_check :allow_public_reading!, only: %i[index? show? read?] + pre_check :allow_public_reading!, only: STANDARD_READ_PERMISSIONS # @param [ApplicationRecord] record # @param [User, AnonymousUser] user @@ -104,6 +122,7 @@ def anonymous? = user.anonymous? def authenticated? = user.authenticated? + # @see .authenticated_readable def authenticated_readable? = self.class.authenticated_readable # @return [GlobalConfiguration] @@ -122,8 +141,6 @@ def has_allowed_action?(action_name) = action_name.to_s.in?(user.allowed_actions # @param [String] action_name def has_admin_or_allowed_action?(action_name) = has_admin? || has_allowed_action?(action_name) - def new_record? = record.try(:new_record?) || false - # Whether the record is readable in development mode. # # @see .readable_in_dev @@ -264,6 +281,24 @@ def resolve_scope_for_anonymous(relation) end class << self + # Declare a pre-check that allows any admin user to pass. + # + # @return [void] + def allows_any_admin!(**opts) + options = pre_check_blacklist(**opts, with_admin: true) + + pre_check :allow_any_admin!, **options + end + + # Declare a pre-check that allows any admin user to pass. + # + # @return [void] + def denies_anonymous!(**opts) + options = pre_check_blacklist(**opts, with_anonymous: true) + + pre_check :deny_anonymous!, **options + end + # Specify that the record is always readable. # # @see .always_readable @@ -287,5 +322,49 @@ def authenticated_readable! def readable_in_dev! readable_in_dev true end + + # @api private + # @param [] only + # @param [Hash] options (@see .pre_check_normalize_list) + # @return [Hash] + def pre_check_whitelist(only: [], **options) + only = pre_check_normalize_list(only, **options) + + { + only:, + } + end + + # @api private + # @param [] except + # @param [Hash] options (@see .pre_check_normalize_list) + # @return [Hash] + def pre_check_blacklist(except: [], **options) + except = pre_check_normalize_list(except, **options) + + { + except:, + } + end + + # @api private + # @param [] base_list base list of predicates to include in the pre-check list + # @param [] extra additional predicates to include + # @param [Boolean] with_read whether to include the standard read permissions + # @param [Boolean] with_write whether to include the standard write permissions + # @param [Boolean] with_anonymous whether to include the anonymous pre-check exceptions + # @param [Boolean] with_admin whether to include the admin pre-check exceptions + # @return [] a list of predicates to use for a pre-check `only`/`except` option + def pre_check_normalize_list(base_list, extra: [], with_read: false, with_write: false, with_anonymous: false, with_admin: false) + list = Array(base_list).flatten.map(&:to_sym) + + list.concat(Array(extra).flatten.map(&:to_sym)) + list.concat(admin_pre_check_exceptions) if with_admin + list.concat(anonymous_pre_check_exceptions) if with_anonymous + list.concat(read_permissions) if with_read + list.concat(write_permissions) if with_write + + list.uniq + end end end diff --git a/app/policies/contributor_user_link_policy.rb b/app/policies/contributor_user_link_policy.rb index 4b8b31a2..5f4b84d7 100644 --- a/app/policies/contributor_user_link_policy.rb +++ b/app/policies/contributor_user_link_policy.rb @@ -4,9 +4,9 @@ class ContributorUserLinkPolicy < ApplicationPolicy pre_check :allow_any_admin!, except: %i[create? update?] - def read? = update_contributor? || read_user? + def read? = authenticated? - def show? = read? + def show? = authenticated? def create? = false diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index f424b566..647bbaf0 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -3,19 +3,31 @@ # Presently, user creation and destruction is managed in Keycloak # and cannot be handled directly in Meru. Permissions reflect this. class UserPolicy < ApplicationPolicy - pre_check :allow_any_admin!, except: %i[create? destroy? receive_review_requests? revalidate_instance? claim_contributor?] - pre_check :deny_anonymous!, only: %i[update? receive_review_requests? reset_password? revalidate_instance? claim_contributor?] - pre_check :allow_authenticated_self_action!, only: %i[read? show? update? reset_password?] + admin_pre_check_exceptions! :access_admin?, :preview?, :create?, :destroy?, :receive_review_requests?, :revalidate_instance?, :claim_contributor? + + allows_any_admin! + + pre_check :deny_anonymous!, only: %i[access_admin? preview? read_privileged? update? receive_review_requests? reset_password? revalidate_instance? claim_contributor?] + + pre_check :allow_authenticated_self_action!, only: %i[read? show? read_privileged? update? reset_password?] + pre_check :allow_reviewers!, only: %i[read? show?] - def read? = record.anonymous? || has_allowed_action?("users.read") || has_any_access_management_permissions? + def read? = authenticated? + + def read_privileged? = has_allowed_action?("users.read") def update? = has_allowed_action?("users.update") - def access_admin? = has_allowed_action?("admin.access") && record.has_allowed_action?("admin.access") + def access_admin? = record.has_allowed_action?("admin.access") + + # Really just {#access_admin?} for now, but exposed as an explicit permission for future expansion. + def preview? = record.has_allowed_action?("admin.access") def claim_contributor? = record.authenticated? && record.may_claim_author? + def manage_access? = has_any_access_management_permissions? + def receive_review_requests? = admin_record? || record_is_reviewer? def reset_password? = has_allowed_action?("users.update") @@ -43,7 +55,7 @@ def authenticated_self_action? = record == user def record_is_reviewer? = record.authenticated? && record.submission_target_reviewers.exists? def resolve_scope_for_authenticated(relation) - if has_allowed_action?("users.read") + if has_allowed_action?("users.read") || has_any_access_management_permissions? relation.all else relation.where(id: user_id) diff --git a/app/services/access/accessible_error.rb b/app/services/access/accessible_error.rb new file mode 100644 index 00000000..9f9dbf0b --- /dev/null +++ b/app/services/access/accessible_error.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +module Access + # An error raised when trying to process an invalid {Accessible}. + class AccessibleError < Error; end +end diff --git a/app/services/access/checking.rb b/app/services/access/checking.rb new file mode 100644 index 00000000..3056befb --- /dev/null +++ b/app/services/access/checking.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Access + module Checking + extend ActiveSupport::Concern + + # @param [Accessible] accessible + # @raise [Access::AccessibleError] if the accessible is not a {HierarchicalEntity}. + def require_entity!(accessible) + case accessible + in Types::Entity => entity + return entity + else + # simplecov:disable + raise AccessibleError, "Expected a HierarchicalEntity, got #{accessible.class}" + # simplecov:enable + end + end + + def require_submittable!(accessible) + entity = require_entity!(accessible) + + case entity + in ::Submittable + return entity + else + # simplecov:disable + raise AccessibleError, "Expected a Submittable entity, got #{entity.class}" + # simplecov:enable + end + end + + # @param [AccessGrantSubject] subject + # @raise [Access::SubjectError] if the subject is not a {User}. + # @return [User] + def require_user!(subject) + case subject + in Types::User => user + return user + else + raise UserOnlyError, "Expected a User, got #{subject.class}" + end + end + end +end diff --git a/app/services/access/error.rb b/app/services/access/error.rb new file mode 100644 index 00000000..10c2e182 --- /dev/null +++ b/app/services/access/error.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Access + # @see Access::Checking + # @abstract Errors raised by {Access} operations. + class Error < StandardError; end +end diff --git a/app/services/access/polymorphic_granter.rb b/app/services/access/polymorphic_granter.rb new file mode 100644 index 00000000..ec50ca39 --- /dev/null +++ b/app/services/access/polymorphic_granter.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Access + # @see Access::PolymorphicGrant + class PolymorphicGranter < Support::HookBased::Actor + include Dry::Initializer[undefined: false].define -> do + param :subject, Access::Types::Subject + + option :manager_on, Access::Types::Accessibles, default: proc { Dry::Core::Constants::EMPTY_ARRAY } + option :editor_on, Access::Types::Accessibles, default: proc { Dry::Core::Constants::EMPTY_ARRAY } + option :reviewer_on, Access::Types::Accessibles, default: proc { Dry::Core::Constants::EMPTY_ARRAY } + option :depositor_on, Access::Types::Accessibles, default: proc { Dry::Core::Constants::EMPTY_ARRAY } + option :author_on, Access::Types::Accessibles, default: proc { Dry::Core::Constants::EMPTY_ARRAY } + option :reader_on, Access::Types::Accessibles, default: proc { Dry::Core::Constants::EMPTY_ARRAY } + end + + standard_execution! + + # @return [] + attr_reader :role_applicators + + # @return [Dry::Monads::Result] + def call + run_callbacks :execute do + yield prepare! + + yield apply_each! + end + + subject.reload + + Success() + end + + wrapped_hook! def prepare + @role_applicators = build_role_applicators + + super + end + + wrapped_hook! def apply_each + role_applicators.each do |applicator| + applicator.apply!(subject) + end + + super + end + + private + + # @return [] + def build_role_applicators + Role::POLYMORPHIC_GRANTABLE_MAP.map do |role, accessibles_key| + accessibles = __send__(accessibles_key) + + Access::RoleApplicator.new(role:, accessibles:) + end + end + end +end diff --git a/app/services/access/role_applicator.rb b/app/services/access/role_applicator.rb new file mode 100644 index 00000000..63d2b839 --- /dev/null +++ b/app/services/access/role_applicator.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +module Access + # A simple applicator pattern that takes a role, multiple accessible targets, + # and can {#apply} that role to any number of subjects by its callers. + # + # @see Access::Grant + # @see Access::PolymorphicGrant + # @see Access::PolymorphicGranter + class RoleApplicator + extend DefinesMonadicOperation + extend Dry::Core::Cache + + include Access::Checking + include Dry::Monads[:result] + include Enumerable + include Support::CallsCommonOperation + + include Dry::Initializer[undefined: false].define -> do + option :role, Access::Types::RoleInput, as: :role_input + + option :accessibles, Access::Types::Accessibles, default: proc { Dry::Core::Constants::EMPTY_ARRAY } + end + + delegate :empty?, to: :accessibles + + # The mode to use for role assignment. + # + # `:reviewer` roles require a little extra handling. + # + # @see #reviewer? + # @see #simple? + # @return [:simple, :reviewer] + attr_reader :mode + + # @return [Role] + attr_reader :role + + delegate :identified_as_reviewer?, to: :role + + def initialize(...) + super + + @role = derive_role + @mode = identified_as_reviewer? ? :reviewer : :simple + end + + # Creating a reviewer is a little different and requires creating + # a {SubmissionTargetReviewer} join record instead. It will handle + # the {Role} assignment. + # + # @see #mode + def reviewer? = mode == :reviewer + + # For most roles, we can just rely on {Access::Grant} to handle everything. + # + # @see #mode + def simple? = mode == :simple + + def each + # simplecov:disable + return enum_for(:each) unless block_given? + # simplecov:enable + + accessibles.each do |accessible| + yield accessible + end + end + + def each_entity + # simplecov:disable + return enum_for(:each_entity) unless block_given? + # simplecov:enable + + each do |accessible| + entity = require_entity!(accessible) + + yield entity + end + end + + # @raise [Access::Error] if any of the role applications failed + # @param [AccessGrantSubject] subject + # @return [Dry::Monads::Success(Integer)] + monadic_operation! def apply(subject) + return Success(0) if empty? + + if reviewer? + apply_reviewers(subject) + else + apply_simple(subject) + end + end + + private + + # @raise [Access::Error] if any of the role applications failed + # @param [AccessGrantSubject] subject + # @return [Dry::Monads::Success(Integer)] + def apply_reviewers(subject) + user = require_user!(subject) + + each_entity do |entity| + assign_reviewer!(user, entity) + end + + Success(accessibles.size) + end + + # @param [AccessGrantSubject] subject + # @return [Dry::Monads::Success(Integer)] + def apply_simple(subject) + each do |accessible| + assign_simple!(accessible, subject) + end + + Success(accessibles.size) + end + + # @param [User] user + # @param [Submittable] entity + # @return [Dry::Monads::Success(void)] + monadic_operation! def assign_reviewer(user, entity) + submission_target = entity.fetch_submission_target! + + SubmissionTargetReviewer.where(submission_target:, user:).first_or_create! + + Success() + end + + # @param [Accessible] on + # @param [AccessGrantSubject] to + # @return [Dry::Monads::Success(void)] + monadic_operation! def assign_simple(on, to) + call_operation("access.grant", role, on:, to:) + end + + # @return [Role] + def derive_role + case role_input + in Types::RoleIdentifier => identifier + fetch_role(identifier) + else + role_input + end + end + + # We can rely upon a fairly static cache of {Role} records + # here, since system roles are not going to be changing much + # at all in runtime. + # + # @param [Symbol] identifier + # @return [Role] + def fetch_role(identifier) + fetch_or_store(identifier) do + Role.fetch(identifier) + end + end + end +end diff --git a/app/services/access/subject_error.rb b/app/services/access/subject_error.rb new file mode 100644 index 00000000..de302a0b --- /dev/null +++ b/app/services/access/subject_error.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +module Access + # An error raised when trying to process an invalid {AccessGrantSubject}. + class SubjectError < Error; end +end diff --git a/app/services/access/types.rb b/app/services/access/types.rb index 70a7be83..6c6bf562 100644 --- a/app/services/access/types.rb +++ b/app/services/access/types.rb @@ -8,13 +8,53 @@ module Types # @see AccessGrant AccessGrant = ModelInstance("AccessGrant") + # A type matching something needs access to be granted to it. + # In effect, an {Entity}. + # + # @see ::Accessible + # @return [Dry::Types::Type] + Accessible = Instance(::Accessible) + + # A list of {Accessible}s. + # + # @return [Dry::Types::Type] + Accessibles = Coercible::Array.of(Accessible) + # @see HierarchicalEntity Entity = Instance(::HierarchicalEntity) + # Multiple entities, as an array. + # + # @return [Dry::Types::Type] + Entities = Coercible::Array.of(Entity) + # @see Role Role = ModelInstance("Role") - # @see User - AuthenticatedUser = ModelInstance("User") + # A symbolic identifier for a role. + # + # @see Roles::Types::Identifier + # @return [Dry::Types::Type] + RoleIdentifier = Roles::Types::Identifier + + # A role or a symbolic identifier for a role. + # + # @see Role + # @see RoleIdentifier + # @return [Dry::Types::Type] + RoleInput = Role | RoleIdentifier + + # A type matching an identity that can be allowed to access {Accessible}s. + # Presently, this is only {User}, but we want to allow for expansion to {UserGroup}. + # + # @see AccessGrantSubject + # @see ::Types::AccessGrantSubjectType + # @return [Dry::Types::Type] + Subject = Instance(::AccessGrantSubject) + + # A type matching a {User}. + # + # @return [Dry::Types::Type] + User = ModelInstance("User") end end diff --git a/app/services/roles/types.rb b/app/services/roles/types.rb index 7960b499..24874a2a 100644 --- a/app/services/roles/types.rb +++ b/app/services/roles/types.rb @@ -42,8 +42,11 @@ module Types end end + Identifier = ApplicationRecord.dry_pg_enum(:role_identifier, symbolize: true) + PolicyPredicate = Symbol + # @deprecated should replace with {Identifier} RoleIdentifier = Coercible::Symbol RoleOptions = Hash.schema(name?: String) diff --git a/app/services/utility/types.rb b/app/services/utility/types.rb index de24f42a..27f90448 100644 --- a/app/services/utility/types.rb +++ b/app/services/utility/types.rb @@ -2,9 +2,10 @@ module Utility module Types - include Dry::Core::Constants extend ::Support::Typespace + include Dry::Core::Constants + Callable = Interface(:to_proc) MethodName = Coercible::Symbol.constrained(format: /\A[a-z][a-z0-9_]+\z/i) diff --git a/config/application.rb b/config/application.rb index b9ddafd8..c6df40d0 100644 --- a/config/application.rb +++ b/config/application.rb @@ -42,7 +42,7 @@ class Application < Rails::Application # config.anyway_config.autoload_static_config_path = "config/configs" # # Initialize configuration defaults for originally generated Rails version. - config.load_defaults 7.0 + config.load_defaults 8.1 config.active_support.isolation_level = :fiber diff --git a/db/migrate/20260603165644_update_primary_role_assignments_to_version_2.rb b/db/migrate/20260603165644_update_primary_role_assignments_to_version_2.rb new file mode 100644 index 00000000..4782866a --- /dev/null +++ b/db/migrate/20260603165644_update_primary_role_assignments_to_version_2.rb @@ -0,0 +1,5 @@ +class UpdatePrimaryRoleAssignmentsToVersion2 < ActiveRecord::Migration[8.1] + def change + update_view :primary_role_assignments, version: 2, revert_to_version: 1 + end +end diff --git a/db/structure.sql b/db/structure.sql index 09895cbb..11115ec2 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -6897,7 +6897,7 @@ CREATE TABLE public.pghero_space_stats ( -- CREATE VIEW public.primary_role_assignments AS - SELECT DISTINCT ON (ag.subject_id, ag.subject_type, ag.role_id) ag.subject_id, + SELECT DISTINCT ON (ag.subject_id, ag.subject_type) ag.subject_id, ag.subject_type, ag.role_id FROM (( SELECT DISTINCT access_grants.subject_id, @@ -6905,7 +6905,7 @@ CREATE VIEW public.primary_role_assignments AS access_grants.role_id FROM public.access_grants) ag JOIN public.roles r ON ((r.id = ag.role_id))) - ORDER BY ag.subject_id, ag.subject_type, ag.role_id, r.primacy, r.priority DESC, r.kind; + ORDER BY ag.subject_id, ag.subject_type, r.primacy, r.priority DESC, r.kind; -- @@ -16874,6 +16874,7 @@ ALTER TABLE ONLY public.templates_ordering_instances SET search_path TO "$user", public; INSERT INTO "schema_migrations" (version) VALUES +('20260603165644'), ('20260509003048'), ('20260509002726'), ('20260509002542'), diff --git a/db/views/primary_role_assignments_v02.sql b/db/views/primary_role_assignments_v02.sql new file mode 100644 index 00000000..66038c3d --- /dev/null +++ b/db/views/primary_role_assignments_v02.sql @@ -0,0 +1,7 @@ +SELECT DISTINCT ON (ag.subject_id, ag.subject_type) + ag.subject_id, + ag.subject_type, + ag.role_id + FROM (SELECT DISTINCT subject_id, subject_type, role_id FROM access_grants) ag + INNER JOIN roles r ON r.id = ag.role_id + ORDER BY 1, 2, r.primacy, r.priority DESC, r.kind diff --git a/lib/support/lib/classy_list/configuration.rb b/lib/support/lib/classy_list/configuration.rb new file mode 100644 index 00000000..1b2d83dc --- /dev/null +++ b/lib/support/lib/classy_list/configuration.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Support + module ClassyList + class Configuration + include Support::Typing + include Dry::Initializer[undefined: false].define -> do + param :list_name, Types::MethodName + + option :item_type, Support::Types::DryType, default: -> { Types::Any } + + option :list_type, Support::Types::DryType, default: -> { Types::Array.of(item_type) } + + option :dsl_base, Types::MethodName, default: -> { list_name } + + option :single_dsl_method, Types::MethodName, default: -> { :"#{dsl_base.to_s.singularize}!" } + option :plural_dsl_method, Types::MethodName, default: -> { :"#{dsl_base.to_s.pluralize}!" } + option :realize_mode, Types::RealizeMode, default: -> { :none } + option :realize_method, Types::MethodName, default: -> { :"realize_#{list_name}" } + end + + # @return [Symbol] + attr_reader :config_name + + # @return [Support::ClassyList::InstanceImplementation] + attr_reader :instance_implementation + + # @return [Symbol] + attr_reader :ivar + + # @return [Support::ClassyList::KlassImplementation] + attr_reader :klass_implementation + + def initialize(...) + super + + @config_name = :"#{list_name}_config" + + @ivar = :"@#{list_name}" + + @klass_implementation = KlassImplementation.new(self) + @instance_implementation = @klass_implementation.instance_implementation + end + + # @return [Support::ClassyList::List] + def build_list = ::Support::ClassyList::List.new(config: self) + + def realize? = realize_mode != :none + end + end +end diff --git a/lib/support/lib/classy_list/dsl.rb b/lib/support/lib/classy_list/dsl.rb new file mode 100644 index 00000000..6aa9b05e --- /dev/null +++ b/lib/support/lib/classy_list/dsl.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Support + module ClassyList + module DSL + extend ActiveSupport::Concern + + module ClassMethods + # @param [Symbol] name The name of the message map to define. + # @param [Dry::Types::Type] item_type The base name for the DSL methods to define. + # @return [void] + def has_classy_list!(name, item_type, list_type: Types::Array.of(item_type), **options) + config = Support::ClassyList::Configuration.new(name, **options, item_type:, list_type:) + + extend config.klass_implementation + end + + # @return [void] + def has_simple_message_map!(name, item_type, **options) + has_classy_list!(name, item_type, **options, realize_mode: :hash) + end + + # @return [void] + def has_simple_message_list!(name, item_type, **options) + has_classy_list!(name, item_type, **options, realize_mode: :array) + end + + # @return [void] + def has_simple_symbol_list!(name, **options) + has_classy_list!(name, Types::Symbol, **options, realize_mode: :none) + end + end + end + end +end diff --git a/lib/support/lib/classy_list/error.rb b/lib/support/lib/classy_list/error.rb new file mode 100644 index 00000000..84ee29ed --- /dev/null +++ b/lib/support/lib/classy_list/error.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Support + module ClassyList + # @abstract + class Error < StandardError; end + end +end diff --git a/lib/support/lib/classy_list/implementation.rb b/lib/support/lib/classy_list/implementation.rb new file mode 100644 index 00000000..c94183a2 --- /dev/null +++ b/lib/support/lib/classy_list/implementation.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Support + module ClassyList + # @api private + # @abstract A base implementation metamodule for classy lists. + class Implementation < Module + extend Dry::Core::ClassAttributes + + include Dry::Initializer[undefined: false].define -> do + param :config, Support::ClassyList::Configuration::Type + end + + defines :module_infix, type: Types::Symbol + + module_infix :unknown + + delegate :list_name, :config_name, :single_dsl_method, :plural_dsl_method, :realize_method, :ivar, to: :config + + # @return [Symbol] + attr_reader :module_name + + def initialize(...) + super + + @module_name = [list_name, module_infix, "methods"].join(?_).camelize(:upper) + end + + def extended(base) + super + + attach_to!(base) + end + + def included(base) + super + + attach_to!(base) + end + + private + + def attach_to!(base) + # base.const_set module_name, self + end + + # @return [Symbol] + def module_infix = self.class.module_infix + end + end +end diff --git a/lib/support/lib/classy_list/instance_implementation.rb b/lib/support/lib/classy_list/instance_implementation.rb new file mode 100644 index 00000000..bd2c20b7 --- /dev/null +++ b/lib/support/lib/classy_list/instance_implementation.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Support + module ClassyList + # Instance method implementations for a specific message map. + # @api private + class InstanceImplementation < Implementation + module_infix :instance + + def initialize(...) + super + + define_instance_methods! + end + + private + + # @return [void] + def define_instance_methods! + class_eval <<~RUBY, __FILE__, __LINE__ + 1 + def #{list_name} = self.class.#{list_name} + RUBY + + if config.realize? + class_eval <<~RUBY, __FILE__, __LINE__ + 1 + def #{realize_method} = #{list_name}.realize(self) + RUBY + end + end + end + end +end diff --git a/lib/support/lib/classy_list/klass_implementation.rb b/lib/support/lib/classy_list/klass_implementation.rb new file mode 100644 index 00000000..decded16 --- /dev/null +++ b/lib/support/lib/classy_list/klass_implementation.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Support + module ClassyList + # Class method implementations for a specific message map. + # + # @api private + class KlassImplementation < Implementation + module_infix :klass + + # @return [Support::ClassyList::InstanceImplementation] + attr_reader :instance_implementation + + def initialize(...) + super + + @instance_implementation = InstanceImplementation.new(config) + + define_dsl_methods! + end + + def extended(base) + super + + _config = config + + base.define_singleton_method(config_name) { _config } + + base.include @instance_implementation + end + + private + + # @return [void] + def define_dsl_methods! + class_eval <<~RUBY, __FILE__, __LINE__ + 1 + def inherited(other) + super + + other.instance_variable_set(#{ivar.inspect}, #{list_name}.dup) + end + + def #{list_name} + #{ivar} ||= #{config_name}.build_list + end + + def #{single_dsl_method}(new_item) + #{list_name}.add!(new_item) + end + + def #{plural_dsl_method}(*new_items) + #{list_name}.merge!(new_items) + end + RUBY + end + end + end +end diff --git a/lib/support/lib/classy_list/list.rb b/lib/support/lib/classy_list/list.rb new file mode 100644 index 00000000..357b4258 --- /dev/null +++ b/lib/support/lib/classy_list/list.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +module Support + module ClassyList + # @api private + class List + include Dry::Core::Equalizer.new(:list_name) + include Enumerable + + include Dry::Initializer[undefined: false].define -> do + option :config, Support::ClassyList::Configuration::Type + end + + delegate :item_type, :list_name, :list_type, :realize_mode, to: :config + + def initialize(...) + super + + @items = Set.new + end + + # @param [Object] new_item + # @return [Boolean] + def add!(new_item) + valid_item = item_type[new_item] + + @items.add?(valid_item) + end + + # @param [Array] new_items + # @return [void] + def merge!(*new_items) + valid_items = list_type[new_items.flatten] + + @items.merge(valid_items) + + return self + end + + def each + return enum_for(:each) unless block_given? + + items.each { |item| yield item } + end + + # @api private + # @param [Support::ClassyList::Map] original + # @return [void] + def initialize_copy(original) + super + + @items = original.items.dup + end + + # @param [Object] + # @return [Array] when `realize_mode` is `:array` + # @return [Hash] when `realize_mode` is `:hash` + def realize(source) + case realize_mode + in :array + items.map { |item| source.__send__(item) } + in :hash + items.index_with { |item| source.__send__(item) } + else + raise Support::ClassyList::Error, "#{list_name} is not realizable" + end + end + + def to_ary = to_a + + protected + + # @return [Set] + attr_reader :items + end + end +end diff --git a/lib/support/lib/classy_list/types.rb b/lib/support/lib/classy_list/types.rb new file mode 100644 index 00000000..67787d19 --- /dev/null +++ b/lib/support/lib/classy_list/types.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Support + module ClassyList + module Types + extend Support::Typespace + + # A very loose type representing a method name. + # + # @return [Dry::Types::Type(Symbol)] + MethodName = Coercible::Symbol.constrained(format: /\A[a-z][a-z0-9_]+[?!]?\z/i) + + # A type representing an array of method names. + # + # @return [Dry::Types::Type(Array)] + MethodNames = Array.of(MethodName) + + RealizeMode = Symbol.default(:none).enum(:none, :hash, :array) + end + end +end diff --git a/lib/support/lib/inspectable.rb b/lib/support/lib/inspectable.rb new file mode 100644 index 00000000..67da5dc7 --- /dev/null +++ b/lib/support/lib/inspectable.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Support + module Inspectable + extend ActiveSupport::Concern + + # @return [String] + def internal_inspect = Support::Inspector.inspect(self, skip_internal_inspect: true) + end +end diff --git a/lib/support/lib/inspecting/types.rb b/lib/support/lib/inspecting/types.rb new file mode 100644 index 00000000..6ae13273 --- /dev/null +++ b/lib/support/lib/inspecting/types.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Support + module Inspecting + module Types + extend Support::Typespace + + # An object that has an `id` attribute. + # + # @return [Dry::Types::Type] + HasId = Interface(:id) + + # An object that has a model name + # + # @return [Dry::Types::Type] + HasModelName = Interface(:model_name) + + # An object that has a `name` attribute. + # + # @return [Dry::Types::Type] + HasName = Interface(:name) + + # An object that has a `title` attribute. + # + # @return [Dry::Types::Type] + HasTitle = Interface(:title) + + # A model instance + # + # @return [Dry::Types::Type] + ModelInstance = ::Support::Models::Types::Model + + # A sum type that matches a model instance or something with a model name. + # + # @see HasModelName + # @see HasId + # @return [Dry::Types::Type] + ModelLike = ModelInstance | HasModelName + end + end +end diff --git a/lib/support/lib/inspector.rb b/lib/support/lib/inspector.rb new file mode 100644 index 00000000..66c46b69 --- /dev/null +++ b/lib/support/lib/inspector.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +module Support + class Inspector + include Support::Inspecting::Types + include Dry::Initializer[undefined: false].define -> do + param :inspectable, Types::Any + + option :skip_internal_inspect, Types::Params::Bool, default: proc { false } + end + + def initialize(...) + super + + @attrs_for_inspect = {} + @wrapper_name = @attrs = nil + + derive_inspection! + end + + # @return [{ Symbol => Object }] + attr_reader :attrs_for_inspect + + # @return [String] + attr_reader :attrs + + # @return [String] + attr_reader :inspection + + # @return [String, nil] + attr_reader :wrapper_name + + def to_s = inspection.to_s + + alias to_str to_s + + private + + def compile_attrs + @attrs_for_inspect.map do |key, value| + "#{key}: #{value.inspect}" + end.join(", ") + end + + # @return [void] + def extract_all_attrs! + extract_attr_if!(HasId, :id) + extract_attr_if!(HasName, :name) + extract_attr_if!(HasTitle, :title) + end + + # @param [Symbol] key + # @param [Object] value + # @return [void] + def extract_attr!(key) + @attrs_for_inspect[key] = inspectable.public_send(key) + end + + # @param [Dry::Types::Type] type + # @param [Symbol] key + # @return [void] + def extract_attr_if!(type, key) + extract_attr!(key) if type.valid?(inspectable) + end + + # @return [void] + def extract_wrapper_name! + case inspectable + in ModelLike => model_like + @wrapper_name = model_like.model_name.to_s + else + @wrapper_name = inspectable.class.name + end + end + + # @return [void] + def process_inspectable! + extract_all_attrs! + + extract_wrapper_name! + + @attrs = compile_attrs + end + + # @return [String] + def derive_inspection + return inspectable.internal_inspect if !skip_internal_inspect && inspectable.respond_to?(:internal_inspect) + + process_inspectable! + + if @attrs.present? + "#{wrapper_name}(#{attrs})" + else + inspectable.inspect + end + end + + # @return [void] + def derive_inspection! + @inspection = derive_inspection.to_s.presence || inspectable.inspect + end + + class << self + # @param [Object] object + # @return [String] + def inspect(object, **options) = new(object, **options).to_s + end + end +end diff --git a/lib/support/lib/message_maps/configuration.rb b/lib/support/lib/message_maps/configuration.rb new file mode 100644 index 00000000..1d9fdfb1 --- /dev/null +++ b/lib/support/lib/message_maps/configuration.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Support + module MessageMaps + class Configuration + include Support::Typing + include Dry::Initializer[undefined: false].define -> do + param :map_name, Types::MethodName + + option :dsl_base, Types::MethodName + + option :single_dsl_method, Types::MethodName, default: -> { :"#{dsl_base.to_s.singularize}!" } + option :plural_dsl_method, Types::MethodName, default: -> { :"#{dsl_base.to_s.pluralize}!" } + option :realize_method, Types::MethodName, default: -> { :"realize_#{map_name}" } + end + + # @return [Symbol] + attr_reader :config_name + + # @return [Support::MessageMaps::InstanceImplementation] + attr_reader :instance_implementation + + # @return [Symbol] + attr_reader :ivar + + # @return [Support::MessageMaps::KlassImplementation] + attr_reader :klass_implementation + + def initialize(...) + super + + @config_name = :"#{map_name}_config" + + @ivar = :"@#{map_name}" + + @klass_implementation = KlassImplementation.new(self) + @instance_implementation = @klass_implementation.instance_implementation + end + end + end +end diff --git a/lib/support/lib/message_maps/dsl.rb b/lib/support/lib/message_maps/dsl.rb new file mode 100644 index 00000000..5c2129b1 --- /dev/null +++ b/lib/support/lib/message_maps/dsl.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Support + module MessageMaps + # A concern for classes that can define a "message map" using a DSL. + # + # A message map allows a class to generate a hash at runtime based on + # a predefined mapping of keys to method names or procs. This can be + # useful for defining a consistent export of common service objects. + module DSL + extend ActiveSupport::Concern + + module ClassMethods + # @param [Symbol] name The name of the message map to define. + # @param [Symbol] dsl_base The base name for the DSL methods to define. + # @return [void] + def has_message_map!(name, dsl_base, **options) + config = Configuration.new(name, **options, dsl_base:) + + extend config.klass_implementation + end + end + end + end +end diff --git a/lib/support/lib/message_maps/implementation.rb b/lib/support/lib/message_maps/implementation.rb new file mode 100644 index 00000000..30dfb807 --- /dev/null +++ b/lib/support/lib/message_maps/implementation.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Support + module MessageMaps + # @api private + # @abstract A base implementation metamodule for message maps. + class Implementation < Module + extend Dry::Core::ClassAttributes + + include Dry::Initializer[undefined: false].define -> do + param :config, Support::MessageMaps::Configuration::Type + end + + defines :module_infix, type: Types::Symbol + + module_infix :unknown + + delegate :map_name, :config_name, :single_dsl_method, :plural_dsl_method, :realize_method, :ivar, to: :config + + # @return [Symbol] + attr_reader :module_name + + def initialize(...) + super + + @module_name = [map_name, module_infix, "methods"].join(?_).camelize(:upper) + end + + def extended(base) + super + + attach_to!(base) + end + + def included(base) + super + + attach_to!(base) + end + + private + + def attach_to!(base) + # base.const_set module_name, self + end + + # @return [Symbol] + def module_infix = self.class.module_infix + end + end +end diff --git a/lib/support/lib/message_maps/instance_implementation.rb b/lib/support/lib/message_maps/instance_implementation.rb new file mode 100644 index 00000000..42c537b4 --- /dev/null +++ b/lib/support/lib/message_maps/instance_implementation.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Support + module MessageMaps + # Instance method implementations for a specific message map. + # @api private + class InstanceImplementation < Implementation + module_infix :instance + + def initialize(...) + super + + define_instance_methods! + end + + private + + # @return [void] + def define_instance_methods! + class_eval <<~RUBY, __FILE__, __LINE__ + 1 + def #{map_name} + self.class.#{map_name} + end + + def #{realize_method} + #{map_name}.realize(self) + end + RUBY + end + end + end +end diff --git a/lib/support/lib/message_maps/klass_implementation.rb b/lib/support/lib/message_maps/klass_implementation.rb new file mode 100644 index 00000000..07999469 --- /dev/null +++ b/lib/support/lib/message_maps/klass_implementation.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +module Support + module MessageMaps + # Class method implementations for a specific message map. + # + # @api private + class KlassImplementation < Implementation + module_infix :klass + + # @return [Support::MessageMaps::InstanceImplementation] + attr_reader :instance_implementation + + def initialize(...) + super + + @instance_implementation = InstanceImplementation.new(config) + + define_dsl_methods! + end + + def extended(base) + super + + _config = config + + base.define_singleton_method(config_name) { _config } + + base.include @instance_implementation + end + + private + + # @return [void] + def define_dsl_methods! + class_eval <<~RUBY, __FILE__, __LINE__ + 1 + def inherited(other) + super + + other.instance_variable_set(#{ivar.inspect}, #{map_name}.dup) + end + + def #{map_name} + #{ivar} ||= Support::MessageMaps::Map.new + end + + def #{single_dsl_method}(key, target = nil, &block) + message = + if block_given? ^ target.present? + block || target + elsif block_given? && target.present? + raise ArgumentError, "Must provide either a block or a target, but not both" + else + key.to_sym + end + + mapping = { key.to_sym => message } + + #{plural_dsl_method}(**mapping) + end + + def #{plural_dsl_method}(*method_names, **additional_mapping) + #{ivar} = #{map_name}.merge(*method_names, **additional_mapping) + end + RUBY + end + end + end +end diff --git a/lib/support/lib/message_maps/map.rb b/lib/support/lib/message_maps/map.rb new file mode 100644 index 00000000..36d38fec --- /dev/null +++ b/lib/support/lib/message_maps/map.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module Support + module MessageMaps + # @api private + class Map + # @param [{ Symbol => #call, Symbol }] mapping A mapping of method names to messages + # that can be used to generate reified mappings. + def initialize(mapping = Dry::Core::Constants::EMPTY_HASH) + @mapping = Types::Mapping[mapping] + + @mapping.freeze + end + + # @api private + # @param [Support::MessageMaps::Map] original + # @return [void] + def initialize_copy(original) + super + + @mapping = original.mapping.dup.freeze + end + + # @param [] method_names A list of method names to map to themselves. + # @param [Hash{Symbol => #call, Symbol }] additional_mapping + # @return [Support::MessageMaps::Map] A new map instance with the merged results + def merge(*method_names, **additional_mapping) + simple_mapping = method_names.reduce({}) do |acc, method_name| + case method_name + in Types::Mapping then acc.merge(method_name) + in Types::MethodName then acc.merge(method_name => method_name) + else + # simplecov:disable + raise ArgumentError, "Invalid method name: #{method_name.inspect}" + # simplecov:enable + end + end + + new_mapping = simple_mapping.merge(additional_mapping) + + merged = mapping.merge(new_mapping) + + Map.new(merged) + end + + # @param [Object] object The object to realize the mapping for. + # @return [{ Symbol => Object }] + def realize(object) + mapping.transform_values do |message| + case message + in Types::Callable + message.call(object) + else + object.__send__(message) + end + end + end + + protected + + # @return [Hash{ Symbol => #call, Symbol }] + attr_reader :mapping + end + end +end diff --git a/lib/support/lib/message_maps/types.rb b/lib/support/lib/message_maps/types.rb new file mode 100644 index 00000000..c753773a --- /dev/null +++ b/lib/support/lib/message_maps/types.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Support + module MessageMaps + module Types + extend Support::Typespace + + # A type representing a callable object, such as a Proc or an object that implements `#call`. + # + # @return [Dry::Types::Type(#call)] + Callable = Interface(:call) + + # A very loose type representing a method name. + # + # @return [Dry::Types::Type(Symbol)] + MethodName = Coercible::Symbol.constrained(format: /\A[a-z][a-z0-9_]+[?!]?\z/i) + + # A type representing an array of method names. + # + # @return [Dry::Types::Type(Array)] + MethodNames = Array.of(MethodName) + + Message = Callable | MethodName + + # A type representing a mapping of method names to messages + # that can be used to generate reified mappings + # + # @return [Dry::Types::Type({ Symbol => #call, Symbol })] + Mapping = Types::Hash.map(MethodName, Message) + end + end +end diff --git a/lib/support/system.rb b/lib/support/system.rb index 7d6ab0ec..f4acddd4 100644 --- a/lib/support/system.rb +++ b/lib/support/system.rb @@ -40,6 +40,7 @@ class System < Dry::System::Container config.inflector = Dry::Inflector.new do |inflections| inflections.acronym("API") inflections.acronym("ANZ") + inflections.acronym("DSL") inflections.acronym("GQL") inflections.acronym("GraphQL") inflections.acronym("HTTP") @@ -49,6 +50,7 @@ class System < Dry::System::Container inflections.acronym("TOC") inflections.acronym("WDP") inflections.acronym("URL") + inflections.acronym("VOG") end end end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 407ef889..23158faf 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -56,25 +56,7 @@ Testing::Keycloak::GlobalRegistry.instance.users.add_existing!(user) end - if evaluator.manager_on.present? - MeruAPI::Container["access.grant"].call(Role.fetch(:manager), on: evaluator.manager_on, to: user).value! - end - - if evaluator.editor_on.present? - MeruAPI::Container["access.grant"].call(Role.fetch(:editor), on: evaluator.editor_on, to: user).value! - end - - if evaluator.reviewer_on.present? - MeruAPI::Container["access.grant"].call(Role.fetch(:reviewer), on: evaluator.reviewer_on, to: user).value! - end - - if evaluator.depositor_on.present? - MeruAPI::Container["access.grant"].call(Role.fetch(:depositor), on: evaluator.depositor_on, to: user).value! - end - - if evaluator.reader_on.present? - MeruAPI::Container["access.grant"].call(Role.fetch(:reader), on: evaluator.reader_on, to: user).value! - end + user.polymorphic_grant_from!(evaluator) end end end diff --git a/spec/models/primary_role_assignment_spec.rb b/spec/models/primary_role_assignment_spec.rb new file mode 100644 index 00000000..c1496386 --- /dev/null +++ b/spec/models/primary_role_assignment_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +RSpec.describe PrimaryRoleAssignment, type: :model, grants_access: true do + include_context "entity authorization testing" + + let(:user) { regular_user } + + subject(:primary_role_assignment) { user.reload_primary_role_assignment } + + shared_context "with editor grant" do + before do + grant_access!(role_editor, on: collection, to: user) + end + end + + shared_context "with author grant" do + before do + grant_access!(role_author, on: item, to: user) + end + end + + context "as an admin user" do + let(:user) { admin_user } + + context "when the admin is also an author" do + include_context "with author grant" + + it "returns the admin role as the primary role" do + expect(primary_role_assignment.role).to eq role_admin + end + end + end + + context "as a regular user" do + context "with no grants" do + it "returns nil" do + expect(primary_role_assignment).to be_nil + end + end + + context "with an editor grant on a collection" do + include_context "with editor grant" + + it "returns the editor role as the primary role" do + expect(primary_role_assignment.role).to eq role_editor + end + end + + context "with an author grant on an item" do + include_context "with author grant" + + it "returns the author role as the primary role" do + expect(primary_role_assignment.role).to eq role_author + end + end + end + + context "as an anonymous user" do + let(:user) { anonymous_user } + let(:primary_role_assignment) { nil } + + it { is_expected.to be_nil } + end +end diff --git a/spec/policies/contributor_user_link_policy_spec.rb b/spec/policies/contributor_user_link_policy_spec.rb index 0f82f0ae..a7e3328e 100644 --- a/spec/policies/contributor_user_link_policy_spec.rb +++ b/spec/policies/contributor_user_link_policy_spec.rb @@ -48,7 +48,7 @@ let(:user) { linked_user } end - failed "as a regular user" do + succeed "as a regular user" do let(:user) { regular_user } end diff --git a/spec/policies/submission_policy_spec.rb b/spec/policies/submission_policy_spec.rb index f9c9d3bf..220c9473 100644 --- a/spec/policies/submission_policy_spec.rb +++ b/spec/policies/submission_policy_spec.rb @@ -1,129 +1,20 @@ # frozen_string_literal: true RSpec.describe SubmissionPolicy, type: :policy do - include_context "policy setup" - - let_it_be(:community, refind: true) { FactoryBot.create(:community) } - - let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } - - let_it_be(:item_schema_version, refind: true) { FactoryBot.create(:schema_version, :item) } - - let_it_be(:submission_target, refind: true) do - collection.fetch_submission_target!.tap do |st| - st.configure!(schema_versions: [item_schema_version], deposit_mode: :direct) - st.transition_to! :open - end - end - - let_it_be(:reviewer, refind: true) do - FactoryBot.create(:user).tap do |user| - FactoryBot.create(:submission_target_reviewer, submission_target:, user:) - end.reload - end - - let_it_be(:submitter, refind: true) do - FactoryBot.create(:user, depositor_on: collection) - end - - let_it_be(:submission, refind: true) do - FactoryBot.create(:submission, - submission_target:, - schema_version: item_schema_version, - parent_entity: collection, - user: submitter, - title: "Test Submission" - ) - end + include_context "depositing policy setup" let(:record) { submission } - shared_examples_for "admin + reviewer + submitter access" do - succeed "as an admin" do - let(:user) { admin } - end - - succeed "as a reviewer" do - let(:user) { reviewer } - end - - succeed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - - shared_examples_for "admin + reviewer access" do - succeed "as an admin" do - let(:user) { admin } - end - - succeed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - - shared_examples_for "admin-only access" do - succeed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - describe_rule :read? do - include_examples "admin + reviewer + submitter access" + include_examples "an admin+reviewer+submitter-only permission" end describe_rule :show? do - include_examples "admin + reviewer + submitter access" + include_examples "an admin+reviewer+submitter-only permission" end describe_rule :create? do - succeed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end + include_examples "an admin-only depositing permission" succeed "as a submitter who has accepted the agreement" do let(:user) { submitter } @@ -132,82 +23,38 @@ submission_target.accept_agreement_for!(user) end end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end end describe_rule :update? do - succeed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - succeed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "an admin+submitter-only permission" end describe_rule :destroy? do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "an admin+submitter-only permission" end describe_rule :alter_schema_version? do - include_examples "admin-only access" + include_examples "an admin-only depositing permission" end describe_rule :comment? do - include_examples "admin + reviewer + submitter access" + include_examples "an admin+reviewer+submitter-only permission" end describe_rule :migrate? do - include_examples "admin-only access" + include_examples "an admin-only depositing permission" end describe_rule :publish? do - include_examples "admin-only access" + include_examples "an admin-only depositing permission" end describe_rule :request_review? do - include_examples "admin + reviewer + submitter access" + include_examples "an admin+reviewer+submitter-only permission" end describe_rule :review? do - include_examples "admin + reviewer access" + include_examples "an admin+reviewer-only permission" end describe "relation scope" do diff --git a/spec/policies/submission_publication_policy_spec.rb b/spec/policies/submission_publication_policy_spec.rb index 32141047..12a6b305 100644 --- a/spec/policies/submission_publication_policy_spec.rb +++ b/spec/policies/submission_publication_policy_spec.rb @@ -1,153 +1,30 @@ # frozen_string_literal: true RSpec.describe SubmissionPublicationPolicy, type: :policy do - include_context "policy setup" - - let_it_be(:community, refind: true) { FactoryBot.create(:community) } - - let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } - - let_it_be(:item_schema_version, refind: true) { FactoryBot.create(:schema_version, :item) } - - let_it_be(:submission_target, refind: true) do - collection.fetch_submission_target!.tap do |st| - st.configure!(schema_versions: [item_schema_version], deposit_mode: :direct) - st.transition_to! :open - end - end - - let_it_be(:reviewer, refind: true) do - FactoryBot.create(:user).tap do |user| - FactoryBot.create(:submission_target_reviewer, submission_target:, user:) - end.reload - end - - let_it_be(:submitter, refind: true) do - FactoryBot.create(:user, depositor_on: collection) - end - - let_it_be(:submission, refind: true) do - FactoryBot.create(:submission, - submission_target:, - schema_version: item_schema_version, - parent_entity: collection, - user: submitter, - title: "Test Submission" - ) - end + include_context "depositing policy setup" let_it_be(:submission_publication, refind: true) { FactoryBot.create :submission_publication, submission: } let(:record) { submission_publication } describe_rule :read? do - succeed "as an admin" do - let(:user) { admin } - end - - succeed "as a reviewer" do - let(:user) { reviewer } - end - - succeed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "an admin+reviewer+submitter-only permission" end describe_rule :show? do - succeed "as an admin" do - let(:user) { admin } - end - - succeed "as a reviewer" do - let(:user) { reviewer } - end - - succeed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "an admin+reviewer+submitter-only permission" end describe_rule :create? do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "a forbidden depositing permission" end describe_rule :update? do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "a forbidden depositing permission" end describe_rule :destroy? do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "a forbidden depositing permission" end describe "relation scope" do diff --git a/spec/policies/submission_target_policy_spec.rb b/spec/policies/submission_target_policy_spec.rb index f27f4a2d..9d063382 100644 --- a/spec/policies/submission_target_policy_spec.rb +++ b/spec/policies/submission_target_policy_spec.rb @@ -1,202 +1,39 @@ # frozen_string_literal: true RSpec.describe SubmissionTargetPolicy, type: :policy do - include_context "policy setup" - - let_it_be(:community, refind: true) { FactoryBot.create(:community) } - - let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } - - let_it_be(:item_schema_version, refind: true) { FactoryBot.create(:schema_version, :item) } - - let_it_be(:submission_target, refind: true) do - collection.fetch_submission_target!.tap do |st| - st.configure!(schema_versions: [item_schema_version], deposit_mode: :direct) - st.transition_to! :open - end - end - - let_it_be(:reviewer, refind: true) do - FactoryBot.create(:user).tap do |user| - FactoryBot.create(:submission_target_reviewer, submission_target:, user:) - end.reload - end - - let_it_be(:submitter, refind: true) do - FactoryBot.create(:user, depositor_on: collection) - end + include_context "depositing policy setup" let(:record) { submission_target } - shared_examples_for "no access" do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - - shared_examples_for "admin + reviewer + submitter access" do - succeed "as an admin" do - let(:user) { admin } - end - - succeed "as a reviewer" do - let(:user) { reviewer } - end - - succeed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - - shared_examples_for "admin + reviewer access" do - succeed "as an admin" do - let(:user) { admin } - end - - succeed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - - shared_examples_for "admin + submitter access" do - succeed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - succeed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - - shared_examples_for "admin-only access" do - succeed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - - shared_examples_for "all access" do - succeed "as an admin" do - let(:user) { admin } - end - - succeed "as a reviewer" do - let(:user) { reviewer } - end - - succeed "as the submitter" do - let(:user) { submitter } - end - - succeed "as a regular user" do - let(:user) { regular_user } - end - - succeed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - describe_rule :read? do - include_examples "all access" + include_examples "a full-access depositing permission" end describe_rule :show? do - include_examples "all access" + include_examples "a full-access depositing permission" end describe_rule :deposit? do - include_examples "admin + submitter access" + include_examples "an admin+submitter-only permission" end describe_rule :manage_reviewers? do - include_examples "admin-only access" + include_examples "an admin-only depositing permission" end describe_rule :publish? do - include_examples "admin-only access" + include_examples "an admin-only depositing permission" end describe_rule :request_deposit_access? do - failed "as an admin" do - let(:user) { admin } - end + include_examples "a permission denied to admin users" + include_examples "a permission granted to authenticated users" + include_examples "a permission denied to submitters" succeed "as a reviewer (with no deposit access)" do let(:user) { reviewer } end - failed "as the submitter" do - let(:user) { submitter } - end - - succeed "as a regular user" do - let(:user) { regular_user } - end - failed "as a regular user who already has a deposit request" do let(:user) { regular_user } @@ -212,36 +49,32 @@ submission_target.transition_to! :closed end end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end end describe_rule :reset_all_agreements? do - include_examples "admin-only access" + include_examples "an admin-only depositing permission" end describe_rule :review? do - include_examples "admin + reviewer access" + include_examples "an admin+reviewer-only permission" end describe_rule :create? do - include_examples "no access" + include_examples "a forbidden depositing permission" end describe_rule :update? do - include_examples "admin-only access" + include_examples "an admin-only depositing permission" end describe_rule :destroy? do - include_examples "no access" + include_examples "a forbidden depositing permission" end describe "relation scope" do - let(:target) { SubmissionTarget.all } + include_context "policy scope setup" - subject { policy.apply_scope(target, type: :active_record_relation) } + let(:target) { SubmissionTarget.all } context "as an admin" do let(:user) { admin } diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index 441b05a0..01cf9da9 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -2,14 +2,7 @@ RSpec.describe UserPolicy, type: :policy do include_context "policy setup" - - let_it_be(:community, refind: true) { FactoryBot.create :community } - - let_it_be(:manager, refind: true) { FactoryBot.create :user, manager_on: [community] } - - let_it_be(:editor, refind: true) { FactoryBot.create :user, editor_on: [community] } - - let_it_be(:reader, refind: true) { FactoryBot.create :user, reader_on: [community] } + include_context "entity authorization testing" let_it_be(:other_users, refind: true) { FactoryBot.create_list :user, 2 } @@ -17,7 +10,7 @@ let(:record) { other_user } - describe_rule :read? do + shared_examples_for "user read access" do succeed "as an admin on another user" do let(:user) { admin } end @@ -36,7 +29,7 @@ let(:record) { manager } end - failed "as an editor on another user" do + succeed "as an editor on another user" do let(:user) { editor } end @@ -45,7 +38,7 @@ let(:record) { editor } end - failed "as a reader on another user" do + succeed "as a reader on another user" do let(:user) { reader } end @@ -54,7 +47,7 @@ let(:record) { reader } end - failed "as a regular user on another user" + succeed "as a regular user on another user" succeed "as a regular user on self" do let(:record) { regular_user } @@ -70,7 +63,7 @@ end end - describe_rule :show? do + shared_examples_for "an admin or self action" do succeed "as an admin on another user" do let(:user) { admin } end @@ -80,7 +73,7 @@ let(:record) { admin } end - succeed "as a manager on another user" do + failed "as a manager on another user" do let(:user) { manager } end @@ -113,201 +106,80 @@ let(:record) { regular_user } end - failed "as an anonymous user on another user" do - let(:user) { anonymous_user } - end - - succeed "as an anonymous user on self" do + failed "as an anonymous user" do let(:user) { anonymous_user } - let(:record) { anonymous_user } end end - describe_rule :create? do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a manager" do - let(:user) { manager } - end - - failed "as an editor" do - let(:user) { editor } - end - - failed "as a reader" do - let(:user) { reader } - end + describe_rule :read? do + include_examples "user read access" + end - failed "as a regular user" + describe_rule :show? do + include_examples "user read access" + end - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + describe_rule :create? do + include_examples "a forbidden permission" end describe_rule :update? do - succeed "as an admin on another user" do - let(:user) { admin } - end - - succeed "as an admin on self" do - let(:user) { admin } - let(:record) { admin } - end - - failed "as a manager on another user" do - let(:user) { manager } - end - - succeed "as a manager on self" do - let(:user) { manager } - let(:record) { manager } - end - - failed "as an editor on another user" do - let(:user) { editor } - end - - succeed "as an editor on self" do - let(:user) { editor } - let(:record) { editor } - end - - failed "as a reader on another user" do - let(:user) { reader } - end - - succeed "as a reader on self" do - let(:user) { reader } - let(:record) { reader } - end - - failed "as a regular user on another user" - - succeed "as a regular user on self" do - let(:record) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "an admin or self action" end describe_rule :destroy? do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a manager" do - let(:user) { manager } - end - - failed "as an editor" do - let(:user) { editor } - end - - failed "as a reader" do - let(:user) { reader } - end - - failed "as a regular user" - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "a forbidden permission" end describe_rule :reset_password? do - succeed "as an admin on another user" do - let(:user) { admin } - end - - succeed "as an admin on self" do - let(:user) { admin } - let(:record) { admin } - end - - failed "as a manager on another user" do - let(:user) { manager } - end - - succeed "as a manager on self" do - let(:user) { manager } - let(:record) { manager } - end + include_examples "an admin or self action" + end - failed "as an editor on another user" do - let(:user) { editor } - end + describe "relation scope" do + include_context "policy scope setup" - succeed "as an editor on self" do - let(:user) { editor } - let(:record) { editor } - end + let(:target) { User.all } - failed "as a reader on another user" do - let(:user) { reader } - end + shared_examples_for "a scope that sees all users" do + include_records! :user, :admin_user, :other_users, :regular_user, :manager, :editor, :reader - succeed "as a reader on self" do - let(:user) { reader } - let(:record) { reader } + include_examples "a scope that includes known records" end - failed "as a regular user on another user" + shared_examples_for "a scope that only sees the current user" do + include_records! :user - succeed "as a regular user on self" do - let(:record) { regular_user } + include_examples "a scope that includes known records" end - failed "as an anonymous user" do - let(:user) { anonymous_user } - end - end - - describe "relation scope" do - let(:target) { User.all } - - subject { policy.apply_scope(target, type: :active_record_relation) } - context "as an admin" do - let(:user) { admin } + let(:user) { admin_user } - it "includes everything" do - is_expected.to include admin, *other_users - end + include_examples "a scope that sees all users" end context "as a manager" do let(:user) { manager } - it "includes everything" do - is_expected.to include manager, *other_users - end + include_examples "a scope that sees all users" end context "as an editor" do let(:user) { editor } - it "includes only the user" do - is_expected.to contain_exactly editor - end + include_examples "a scope that only sees the current user" end context "as a reader" do let(:user) { reader } - it "includes only the user" do - is_expected.to contain_exactly reader - end + include_examples "a scope that only sees the current user" end context "as a regular user" do - it "includes only the user" do - is_expected.to contain_exactly regular_user - end + let(:user) { regular_user } + + include_examples "a scope that only sees the current user" end context "as an anonymous user" do diff --git a/spec/requests/graphql/query/submission_spec.rb b/spec/requests/graphql/query/submission_spec.rb index 54f1c04f..0c9b6877 100644 --- a/spec/requests/graphql/query/submission_spec.rb +++ b/spec/requests/graphql/query/submission_spec.rb @@ -91,7 +91,7 @@ as_an_admin_user do let(:can_update) { true } - let(:can_destroy) { false } + let(:can_destroy) { true } include_examples "an authorized lookup" end diff --git a/spec/requests/graphql/query/submissions_spec.rb b/spec/requests/graphql/query/submissions_spec.rb index bafbe069..354c05ed 100644 --- a/spec/requests/graphql/query/submissions_spec.rb +++ b/spec/requests/graphql/query/submissions_spec.rb @@ -131,7 +131,7 @@ def order_records(records, order: "RECENT") as_an_admin_user do let(:can_update) { true } - let(:can_destroy) { false } + let(:can_destroy) { true } include_examples "ordering by each option" end diff --git a/spec/requests/graphql/query/user_spec.rb b/spec/requests/graphql/query/user_spec.rb index d87704c9..62939996 100644 --- a/spec/requests/graphql/query/user_spec.rb +++ b/spec/requests/graphql/query/user_spec.rb @@ -9,6 +9,10 @@ accessManagement globalAdmin + canAccessAdmin { + ... AuthorizationResultFragment + } + canReceiveReviewRequests { ... AuthorizationResultFragment } @@ -105,7 +109,9 @@ let_it_be(:existing_user, refind: true) { FactoryBot.create :user, :with_avatar } + let(:can_access_admin) { false } let(:can_destroy) { false } + let(:can_read_privileged) { false } let(:can_reset_password) { false } let(:can_receive_review_requests) { false } let(:can_update) { false } @@ -161,6 +167,7 @@ end as_an_admin_user do + let(:can_read_privileged) { true } let(:can_reset_password) { true } let(:can_receive_review_requests) { false } let(:can_update) { true } @@ -186,6 +193,7 @@ as_a_regular_user do it_behaves_like "a self-lookup" do + let(:can_read_privileged) { true } let(:can_reset_password) { true } let(:can_update) { true } let(:has_upload_access) { false } @@ -194,7 +202,11 @@ context "against another user" do let(:found_user) { existing_user } - it_behaves_like "a not found user" + let(:can_read_privileged) { false } + let(:can_reset_password) { false } + let(:can_update) { false } + + it_behaves_like "a found user" end context "with an invalid slug" do diff --git a/spec/requests/graphql/query/viewer_spec.rb b/spec/requests/graphql/query/viewer_spec.rb index cada8954..6ac33528 100644 --- a/spec/requests/graphql/query/viewer_spec.rb +++ b/spec/requests/graphql/query/viewer_spec.rb @@ -9,6 +9,14 @@ accessManagement globalAdmin + canAccessAdmin { + ... AuthorizationResultFragment + } + + canPreview { + ... AuthorizationResultFragment + } + canReceiveReviewRequests { ... AuthorizationResultFragment } @@ -103,6 +111,10 @@ } GRAPHQL + let_it_be(:community, refind: true) { FactoryBot.create :community } + let_it_be(:reviewer, refind: true) { FactoryBot.create :user, reviewer_on: community } + let_it_be(:depositor, refind: true) { FactoryBot.create :user, depositor_on: community } + let(:expected_upload_token) do current_user.has_any_upload_access? ? be_present : be_nil end @@ -111,6 +123,8 @@ let(:expected_access_management) { ::Types::AccessManagementType.name_for_value(current_user.access_management) } + let(:can_access_admin) { false } + let(:can_preview) { false } let(:can_receive_review_requests) { false } let(:expected_shape) do @@ -128,7 +142,7 @@ v[:upload_access] = current_user.has_any_upload_access? v[:upload_token] = expected_upload_token - v.auth_results(can_receive_review_requests:) + v.auth_results(can_access_admin:, can_preview:, can_receive_review_requests:) end end end @@ -144,18 +158,46 @@ end as_an_admin_user do + let(:can_access_admin) { true } + let(:can_preview) { true } let(:can_receive_review_requests) { true } include_examples "a found viewer" end + context "as a reviewer" do + let(:current_user) { reviewer } + + let(:can_access_admin) { true } + let(:can_preview) { true } + let(:can_receive_review_requests) { true } + + include_examples "a found viewer" + end + + context "as a depositor" do + let(:current_user) { depositor } + + let(:can_access_admin) { true } + let(:can_preview) { true } + let(:can_receive_review_requests) { false } + + include_examples "a found viewer" + end + as_a_regular_user do + let(:can_access_admin) { false } + let(:can_preview) { false } let(:can_receive_review_requests) { false } include_examples "a found viewer" end as_an_anonymous_user do + let(:can_access_admin) { false } + let(:can_preview) { false } + let(:can_receive_review_requests) { false } + include_examples "a found viewer" end end diff --git a/spec/requests/graphql/viewer_spec.rb b/spec/requests/graphql/viewer_spec.rb deleted file mode 100644 index 11589bcb..00000000 --- a/spec/requests/graphql/viewer_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe "GraphQL Viewer", type: :request do - context "when authenticated" do - let(:email) { Faker::Internet.email } - let(:token) { token_helper.build_token data: { email: } } - - let(:expected_shape) do - { - viewer: { - email:, - }, - } - end - - it "has the expected email" do - make_graphql_request!(<<~GRAPHQL, token:) - query getViewerQuery { - viewer { - email - } - } - GRAPHQL - - expect_graphql_data expected_shape - end - end -end diff --git a/spec/services/users/access_info_spec.rb b/spec/services/users/access_info_spec.rb index 18249fe4..fdd2c3c4 100644 --- a/spec/services/users/access_info_spec.rb +++ b/spec/services/users/access_info_spec.rb @@ -13,7 +13,7 @@ expect(described_class.wrap(user_access_info)).to be_a_kind_of(described_class).and(be_global) end - it "accepts an anonymous user and produces forbidden access" do + it "accepts an anonymous user and produces a forbidden access permission" do expect(described_class.wrap(AnonymousUser.new)).to be_a_kind_of(described_class).and(be_forbidden) end end diff --git a/spec/support/contexts/authorization_testing.rb b/spec/support/contexts/authorization_testing.rb new file mode 100644 index 00000000..09ee76fa --- /dev/null +++ b/spec/support/contexts/authorization_testing.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +RSpec.shared_context "all roles" do + let_it_be(:role_admin, refind: true) { Role.fetch(:admin) } + let_it_be(:role_manager, refind: true) { Role.fetch(:manager) } + let_it_be(:role_editor, refind: true) { Role.fetch(:editor) } + let_it_be(:role_reviewer, refind: true) { Role.fetch(:reviewer) } + let_it_be(:role_depositor, refind: true) { Role.fetch(:depositor) } + let_it_be(:role_author, refind: true) { Role.fetch(:author) } + let_it_be(:role_reader, refind: true) { Role.fetch(:reader) } +end + +RSpec.shared_context "all standard users" do + let_it_be(:admin_user, refind: true) { FactoryBot.create :user, :admin } + + let_it_be(:admin) { admin_user } + + let_it_be(:regular_user, refind: true) { FactoryBot.create :user } + + let_it_be(:anonymous_user) { AnonymousUser.new } +end + +RSpec.shared_context "authorization testing" do + include_context "all roles" + include_context "all standard users" +end + +RSpec.shared_context "entity authorization testing" do + include_context "authorization testing" + + let_it_be(:item_schema_version, refind: true) { FactoryBot.create(:schema_version, :item) } + + let_it_be(:community, refind: true) { FactoryBot.create(:community) } + let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } + let_it_be(:item, refind: true) { FactoryBot.create(:item, collection:) } + + let_it_be(:community_manager, refind: true) { FactoryBot.create(:user, manager_on: community) } + let_it_be(:community_editor, refind: true) { FactoryBot.create(:user, editor_on: community) } + let_it_be(:community_reader, refind: true) { FactoryBot.create(:user, reader_on: community) } + let_it_be(:collection_editor, refind: true) { FactoryBot.create(:user, editor_on: collection) } + + let(:manager) { community_manager } + let(:editor) { community_editor } + let(:reader) { community_reader } +end + +RSpec.shared_context "depositing authorization testing" do + include_context "entity authorization testing" + + let_it_be(:submission_target, refind: true) do + collection.fetch_submission_target!.tap do |st| + st.configure!(schema_versions: [item_schema_version], deposit_mode: :direct) + st.transition_to! :open + end + end + + let_it_be(:reviewer, refind: true) do + FactoryBot.create(:user, reviewer_on: collection) + end + + let_it_be(:submitter, refind: true) do + FactoryBot.create(:user, depositor_on: collection) + end + + let_it_be(:submission, refind: true) do + FactoryBot.create(:submission, + submission_target:, + schema_version: item_schema_version, + parent_entity: collection, + user: submitter, + title: "Test Submission" + ) + end +end diff --git a/spec/support/contexts/policy_scope_setup.rb b/spec/support/contexts/policy_scope_setup.rb new file mode 100644 index 00000000..a9344639 --- /dev/null +++ b/spec/support/contexts/policy_scope_setup.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +require_relative "policy_setup" +require_relative "../matchers/record_matching" + +RSpec.shared_context "policy scope setup", with_known_records: true do + let(:target) { record.class.all } + + let(:authorized_scope) { policy.apply_scope(target, type: :active_record_relation) } + + subject { authorized_scope } + + shared_examples_for "a scope that includes known records" do + it "includes accessible records" do + is_expected.to match_known_records(mode: :inclusion) + end + end + + shared_examples_for "a scope that excludes known records" do + it "excludes known records" do + is_expected.to match_known_records(mode: :exclusion) + end + end + + shared_examples_for "an empty scope" do + it "contains no records" do + is_expected.to be_blank + end + end + + shared_examples_for "a scope that is visible to admin users" do + context "as an admin" do + let(:user) { admin } + + include_examples "a scope that includes known records" + end + end + + shared_examples_for "a scope that is hidden from admin users" do + context "as an admin" do + let(:user) { admin } + + include_examples "a scope that excludes known records" + end + end + + shared_examples_for "a scope that is visible to authenticated non-admin users" do + context "as a regular user" do + let(:user) { regular_user } + + include_examples "a scope that includes known records" + end + + context "as an anonymous user" do + let(:user) { anonymous_user } + + include_examples "a scope that excludes known records" + end + end + + shared_examples_for "a scope that is visible to non-admin users" do + context "as a regular user" do + let(:user) { regular_user } + + include_examples "a scope that includes known records" + end + + context "as an anonymous user" do + let(:user) { anonymous_user } + + include_examples "a scope that includes known records" + end + end + + shared_examples_for "a scope that is hidden from non-admin users" do + context "as a regular user" do + let(:user) { regular_user } + + include_examples "a scope that excludes known records" + end + + context "as an anonymous user" do + let(:user) { anonymous_user } + + include_examples "a scope that excludes known records" + end + end + + shared_examples_for "a full-access scope" do + context "as an admin" do + let(:user) { admin } + + include_examples "a scope that includes known records" + end + + context "as a regular user" do + let(:user) { regular_user } + + include_examples "a scope that includes known records" + end + + context "as an anonymous user" do + let(:user) { anonymous_user } + + include_examples "a scope that includes known records" + end + end + + shared_examples_for "an admin-only scope" do + include_examples "a scope that is visible to admin users" + include_examples "a scope that is hidden from non-admin users" + end + + shared_examples_for "an admin-and-authenticated-only scope" do + include_examples "a scope that is visible to admin users" + include_examples "a scope that is visible to authenticated non-admin users" + end + + shared_examples_for "a forbidden access scope" do + include_examples "a scope that is hidden from admin users" + include_examples "a scope that is hidden from non-admin users" + end +end + +RSpec.shared_context "depositing policy scope setup", policy_scope: true do + include_context "policy scope setup" + include_context "depositing policy setup" +end + +RSpec.configure do |config| + config.include RecordMatching::KnownRecordHelpers, policy_scope: true + config.extend RecordMatching::KnownRecordHelpers::ClassMethods, policy_scope: true +end diff --git a/spec/support/contexts/policy_setup.rb b/spec/support/contexts/policy_setup.rb index 7a3fa0da..2dcf578c 100644 --- a/spec/support/contexts/policy_setup.rb +++ b/spec/support/contexts/policy_setup.rb @@ -1,13 +1,134 @@ # frozen_string_literal: true -RSpec.shared_context "policy setup" do - let_it_be(:admin, refind: true) { FactoryBot.create :user, :admin } - - let_it_be(:regular_user, refind: true) { FactoryBot.create :user } +require_relative "authorization_testing" - let_it_be(:anonymous_user) { AnonymousUser.new } +RSpec.shared_context "policy setup" do + include_context "authorization testing" let(:user) { regular_user } let(:context) { { user:, } } + + shared_examples_for "a permission granted to admin users" do + succeed "as an admin" do + let(:user) { admin_user } + end + end + + shared_examples_for "a permission denied to admin users" do + failed "as an admin" do + let(:user) { admin_user } + end + end + + shared_examples_for "a permission granted to non-admin users" do + succeed "as a regular user" do + let(:user) { regular_user } + end + + succeed "as an anonymous user" do + let(:user) { anonymous_user } + end + end + + shared_examples_for "a permission granted to authenticated users" do + succeed "as a regular user" do + let(:user) { regular_user } + end + + failed "as an anonymous user" do + let(:user) { anonymous_user } + end + end + + shared_examples_for "a permission denied to non-admin users" do + failed "as a regular user" do + let(:user) { regular_user } + end + + failed "as an anonymous user" do + let(:user) { anonymous_user } + end + end + + shared_examples_for "an admin-only permission" do + include_examples "a permission granted to admin users" + include_examples "a permission denied to non-admin users" + end + + shared_examples_for "a forbidden permission" do + include_examples "a permission denied to admin users" + include_examples "a permission denied to non-admin users" + end + + shared_examples_for "a full access permission" do + include_examples "a permission granted to admin users" + include_examples "a permission granted to non-admin users" + end +end + +RSpec.shared_context "depositing policy setup" do + include_context "policy setup" + include_context "depositing authorization testing" + + shared_examples_for "a permission granted to reviewers" do + succeed "as a reviewer" do + let(:user) { reviewer } + end + end + + shared_examples_for "a permission denied to reviewers" do + failed "as a reviewer" do + let(:user) { reviewer } + end + end + + shared_examples_for "a permission granted to submitters" do + succeed "as the submitter" do + let(:user) { submitter } + end + end + + shared_examples_for "a permission denied to submitters" do + failed "as the submitter" do + let(:user) { submitter } + end + end + + shared_examples_for "a full-access depositing permission" do + include_examples "a full access permission" + include_examples "a permission granted to reviewers" + include_examples "a permission granted to submitters" + end + + shared_examples_for "an admin+reviewer+submitter-only permission" do + include_examples "an admin-only permission" + include_examples "a permission granted to reviewers" + include_examples "a permission granted to submitters" + end + + shared_examples_for "an admin+reviewer-only permission" do + include_examples "an admin-only permission" + + include_examples "a permission granted to reviewers" + include_examples "a permission denied to submitters" + end + + shared_examples_for "an admin+submitter-only permission" do + include_examples "an admin-only permission" + include_examples "a permission denied to reviewers" + include_examples "a permission granted to submitters" + end + + shared_examples_for "an admin-only depositing permission" do + include_examples "an admin-only permission" + include_examples "a permission denied to reviewers" + include_examples "a permission denied to submitters" + end + + shared_examples_for "a forbidden depositing permission" do + include_examples "a forbidden permission" + include_examples "a permission denied to reviewers" + include_examples "a permission denied to submitters" + end end diff --git a/spec/support/matchers/record_matching.rb b/spec/support/matchers/record_matching.rb new file mode 100644 index 00000000..3766c01f --- /dev/null +++ b/spec/support/matchers/record_matching.rb @@ -0,0 +1,404 @@ +# frozen_string_literal: true + +module RecordMatching + module Types + extend Support::Typespace + + KnownMatchingMode = Coercible::Symbol.default(:inclusion).enum(:inclusion, :exclusion, :exact) + + RecordName = Support::Types::Coercible::Symbol + + RecordNames = Support::Types::Array.of(RecordName) + + RecordVisibility = Support::Types::Symbol.default(:included).enum(:included, :excluded, :ignored) + + RecordOptions = Support::Types::Hash.schema( + visibility?: RecordVisibility + ) + + RecordsMap = Support::Types::Hash.map(RecordName, RecordOptions) + end + + class KnownRecord < Support::FlexibleStruct + include Dry::Core::Equalizer.new(:name) + + include Support::Typing + + attribute :name, Types::RecordName + attribute :visibility, Types::RecordVisibility + + def excluded? = visibility == :excluded + + def included? = visibility == :included + end + + module RecordInspection + def inspect_record(record) = Support::Inspector.inspect(record) + + def inspect_scope(scope) + if scope.is_a?(ActiveRecord::Relation) + "scope of #{scope.klass.name} with #{scope.where_values_hash.inspect} conditions" + else + "array of records: #{scope.map { |r| inspect_record(r) }.join(", ")}" + end + end + end + + module KnownRecordHelpers + extend ActiveSupport::Concern + + included do + extend Dry::Core::ClassAttributes + + defines :known_records_map, type: Types::RecordsMap + + defines :known_records, type: RecordMatching::KnownRecord::List + + defines :record_matching_mode, type: Types::KnownMatchingMode + + record_matching_mode :inclusion + + known_records_map Dry::Core::Constants::EMPTY_HASH + + known_records Dry::Core::Constants::EMPTY_ARRAY + end + + def find_included_records = known_records_for(&:included?) + + def find_excluded_records = known_records_for(&:excluded?) + + # @!attribute [r] known_records + # @return [] the list of known records with their visibility + def known_records = self.class.known_records + + # @return [] + def known_records_for(&) + raise "must provide a block" unless block_given? + + known = known_records.select(&) + + known.flat_map do |record| + __send__(record.name) + end + end + + def match_known_records(mode: record_matching_mode) + case RecordMatching::Types::KnownMatchingMode[mode] + in :exact + match_records.containing_exactly(find_included_records) + in :exclusion + match_records.excluding(*find_included_records) + in :empty + match_no_records + else + match_records.including(*find_included_records).excluding(*find_excluded_records) + end + end + + # @return [:inclusion, :exclusion, :exact] + def record_matching_mode = self.class.record_matching_mode + + module ClassMethods + def ignore_records!(*record_names) + mapping = map_record_names(*record_names, visibility: :ignored) + + known_records!(_clear: false, **mapping) + end + + # @param [] record_names + # @return [void] + def include_records!(*record_names, _clear: false) + mapping = map_record_names(*record_names, visibility: :included) + + known_records!(_clear:, **mapping) + end + + def exclude_records!(*record_names, _clear: false) + mapping = map_record_names(*record_names, visibility: :excluded) + + known_records!(_clear:, **mapping) + end + + # @param [{ RecordName => RecordVisibility, RecordOptions, Boolean, nil }] mapping + # @return [void] + def known_records!(_clear: false, **mapping) + new_records_map = mapping.transform_values do |input| + known_record_options_for(input) + end + + new_known_records_map = _clear ? new_records_map : known_records_map.merge(new_records_map) + + known_records_map new_known_records_map.freeze + + new_known_records = new_known_records_map.map do |name, options| + RecordMatching::KnownRecord.new(name:, **options) + end + + known_records new_known_records + end + + # @return [void] + def exclude_known_records! + record_matching_mode :exclusion + end + + # @return [void] + def include_known_records! + record_matching_mode :inclusion + end + + # @return [void] + def only_known_records! + record_matching_mode :exact + end + + def known_record_options_for(input) + case input + in RecordMatching::Types::RecordOptions then input + in RecordMatching::Types::RecordVisibility => visibility then { visibility: } + in true then { visibility: :included } + in false then { visibility: :excluded } + in nil then { visibility: :ignored } + else + raise ArgumentError, "Invalid record visibility value: #{input.inspect}" + end + end + + private + + # @param [<#to_sym>] record_names + # @return [] + def enforce_record_names(*record_names) + record_names.flatten.map { RecordMatching::Types::RecordName[_1] }.sort.uniq + end + + # @return [{ RecordName => RecordOptions }] + def map_record_names(*record_names, visibility:) + enforce_record_names(*record_names).index_with { { visibility: } } + end + end + end + + # A safeguard for {RecordsMatcher} to ensure that inconsistent options can't be used. + class EnumerableMachine + include Statesman::Machine + + state :unset, initial: true + state :exact + state :fuzzy + state :count + + transition from: :unset, to: :exact + transition from: :unset, to: :fuzzy + transition from: :unset, to: :count + + transition from: :count, to: :count + transition from: :exact, to: :exact + transition from: :fuzzy, to: :fuzzy + end + + # This custom matcher is exposed as `match_records` and can be used to test + # that a given enumerable "scope" (`ActiveRecord::Relation` **or** an array of records) + # includes or excludes specific records. It provides a slightly + # more descriptive failure message that won't dump a dozen + # records if the scope doesn't match. + class RecordsMatcher + include RecordMatching::RecordInspection + include RSpec::Matchers::Composable + + def initialize + @machine = EnumerableMachine.new(self) + + @excluded = Set.new + @included = Set.new + @exclusive = Set.new + @expected_count = nil + + @missing_inclusions = [] + @unexpected_inclusions = [] + end + + # @return [ActiveRecord::Relation, Array, #include?] + attr_reader :scope + + def containing_exactly(*records) + records.flatten! + + set_mode!(:exact) + + @exclusive.merge(records) + + self + end + + # @param [] records + # @return [void] + def including(*records) + records.flatten! + + set_mode!(:fuzzy) + + @included.merge(records) + @excluded.subtract(records) + + self + end + + # @param [] records + # @return [void] + def excluding(*records) + records.flatten! + + set_mode!(:fuzzy) + + @excluded.merge(records) + @included.subtract(records) + + self + end + + def with_count(number) + set_mode!(:count) + + @expected_count = number + + self + end + + def empty = with_count(0) + + def has_any_constraints? + case current_state + in "fuzzy" + @included.any? || @excluded.any? + in "exact" + @exclusive.any? + in "count" + !@expected_count.nil? + else + false + end + end + + def has_no_constraints? = !has_any_constraints? + + # @param [Enumerable] + # @return [Boolean] + def match(scope) + raise "No constraints specified for collection matcher" if has_no_constraints? + + @scope = scope + + case current_state + in "count" + check_count_match + in "exact" + check_exact_match + in "fuzzy" + check_fuzzy_match + end + end + + alias matches? match + + def description + parts = [] + + case current_state + in "count" + if @expected_count == 0 + parts << "be empty" + else + parts << "have count #{@expected_count}" + end + in "exact" + parts << "contain exactly #{@exclusive.map { |r| inspect_record(r) }.join(", ")}" + in "fuzzy" + parts << "include #{@included.map { |r| inspect_record(r) }.join(", ")}" if @included.any? + parts << "exclude #{@excluded.map { |r| inspect_record(r) }.join(", ")}" if @excluded.any? + end + + "be a scope of records that #{parts.join(" and ")}" + end + + def failure_message + lines = ["expected records to satisfy constraints"] + lines << " defined as: #{inspect_scope(scope)}" + + case current_state + in "count" + if @expected_count == 0 + lines << " expected to be empty, but had #{@actual_count} record(s)" + else + lines << " expected count: #{@expected_count}, but got #{@actual_count}" + end + in "exact" + if @missing_exclusives.any? + lines << " missing: #{@missing_exclusives.map { |r| inspect_record(r) }.join(", ")}" + end + + if @unexpected_additional.any? + lines << " unexpected: #{@unexpected_additional.map { |r| inspect_record(r) }.join(", ")}" + end + in "fuzzy" + if @missing_inclusions.any? + lines << " missing: #{@missing_inclusions.map { |r| inspect_record(r) }.join(", ")}" + end + + if @unexpected_inclusions.any? + lines << " unexpected: #{@unexpected_inclusions.map { |r| inspect_record(r) }.join(", ")}" + end + end + + lines.join("\n") + end + + def failure_message_when_negated + "expected scope not to satisfy constraints, but it did" + end + + private + + def check_count_match + @actual_count = @scope.count + + @actual_count == @expected_count + end + + # @return [Boolean] + def check_exact_match + found_records = @scope.limit(@exclusive.size * 2).to_a + @missing_exclusives = @exclusive.reject { |record| record.in?(found_records) } + @unexpected_additional = found_records - @exclusive.to_a + + @missing_exclusives.empty? && @unexpected_additional.empty? + end + + # @return [Boolean] + def check_fuzzy_match + @missing_inclusions = @included.reject { |record| @scope.include?(record) } + @unexpected_inclusions = @excluded.select { |record| @scope.include?(record) } + + @missing_inclusions.empty? && @unexpected_inclusions.empty? + end + + def current_state = @machine.current_state + + def set_mode!(new_mode) + @machine.transition_to!(new_mode) + end + end + + module MatcherMethods + # Add `match_records` matcher for testing record collections. + def match_records = RecordsMatcher.new + + def match_no_records = RecordsMatcher.new.empty + end +end + +RSpec.configure do |config| + config.include RecordMatching::MatcherMethods + config.include RecordMatching::KnownRecordHelpers, with_known_records: true + config.extend RecordMatching::KnownRecordHelpers::ClassMethods, with_known_records: true +end From cc6f9c3559270afe7858ea6bb4422f70f0840e70 Mon Sep 17 00:00:00 2001 From: Alexa Grey Date: Tue, 30 Jun 2026 09:51:14 -0700 Subject: [PATCH 3/6] feat: delete pending reviews when publishing Resolves TASK-673 --- app/models/submission_review.rb | 2 ++ app/services/submissions/publisher.rb | 9 +++++++ spec/factories/submission_reviews.rb | 18 ++++++++++++++ .../mutations/submission_publish_spec.rb | 24 +++++++++++++++++++ 4 files changed, 53 insertions(+) diff --git a/app/models/submission_review.rb b/app/models/submission_review.rb index 10956751..c98a30d9 100644 --- a/app/models/submission_review.rb +++ b/app/models/submission_review.rb @@ -6,6 +6,8 @@ class SubmissionReview < ApplicationRecord include TimestampScopes include UsesStatesman + pg_enum! :state, as: :submission_review_state, allow_blank: false, default: :pending + has_state_machine! belongs_to :submission, inverse_of: :submission_reviews diff --git a/app/services/submissions/publisher.rb b/app/services/submissions/publisher.rb index 8c7268da..c879ccca 100644 --- a/app/services/submissions/publisher.rb +++ b/app/services/submissions/publisher.rb @@ -44,6 +44,8 @@ def call wrapped_hook! def try_to_publish actually_publish_entity! + prune_pending_reviews! + handle_state_transitions! remove_author_role! @@ -83,6 +85,13 @@ def handle_state_transitions! submission.transition_to!(:published) end + # @return [void] + def prune_pending_reviews! + SubmissionReview.pending.where(submission:).find_each do |review| + review.destroy! + end + end + # @return [void] def remove_author_role! MeruAPI::Container["access.revoke"].(author_role, on: entity, to: submission.user) diff --git a/spec/factories/submission_reviews.rb b/spec/factories/submission_reviews.rb index d03847ff..b3f4afa4 100644 --- a/spec/factories/submission_reviews.rb +++ b/spec/factories/submission_reviews.rb @@ -6,5 +6,23 @@ association :user comment { "This is a comment on the review." } + + trait :revision_requested do + after :create do |review| + review.transition_to! :revision_requested + end + end + + trait :approved do + after :create do |review| + review.transition_to! :approved + end + end + + trait :rejected do + after :create do |review| + review.transition_to! :rejected + end + end end end diff --git a/spec/requests/graphql/mutations/submission_publish_spec.rb b/spec/requests/graphql/mutations/submission_publish_spec.rb index b40087f5..472fce90 100644 --- a/spec/requests/graphql/mutations/submission_publish_spec.rb +++ b/spec/requests/graphql/mutations/submission_publish_spec.rb @@ -69,6 +69,18 @@ ) end + let_it_be(:pending_submission_review, refind: true) do + FactoryBot.create(:submission_review, submission: approved_submission) + end + + let_it_be(:revision_requested_submission_review, refind: true) do + FactoryBot.create(:submission_review, :revision_requested, submission: approved_submission) + end + + let_it_be(:approved_submission_review, refind: true) do + FactoryBot.create(:submission_review, :approved, submission: approved_submission) + end + let_it_be(:approved_entity, refind: true) { approved_submission.entity } let_it_be(:rejected_submission, refind: true) do @@ -111,11 +123,23 @@ expect_request! do |req| req.effect! change(SubmissionPublication, :count).by(1) req.effect! change(SubmissionPublicationTransition, :count).by(2) + req.effect! change(SubmissionReview, :count).by(-1) req.effect! change { approved_submission.current_state(force_reload: true) }.from("approved").to("published") req.effect! change { approved_entity.reload.submission_status }.from("submission_draft").to("submission_published") req.data! expected_shape end + + # sanity checks + + expect do + pending_submission_review.reload + end.to raise_error ActiveRecord::RecordNotFound + + expect do + approved_submission_review.reload + revision_requested_submission_review.reload + end.to execute_safely end context "when provided a submission that is not approved" do From 3ca1ef8aa1323ec02577dbf1940ea8ac5915b405 Mon Sep 17 00:00:00 2001 From: Alexa Grey Date: Tue, 30 Jun 2026 09:52:09 -0700 Subject: [PATCH 4/6] fix: properly handle hidden entity permissions Resolves TASK-675 --- .../concerns/checks_contextual_permissions.rb | 4 +- app/models/submission_batch_publication.rb | 4 +- app/models/submission_target.rb | 18 ++--- app/policies/submission_target_policy.rb | 18 +++-- .../policies/submission_target_policy_spec.rb | 67 +++++++++++++--- .../graphql/query/submission_target_spec.rb | 79 +++++++++++++------ .../graphql/query/submission_targets_spec.rb | 69 +++++++++------- 7 files changed, 171 insertions(+), 88 deletions(-) diff --git a/app/models/concerns/checks_contextual_permissions.rb b/app/models/concerns/checks_contextual_permissions.rb index 3ae95e87..1876f46e 100644 --- a/app/models/concerns/checks_contextual_permissions.rb +++ b/app/models/concerns/checks_contextual_permissions.rb @@ -51,7 +51,7 @@ def visible_to(user) # @param [] actions # @return [ActiveRecord::Relation] def with_permitted_actions_for(user, *actions) - constraint = ContextualSinglePermission.for_hierarchical_type(model_name.to_s).with_permitted_actions_for(user, *actions).select(:hierarchical_id) + constraint = ContextualSinglePermission.with_permitted_actions_for(user, *actions).select(:hierarchical_id) where(contextual_permission_primary_key => constraint) end @@ -63,7 +63,7 @@ def with_permitted_actions_for(user, *actions) def arel_visible_to(user) cppk = arel_table[contextual_permission_primary_key] - permission_constraint = ContextualSinglePermission.for_hierarchical_type(model_name.to_s).with_permitted_actions_for(user, "self.read").select(:hierarchical_id) + permission_constraint = ContextualSinglePermission.with_permitted_actions_for(user, "self.read").select(:hierarchical_id) has_read_permission = arel_expr_in_query(cppk, permission_constraint) diff --git a/app/models/submission_batch_publication.rb b/app/models/submission_batch_publication.rb index 391ee593..534f6abf 100644 --- a/app/models/submission_batch_publication.rb +++ b/app/models/submission_batch_publication.rb @@ -32,13 +32,11 @@ class << self # @param [User, AnonymousUser] user # @return [ActiveRecord::Relation] def visible_to(user) - # :nocov: return none if user.blank? || user.anonymous? return all if user.has_global_admin_access? - # :nocov: - where(submission_target: SubmissionTarget.visible_to(user)) + where(submission_target: SubmissionTarget.readable_by(user)) end end end diff --git a/app/models/submission_target.rb b/app/models/submission_target.rb index bbf7b082..e3f61808 100644 --- a/app/models/submission_target.rb +++ b/app/models/submission_target.rb @@ -5,6 +5,8 @@ # It contains information about requirements for submitting to the journal / unit / community / etc. class SubmissionTarget < ApplicationRecord include AssignsPolymorphicForeignKey + include ChecksContextualPermissions + include EntityAdjacent include HasEphemeralSystemSlug include TimestampScopes include UsesStatesman @@ -16,6 +18,8 @@ class SubmissionTarget < ApplicationRecord has_state_machine! predicates: :ALL + contextual_permission_primary_key :entity_id + belongs_to :entity, polymorphic: true, inverse_of: :submission_target belongs_to :schema_version, inverse_of: :submission_targets @@ -67,6 +71,8 @@ class SubmissionTarget < ApplicationRecord validate :must_have_schema_versions!, on: :opening + def title = "#{entity_type}(#{entity.title})" + # @param [User] user # @see DepositorAgreements::Accept # @see DepositorAgreements::Accepter @@ -184,18 +190,6 @@ def reviewable_by(user) with_contextual_action_for(user, "self.review") end - def visible_to(user) - # :nocov: - return none if user.blank? || user.anonymous? - - return all if user.has_global_admin_access? - # :nocov: - - actions = %w[self.read self.review self.deposit self.update] - - with_contextual_action_for(user, actions) - end - private # @param [User] user diff --git a/app/policies/submission_target_policy.rb b/app/policies/submission_target_policy.rb index e23fb899..153593f5 100644 --- a/app/policies/submission_target_policy.rb +++ b/app/policies/submission_target_policy.rb @@ -2,12 +2,14 @@ # @see SubmissionTarget class SubmissionTargetPolicy < ApplicationPolicy - always_readable! - pre_check :deny_anonymous!, only: %i[create? update? destroy? deposit? request_deposit_access? review?] delegate :open?, to: :record + def read? = allowed_to?(:show?, record.entity) + + def show? = allowed_to?(:show?, record.entity) + # Submission targets are not directly created. def create? = false @@ -28,11 +30,15 @@ def reset_all_agreements? = update? def review? = allowed_to?(:review?, record.entity) - relation_scope do |relation| - resolve_default_scope_for(relation) - end - private def no_deposit_request_exists? = !record.depositor_requests.exists?(user:) + + def resolve_scope_for_authenticated(relation) + relation.visible_to(user) + end + + def resolve_scope_for_anonymous(relation) + relation.visible_to(nil) + end end diff --git a/spec/policies/submission_target_policy_spec.rb b/spec/policies/submission_target_policy_spec.rb index 9d063382..f4fbe82f 100644 --- a/spec/policies/submission_target_policy_spec.rb +++ b/spec/policies/submission_target_policy_spec.rb @@ -71,31 +71,74 @@ include_examples "a forbidden depositing permission" end - describe "relation scope" do - include_context "policy scope setup" + describe "relation scope", policy_scope: true do + include_context "depositing policy scope setup" let(:target) { SubmissionTarget.all } + let_it_be(:hidden_collection, refind: true) { FactoryBot.create :collection, :hidden, community:, title: "Hidden Collection" } + + let_it_be(:hidden_target, refind: true) { hidden_collection.fetch_submission_target! } + + let_it_be(:community_reviewer, refind: true) { FactoryBot.create :user, reviewer_on: community } + let_it_be(:community_depositor, refind: true) { FactoryBot.create :user, depositor_on: community } + + shared_examples_for "a scope that sees the hidden target" do + include_records! :hidden_target + + include_examples "a scope that includes known records" + end + + shared_examples_for "a scope that only sees public targets" do + exclude_records! :hidden_target + + include_examples "a scope that includes known records" + end + + include_records! :submission_target + context "as an admin" do - let(:user) { admin } + let(:user) { admin_user } - it "includes everything" do - is_expected.to include record - end + include_examples "a scope that sees the hidden target" + end + + context "as a reviewer with access" do + let(:user) { community_reviewer } + + include_examples "a scope that sees the hidden target" + end + + context "as a reviewer without access" do + let(:user) { reviewer } + + include_examples "a scope that only sees public targets" + end + + context "as a depositor with access" do + let(:user) { community_depositor } + + include_examples "a scope that sees the hidden target" + end + + context "as a depositor without access" do + let(:user) { submitter } + + include_examples "a scope that only sees public targets" end context "as a regular user" do - it "includes accessible records" do - is_expected.to include(record) - end + let(:user) { regular_user } + + include_examples "a scope that only sees public targets" end context "as an anonymous user" do let(:user) { anonymous_user } - it "includes accessible records" do - is_expected.to include(record) - end + exclude_records! :submission_target, :hidden_target + + include_examples "a scope that includes known records" end end end diff --git a/spec/requests/graphql/query/submission_target_spec.rb b/spec/requests/graphql/query/submission_target_spec.rb index 318f7206..d132aee9 100644 --- a/spec/requests/graphql/query/submission_target_spec.rb +++ b/spec/requests/graphql/query/submission_target_spec.rb @@ -1,33 +1,29 @@ # frozen_string_literal: true RSpec.describe "Query.submissionTarget", type: :request do - let(:query) do - <<~GRAPHQL - query getSubmissionTarget($slug: Slug!) { - submissionTarget(slug: $slug) { - id - slug + graphql_query! <<~GRAPHQL + query getSubmissionTarget($slug: Slug!) { + submissionTarget(slug: $slug) { + id + slug - canUpdate { - ... AuthorizationResultFragment - } + entity { + __typename - canDestroy { - ... AuthorizationResultFragment - } + id + title } - } - fragment AuthorizationResultFragment on AuthorizationResult { - value - message - reasons { - details - fullMessages + canUpdate { + ... AuthorizationResultFragment + } + + canDestroy { + ... AuthorizationResultFragment } } - GRAPHQL - end + } + GRAPHQL let(:can_update) { false } let(:can_destroy) { false } @@ -35,8 +31,12 @@ let(:found_shape) do gql.query do |q| q.prop :submission_target do |m| - m[:id] = existing_model.to_encoded_id - m[:slug] = existing_model.system_slug + m[:id] = submission_target.to_encoded_id + m[:slug] = submission_target.system_slug + + m.prop :entity do |ent| + ent.typename("Collection") + end m.auth_results(can_update:, can_destroy:) end @@ -49,9 +49,13 @@ end end - let_it_be(:existing_model, refind: true) { FactoryBot.create :submission_target } + let_it_be(:community, refind: true) { FactoryBot.create(:community, title: "Test Submission Target Community") } + + let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:, title: "Test Submission Target Collection") } + + let_it_be(:submission_target, refind: true) { FactoryBot.create :submission_target, entity: collection } - let(:slug) { existing_model.system_slug } + let(:slug) { submission_target.system_slug } let(:graphql_variables) do { slug:, } @@ -89,11 +93,34 @@ end end + shared_examples_for "can see hidden submission targets" do + context "when looking for a hidden submission target" do + before do + collection.visibility = "hidden" + collection.save! + end + + include_examples "a found record" + end + end + + shared_examples_for "cannot see hidden submission targets" do + context "when looking for a hidden submission target" do + before do + collection.visibility = "hidden" + collection.save! + end + + include_examples "a not found record" + end + end + as_an_admin_user do let(:can_update) { true } let(:can_destroy) { false } include_examples "an authorized lookup" + include_examples "can see hidden submission targets" end as_a_regular_user do @@ -101,6 +128,7 @@ let(:can_destroy) { false } include_examples "an authorized lookup" + include_examples "cannot see hidden submission targets" end as_an_anonymous_user do @@ -108,5 +136,6 @@ let(:can_destroy) { false } include_examples "an authorized lookup" + include_examples "cannot see hidden submission targets" end end diff --git a/spec/requests/graphql/query/submission_targets_spec.rb b/spec/requests/graphql/query/submission_targets_spec.rb index 2a466614..b61cf906 100644 --- a/spec/requests/graphql/query/submission_targets_spec.rb +++ b/spec/requests/graphql/query/submission_targets_spec.rb @@ -1,43 +1,41 @@ # frozen_string_literal: true RSpec.describe "Query.submissionTargets", type: :request do + let_it_be(:community, refind: true) { FactoryBot.create(:community, title: "Test Submission Target Community") } + context "when ordering" do - let(:query) do - <<~GRAPHQL - query getSubmissionTargetCollection($order: SubmissionTargetOrder) { - submissionTargets(order: $order) { - edges { - node { - id - slug + graphql_query! <<~GRAPHQL + query getSubmissionTargetCollection($order: SubmissionTargetOrder) { + submissionTargets(order: $order) { + edges { + node { + id + slug - canUpdate { - ... AuthorizationResultFragment - } + entity { + __typename - canDestroy { - ... AuthorizationResultFragment - } + id + title } - } - pageInfo { - totalCount - totalUnfilteredCount + canUpdate { + ... AuthorizationResultFragment + } + + canDestroy { + ... AuthorizationResultFragment + } } } - } - fragment AuthorizationResultFragment on AuthorizationResult { - value - message - reasons { - details - fullMessages + pageInfo { + totalCount + totalUnfilteredCount } } - GRAPHQL - end + } + GRAPHQL let(:can_update) { false } let(:can_destroy) { false } @@ -75,6 +73,7 @@ let_it_be(:records, refind: true) do 1.upto(4).map do |n| attrs = { + n:, _at: n.days.ago, } @@ -84,8 +83,18 @@ let(:sorted_records) { order_records(records, order:) } - def create_record(_at:, **attrs) + def create_record(_at:, n:, **attrs) Timecop.freeze _at do + collection = FactoryBot.create(:collection, community:) + + if n == 1 + collection.visibility = "hidden" + + collection.save! + end + + attrs[:entity] = collection + FactoryBot.create(:submission_target, **attrs) end end @@ -140,6 +149,8 @@ def order_records(records, order: "RECENT") let(:can_update) { false } let(:can_destroy) { false } + let(:sorted_records) { super().reject(&:currently_hidden?) } + include_examples "ordering by each option" end @@ -147,6 +158,8 @@ def order_records(records, order: "RECENT") let(:can_update) { false } let(:can_destroy) { false } + let(:sorted_records) { super().reject(&:currently_hidden?) } + include_examples "ordering by each option" end end From 4046e2c204eadb030ea63d5b91df16cf4646a019 Mon Sep 17 00:00:00 2001 From: Alexa Grey Date: Tue, 30 Jun 2026 09:56:20 -0700 Subject: [PATCH 5/6] chore: update gems --- Gemfile.lock | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index e93cebcd..4e06cd6d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -165,7 +165,7 @@ GEM with_advisory_lock (>= 7.5.0) zeitwerk (~> 2.7) coderay (1.1.3) - concurrent-ruby (1.3.6) + concurrent-ruby (1.3.7) connection_pool (3.0.2) console (1.34.3) fiber-annotation @@ -296,13 +296,13 @@ GEM railties (>= 6.1.0) faker (3.8.0) i18n (>= 1.8.11, < 2) - faraday (2.14.1) + faraday (2.14.3) faraday-net_http (>= 2.0, < 3.5) json logger faraday-follow_redirects (0.5.0) faraday (>= 1, < 3) - faraday-net_http (3.4.2) + faraday-net_http (3.4.4) net-http (~> 0.5) faraday-retry (2.4.0) faraday (~> 2.0) @@ -412,7 +412,7 @@ GEM jmespath (1.6.2) job-iteration (1.13.1) activejob (>= 7.0) - json (2.19.5) + json (2.20.0) json-jwt (1.17.0) activesupport (>= 4.2) aes_key_wrap @@ -425,7 +425,7 @@ GEM hana (~> 1.3) regexp_parser (~> 2.0) simpleidn (~> 0.2) - jwt (2.10.2) + jwt (2.10.3) base64 keycloak-admin (1.1.7) http-cookie (~> 1.0, >= 1.0.3) @@ -507,7 +507,7 @@ GEM naught (1.1.0) net-http (0.9.1) uri (>= 0.11.1) - net-imap (0.6.4) + net-imap (0.6.4.1) date net-protocol net-pop (0.1.2) @@ -520,24 +520,24 @@ GEM newrelic_rpm (10.4.0) logger nio4r (2.7.5) - nokogiri (1.19.3) + nokogiri (1.19.4) mini_portile2 (~> 2.8.2) racc (~> 1.4) - nokogiri (1.19.3-aarch64-linux-gnu) + nokogiri (1.19.4-aarch64-linux-gnu) racc (~> 1.4) - nokogiri (1.19.3-aarch64-linux-musl) + nokogiri (1.19.4-aarch64-linux-musl) racc (~> 1.4) - nokogiri (1.19.3-arm-linux-gnu) + nokogiri (1.19.4-arm-linux-gnu) racc (~> 1.4) - nokogiri (1.19.3-arm-linux-musl) + nokogiri (1.19.4-arm-linux-musl) racc (~> 1.4) - nokogiri (1.19.3-arm64-darwin) + nokogiri (1.19.4-arm64-darwin) racc (~> 1.4) - nokogiri (1.19.3-x86_64-darwin) + nokogiri (1.19.4-x86_64-darwin) racc (~> 1.4) - nokogiri (1.19.3-x86_64-linux-gnu) + nokogiri (1.19.4-x86_64-linux-gnu) racc (~> 1.4) - nokogiri (1.19.3-x86_64-linux-musl) + nokogiri (1.19.4-x86_64-linux-musl) racc (~> 1.4) nom-xml (1.2.0) i18n @@ -547,7 +547,7 @@ GEM faraday (< 3) faraday-follow_redirects (>= 0.3.0, < 2) rexml - oj (3.17.0) + oj (3.17.3) bigdecimal (>= 3.0) ostruct (>= 0.2) openid_connect (2.3.1) @@ -860,7 +860,7 @@ GEM with_advisory_lock (7.5.0) activerecord (>= 7.2) zeitwerk (>= 2.7) - yard (0.9.43) + yard (0.9.44) yard-activerecord (0.0.17) activesupport yard (>= 0.8.3) From f9c2f0b33c7e1861831f114092dadc152d171fd1 Mon Sep 17 00:00:00 2001 From: Alexa Grey Date: Tue, 30 Jun 2026 16:49:47 -0700 Subject: [PATCH 6/6] fix: clean up shared contexts + auth checks let_it_be + anyfixture don't play well together, and let_it_be also doesn't deduplicate itself when included in shared contexts, which is causing weird errors when relying on normalized entity definitions. This moves core user/entity definitions into a before(:suite) hook so that some of the most common, expensive models can be centralized and also applies some normalization to entity titles in factories so that they are less lorem ipsum and can be more easily traced --- app/models/application_record.rb | 1 + app/models/concerns/scope_locking.rb | 33 ++++ app/services/resolvers/abstract_resolver.rb | 6 + config/application.rb | 1 + config/environments/test.rb | 6 + lib/generators/rspec/templates/policy_spec.rb | 6 +- spec/factories/collections.rb | 20 ++- spec/factories/communities.rb | 8 +- spec/factories/items.rb | 12 +- spec/factories/submission_publications.rb | 12 +- spec/factories/submission_reviews.rb | 3 +- spec/factories/submission_target_reviewers.rb | 2 +- spec/factories/submission_targets.rb | 2 +- spec/factories/submissions.rb | 20 ++- spec/factories/users.rb | 16 +- .../reprocess_all_derivatives_job_spec.rb | 6 +- .../synchronize_collections_job_spec.rb | 2 +- .../synchronize_communities_job_spec.rb | 2 +- .../entities/synchronize_items_job_spec.rb | 5 +- .../audit_contribution_counts_spec.rb | 2 +- spec/policies/access_grant_policy_spec.rb | 2 - .../collection_contribution_policy_spec.rb | 2 - spec/policies/collection_policy_spec.rb | 2 - spec/policies/community_policy_spec.rb | 2 - .../contextual_permission_policy_spec.rb | 2 - spec/policies/contributor_policy_spec.rb | 2 - .../contributor_user_link_policy_spec.rb | 2 - .../depositor_agreement_policy_spec.rb | 2 - .../policies/depositor_request_policy_spec.rb | 2 - spec/policies/entity_policy_spec.rb | 2 - .../global_configuration_policy_spec.rb | 2 - spec/policies/harvest_attempt_policy_spec.rb | 2 - spec/policies/harvest_entity_policy_spec.rb | 2 - spec/policies/harvest_mapping_policy_spec.rb | 2 - spec/policies/harvest_record_policy_spec.rb | 2 - spec/policies/harvest_set_policy_spec.rb | 2 - spec/policies/harvest_source_policy_spec.rb | 2 - .../policies/item_contribution_policy_spec.rb | 2 - spec/policies/item_policy_spec.rb | 2 - spec/policies/permalink_policy_spec.rb | 2 - spec/policies/role_policy_spec.rb | 2 - ...ubmission_batch_publication_policy_spec.rb | 163 ++---------------- .../submission_comment_policy_spec.rb | 2 - .../submission_deposit_target_policy_spec.rb | 2 - .../policies/submission_review_policy_spec.rb | 32 +--- .../policies/submission_target_policy_spec.rb | 8 +- .../submission_target_reviewer_policy_spec.rb | 2 - spec/policies/user_policy_spec.rb | 1 - spec/rails_helper.rb | 46 +++-- .../graphql/query/collections_spec.rb | 10 +- .../graphql/query/communities_spec.rb | 6 + .../graphql/query/contributors_spec.rb | 46 ++--- spec/requests/graphql/query/item_spec.rb | 120 +++++++------ spec/requests/graphql/query/roles_spec.rb | 8 +- .../query/submission_target_reviewers_spec.rb | 19 +- .../graphql/query/submission_targets_spec.rb | 6 + .../graphql/query/submissions_spec.rb | 6 + spec/requests/graphql/query/users_spec.rb | 54 +++--- spec/requests/graphql/query_spec.rb | 130 +------------- .../support/contexts/authorization_testing.rb | 75 ++++---- spec/support/contexts/journal_hierarchy.rb | 2 +- spec/support/contexts/policy_scope_setup.rb | 61 ++++++- spec/support/contexts/policy_setup.rb | 7 +- spec/support/helpers/current_user_helpers.rb | 16 +- .../an_entity_child_record_policy.rb | 2 - .../lib/factories/factory_enhancement.rb | 15 ++ .../system/lib/factories/model_enhancement.rb | 14 ++ spec/system/operations/factories/tracker.rb | 27 +++ spec/system/operations/initialize_database.rb | 135 ++++++++++++++- .../factory_bot_location_tracking.rb | 12 ++ spec/system/test_container.rb | 2 + 71 files changed, 620 insertions(+), 616 deletions(-) create mode 100644 app/models/concerns/scope_locking.rb create mode 100644 spec/system/lib/factories/factory_enhancement.rb create mode 100644 spec/system/lib/factories/model_enhancement.rb create mode 100644 spec/system/operations/factories/tracker.rb create mode 100644 spec/system/system/providers/factory_bot_location_tracking.rb diff --git a/app/models/application_record.rb b/app/models/application_record.rb index b2be86a0..c0343531 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -18,6 +18,7 @@ class ApplicationRecord < ActiveRecord::Base include ModelMutationSupport include RecordPreloading include PostgresEnums + include ScopeLocking include StoreModelIntrospection include ::Support::CallsCommonOperation include ::Support::Inspectable diff --git a/app/models/concerns/scope_locking.rb b/app/models/concerns/scope_locking.rb new file mode 100644 index 00000000..83564453 --- /dev/null +++ b/app/models/concerns/scope_locking.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +# A test concern for dealing with locking scopes for GraphQL resolvers. +# +# In the future, we'll test resolvers directly so that this is easier. +module ScopeLocking + extend ActiveSupport::Concern + + module ClassMethods + def lock_to!(*records) + records.flatten! + + @record_ids, original = records.map(&:id), record_ids + + yield + ensure + @record_ids = original + end + + # @return [ActiveRecord::Relation] + def maybe_locked + if record_ids.any? + where(id: record_ids) + else + all + end + end + + def record_ids + @record_ids ||= Dry::Core::Constants::EMPTY_ARRAY + end + end +end diff --git a/app/services/resolvers/abstract_resolver.rb b/app/services/resolvers/abstract_resolver.rb index a39a6f92..7ec664e0 100644 --- a/app/services/resolvers/abstract_resolver.rb +++ b/app/services/resolvers/abstract_resolver.rb @@ -273,6 +273,10 @@ def fetch_unfiltered_count = unfiltered_scope.count_from_subquery # @return [Class] def implicit_authorization_target = model_klass + def maybe_lock_scope(scope) + scope.all.maybe_locked + end + # @param [ActiveRecord::Relation, nil] scope # @param [{ Symbol => Object }] options # @return [ActiveRecord::Relation] @@ -282,6 +286,8 @@ def normalize_scope_option(scope: nil, **options) base_scope = scope || (config[:scope] && instance_eval(&config[:scope])) # :nocov: + base_scope = maybe_lock_scope(base_scope) if Rails.env.test? + base_scope = base_scope.preloaded_for_record_loading if MeruConfig.record_preloading_enabled? && base_scope.respond_to?(:preloaded_for_record_loading) # We may want to skip authorization in some cases. diff --git a/config/application.rb b/config/application.rb index c6df40d0..e6404d74 100644 --- a/config/application.rb +++ b/config/application.rb @@ -107,6 +107,7 @@ class Application < Rails::Application config.active_record.query_log_tags = [ # Rails query log tags: :application, :controller, :action, :job, + :source_location, # GraphQL-Ruby query log tags: { current_graphql_operation: -> { GraphQL::Current.operation_name }, diff --git a/config/environments/test.rb b/config/environments/test.rb index 55b4d3f4..561abbd6 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -45,6 +45,12 @@ # Set host to be used by links generated in mailer templates. config.action_mailer.default_url_options = { host: "example.com" } + # Highlight code that triggered database queries in logs. + config.active_record.verbose_query_logs = ENV["CI"].blank? + + # Append comments with runtime information tags to SQL queries in logs. + config.active_record.query_log_tags_enabled = ENV["CI"].blank? + # Print deprecation notices to the stderr. config.active_support.deprecation = :stderr diff --git a/lib/generators/rspec/templates/policy_spec.rb b/lib/generators/rspec/templates/policy_spec.rb index ee6b8734..9f60f0c7 100644 --- a/lib/generators/rspec/templates/policy_spec.rb +++ b/lib/generators/rspec/templates/policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe <%= class_name %>Policy, type: :policy do - include_context "policy setup" - let_it_be(<%= factory_name.inspect %>, refind: true) { FactoryBot.create <%= factory_name.inspect %> } let(:record) { <%= factory_name %> } @@ -77,7 +75,7 @@ end end - describe "relation scope" do + describe "relation scope", policy_scope: true do let(:target) { <%= class_name %>.all } subject { policy.apply_scope(target, type: :active_record_relation) } @@ -91,6 +89,8 @@ end context "as a regular user" do + let(:user) { regular_user } + it "includes accessible records" do is_expected.to include(record) end diff --git a/spec/factories/collections.rb b/spec/factories/collections.rb index 8cdc3bb7..61b00a7d 100644 --- a/spec/factories/collections.rb +++ b/spec/factories/collections.rb @@ -4,13 +4,17 @@ factory :collection do transient do schema { nil } + + sequence(:seq) { _1 } + + title_prefix { "Collection" } end association :community schema_version { schema.present? ? SchemaVersion[schema] : SchemaVersion.default_collection } - title { Faker::Lorem.sentence } + title { "#{title_prefix} #{seq}" } identifier { title.parameterize } raw_doi { nil } @@ -21,10 +25,14 @@ pending_properties { {} } trait :hidden do + title_prefix { "Hidden Collection" } + visibility { :hidden } end trait :limited do + title_prefix { "Limited Visibility Collection" } + visibility { :limited } visible_after_at { 1.day.ago } @@ -44,22 +52,32 @@ end trait :journal do + title_prefix { "Journal" } + schema_version { SchemaVersion["nglp:journal"] } end trait :journal_volume do + title_prefix { "Journal Volume" } + schema_version { SchemaVersion["nglp:journal_volume"] } end trait :journal_issue do + title_prefix { "Journal Issue" } + schema_version { SchemaVersion["nglp:journal_issue"] } end trait :series do + title_prefix { "Series" } + schema_version { SchemaVersion["nglp:series"] } end trait :unit do + title_prefix { "Unit" } + schema_version { SchemaVersion["nglp:unit"] } end end diff --git a/spec/factories/communities.rb b/spec/factories/communities.rb index 7c18bf2b..755d56f1 100644 --- a/spec/factories/communities.rb +++ b/spec/factories/communities.rb @@ -4,15 +4,21 @@ factory :community do transient do schema { nil } + + sequence(:seq) { _1 } + + title_prefix { "Community" } end - title { Faker::Commerce.product_name } + title { "#{title_prefix} #{seq}" } schema_version { schema.present? ? SchemaVersion[schema] : SchemaVersion.default_community } pending_properties { {} } trait :simple do + title_prefix { "Simple Community" } + association :schema_version, :simple_community, :v1 end diff --git a/spec/factories/items.rb b/spec/factories/items.rb index b93a72fc..fe27fe97 100644 --- a/spec/factories/items.rb +++ b/spec/factories/items.rb @@ -4,13 +4,17 @@ factory :item do transient do schema { nil } + + sequence(:seq) { _1 } + + title_prefix { "Item" } end association :collection schema_version { schema.present? ? SchemaVersion[schema] : SchemaVersion.default_item } - title { Faker::Lorem.sentence } + title { "#{title_prefix} #{seq}" } identifier { title.parameterize } raw_doi { nil } @@ -21,10 +25,14 @@ pending_properties { {} } trait :hidden do + title_prefix { "Hidden Item" } + visibility { :hidden } end trait :limited do + title_prefix { "Limited Visibility Item" } + visibility { :limited } visible_after_at { 1.day.ago } @@ -54,6 +62,8 @@ end end + title_prefix { "Journal Article" } + schema { "nglp:journal_article" } after(:create) do |item, evaluator| diff --git a/spec/factories/submission_publications.rb b/spec/factories/submission_publications.rb index 64949cd7..9a05018a 100644 --- a/spec/factories/submission_publications.rb +++ b/spec/factories/submission_publications.rb @@ -2,18 +2,22 @@ FactoryBot.define do factory :submission_publication do + transient do + sequence(:title) { "Submission Publication #{_1}" } + end + submission_batch_publication { nil } submission do - next FactoryBot.create(:submission, :approved) unless submission_batch_publication.present? + attrs = { title: } - submission_target = submission_batch_publication.submission_target + attrs[:submission_target] = submission_batch_publication.submission_target if submission_batch_publication.present? - FactoryBot.create(:submission, :approved, submission_target:) + FactoryBot.create(:submission, :approved, **attrs) end user do - submission_batch_publication&.user || create(:user) + submission_batch_publication&.user || create(:user, given_name: "Submission Publication", family_name: "Publisher") end end end diff --git a/spec/factories/submission_reviews.rb b/spec/factories/submission_reviews.rb index b3f4afa4..c08dce81 100644 --- a/spec/factories/submission_reviews.rb +++ b/spec/factories/submission_reviews.rb @@ -3,7 +3,8 @@ FactoryBot.define do factory :submission_review do association :submission - association :user + + association :user, name_prefix: "Submission", name_suffix: "Reviewer" comment { "This is a comment on the review." } diff --git a/spec/factories/submission_target_reviewers.rb b/spec/factories/submission_target_reviewers.rb index 7f9c48fe..b68a9939 100644 --- a/spec/factories/submission_target_reviewers.rb +++ b/spec/factories/submission_target_reviewers.rb @@ -3,6 +3,6 @@ FactoryBot.define do factory :submission_target_reviewer do association :submission_target - association :user + association :user, name_prefix: "Submission", name_suffix: "Reviewer" end end diff --git a/spec/factories/submission_targets.rb b/spec/factories/submission_targets.rb index 7871fbcc..b6d5ed2a 100644 --- a/spec/factories/submission_targets.rb +++ b/spec/factories/submission_targets.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :submission_target do - entity { FactoryBot.create :collection } + association :entity, factory: :collection, title_prefix: "Submission Target" schema_version { entity.schema_version } end diff --git a/spec/factories/submissions.rb b/spec/factories/submissions.rb index edb5899f..ee2a697a 100644 --- a/spec/factories/submissions.rb +++ b/spec/factories/submissions.rb @@ -3,14 +3,24 @@ FactoryBot.define do factory :submission do association :submission_target - association :user - association :schema_version, :collection - association :parent_entity, factory: :collection + association :user, name_prefix: "Submission", name_suffix: "Depositor" - title { Faker::Lorem.sentence } + schema_version do + SchemaVersion["default:item"] + end + + parent_entity do + case submission_target.entity + in Collection then submission_target.entity + else + FactoryBot.create(:collection, title: "Submission Parent Collection") + end + end + + sequence(:title) { "Submission #{_1}" } trait :item do - association :schema_version, :item + schema_version { SchemaVersion["default:item"] } end trait :submitted do diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 23158faf..581d4a6f 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -10,6 +10,13 @@ depositor_on { nil } reader_on { nil } register_in_keycloak { true } + + name_prefix { "Testing" } + name_suffix { "User" } + + testing { true } + + sequence(:seq) { _1 } end keycloak_id { SecureRandom.uuid } @@ -18,14 +25,15 @@ email_verified { false } username { email } - name { "#{given_name} #{family_name}" } - given_name { Faker::Name.first_name } - family_name { Faker::Name.last_name } + given_name { name_prefix } + family_name { "#{name_suffix} #{seq}" } + + name { "#{given_name} #{family_name}" } roles { [] } resource_roles { {} } - metadata { { "testing" => true } } + metadata { { "testing" => testing } } global_access_control_list do Roles::GlobalAccessControlList.build_with(acl).as_json diff --git a/spec/jobs/entities/reprocess_all_derivatives_job_spec.rb b/spec/jobs/entities/reprocess_all_derivatives_job_spec.rb index 5e66d847..a5c2af75 100644 --- a/spec/jobs/entities/reprocess_all_derivatives_job_spec.rb +++ b/spec/jobs/entities/reprocess_all_derivatives_job_spec.rb @@ -1,12 +1,12 @@ # frozen_string_literal: true RSpec.describe Entities::ReprocessAllDerivativesJob, type: :job do - let!(:community) { FactoryBot.create :community } - let!(:collection) { FactoryBot.create :collection, community: } + let!(:community) { fixture(:community) } + let!(:collection) { fixture(:collection) } it "enqueues the expected amount of subjobs" do expect do described_class.perform_now - end.to have_enqueued_job(Entities::ReprocessDerivativesJob).twice + end.to have_enqueued_job(Entities::ReprocessDerivativesJob).at_least(2).times end end diff --git a/spec/jobs/entities/synchronize_collections_job_spec.rb b/spec/jobs/entities/synchronize_collections_job_spec.rb index d26d7195..329b25b5 100644 --- a/spec/jobs/entities/synchronize_collections_job_spec.rb +++ b/spec/jobs/entities/synchronize_collections_job_spec.rb @@ -2,6 +2,6 @@ RSpec.describe Entities::SynchronizeCollectionsJob, type: :job do it_behaves_like "an entity sync job" do - let!(:entities) { FactoryBot.create_list :collection, 2 } + let!(:entities) { [fixture(:collection)] } end end diff --git a/spec/jobs/entities/synchronize_communities_job_spec.rb b/spec/jobs/entities/synchronize_communities_job_spec.rb index 609b1fd7..213f47a1 100644 --- a/spec/jobs/entities/synchronize_communities_job_spec.rb +++ b/spec/jobs/entities/synchronize_communities_job_spec.rb @@ -2,6 +2,6 @@ RSpec.describe Entities::SynchronizeCommunitiesJob, type: :job do it_behaves_like "an entity sync job" do - let!(:entities) { FactoryBot.create_list :community, 2 } + let!(:entities) { [fixture(:community)] } end end diff --git a/spec/jobs/entities/synchronize_items_job_spec.rb b/spec/jobs/entities/synchronize_items_job_spec.rb index 0777f58e..3a6e2386 100644 --- a/spec/jobs/entities/synchronize_items_job_spec.rb +++ b/spec/jobs/entities/synchronize_items_job_spec.rb @@ -2,6 +2,9 @@ RSpec.describe Entities::SynchronizeItemsJob, type: :job do it_behaves_like "an entity sync job" do - let!(:entities) { FactoryBot.create_list :item, 2 } + let!(:entities) { [fixture(:item)] } + + # We get one extra from the global `submission` fixture + let(:entity_count) { 2 } end end diff --git a/spec/operations/contributors/audit_contribution_counts_spec.rb b/spec/operations/contributors/audit_contribution_counts_spec.rb index 4ff57e1f..9115ef81 100644 --- a/spec/operations/contributors/audit_contribution_counts_spec.rb +++ b/spec/operations/contributors/audit_contribution_counts_spec.rb @@ -40,7 +40,7 @@ end it "updates the necessary row(s)" do - expect_calling.to succeed.with(2) + expect_calling.to succeed.with(3) end context "when specifying a list of IDs" do diff --git a/spec/policies/access_grant_policy_spec.rb b/spec/policies/access_grant_policy_spec.rb index c8566a1b..4f541642 100644 --- a/spec/policies/access_grant_policy_spec.rb +++ b/spec/policies/access_grant_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe AccessGrantPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create :community } let_it_be(:manager, refind: true) { FactoryBot.create :user, manager_on: [community] } diff --git a/spec/policies/collection_contribution_policy_spec.rb b/spec/policies/collection_contribution_policy_spec.rb index 3d7406ae..f5a9a380 100644 --- a/spec/policies/collection_contribution_policy_spec.rb +++ b/spec/policies/collection_contribution_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe CollectionContributionPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create :community } let_it_be(:editor_role, refind: true) { FactoryBot.create :role, :editor } diff --git a/spec/policies/collection_policy_spec.rb b/spec/policies/collection_policy_spec.rb index 757981ce..f6703428 100644 --- a/spec/policies/collection_policy_spec.rb +++ b/spec/policies/collection_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe CollectionPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community) { FactoryBot.create :community } let_it_be(:collection, refind: true) { FactoryBot.create :collection, community:, title: "Collection" } diff --git a/spec/policies/community_policy_spec.rb b/spec/policies/community_policy_spec.rb index 92c1ca31..a2cf6b00 100644 --- a/spec/policies/community_policy_spec.rb +++ b/spec/policies/community_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe CommunityPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create :community, title: "Community" } let_it_be(:other_community, refind: true) { FactoryBot.create :community, title: "Other Community" } diff --git a/spec/policies/contextual_permission_policy_spec.rb b/spec/policies/contextual_permission_policy_spec.rb index 1a029d38..3f881c65 100644 --- a/spec/policies/contextual_permission_policy_spec.rb +++ b/spec/policies/contextual_permission_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe ContextualPermissionPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create :community } let!(:contextual_permission) do diff --git a/spec/policies/contributor_policy_spec.rb b/spec/policies/contributor_policy_spec.rb index e0822002..ea9d4afc 100644 --- a/spec/policies/contributor_policy_spec.rb +++ b/spec/policies/contributor_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe ContributorPolicy, type: :policy do - include_context "policy setup" - let_it_be(:contributor, refind: true) { FactoryBot.create :contributor, :person } let(:record) { contributor } diff --git a/spec/policies/contributor_user_link_policy_spec.rb b/spec/policies/contributor_user_link_policy_spec.rb index a7e3328e..5e140c5d 100644 --- a/spec/policies/contributor_user_link_policy_spec.rb +++ b/spec/policies/contributor_user_link_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe ContributorUserLinkPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create :community } let_it_be(:manager_role) { FactoryBot.create :role, :manager } diff --git a/spec/policies/depositor_agreement_policy_spec.rb b/spec/policies/depositor_agreement_policy_spec.rb index aec5fa62..9f507ae6 100644 --- a/spec/policies/depositor_agreement_policy_spec.rb +++ b/spec/policies/depositor_agreement_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe DepositorAgreementPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create(:community) } let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } diff --git a/spec/policies/depositor_request_policy_spec.rb b/spec/policies/depositor_request_policy_spec.rb index dfb6a6c4..28141f75 100644 --- a/spec/policies/depositor_request_policy_spec.rb +++ b/spec/policies/depositor_request_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe DepositorRequestPolicy, type: :policy do - include_context "policy setup" - let_it_be(:depositor_request, refind: true) { FactoryBot.create :depositor_request } let(:record) { depositor_request } diff --git a/spec/policies/entity_policy_spec.rb b/spec/policies/entity_policy_spec.rb index 88eabaa9..80f8e34b 100644 --- a/spec/policies/entity_policy_spec.rb +++ b/spec/policies/entity_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe EntityPolicy, type: :policy do - include_context "policy setup" - let_it_be(:editor_role, refind: true) { FactoryBot.create :role, :editor } let_it_be(:community, refind: true) { FactoryBot.create :community } diff --git a/spec/policies/global_configuration_policy_spec.rb b/spec/policies/global_configuration_policy_spec.rb index b0537a7d..78695478 100644 --- a/spec/policies/global_configuration_policy_spec.rb +++ b/spec/policies/global_configuration_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe GlobalConfigurationPolicy, type: :policy do - include_context "policy setup" - let!(:record) { GlobalConfiguration.fetch } describe_rule :show? do diff --git a/spec/policies/harvest_attempt_policy_spec.rb b/spec/policies/harvest_attempt_policy_spec.rb index b4d22f01..a01f9600 100644 --- a/spec/policies/harvest_attempt_policy_spec.rb +++ b/spec/policies/harvest_attempt_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe HarvestAttemptPolicy, type: :policy do - include_context "policy setup" - let_it_be(:harvest_attempt) { FactoryBot.create :harvest_attempt } let(:record) { harvest_attempt } diff --git a/spec/policies/harvest_entity_policy_spec.rb b/spec/policies/harvest_entity_policy_spec.rb index 79f36e0d..fe273d12 100644 --- a/spec/policies/harvest_entity_policy_spec.rb +++ b/spec/policies/harvest_entity_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe HarvestEntityPolicy, type: :policy do - include_context "policy setup" - let_it_be(:harvest_entity) { FactoryBot.create :harvest_entity } let(:record) { harvest_entity } diff --git a/spec/policies/harvest_mapping_policy_spec.rb b/spec/policies/harvest_mapping_policy_spec.rb index 89b9faa6..c80e69c7 100644 --- a/spec/policies/harvest_mapping_policy_spec.rb +++ b/spec/policies/harvest_mapping_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe HarvestMappingPolicy, type: :policy do - include_context "policy setup" - let_it_be(:harvest_mapping) { FactoryBot.create :harvest_mapping } let(:record) { harvest_mapping } diff --git a/spec/policies/harvest_record_policy_spec.rb b/spec/policies/harvest_record_policy_spec.rb index d187d31b..ffb446a1 100644 --- a/spec/policies/harvest_record_policy_spec.rb +++ b/spec/policies/harvest_record_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe HarvestRecordPolicy, type: :policy do - include_context "policy setup" - let_it_be(:harvest_record) { FactoryBot.create :harvest_record } let(:record) { harvest_record } diff --git a/spec/policies/harvest_set_policy_spec.rb b/spec/policies/harvest_set_policy_spec.rb index 0650cfa6..fbc4b9b1 100644 --- a/spec/policies/harvest_set_policy_spec.rb +++ b/spec/policies/harvest_set_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe HarvestSetPolicy, type: :policy do - include_context "policy setup" - let_it_be(:harvest_set) { FactoryBot.create :harvest_set } let(:record) { harvest_set } diff --git a/spec/policies/harvest_source_policy_spec.rb b/spec/policies/harvest_source_policy_spec.rb index ce2d087a..8c77db0d 100644 --- a/spec/policies/harvest_source_policy_spec.rb +++ b/spec/policies/harvest_source_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe HarvestSourcePolicy, type: :policy do - include_context "policy setup" - let_it_be(:harvest_source) { FactoryBot.create :harvest_source } let(:record) { harvest_source } diff --git a/spec/policies/item_contribution_policy_spec.rb b/spec/policies/item_contribution_policy_spec.rb index 3b196088..c7c59518 100644 --- a/spec/policies/item_contribution_policy_spec.rb +++ b/spec/policies/item_contribution_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe ItemContributionPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create :community } let_it_be(:collection, refind: true) { FactoryBot.create :collection, community: } diff --git a/spec/policies/item_policy_spec.rb b/spec/policies/item_policy_spec.rb index 0d8d179b..9495f154 100644 --- a/spec/policies/item_policy_spec.rb +++ b/spec/policies/item_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe ItemPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create(:community) } let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } diff --git a/spec/policies/permalink_policy_spec.rb b/spec/policies/permalink_policy_spec.rb index ddc679f7..a7a185f2 100644 --- a/spec/policies/permalink_policy_spec.rb +++ b/spec/policies/permalink_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe PermalinkPolicy, type: :policy do - include_context "policy setup" - let_it_be(:permalink) { FactoryBot.create :permalink } let(:record) { permalink } diff --git a/spec/policies/role_policy_spec.rb b/spec/policies/role_policy_spec.rb index a1841cb5..cda5de55 100644 --- a/spec/policies/role_policy_spec.rb +++ b/spec/policies/role_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe RolePolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create :community } let_it_be(:manager, refind: true) { FactoryBot.create :user, manager_on: [community] } diff --git a/spec/policies/submission_batch_publication_policy_spec.rb b/spec/policies/submission_batch_publication_policy_spec.rb index 0175e248..c62fc1a7 100644 --- a/spec/policies/submission_batch_publication_policy_spec.rb +++ b/spec/policies/submission_batch_publication_policy_spec.rb @@ -1,196 +1,63 @@ # frozen_string_literal: true -RSpec.describe SubmissionBatchPublicationPolicy, type: :policy do - include_context "policy setup" - - let_it_be(:community, refind: true) { FactoryBot.create(:community) } - - let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } - - let_it_be(:item_schema_version, refind: true) { FactoryBot.create(:schema_version, :item) } - - let_it_be(:submission_target, refind: true) do - collection.fetch_submission_target!.tap do |st| - st.configure!(schema_versions: [item_schema_version], deposit_mode: :direct) - st.transition_to! :open - end - end - - let_it_be(:reviewer, refind: true) do - FactoryBot.create(:user).tap do |user| - FactoryBot.create(:submission_target_reviewer, submission_target:, user:) - end.reload - end - - let_it_be(:submitter, refind: true) do - FactoryBot.create(:user, depositor_on: collection) - end - - let_it_be(:submission, refind: true) do - FactoryBot.create(:submission, - submission_target:, - schema_version: item_schema_version, - parent_entity: collection, - user: submitter, - title: "Test Submission" - ) - end - +RSpec.describe SubmissionBatchPublicationPolicy, :depositing_policy, type: :policy do let_it_be(:submission_batch_publication, refind: true) { FactoryBot.create :submission_batch_publication, submission_target: } let(:record) { submission_batch_publication } describe_rule :read? do - succeed "as an admin" do - let(:user) { admin } - end - - succeed "as a reviewer" do - let(:user) { reviewer } - end - - succeed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "an admin+reviewer+submitter-only permission" end describe_rule :show? do - succeed "as an admin" do - let(:user) { admin } - end - - succeed "as a reviewer" do - let(:user) { reviewer } - end - - succeed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "an admin+reviewer+submitter-only permission" end describe_rule :create? do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "a forbidden depositing permission" end describe_rule :update? do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "a forbidden depositing permission" end describe_rule :destroy? do - failed "as an admin" do - let(:user) { admin } - end - - failed "as a reviewer" do - let(:user) { reviewer } - end - - failed "as the submitter" do - let(:user) { submitter } - end - - failed "as a regular user" do - let(:user) { regular_user } - end - - failed "as an anonymous user" do - let(:user) { anonymous_user } - end + include_examples "a forbidden depositing permission" end - describe "relation scope" do + describe "relation scope", :depositing_policy_scope do let(:target) { SubmissionBatchPublication.all } - subject { policy.apply_scope(target, type: :active_record_relation) } + include_records! :submission_batch_publication context "as an admin" do let(:user) { admin } - it "includes everything" do - is_expected.to include record - end + include_examples "a scope that includes known records" end context "as a reviewer" do let(:user) { reviewer } - it "includes everything" do - is_expected.to include record - end + include_examples "a scope that includes known records" end context "as the submitter" do let(:user) { submitter } - it "includes everything" do - is_expected.to include record - end + include_examples "a scope that includes known records" end context "as a regular user" do - it "forbids access" do - is_expected.to exclude(record) - end + let(:user) { regular_user } + + include_examples "a scope that excludes known records" end context "as an anonymous user" do let(:user) { anonymous_user } - it "forbids access" do - is_expected.to exclude(record) - end + include_examples "a scope that excludes known records" end end end diff --git a/spec/policies/submission_comment_policy_spec.rb b/spec/policies/submission_comment_policy_spec.rb index cda1c5ae..f8a91913 100644 --- a/spec/policies/submission_comment_policy_spec.rb +++ b/spec/policies/submission_comment_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe SubmissionCommentPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create(:community) } let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } diff --git a/spec/policies/submission_deposit_target_policy_spec.rb b/spec/policies/submission_deposit_target_policy_spec.rb index 5d2adc6a..27a313ff 100644 --- a/spec/policies/submission_deposit_target_policy_spec.rb +++ b/spec/policies/submission_deposit_target_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe SubmissionDepositTargetPolicy, type: :policy do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create(:community) } let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } diff --git a/spec/policies/submission_review_policy_spec.rb b/spec/policies/submission_review_policy_spec.rb index 0a1c075c..e4d3d799 100644 --- a/spec/policies/submission_review_policy_spec.rb +++ b/spec/policies/submission_review_policy_spec.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true -RSpec.describe SubmissionReviewPolicy, type: :policy do - include_context "policy setup" - - let_it_be(:submission_review, refind: true) { FactoryBot.create :submission_review } +RSpec.describe SubmissionReviewPolicy, :depositing_policy, type: :policy do + let_it_be(:submission_review, refind: true) { FactoryBot.create :submission_review, submission: } let(:record) { submission_review } @@ -85,31 +83,11 @@ end end - describe "relation scope" do + describe "relation scope", :depositing_policy_scope do let(:target) { SubmissionReview.all } - subject { policy.apply_scope(target, type: :active_record_relation) } - - context "as an admin" do - let(:user) { admin } - - it "includes everything" do - is_expected.to include record - end - end - - context "as a regular user" do - it "includes accessible records" do - is_expected.to exclude(record) - end - end + include_records! :submission_review - context "as an anonymous user" do - let(:user) { anonymous_user } - - it "includes accessible records" do - is_expected.to exclude(record) - end - end + include_examples "a depositing scope" end end diff --git a/spec/policies/submission_target_policy_spec.rb b/spec/policies/submission_target_policy_spec.rb index f4fbe82f..e4a9b879 100644 --- a/spec/policies/submission_target_policy_spec.rb +++ b/spec/policies/submission_target_policy_spec.rb @@ -71,9 +71,7 @@ include_examples "a forbidden depositing permission" end - describe "relation scope", policy_scope: true do - include_context "depositing policy scope setup" - + describe "relation scope", :depositing_policy_scope do let(:target) { SubmissionTarget.all } let_it_be(:hidden_collection, refind: true) { FactoryBot.create :collection, :hidden, community:, title: "Hidden Collection" } @@ -136,9 +134,7 @@ context "as an anonymous user" do let(:user) { anonymous_user } - exclude_records! :submission_target, :hidden_target - - include_examples "a scope that includes known records" + include_examples "a scope that only sees public targets" end end end diff --git a/spec/policies/submission_target_reviewer_policy_spec.rb b/spec/policies/submission_target_reviewer_policy_spec.rb index 466832b3..bb66f982 100644 --- a/spec/policies/submission_target_reviewer_policy_spec.rb +++ b/spec/policies/submission_target_reviewer_policy_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe SubmissionTargetReviewerPolicy, type: :policy do - include_context "policy setup" - let_it_be(:submission_target_reviewer, refind: true) { FactoryBot.create :submission_target_reviewer } let(:record) { submission_target_reviewer } diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index 01cf9da9..c9fc09e8 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true RSpec.describe UserPolicy, type: :policy do - include_context "policy setup" include_context "entity authorization testing" let_it_be(:other_users, refind: true) { FactoryBot.create_list :user, 2 } diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index faf31f21..9080a544 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -127,6 +127,8 @@ require "rspec/rails" require "rspec/json_expectations" +require "test_prof/any_fixture/dsl" +require "test_prof/recipes/rspec/any_fixture" require "test_prof/recipes/rspec/let_it_be" require "test_prof/recipes/rspec/sample" require "dry/container/stub" @@ -213,24 +215,34 @@ end RSpec.configure do |config| + config.include ActiveJob::TestHelper + config.include TestProf::AnyFixture::DSL + config.include WebMock::API + config.include WebMock::Matchers + + # Filter lines from Rails gems in backtraces. + config.filter_rails_from_backtrace! + + # arbitrary gems may also be filtered via: + # config.filter_gems_from_backtrace("gem name") + + config.infer_spec_type_from_file_location! + # We use database cleaner to do this config.use_transactional_fixtures = false config.before(:suite) do - DatabaseCleaner[:active_record].strategy = :transaction - DatabaseCleaner[:redis].strategy = :deletion - DatabaseCleaner[:active_record].clean_with(:truncation) DatabaseCleaner[:redis].clean_with(:deletion) + DatabaseCleaner[:active_record].strategy = :transaction + DatabaseCleaner[:redis].strategy = :deletion + Scenic.database.views.select(&:materialized).each do |view| Scenic.database.refresh_materialized_view view.name, concurrently: false, cascade: false end end - config.include WebMock::API - config.include WebMock::Matchers - config.before(:suite) do WebMock.enable! WebMock.disable_net_connect! @@ -246,7 +258,9 @@ config.around do |example| STUB_HARVEST_PROVIDERS.() - example.run + DatabaseCleaner.cleaning do + example.run + end WebMock.reset! @@ -254,24 +268,6 @@ end config.before(:suite) do - Common::Container["system.reload_everything"].(skip_refresh: true).value! TestingAPI::TestContainer["initialize_database"].().value! end - - config.around do |example| - DatabaseCleaner.cleaning do - example.run - end - end - - config.infer_spec_type_from_file_location! - - # Filter lines from Rails gems in backtraces. - config.filter_rails_from_backtrace! - - # Include AJ Test helpers in all specs. - config.include ActiveJob::TestHelper - - # arbitrary gems may also be filtered via: - # config.filter_gems_from_backtrace("gem name") end diff --git a/spec/requests/graphql/query/collections_spec.rb b/spec/requests/graphql/query/collections_spec.rb index 45180060..1ba6211e 100644 --- a/spec/requests/graphql/query/collections_spec.rb +++ b/spec/requests/graphql/query/collections_spec.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true RSpec.describe "Query.collections", type: :request do - context "when ordering" do - let_it_be(:community, refind: true) { FactoryBot.create :community } + def community = fixture(:community) + context "when ordering" do let_it_be(:collection_a3_202105, refind: true) do Timecop.freeze(3.days.ago) do FactoryBot.create :collection, community:, title: "AA", published: variable_date("2021-05"), schema: "nglp:journal_issue" @@ -72,6 +72,12 @@ def to_rep(model) GRAPHQL end + around do |example| + Collection.lock_to!(collection_a3_202105, collection_m2_202103, collection_n4_2021, collection_z1_202104, collection_k0) do + example.run + end + end + shared_examples_for "an ordered list of collections" do |order_value, *keys| let!(:order) { order_value } diff --git a/spec/requests/graphql/query/communities_spec.rb b/spec/requests/graphql/query/communities_spec.rb index 0b9237b8..0fd7aef3 100644 --- a/spec/requests/graphql/query/communities_spec.rb +++ b/spec/requests/graphql/query/communities_spec.rb @@ -67,6 +67,12 @@ def to_rep(model) model.slice(:position, :title).merge(id: model.to_encoded_id) end + around do |example| + Community.lock_to!(community_a3, community_k0, community_m2, community_n4, community_z1) do + example.run + end + end + context "when ordering" do let(:token) { token_helper.build_token has_global_admin: true } diff --git a/spec/requests/graphql/query/contributors_spec.rb b/spec/requests/graphql/query/contributors_spec.rb index bae9fdfa..b454b50a 100644 --- a/spec/requests/graphql/query/contributors_spec.rb +++ b/spec/requests/graphql/query/contributors_spec.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true RSpec.describe "Query.contributors", type: :request, disable_ordering_refresh: true do + def community = fixture(:community) + + def collection = fixture(:collection) + def create_person_contributor_at(time, item_count:, **attributes) Timecop.freeze time do FactoryBot.create(:contributor, :person, **attributes).tap do |contributor| @@ -40,11 +44,25 @@ def to_rep(model) end context "when ordering" do - let(:token) { token_helper.build_token has_global_admin: true } + graphql_query! <<~GRAPHQL + query getOrderedContributors($order: ContributorOrder!, $prefix: String) { + contributors(order: $order, prefix: $prefix) { + edges { + node { + ... on Node { id } + ... on Contributor { name } + } + } - let_it_be(:community) { FactoryBot.create :community } + pageInfo { + totalCount + totalUnfilteredCount + } + } + } + GRAPHQL - let_it_be(:collection) { FactoryBot.create :collection, community: } + let(:token) { token_helper.build_token has_global_admin: true } let_it_be(:contributor_asimov) do create_person_contributor_at(1.day.ago, item_count: 5, given_name: "Isaac", family_name: "Asimov", affiliation: "everything") @@ -81,24 +99,10 @@ def to_rep(model) } end - let!(:query) do - <<~GRAPHQL - query getOrderedContributors($order: ContributorOrder!, $prefix: String) { - contributors(order: $order, prefix: $prefix) { - edges { - node { - ... on Node { id } - ... on Contributor { name } - } - } - - pageInfo { - totalCount - totalUnfilteredCount - } - } - } - GRAPHQL + around do |example| + Contributor.lock_to!(contributor_asimov, contributor_salinger, contributor_angelou, contributor_shelley) do + example.run + end end shared_examples_for "an ordered list of contributors" do |order_value, *keys| diff --git a/spec/requests/graphql/query/item_spec.rb b/spec/requests/graphql/query/item_spec.rb index 5340cefa..c64aa0a9 100644 --- a/spec/requests/graphql/query/item_spec.rb +++ b/spec/requests/graphql/query/item_spec.rb @@ -1,84 +1,84 @@ # frozen_string_literal: true RSpec.describe "Query.item", type: :request do - let!(:query) do - <<~GRAPHQL - query getItem($slug: Slug!) { - item(slug: $slug) { - title + include_context "entity authorization testing" - applicableRoles { - id - name - } + graphql_query! <<~GRAPHQL + query getItem($slug: Slug!) { + item(slug: $slug) { + title - allowedActions + applicableRoles { + id + name + } - contributors { - nodes { - ... on OrganizationContributor { - legalName - } + allowedActions - ... on PersonContributor { - givenName - familyName - } + contributors { + nodes { + ... on OrganizationContributor { + legalName } - pageInfo { - totalCount + ... on PersonContributor { + givenName + familyName } } - items { - nodes { id } - - pageInfo { - totalCount - } + pageInfo { + totalCount } + } + + items { + nodes { id } - links { - ... EntityLinksListDataFragment + pageInfo { + totalCount } } - } - fragment EntityLinksListDataFragment on EntityLinkConnection { - nodes { - id - slug - operator - target { - __typename - ... on Item { - slug - title - schemaDefinition { - name - kind - id - } - } - ... on Collection { - slug - title - schemaDefinition { - name - kind - id - } + links { + ... EntityLinksListDataFragment + } + } + } + + fragment EntityLinksListDataFragment on EntityLinkConnection { + nodes { + id + slug + operator + target { + __typename + ... on Item { + slug + title + schemaDefinition { + name + kind + id } - ... on Node { - __isNode: __typename + } + ... on Collection { + slug + title + schemaDefinition { + name + kind id } } + ... on Node { + __isNode: __typename + id + } } } - GRAPHQL - end + } + GRAPHQL let(:slug) { random_slug } @@ -111,9 +111,7 @@ ] end - let_it_be(:item) { FactoryBot.create :item } - - let_it_be(:subitems) { FactoryBot.create_list :item, 2, parent: item } + let_it_be(:subitems) { FactoryBot.create_list :item, 2, collection:, parent: item } let_it_be(:contributors) do %i[person organization].map do |trait| diff --git a/spec/requests/graphql/query/roles_spec.rb b/spec/requests/graphql/query/roles_spec.rb index 5bb0d698..5087a4ac 100644 --- a/spec/requests/graphql/query/roles_spec.rb +++ b/spec/requests/graphql/query/roles_spec.rb @@ -22,13 +22,7 @@ end context "when ordering" do - let_it_be(:role_admin, refind: true) { Role.fetch(:admin) } - let_it_be(:role_manager, refind: true) { Role.fetch(:manager) } - let_it_be(:role_editor, refind: true) { Role.fetch(:editor) } - let_it_be(:role_reviewer, refind: true) { Role.fetch(:reviewer) } - let_it_be(:role_depositor, refind: true) { Role.fetch(:depositor) } - let_it_be(:role_author, refind: true) { Role.fetch(:author) } - let_it_be(:role_reader, refind: true) { Role.fetch(:reader) } + include_context "all roles" let_it_be(:role_a3, refind: true) do Timecop.freeze(3.days.ago) do diff --git a/spec/requests/graphql/query/submission_target_reviewers_spec.rb b/spec/requests/graphql/query/submission_target_reviewers_spec.rb index ffeb9b34..e3b24168 100644 --- a/spec/requests/graphql/query/submission_target_reviewers_spec.rb +++ b/spec/requests/graphql/query/submission_target_reviewers_spec.rb @@ -1,18 +1,7 @@ # frozen_string_literal: true RSpec.describe "Query.submissionTargetReviewers", type: :request do - let_it_be(:community, refind: true) { FactoryBot.create(:community) } - - let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } - - let_it_be(:item_schema_version, refind: true) { FactoryBot.create(:schema_version, :item) } - - let_it_be(:submission_target, refind: true) do - collection.fetch_submission_target!.tap do |st| - st.configure!(schema_versions: [item_schema_version], deposit_mode: :direct) - st.transition_to! :open - end - end + include_context "depositing authorization testing" context "when ordering" do let(:query) do @@ -132,6 +121,12 @@ def order_records(records, order: "RECENT") end end + around do |example| + SubmissionTargetReviewer.lock_to!(*records) do + example.run + end + end + shared_examples_for "a properly-ordered collection" do it "retrieves everything in the right order" do expect_request! do |req| diff --git a/spec/requests/graphql/query/submission_targets_spec.rb b/spec/requests/graphql/query/submission_targets_spec.rb index b61cf906..b3fb0a84 100644 --- a/spec/requests/graphql/query/submission_targets_spec.rb +++ b/spec/requests/graphql/query/submission_targets_spec.rb @@ -110,6 +110,12 @@ def order_records(records, order: "RECENT") end end + around do |example| + SubmissionTarget.lock_to!(*records) do + example.run + end + end + shared_examples_for "a properly-ordered collection" do it "retrieves everything in the right order" do expect_request! do |req| diff --git a/spec/requests/graphql/query/submissions_spec.rb b/spec/requests/graphql/query/submissions_spec.rb index 354c05ed..ca406ddc 100644 --- a/spec/requests/graphql/query/submissions_spec.rb +++ b/spec/requests/graphql/query/submissions_spec.rb @@ -101,6 +101,12 @@ def order_records(records, order: "RECENT") end end + around do |example| + Submission.lock_to!(*records) do + example.run + end + end + shared_examples_for "a properly-ordered collection" do it "retrieves everything in the right order" do expect_request! do |req| diff --git a/spec/requests/graphql/query/users_spec.rb b/spec/requests/graphql/query/users_spec.rb index ca1e858d..766ef2af 100644 --- a/spec/requests/graphql/query/users_spec.rb +++ b/spec/requests/graphql/query/users_spec.rb @@ -1,32 +1,26 @@ # frozen_string_literal: true RSpec.describe "Query.users", type: :request do - let!(:query) do - <<~GRAPHQL - query getOrderedUsers($order: UserOrder!, $prefix: String) { - users(order: $order, prefix: $prefix) { - edges { - node { - id - name - globalAdmin - } + graphql_query! <<~GRAPHQL + query getOrderedUsers($order: UserOrder!, $prefix: String) { + users(order: $order, prefix: $prefix) { + edges { + node { + id + name + globalAdmin } + } - pageInfo { - totalCount - totalUnfilteredCount - } + pageInfo { + totalCount + totalUnfilteredCount } } - GRAPHQL - end + } + GRAPHQL - let_it_be(:user_admin) do - Timecop.freeze(4.days.ago) do - FactoryBot.create :user, :admin, given_name: "Admin", family_name: "User", email: "admin@example.com", metadata: { testing: false } - end - end + let_it_be(:user_admin) { fixture(:admin_user) } let_it_be(:user_aa) do Timecop.freeze(3.days.ago) do @@ -40,14 +34,7 @@ end end - let_it_be(:user_test) do - Timecop.freeze(1.day.ago) do - FactoryBot.create :user, given_name: "Test", family_name: "User", email: "test@example.com", metadata: { testing: true } - end - end - - let_it_be(:default_admin_user) { user_admin } - let_it_be(:default_user) { user_test } + let_it_be(:user_test) { fixture(:regular_user) } let!(:keyed_users) do { @@ -69,9 +56,12 @@ let!(:graphql_variables) { { order:, prefix: search_prefix } } - # We gotta make sure our keycloak default users are not present - before do - User.where.not(id: [user_admin, user_aa, user_test, user_zz]).destroy_all + # We gotta make sure our fixture users are not included in the query + # @todo in the future, test resolvers directly instead of through the GraphQL API, so we can avoid this + around do |example| + User.lock_to!(user_admin, user_aa, user_test, user_zz) do + example.run + end end def shape_for(*keys) diff --git a/spec/requests/graphql/query_spec.rb b/spec/requests/graphql/query_spec.rb index 31655441..e3e36083 100644 --- a/spec/requests/graphql/query_spec.rb +++ b/spec/requests/graphql/query_spec.rb @@ -3,7 +3,7 @@ RSpec.describe "GraphQL Query", type: :request do as_an_admin_user do describe "using the relay node resolver" do - let_it_be(:collection) { FactoryBot.create :collection } + let_it_be(:collection) { fixture(:collection) } let(:expected_shape) do gql.query do |q| @@ -35,133 +35,5 @@ end end end - - describe "fetching a list of communities" do - let!(:communities) { FactoryBot.create_list :community, 5 } - - let(:query) do - <<~GRAPHQL - query getCommunities { - communities { - nodes { - id - } - } - } - GRAPHQL - end - - let(:expected_shape) do - gql.query do |q| - q.prop :communities do |c| - c[:nodes] = have_length(5) - end - end - end - - it "has the expected count" do - expect_request! do |req| - req.data! expected_shape - end - end - end - - describe "fetching a list of roles" do - let_it_be(:roles, refind: true) { FactoryBot.create_list :role, 4 } - - let(:query) do - <<~GRAPHQL - query getRoles { - roles { - nodes { - id - allowedActions - } - } - } - GRAPHQL - end - - let(:expected_shape) do - gql.query do |q| - q.prop :roles do |c| - c[:nodes] = have_length(Role.count) - end - end - end - - it "fetches all known roles in the system" do - expect_request! do |req| - req.data! expected_shape - end - end - end - - describe "fetching contributors" do - let!(:people) { FactoryBot.create_list :contributor, 2, :person } - let!(:organizations) { FactoryBot.create_list :contributor, 2, :organization } - - let(:filter_kind) { "ALL" } - - let!(:graphql_variables) do - { - kind: filter_kind - } - end - - let!(:query) do - <<~GRAPHQL - query getContributors($kind: ContributorFilterKind) { - contributors(kind: $kind) { - nodes { - __typename - - ... on OrganizationContributor { - legalName - } - - ... on PersonContributor { - givenName - familyName - } - } - } - } - GRAPHQL - end - - let(:graphql_query_options) do - { - token:, - variables: graphql_variables - } - end - - it "can fetch all contributors" do - make_graphql_request! query, **graphql_query_options - - expect(graphql_response(:data, :contributors, :nodes)).to have(4).items - end - - context "when filtering only organizations" do - let(:filter_kind) { "ORGANIZATION" } - - it "fetches only people" do - make_graphql_request! query, **graphql_query_options - - expect(graphql_response(:data, :contributors, :nodes)).to have(2).items.and all(include_json(__typename: "OrganizationContributor")) - end - end - - context "when filtering only people" do - let(:filter_kind) { "PERSON" } - - it "fetches only people" do - make_graphql_request! query, **graphql_query_options - - expect(graphql_response(:data, :contributors, :nodes)).to have(2).items.and all(include_json(__typename: "PersonContributor")) - end - end - end end end diff --git a/spec/support/contexts/authorization_testing.rb b/spec/support/contexts/authorization_testing.rb index 09ee76fa..2e4402f3 100644 --- a/spec/support/contexts/authorization_testing.rb +++ b/spec/support/contexts/authorization_testing.rb @@ -1,23 +1,29 @@ # frozen_string_literal: true RSpec.shared_context "all roles" do - let_it_be(:role_admin, refind: true) { Role.fetch(:admin) } - let_it_be(:role_manager, refind: true) { Role.fetch(:manager) } - let_it_be(:role_editor, refind: true) { Role.fetch(:editor) } - let_it_be(:role_reviewer, refind: true) { Role.fetch(:reviewer) } - let_it_be(:role_depositor, refind: true) { Role.fetch(:depositor) } - let_it_be(:role_author, refind: true) { Role.fetch(:author) } - let_it_be(:role_reader, refind: true) { Role.fetch(:reader) } + def role_admin = fixture(:role_admin) + + def role_manager = fixture(:role_manager) + + def role_editor = fixture(:role_editor) + + def role_reviewer = fixture(:role_reviewer) + + def role_depositor = fixture(:role_depositor) + + def role_author = fixture(:role_author) + + def role_reader = fixture(:role_reader) end RSpec.shared_context "all standard users" do - let_it_be(:admin_user, refind: true) { FactoryBot.create :user, :admin } + def admin_user = fixture(:admin_user) - let_it_be(:admin) { admin_user } + alias_method :admin, :admin_user - let_it_be(:regular_user, refind: true) { FactoryBot.create :user } + def regular_user = fixture(:regular_user) - let_it_be(:anonymous_user) { AnonymousUser.new } + def anonymous_user = @anonymous_user ||= AnonymousUser.new end RSpec.shared_context "authorization testing" do @@ -28,16 +34,16 @@ RSpec.shared_context "entity authorization testing" do include_context "authorization testing" - let_it_be(:item_schema_version, refind: true) { FactoryBot.create(:schema_version, :item) } + def item_schema_version = fixture(:item_schema_version) - let_it_be(:community, refind: true) { FactoryBot.create(:community) } - let_it_be(:collection, refind: true) { FactoryBot.create(:collection, community:) } - let_it_be(:item, refind: true) { FactoryBot.create(:item, collection:) } + def community = fixture(:community) + def collection = fixture(:collection) + def item = fixture(:item) - let_it_be(:community_manager, refind: true) { FactoryBot.create(:user, manager_on: community) } - let_it_be(:community_editor, refind: true) { FactoryBot.create(:user, editor_on: community) } - let_it_be(:community_reader, refind: true) { FactoryBot.create(:user, reader_on: community) } - let_it_be(:collection_editor, refind: true) { FactoryBot.create(:user, editor_on: collection) } + def community_manager = fixture(:community_manager) + def community_editor = fixture(:community_editor) + def community_reader = fixture(:community_reader) + def collection_editor = fixture(:collection_editor) let(:manager) { community_manager } let(:editor) { community_editor } @@ -47,28 +53,11 @@ RSpec.shared_context "depositing authorization testing" do include_context "entity authorization testing" - let_it_be(:submission_target, refind: true) do - collection.fetch_submission_target!.tap do |st| - st.configure!(schema_versions: [item_schema_version], deposit_mode: :direct) - st.transition_to! :open - end - end - - let_it_be(:reviewer, refind: true) do - FactoryBot.create(:user, reviewer_on: collection) - end - - let_it_be(:submitter, refind: true) do - FactoryBot.create(:user, depositor_on: collection) - end - - let_it_be(:submission, refind: true) do - FactoryBot.create(:submission, - submission_target:, - schema_version: item_schema_version, - parent_entity: collection, - user: submitter, - title: "Test Submission" - ) - end + def submission_target = fixture(:submission_target) + + def reviewer = fixture(:reviewer) + + def submitter = fixture(:submitter) + + def submission = fixture(:submission) end diff --git a/spec/support/contexts/journal_hierarchy.rb b/spec/support/contexts/journal_hierarchy.rb index 22ca676f..9addacb1 100644 --- a/spec/support/contexts/journal_hierarchy.rb +++ b/spec/support/contexts/journal_hierarchy.rb @@ -136,7 +136,7 @@ module ExampleHelpers let_it_be(:base_journal_community) do Schemas::Orderings.with_disabled_refresh do - FactoryBot.create(:community, :with_logo, :with_thumbnail, identifier: "journal-community").tap(&:reload) + FactoryBot.create(:community, :with_logo, :with_thumbnail, identifier: "journal-community", title: "Base Journal Community").tap(&:reload) end end diff --git a/spec/support/contexts/policy_scope_setup.rb b/spec/support/contexts/policy_scope_setup.rb index a9344639..5e343f34 100644 --- a/spec/support/contexts/policy_scope_setup.rb +++ b/spec/support/contexts/policy_scope_setup.rb @@ -3,7 +3,10 @@ require_relative "policy_setup" require_relative "../matchers/record_matching" -RSpec.shared_context "policy scope setup", with_known_records: true do +RSpec.shared_context "policy scope setup" do + include RecordMatching::KnownRecordHelpers + extend RecordMatching::KnownRecordHelpers::ClassMethods + let(:target) { record.class.all } let(:authorized_scope) { policy.apply_scope(target, type: :active_record_relation) } @@ -122,12 +125,60 @@ end end -RSpec.shared_context "depositing policy scope setup", policy_scope: true do +RSpec.shared_context "depositing policy scope setup" do include_context "policy scope setup" - include_context "depositing policy setup" + + shared_examples_for "a depositing admin-only scope" do + include_examples "an admin-only scope" + + context "as a reviewer" do + let(:user) { reviewer } + + include_examples "a scope that excludes known records" + end + + context "as a depositor" do + let(:user) { submitter } + + include_examples "a scope that excludes known records" + end + end + + shared_examples_for "a depositing review scope" do + include_examples "an admin-only scope" + + context "as a reviewer" do + let(:user) { reviewer } + + include_examples "a scope that includes known records" + end + + context "as a depositor" do + let(:user) { submitter } + + include_examples "a scope that excludes known records" + end + end + + shared_examples_for "a depositing scope" do + include_examples "an admin-only scope" + + context "as a reviewer" do + let(:user) { reviewer } + + include_examples "a scope that includes known records" + end + + context "as a depositor" do + let(:user) { submitter } + + include_examples "a scope that includes known records" + end + end end RSpec.configure do |config| - config.include RecordMatching::KnownRecordHelpers, policy_scope: true - config.extend RecordMatching::KnownRecordHelpers::ClassMethods, policy_scope: true + config.include_context "policy scope setup", policy_scope: true + + config.include_context "depositing policy scope setup", depositing_policy_scope: true end diff --git a/spec/support/contexts/policy_setup.rb b/spec/support/contexts/policy_setup.rb index 2dcf578c..d65d399b 100644 --- a/spec/support/contexts/policy_setup.rb +++ b/spec/support/contexts/policy_setup.rb @@ -68,7 +68,6 @@ end RSpec.shared_context "depositing policy setup" do - include_context "policy setup" include_context "depositing authorization testing" shared_examples_for "a permission granted to reviewers" do @@ -132,3 +131,9 @@ include_examples "a permission denied to submitters" end end + +RSpec.configure do |config| + config.include_context "policy setup", type: :policy + + config.include_context "depositing policy setup", depositing_policy: true +end diff --git a/spec/support/helpers/current_user_helpers.rb b/spec/support/helpers/current_user_helpers.rb index c04c186e..207c8615 100644 --- a/spec/support/helpers/current_user_helpers.rb +++ b/spec/support/helpers/current_user_helpers.rb @@ -42,23 +42,9 @@ def as_an_anonymous_user(&) end RSpec.shared_context "with current user context" do - let_it_be(:anonymous_user) { AnonymousUser.new } - - let_it_be(:admin_user, refind: true) do - FactoryBot.create :user, :admin, given_name: "Admin", family_name: "User" - end - - let_it_be(:regular_user, refind: true) do - FactoryBot.create :user, given_name: "Regular", family_name: "User" - end + include_context "all standard users" let(:current_user) { anonymous_user } - - before do - [admin_user, regular_user, current_user].uniq.each do |user| - Testing::Keycloak::GlobalRegistry.users.add_existing! user - end - end end RSpec.configure do |config| diff --git a/spec/support/shared_examples/an_entity_child_record_policy.rb b/spec/support/shared_examples/an_entity_child_record_policy.rb index 36e42bb6..fb23df19 100644 --- a/spec/support/shared_examples/an_entity_child_record_policy.rb +++ b/spec/support/shared_examples/an_entity_child_record_policy.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.shared_examples_for "an entity child record policy" do - include_context "policy setup" - let_it_be(:community, refind: true) { FactoryBot.create :community } let_it_be(:collection, refind: true) { FactoryBot.create :collection } diff --git a/spec/system/lib/factories/factory_enhancement.rb b/spec/system/lib/factories/factory_enhancement.rb new file mode 100644 index 00000000..093a7654 --- /dev/null +++ b/spec/system/lib/factories/factory_enhancement.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Testing + module Factories + module FactoryEnhancement + def create(...) + record = super + + TestingAPI::TestContainer["factories.tracker"].store!(record, caller_locations(1, 1).first.to_s) + + return record + end + end + end +end diff --git a/spec/system/lib/factories/model_enhancement.rb b/spec/system/lib/factories/model_enhancement.rb new file mode 100644 index 00000000..84f761a5 --- /dev/null +++ b/spec/system/lib/factories/model_enhancement.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Testing + module Factories + module ModelEnhancement + extend ActiveSupport::Concern + + # @return [String, nil] + def factory_bot_location + TestingAPI::TestContainer["factories.tracker"].location_for(self) + end + end + end +end diff --git a/spec/system/operations/factories/tracker.rb b/spec/system/operations/factories/tracker.rb new file mode 100644 index 00000000..dbe960be --- /dev/null +++ b/spec/system/operations/factories/tracker.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Testing + module Factories + class Tracker + def initialize + @models = Hash.new do |h, k| + h[k] = {} + end + end + + # @param [ApplicationRecord] record + # @return [String, nil] + def location_for(record) + @models[record.class.name][record.id] + end + + # @param [ApplicationRecord] record + # @return [ApplicationRecord] + def store!(record, location) + @models[record.class.name][record.id] = location + + return record + end + end + end +end diff --git a/spec/system/operations/initialize_database.rb b/spec/system/operations/initialize_database.rb index 40cd9fc5..05dc736b 100644 --- a/spec/system/operations/initialize_database.rb +++ b/spec/system/operations/initialize_database.rb @@ -3,17 +3,150 @@ module Testing # This is used to prepare the test database with certain records # that should always exist in the API. Override as necessary + # + # We need to load our fixtures in a `before(:suite)` block because + # we also use let_it_be, and let_it_be's `before_all` messes with + # the normal RSpec `before(:all)`. class InitializeDatabase + include ActiveSupport::Benchmarkable include Dry::Monads[:do, :result] + include TestProf::AnyFixture::DSL include Common::Deps[ reload_everything: "system.reload_everything", ] + delegate :logger, to: Rails + def call - yield reload_everything.call(skip_refresh: true) + benchmark "Reloading system data" do + yield reload_everything.call(skip_refresh: true) + end + + benchmark "Loading role fixtures" do + load_role_fixtures! + end + + benchmark "Loading user fixtures" do + load_user_fixtures! + end + + benchmark "Loading entity fixtures" do + load_entity_fixtures! + end + + benchmark "Loading depositing fixtures" do + load_depositing_fixtures! + end Success nil end + + private + + # @return [void] + def load_role_fixtures! + fixture(:role_admin) do + Role.fetch(:admin) + end + + fixture(:role_manager) do + Role.fetch(:manager) + end + + fixture(:role_editor) do + Role.fetch(:editor) + end + + fixture(:role_reviewer) do + Role.fetch(:reviewer) + end + + fixture(:role_depositor) do + Role.fetch(:depositor) + end + + fixture(:role_author) do + Role.fetch(:author) + end + + fixture(:role_reader) do + Role.fetch(:reader) + end + end + + # @return [void] + def load_user_fixtures! + fixture(:admin_user) do + FactoryBot.create :user, :admin, given_name: "Admin", family_name: "User", email: "admin@example.com", testing: false, created_at: 4.days.ago + end + + fixture(:regular_user) do + FactoryBot.create :user, given_name: "Test", family_name: "User", email: "test@example.com", created_at: 1.day.ago + end + end + + # @return [void] + def load_entity_fixtures! + fixture(:item_schema_version) do + FactoryBot.create(:schema_version, :item, name: "Entity Authorization Test Item Schema") + end + + fixture(:community) do + FactoryBot.create(:community, title: "Entity Authorization Test Community") + end + + fixture(:collection) do + FactoryBot.create(:collection, community: fixture(:community), title: "Entity Authorization Test Collection") + end + + fixture(:item) do + FactoryBot.create(:item, collection: fixture(:collection), title: "Entity Authorization Test Item") + end + + fixture(:community_manager) do + FactoryBot.create(:user, given_name: "Community", family_name: "Manager", manager_on: fixture(:community)) + end + + fixture(:community_editor) do + FactoryBot.create(:user, given_name: "Community", family_name: "Editor", editor_on: fixture(:community)) + end + + fixture(:community_reader) do + FactoryBot.create(:user, given_name: "Community", family_name: "Reader", reader_on: fixture(:community)) + end + + fixture(:collection_editor) do + FactoryBot.create(:user, given_name: "Collection", family_name: "Editor", editor_on: fixture(:collection)) + end + end + + # @return [void] + def load_depositing_fixtures! + fixture(:submission_target) do + fixture(:collection).fetch_submission_target!.tap do |st| + st.configure!(schema_versions: [fixture(:item_schema_version)], deposit_mode: :direct) + st.transition_to! :open + end + end + + fixture(:reviewer) do + FactoryBot.create(:user, reviewer_on: fixture(:collection)) + end + + fixture(:submitter) do + FactoryBot.create(:user, depositor_on: fixture(:collection)) + end + + fixture(:submission) do + FactoryBot.create(:submission, + submission_target: fixture(:submission_target), + schema_version: fixture(:item_schema_version), + parent_entity: fixture(:collection), + user: fixture(:submitter), + title: "Test Submission" + ) + end + end end end diff --git a/spec/system/system/providers/factory_bot_location_tracking.rb b/spec/system/system/providers/factory_bot_location_tracking.rb new file mode 100644 index 00000000..303c25fc --- /dev/null +++ b/spec/system/system/providers/factory_bot_location_tracking.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +TestingAPI::TestContainer.register_provider(:factory_bot_location_tracking) do + start do + FactoryBot.singleton_class.prepend Testing::Factories::FactoryEnhancement + FactoryBot::SyntaxRunner.prepend Testing::Factories::FactoryEnhancement + + ActiveSupport.on_load :active_record do + include Testing::Factories::ModelEnhancement + end + end +end diff --git a/spec/system/test_container.rb b/spec/system/test_container.rb index 4289fbc7..a50bb13d 100644 --- a/spec/system/test_container.rb +++ b/spec/system/test_container.rb @@ -38,4 +38,6 @@ class TestContainer < Dry::System::Container require_relative "lib/types" +TestingAPI::TestContainer.start(:factory_bot_location_tracking) + TestingAPI::TestContainer.finalize!