Skip to content

Commit

Permalink
Merge pull request Shopify#199 from Shopify/remove_unnesessary_assign…
Browse files Browse the repository at this point in the history
…ments

remove unnecessary assignments
  • Loading branch information
robfoster committed Nov 24, 2014
2 parents eb3b353 + a356f1f commit c6762f6
Show file tree
Hide file tree
Showing 16 changed files with 108 additions and 138 deletions.
2 changes: 1 addition & 1 deletion lib/active_shipping.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

begin
require 'active_support/all'
rescue LoadError => e
rescue LoadError
require 'rubygems'
gem "activesupport", ">= 2.3.5"
require "active_support/all"
Expand Down
2 changes: 1 addition & 1 deletion lib/active_shipping/shipping/carriers/canada_post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def self.default_location
protected

def commit(request, origin, destination, options = {})
response = parse_rate_response(ssl_post(URL, request), origin, destination, options)
parse_rate_response(ssl_post(URL, request), origin, destination, options)
end

private
Expand Down
8 changes: 4 additions & 4 deletions lib/active_shipping/shipping/carriers/canada_post_pws.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def find_tracking_info(pin, options = {})
else
CPPWSTrackingResponse.new(false, e.message, {}, :carrier => @@name)
end
rescue InvalidPinFormatError => e
rescue InvalidPinFormatError
CPPWSTrackingResponse.new(false, "Invalid Pin Format", {}, :carrier => @@name)
end

Expand All @@ -94,18 +94,18 @@ def create_shipment(origin, destination, package, line_items = [], options = {})
parse_shipment_response(response)
rescue ActiveMerchant::ResponseError, ActiveMerchant::Shipping::ResponseError => e
error_response(e.response.body, CPPWSShippingResponse)
rescue MissingCustomerNumberError => e
rescue MissingCustomerNumberError
CPPWSShippingResponse.new(false, "Missing Customer Number", {}, :carrier => @@name)
end

def retrieve_shipment(shipping_id, options = {})
response = ssl_post(shipment_url(shipping_id, options), nil, headers(options, SHIPMENT_MIMETYPE, SHIPMENT_MIMETYPE))
shipping_response = parse_shipment_response(response)
parse_shipment_response(response)
end

def find_shipment_receipt(shipping_id, options = {})
response = ssl_get(shipment_receipt_url(shipping_id, options), headers(options, SHIPMENT_MIMETYPE, SHIPMENT_MIMETYPE))
shipping_response = parse_shipment_receipt_response(response)
parse_shipment_receipt_response(response)
end

def retrieve_shipping_label(shipping_response, options = {})
Expand Down
7 changes: 2 additions & 5 deletions lib/active_shipping/shipping/carriers/fedex.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,6 @@ def build_location_node(name, location)

def parse_rate_response(origin, destination, packages, response, options)
rate_estimates = []
success, message = nil

xml = build_document(response)
root_node = xml.elements['RateReply']
Expand Down Expand Up @@ -422,14 +421,12 @@ def parse_tracking_response(response, options)
message = response_message(xml)

if success
tracking_number, shipper_address, origin, destination, status = nil
status_code, status_description, ship_time = nil
scheduled_delivery_time, actual_delivery_time, delivery_signature = nil
origin = nil
delivery_signature = nil
shipment_events = []

tracking_details = root_node.elements['TrackDetails']
tracking_number = tracking_details.get_text('TrackingNumber').to_s

status_code = tracking_details.get_text('StatusCode').to_s
status_description = tracking_details.get_text('StatusDescription').to_s
status = TRACKING_STATUS_CODES[status_code]
Expand Down
19 changes: 8 additions & 11 deletions lib/active_shipping/shipping/carriers/stamps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,45 +135,42 @@ class Stamps < Carrier

def account_info
request = build_get_account_info_request
response = commit(:GetAccountInfo, request)
commit(:GetAccountInfo, request)
end

def purchase_postage(purchase_amount, control_total)
request = build_purchase_postage_request(purchase_amount, control_total)
response = commit(:PurchasePostage, request)
commit(:PurchasePostage, request)
end

def purchase_status(transaction_id)
request = build_get_purchase_status(transaction_id)
response = commit(:GetPurchaseStatus, request)
commit(:GetPurchaseStatus, request)
end

def validate_address(address, options = {})
address = standardize_address(address)

request = build_cleanse_address_request(address)
response = commit(:CleanseAddress, request)
commit(:CleanseAddress, request)
end

def find_rates(origin, destination, package, options = {})
origin = standardize_address(origin)
destination = standardize_address(destination)

