Skip to content

FINERACT-2711: Fix shared-account template named-range bounds built by string concat - #6183

Open
oluexpert99 wants to merge 1 commit into
apache:developfrom
TECHSERVICES-LIMITED:bugfix/FINERACT-2711
Open

FINERACT-2711: Fix shared-account template named-range bounds built by string concat#6183
oluexpert99 wants to merge 1 commit into
apache:developfrom
TECHSERVICES-LIMITED:bugfix/FINERACT-2711

Conversation

@oluexpert99

Copy link
Copy Markdown
Contributor
 - SharedAccountWorkBookPopulator.setNames built the Clients and Products named ranges as
    SHEET + "!$B$2:$B$" + list.size() + 1. The whole expression is left-to-right string
    concatenation, so size() + 1 does not add: the "1" is appended as text. With two
    products the range became SharedProducts!$B$2:$B$21 instead of $B$2:$B$3, and likewise
    for the client range.
  - The template still downloads, so this is not a 500; the effect is that both dropdowns
    reference far more rows than exist and the picker is padded with blank entries. On a
    tenant with ten products the bound reads $B$101.
  - Parenthesise both bounds so the arithmetic happens before the concatenation, matching
    every sibling populator (LoanWorkbookPopulator, SavingsWorkbookPopulator and others
    already write (size() + 1)).
  - Add a unit test asserting both named ranges end at $B$2:$B$3 for two clients and two
    products. It fails on the unfixed code with the concatenated bound
    (SharedProducts!$B$2:$B$21). Spies feed setNames controlled client and product counts
    while the real populators write their own (empty) sheets, so the assertion isolates the
    range arithmetic.

Description

Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per our guidelines
  • Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
  • Create/update unit or integration tests for verifying the changes made.
  • Follow our coding conventions.
  • Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

…y string concat

- SharedAccountWorkBookPopulator.setNames built the Clients and Products named ranges as
  SHEET + "!$B$2:$B$" + list.size() + 1. The whole expression is left-to-right string
  concatenation, so size() + 1 does not add: the "1" is appended as text. With two
  products the range became SharedProducts!$B$2:$B$21 instead of $B$2:$B$3, and likewise
  for the client range.
- The template still downloads, so this is not a 500; the effect is that both dropdowns
  reference far more rows than exist and the picker is padded with blank entries. On a
  tenant with ten products the bound reads $B$101.
- Parenthesise both bounds so the arithmetic happens before the concatenation, matching
  every sibling populator (LoanWorkbookPopulator, SavingsWorkbookPopulator and others
  already write (size() + 1)).
- Add a unit test asserting both named ranges end at $B$2:$B$3 for two clients and two
  products. It fails on the unfixed code with the concatenated bound
  (SharedProducts!$B$2:$B$21). Spies feed setNames controlled client and product counts
  while the real populators write their own (empty) sheets, so the assertion isolates the
  range arithmetic.

Signed-off-by: oluexpert99 <farooq@techservicehub.io>
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.

1 participant