FINERACT-2712: Fix duplicate defined name 500 in loan and savings opening templates - #6184
Open
oluexpert99 wants to merge 1 commit into
Open
FINERACT-2712: Fix duplicate defined name 500 in loan and savings opening templates#6184oluexpert99 wants to merge 1 commit into
oluexpert99 wants to merge 1 commit into
Conversation
…ning templates - GET /v1/loans/downloadtemplate and GET /v1/savingsaccounts/downloadtemplate returned HTTP 500 (IllegalArgumentException: The workbook already contains this name) on any tenant holding two products, or two loan charges, that resolve to the same Excel defined name. Both opening templates are unusable for the whole tenant and there is no API-side workaround. - LoanWorkbookPopulator.setNames and SavingsWorkbookPopulator.setNames create a block of per-product (and, for loans, per-charge) defined names keyed by the product or charge name, with no de-duplication. The second entry resolving to the same name calls Name.setNameName with a name the workbook already holds and POI rejects it. Excel defined names are case-insensitive, so names differing only in case or in surrounding whitespace collide too -- e.g. products "Target Savings" and "TARGET SAVINGS " both produce Interest_Compouding_TARGET_SAVINGS. - Guard each loop with a Set of names already emitted and skip a repeat, so every name produces its defined-name block exactly once. The key is the sanitised, upper-cased name, which is the exact string the workbook will hold, so two source values that differ only in stripped punctuation or case are treated as the collision they are. - Extract the sanitising step of AbstractWorkbookPopulator.setSanitized into a protected sanitizeName helper so the de-duplication key is computed by the same code that builds the name, rather than duplicating the regex. setSanitized keeps its behaviour exactly. - In both loops the name is now resolved before any createName call, so a skipped duplicate leaves no orphan Name object in the workbook. - Semantics of the skip: the FIRST entry with a given name keeps the defined name, so the dropdown for that name resolves to the first product's or charge's lookup row. The second entry's per-product defaults are not reachable through the dropdown -- which is inherent to keying defined names by a non-unique field, and is still strictly better than failing the download for every user of the tenant. The lookup sheet itself is unchanged and still lists every product, so the row indices behind the ranges stay correct. - Add unit tests for the loan charge loop, the loan product loop and the savings product loop. Each fails on the unfixed code with the exact production exception (CHARGE_NAME_LATE_FEE, FUND_MICRO_LOAN, Interest_Compouding_TARGET_SAVINGS). Note the loan product loop substitutes spaces before sanitising, so a trailing space yields a genuinely different name there and only a case-only clash collides; the charge and savings loops trim first, so both variants collide. Signed-off-by: oluexpert99 <farooq@techservicehub.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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!
Your assigned reviewer(s) will follow our guidelines for code reviews.