MONEI / Shopify-api-node

Node Shopify connector sponsored by MONEI
https://monei.com/shopify-payment-gateway/
MIT License
946 stars 278 forks source link

Event list does not support orderID parameter #421

Open LukasGirsch opened 3 years ago

LukasGirsch commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 https://github.com/MONEI/Shopify-api-node/issues/329.

LukasGirsch commented 3 years ago

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_createdsuccess_' 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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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