FINERACT-2455: WC - created and submitted date of all features supported in WCP should follow business date or system date based on configuration

[mariiaKraievska]

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!

• Write the commit message as per our guidelines
• 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.
• Create/update unit or integration tests for verifying the changes made.
• Follow our 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
• This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
• If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

[galovics]

The core mechanism is right - DateUtils.getBusinessLocalDate() already falls back to the tenant date when enable-business-date is off, so both config states are covered by construction, and the Liquibase backfill/constraint split is done correctly. Two things need fixing before merge though, plus a flaky assertion.

1. Breaking API change - response attribute removed

WorkingCapitalLoanNearBreachActionData and WorkingCapitalLoanPeriodPaymentRateChangeData drop OffsetDateTime createdDate in favor of LocalDate submittedOnDate. These records are the API response payloads (GET .../nearbreachactions, GET .../ratechanges), so this removes a field from the JSON contract - the repo's swagger-brake backward-compatibility check (R014 "Response attribute removed") should flag this. createdDate (system-clock audit stamp) and submittedOnDate (business date) aren't the same thing either, per your own changelog comment - can we add the new field and keep the old one rather than replacing it?

2. Liquibase NOT NULL column with no default - rolling deployment hazard

parts/0070_...xml and parts/0071_...xml add submitted_on_date DATE to existing tables, then addNotNullConstraint with no default value. N-1 code that doesn't know about the column will fail with a NOT NULL violation on insert during a rolling deploy. Either defaultValueComputed="CURRENT_DATE" on the added column, or keep it nullable in this release and tighten it in a follow-up.

3. Flaky e2e assertion (WorkingCapitalNearBreachActionStepDef / WorkingCapitalLoanAccountStepDef)

assertThat(latest.getSubmittedOnDate()).as(...).isEqualTo(Utils.now());

Utils.now() is LocalDate.now(Clock.systemUTC()), but the value under test comes from DateUtils.getLocalDateOfTenant() - tenant timezone, which defaults to Asia/Kolkata in this stack. Between 18:30 and 24:00 UTC the tenant is already on the next day and this fails. Can we read the tenant date instead of using Utils.now() for equality here?

4. Global config mutation without the repo's isolation mechanism

Both new e2e scenarios flip enable-business-date globally without using the existing cucumber.execution.exclusive-resources pattern already used for exactly this kind of shared state. Parallelism is off today so it's masked, but it'll silently corrupt other business-date-dependent scenarios the moment it's turned on.

Smaller: the trailing "Then Global configuration is enabled" step duplicates the existing @BusinessDateDisabledCheck after-hook and doesn't run on mid-scenario failure - probably fine to drop in favor of the hook. And the ticket title says "all features supported in WCP" but this only covers rate change + near-breach action - is that the full scope or a first slice?

Recommendation: CHANGES_REQUESTED

[mariiaKraievska]

The core mechanism is right - DateUtils.getBusinessLocalDate() already falls back to the tenant date when enable-business-date is off, so both config states are covered by construction, and the Liquibase backfill/constraint split is done correctly. Two things need fixing before merge though, plus a flaky assertion.

1. Breaking API change - response attribute removed

WorkingCapitalLoanNearBreachActionData and WorkingCapitalLoanPeriodPaymentRateChangeData drop OffsetDateTime createdDate in favor of LocalDate submittedOnDate. These records are the API response payloads (GET .../nearbreachactions, GET .../ratechanges), so this removes a field from the JSON contract - the repo's swagger-brake backward-compatibility check (R014 "Response attribute removed") should flag this. createdDate (system-clock audit stamp) and submittedOnDate (business date) aren't the same thing either, per your own changelog comment - can we add the new field and keep the old one rather than replacing it?

2. Liquibase NOT NULL column with no default - rolling deployment hazard

parts/0070_...xml and parts/0071_...xml add submitted_on_date DATE to existing tables, then addNotNullConstraint with no default value. N-1 code that doesn't know about the column will fail with a NOT NULL violation on insert during a rolling deploy. Either defaultValueComputed="CURRENT_DATE" on the added column, or keep it nullable in this release and tighten it in a follow-up.

3. Flaky e2e assertion (WorkingCapitalNearBreachActionStepDef / WorkingCapitalLoanAccountStepDef)

assertThat(latest.getSubmittedOnDate()).as(...).isEqualTo(Utils.now());

Utils.now() is LocalDate.now(Clock.systemUTC()), but the value under test comes from DateUtils.getLocalDateOfTenant() - tenant timezone, which defaults to Asia/Kolkata in this stack. Between 18:30 and 24:00 UTC the tenant is already on the next day and this fails. Can we read the tenant date instead of using Utils.now() for equality here?

4. Global config mutation without the repo's isolation mechanism

Both new e2e scenarios flip enable-business-date globally without using the existing cucumber.execution.exclusive-resources pattern already used for exactly this kind of shared state. Parallelism is off today so it's masked, but it'll silently corrupt other business-date-dependent scenarios the moment it's turned on.

Smaller: the trailing "Then Global configuration is enabled" step duplicates the existing @BusinessDateDisabledCheck after-hook and doesn't run on mid-scenario failure - probably fine to drop in favor of the hook. And the ticket title says "all features supported in WCP" but this only covers rate change + near-breach action - is that the full scope or a first slice?

Recommendation: CHANGES_REQUESTED

@galovics Thank you for your review

1. I’d keep the removal, since createdDate here was audit/system time, not the booking date. For WC history we want submittedOnDate (business/tenant date), same as transactions/charges. Audit stays on created_on_utc; re-exposing both on the response just recreates the confusion this change is meant to fix.
2. Agreed. Dropped NOT NULL from this PR for rolling safety. Follow-up PR FINERACT-2455: enforce NOT NULL on WC submitted_on_date columns #6327 re-backfills any remaining NULLs and adds the constraint (plus entity nullable = false).

[adamsaghy]

The core mechanism is right - DateUtils.getBusinessLocalDate() already falls back to the tenant date when enable-business-date is off, so both config states are covered by construction, and the Liquibase backfill/constraint split is done correctly. Two things need fixing before merge though, plus a flaky assertion.
1. Breaking API change - response attribute removed
WorkingCapitalLoanNearBreachActionData and WorkingCapitalLoanPeriodPaymentRateChangeData drop OffsetDateTime createdDate in favor of LocalDate submittedOnDate. These records are the API response payloads (GET .../nearbreachactions, GET .../ratechanges), so this removes a field from the JSON contract - the repo's swagger-brake backward-compatibility check (R014 "Response attribute removed") should flag this. createdDate (system-clock audit stamp) and submittedOnDate (business date) aren't the same thing either, per your own changelog comment - can we add the new field and keep the old one rather than replacing it?
2. Liquibase NOT NULL column with no default - rolling deployment hazard
parts/0070_...xml and parts/0071_...xml add submitted_on_date DATE to existing tables, then addNotNullConstraint with no default value. N-1 code that doesn't know about the column will fail with a NOT NULL violation on insert during a rolling deploy. Either defaultValueComputed="CURRENT_DATE" on the added column, or keep it nullable in this release and tighten it in a follow-up.
3. Flaky e2e assertion (WorkingCapitalNearBreachActionStepDef / WorkingCapitalLoanAccountStepDef)

assertThat(latest.getSubmittedOnDate()).as(...).isEqualTo(Utils.now());

Utils.now() is LocalDate.now(Clock.systemUTC()), but the value under test comes from DateUtils.getLocalDateOfTenant() - tenant timezone, which defaults to Asia/Kolkata in this stack. Between 18:30 and 24:00 UTC the tenant is already on the next day and this fails. Can we read the tenant date instead of using Utils.now() for equality here?
4. Global config mutation without the repo's isolation mechanism
Both new e2e scenarios flip enable-business-date globally without using the existing cucumber.execution.exclusive-resources pattern already used for exactly this kind of shared state. Parallelism is off today so it's masked, but it'll silently corrupt other business-date-dependent scenarios the moment it's turned on.
Smaller: the trailing "Then Global configuration is enabled" step duplicates the existing @BusinessDateDisabledCheck after-hook and doesn't run on mid-scenario failure - probably fine to drop in favor of the hook. And the ticket title says "all features supported in WCP" but this only covers rate change + near-breach action - is that the full scope or a first slice?
Recommendation: CHANGES_REQUESTED

@galovics Thank you for your review

