Add order note if present for Shipwire fulfillments #15

Merged
merged 1 commit into from Sep 6, 2012

3 participants

@BlakeMesdag
Shopify member

Changes

This adds a note to the Order xml as a CDATA element when provided with a note in the options hash. Has tests for when options[:note] is and isn't set.

Review

@Soleone @jduff

@Soleone
Shopify member

How would the XML look? The CDATA block needs to be inside of a new element I think is what they said.

@Soleone Soleone commented on an outdated diff Sep 6, 2012
lib/active_fulfillment/fulfillment/services/shipwire.rb
@@ -133,6 +133,7 @@ def add_order(xml, order_id, shipping_address, line_items, options)
Array(line_items).each_with_index do |line_item, index|
add_item(xml, line_item, index)
end
+ xml.cdata! options[:note] unless options[:note].blank?
@Soleone
Shopify member
Soleone added a note Sep 6, 2012

I try to avoid negations as much as possible, maybe change it to: if options[:note].present?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@BlakeMesdag
Shopify member

Modified it to add a Note node with a CDATA element. Extra XML will look like this:

<Note>
  <![CDATA[Some Order Note Content]]>
</Note>

It would have just been a <![CDATA[...]]> at the end of the Order node before.

@jduff
Shopify member

I think what you have now is what they are looking for, is there any way to send them a request and verify that this is what they are expecting?

@jduff
Shopify member

lgtm, would be nice to have verification on their end..even just spit out some xml and send them an email.

@BlakeMesdag
Shopify member

I sent a sample over to Shipwire. I'll wait to see what they think and then merge this in when they get back to me.

@Soleone
Shopify member

👍

@BlakeMesdag BlakeMesdag merged commit 113d3e7 into master Sep 6, 2012
@kevinhughes27 kevinhughes27 deleted the add-order-note-if-present branch Aug 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment