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

Fixes of service points integration #33

Merged
merged 6 commits into from
Feb 10, 2024

Conversation

AlexisPPLIN
Copy link
Contributor

Happy new year !

I recently started using my service point contribution from v5.1.0, and I realized that everything was broken 🤕
So here is my patch:

Solves

  • Sendcloud service points API uses servicepoints.sendcloud.sc URL instead of the standard parcel.sendcloud.sc. So I added a new constant SERVICE_POINTS_BASE_URL.
  • Typo in API endpoint service-point -> service-points.
  • Made distance optional (null) because only returned by the api if latitude and longitude are provided.

Additions

  • How to use the service points API in README.md.
  • Test for optional distance value.

Have a nice day 👋

src/Client.php Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@villermen villermen assigned villermen and unassigned villermen Feb 7, 2024
@villermen
Copy link
Member

@AlexisPPLIN I've split the services as per your suggestion and updated the documentation to match. Can you check if I've missed anything? I've already verified this to work on production to be safe.

@AlexisPPLIN
Copy link
Contributor Author

@villermen I've not tested this in production but LGTM 👍
Obviously this is a breaking change for thoses using ShippingPoint methods (even if it was broken before).

@villermen villermen merged commit fd077dc into Webador:master Feb 10, 2024
1 check passed
@AlexisPPLIN AlexisPPLIN deleted the fix/servicepoints branch March 14, 2024 15:12
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