From 1f9c6aa64c717f30af07769e783c1b3b305e8a02 Mon Sep 17 00:00:00 2001 From: Rob Sterner Date: Sat, 14 Dec 2024 12:30:42 -0500 Subject: [PATCH 1/5] chore: remove .env file requirement locally, set env vars explicitly --- docker-compose.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index c45f0a2c..aeac4201 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -5,7 +5,10 @@ services: - 5432:5432 volumes: - data:/var/lib/postgresql/data - env_file: .env + environment: + - POSTGRES_PASSWORD=${POSTGRES_PASSWORD} + - DB_HOST=${DB_HOST} + - DB_USERNAME=${DB_USERNAME} container_name: staffplan-postgres redis: @@ -14,7 +17,10 @@ services: - 6379:6379 volumes: - redis:/var/lib/redis/data - env_file: .env + environment: + - POSTGRES_PASSWORD=${POSTGRES_PASSWORD} + - DB_HOST=${DB_HOST} + - DB_USERNAME=${DB_USERNAME} container_name: staffplan-redis # localstack: From 2111c48589ad6d65af0b4abb90b1ea5a52157bc4 Mon Sep 17 00:00:00 2001 From: Rob Sterner Date: Sat, 14 Dec 2024 12:34:46 -0500 Subject: [PATCH 2/5] fix: not sure how this logic slipped in, but it was reversed tl;dr: if the current date's year is greater than the work week's year then actual hours are allowed (since the work week is in the past) --- app/models/work_week.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/work_week.rb b/app/models/work_week.rb index 0be03615..5bc0cbe7 100644 --- a/app/models/work_week.rb +++ b/app/models/work_week.rb @@ -39,6 +39,6 @@ def actual_hours_allowed? return false if year_zero? || cweek_zero? today = Date.today - today.year < year || (today.year == year && today.cweek >= cweek) + today.year > year || (today.year == year && today.cweek >= cweek) end end From 00aeefe7931ee7a9e45ab31c07e9efbd11ae4d2e Mon Sep 17 00:00:00 2001 From: Rob Sterner Date: Sat, 14 Dec 2024 13:10:51 -0500 Subject: [PATCH 3/5] feat: add validation for project deletion --- app/models/project.rb | 13 +++++++++++++ spec/models/project_spec.rb | 31 +++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/app/models/project.rb b/app/models/project.rb index 23fab703..aa761a13 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -35,6 +35,8 @@ class Project < ApplicationRecord validates :rate_type, inclusion: { in: VALID_RATE_TYPES }, allow_blank: true validates :hours, numericality: { greater_than_or_equal_to: 0 }, allow_blank: true + before_destroy :ensure_project_is_destroyable, prepend: true + def confirmed? status == CONFIRMED end @@ -54,4 +56,15 @@ def cancelled? def completed? status == COMPLETED end + + private + + # a project is destroyable if it has no assignments OR if its assignments have no actual hours recorded + def ensure_project_is_destroyable + return if assignments.empty? || + WorkWeek.where(assignment: assignments).where.not(actual_hours: 0).empty? + + errors.add(:base, "Cannot delete a project that has assignments with hours recorded. Try archiving the project instead.") + false + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index cd9bb2b1..1492135c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -20,6 +20,37 @@ it { should have_many(:users).through(:assignments) } end + describe 'callbacks' do + describe 'before_destroy: ensure_project_is_destroyable' do + it 'allows project deletion if there are no assignments' do + project = create(:project) + + expect { project.destroy }.to change { Project.count }.by(-1) + expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'allows project deletion if there are assignments with assignees, but no actual hours' do + project = create(:project) + user = create(:membership, company: project.client.company).user + assignment = create(:assignment, project: project, user: user) + create(:work_week, assignment: assignment, actual_hours: 0) + + expect { project.destroy }.to change { Project.count }.by(-1) + expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'blocks project deletion if there are assignments with actual hours' do + project = create(:project) + user = create(:membership, company: project.client.company).user + assignment = create(:assignment, project: project, user: user) + create(:work_week, assignment: assignment, actual_hours: 8) + + expect { project.destroy }.to_not change { Project.count } + expect(project.errors.full_messages).to include("Cannot delete a project that has assignments with hours recorded. Try archiving the project instead.") + end + end + end + describe "#confirmed?" do it "returns true if the status is confirmed" do project = build(:project, status: "confirmed") From 2debbb1c11fdca0009a573cd5902e67b3be4a787 Mon Sep 17 00:00:00 2001 From: Rob Sterner Date: Sat, 14 Dec 2024 13:24:34 -0500 Subject: [PATCH 4/5] feat: add deleteProject mutation + tests --- app/graphql/mutations/delete_project.rb | 33 +++++++++ app/graphql/schema.graphql | 11 +++ app/graphql/types/mutation_type.rb | 1 + spec/graphql/mutations/delete_project_spec.rb | 74 +++++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 app/graphql/mutations/delete_project.rb create mode 100644 spec/graphql/mutations/delete_project_spec.rb diff --git a/app/graphql/mutations/delete_project.rb b/app/graphql/mutations/delete_project.rb new file mode 100644 index 00000000..2a1f5a79 --- /dev/null +++ b/app/graphql/mutations/delete_project.rb @@ -0,0 +1,33 @@ +module Mutations + class DeleteProject < BaseMutation + description "Delete a project." + + # arguments passed to the `resolve` method + argument :project_id, ID, required: true, + description: "The ID of the project to delete. The project must meet the delete-ability requirements: no assignments, or all assignments having no actual hours recorded." + + # return type from the mutation + type Types::StaffPlan::ProjectType + + def resolve(project_id:) + project = context[:current_company].projects.find(project_id) + + project.destroy + + if project.errors.any? + project.errors.each do |error| + context.add_error( + GraphQL::ExecutionError.new( + error.full_message, + extensions: { + attribute: error.attribute.to_s, + } + ) + ) + end + end + + project + end + end +end diff --git a/app/graphql/schema.graphql b/app/graphql/schema.graphql index e5a281bd..540c45d8 100644 --- a/app/graphql/schema.graphql +++ b/app/graphql/schema.graphql @@ -154,6 +154,17 @@ type Mutation { """ assignmentId: ID! ): Assignment! + + """ + Delete a project. + """ + deleteProject( + """ + The ID of the project to delete. The project must meet the delete-ability + requirements: no assignments, or all assignments having no actual hours recorded. + """ + projectId: ID! + ): Project! setCurrentCompany( """ The ID of the company to set as the current user's current_company_id. User must be an active member of the company. diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 9c582990..c372b98b 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -9,5 +9,6 @@ class MutationType < Types::BaseObject field :upsert_project, mutation: Mutations::UpsertProject field :upsert_client, mutation: Mutations::UpsertClient field :delete_assignment, mutation: Mutations::DeleteAssignment + field :delete_project, mutation: Mutations::DeleteProject end end diff --git a/spec/graphql/mutations/delete_project_spec.rb b/spec/graphql/mutations/delete_project_spec.rb new file mode 100644 index 00000000..af4287ce --- /dev/null +++ b/spec/graphql/mutations/delete_project_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Mutations::DeleteProject do + + context "resolve" do + it 'resolves to an error when a project cannot be destroyed' do + query_string = <<-GRAPHQL + mutation($projectId: ID!) { + deleteProject(projectId: $projectId) { + id + status + } + } + GRAPHQL + + project = create(:project) + company = project.client.company + user = create(:membership, company:).user + assignment = create(:assignment, project: project, user: user) + create(:work_week, assignment: assignment, actual_hours: 5) + + result = StaffplanReduxSchema.execute( + query_string, + context: { + current_user: user, + current_company: company + }, + variables: { + projectId: project.id + } + ) + + expect(result["errors"].first["message"]).to eq("Cannot delete a project that has assignments with hours recorded. Try archiving the project instead.") + expect(project.reload.persisted?).to eq(true) + post_result = result["data"]["deleteProject"] + expect(post_result["id"]).to eq(project.id.to_s) + expect(post_result["status"]).to eq(Project::CONFIRMED) + end + + it "destroys a project that can be destroyed" do + query_string = <<-GRAPHQL + mutation($projectId: ID!) { + deleteProject(projectId: $projectId) { + id + } + } + GRAPHQL + + project = create(:project) + company = project.client.company + user = create(:membership, company:).user + assignment = create(:assignment, project: project, user: user) + create(:work_week, assignment: assignment, actual_hours: 0) + + result = StaffplanReduxSchema.execute( + query_string, + context: { + current_user: user, + current_company: company + }, + variables: { + projectId: project.id + } + ) + + post_result = result["data"]["deleteProject"] + expect(result["errors"]).to be_nil + expect(post_result["id"]).to eq(project.id.to_s) + expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end +end From 622bc164089ddc5165830a7f9d60381c80165a2b Mon Sep 17 00:00:00 2001 From: Rob Sterner Date: Sat, 14 Dec 2024 13:29:06 -0500 Subject: [PATCH 5/5] feat: add Project#canBeDeleted --- app/graphql/schema.graphql | 5 +++++ app/graphql/types/staff_plan/project_type.rb | 6 +++++- app/models/project.rb | 8 ++++++-- spec/models/project_spec.rb | 20 ++++++++++++++++++++ 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/app/graphql/schema.graphql b/app/graphql/schema.graphql index 540c45d8..6fc3c048 100644 --- a/app/graphql/schema.graphql +++ b/app/graphql/schema.graphql @@ -355,6 +355,11 @@ type Mutation { type Project { assignments: [Assignment!]! + + """ + Whether the assignment can be deleted + """ + canBeDeleted: Boolean! client: Client! cost: Float! createdAt: ISO8601DateTime! diff --git a/app/graphql/types/staff_plan/project_type.rb b/app/graphql/types/staff_plan/project_type.rb index 1325253b..49d0ca92 100644 --- a/app/graphql/types/staff_plan/project_type.rb +++ b/app/graphql/types/staff_plan/project_type.rb @@ -17,7 +17,7 @@ class ProjectType < Types::BaseObject field :ends_on, GraphQL::Types::ISO8601Date, null: true field :created_at, GraphQL::Types::ISO8601DateTime, null: false field :updated_at, GraphQL::Types::ISO8601DateTime, null: false - + field :can_be_deleted, Boolean, null: false, description: 'Whether the assignment can be deleted' field :assignments, [Types::StaffPlan::AssignmentType], null: false def assigmnents @@ -35,6 +35,10 @@ def users def work_weeks object.work_weeks end + + def can_be_deleted + object.can_be_deleted? + end end end end diff --git a/app/models/project.rb b/app/models/project.rb index aa761a13..36e86dfc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -57,12 +57,16 @@ def completed? status == COMPLETED end + def can_be_deleted? + assignments.empty? || + WorkWeek.where(assignment: assignments).where.not(actual_hours: 0).empty? + end + private # a project is destroyable if it has no assignments OR if its assignments have no actual hours recorded def ensure_project_is_destroyable - return if assignments.empty? || - WorkWeek.where(assignment: assignments).where.not(actual_hours: 0).empty? + return if can_be_deleted? errors.add(:base, "Cannot delete a project that has assignments with hours recorded. Try archiving the project instead.") false diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1492135c..f8eed554 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -51,6 +51,26 @@ end end + describe 'can_be_deleted?' do + it 'allows deletion if there are no assignments with actual hours recorded' do + project = create(:project) + user = create(:membership, company: project.client.company).user + assignment = create(:assignment, project: project, user: user) + create(:work_week, assignment: assignment, actual_hours: 0) + + expect(project.can_be_deleted?).to be_truthy + end + + it 'blocks deletion if there are assignments with actual hours recorded' do + project = create(:project) + user = create(:membership, company: project.client.company).user + assignment = create(:assignment, project: project, user: user) + create(:work_week, assignment: assignment, actual_hours: 8) + + expect(project.can_be_deleted?).to be_falsey + end + end + describe "#confirmed?" do it "returns true if the status is confirmed" do project = build(:project, status: "confirmed")