Skip to content

Commit

Permalink
In testing the claim form we concluded that we cannot edit the same
Browse files Browse the repository at this point in the history
claim record on the check page when creating a claim.

Issues were found when the user would click the back buttons to go back
to the check page after completing half their changes. The changes they
did were being saved. Even if they backed off from doing them.

To solve this issue and many others, we are implementing a revision
system on the claim.

Creating a revision every time a user wants to edit a claim from the
check page. If they back off half way through their changes we revert
back to the last valid claim revision.

This implementation will increase the audit records and possibly pollute
some of them with `system changes` i.e. changes to `previous_revision_id`
or `next_revision_id` which the user doesn't care about.

For the draft claim

This should fix the issue where a support user edits a draft claim and
with every edit, the draft claim is updated for everyone.

This will only `update` the draft claim when the support user actually
clicks `Update claim`.

They way this works is by using the `internal_draft` status. A generated
revision starts with `internal_draft` as its status and is changed to
draft when the support user finishes editing the draft claim. The
previous revision is then set to `internal_draft`, to not show more than
1 claim per reference.
  • Loading branch information
CatalinVoineag committed May 2, 2024
1 parent e0ffcf2 commit 6952488
Show file tree
Hide file tree
Showing 37 changed files with 670 additions and 241 deletions.
1 change: 1 addition & 0 deletions app/assets/stylesheets/application.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ $govuk-assets-path: "";
@import "filter-form";
@import "app-related-aside";
@import "lists";
@import "buttons";
12 changes: 12 additions & 0 deletions app/assets/stylesheets/buttons.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
button.govuk-link {
all: unset;
}

