beardon / mws-api

Amazon Marketplace Web Services client for Node.js.
79 stars 55 forks source link

Creating fulfillment with multiple <Message>'s #54

Closed dciccale closed 7 years ago

dciccale commented 7 years ago

normally, this should be allowed and is encouraged by amazon api docs, sending less requests and somewhat bigger files with multiple fulfillments. (up to around 10mb). Under multiple <Message> tags by increasing the <MessageID>

<?xml version="1.0" encoding="UTF-8"?>
<AmazonEnvelope xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="amzn-envelope.xsd">
  <Header>
    <DocumentVersion>1.01</DocumentVersion>
    <MerchantIdentifier>test</MerchantIdentifier>
  </Header>
  <MessageType>OrderFulfillment</MessageType>
  <Message>
    <MessageID>1</MessageID>
    <OrderFulfillment>
      <AmazonOrderID>102-9395366-2079430</AmazonOrderID> // order id
      <MerchantFulfillmentID>1</MerchantFulfillmentID> // May not be needed
      <FulfillmentDate>2013-07-18T21:58:28.000-04:00</FulfillmentDate>
      <FulfillmentData>
        <CarrierCode>UPS</CarrierCode>
        <ShippingMethod>UPS Ground</ShippingMethod>
        <ShipperTrackingNumber>1Z86A94T0398667660</ShipperTrackingNumber>
      </FulfillmentData>
    </OrderFulfillment>
  </Message>
  <Message>
    <MessageID>2</MessageID>
    <OrderFulfillment>
      <AmazonOrderID>102-9395366-2079430</AmazonOrderID> // order id
      <MerchantFulfillmentID>1</MerchantFulfillmentID>
      <FulfillmentDate>2013-07-18T21:58:28.000-04:00</FulfillmentDate>
      <FulfillmentData>
        <CarrierCode>UPS</CarrierCode>
        <ShippingMethod>UPS Ground</ShippingMethod>
        <ShipperTrackingNumber>1Z86A94T0398667660</ShipperTrackingNumber>
      </FulfillmentData>
    </OrderFulfillment>
  </Message>
</AmazonEnvelope>

However with mws-api I can only create one at a time:

const Feeds = require('mws-api/feeds');
mws.Feeds.SubmitFeed({
    FeedContent: Feeds._POST_ORDER_FULFILLMENT_DATA_({
      MessageID: 1,
      MerchantOrderID: 1234567,
      MerchantFulfillmentID: 1234567,
      FulfillmentDate: '2002-05-01T15:36:33-08:00',
      CarrierCode: 'DHL Paket',
      ShippingMethod: 'DHL Paket',
      ShipperTrackingNumber: '1234567890'
    }),
    FeedType: '_POST_ORDER_FULFILLMENT_DATA_'
  })
  .then((res) => {
    console.log(res)
  })
  .catch((e) => {
    console.log('error', e);
  })

Is there a way to do this which I am not aware of?

I have been looking at the code in libs/feeds/orders.js which uses createEnvelope function, however this is used like everywhere and I don't feel confortable change it.

I could propose a PR after some feedback on this.

tyscorp commented 7 years ago

Yeah, this is a big issue. I'm running into it too.

I'm thinking of just destroying all XML generation and leaving it up to the users because the current implementation is so limiting and kind of convoluted.

If you have any suggestions I'm very open to them.

dciccale commented 7 years ago

Wrapping the whole xml construction is nice, specially if you are new to the api. However it seems to be ton of work. Plus it requires also some good amount of documentation. This is the best lib I saw so far and I want to contribute. But for now I guess I'll just generate my xml. This should be easily done with strings or some json->xml lib out there.

I find it most useful when calling api methods of the sections api. That, I would keep. But maybe for feeds, it could be "provide your own xml string" for now, and leave that for feature development/enhancement.

The api for a fulfillment feed should look like this:

Notice the Messages array, and subkeys following the xml format.

mws.Feeds.SubmitFeed({
    FeedContent: Feeds._POST_ORDER_FULFILLMENT_DATA_({
      Messages: [{
        MessageID: 1,
        OrderFulfillment: {
          AmazonOrderID: 1234567,
          MerchantFulfillmentID: 1234567,
          FulfillmentDate: '2002-05-01T15:36:33-08:00',
          FulfillmentData: {
            CarrierCode: 'DHL Paket',
            ShippingMethod: 'DHL Paket',
            ShipperTrackingNumber: '1234567890'
          }
        }
      }, {/*.. other message */}]
    }),
    FeedType: '_POST_ORDER_FULFILLMENT_DATA_'
  })
