Skip to content

Commit 9bf29c1

Browse files
sc-17432 decouple Indicator from Target period; use ActionPlan.target… (#5742)
**Story card:** [sc-17432](https://app.shortcut.com/simpledotorg/story/17432/fix-targets-association) ### **Because** Indicators were using indicator.target.period, which caused numerator/denominator to be calculated using the wrong quarter. The ticket requires that ActionPlan’s target.period should be the source of truth. ### **This addresses…** 1. Removed the Indicator → Target link (no more indicator.target). 2. Targets are now created independently and attached only to ActionPlans. 3. ActionPlan#numerator now passes its own quarter: `indicator.numerator(region, Period.new(type: :quarter, value: target.period)) ` 4. Updated controller, specs, and factories. 5. Made dr_rai_indicators_id nullable via migration. ### **Test Instructions** Suite test
1 parent f89cd80 commit 9bf29c1

File tree

10 files changed

+25
-27
lines changed

10 files changed

+25
-27
lines changed

app/controllers/dr_rai/action_plans_controller.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ def hydrate_plan
6363
@target = DrRai::Target.create!(
6464
type: dr_rai_action_plan_params[:target_type],
6565
period: period,
66-
indicator: @indicator,
6766
numeric_value: dr_rai_action_plan_params[:target_value]
6867
)
6968
end

app/models/dr_rai/action_plan.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ def target
1616
end
1717

1818
def numerator
19-
indicator.numerator(region)
19+
target_period = Period.new(type: :quarter, value: target.period)
20+
indicator.numerator(region, target_period)
2021
end
2122

2223
def denominator

app/models/dr_rai/indicator.rb

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,11 @@ class DrRai::Indicator < ApplicationRecord
1717
# There should only ever be one instance of any indicator in the db
1818
validates :type, uniqueness: true
1919

20-
has_one :target, class_name: "DrRai::Target", dependent: :destroy, foreign_key: "dr_rai_indicators_id"
21-
accepts_nested_attributes_for :target
22-
2320
def quarterlies(region)
2421
data = Reports::RegionSummary.call(region, range: DEFAULT_RANGE)
2522
Reports::RegionSummaryAggregator.new(data).quarterly(with: :sum)[region.slug]
2623
end
2724

28-
def period
29-
Period.new(type: :quarter, value: target.period)
30-
end
31-
3225
def has_action_plans?(region, period)
3326
raise "Must use a quarter period" unless period.type == :quarter
3427
raise "Must use a Region type" unless region.is_a? Region
@@ -51,26 +44,30 @@ def target_type
5144
DrRai::Target::TYPES[target_type_frontend]
5245
end
5346

54-
def numerator(region, the_period = period, with_non_contactable: nil)
47+
def numerator(region, the_period, with_non_contactable: nil)
5548
return 0 unless is_supported?(region)
5649
numerators(region, all: with_non_contactable)[the_period]
5750
end
5851

59-
def denominator(region, the_period = period, with_non_contactable: nil)
52+
def denominator(region, the_period, with_non_contactable: nil)
6053
return 0 unless is_supported?(region)
6154
denominators(region, all: with_non_contactable)[the_period]
6255
end
6356

6457
def numerators(region, all: nil)
65-
return [] unless is_supported?(region)
66-
datasource(region).map do |t, data|
58+
return {} unless is_supported?(region)
59+
data_source = datasource(region)
60+
return {} if data_source.nil?
61+
data_source.map do |t, data|
6762
[t, data[numerator_key(all: all)]]
6863
end.to_h
6964
end
7065

7166
def denominators(region, all: nil)
72-
return [] unless is_supported?(region)
73-
datasource(region).map do |t, data|
67+
return {} unless is_supported?(region)
68+
data_source = datasource(region)
69+
return {} if data_source.nil?
70+
data_source.map do |t, data|
7471
[t, data[denominator_key(all: all)]]
7572
end.to_h
7673
end

app/models/dr_rai/target.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,5 @@ class DrRai::Target < ApplicationRecord
55
"boolean" => "DrRai::BooleanTarget"
66
}
77

8-
belongs_to :indicator, class_name: "DrRai::Indicator", foreign_key: "dr_rai_indicators_id"
9-
108
validates :period, presence: true, format: {with: /\AQ\d-\d{4}\Z/}
119
end
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class MakeDrRaiIndicatorsIdNullableInDrRaiTargets < ActiveRecord::Migration[6.1]
2+
def change
3+
change_column_null :dr_rai_targets, :dr_rai_indicators_id, true
4+
end
5+
end

db/structure.sql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,7 +1254,7 @@ CREATE TABLE public.dr_rai_targets (
12541254
deleted_at timestamp without time zone,
12551255
created_at timestamp(6) without time zone NOT NULL,
12561256
updated_at timestamp(6) without time zone NOT NULL,
1257-
dr_rai_indicators_id bigint NOT NULL,
1257+
dr_rai_indicators_id bigint,
12581258
period character varying
12591259
);
12601260

@@ -8596,6 +8596,6 @@ INSERT INTO "schema_migrations" (version) VALUES
85968596
('20250924101441'),
85978597
('20250924102156'),
85988598
('20250925094123'),
8599-
('20251125090819');
8600-
8599+
('20251125090819'),
8600+
('20251211073126');
86018601

spec/factories/dr_rai.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
factory :target, class: "DrRai::Target" do
33
completed { false }
44
period { "Q1-2021" }
5-
indicator { contact_overdue_patients_indicator }
65

76
trait :numeric do
87
type { "DrRai::NumericTarget" }

spec/models/dr_rai/action_plan_spec.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
describe "#denominator" do
55
context "for numeric targets" do
66
let(:indicator) { DrRai::ContactOverduePatientsIndicator.create }
7-
let(:target) { DrRai::NumericTarget.create(indicator: indicator, numeric_value: 20) }
7+
let(:target) { DrRai::NumericTarget.create(numeric_value: 20, period: "Q1-2025") }
88
let(:district_with_facilities) { setup_district_with_facilities }
99
let(:region) { district_with_facilities[:region] }
1010

@@ -20,12 +20,12 @@
2020

2121
describe "#progress" do
2222
let(:indicator) { DrRai::ContactOverduePatientsIndicator.create }
23-
let(:target) { DrRai::NumericTarget.create(indicator: indicator, numeric_value: 20, period: "Q1-2025") }
23+
let(:target) { DrRai::NumericTarget.create(numeric_value: 20, period: "Q1-2025") }
2424
let(:district_with_facilities) { setup_district_with_facilities }
2525
let(:region) { district_with_facilities[:region] }
2626

2727
before do
28-
allow_any_instance_of(DrRai::ContactOverduePatientsIndicator).to receive(:numerator).with(anything).and_return(9)
28+
allow_any_instance_of(DrRai::ContactOverduePatientsIndicator).to receive(:numerator).with(anything, anything).and_return(9)
2929
end
3030

3131
it "calculates pecentage to 2 decimal places" do

spec/models/dr_rai/indicator_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
create(:action_plan,
2727
region: region,
2828
dr_rai_indicator: the_indicator,
29-
dr_rai_target: create(:target, :percentage, indicator: the_indicator, period: this_period.value.to_s))
29+
dr_rai_target: create(:target, :percentage, period: this_period.value.to_s))
3030
end
3131

3232
context "in that period" do

spec/models/dr_rai/target_spec.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@
2727

2828
it "Q1-2023 is a valid period format" do
2929
target = DrRai::Target.new period: "Q1-2025"
30-
expect(target).not_to be_valid
31-
expect(target.errors.include?(:period)).to be_falsey
30+
expect(target).to be_valid
3231
end
3332
end
3433
end

0 commit comments

Comments
 (0)