request = build_rate_request(origin, destination, package, options)
response = commit(:GetRates, request)
commit(:GetRates, request)
end

def create_shipment(origin, destination, package, line_items = [], options = {})
origin = standardize_address(origin)
destination = standardize_address(destination)

request = build_create_indicium_request(origin, destination, package, line_items, options)
response = commit(:CreateIndicium, request)
commit(:CreateIndicium, request)
end

def find_tracking_info(shipment_id, options = {})
request = build_track_shipment_request(shipment_id, options)
response = commit(:TrackShipment, request)
commit(:TrackShipment, request)
end

def namespace
Expand Down Expand Up @@ -226,7 +223,7 @@ def renew_authenticator(request)

def get_authenticator
request = build_authenticate_user_request
response = commit(:AuthenticateUser, request)
commit(:AuthenticateUser, request)
end

def build_header
Expand Down
11 changes: 4 additions & 7 deletions lib/active_shipping/shipping/carriers/ups.rb
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ def build_location_node(name, location, options = {})
# * Shipment/(ShipTo|ShipFrom)/CompanyName element
# * Shipment/(Shipper|ShipTo|ShipFrom)/AttentionName element
# * Shipment/(Shipper|ShipTo|ShipFrom)/TaxIdentificationNumber element
location_node = XmlNode.new(name) do |location_node|
XmlNode.new(name) do |location_node|
# You must specify the shipper name when creating labels.
if shipper_name = (options[:origin_name] || @options[:origin_name])
location_node << XmlNode.new('Name', shipper_name)
Expand Down Expand Up @@ -447,8 +447,6 @@ def build_package_node(package, options = {})
end

def parse_rate_response(origin, destination, packages, response, options = {})
rates = []

xml = REXML::Document.new(response)
success = response_success?(xml)
message = response_message(xml)
Expand Down Expand Up @@ -480,11 +478,10 @@ def parse_tracking_response(response, options = {})
message = response_message(xml)

if success
tracking_number, origin, destination, status_code, status_description, delivery_signature = nil
delivery_signature = nil
exception_event, scheduled_delivery_date, actual_delivery_date = nil
delivered, exception = false
shipment_events = []
status = {}

first_shipment = xml.elements['/*/Shipment']
first_package = first_shipment.elements['Package']
Expand Down Expand Up @@ -614,7 +611,7 @@ def response_digest(xml)
end

def parse_ship_confirm(response)
xml = REXML::Document.new(response)
REXML::Document.new(response)
end

def parse_ship_accept(response)
Expand Down Expand Up @@ -646,7 +643,7 @@ def service_name_for(origin, code)
end

name ||= OTHER_NON_US_ORIGIN_SERVICES[code] unless name == 'US'
name ||= DEFAULT_SERVICES[code]
name || DEFAULT_SERVICES[code]
end
end
end
Expand Down
10 changes: 3 additions & 7 deletions lib/active_shipping/shipping/carriers/usps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,8 @@ def find_rates(origin, destination, packages, options = {})
destination = Location.from(destination)
packages = Array(packages)

# raise ArgumentError.new("USPS packages must originate in the U.S.") unless ['US',nil].include?(origin.country_code(:alpha2))

# domestic or international?

domestic_codes = US_POSSESSIONS + ['US', nil]
response = if domestic_codes.include?(destination.country_code(:alpha2))
if domestic_codes.include?(destination.country_code(:alpha2))
us_rates(origin, destination, packages, options)
else
world_rates(origin, destination, packages, options)
Expand All @@ -249,7 +245,7 @@ def extract_event_details(message)
time = Time.parse(timestamp)
zoneless_time = Time.utc(time.year, time.month, time.mday, time.hour, time.min, time.sec)
location = Location.new(city: city, state: state, postal_code: zip_code, country: 'USA')
EventDetails.new($1.upcase, time, zoneless_time, location)
EventDetails.new(description, time, zoneless_time, location)
end

protected
Expand Down Expand Up @@ -538,7 +534,7 @@ def parse_tracking_response(response, options)
message = response_message(xml)

if success
tracking_number, origin, destination = nil
destination = nil
shipment_events = []
tracking_details = xml.elements.collect('*/*/TrackDetail') { |e| e }

Expand Down
2 changes: 1 addition & 1 deletion lib/vendor/quantified/test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

