accodeing / fortnox-api

Gem that abstracts Fortnox's F3 API
GNU Lesser General Public License v3.0
9 stars 8 forks source link

Question regarding model with unsaved parent #233

Open marcus-allabrf opened 1 year ago

marcus-allabrf commented 1 year ago

Here, invoice1 is the parent of invoice2:

invoice1 = Fortnox::API::Model::Invoice.new(customer_number: '1')
invoice2 = invoice1.update(due_date: '2023-01-01')

We have:

puts invoice2.customer_number # 1
puts invoice2.due_date # 2023-01-01

However, when saving invoice2:

Fortnox::API::Repository::Invoice.new.save(invoice2)

the following data is posted via POST /3/invoices/:

{"Invoice": {"DueDate": "2023-01-01"}}

but shouldn't the following data be posted or am I missing something?:

{"Invoice"=>{"CustomerNumber"=>"1", "DueDate"=>"2023-01-01"}}
ehannes commented 1 year ago

Hi @marcus-allabrf!

This should work, yes. I'll need to test this myself. I'll get back to you.

ehannes commented 1 year ago

@marcus-allabrf I've verified that this is a bug. Would you like to join the fortnox-api Slack channel and continue the discussion there? Please contact us at info@accodeing.com 😄

marcus-allabrf commented 1 year ago

Sure, I send you an email! 😄

marcus-allabrf commented 1 year ago

@ehannes Thanks for the fix 👍 . I try different edge cases in order to understand how things work. Is this still a bug related to unsaved parent?

invoice1 = Fortnox::API::Model::Invoice.new(customer_number: '1')
invoice2 = Fortnox::API::Repository::Invoice.new.save(invoice1)

invoice3 = invoice2.update(due_date: '2023-01-01')
invoice4 = invoice3.update(due_date: '2023-01-02')

puts invoice3.parent.saved? # true
puts invoice4.parent.saved? # false

Fortnox::API::Repository::Invoice.new.save(invoice3)
{"Invoice": {"DueDate": "2023-01-01"}}
Fortnox::API::Repository::Invoice.new.save(invoice4)
{"Invoice"=>{"CustomerNumber"=>"1", "DueDate"=>"2023-01-02"}} # we haven't updated customer_number

In order words, shouldn't invoice4's parent actually be invoice2 instead of invoice3 here?

ehannes commented 1 year ago

@marcus-allabrf looks like another bug, yes. I'll reopen this issue since it's not completely resolved yet.

d-Pixie commented 1 year ago

No, this is not a bug.

invoice2 is saved. invoice3 is based on that, with a change, and is hence not saved. invoice4 is based on invoice3, which is not saved, so its parent is not saved.

marcus-allabrf commented 1 year ago

I thought the purpose of parent was that when we do update_existing(entity), we get the state of the entity from the previous save and we can then do @mapper.diff(hash, parent_hash) to know which attributes to update. But if invoice4.parent.saved? is false, we will always update all attributes (even "CustomerNumber" in this case that has not been updated) due to return hash unless entity.parent.saved?.

d-Pixie commented 1 year ago

To be honest the above example is a bit outside of what we considered when we designed the initial version of the gem. Version 1 - which is coming in not too distant future - solves this problem in a different way and avoids the issue altogether.

I'll leave it up to @ehannes if he wants to fix it for the "legacy" version 🙂

marcus-allabrf commented 1 year ago

Great that it will be solved in Version 1. 👍