Skip to content

FINERACT-2717: Check all currency usages before allowing currency removal - #6191

Open
Samer-Melhem-FOO wants to merge 2 commits into
apache:developfrom
foodeveloper:port/CBS-589-currency-in-use-deletion-check
Open

FINERACT-2717: Check all currency usages before allowing currency removal#6191
Samer-Melhem-FOO wants to merge 2 commits into
apache:developfrom
foodeveloper:port/CBS-589-currency-in-use-deletion-check

Conversation

@Samer-Melhem-FOO

Copy link
Copy Markdown
Contributor

Description

CurrencyWritePlatformServiceJpaRepositoryImpl#updateAllowedCurrencies allows removing a currency from the organisation's allowed-currency list. The existing in-use check only looked at loan products, savings products, and charges, so a currency still referenced by actual loans, savings/share accounts, client transactions, account transfers, cashier transactions, or GL journal entries could be silently removed.

This broadens the check to a JdbcTemplate-based scan across all relevant tables (m_product_loan, m_savings_product, m_share_product, m_charge, m_loan, m_savings_account, m_share_account, m_client_transaction, m_account_transfer_transaction, m_cashier_transactions, acc_gl_journal_entry) and improves CurrencyInUseException's message to be clearer to end users.

JIRA

https://issues.apache.org/jira/browse/FINERACT-2717

Test plan

  • ./gradlew :fineract-core:compileJava :fineract-provider:compileJava passes
  • Added CurrencyWritePlatformServiceJpaRepositoryImplTest (no prior test existed for this class), covering:
    • currency removed successfully when unused
    • CurrencyInUseException thrown when the removed currency is still referenced
    • the usage check is skipped entirely for currencies that remain allowed
  • ./gradlew :fineract-provider:test --tests "...CurrencyWritePlatformServiceJpaRepositoryImplTest" — all 3 tests pass

…oval

The currency-in-use check when removing an organisation currency only
looked at loan products, savings products, and charges, so a currency
still referenced by actual loans, savings/share accounts, client
transactions, account transfers, cashier transactions, or GL journal
entries could be silently removed. Broadened the check to cover all
of those tables and gave CurrencyInUseException a clearer message.

Added CurrencyWritePlatformServiceJpaRepositoryImplTest covering: a
currency removed successfully when unused, CurrencyInUseException
thrown when still referenced, and the usage check being skipped for
currencies that remain allowed.
@Samer-Melhem-FOO

Copy link
Copy Markdown
Contributor Author

Two checks are currently failing here, both unrelated to this PR's change (CurrencyWritePlatformServiceJpaRepositoryImpl, CurrencyInUseException, OrganisationMonetaryConfiguration, plus a new unit test — no Liquibase/schema changes):

  • run-integrations-tests-on-mariadb / MariaDB Tests (Shard 4 of 15): fails at jibDockerBuild with java.net.SocketException: Network is unreachable pulling azul/zulu-openjdk-alpine — the same transient Docker Hub registry issue seen on other PRs recently.
  • run-integrations-tests-on-mysql / MySQL Tests (Shard 5 of 15): the app container built and started fine (Spring context came up, DB connection pool started), but the tenant Liquibase migration stalled for ~22 minutes with no progress between two log lines, and the health-check retry loop eventually timed out. This PR doesn't touch any Liquibase changelogs, so this looks like CI-runner resource contention/slowness rather than something caused by the change.

Could a maintainer re-run the failed jobs? Thanks!

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.

1 participant