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

[WIP] [FIX] Fix URL generation for Fulfillment #635

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

Conversation

iCodeTechnologies
Copy link

WHY are these changes introduced?

Fix Fulfillment.py to generate the correct URL for API 2023-01.

If no order_id supplied, URL should be /admin/api/<api_version>/fulfillments.py

currently the library produces the URL

/admin/api/<api_version>/orders//fulfillments.py

See issue #634

WHAT is this pull request doing?

Modifying the code in Fulfillment that generates the URL so that it complies with the requirements for API version 2023-01

NOTE: The added code is based on code already in the library for other resource classes

Checklist

  • I have updated the CHANGELOG (if applicable)
  • I have followed the Shopify Python guide

Fix Fulfillment.py to generate the correct URL for API 2023-01.

If no order_id supplied, URL should be /admin/api/<api_version>/fulfillments.py

See issue Shopify#634
@MohamedAliHamza
Copy link

Hi @iCodeTechnologies, did you test it? because seems to me it consumes the same endpoint, but we need to consume this one. which will require changes in the request body I think.

@remibergsma
Copy link

@MohamedAliHamza This PR solves an issue for me where the url generated by the library is incorrect (when no order_id is given): /admin/api/<api_version>/orders//fulfillments.py
(Double slash) This results in a 403. With this PR, the URL is correct:
/admin/api/<api_version>/orders/fulfillments.py
And the request is successful.

I run into this when migrating my code to the new fulfillment options, where the old ones are not working after 2023-04-01. Did some searing and found that this PR solves the issue I had.

@iCodeTechnologies
Copy link
Author

iCodeTechnologies commented Feb 14, 2023

Hi @iCodeTechnologies, did you test it? because seems to me it consumes the same endpoint, but we need to consume this one. which will require changes in the request body I think.

Hi @MohamedAliHamza , I tested the pull request with my application; it solves the issue I was having, which is that the Fulfillment POST URL was not correct
/admin/api/<api_version>/orders//fulfillments.py
instead of
/admin/api/<api_version>/fulfillments.py

Of course, other changes are required when moving to the latest API.

This is a simplified extract of what my application is doing to send fulfillments back to Shopify; in the real system I compare the line_items against the orders in my system etc

It should give you an idea of how you can do fulfillment with FulfillmentOrders.

fulfillment_orders = []
fulfillment_orders = get_all_resources(shopify.FulfillmentOrders, limit=250, order_id=XXXXXX)

payload = {}
for fulfillment_order in fulfillment_orders:
    fulfillment_order_item = dict(fulfillment_order_id=str(fulfillment_order.attributes['id']),
                                          fulfillment_order_line_items=[])

    for fulfillment_line_item in fulfillment_order.attributes['line_items']:
        fulfillment_order_item['fulfillment_order_line_items'].append(
                        dict(id=str(fulfillment_line_item.attributes['id']),
                             quantity=str(fulfillment_line_item.attributes['quantity'])))

payload['notify_customer'] = True
payload['line_items_by_fulfillment_order'] = []
payload['line_items_by_fulfillment_order'].append(fulfillment_order_item)
shopify.Fulfillment.create(payload)

Cheers
Dan

@joantune
Copy link

Definitely this is an ongoing issue - I am resorting to #634 (comment)

but the latest version of the library doesn't really work with 2023-04 and onwards I would suspect for the fulfillments

@pama1999
Copy link

why is the python library from shopify so dead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants