Skip to content

FINERACT-2326: Tax component and group issue serialization #4928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
adamsaghy merged 1 commit into from
Aug 13, 2025
Merged

FINERACT-2326: Tax component and group issue serialization #4928

adamsaghy merged 1 commit into from
Aug 13, 2025

Conversation

@alberto-art3ch
Copy link
Contributor

@alberto-art3ch alberto-art3ch commented Aug 7, 2025

Description

Tax component and group serialization was not done completed for the Id and Name attributes

FINERACT-2326

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
adamsaghy requested changes Aug 7, 2025
View reviewed changes
Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont understand this PR. You removed the Serializable interface and using lombok... what serialization issue you are referring to?

@alberto-art3ch
Copy link
Contributor Author

alberto-art3ch commented Aug 8, 2025

I dont understand this PR. You removed the Serializable interface and using lombok... what serialization issue you are referring to?

@adamsaghy
There is an issue with the TaxComponent and TaxGroup DTO please see the attached image, the taxGroupOption field has empty objects

Screenshot 2025-08-07 at 7 56 55 p m

@adamsaghy
Copy link
Contributor

adamsaghy commented Aug 8, 2025

I dont understand this PR. You removed the Serializable interface and using lombok... what serialization issue you are referring to?

@adamsaghy There is an issue with the TaxComponent and TaxGroup DTO please see the attached image, the taxGroupOption field has empty objects

Screenshot 2025-08-07 at 7 56 55 p m

I think the issue was with the missing getter, setter, constructor... not the Serializable interface itself... would you mind to put it back?

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2326/tax_objects_serializtion_issue branch from e02708c to 298ec5e Compare August 8, 2025 21:02
@alberto-art3ch
Copy link
Contributor Author

alberto-art3ch commented Aug 8, 2025

I dont understand this PR. You removed the Serializable interface and using lombok... what serialization issue you are referring to?

@adamsaghy There is an issue with the TaxComponent and TaxGroup DTO please see the attached image, the taxGroupOption field has empty objects
Screenshot 2025-08-07 at 7 56 55 p m

I think the issue was with the missing getter, setter, constructor... not the Serializable interface itself... would you mind to put it back?

@adamsaghy Done! Code updated

@adamsaghy adamsaghy merged commit c55cfe9 into apache:develop Aug 13, 2025
39 checks passed
@adamsaghy adamsaghy deleted the FINERACT-2326/tax_objects_serializtion_issue branch August 13, 2025 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamsaghy adamsaghy adamsaghy requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alberto-art3ch @adamsaghy