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

Event list does not support orderID parameter #421

Open
LukasGirsch opened this issue Oct 22, 2020 · 6 comments
Open

Event list does not support orderID parameter #421

LukasGirsch opened this issue Oct 22, 2020 · 6 comments

Comments

@LukasGirsch
Copy link

According to the shopify documentation we are able to query a list by orderID:

GET /admin/api/2020-10/orders/450789469/events.json
docs: https://shopify.dev/docs/admin-api/rest/reference/events/event?api%5Bversion%5D=2020-10

However the list function does not support providing an orderID.
list([params])

I think it should be:
list(orderID [, params])

Using the orderID as a filter param does not work since the orderID is not part of the event object.

Kind regards

@LukasGirsch
Copy link
Author

UPDATE

I can confirm there actually is an error in event.js (v3.5.1)

I'am not to confident I did not miss something so I did not pull. Any chance someone from the devs can take a look at this and push it himself ?

it should be:

'use strict';

const assign = require('lodash/assign');
const pick = require('lodash/pick');


const baseChild = require('../mixins/base-child');

/**
 * Creates an Event instance.
 *
 * @param {Shopify} shopify Reference to the Shopify instance
 * @constructor
 * @public
 */
function Event(shopify) {
  this.shopify = shopify;

  this.parentName = 'orders';
  this.name = 'events';
  this.key = 'event';
}

assign(Event.prototype, pick(baseChild, ['buildUrl','create', 'delete', 'update']));

/**
* Gets a list of events for an order.
*
* @param {Number} orderId Order ID
* @param {Object} [params] Query parameters
* @return {Promise} Promise that resolves with the result
* @public
*/
Event.prototype.list = function list(orderID, params) {
    const url = this.buildUrl(orderID, undefined, params);
    return this.shopify.request(url, 'GET', this.name);
};


module.exports = Event;

@lpinca
Copy link
Collaborator

lpinca commented Oct 22, 2020

The existing API is for GET /admin/api/2020-10/events.json. There is no support for events of a specific resource. This is a duplicate of #329.

@LukasGirsch
Copy link
Author

Hi @lpinca ,
first of all thank you for the work in this repository I do really appreciate your work.

I have tested the change I have suggested above and there is an endpoint to query events based on the orderID. It's also documented on shopify.
Infact there are verbs that are undocumented but only available through this endpoint that I need, like 'shipping_label_created_success'
link: https://community.shopify.com/c/Shopify-APIs-SDKs/Retrieving-Shipping-Costs-via-API/m-p/393282/highlight/true#M21886

Please let me know if this is something that you don't want to support in which case I will have to fork this rep.

if you need any help developing this I am more than happy to assist you in any way I can.

Kind regards,
Lukas

@lpinca
Copy link
Collaborator

lpinca commented Oct 23, 2020

Yes but the same is valid also for other resources. From the documentation:

Events are generated by the following resources:

  • Articles
  • Blogs
  • Custom collections
  • Comments
  • Orders
  • Pages
  • Price rules
  • Products
  • Smart collections

For example GET /admin/api/2020-10/products/921728736/events.json, so shopify.event.list(orderId[, params]) is not ok because there should also be a shopify.event.list(productId[, params]), etc.

It might make more sense to add the events action to the Order, Product, and the other resources. For example shopify.order.events(orderId), and shopify.product.events(productId).

@LukasGirsch
Copy link
Author

I see, I forgot about the other resources.

Are you open for a pull request? I might have time on the weekend to make the changes you have suggested.

@lpinca
Copy link
Collaborator

lpinca commented Oct 23, 2020

Yes, events are like metafields but owner_resorce and owner_id do not work for events so I see no other way.

@lpinca lpinca closed this as completed Jan 13, 2024
@lpinca lpinca reopened this Jan 13, 2024
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

No branches or pull requests

2 participants