From 50f14c13ca570c73f8f53e24271a2a8feceec21c Mon Sep 17 00:00:00 2001 From: Zach Wolfenbarger Date: Fri, 24 May 2024 17:01:18 -0500 Subject: [PATCH] Add project_id to Aggregations to allow Doorkeeper to scope by project --- app/controllers/api/v1/aggregations_controller.rb | 6 ++++-- app/models/aggregation.rb | 3 ++- app/policies/aggregation_policy.rb | 4 ++++ app/serializers/aggregation_serializer.rb | 4 ++-- .../20240304201959_refactor_aggregation_model.rb | 4 ++++ db/structure.sql | 11 ++++++++++- .../api/v1/aggregations_controller_spec.rb | 7 ++++++- spec/factories/aggregations.rb | 1 + spec/models/aggregation_spec.rb | 8 +++++--- 9 files changed, 38 insertions(+), 10 deletions(-) diff --git a/app/controllers/api/v1/aggregations_controller.rb b/app/controllers/api/v1/aggregations_controller.rb index 32066d560..213605eb9 100644 --- a/app/controllers/api/v1/aggregations_controller.rb +++ b/app/controllers/api/v1/aggregations_controller.rb @@ -3,14 +3,16 @@ class Api::V1::AggregationsController < Api::ApiController include JsonApiController::PunditPolicy - require_authentication :index, :show, :update, :create, scopes: [:workflow] + require_authentication :index, :show, :update, :create, scopes: [:project] resource_actions :index, :show, :create, :update schema_type :json_schema def create workflow = Workflow.find(create_params['links']['workflow']) + project_id = workflow.project.id + create_params['links']['project'] = project_id response = AggregationClient.new.send_aggregation_request( - workflow.project.id, + project_id, workflow.id, create_params['links']['user'] ) diff --git a/app/models/aggregation.rb b/app/models/aggregation.rb index 0d3506a07..15e45326d 100644 --- a/app/models/aggregation.rb +++ b/app/models/aggregation.rb @@ -2,7 +2,9 @@ class Aggregation < ApplicationRecord belongs_to :workflow + belongs_to :project belongs_to :user + validates :project, presence: true validates :workflow, presence: true validates :user, presence: true validates :user_id, uniqueness: { scope: :workflow_id } @@ -15,5 +17,4 @@ class Aggregation < ApplicationRecord completed: 2, failed: 3 } - end diff --git a/app/policies/aggregation_policy.rb b/app/policies/aggregation_policy.rb index 534b47463..7582f5b78 100644 --- a/app/policies/aggregation_policy.rb +++ b/app/policies/aggregation_policy.rb @@ -12,6 +12,10 @@ def linkable_workflows policy_for(Workflow).scope_for(:update) end + def linkable_projects + policy_for(Project).scope_for(:update) + end + def linkable_users policy_for(User).scope_for(:update) end diff --git a/app/serializers/aggregation_serializer.rb b/app/serializers/aggregation_serializer.rb index 34e50d231..73db29b54 100644 --- a/app/serializers/aggregation_serializer.rb +++ b/app/serializers/aggregation_serializer.rb @@ -3,7 +3,7 @@ class AggregationSerializer include CachedSerializer attributes :id, :href, :created_at, :updated_at, :uuid, :task_id, :status - can_include :workflow, :user + can_include :project, :workflow, :user - can_filter_by :workflow + can_filter_by :project, :workflow end diff --git a/db/migrate/20240304201959_refactor_aggregation_model.rb b/db/migrate/20240304201959_refactor_aggregation_model.rb index 83567daf1..68a86aa34 100644 --- a/db/migrate/20240304201959_refactor_aggregation_model.rb +++ b/db/migrate/20240304201959_refactor_aggregation_model.rb @@ -7,6 +7,9 @@ def up safety_assured { remove_column :aggregations, :aggregation } # and the new aggregations columns + add_column :aggregations, :project_id, :integer + add_foreign_key :aggregations, :projects, column: :project_id, validate: false + add_column :aggregations, :user_id, :integer add_foreign_key :aggregations, :users, column: :user_id, validate: false @@ -20,6 +23,7 @@ def down add_column :aggregations, :aggregation, :jsonb remove_column :aggregations, :user_id + remove_column :aggregations, :project_id remove_column :aggregations, :uuid remove_column :aggregations, :task_id remove_column :aggregations, :status diff --git a/db/structure.sql b/db/structure.sql index eecf1a925..5273c031f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -98,10 +98,11 @@ CREATE TABLE public.aggregations ( workflow_id integer, created_at timestamp without time zone, updated_at timestamp without time zone, + project_id integer, user_id integer, uuid character varying, task_id character varying, - status integer + status integer DEFAULT 0 ); @@ -4209,6 +4210,14 @@ ALTER TABLE ONLY public.organization_versions ADD CONSTRAINT fk_rails_be858ed31d FOREIGN KEY (organization_id) REFERENCES public.organizations(id); +-- +-- Name: aggregations fk_rails_c7d229ada4; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.aggregations + ADD CONSTRAINT fk_rails_c7d229ada4 FOREIGN KEY (project_id) REFERENCES public.projects(id) NOT VALID; + + -- -- Name: subject_set_imports fk_rails_d596712569; Type: FK CONSTRAINT; Schema: public; Owner: - -- diff --git a/spec/controllers/api/v1/aggregations_controller_spec.rb b/spec/controllers/api/v1/aggregations_controller_spec.rb index ea58a50c5..12e957931 100644 --- a/spec/controllers/api/v1/aggregations_controller_spec.rb +++ b/spec/controllers/api/v1/aggregations_controller_spec.rb @@ -7,7 +7,7 @@ let(:api_resource_attributes) { %w[id created_at updated_at uuid task_id status] } let(:api_resource_links) { %w[aggregations.workflow] } - let(:scopes) { %w[public workflow] } + let(:scopes) { %w[project] } let(:resource_class) { Aggregation } describe '#index' do @@ -55,6 +55,11 @@ it_behaves_like 'is creatable' + it 'saves the project id' do + post :create, params: create_params + expect(Aggregation.first.project_id).to eq(workflow.project.id) + end + it 'makes a request to the aggregation service' do post :create, params: create_params expect(mock_agg).to have_received(:send_aggregation_request) diff --git a/spec/factories/aggregations.rb b/spec/factories/aggregations.rb index 16e165013..df01cb93f 100644 --- a/spec/factories/aggregations.rb +++ b/spec/factories/aggregations.rb @@ -1,6 +1,7 @@ FactoryBot.define do factory :aggregation do workflow + project user status { Aggregation.statuses[:pending] } end diff --git a/spec/models/aggregation_spec.rb b/spec/models/aggregation_spec.rb index 89635e897..ef61b0b2d 100644 --- a/spec/models/aggregation_spec.rb +++ b/spec/models/aggregation_spec.rb @@ -11,14 +11,16 @@ expect(build(:aggregation, workflow: nil)).not_to be_valid end + it 'is not be valid without a project' do + expect(build(:aggregation, project: nil)).not_to be_valid + end + it 'is not be valid without a user' do expect(build(:aggregation, user: nil)).not_to be_valid end context 'when there is a duplicate user_id workflow_id entry' do - before(:each) do - aggregation.save - end + before { aggregation.save } let(:duplicate) do build(:aggregation, workflow: aggregation.workflow, user: aggregation.user)