Skip to content

Commit

Permalink
Fix the mismatch between headers and rows
Browse files Browse the repository at this point in the history
In certain circumstances, such as when a report has forecasts but no
actual spend, there is an extra column for the data that is not matched
to a header.

The reason the extra column shows up is due to the data structures used
for the actuals and variance, which will always return a hash with keys
and values, even if the values are not meaningful (empty arrays or zero
values).

We cannot simply filter out empty arrays or zero values, because they
can be meaningful in other contexts, such as when there are some nil or
zero values in some quarters, but they are surrounded by quarters with
non-zero values, so we have to include them.

The simplest solution is to trust the logic of the headers generation,
and not attempt to compute actuals or variance for reports where the
app has already determined there are no actuals or variance to display.
  • Loading branch information
CristinaRO committed Nov 9, 2023
1 parent 4d892ba commit 692c4a0
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Add `is_oda` attribute to reports and populate it to `false` for all existing ISPF reports
- Amend report validation to permit two editable reports per fund and organisation, as long as they have different ODA types
- Amend the check for a pre-existing later report to take ODA type into account
- Fix the logic that generates actuals and variance rows for reports, to avoid inserting a spurious column for reports that have forecasts but no actuals

## Release 138 - 2023-10-27

Expand Down
6 changes: 3 additions & 3 deletions app/services/export/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ def rows
row << implementing_organisations_rows.fetch(activity.id, nil)
row << partner_organisation_rows.fetch(activity.id, nil)
row << change_state_rows.fetch(activity.id, nil)
row << actuals_rows.fetch(activity.id, nil) if actuals_rows.any?
row << variance_rows.fetch(activity.id, nil) if actuals_rows.any? && has_forecast_rows?
row << forecast_rows.fetch(activity.id, nil) if has_forecast_rows?
row << actuals_rows.fetch(activity.id, nil) if @actuals_columns.headers.any?
row << variance_rows.fetch(activity.id, nil) if @actuals_columns.headers.any? && @forecast_columns.headers.any?
row << forecast_rows.fetch(activity.id, nil) if @forecast_columns.headers.any?
row << comment_rows.fetch(activity.id, nil)
row << tags_rows.fetch(activity.id, nil) if @report.for_ispf?
row << link_rows.fetch(activity.id, nil)
Expand Down
83 changes: 83 additions & 0 deletions spec/services/export/report_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@

subject { described_class.new(report: @report_without_forecasts) }

it "returns a corresponding number of header columns and row columns" do
expect(subject.headers.size).to eql(subject.rows.first.to_a.size)
end

describe "#headers" do
it "returns the headers without Variances or Forecasts" do
headers = subject.headers
Expand Down Expand Up @@ -142,6 +146,85 @@
end
end

context "when there are activities and forecasts but no actual spend" do
before do
financial_quarter = FinancialQuarter.for_date(Date.parse("31-Mar-2022"))
financial_year = FinancialYear.for_date(Date.parse("31-Mar-2022"))

@report_without_actuals = create(
:report,
financial_quarter: financial_quarter.to_i,
financial_year: financial_year.to_i
)

@project_for_report_without_actuals = create(
:project_activity_with_implementing_organisations,
implementing_organisations_count: 2
)

@forecast =
ForecastHistory.new(
@project_for_report_without_actuals,
report: @report_without_actuals,
financial_quarter: financial_quarter.succ.to_i,
financial_year: financial_year.succ.to_i
).set_value(10_000)

@comment = create(:comment, commentable: @project_for_report_without_actuals, report: @report_without_actuals)

relation = Activity.where(id: @project_for_report_without_actuals.id)
finder_double = double(Activity::ProjectsForReportFinder, call: relation)
allow(Activity::ProjectsForReportFinder).to receive(:new).and_return(finder_double)
end

subject { described_class.new(report: @report_without_actuals) }

it "returns a corresponding number of header columns and row columns" do
expect(subject.headers.size).to eql(subject.rows.first.to_a.size)
end

describe "#headers" do
it "returns the headers without Actuals or Variance" do
headers = subject.headers

expect(headers).to include(@headers_for_report.first)
expect(headers).to include(@headers_for_report.last)
expect(headers).to include("Implementing organisations")
expect(headers).to include("Partner organisation")
expect(headers).to include("Change state")
expect(headers.to_s).to_not include("Actual net")
expect(headers.to_s).to_not include("Variance")
expect(headers).to include("Forecast #{@forecast.own_financial_quarter}")
expect(headers).to include("Forecast #{@report_without_actuals.own_financial_quarter}")
expect(headers).to include("Comments in report")
expect(headers).to include("Link to activity")
end
end

describe "#rows" do
it "returns the rows correctly" do
first_row = subject.rows.first.to_a

expect(roda_identifier_value_for_row(first_row))
.to eq(@project_for_report_without_actuals.roda_identifier)

expect(partner_organisation_value_for_row(first_row))
.to eq(@project_for_report_without_actuals.organisation.name)
expect(change_state_value_for_row(first_row))
.to eq("Unchanged")
# forecast for the report's own financial quarter
expect(first_row[@headers_for_report.length + 3])
.to eq(0)
# forecast for the next financial quarter
expect(first_row[@headers_for_report.length + 4])
.to eq(@forecast.value)
# comments
expect(first_row[@headers_for_report.length + 5])
.to eq(@comment.body)
end
end
end

context "when there are activities" do
subject { described_class.new(report: @report) }

Expand Down

0 comments on commit 692c4a0

Please sign in to comment.