Skip to content

Conversation

@alberto-art3ch
Copy link
Contributor

@alberto-art3ch alberto-art3ch commented Jun 19, 2025

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 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 changed the title FINERACT-2198: Extend Manual Journal entry to support Asset Externali… FINERACT-2198: Extend Manual Journal entry to support Asset Externalization Jun 19, 2025
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.

Please kindly see my review!
Please make sure you are adding proper testing!

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2198/extend-manual-journal-entry-to-support-asset-externalization branch from 016de77 to 2bcfb14 Compare June 20, 2025 01:33
@alberto-art3ch
Copy link
Contributor Author

Please kindly see my review! Please make sure you are adding proper testing!

Testing has been added

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2198/extend-manual-journal-entry-to-support-asset-externalization branch from 2bcfb14 to 40d49c9 Compare June 27, 2025 02:37
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.

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2198/extend-manual-journal-entry-to-support-asset-externalization branch 4 times, most recently from 7a56656 to ac0a403 Compare July 2, 2025 02:34
@alberto-art3ch
Copy link
Contributor Author

https://github.com/apache/fineract/pull/4785/files#r2160481795

Please address this!

Done! code updated

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2198/extend-manual-journal-entry-to-support-asset-externalization branch from ac0a403 to a241201 Compare July 2, 2025 12:20
}
final Optional<ExternalAssetOwner> optExternalAssetOwner = externalAssetOwnerRepository.findByExternalId(externalId);
if (!optExternalAssetOwner.isPresent()) {
throw new ExternalAssetOwnerTransferNotFoundException(externalId);
Copy link
Contributor

Choose a reason for hiding this comment

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

You meant "ExternalAssetOwnerNotFoundException", right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ExternalAssetOwnerNotFoundException

That exception is new, code updated

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.

Please see my review!

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2198/extend-manual-journal-entry-to-support-asset-externalization branch from 92c7860 to 2b78d7d Compare July 4, 2025 12:53
@adamsaghy
Copy link
Contributor

@alberto-art3ch Please fix the failing test!

@alberto-art3ch alberto-art3ch force-pushed the FINERACT-2198/extend-manual-journal-entry-to-support-asset-externalization branch from 2b78d7d to 0e52067 Compare July 4, 2025 13:45
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.

LGTM

@adamsaghy adamsaghy merged commit c8b88c3 into apache:develop Jul 7, 2025
45 of 46 checks passed
@adamsaghy adamsaghy deleted the FINERACT-2198/extend-manual-journal-entry-to-support-asset-externalization branch July 7, 2025 11:59
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.

4 participants