rymghosn commented on PR #6543: URL: https://github.com/apache/fineract/pull/6543#issuecomment-6141195108
@adamsaghy Thanks for the review. Both points were right, and both are addressed in f7c0633998. I also rebased onto the latest `develop`. **1. `dateFormat` required for every template.** You're right. I went through every populator, including the sheet populators each one delegates to. Offices, staff, users and chart of accounts never format a date, and neither do journal entries or share accounts. Each one either ignores `dateFormat` or only passes it to `createDateConstraint`, which accepts null. `getTemplate` now requires `dateFormat` only when the entity type is outside that set. The check still runs before any permission check or data fetch. Every other template reaches `getFormat(dateFormat)`, `DateTimeFormatter` or `writeDate` and fails without it, so those keep the 400. `BulkImportWorkbookPopulatorServiceImplTest` covers both outcomes: the six templates above return 200 without `dateFormat`, and loans, clients, centers, savings and loan repayments are rejected. **2. Orphan `Name` objects in the office loop.** Confirmed. The three `createName()` calls ran before the guard, so a skipped office left unnamed `Name`s in the workbook. Each `Name` is now created inside its guard, as in the charge loop. `officesCollidingOnNameEmitOneDefinedNameSetPerScope` now also asserts that no unnamed `Name` remains. It fails on the previous version of the loop and passes now. -- 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]
