Feature/amazon mws without removing fws #4

Merged
merged 14 commits into from May 28, 2012

1 participant

@csaunders

please review @wisq @odorcicd

@csaunders

Disregard that @wisq, @Soleone would you mind?

@odorcicd odorcicd and 1 other commented on an outdated diff May 23, 2012
...active_fulfillment/fulfillment/services/amazon_mws.rb
+ response = {}
+ response[:tracking_numbers] = {}
+
+ tracking_node = REXML::XPath.first(document, "//FulfillmentShipmentPackage/member/TrackingNumber")
+ if tracking_node
+ id_node = REXML::XPath.first(document, "//FulfillmentOrder/SellerFulfillmentOrderId")
+ response[:tracking_numbers][id_node.text.strip] = tracking_node.text.strip
+ end
+
+ response[:response_status] = SUCCESS
+ response
+ end
+
+ def parse_fulfillment_response(op, document)
+ response = {}
+ action = OPERATIONS[:outbound][op]
@odorcicd
odorcicd added a line comment May 23, 2012

is action and node used?

@csaunders
csaunders added a line comment May 24, 2012

I'll take a look at how FWS was implemented, but I believe it's one of those Exception driven parts of the code (kinda dirty, I know).

@csaunders
csaunders added a line comment May 24, 2012

Yeah, it doesn't appear to do anything. I'm just going to remove all of it down to something simple

def parse_fulfillment_response(op, document)
  { :response_status => SUCCESS, :response_comment => MESSAGES[op][SUCCESS] }
end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@odorcicd odorcicd commented on the diff May 23, 2012
...active_fulfillment/fulfillment/services/amazon_mws.rb
+ params = {
+ :NextToken => token
+ }
+
+ build_basic_api_query(params)
+ end
+
+ def build_tracking_request(order_id, options)
+ params = {:Action => OPERATIONS[:outbound][:tracking], :SellerFulfillmentOrderId => order_id}
+
+ build_basic_api_query(params.merge(options))
+ end
+
+ def build_address(address)
+ requires!(address, :name, :address1, :city, :state, :country, :zip)
+ ary = address.map{ |key, value|
@odorcicd
odorcicd added a line comment May 23, 2012

You can one-line the map, and use compact to remove nils?

ary = address.map{ |key, value| [escape(LOOKUPS[:destination_address][key]), escape(value)] if value.length > 0 }
Hash[ary.compact]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@odorcicd odorcicd commented on an outdated diff May 23, 2012
test/unit/services/amazon_mws_test.rb
+ <Messages>
+ <Message>
+ <Locale>en_US</Locale>
+ <Text>We are experiencing high latency in UK because of heavy
+traffic.</Text>
+ </Message>
+ </Messages>
+ </GetServiceStatusResult>
+ <ResponseMetadata>
+ <RequestId>d80c6c7b-f7c7-4fa7-bdd7-854711cb3bcc</RequestId>
+ </ResponseMetadata>
+ </GetServiceStatusResponse>
+ XML
+ end
+
+ def invalid_create_response
@odorcicd
odorcicd added a line comment May 23, 2012

remove if not used or add a test with it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Soleone Soleone and 1 other commented on an outdated diff May 24, 2012
...active_fulfillment/fulfillment/services/amazon_mws.rb
+ def initialize(options = {})
+ requires!(options, :login, :password)
+ @seller_id = options[:seller_id]
+ super
+ end
+
+ def seller_id=(seller_id)
+ @seller_id = seller_id
+ end
+
+ def endpoint
+ ENDPOINTS[@options[:endpoint] || :us]
+ end
+
+ def fulfill(order_id, shipping_address, line_items, options = {})
+ requires!(options, :order_date, :comment, :shipping_method)
@Soleone
Shopify member
Soleone added a line comment May 24, 2012

Do we really require a comment and is that always sent from shopify?

@csaunders
csaunders added a line comment May 28, 2012

No we don't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jduff jduff commented on an outdated diff May 25, 2012
...active_fulfillment/fulfillment/services/amazon_mws.rb
@@ -0,0 +1,433 @@
+require 'base64'
+require 'openssl'
@jduff
Shopify member
jduff added a line comment May 25, 2012

You shouldn't need to require this in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@csaunders csaunders merged commit 1860a46 into Shopify:master May 28, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment