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

Fix the mismatch between headers and rows #2197

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

CristinaRO
Copy link
Contributor

@CristinaRO CristinaRO commented Nov 1, 2023

Changes in this PR

  • 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

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.

Screenshots of UI changes

N/A

Next steps

  • Is an ADR required? An ADR should be added if this PR introduces a change to the architecture.
  • Is a changelog entry required? An entry should always be made in CHANGELOG.md, unless this PR is a small tweak which has no impact outside the development team.
  • Do any environment variables need amending or adding?
  • Have any changes to the XML been checked with the IATI validator? See XML Validation

Copy link
Collaborator

@shuldt shuldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this makes sense to me, I think this is a good solution

@CristinaRO CristinaRO force-pushed the spike-fix-column-header-discrepancy branch 2 times, most recently from d01f826 to 692c4a0 Compare November 9, 2023 19:03
@CristinaRO CristinaRO marked this pull request as ready for review November 9, 2023 19:03
@CristinaRO
Copy link
Contributor Author

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.
@CristinaRO CristinaRO force-pushed the spike-fix-column-header-discrepancy branch from 692c4a0 to e5ae138 Compare November 13, 2023 13:49
@CristinaRO CristinaRO merged commit 9769812 into develop Nov 13, 2023
@CristinaRO CristinaRO deleted the spike-fix-column-header-discrepancy branch November 13, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants