adamsaghy commented on PR #6071:
URL: https://github.com/apache/fineract/pull/6071#issuecomment-4852976969

   > > I have some concerns about this functionality. Specifically, I’m worried 
about limiting the number of fetched data and the data that can be retrieved. 
What if I need all 20,000 clients or loans? Perhaps I don’t fully understand 
how these `templates`, `downloadTemplates`, and `bulk-import` work. Could you 
please explain their exact purpose and usage?
   > 
   > Thanks — happy to explain, and your instinct about "limiting data" is 
worth addressing head-on.
   > 
   > What these templates are. Bulk import is two phases:
   > 
   > 1. Download template (downloadtemplate) — the server generates a 
pre-formatted .xls to help a user data-enter transactions. Besides the 
data-entry sheet, it embeds hidden Clients/Offices sheets plus cascading Excel 
dropdowns so the user can pick
   >    Office → Client → Account instead of typing.
   > 2. Upload (uploadtemplate) — the user fills rows and uploads; the import 
handler posts each row.
   > 
   > The key point: the import handler never reads the Clients/Offices sheets. 
For loan repayment, it resolves the loan straight from the typed account number 
(retrieveLoanIdByAccountNumber). So the embedded client/office dataset is 
purely a data-entry convenience for the downloaded file — it is discarded at 
import.
   > 
   > This PR does not limit what you can import. You can still upload and 
import 20,000 (or 200,000) rows — the upload/import path is completely 
unchanged.
   > 
   > And it does not permanently limit the download either. 
?includeLookups=true fetches all clients/loans, unbounded — exactly today's 
behaviour. If you want all 20,000 in the dropdowns, that flag gives you all 
20,000.
   > 
   > What changed is only the default for very large tenants: today, 
retrieveAll(null) loads every client with no LIMIT and writes them all (plus a 
3000-row VLOOKUP block) into the workbook — a multi-MB file that's slow to open 
and can OOM the server under concurrent downloads, built from data the importer 
ignores. So AUTO (default) degrades to a lean template above a configurable 
threshold; the lean template still imports everything — the user just types the 
account number instead of picking from a 20,000-row dropdown.
   > 
   > If you'd prefer zero change to the default behaviour, I'm glad to flip it 
so the default stays the full (unbounded) template and the lean form is purely 
opt-in via ?includeLookups=false — that keeps this PR strictly additive. Happy 
to go whichever way.
   
   Thank you for the explanation.
   
   I understand the lean vs not lean approach and i am okay with that:
   - Lean should not fetch any of the client / loan / etc, but only the small 
entry size selectors (payment type, transaction type, etc.).
   
   What i dont get is `max-lookup-rows`: We should either read all entries or 
none... reading some number of entries are incorrect, misleading and lead 
towards weird situations where dropdown list is incomplete...
   
   What do you think?


-- 
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]

Reply via email to