FINERACT-2351: Minor fixes to make it work with PostgreSQL and response fix for TaxGroup #4939

sidhantgoel · 2025-08-11T10:13:30Z

Description

The TaxGroup API wasn’t returning any data because the necessary getter annotations were missing, so the fields weren’t being serialized in the response.
There was a spelling mismatch, which caused the TaxComponent APIs to fail.

Both issues have been fixed.

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

Write the commit message as per https://github.com/apache/fineract/#pull-requests
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 coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/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
Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)

FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.

adamsaghy · 2025-08-13T12:13:35Z

@sidhantgoel Please rebase with latest develop branch.

adamsaghy · 2025-08-13T12:13:47Z

fineract-core/src/main/java/org/apache/fineract/portfolio/tax/request/TaxComponentRequest.java

    private BigDecimal percentage;
    private Integer debitAccountType;
-    private Long debitAccountId;
+    private Long debitAcountId;


it is not a typo, intentional because this is the spelling used in many places including web app.

I think this must be set to debitAccountId, if it exists in other parts of the Apache Fineract then it must be fixed. WebApp is not an Apache Fineract Project.

@sidhantgoel As Victor stated, please dont introduce typos into the Fineract backend! Kindly asking you to fix the UI instead.

@sidhantgoel Please undo this!

adamsaghy · 2025-08-13T12:14:37Z

...ider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxReadPlatformServiceImpl.java

        String sql = "select " + TAX_COMPONENT_MAPPER.getSchema();
