Skip to content

FINERACT-2682: Fix bulk client upload failure when custom datatables …#6116

Open
Samer-Melhem-FOO wants to merge 6 commits into
apache:developfrom
foodeveloper:port/CBS-1-bulk-client-upload-custom-fields
Open

FINERACT-2682: Fix bulk client upload failure when custom datatables …#6116
Samer-Melhem-FOO wants to merge 6 commits into
apache:developfrom
foodeveloper:port/CBS-1-bulk-client-upload-custom-fields

Conversation

@Samer-Melhem-FOO

Copy link
Copy Markdown

Problem

Bulk Client Upload (Excel import) fails once a custom datatable is registered
against the Client entity and an Entity Datatable Check makes it mandatory
for client creation (Entity = Client, Status = Create). The bulk import
workbook populator and import handlers have no support for datatable
columns: the downloaded template doesn't render columns for datatables
configured on the Client entity, and the import handlers have no way to
validate, parse, or submit datatable data as part of client creation — so a
mandatory datatable check blocks every row in the upload.

Fix

  • ClientEntityWorkbookPopulator / ClientPersonWorkbookPopulator now
    render a column for every datatable configured against the Client entity
    (including ones with no existing check, so they can be optionally
    populated), with support for dropdown, date, datetime, and boolean column
    types. Columns from single-row datatables are marked mandatory (*) to
    match Entity Datatable Check requirements; multi-row (repeatable child)
    datatable columns are always optional.
  • ImportHandlerUtils gains helpers to parse and validate datatable columns
    from the workbook, and validateRequiredDatatables() now skips multi-row
    datatables, since they represent repeatable child data rather than a
    single mandatory value.
  • ClientEntityImportHandler / ClientPersonImportHandler submit parsed
    datatable data as part of the client-create command, with clearer error
    reporting when validation fails.
  • DatatableData gains an isMultiRow() flag, threaded through
    DatatableReadServiceImpl and ReadSurveyServiceImpl (which shares the
    same DatatableData.create(...) factory).

Affected APIs

  • GET /v1/clients/downloadtemplate?legalFormType=CLIENTS_PERSON|CLIENTS_ENTITY
  • POST /v1/clients/uploadtemplate?legalFormType=CLIENTS_PERSON|CLIENTS_ENTITY
  • GET /v1/imports, GET /v1/imports/downloadOutputTemplate

Testing

  • Added unit tests for DatatableData covering single-row vs. multi-row
    behavior (DatatableDataTest).
  • Manually verified end-to-end: registered a custom datatable on the Client
    entity, added an Entity Datatable Check (Create), downloaded the template
    (columns present, single-row marked mandatory), filled it in, uploaded it,
    and confirmed the client was created successfully for both Person and
    Entity legal forms, and for both single-row and multi-row datatables.
  • ./gradlew :fineract-provider:compileJava,
    :fineract-core:compileTestJava, :fineract-provider:compileTestJava all
    pass.

JIRA: https://issues.apache.org/jira/browse/FINERACT-2682

…are configured

Ports the fix from an internal fork (foohelpdesk CBS-1: "Bulk Client
Upload fails when additional (custom) fields are configured").

Bulk client import via Excel previously failed once an Entity
Datatable Check made a custom datatable mandatory for client creation,
because the import handlers had no way to validate, populate, or parse
datatable columns in the workbook. This adds that support end-to-end:

- ClientEntityWorkbookPopulator / ClientPersonWorkbookPopulator now
  render columns for all datatables configured against the client
  entity (including empty ones), with dropdown, date, datetime and
  boolean column support, and mark single-row datatable columns as
  mandatory via a "*" suffix.
- ImportHandlerUtils gains parsing/validation helpers for datatable
  columns, and validateRequiredDatatables() now skips multi-row
  (repeatable child) datatables since they are optional, using the
  new DatatableData.isMultiRow() flag.
- ClientEntityImportHandler / ClientPersonImportHandler validate and
  submit datatable data as part of client creation, with clearer
  error reporting on failure.
- DatatableData / DatatableReadServiceImpl / ReadSurveyServiceImpl
  carry the multi-row flag through DatatableData.create(...).

Includes unit tests for single-row vs multi-row DatatableData handling.
The header text omitted "under the License is distributed", causing
the license-check plugin to flag the file as missing a header.
- Remove unused LegalForm import in BulkImportWorkbookPopulatorServiceImpl.
- Use Guava Splitter instead of String.split in
  ImportHandlerUtils.getFriendlyDatatableName, since split() has
  surprising behavior with trailing delimiters (StringSplitter).
- Wire up ClientEntityWorkbookPopulator.handleDatatableColumnsComplete,
  which was never called (UnusedMethod), so datatable dropdown columns
  in the bulk import template get their lookup values and Excel
  validation, matching ClientPersonWorkbookPopulator's existing wiring.
  Removed its duplicate named-range creation, since setNames() already
  creates those ranges.
@adamsaghy

Copy link
Copy Markdown
Contributor

@Samer-Melhem-FOO Please review the failing checks.

Resolves a conflict in ReadSurveyServiceImpl/-Test where develop
independently added a DatabaseSpecificSQLGenerator field to the same
constructor this branch adds a DatatableUtil field to; both fields are
kept.
- SearchUtil#extractIdFromDisplayValue: rewrap javadoc to the project's
  palantir-java-format style and drop trailing whitespace on the blank
  comment line (checkstyle RegexpSingleline).
- DatatableDataTest: collapse two DatatableData.create(...) calls back
  onto a single line per spotless.
ClientEntityWorkbookPopulator and ClientPersonWorkbookPopulator write
lookup dropdown cells (client type, classification, main business
line, address type, state/province, country) in "Name (Id)" format,
but ClientEntityImportHandler and ClientPersonImportHandler were still
parsing them with the legacy "Name-Id" dash-splitter, so every one of
those fields silently resolved to a null ID on import (only
constitution had been updated to the new format). Both handlers now
use ImportHandlerUtils#extractIdFromDisplayValue, the helper already
added for this purpose but left unused.

Also fixes ClientEntityImportHandlerTest#testClientImport, which built
its dummy row from ClientEntityConstants.LOOKUP_* column indices on the
visible sheet. Those columns are now dynamically positioned after the
datatable columns and were marked @deprecated in favor of the hidden
"_CLIENT_LOOKUPS" sheet, but the test wasn't updated to match, so it
picked up an unrelated lookup's value/ID and the import failed
downstream.
@Samer-Melhem-FOO
Samer-Melhem-FOO force-pushed the port/CBS-1-bulk-client-upload-custom-fields branch from 47f7cad to b20e15b Compare July 17, 2026 12:08
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.

2 participants