rymghosn opened a new pull request, #6543: URL: https://github.com/apache/fineract/pull/6543
## Description JIRA: [FINERACT-2878](https://issues.apache.org/jira/browse/FINERACT-2878) `GET /loans/downloadtemplate` (the Loan Accounts bulk-import template) returns HTTP 500 in three situations, which the web app shows as a Download button that does nothing: 1. **A loan charge name contains a character Excel does not allow in a defined name** (`%`, `-`, `&`, `'`, …): `IllegalArgumentException: Invalid name: 'CHARGE_NAME_Service_Fee_5%-10%__VAT_': name must be letter, digit, period, or underscore`. 2. **Two office names map to the same defined name.** Office names are unique, but Excel defined names are case-insensitive and the sanitiser maps disallowed characters to `_`, so `Head Office` and `head office` (or `Head-Office`) both become `Group_Head_Office`: `IllegalArgumentException: The workbook already contains this name: Group_head_office`. 3. **`dateFormat` is omitted**: `NullPointerException: ... because "pFormat" is null` from `HSSFDataFormat.getFormat`, reached via `LoanProductSheetPopulator.populate`. The other `*/downloadtemplate` endpoints behave the same. **Root cause and fix:** - `LoanWorkbookPopulator.setNames` builds the `CHARGE_NAME_`/`CHARGE_AMOUNT_`/`CHARGE_AMOUNT_TYPE_<charge>` names with `replaceAll("[ )(]", "_")` instead of the shared `sanitizeName()` allowlist (FINERACT-2669) that every other named-range loop uses. FINERACT-2712 added the duplicate guard to this loop but kept the old regex. Now uses `sanitizeName()`. No formula references these names, so the template content is unchanged. - The per-office `Staff_`/`Client_`/`Group_<office>` loop in the same method creates names for every office, because the sheet populators record a begin/end index even for an office with no rows. Unlike the product and charge loops (FINERACT-2712), it has no de-duplication guard. Added the same seen-names guard, keyed on the sanitised, upper-cased name. - `BulkImportWorkbookPopulatorServiceImpl.getTemplate` never checked `dateFormat`. A blank value is now rejected up front with a `PlatformApiDataValidationException` (`error.msg.dateformat.mandatory`, parameter `dateFormat`), so every `downloadtemplate` endpoint answers 400. Two commits: the office guard first, then the charge sanitiser and the `dateFormat` check. **Tests:** - `LoanWorkbookPopulatorTest`: a charge named `Service Fee 5%-10% (VAT)`, and two offices named `Head Office`/`head office`, both populate without throwing and emit one defined-name set. - New `BulkImportWorkbookPopulatorServiceImplTest`: a null and a blank `dateFormat` are rejected with a validation error on `dateFormat`. All four new tests fail on current `develop` and pass with the fix, and `spotlessJavaCheck` is clean. **Live check** (local `bootRun` on a stock PostgreSQL schema): - before: - `GET /v1/loans/downloadtemplate?dateFormat=dd MMMM yyyy` with that charge configured → 500. - After also creating office `head office` → 500. - Without `dateFormat` → 500 (same on `/v1/savingsaccounts/downloadtemplate`). - after: - 200 with a valid `.xls`. - 200 with the office present. - 400 `The parameter dateFormat is mandatory` on both endpoints. - The savings and loan-repayment templates with `dateFormat` still return 200. ## Checklist Please make sure these boxes are checked before submitting your pull request - thanks! - [x] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests) - [x] 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. - [x] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made. - [x] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/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 (no API change: a request without `dateFormat` could never succeed; it now gets a 400 instead of a 500) - [x] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute). - [x] If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately. - [x] I followed the [AI Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy). Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide). 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
