Skip to content

fix(DeliveryRequestApiService): filter deliveries requested by partner role and incoterms#435

Merged
tom-rm-meyer-ISST merged 3 commits intoeclipse-tractusx:mainfrom
FraunhoferISST:fix/432-delivery
Jun 5, 2024
Merged

fix(DeliveryRequestApiService): filter deliveries requested by partner role and incoterms#435
tom-rm-meyer-ISST merged 3 commits intoeclipse-tractusx:mainfrom
FraunhoferISST:fix/432-delivery

Conversation

@tom-rm-meyer-ISST
Copy link
Copy Markdown
Contributor

@tom-rm-meyer-ISST tom-rm-meyer-ISST commented Jun 3, 2024

Description

resolves #432

Maybe we need also to do similar enhancements to the following fields

  • DeliveryService / OwnDeliveryService
  • Frontend > incoterm and partner relationship including current role in dashboard (not sure, if we already have it but I think there is something in place)

Please let me know what you think. Based on the changes I would release a patch version

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

Check failure

Code scanning / CodeQL

Log Injection

This log entry depends on a [user-provided value](1). This log entry depends on a [user-provided value](2).
Comment on lines 101 to 107

Check failure

Code scanning / CodeQL

Log Injection

This log entry depends on a [user-provided value](1).
Comment on lines 121 to 127

Check failure

Code scanning / CodeQL

Log Injection

This log entry depends on a [user-provided value](1). This log entry depends on a [user-provided value](2).
@tom-rm-meyer-ISST tom-rm-meyer-ISST marked this pull request as ready for review June 3, 2024 16:23
Copy link
Copy Markdown
Member

@ReneSchroederLJ ReneSchroederLJ left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

Overall solid work, but besides a comment I left on a bug there seems to be an issue with how the material is retrieved in handleDeliverySubmodelRequest. There is a disconnect between the comment and what is actually happening.

At first we attempt to find the material by assuming that we are supplier and the Cx number is our own. If this fails we should check if we are customer and the Cx number matches the partnerCxNumber in any mpr for that partner. only if that fails we should triggerPartTypeRetrieval and afterwards check the mpr again.

Currently we do not check the mpr at all so we can not handle requests where we are the customer.

@tom-rm-meyer-ISST
Copy link
Copy Markdown
Contributor Author

tom-rm-meyer-ISST commented Jun 4, 2024

Thank you for your contribution.

Overall solid work, but besides a comment I left on a bug there seems to be an issue with how the material is retrieved in handleDeliverySubmodelRequest. There is a disconnect between the comment and what is actually happening.

At first we attempt to find the material by assuming that we are supplier and the Cx number is our own. If this fails we should check if we are customer and the Cx number matches the partnerCxNumber in any mpr for that partner. only if that fails we should triggerPartTypeRetrieval and afterwards check the mpr again.

Currently we do not check the mpr at all so we can not handle requests where we are the customer.

Well seen! While implementing the change and a test to get this scenario tested, I recognized that this might even be unwanted behavior: If we don't have the CX ID when being customer, this means we did not create a Shell Descriptor with a href. Someone tries to access data directly via the asset / interface without the DTR to see how to access the data.

I added a warning message as I think this could be a security issue, if a partner tries to do that.

// Sidenote: This still means that the ShellDescriptor has not been created and someone tries to access our
// api without using the href from DTR
if (mpr == null) {
log.warn("Could not find " + materialNumberCx + " from partner " + partner.getBpnl());

Check failure

Code scanning / CodeQL

Log Injection

This log entry depends on a [user-provided value](1).
log.error("No material partner relation for BPNL '{}' and material global asset id '{}'." +
"Abort answering delivery request.",
partner.getBpnl(),
materialNumberCx

Check failure

Code scanning / CodeQL

Log Injection

This log entry depends on a [user-provided value](1).
Copy link
Copy Markdown
Member

@ReneSchroederLJ ReneSchroederLJ left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you for your efforts.

@tom-rm-meyer-ISST
Copy link
Copy Markdown
Contributor Author

Bumped version to create a patch release

@tom-rm-meyer-ISST tom-rm-meyer-ISST requested review from mhellmeier and removed request for mhellmeier June 5, 2024 09:55
@tom-rm-meyer-ISST tom-rm-meyer-ISST merged commit c2d02c5 into eclipse-tractusx:main Jun 5, 2024
@tom-rm-meyer-ISST tom-rm-meyer-ISST deleted the fix/432-delivery branch June 5, 2024 11:30
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.

Customer can't refresh Delivery Information

4 participants