Skip to content

Add support for Related party identification structure #168

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.

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

Merged
merged 3 commits into from
May 14, 2025

Conversation

fluffis
Copy link
Contributor

@fluffis fluffis commented Mar 26, 2025

This PR resolves issue #167 and partly covers PR #141

The Id structure used in Related parties is extended to include Private Identification or Organisation Identification. This structure is reused in many parts of the CAMT (and PAIN) but for now implemented in related parties. It could probably be reused in other parts.

@PowerKiKi PowerKiKi requested a review from Copilot May 14, 2025 03:26
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for a new related party identification structure by extending the identification in related parties to include private and organisation identification types. It introduces new XML/JSON test files with the additional getIdentification attribute and updates the PHP DTOs and decoder factories accordingly.

  • Adds new XML and JSON test fixtures for the extended party identification.
  • Updates DTO interfaces (and implementations) by introducing getIdentification and setIdentification methods.
  • Implements decoding of both PrivateIdentification and OrganisationIdentification in the EntryTransactionDetail decoder.

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/data/camt053.v2.with-party-ids.xml New XML test data with party IDs incorporated
test/data/*.json Updated JSON test fixtures with added getIdentification fields
src/Decoder/Factory/DTO/PrivateIdentification.php New factory for creating PrivateIdentification from XML
src/Decoder/EntryTransactionDetail.php Added handling for setting identification using private or organisation branches
src/DTO/* Extended DTO classes (Debtor, Creditor, etc.) with identification getters and setters

@PowerKiKi PowerKiKi requested a review from Copilot May 14, 2025 03:39
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds support for an extended Related Party identification structure across various CAMT data formats and DTO implementations. Key changes include:

  • Extending XML and JSON samples with identification details.
  • Updating DTOs and interfaces to support the new identification field.
  • Adding unit tests and factory methods for Private and Organisation Identification.

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

File Description
test/data/camt053.v2.with-party-ids.* New CAMT053 samples with extended identification in XML and JSON.
test/data/camt05*.json Updates to JSON sample files incorporating the new identification field.
test/Unit/Camt053/EndToEndTest.php Added assertions to verify the new identification properties.
src/Decoder/* and src/DTO/* New factory creation logic and DTO enhancements to support identification.

@@ -201,6 +201,23 @@
"getSubDepartment": null,
"getTownName": null
},
"getIdentification": {
Copy link
Preview

Copilot AI May 14, 2025

Choose a reason for hiding this comment

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

The JSON representation for identification here is provided as a full object, whereas in similar contexts it is set to null. Consider adding documentation or comments to clarify when a full identification object is expected versus a null value.

Copilot uses AI. Check for mistakes.

@@ -160,6 +177,7 @@
"getSubDepartment": null,
"getTownName": null
},
"getIdentification": null,
Copy link
Preview

Copilot AI May 14, 2025

Choose a reason for hiding this comment

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

There is an inconsistency in how identification is represented; in some sections it is defined as an object and here as null. It would be helpful to document the conditions under which each representation is used to avoid confusion.

Copilot uses AI. Check for mistakes.

@PowerKiKi PowerKiKi merged commit 8e7070e into genkgo:master May 14, 2025
8 checks passed
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