Update saved model with nil value

[ehannes]

This flow currently leads to a strange bug:

[1] pry(main)> repository = Fortnox::API::Repository::Customer.new
=> #<Fortnox::API::Repository::Customer:0x0000000107b32218 @keys_filtered_on_save=[:url], @mapper=#<Fortnox::API::Mapper::Customer:0x0000000107b32010>>

[2] pry(main)> new_customer = Fortnox::API::Model::Customer.new(name: 'Test')
=> #<Fortnox::API::Model::Customer url=nil address1=nil address2=nil city=nil country=nil comments=nil currency=nil cost_center=nil country_code=nil customer_number=nil default_delivery_types=nil default_templates=nil delivery_address1=nil delivery_address2=nil delivery_city=nil delivery_country=nil delivery_country_code=nil delivery_fax=nil delivery_name=nil delivery_phone1=nil delivery_phone2=nil delivery_zip_code=nil email=nil email_invoice=nil email_invoice_bcc=nil email_invoice_cc=nil email_offer=nil email_offer_bcc=nil email_offer_cc=nil email_order=nil email_order_bcc=nil email_order_cc=nil fax=nil invoice_administration_fee=nil invoice_discount=nil invoice_freight=nil invoice_remark=nil name="Test" organisation_number=nil our_reference=nil phone1=nil phone2=nil price_list=nil project=nil sales_account=nil show_price_vat_included=nil terms_of_delivery=nil terms_of_payment=nil type=nil vat_number=nil vat_type=nil visiting_address=nil visiting_city=nil visiting_country=nil visiting_country_code=nil visiting_zip_code=nil way_of_delivery=nil your_reference=nil zip_code=nil>

[3] pry(main)> saved_customer = repository.save(new_customer)
=> #<Fortnox::API::Model::Customer url="https://api.fortnox.se/3/customers/10" address1=nil address2=nil city=nil country=nil comments=nil currency="SEK" cost_center=nil country_code=nil customer_number="10" default_delivery_types=#<Fortnox::API::Types::DefaultDeliveryTypes invoice="PRINT" order="PRINT" offer="PRINT"> default_templates=#<Fortnox::API::Types::DefaultTemplates order="DEFAULTTEMPLATE" offer="DEFAULTTEMPLATE" invoice="DEFAULTTEMPLATE" cash_invoice="DEFAULTTEMPLATE"> delivery_address1=nil delivery_address2=nil delivery_city=nil delivery_country=nil delivery_country_code=nil delivery_fax=nil delivery_name=nil delivery_phone1=nil delivery_phone2=nil delivery_zip_code=nil email="" email_invoice="" email_invoice_bcc="" email_invoice_cc="" email_offer="" email_offer_bcc="" email_offer_cc="" email_order="" email_order_bcc="" email_order_cc="" fax=nil invoice_administration_fee=nil invoice_discount=nil invoice_freight=nil invoice_remark="" name="Test" organisation_number="" our_reference="" phone1=nil phone2=nil price_list="A" project="" sales_account=nil show_price_vat_included=false terms_of_delivery="" terms_of_payment="" type="COMPANY" vat_number="" vat_type="SEVAT" visiting_address=nil visiting_city=nil visiting_country=nil visiting_country_code=nil visiting_zip_code=nil way_of_delivery="" your_reference="" zip_code=nil>

[4] pry(main)> puts saved_customer.saved?
true
=> nil

updated_customer = saved_customer.update(address1: nil)
=> #<Fortnox::API::Model::Customer url="https://api.fortnox.se/3/customers/10" address1=nil address2=nil city=nil country=nil comments=nil currency="SEK" cost_center=nil country_code=nil customer_number="10" default_delivery_types=#<Fortnox::API::Types::DefaultDeliveryTypes invoice="PRINT" order="PRINT" offer="PRINT"> default_templates=#<Fortnox::API::Types::DefaultTemplates order="DEFAULTTEMPLATE" offer="DEFAULTTEMPLATE" invoice="DEFAULTTEMPLATE" cash_invoice="DEFAULTTEMPLATE"> delivery_address1=nil delivery_address2=nil delivery_city=nil delivery_country=nil delivery_country_code=nil delivery_fax=nil delivery_name=nil delivery_phone1=nil delivery_phone2=nil delivery_zip_code=nil email="" email_invoice="" email_invoice_bcc="" email_invoice_cc="" email_offer="" email_offer_bcc="" email_offer_cc="" email_order="" email_order_bcc="" email_order_cc="" fax=nil invoice_administration_fee=nil invoice_discount=nil invoice_freight=nil invoice_remark="" name="Test" organisation_number="" our_reference="" phone1=nil phone2=nil price_list="A" project="" sales_account=nil show_price_vat_included=false terms_of_delivery="" terms_of_payment="" type="COMPANY" vat_number="" vat_type="SEVAT" visiting_address=nil visiting_city=nil visiting_country=nil visiting_country_code=nil visiting_zip_code=nil way_of_delivery="" your_reference="" zip_code=nil>

[5] pry(main)> updated_customer.saved?
=> false

[6] pry(main)> repository.save(updated_customer)
Fortnox::API::RemoteServerError: Felaktigt fältnamn (Customer)
from ~/fortnox-api/lib/fortnox/api/request_handling.rb:13:in `raise_api_error'

The request body to Fortnox looks like this:

{\"Customer\":{}}

The problem that we remove all nil values before pushing data to Fortnox. In this case it means we are removing all values and sends an empty object to Fortnox.

We can catch this error. The problem in my opinion is updated_customer.saved? => false.

This is the update method:

        def update(hash)
          old_attributes = to_hash
          new_attributes = old_attributes.merge(hash)

          return self if new_attributes == old_attributes

          new_hash = new_attributes.delete_if { |_, value| value.nil? }
          new_hash[:new] = @new
          new_hash[:parent] = self
          self.class.new(new_hash)
        end

The to_hash function removes nil values and we are also removing nil values when we are creating the new_hash, but we are allowing nil values as input in the hash argument. We could easily just delete the nil values from new_attributes before comparing it to old_attributes to avoid this problem. Then this method would return self, which would result in updated_customer.saved? => true meaning no request will be made.