Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM457-2149: Don't override adjustment comments when removing all uplifts #805

Merged
merged 4 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions app/forms/base_adjustment_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ class BaseAdjustmentForm

private

def process_field(value:, field:, comment_field: 'adjustment_comment')
COMMENT_FIELD = 'adjustment_comment'.freeze

def process_field(value:, field:)
selected_record[self.class::COMMENT_FIELD] = explanation

return if selected_record[field] == value

# does this belong in the Event object as that is where it is
Expand All @@ -25,7 +29,7 @@ def process_field(value:, field:, comment_field: 'adjustment_comment')
}.merge(changed_value(value, selected_record[field]))

ensure_original_field_value_set(field)
assign_new_attributes(field, value, comment_field)
assign_new_attributes(field, value)

Event::Edit.build(submission:, details:, linked:, current_user:)
end
Expand All @@ -34,9 +38,8 @@ def ensure_original_field_value_set(field)
selected_record["#{field}_original"] ||= selected_record[field]
end

def assign_new_attributes(field, value, comment_field)
def assign_new_attributes(field, value)
selected_record[field] = value
selected_record[comment_field] = explanation
end

def changed_value(val1, val2)
Expand All @@ -57,7 +60,7 @@ def linked_id(row)
end

def data_changed
return if data_has_changed?
return if data_has_changed? || explanation_has_changed?

errors.add(no_change_field, :no_change)
end
Expand All @@ -70,6 +73,12 @@ def explanation_required?
data_has_changed?
end

def explanation_has_changed?
return false if selected_record[self.class::COMMENT_FIELD].blank?

explanation != selected_record[self.class::COMMENT_FIELD]
end

# :nocov:
def data_has_changed?
raise 'implement in class'
Expand Down
9 changes: 8 additions & 1 deletion app/forms/nsm/uplift/base_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ def save

Claim.transaction do
claim.data[self.class::SCOPE].each do |selected_record|
row = self.class::Remover.new(claim:, explanation:, current_user:, selected_record:)
row = self.class::Remover.new(claim: claim,
explanation: explanation_for(selected_record),
current_user: current_user,
selected_record: selected_record)
next unless row.valid?

row.save
Expand All @@ -34,6 +37,10 @@ def save
rescue StandardError
false
end

def explanation_for(record)
[record['adjustment_comment'].presence, explanation].compact.join("\n\n")
end
end
end
end
8 changes: 4 additions & 4 deletions app/forms/prior_authority/travel_cost_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ def save

private

def process_fields
comment_field = 'travel_adjustment_comment'
COMMENT_FIELD = 'travel_adjustment_comment'.freeze

process_field(value: travel_time.to_i, field: 'travel_time', comment_field: comment_field)
process_field(value: travel_cost_per_hour.to_s, field: 'travel_cost_per_hour', comment_field: comment_field)
def process_fields
process_field(value: travel_time.to_i, field: 'travel_time')
process_field(value: travel_cost_per_hour.to_s, field: 'travel_cost_per_hour')
end

def selected_record
Expand Down
17 changes: 17 additions & 0 deletions spec/forms/nsm/uplift/base_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@
end
end

context 'when the record has already been adjusted' do
before do
claim.data['letters_and_calls'][0]['adjustment_comment'] = 'Previous comment'
end

it 'preserves the previous comment' do
subject.save
expect(implementation_class::Remover).to have_received(:new)
.with(
claim: claim,
explanation: "Previous comment\n\n#{explanation}",
current_user: current_user,
selected_record: claim.data['letters_and_calls'][0]
)
end
end

context 'when invalid' do
let(:explanation) { nil }

Expand Down
43 changes: 41 additions & 2 deletions spec/forms/nsm/work_item_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,25 @@
end
end

context 'and explanation does not have an error' do
context 'and explanation would otherwise have an error' do
let(:explanation) { '' }

it 'is not valid' do
it 'ignores the explanation error' do
expect(subject).not_to be_valid
expect(subject.errors.of_kind?(:explanation, :blank)).to be(false)
end
end

context 'but the explanation has' do
before do
work_item = claim.data['work_items'].detect { _1['id'] == id }
work_item['adjustment_comment'] = 'Previous explanation'
end

it 'is valid' do
expect(subject).to be_valid
end
end
end
end

Expand Down Expand Up @@ -203,6 +214,34 @@
end
end

context 'when only explanation has changed' do
let(:uplift) { 'no' }
let(:time_spent) { 161 }

before do
work_item = claim.data['work_items'].detect { _1['id'] == id }
work_item['adjustment_comment'] = 'Previous explanation'
claim.save!
end

it 'updates the JSON data' do
subject.save
work_item = claim.reload
.data['work_items']
.detect { |row| row['work_type'] == 'waiting' }
expect(work_item).to eq(
'id' => 'cf5e303e-98dd-4b0f-97ea-3560c4c5f137',
'time_spent' => 161,
'pricing' => 24.0,
'work_type' => 'waiting',
'uplift' => 95,
'fee_earner' => 'aaa',
'completed_on' => '2022-12-12',
'adjustment_comment' => 'change to work items',
)
end
end

context 'when claim has legacy translations and work type value has changed' do
let(:claim) { create :claim, :legacy_translations }
let(:work_type_value) { 'travel' }
Expand Down