-        return this.jdbcTemplate.query(sql, TAX_COMPONENT_MAPPER); // NOSONAR
+        return this.jdbcTemplate.query(con -> con.prepareStatement(sql,
+                                                                   ResultSet.TYPE_SCROLL_SENSITIVE,


why do we need this: ResultSet.TYPE_SCROLL_SENSITIVE, ?

If we dont use this, the resulset returned in case of PostgreSQL we can navigate in only one direction. but since we are moving back in some cases a scrollable result set is required.

adamsaghy

Please review my comments!
Testing is missing, please make sure to attach proper test cases which ensures the fixed, correct behaviour!

adamsaghy · 2025-08-14T18:59:36Z

@sidhantgoel Please squash your commits!

sidhantgoel · 2025-08-18T04:58:37Z

@adamsaghy done

sidhantgoel · 2025-08-20T06:39:38Z

@adamsaghy would you be able to review it please.

adamsaghy · 2025-08-20T08:09:31Z

@adamsaghy would you be able to review it please.

@sidhantgoel Please fix the failing tests first!

sidhantgoel · 2025-08-20T09:13:20Z

@adamsaghy Is there a document on how to run test cases locally?

adamsaghy · 2025-08-20T10:23:05Z

@adamsaghy Is there a document on how to run test cases locally?

you can find some materials on my website: www.fineract-academy.com

sidhantgoel · 2025-08-20T11:50:02Z

@adamsaghy fixed the errors.

adamsaghy · 2025-08-22T08:18:37Z

Please use appropriate FINERACT jira number in the title and in the commit message!
See example: #4932

sidhantgoel · 2025-08-23T15:57:51Z

There is no jira ticket for this, shall I create one?

…fix variable name typos in TaxComponentRequest; update SQL query preparation in TaxReadPlatformServiceImpl for PostgreSQL compatibility.

sidhantgoel · 2025-08-23T16:05:21Z

@adamsaghy is it ok now?

adamsaghy · 2025-08-28T08:57:06Z

...ider/src/main/java/org/apache/fineract/portfolio/tax/service/TaxReadPlatformServiceImpl.java

    public List<TaxComponentData> retrieveAllTaxComponents() {
        String sql = "select " + TAX_COMPONENT_MAPPER.getSchema();
-        return this.jdbcTemplate.query(sql, TAX_COMPONENT_MAPPER); // NOSONAR
+        return this.jdbcTemplate.query(con -> con.prepareStatement(sql, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE),


I am not convinced we need ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE), this services just retrieve a List of TaxComponentData AS-IS, no any iteration on them back or forth...

Can you help me to understand why this is an issue?

adamsaghy

Kindly see my review and concerns!
Also please make sure the functionality to be covered with test cases to ensure, now it works as expected!

github-actions · 2025-09-28T00:24:08Z

This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.

IOhacker · 2025-10-14T15:14:19Z

@sidhantgoel are you still working on it?

github-actions · 2025-11-14T00:23:22Z

This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.

adamsaghy · 2025-11-25T09:50:02Z

@sidhantgoel Are you still working on this?

github-actions · 2026-01-02T00:25:11Z

This pull request seems to be stale. Are you still planning to work on it? We will automatically close it in 30 days.

adamsaghy reviewed Aug 13, 2025

View reviewed changes

adamsaghy requested changes Aug 13, 2025

View reviewed changes

sidhantgoel force-pushed the minor_fixes branch from 587a0ed to c315dca Compare August 14, 2025 05:14

sidhantgoel requested a review from adamsaghy August 14, 2025 05:17

sidhantgoel force-pushed the minor_fixes branch from c315dca to fe12c72 Compare August 14, 2025 05:47

sidhantgoel force-pushed the minor_fixes branch from fe12c72 to 285f1d0 Compare August 18, 2025 04:58

sidhantgoel force-pushed the minor_fixes branch from 285f1d0 to eff2983 Compare August 18, 2025 05:00

sidhantgoel force-pushed the minor_fixes branch from eff2983 to c073e8a Compare August 20, 2025 08:54

sidhantgoel force-pushed the minor_fixes branch from c073e8a to aab4042 Compare August 20, 2025 11:49

sidhantgoel changed the title ~~Minor fixes to make it work with PostgreSQL and response fix for TaxGroup~~ FINERACT-2351: Minor fixes to make it work with PostgreSQL and response fix for TaxGroup Aug 23, 2025

FINERACT-2351: Enhance TaxGroupData with Lombok @DaTa annotation and …

e53e4a8

…fix variable name typos in TaxComponentRequest; update SQL query preparation in TaxReadPlatformServiceImpl for PostgreSQL compatibility.

sidhantgoel force-pushed the minor_fixes branch from aab4042 to e53e4a8 Compare August 23, 2025 16:04

adamsaghy reviewed Aug 28, 2025

View reviewed changes

adamsaghy requested changes Aug 28, 2025

View reviewed changes

github-actions bot added the stale label Sep 28, 2025

github-actions bot removed the stale label Oct 15, 2025

github-actions bot added the stale label Nov 14, 2025

github-actions bot removed the stale label Nov 26, 2025

adamsaghy marked this pull request as draft December 2, 2025 09:07

github-actions bot added the stale label Jan 2, 2026

FINERACT-2351: Minor fixes to make it work with PostgreSQL and response fix for TaxGroup #4939

Are you sure you want to change the base?

FINERACT-2351: Minor fixes to make it work with PostgreSQL and response fix for TaxGroup #4939

Uh oh!

Conversation

sidhantgoel commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

adamsaghy commented Aug 13, 2025

Uh oh!

adamsaghy Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

sidhantgoel Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IOhacker Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

adamsaghy Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

adamsaghy Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

adamsaghy Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

sidhantgoel Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

adamsaghy left a comment

Choose a reason for hiding this comment

Uh oh!

adamsaghy commented Aug 14, 2025

Uh oh!

sidhantgoel commented Aug 18, 2025

Uh oh!

sidhantgoel commented Aug 20, 2025

Uh oh!

adamsaghy commented Aug 20, 2025

Uh oh!

sidhantgoel commented Aug 20, 2025

Uh oh!

adamsaghy commented Aug 20, 2025

Uh oh!

sidhantgoel commented Aug 20, 2025

Uh oh!

adamsaghy commented Aug 22, 2025

Uh oh!

sidhantgoel commented Aug 23, 2025

Uh oh!

sidhantgoel commented Aug 23, 2025

Uh oh!

adamsaghy Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

adamsaghy left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 28, 2025

Uh oh!

IOhacker commented Oct 14, 2025

Uh oh!

github-actions bot commented Nov 14, 2025

Uh oh!

adamsaghy commented Nov 25, 2025

Uh oh!

github-actions bot commented Jan 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

sidhantgoel commented Aug 11, 2025 •

edited

Loading

sidhantgoel Aug 14, 2025 •

edited

Loading