begin
require 'active_support/inflector'
rescue LoadError => e
rescue LoadError
require 'rubygems'
gem "activesupport", ">= 2.3.5"
require 'active_support/inflector'
Expand Down
4 changes: 2 additions & 2 deletions test/remote/canada_post_pws_platform_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def test_tracking
def test_tracking_invalid_pin_raises_exception
pin = "000000000000000"
exception = assert_raise ResponseError do
response = @cp.find_tracking_info(pin, build_options)
@cp.find_tracking_info(pin, build_options)
end
assert_equal "No Pin History", exception.message
end
Expand Down Expand Up @@ -178,7 +178,7 @@ def test_find_services_country_JP

def test_find_services_invalid_country
exception = assert_raise ResponseError do
response = @cp.find_services('XX', build_options)
@cp.find_services('XX', build_options)
end
assert_equal "A valid destination country must be supplied.", exception.message
end
Expand Down
22 changes: 11 additions & 11 deletions test/remote/fedex_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,22 +93,22 @@ def test_rates_for_locations_with_only_zip_and_country
end

def test_rates_for_location_with_only_country_code
response = @carrier.find_rates(
@locations[:bare_beverly_hills],
Location.new(:country => 'CA'),
@packages.values_at(:wii)
)
@carrier.find_rates(
@locations[:bare_beverly_hills],
Location.new(:country => 'CA'),
@packages.values_at(:wii)
)
rescue ResponseError => e
assert_match /postal code/i, e.message
assert_match /(missing|invalid)/i, e.message
end

def test_invalid_recipient_country
response = @carrier.find_rates(
@locations[:bare_beverly_hills],
Location.new(:country => 'JP', :zip => '108-8361'),
@packages.values_at(:wii)
)
@carrier.find_rates(
@locations[:bare_beverly_hills],
Location.new(:country => 'JP', :zip => '108-8361'),
@packages.values_at(:wii)
)
rescue ResponseError => e
assert_match /postal code/i, e.message
assert_match /(missing|invalid)/i, e.message
Expand Down Expand Up @@ -202,7 +202,7 @@ def test_tracking

def test_tracking_with_bad_number
assert_raises ResponseError do
response = @carrier.find_tracking_info('12345')
@carrier.find_tracking_info('12345')
end
end

Expand Down
12 changes: 6 additions & 6 deletions test/remote/kunaki_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ def test_successful_rates_request
def test_send_no_items
assert_raise(ActiveMerchant::ResponseError) do
begin
response = @carrier.find_rates(
@locations[:ottawa],
@locations[:beverly_hills],
@packages.values_at(:book, :wii),
:items => []
)
@carrier.find_rates(
@locations[:ottawa],
@locations[:beverly_hills],
@packages.values_at(:book, :wii),
:items => []
)
rescue ActiveMerchant::ResponseError => e
assert_equal 500, e.response.code.to_i
raise
Expand Down
2 changes: 1 addition & 1 deletion test/remote/shipwire_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def test_invalid_xml_raises_response_content_error
@carrier.expects(:ssl_post).returns("")

assert_raises ActiveMerchant::Shipping::ResponseContentError do
rate_estimates = @carrier.find_rates(
@carrier.find_rates(
@locations[:ottawa],
@locations[:london],
@packages.values_at(:book, :wii),
Expand Down
6 changes: 1 addition & 5 deletions test/remote/stamps_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def test_purchase_postage
purchase_amount = 10.62 # Based on the amount used in the track shipment tests
assert_nothing_raised do
account = @carrier.account_info
purchase = @carrier.purchase_postage(purchase_amount, account.control_total)
@carrier.purchase_postage(purchase_amount, account.control_total)
end
end

Expand Down Expand Up @@ -392,7 +392,6 @@ def test_us_to_us_possession
end

def test_bare_packages_domestic
response = nil
response = begin
@carrier.find_rates(
@locations[:beverly_hills], # imperial (U.S. origin)
Expand All @@ -407,7 +406,6 @@ def test_bare_packages_domestic
end

def test_bare_packages_international
response = nil
response = begin
@carrier.find_rates(
@locations[:beverly_hills], # imperial (U.S. origin)
Expand All @@ -422,7 +420,6 @@ def test_bare_packages_international
end

def test_first_class_packages_with_mail_type
response = nil
response = begin
@carrier.find_rates(
@locations[:beverly_hills], # imperial (U.S. origin)
Expand All @@ -441,7 +438,6 @@ def test_first_class_packages_with_mail_type
end

def test_first_class_packages_with_invalid_mail_type
response = nil
assert_raise ResponseError do
@carrier.find_rates(
@locations[:beverly_hills], # imperial (U.S. origin)
Expand Down
Loading

0 comments on commit c6762f6

Please sign in to comment.