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

Add LabelLinksIndicator to LabelDelivery for ShipmentServiceOption #331

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add LabelLinksIndicator to LabelDelivery for ShipmentServiceOption #331

wants to merge 1 commit into from

Conversation

pista7
Copy link
Contributor

@pista7 pista7 commented Oct 1, 2021

Hi again!

At work, we realized what could be a small error. For ShipmentServiceOptions, the node expects a LabelLinksIndicator instead of the LabelLinkIndicator (note the 's'), in contrast with the LabelRecoveryRequest which indeed expects the LabelLinkIndicator, without the 's'.

The result was that the URL was not returned in the accept response.

To fix that, we added an option to set LabelLinksIndicator modifying as well the relevant if statement for ShipmentServiceOptions.

Again, let me know if I need to do something else in order to have it merged.

Cheers!
X.

Copy link
Collaborator

@ptondereau ptondereau 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 PR!
Just some small feedback here

@@ -155,6 +170,24 @@ public function setLabelLinkIndicator($labelLinkIndicator)
return $this;
}

/**
* @return string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @return string
* @return boolean

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.

3 participants