Skip to content

FINERACT-2255: New command processing - external Assets Owner (org.apache.fineract.investor)#5773

Draft
elnafateh wants to merge 1 commit intoapache:developfrom
elnafateh:FINERACT-2255/External_assets_owners_read_requests
Draft

FINERACT-2255: New command processing - external Assets Owner (org.apache.fineract.investor)#5773
elnafateh wants to merge 1 commit intoapache:developfrom
elnafateh:FINERACT-2255/External_assets_owners_read_requests

Conversation

@elnafateh
Copy link
Copy Markdown

@elnafateh elnafateh commented Apr 15, 2026

Description

New command processing - external Assets Owner

Checklist

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

  • [] Write the commit message as per our guidelines
  • [] 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 our 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
  • [] This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.

Your assigned reviewer(s) will follow our guidelines for code reviews.

@elnafateh elnafateh changed the title FINERACT-2255: New command processing - External Assets Owner ;External Assets Owner read requests FINERACT-2255: New command processing - external assets owner read requests Apr 15, 2026
@elnafateh elnafateh force-pushed the FINERACT-2255/External_assets_owners_read_requests branch from 40e82b4 to b603176 Compare April 16, 2026 13:27
@elnafateh
Copy link
Copy Markdown
Author

@adamsaghy kindly rerun test on the failed check "E2E Tests (Shard 8 of 15)". thanks

@elnafateh
Copy link
Copy Markdown
Author

@adamsaghy @vidakovic pls do review this PR when u have the time, thanks

@vidakovic
Copy link
Copy Markdown
Contributor

vidakovic commented Apr 19, 2026

@elnafateh ... this PR is missing the goal of the refactoring... I don't see anywhere the new CommandDispatch being used... please read instructions again or contact me. As is can't be merged.

@vidakovic vidakovic marked this pull request as draft April 19, 2026 06:58
@elnafateh
Copy link
Copy Markdown
Author

@vidakovic thanks for the feedback. I have read (studied) Instruction again for both parent FINERACT-2169 and the child FINERACT-2255 Tickets. Now here's my Initial plan to completing FINERACT-2255 remaining work. Since the description Itemized the work from 1-6, with item 1. PR #451 covering some parts item 1, I intended to complete item 2. PR #5773 which only treats everything "READ" and then open another PR for item 3. Treat everything "WRITE" which is where "new CommandDispatch" is gonna come in and so does the rest of them items, each will have seperate PR under same ticket.. what do u make of this approach? your feedback is essential!

@vidakovic
Copy link
Copy Markdown
Contributor

@elnafateh I'd do this in one PR... splitting this up doesn't really help and probably creates more confusion. Just look at one of the completed tickets under FINERACT-2169 (I suggest "notes", dead easy)... that gives you a template on what and how to do this.

@elnafateh
Copy link
Copy Markdown
Author

@vidakovic I have started the impl based on the approach you suggested.

@elnafateh
Copy link
Copy Markdown
Author

@vidakovic the shared POST /transfers/loans/{loanId}?command=sale|buyback|intermediarySale endpoint makes it difficult to apply Jakarta Validation cleanly since each command has different required fields.
Am thinking options:
(1) keep the shared endpoint and use a class-level custom validator,
(2) split into separate endpoints per operation for cleaner validation, maintaining backward compatibility by keeping the old endpoint as deprecated. what do u prefer? Or is there any other way around?

@elnafateh elnafateh force-pushed the FINERACT-2255/External_assets_owners_read_requests branch from b603176 to 40de112 Compare April 28, 2026 17:10
@elnafateh elnafateh changed the title FINERACT-2255: New command processing - external assets owner read requests FINERACT-2255: New command processing - external Assets Owner (org.apache.fineract.investor) Apr 28, 2026
@elnafateh elnafateh force-pushed the FINERACT-2255/External_assets_owners_read_requests branch from 40de112 to 0006c56 Compare April 28, 2026 17:37
@elnafateh
Copy link
Copy Markdown
Author

@vidakovic kindly review the update

@elnafateh elnafateh marked this pull request as ready for review April 29, 2026 16:50
@elnafateh elnafateh marked this pull request as draft April 29, 2026 16:51
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.

2 participants