[
https://issues.apache.org/jira/browse/FINERACT-2992?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18134288#comment-18134288
]
Adam Saghy commented on FINERACT-2992:
--------------------------------------
[~alberto_art3ch] Duplicate with
[https://issues.apache.org/jira/browse/FINERACT-1322?page=com.atlassian.jira.plugin.system.issuetabpanels%3Aall-tabpanel?]
> Introduce a loan-only repayment frequency type instead of extending
> PeriodFrequencyType
> ---------------------------------------------------------------------------------------
>
> Key: FINERACT-2992
> URL: https://issues.apache.org/jira/browse/FINERACT-2992
> Project: Apache Fineract
> Issue Type: Improvement
> Components: Loan, Savings, Shares
> Affects Versions: 1.16.0
> Reporter: Jose Alberto Hernandez
> Priority: Minor
> Labels: backwards-compatibility, loan, refactoring, tech-debt
> Fix For: 1.17.0
>
>
> ```
> h2. Background
> FINERACT-1322 (PR #6546) adds the semi-monthly repayment frequency to loans
> by appending \{{SEMI_MONTHLY(6)}} to
> \{{org.apache.fineract.portfolio.common.domain.PeriodFrequencyType}}. That
> enum is shared by loans, savings and deposit lock-in periods, recurring
> deposit frequencies, share lock-in periods and interest rate charts.
> Semi-monthly only has a meaning for loan repayments, so the shared enum now
> carries a value that most of its users must explicitly ignore.
> h2. Problem / Motivation
> * Every exhaustive \{{switch}} over \{{PeriodFrequencyType}} outside the loan
> domain needed a dummy \{{case SEMI_MONTHLY}} branch that only exists to keep
> the switch exhaustive: \{{SavingsAccount}} (lock-in),
> \{{DepositAccountUtils}} (recurring deposit date),
> \{{ShareAccountDataSerializer}} (lock-in) and
> \{{InterestRateChartEnumerations}}.
> * \{{PeriodFrequencyType.integerValues()}} filters \{{SEMI_MONTHLY}} out so
> that interest rate chart validation does not accept it; the savings, deposit
> and share validators rely on \{{inMinMaxRange(0, 3)}} to keep the value out.
> The fence works, but it is spread over several modules and easy to break when
> the next loan-only frequency is added.
> * This was raised in the review of PR #6546 by @galovics and agreed as a
> follow-up; see the dev@ thread: <link>.
> h2. Proposed change
> * Add a loan-specific frequency type (for example
> \{{LoanRepaymentFrequencyType}} in \{{fineract-loan}}) with \{{DAYS}},
> \{{WEEKS}}, \{{MONTHS}}, \{{YEARS}} and \{{SEMI_MONTHLY}}, keeping the
> persisted integer values of \{{PeriodFrequencyType}} (0-3 and 6) so no data
> migration is needed for \{{m_product_loan.repayment_period_frequency_enum}}
> and \{{m_loan.repayment_period_frequency_enum}}.
> * Change \{{repaymentPeriodFrequencyType}} to the new type in the loan
> domain: \{{LoanProductRelatedDetail}}, \{{ILoanConfigurationDetails}} /
> \{{LoanConfigurationDetails}}, \{{LoanApplicationTerms}},
> \{{ScheduledDateGenerator}} / \{{DefaultScheduledDateGenerator}},
> \{{PaymentPeriodsInOneYearCalculator}}, \{{AprCalculator}},
> \{{LoanScheduleAssembler}}, the progressive EMI calculator and interest
> schedule model, and the loan read/write services and enumerations.
> * Provide explicit conversion where the loan domain still talks to shared
> code that expects \{{PeriodFrequencyType}} (calendar utilities, re-aging,
> charge frequencies, API option lists), instead of relying on the shared enum
> carrying loan-only values.
> * Remove \{{SEMI_MONTHLY}} from \{{PeriodFrequencyType}}, together with the
> dummy \{{case SEMI_MONTHLY}} branches in savings, deposits, shares and
> interest rate charts and the special case in \{{integerValues()}}.
> * Keep the REST contract unchanged: \{{repaymentFrequencyType}} stays an
> integer with the same values on loan products, loans, templates and schedule
> calculation; the option lists returned by the templates must still include
> semi-monthly for loans only.
> h2. Affected modules
> * \{{fineract-core}} — \{{PeriodFrequencyType}} loses \{{SEMI_MONTHLY}} and
> the \{{integerValues()}} special case; \{{CommonEnumerations}} loses the
> loan-only option.
> * \{{fineract-loan}} — new frequency type, \{{repaymentPeriodFrequencyType}}
> retyped across product, loan terms and schedule generation.
> * \{{fineract-progressive-loan}} — EMI calculator and interest schedule model
> use the new type.
> * \{{fineract-provider}} — loan assemblers, validators, read/write services,
> re-aging validator and interest rate chart enumerations; share account
> serializer loses the dummy branch.
> * \{{fineract-savings}} — \{{SavingsAccount}} and \{{DepositAccountUtils}}
> lose the dummy branch.
> * \{{fineract-e2e-tests-core}} / \{{integration-tests}} — test enums and
> builders follow the new type where they reference it by name.
> h2. Acceptance criteria
> * \{{PeriodFrequencyType}} has no \{{SEMI_MONTHLY}} constant and no module
> outside loans contains a \{{case SEMI_MONTHLY}} branch.
> * Semi-monthly loan products and loans created before the change load,
> generate schedules and repay exactly as before; no Liquibase changeset is
> required.
> * The loan product and loan APIs accept and return
> \{{repaymentFrequencyType}} with the same integer values as today, including
> \{{6}} for semi-monthly.
> * Savings, deposit, share lock-in and interest rate chart APIs keep rejecting
> value \{{6}} with HTTP 400.
> * Existing unit, integration and E2E tests for the semi-monthly frequency
> (for example \{{LoanSemiMonthlyRepaymentScheduleTest}},
> \{{SemiMonthlyScheduleDatesTest}}, \{{LoanSemiMonthlyRepayment.feature}})
> pass unchanged in behaviour.
> ```
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
