Skip to content
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

[SDK-Scheme-Adapter] TransactionId is being used instead of TransferId #3018

Closed
29 of 36 tasks
PaulGregoryBaker opened this issue Nov 9, 2022 · 1 comment
Closed
29 of 36 tasks

[SDK-Scheme-Adapter] TransactionId is being used instead of TransferId #3018

PaulGregoryBaker opened this issue Nov 9, 2022 · 1 comment
Assignees
@kleyow @mdebarros
Labels
core-dev-squad oss-bulk Mojaloop bulk implementation and support oss-core This is an issue - story or epic related to a feature on a Mojaloop core service or related to it story

Comments

@PaulGregoryBaker
Copy link

PaulGregoryBaker commented Nov 9, 2022 •
edited by kleyow
Loading

In the bulk SDK API for the discovery and agreement phase implementations, the transactionId was used instead of the transferId. Although this doesn't break anything, it is confusing and inconsistent so should be corrected.

Acceptance Criteria:

  • Verify that the API Snippets are update to require the transferId instead of the transactionId
  • Verify that the Swagger API for the inbound PUT callback. Snippets are update to require the transferId instead of the transactionId
  • Verify that the API module is updated with the transferId to transactionId changes.
  • Verify that the command handler module is updated with the transferId to transactionId changes.
  • Verify that the domain event handler module is updated with the transferId to transactionId changes.
  • Verify that the integration test are updated with the transferId to transactionId changes.
  • Verify that the functional test are updated with the transferId to transactionId changes.

Complexity: <High|Medium|Low> > A short comment to remind the reason for the rating

Uncertainty: <High|Medium|Low> > A short comment to remind the reason for the rating

Tasks:

  • API Snippets
  • Update the swagger for the DFSP Backend API + required fields change to TransferId
    • (PUT inbound\bulkTransactions) -- present party info
    • (PUT inbound\bulkTransactions) -- present quote info
  • Updating SDK outbound API + required fields change to TransferId
    • (PUT outbound\bulkTransactions) - accept party info
    • (PUT outbound\bulkTransactions) - accept quote info
  • Update command& domain handler interface types to map TransferId & TranactionId correctly
    • For party processing
    • Update integration tests for party processing
    • For quote processing
    • Update integration tests for quote processing
  • Update Unit tests <-- there are no unit tests that cover these use-cases
  • Update Integration tests
  • Updating functional test cases
    • Update test assertions in local functional tests
    • Update test cases in TTK repo

Done

  • Acceptance Criteria pass
  • Designs are up-to date
  • Unit Tests pass
  • Integration Tests pass
  • Code Style & Coverage meets standards
  • Changes made to config (default.json) are broadcast to team and follow-up tasks added to update helm charts and other deployment config.
  • TBD

Pull Requests:

Follow-up:

  • N/A

Dependencies:

  • N/A

Accountability:

  • Owner: TBC
  • QA/Review: TBC
@PaulGregoryBaker PaulGregoryBaker added core-dev-squad oss-bulk Mojaloop bulk implementation and support story labels Nov 9, 2022
@elnyry-sam-k elnyry-sam-k added the oss-core This is an issue - story or epic related to a feature on a Mojaloop core service or related to it label Nov 9, 2022
@mdebarros mdebarros self-assigned this Nov 11, 2022
@mdebarros mdebarros changed the title [SDK-Scheme-Adpater] TransactionId is being used instead of TransferId [SDK-Scheme-Adapter] TransactionId is being used instead of TransferId Nov 11, 2022
@mdebarros mdebarros mentioned this issue Nov 11, 2022
Merged
mdebarros added a commit to mojaloop/sdk-scheme-adapter that referenced this issue Nov 11, 2022
@mdebarros
feat(mojaloop/3018): [SDK-Scheme-Adapter] TransactionId is being used…
0c27bbb 
… instead of TransferId