button.govuk-link {
font-family: "GDS Transport", arial, sans-serif;
-webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale;
text-decoration: underline;
text-decoration-thickness: max(1px, .0625rem);
text-underline-offset: 0.1578em;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@ class Claims::Schools::Claims::MentorTrainingsController < Claims::ApplicationCo
before_action :authorize_claim
helper_method :mentor_training_form

def create_revision
revision = Claims::Claim::CreateRevision.call(claim: @claim)
mentor_training = revision.mentor_trainings.find_by(mentor_id: params.require(:id))

redirect_to edit_claims_school_claim_mentor_training_path(
@school,
revision,
mentor_training,
)
end

def edit; end

def update
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/claims/schools/claims/mentors_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ def create
end
end

def create_revision
revision = Claims::Claim::CreateRevision.call(claim: @claim)
redirect_to edit_claims_school_claim_mentors_path(@school, revision)
end

def edit; end

def update
Expand Down
14 changes: 13 additions & 1 deletion app/controllers/claims/schools/claims_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ class Claims::Schools::ClaimsController < Claims::ApplicationController
include Claims::BelongsToSchool

before_action :has_school_accepted_grant_conditions?
before_action :set_claim, only: %i[show check confirmation submit edit update rejected]
before_action :set_claim, only: %i[show check confirmation submit edit update rejected create_revision]
before_action :authorize_claim
before_action :get_valid_revision, only: :check

helper_method :claim_provider_form

Expand All @@ -21,6 +22,11 @@ def create
end
end

def create_revision
revision = Claims::Claim::CreateRevision.call(claim: @claim)
redirect_to edit_claims_school_claim_path(@school, revision)
end

def show; end

def check
Expand Down Expand Up @@ -88,4 +94,10 @@ def claim_id
def authorize_claim
authorize @claim || Claims::Claim
end

def get_valid_revision
revision = @claim.get_valid_revision

redirect_to check_claims_school_claim_path(@school, revision) if revision != @claim
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@ class Claims::Support::Schools::Claims::MentorTrainingsController < Claims::Appl
before_action :authorize_claim
helper_method :mentor_training_form

def create_revision
revision = Claims::Claim::CreateRevision.call(claim: @claim)
mentor_training = revision.mentor_trainings.find_by(mentor_id: params.require(:id))

redirect_to edit_claims_support_school_claim_mentor_training_path(
@school,
revision,
mentor_training,
)
end

def edit; end

def update
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ def create
end
end

def create_revision
revision = Claims::Claim::CreateRevision.call(claim: @claim)
redirect_to edit_claims_support_school_claim_mentors_path(@school, revision)
end

def edit; end

def update
Expand Down
25 changes: 22 additions & 3 deletions app/controllers/claims/support/schools/claims_controller.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
class Claims::Support::Schools::ClaimsController < Claims::Support::ApplicationController
include Claims::BelongsToSchool

before_action :set_claim, only: %i[check draft show edit update remove destroy rejected]
before_action :set_claim, only: %i[check draft show edit update remove destroy rejected create_revision]
before_action :authorize_claim
before_action :get_valid_revision, only: :check

helper_method :claim_provider_form

def index
Expand All @@ -29,6 +31,11 @@ def create
end
end

def create_revision
revision = Claims::Claim::CreateRevision.call(claim: @claim)
redirect_to edit_claims_support_school_claim_path(@school, revision)
end

def check
last_mentor_training = @claim.mentor_trainings.order_by_mentor_full_name.last

Expand Down Expand Up @@ -56,8 +63,14 @@ def update
def draft
return redirect_to rejected_claims_support_school_claim_path unless @claim.valid_mentor_training_hours?

success_message = @claim.draft? ? t(".update_success") : t(".add_success")
Claims::Claim::CreateDraft.call(claim: @claim)
claim_was_draft = @claim.was_draft?
success_message = claim_was_draft ? t(".update_success") : t(".add_success")

if claim_was_draft
Claims::Claim::UpdateDraft.call(claim: @claim)
else
Claims::Claim::CreateDraft.call(claim: @claim)
end

redirect_to claims_support_school_claims_path(@school), flash: { success: success_message }
end
Expand Down Expand Up @@ -96,4 +109,10 @@ def claim_provider_form
def authorize_claim
authorize @claim || Claims::Claim
end

def get_valid_revision
revision = @claim.get_valid_revision

redirect_to check_claims_support_school_claim_path(@school, revision) if revision != @claim
end
end
2 changes: 1 addition & 1 deletion app/forms/claims/claim/mentor_training_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def persist

def back_path
if mentor_trainings.index(mentor_training).zero?
edit_claims_school_claim_mentor_path(school, claim, mentor_training.mentor_id)
edit_claims_school_claim_mentors_path(school, claim)
else
edit_claims_school_claim_mentor_training_path(school, claim, previous_mentor_training)
end
Expand Down
2 changes: 1 addition & 1 deletion app/forms/claims/support/claim/mentor_training_form.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class Claims::Support::Claim::MentorTrainingForm < Claims::Claim::MentorTrainingForm
def back_path
if mentor_trainings.index(mentor_training).zero?
edit_claims_support_school_claim_mentor_path(school, claim, mentor_training.mentor_id)
edit_claims_support_school_claim_mentors_path(school, claim)
else
edit_claims_support_school_claim_mentor_training_path(school, claim, previous_mentor_training)
end
Expand Down
78 changes: 56 additions & 22 deletions app/models/claims/claim.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,28 @@
#
# Table name: claims
#
# id :uuid not null, primary key
# created_by_type :string
# reference :string
# status :enum
# submitted_at :datetime
# submitted_by_type :string
# created_at :datetime not null
# updated_at :datetime not null
# created_by_id :uuid
# provider_id :uuid
# school_id :uuid not null
# submitted_by_id :uuid
# id :uuid not null, primary key
# created_by_type :string
# reference :string
# status :enum
# submitted_at :datetime
# submitted_by_type :string
# created_at :datetime not null
# updated_at :datetime not null
# created_by_id :uuid
# previous_revision_id :uuid
# provider_id :uuid
# school_id :uuid not null
# submitted_by_id :uuid
#
# Indexes
#
# index_claims_on_created_by (created_by_type,created_by_id)
# index_claims_on_provider_id (provider_id)
# index_claims_on_reference (reference) UNIQUE
# index_claims_on_school_id (school_id)
# index_claims_on_submitted_by (submitted_by_type,submitted_by_id)
# index_claims_on_created_by (created_by_type,created_by_id)
# index_claims_on_previous_revision_id (previous_revision_id)
# index_claims_on_provider_id (provider_id)
# index_claims_on_reference (reference)
# index_claims_on_school_id (school_id)
# index_claims_on_submitted_by (submitted_by_type,submitted_by_id)
#
# Foreign Keys
#
Expand All @@ -41,8 +43,15 @@ class Claims::Claim < ApplicationRecord
has_many :mentor_trainings, dependent: :destroy
has_many :mentors, through: :mentor_trainings

belongs_to :previous_revision, class_name: "Claims::Claim", optional: true

validates :status, presence: true
validates :reference, uniqueness: { case_sensitive: false }, allow_nil: true
validates(
:reference,
uniqueness: { case_sensitive: false },
allow_nil: true,
unless: :has_revision?,
)

ACTIVE_STATUSES = %i[draft submitted].freeze

Expand All @@ -58,10 +67,11 @@ class Claims::Claim < ApplicationRecord
delegate :full_name, to: :submitted_by, prefix: true, allow_nil: true

def valid_mentor_training_hours?
mentor_trainings_without_current_claim = Claims::MentorTraining.joins(:claim).merge(Claims::Claim.active).where(
mentor_id: mentor_trainings.select(:mentor_id),
provider_id:,
).where.not(claim_id: id)
mentor_trainings_without_current_claim = Claims::MentorTraining.joins(:claim)
.merge(Claims::Claim.active).where(
mentor_id: mentor_trainings.select(:mentor_id),
provider_id:,
).where.not(claim_id: [id, previous_revision_id])
grouped_trainings = [*mentor_trainings, *mentor_trainings_without_current_claim].group_by { [_1.mentor_id, _1.provider_id] }

grouped_trainings.transform_values { _1.sum(&:hours_completed) }.values.all? { _1 <= MAXIMUM_CLAIMABLE_HOURS }
Expand All @@ -78,4 +88,28 @@ def amount
def active?
ACTIVE_STATUSES.include?(status.to_sym)
end

def ready_to_be_checked?
mentors.present? && mentor_trainings.without_hours.blank?
end

def get_valid_revision
Claims::Claim::RemoveEmptyMentorTrainingHours.call(claim: self)

mentor_trainings.present? ? self : previous_revision
end

def was_draft?
claim_record = self
claim_record = claim_record.previous_revision while claim_record.present? && !claim_record.draft?

claim_record.nil? ? false : claim_record.draft?
end

private

def has_revision?
previous_revision_id.present? ||
id.present? && Claims::Claim.find_by(previous_revision_id: id).present?
end
end
4 changes: 4 additions & 0 deletions app/policies/claims/claim_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ def update?
edit?
end

def create_revision?
edit?
end

def submit?
!user.support_user? && !record.submitted?
end
Expand Down
26 changes: 26 additions & 0 deletions app/services/claims/claim/create_revision.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
class Claims::Claim::CreateRevision
include ServicePattern

def initialize(claim:)
@claim = claim
end

def call
revision_record = deep_dup
revision_record.save!

revision_record
end

private

attr_reader :claim

def deep_dup
dup_record = claim.dup
dup_record.mentor_trainings = claim.mentor_trainings.map(&:dup)
dup_record.previous_revision_id = claim.id
dup_record.status = :internal_draft
dup_record
end
end
28 changes: 28 additions & 0 deletions app/services/claims/claim/update_draft.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
class Claims::Claim::UpdateDraft < Claims::Claim::CreateDraft
include ServicePattern

def call
ActiveRecord::Base.transaction do
updated_claim.save!
update_previous_revisions_to_internal_draft
end
end

private

def update_previous_revisions_to_internal_draft
claim_record = updated_claim.previous_revision

while claim_record.present?
claim_record.update!(status: :internal_draft) if claim_record.draft?
claim_record = claim_record.previous_revision
end
end

def updated_claim
@updated_claim ||= begin
claim.status = :draft
claim
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def initialize(mentor:, provider:, claim: nil, mentor_training: nil)

def call
MAXIMUM_CLAIMABLE_HOURS_PER_PROVIDER -
Claims::Mentor::CalculateTotalMentorTrainingHoursForProvider.call(mentor:, provider:) +
Claims::Mentor::CalculateTotalMentorTrainingHoursForProvider.call(mentor:, provider:, claim: mentor_training&.claim) +
current_mentor_training_hours
end

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
class Claims::Mentor::CalculateTotalMentorTrainingHoursForProvider
include ServicePattern

def initialize(mentor:, provider:)
def initialize(mentor:, provider:, claim: nil)
@mentor = mentor
@provider = provider
@claim = claim
end

def call
mentor.mentor_trainings.joins(:claim).merge(Claims::Claim.active).where(provider:).sum(:hours_completed)
mentor.mentor_trainings.joins(:claim).merge(Claims::Claim.active).where(provider:).where.not(claim_id: claim&.previous_revision_id).sum(:hours_completed)
end

private

attr_reader :mentor, :provider
attr_reader :mentor, :provider, :claim
end
Loading

0 comments on commit 6952488

Please sign in to comment.