[UPS] CompanyName should not be present in Shipper element

[mdking]

Issue

Both Name and CompanyName are being populated in Shipper element.

Based on the schema

• Name should only be present for Shipper and
• CompanyName should only be populated for ShipTo and ShipFrom elements.

While the API may still work it causes the Shipper element to have a duplicate Name and CompanyName if only location.company_name is populated.

See https://github.com/Shopify/active_shipping/blob/master/lib/active_shipping/carriers/ups.rb#L593

CC @trishume @kmcphillips

[jbeker]

I'm not sure if this is a related or a different issue, but will start here.

I am seeing that for UPS if I include both a name and a company_name in the origin for create_shipment, the company_name value is printed twice on the label I get back instead of the name and then company_name

[xinlangzi]

@jbeker

I'm experiencing the same problem. Did you have update to know how to fix this?

[jbeker]

@xinlangzi We did. The issue had to do with the way the fields from the Location object are transformed into the XML that gets sent to UPS. We ended up creating a subclass of ActiveShipping::UPS that overrode build_location_node to add a bunch more logic in the creation of the XML. We then use our custom subclass.

def build_location_node(xml, name, location, options = {})
# not implemented:  * Shipment/Shipper/Name element
#                   * Shipment/(ShipTo|ShipFrom)/CompanyName element
#                   * Shipment/(Shipper|ShipTo|ShipFrom)/AttentionName element
#                   * Shipment/(Shipper|ShipTo|ShipFrom)/TaxIdentificationNumber element

xml.public_send(name) do

  # "Shipper" is what gets put in the return address field
  # The return address is made up of the fiollowing fields:
  # <AttentionName>
  # <Name>
  # <Address1>
  # <Address2>
  # etc, etc.

  # "ShipFrom" *does not get printed*

  # "ShiptTo" is the recipient's address
  # <AttentionName>
  # <CompanyName>
  # <Address1>
  # <Address2>
  # etc, etc.

  # Assumptions: We will have either a Name or a Company name

  label_name = nil
  label_attention = nil
  label_company = nil

  if name == 'Shipper'

    if !location.name.blank? && !location.company_name.blank? # we have both a name and company name
      label_attention = location.name
      label_name = location.company_name
      label_company = location.name
    elsif location.name.blank? && !location.company_name.blank? # name is blank, company is not blank
      label_name = location.company_name
      label_company = location.company_name
    elsif !location.name.blank? && location.company_name.blank? # name is blank, company is not blank
      label_name = location.name
      label_company = location.name
    else
      raise "need a name or an address"
    end

  else
    if !location.name.blank? && !location.company_name.blank? # we have both a name and company name
      label_attention = location.name
      label_name = location.name
      label_company = location.company_name
    elsif location.name.blank? && !location.company_name.blank? # name is blank, company is not blank
      label_company = location.company_name
      label_name = location.company_name
    elsif !location.name.blank? && location.company_name.blank? # name is blank, company is not blank
      label_company = location.name
      label_name = location.name
    else
      raise "need a name or an address"
    end
  end

  xml.Name(label_name) unless label_name.blank?
  xml.AttentionName(label_attention) unless label_attention.blank?
  xml.CompanyName(label_company) unless label_company.blank?

  xml.PhoneNumber(location.phone.gsub(/[^\d]/, '')) unless location.phone.blank?
  xml.FaxNumber(location.fax.gsub(/[^\d]/, '')) unless location.fax.blank?

  if name == 'Shipper' and (origin_account = options[:origin_account] || @options[:origin_account])
    xml.ShipperNumber(origin_account)
  elsif name == 'ShipTo' and (destination_account = options[:destination_account] || @options[:destination_account])
    xml.ShipperAssignedIdentificationNumber(destination_account)
  end

  if phone = location.phone
    xml.PhoneNumber(phone)
  end

  xml.Address do
    xml.AddressLine1(location.address1) unless location.address1.blank?
    xml.AddressLine2(location.address2) unless location.address2.blank?
    xml.AddressLine3(location.address3) unless location.address3.blank?
    xml.City(location.city) unless location.city.blank?
    xml.StateProvinceCode(location.province) unless location.province.blank?
    # StateProvinceCode required for negotiated rates but not otherwise, for some reason
    xml.PostalCode(location.postal_code) unless location.postal_code.blank?
    xml.CountryCode(location.country_code(:alpha2)) unless location.country_code(:alpha2).blank?
    xml.ResidentialAddressIndicator(true) unless location.commercial? # the default should be that UPS returns residential rates for destinations that it doesn't know about
    # not implemented: Shipment/(Shipper|ShipTo|ShipFrom)/Address/ResidentialAddressIndicator element
  end
end
end

[xinlangzi]

@jbeker

Thank you very much! I see the problem is on this shipper node. Your code works well.