dciccale commented 7 years ago

My example is missing some other data like MerchantIdentifier.

Also notice that <Item> for example is optional.

dciccale commented 7 years ago

Using something like https://github.com/michaelkourlas/node-js2xmlparser

You can get an xml pretty easy,

It automatically recognizes if Message is an object, it will generate 1 tag, and multiple if it's an Array of objects.

Right now, the following code produces the same output as the current functionality.

npm install js2xmlparser

const js2xml = require('js2xmlparser');
const Feeds = require('mws-api/feeds');

const xml1 = Feeds._POST_ORDER_FULFILLMENT_DATA_({
  MessageID: 1,
  AmazonOrderID: 1234567,
  MerchantFulfillmentID: 1234567,
  FulfillmentDate: '2002-05-01T15:36:33-08:00',
  CarrierCode: 'UPS',
  ShippingMethod: 'Second Day',
  ShipperTrackingNumber: '1234567890'
});

const obj = {
  // this would be prepended to every object by default
  "@": {
    "xmlns:xsi": "http://www.w3.org/2001/XMLSchema-instance",
    "xsi:noNamespaceSchemaLocation": "amzn-envelope.xsd"
  },
  // this also
  Header: {
    DocumentVersion: '1.01'
  },
  // this could also be appended like it is done now
  MessageType: 'OrderFulfillment',
  Message: [{
    MessageID: "1",
    OrderFulfillment: {
      AmazonOrderID: 1234567,
      MerchantFulfillmentID: 1234567,
      FulfillmentDate: '2002-05-01T15:36:33-08:00',
      FulfillmentData: {
        CarrierCode: 'UPS',
        ShippingMethod: 'Second Day',
        ShipperTrackingNumber: '1234567890'
      }
    }
  }]
};

const options = {
  declaration: {
    encoding: 'iso-8859-1'
  },
  format: {
    doubleQuotes: true
  }
};

const xml2 = js2xml.parse('AmazonEnvelope', obj, options)
  .replace(/>\s+/g, '>'); // to minify it

console.log(xml1 === xml2); // true

More review may be needed, but with this approach I think we could keep the xml generation, without much of the burden. wdyt?

tyscorp commented 7 years ago

That looks nice.

If you wanted to make a PR that abstracts some of that boilerplate that would be great. :)

Does it still make sense to have individual functions for each feed, or should we just consolidate it into one? I guess if we wanted to do validation...

dciccale commented 7 years ago

current version does not provide validation of feeds content right? If not, I would first make this work each with it's own function

dciccale commented 7 years ago

i will prepare a PR as soon as i can

rtdllnn commented 7 years ago

up!

dciccale commented 7 years ago

I am really short of time currently to implement this. FWIW I am successfully fulfilling hundreds of messages using the custom implementation mentioned here.

If it helps, I have a function that generates the xml

const js2xml = require('js2xmlparser');

function createFulfillmentXML(shipments, merchantId) {
  const messages = shipments.map((shipment, i) => ({
    MessageID: ++i,
    OrderFulfillment: {
      AmazonOrderID: shipment.reference,
      FulfillmentDate: new Date(shipment.created_at).toISOString(),
      FulfillmentData: {
        CarrierName: shipment.shipping_option,
        ShipperTrackingNumber: shipment.tracking_number
      }
    }
  }));

  const fulfillment = {
    '@': {
      'xmlns:xsi': 'http://www.w3.org/2001/XMLSchema-instance',
      'xsi:noNamespaceSchemaLocation': 'amzn-envelope.xsd'
    },
    Header: {
      DocumentVersion: '1.01',
      MerchantIdentifier: merchantId
    },
    MessageType: 'OrderFulfillment',
    Message: messages
  };

  const options = {
    declaration: {
      encoding: 'ISO-8859-1'
    },
    format: {
      doubleQuotes: true
    }
  };

  return js2xml.parse('AmazonEnvelope', fulfillment, options);
}

Later I call it like this

mws.Feeds.SubmitFeed({
  FeedContent: createFulfillmentXML(shipments, merchantId),
  FeedType: '_POST_ORDER_FULFILLMENT_DATA_'
})
.then(() => {})
.catch(() => {});

NOTE: take into account that this is fulfilling every item in the order. so a more versatile version of this should be done to allow passing items

dciccale commented 7 years ago

75 fixed this issue. closing