1. I’d keep the removal, since createdDate here was audit/system time, not the booking date. For WC history we want submittedOnDate (business/tenant date), same as transactions/charges. Audit stays on created_on_utc; re-exposing both on the response just recreates the confusion this change is meant to fix.
2. Agreed. Dropped NOT NULL from this PR for rolling safety. Follow-up PR FINERACT-2455: enforce NOT NULL on WC submitted_on_date columns #6327 re-backfills any remaining NULLs and adds the constraint (plus entity nullable = false).

@mariiaKraievska Do we need a new PR to backfill?

[mariiaKraievska]

Do we need a new PR to backfill?

No, backfill of existing rows is already in this PR. The follow-up is for NOT NULL, the extra WHERE IS NULL update there is just a safety net before adding the constraint, not a separate backfill deliverable.

[ruzeynalov]

The core mechanism is right - DateUtils.getBusinessLocalDate() already falls back to the tenant date when enable-business-date is off, so both config states are covered by construction, and the Liquibase backfill/constraint split is done correctly. Two things need fixing before merge though, plus a flaky assertion.
1. Breaking API change - response attribute removed
WorkingCapitalLoanNearBreachActionData and WorkingCapitalLoanPeriodPaymentRateChangeData drop OffsetDateTime createdDate in favor of LocalDate submittedOnDate. These records are the API response payloads (GET .../nearbreachactions, GET .../ratechanges), so this removes a field from the JSON contract - the repo's swagger-brake backward-compatibility check (R014 "Response attribute removed") should flag this. createdDate (system-clock audit stamp) and submittedOnDate (business date) aren't the same thing either, per your own changelog comment - can we add the new field and keep the old one rather than replacing it?
2. Liquibase NOT NULL column with no default - rolling deployment hazard
parts/0070_...xml and parts/0071_...xml add submitted_on_date DATE to existing tables, then addNotNullConstraint with no default value. N-1 code that doesn't know about the column will fail with a NOT NULL violation on insert during a rolling deploy. Either defaultValueComputed="CURRENT_DATE" on the added column, or keep it nullable in this release and tighten it in a follow-up.
3. Flaky e2e assertion (WorkingCapitalNearBreachActionStepDef / WorkingCapitalLoanAccountStepDef)

assertThat(latest.getSubmittedOnDate()).as(...).isEqualTo(Utils.now());

Utils.now() is LocalDate.now(Clock.systemUTC()), but the value under test comes from DateUtils.getLocalDateOfTenant() - tenant timezone, which defaults to Asia/Kolkata in this stack. Between 18:30 and 24:00 UTC the tenant is already on the next day and this fails. Can we read the tenant date instead of using Utils.now() for equality here?
4. Global config mutation without the repo's isolation mechanism
Both new e2e scenarios flip enable-business-date globally without using the existing cucumber.execution.exclusive-resources pattern already used for exactly this kind of shared state. Parallelism is off today so it's masked, but it'll silently corrupt other business-date-dependent scenarios the moment it's turned on.
Smaller: the trailing "Then Global configuration is enabled" step duplicates the existing @BusinessDateDisabledCheck after-hook and doesn't run on mid-scenario failure - probably fine to drop in favor of the hook. And the ticket title says "all features supported in WCP" but this only covers rate change + near-breach action - is that the full scope or a first slice?
Recommendation: CHANGES_REQUESTED

@galovics Thank you for your review

1. I’d keep the removal, since createdDate here was audit/system time, not the booking date. For WC history we want submittedOnDate (business/tenant date), same as transactions/charges. Audit stays on created_on_utc; re-exposing both on the response just recreates the confusion this change is meant to fix.
2. Agreed. Dropped NOT NULL from this PR for rolling safety. Follow-up PR FINERACT-2455: enforce NOT NULL on WC submitted_on_date columns #6327 re-backfills any remaining NULLs and adds the constraint (plus entity nullable = false).

@galovics Thank you for your review

3 and 4 - I've addressed both

[adamsaghy]