feat(mojaloop/3018): [SDK-Scheme-Adapter] TransactionId is being used instead of TransferId - mojaloop/project#3018
- bumped api-snippets to v17.0.0-snapshot.0 --> TODO: Upgrade this to final version once PR (mojaloop/api-snippets#148) has been merged
- updated applicable events to include the transferId
- updated aggregate handlers to correctly map the transferId instead of the transactionId
- updated TTK API definitions for SDK Outbound API specifications (copied from API-Snippets)
- updated TTK Test Case "bulk_happy_path_dynamic" to correctly use the transferId instead of the transactionId

chore: improvements to docker-compose for debugging
- added "start:debug" to each of the modules to start with debugging enabled
- updated docker-compose to use the "start:debug" command to be able to attach and debug issues
@mdebarros mdebarros mentioned this issue Nov 11, 2022
Merged
mdebarros added a commit to mojaloop/api-snippets that referenced this issue Nov 11, 2022
@mdebarros
feat(mojaloop/3018): [SDK-Scheme-Adapter] TransactionId is being used…
af6c1e2 
… instead of TransferId (#148)

feat(mojaloop/3018): [SDK-Scheme-Adapter] TransactionId is being used instead of TransferId - mojaloop/project#3018
- removed transactionId as required from bulkTransaction Individual Transfer Accept PUT callback (for both accept parties & quotes)
- added transferId as required to bulkTransaction Individual Transfer Accept PUT callback (for both accept parties & quotes)
- Removed bulkHomeTransaction, homeTransaction, and transaction Id's for bulkTransactions Accept PUT callback (for both accept parties & quotes)
- Rebuild API specification definitions

BREAKING CHANGE: transferId is now required on bulkTransaction Individual Transfer Accept PUT callback (for both accept parties & quotes), and bulkHomeTransaction, homeTransaction, and transaction Id' have been removed.
kleyow added a commit to mojaloop/sdk-scheme-adapter that referenced this issue Nov 12, 2022
@mdebarros @kleyow
feat(mojaloop/3018): [SDK-Scheme-Adapter] TransactionId is being used…
5d349fe 
… instead of TransferId (#422)

* feat(mojaloop/3018): [SDK-Scheme-Adapter] TransactionId is being used instead of TransferId

feat(mojaloop/3018): [SDK-Scheme-Adapter] TransactionId is being used instead of TransferId - mojaloop/project#3018
- bumped api-snippets to v17.0.0-snapshot.0 --> TODO: Upgrade this to final version once PR (mojaloop/api-snippets#148) has been merged
- updated applicable events to include the transferId
- updated aggregate handlers to correctly map the transferId instead of the transactionId
- updated TTK API definitions for SDK Outbound API specifications (copied from API-Snippets)
- updated TTK Test Case "bulk_happy_path_dynamic" to correctly use the transferId instead of the transactionId

chore: improvements to docker-compose for debugging
- added "start:debug" to each of the modules to start with debugging enabled
- updated docker-compose to use the "start:debug" command to be able to attach and debug issues

* chore(snapshot): 21.4.0-snapshot.0

* docs: cleanup on readme

* fix: for outbound-command-handler integration tests

* chore(snapshot): 21.4.0-snapshot.1

* updated api-snippets

* chore(snapshot): 21.4.0-snapshot.2

* chore: made updates for api-snippets 17.0.0-snapshot.2 changes

- updated mappings based on the api-snippets changes
- updated api_specs for fun-test TTK api definitions

* updates to main readme about redis-insights and added todo on docker-compose

* chore(snapshot): 21.4.0-snapshot.3

* updated api-snippets to final v17.0.0 version

* chore(snapshot): 21.4.0-snapshot.4

* test: updated bulk_happy_path_dynamic to align to api-snippets v17.0.0 release

* removed bulkHomeTransactionId from PUT ACCEPT /bulkTransaction callbacks from bulk_happy_path_dynamic func test

* fix for integration tests

* chore(snapshot): 21.4.0-snapshot.5

* chore: yarn and merge

* chore: remove ncu

* chore: id

* chore(snapshot): 21.4.0-snapshot.0

Co-authored-by: Kevin <[email protected]>
@kleyow kleyow self-assigned this Nov 12, 2022
@mdebarros
Copy link
Member 

Testing Toolkit Report
Test Result: PASSED
Test Suite: SDK Bulk Tests
Environment: moja4.test.mojaloop.live
Total assertions: 195
Passed assertions: 195
Failed assertions: 0
Total requests: 33
Total test cases: 18
Passed percentage: 100.00%
Started time: Mon, 14 Nov 2022 12:57:31 GMT
Completed time: Mon, 14 Nov 2022 12:59:41 GMT
Runtime duration: 129317 ms

View Report

This was referenced Nov 15, 2022
Merged
Closed
mdebarros pushed a commit to mojaloop/sdk-scheme-adapter that referenced this issue Nov 15, 2022
@kleyow
chore(mojaloop/#3018): update documentation for transferId refactor […
9680e32 
…skip ci] (#424)

chore(mojaloop/#3018): update documentation for transferId refactor [skip ci] - mojaloop/project#3018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kleyow kleyow

@mdebarros mdebarros

Labels
core-dev-squad oss-bulk Mojaloop bulk implementation and support oss-core This is an issue - story or epic related to a feature on a Mojaloop core service or related to it story
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@kleyow @elnyry-sam-k @mdebarros @PaulGregoryBaker @chris-me-law