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.

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

Gate: Only External-ID in Generic Business Partner Input is required #476

Closed
4 tasks done
Tracked by #382
nicoprow opened this issue Sep 19, 2023 · 5 comments
Closed
4 tasks done
Tracked by #382
Assignees
Labels
enhancement New feature or request

Comments

@nicoprow
Copy link
Contributor

nicoprow commented Sep 19, 2023

When upserting generic business partner data input only the externalId is required. All other fields are optional. If the externalId is empty or null it should return an error.

Tasks

  • Make city optional
  • Make postal address optional
  • Make country optional
  • Other fields in subobjects like Identifiers, Classifications and so on
@nicoprow nicoprow changed the title Generic Business Partner: Only External-ID in Input is required Gate: Only External-ID in Input is required Sep 19, 2023
@nicoprow nicoprow changed the title Gate: Only External-ID in Input is required Gate: Only External-ID in Generic Business Partner Input is required Sep 19, 2023
@nicoprow nicoprow added the enhancement New feature or request label Sep 19, 2023
@martinfkaeser martinfkaeser self-assigned this Sep 20, 2023
@martinfkaeser martinfkaeser moved this to 🏗 In progress in BPDM Kanban Sep 20, 2023
@martinfkaeser
Copy link
Contributor

martinfkaeser commented Sep 20, 2023

So, as I understand a BusinessPartnerInputRequest doesn't need a postalAddress. But if there is one, physicalPostalAddress is still required inside of it, right?
And PhysicalPostalAddressGateDto doesn't have any required fields?
What about AlternativePostalAddressDto? Going by the original ticket I guess it shouldn't have any required fields either, not even deliveryServiceType and deliveryServiceNumber?

And what about the output version?
Ideally we wouldn't want to change the "required"-ness of fields, but that would mean introducing input and output versions of PhysicalPostalAddressGateDto and AlternativePostalAddressDto.
In my opinion this is not worth the effort!

And AlternativePostalAddressDto, which now is used in the Pool and Gate needs to be split.

@nicoprow
Copy link
Contributor Author

So, as I understand a BusinessPartnerInputRequest doesn't need a postalAddress. But if there is one, physicalPostalAddress is still required inside of it, right? And PhysicalPostalAddressGateDto doesn't have any required fields? What about AlternativePostalAddressDto? Going by the original ticket I guess it shouldn't have any required fields either, not even deliveryServiceType and deliveryServiceNumber?

And what about the output version? Ideally we wouldn't want to change the "required"-ness of fields, but that would mean introducing input and output versions of PhysicalPostalAddressGateDto and AlternativePostalAddressDto. In my opinion this is not worth the effort!

And AlternativePostalAddressDto, which now is used in the Pool and Gate needs to be split.

  1. True, we don't have an overview of which fields are required in case someone actually gives a postal address, physical postal address or alternative postal address. I don't like the idea of having completely empty objects there. We should get into this and see if we can agree on some minimum required fields for these address objects.
  2. Validating the integrity of the data is of course a given. We can of course have the same DTO for both, having nullable fields there and just use service logic to validate this. It is important that these fields are validated to not be null where applicable and at the same time the API user needs to understand that these fields are required. In my opinion, whatever we do these are the requirements.
  3. I think in the long term we should split the explicit DTOs between all applications anyway to reduce cohesion. Instead, we can rely on interfaces for these DTOs to define common API model structures.

@martinfkaeser martinfkaeser moved this from 🏗 In progress to 👀 In review in BPDM Kanban Sep 21, 2023
@nicoprow nicoprow moved this from 👀 In review to New in BPDM Kanban Sep 27, 2023
@nicoprow
Copy link
Contributor Author

We got an update on this. The idea is really to make every single field optional in the generic business partner model except the external-id. This also includes fields like identifier type value. Idea is to accept in the Gate all possible business partners without performing any kind of validation that prevents it from being persisted in the input.

@nicoprow nicoprow moved this from New to 📋 Backlog in BPDM Kanban Sep 28, 2023
@nicoprow nicoprow moved this from 📋 Backlog to 🔖 Ready in BPDM Kanban Oct 10, 2023
@alexsilva-CGI alexsilva-CGI self-assigned this Oct 11, 2023
@alexsilva-CGI alexsilva-CGI moved this from 🔖 Ready to 🏗 In progress in BPDM Kanban Oct 11, 2023
@nicoprow
Copy link
Contributor Author

These changes should only affect the Gate DTOs used for upserting business partners in the input stage. No changes in the persistence layer needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

4 participants