bold-commerce / go-shopify

Go client for the Shopify API
MIT License
330 stars 256 forks source link

`ListWithPagination` function fails when `status` is passed to `OrderListOptions` #215

Open misraved opened 1 year ago

misraved commented 1 year ago

Question Is there a way by which we can list all the Shopify orders (including open, closed and canceled) using ListWithPagination function?

Roadblock I tried the following code to extract the list of orders:

func listOrders(ctx context.Context, d *plugin.QueryData, _ *plugin.HydrateData) (interface{}, error) {
    conn, err := connect(ctx, d)
    if err != nil {
        plugin.Logger(ctx).Error("shopify_order.listOrders", "connection_error", err)
        return nil, err
    }

    // max limit defined by the api is 250
    // We are setting status to 'any' to get all the orders(open, closed, canceled)
    options := goshopify.OrderListOptions{
        ListOptions: goshopify.ListOptions{},
        Status:      "any",
    }

    maxLimit := int64(250)
    // set the limit if a lower limit is passed in query context
    limit := d.QueryContext.Limit
    if limit != nil {
        if *limit < maxLimit {
            options.ListOptions.Limit = int(*limit)
        }
    } else {
        options.ListOptions.Limit = int(maxLimit)
    }

    for {
        orders, paginator, err := conn.Order.ListWithPagination(options)
        if err != nil {
            plugin.Logger(ctx).Error("shopify_order.listOrders", "api_error", err)
            return nil, err
        }

        for _, order := range orders {
            d.StreamListItem(ctx, order)

            if d.RowsRemaining(ctx) == 0 {
                return nil, nil
            }
        }

        if paginator.NextPageOptions == nil {
            return nil, nil
        }

        options.ListOptions.PageInfo = paginator.NextPageOptions.PageInfo

    }
}

However if the number of orders is more than 250, the API returns the following error - status: status cannot be passed when page_info is present. See https://shopify.dev/api/usage/pagination-rest for more information.

This link suggests that we can only pass limit and page_info, but there is no scope to pass the status field.

Alternatives considered If I don't pass the Status parameter to the OrderListOptions, the results are correctly paginated, however, the caveat is that I cannot access the closed or canceled orders.

Expectation ListWithPagination function should include the option to pass the Status field so that we can retrieve all kinds of orders.

Additional Context Issue reference - https://github.com/turbot/steampipe-plugin-shopify/issues/25 Please let me know if additional details are needed. Any kind suggestion would be very helpful. Thanks!!

oliver006 commented 1 year ago

That's a good question and I understand your conundrum. tbh, this seems to be more of a question for the Shopify dev community board to see if they even support this (paginated listing of orders with status==any). Did you try asking over there?

misraved commented 1 year ago

Thanks @oliver006 for the response 👍.

I have started a community discussion pertaining to this issue - https://community.shopify.com/c/shopify-discussions/listwithpagination-function-fails-when-status-is-passed-to/m-p/2125995#M369240

bbtb1982 commented 9 months ago

I think the answer is yes the Shopify API allows for pagination with status applied as the initial request builds the set based upon the query parameters. A cursor can only point to a location in a known set otherwise the cursor is invalid and not a valid offset in the set. As the set cannot be modified after the initial query. There is no reason for a request contain both a page_info (cursor) and status parameters.

Otherwise the API is wrong and needs to be updated to either always require a status query parameter to be defined or reject the request indicating the request is invalid and reference a error ID that will point to explain the behavior and why it does what it does.