The core mechanism is right - DateUtils.getBusinessLocalDate() already falls back to the tenant date when enable-business-date is off, so both config states are covered by construction, and the Liquibase backfill/constraint split is done correctly. Two things need fixing before merge though, plus a flaky assertion.
1. Breaking API change - response attribute removed
WorkingCapitalLoanNearBreachActionData and WorkingCapitalLoanPeriodPaymentRateChangeData drop OffsetDateTime createdDate in favor of LocalDate submittedOnDate. These records are the API response payloads (GET .../nearbreachactions, GET .../ratechanges), so this removes a field from the JSON contract - the repo's swagger-brake backward-compatibility check (R014 "Response attribute removed") should flag this. createdDate (system-clock audit stamp) and submittedOnDate (business date) aren't the same thing either, per your own changelog comment - can we add the new field and keep the old one rather than replacing it?
2. Liquibase NOT NULL column with no default - rolling deployment hazard
parts/0070_...xml and parts/0071_...xml add submitted_on_date DATE to existing tables, then addNotNullConstraint with no default value. N-1 code that doesn't know about the column will fail with a NOT NULL violation on insert during a rolling deploy. Either defaultValueComputed="CURRENT_DATE" on the added column, or keep it nullable in this release and tighten it in a follow-up.
3. Flaky e2e assertion (WorkingCapitalNearBreachActionStepDef / WorkingCapitalLoanAccountStepDef)

assertThat(latest.getSubmittedOnDate()).as(...).isEqualTo(Utils.now());

Utils.now() is LocalDate.now(Clock.systemUTC()), but the value under test comes from DateUtils.getLocalDateOfTenant() - tenant timezone, which defaults to Asia/Kolkata in this stack. Between 18:30 and 24:00 UTC the tenant is already on the next day and this fails. Can we read the tenant date instead of using Utils.now() for equality here?
4. Global config mutation without the repo's isolation mechanism
Both new e2e scenarios flip enable-business-date globally without using the existing cucumber.execution.exclusive-resources pattern already used for exactly this kind of shared state. Parallelism is off today so it's masked, but it'll silently corrupt other business-date-dependent scenarios the moment it's turned on.
Smaller: the trailing "Then Global configuration is enabled" step duplicates the existing @BusinessDateDisabledCheck after-hook and doesn't run on mid-scenario failure - probably fine to drop in favor of the hook. And the ticket title says "all features supported in WCP" but this only covers rate change + near-breach action - is that the full scope or a first slice?
Recommendation: CHANGES_REQUESTED

@galovics Thank you for your review

1. I’d keep the removal, since createdDate here was audit/system time, not the booking date. For WC history we want submittedOnDate (business/tenant date), same as transactions/charges. Audit stays on created_on_utc; re-exposing both on the response just recreates the confusion this change is meant to fix.
2. Agreed. Dropped NOT NULL from this PR for rolling safety. Follow-up PR FINERACT-2455: enforce NOT NULL on WC submitted_on_date columns #6327 re-backfills any remaining NULLs and adds the constraint (plus entity nullable = false).

I am fine with dropping the audit field from the response:

• It was a mistake to be introduced
• No real business logic depends on it
• Working capital functionality was not finished completely by 1.15, and it was highlighted, changes still might happen, even breaking...in some cases...

[galovics]

Nice work on the follow-up. Three of the four points from last round are properly addressed:

• Liquibase: nullable column + separate backfill changeset now, rolling-deploy safe.
• The flaky e2e assertion: Utils.now() is gone, replaced with a server-round-trip capture-before/after pattern - genuinely fixes the tenant-timezone issue rather than papering over it.
• Test isolation: proper exclusive-resources entry plus an @After restore hook for enable-business-date, matching the existing pattern in this file.

One thing is still open though, and CI is now red on exactly it: createdDate is still being replaced by submittedOnDate on both WorkingCapitalLoanNearBreachActionData and WorkingCapitalLoanPeriodPaymentRateChangeData instead of the new field being added alongside it. run-api-backward-compatibility / api-compatibility-check is failing with:

R014 createdDate was removed from response default in GET /v1/working-capital-loans/{loanId}/rate-changes
R014 createdDate was removed from response default in GET /v1/working-capital-loans/external-id/{loanExternalId}/rate-changes
R014 createdDate was removed from response default in GET /v1/working-capital-loans/{loanId}/near-breach-actions
R014 createdDate was removed from response default in GET /v1/working-capital-loans/external-id/{loanExternalId}/near-breach-actions

Could you restore createdDate on both records (and the mapper mapping / constructor arg that were removed alongside it) and keep submittedOnDate as an addition rather than a replacement? If the intent is to phase createdDate out eventually, swaggerBrake { deprecatedApiDeletionAllowed = true } is already configured in fineract-provider/build.gradle, so marking it @Schema(deprecated = true) first and removing it in a later PR would work too.

Two small non-blocking things while I was back in there:

• The new BusinessDateHelper methods (captureCurrentTenantDateBeforeWorkingCapitalAction, getEffectiveWorkingCapitalDateFromServer) hard-depend on the WC charge-off template endpoint even though BusinessDateHelper is otherwise a generic helper - might be cleaner in a WC-specific helper.
• assertStampedOnCurrentTenantDate also asserts .isNotEqualTo(storedBusinessDate) against the hardcoded 01 January 2026 stub date - harmless today, but it's a latent false failure if this suite is ever run on that calendar date. The .isIn(before, after) check already proves the point on its own.

Recommendation: CHANGES_REQUESTED (down from the prior review - just the API compat piece left)

[ruzeynalov]

@galovics I've pushed 74a2c616cf (amended e2e commit) with both non-blocking points addressed:

• WC-specific helper: the tenant-date probe/assert moved out of BusinessDateHelper into a new WorkingCapitalTenantDateHelper; BusinessDateHelper is generic again and the two WC stepdefs inject the new helper.
• isNotEqualTo(storedBusinessDate): kept, but reframed as an explicit precondition (assertThat(storedBusinessDate).isNotIn(before, after) with a self-explaining message). Rationale: the probe (charge-off template) and the stamp both resolve through DateUtils.getBusinessLocalDate(), so a regression that returned the stored row while the config is off would move both together and isIn(before, after) alone would still pass — the inequality is the only assertion that proves the fallback path actually ran. The stub date is 01 Jan 2026, already in the past, so the calendar collision cannot occur going forward; if it ever did, the failure now states exactly why.

[mariiaKraievska]

• It was a mistake to be introduced
• No real business logic depends on it
• Working capital functionality was not finished completely by 1.15, and it was highlighted, changes still might happen, even breaking...in some cases...

Nice work on the follow-up. Three of the four points from last round are properly addressed:

• Liquibase: nullable column + separate backfill changeset now, rolling-deploy safe.
• The flaky e2e assertion: Utils.now() is gone, replaced with a server-round-trip capture-before/after pattern - genuinely fixes the tenant-timezone issue rather than papering over it.
• Test isolation: proper exclusive-resources entry plus an @After restore hook for enable-business-date, matching the existing pattern in this file.

One thing is still open though, and CI is now red on exactly it: createdDate is still being replaced by submittedOnDate on both WorkingCapitalLoanNearBreachActionData and WorkingCapitalLoanPeriodPaymentRateChangeData instead of the new field being added alongside it. run-api-backward-compatibility / api-compatibility-check is failing with:

R014 createdDate was removed from response default in GET /v1/working-capital-loans/{loanId}/rate-changes
R014 createdDate was removed from response default in GET /v1/working-capital-loans/external-id/{loanExternalId}/rate-changes
R014 createdDate was removed from response default in GET /v1/working-capital-loans/{loanId}/near-breach-actions
R014 createdDate was removed from response default in GET /v1/working-capital-loans/external-id/{loanExternalId}/near-breach-actions

Could you restore createdDate on both records (and the mapper mapping / constructor arg that were removed alongside it) and keep submittedOnDate as an addition rather than a replacement? If the intent is to phase createdDate out eventually, swaggerBrake { deprecatedApiDeletionAllowed = true } is already configured in fineract-provider/build.gradle, so marking it @Schema(deprecated = true) first and removing it in a later PR would work too.

Two small non-blocking things while I was back in there:

• The new BusinessDateHelper methods (captureCurrentTenantDateBeforeWorkingCapitalAction, getEffectiveWorkingCapitalDateFromServer) hard-depend on the WC charge-off template endpoint even though BusinessDateHelper is otherwise a generic helper - might be cleaner in a WC-specific helper.
• assertStampedOnCurrentTenantDate also asserts .isNotEqualTo(storedBusinessDate) against the hardcoded 01 January 2026 stub date - harmless today, but it's a latent false failure if this suite is ever run on that calendar date. The .isIn(before, after) check already proves the point on its own.

Recommendation: CHANGES_REQUESTED (down from the prior review - just the API compat piece left)

@galovics Restored createdDate on both response records and kept submittedOnDate as an addition. Marked createdDate as @Schema(deprecated = true) so we can remove it later.

[adamsaghy]

LGTM

All the requested changes were addressed.