Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allowing re-use of the same invoice numbers in ACCPAY invoices #125

Closed
sinad opened this issue Jan 27, 2014 · 5 comments
Closed

Allowing re-use of the same invoice numbers in ACCPAY invoices #125

sinad opened this issue Jan 27, 2014 · 5 comments

Comments

@sinad
Copy link

sinad commented Jan 27, 2014

We often have cases where we use the same invoice numbers for different purchase invoices. (E.g. Vendor 1 has invoice #1001 and then Vendor 2 also bills us with an invoice #1001.)

When attempting to create a new invoice with the same invoice number, it looks like Xeroizer is fetching the existing invoice with the same invoice number and updating / overwriting it (or throwing an error if the original invoice can't be modified).

I was just wondering why it's necessary for Xeroizer to use "invoice_number" in the "possible_primary_keys" hash given that "invoice_id" seems like a solid unique identifier.

The Xero API docs state ACCPAY invoice numbers are non-unique (http://developer.xero.com/documentation/api/invoices/), which I would argue makes it an unreliable field as a possible primary key.

The code I'm using looks something like this:

xero_invoice = xero_client.Invoice.build(type: 'ACCPAY', date: Date.today, due_date: 30.days.from_now, invoice_number: '1001', line_amount_types: 'NoTax', status: 'AUTHORISED')
... add line items ...
xero_invoice.save

My workaround is to save the new invoice without an invoice number, assign the invoice, and then re-save (adding some extraneous API calls):

x_invoice.save
x_invoice.invoice_number = '1001'
x_invoice.save

Thoughts on removing "invoice_number" as a possible primary key?

@waynerobinson
Copy link
Owner

This makes sense. AP has never been part of my use case. Happy to receive a
pull request or I'll try and get a look at this during the week.

On 28 January 2014 04:51, sinad notifications@github.com wrote:

We often have cases where we use the same invoice numbers for different
purchase invoices. (E.g. Vendor 1 has invoice #1001 and then Vendor 2 also
bills us with an invoice #1001.)

When attempting to create a new invoice with the same invoice number, it
looks like Xeroizer is fetching the existing invoice with the same invoice
number and updating / overwriting it (or throwing an error if the original
invoice can't be modified).

I was just wondering why it's necessary for Xeroizer to use
"invoice_number" in the "possible_primary_keys" hash given that
"invoice_id" seems like a solid unique identifier.

The Xero API docs state ACCPAY invoice numbers are non-unique (
http://developer.xero.com/documentation/api/invoices/), which I would
argue makes it an unreliable field as a possible primary key.

The code I'm using looks something like this:

xero_invoice = xero_client.Invoice.build(type: 'ACCPAY', date: Date.today, due_date: 30.days.from_now, invoice_number: '1001', line_amount_types: 'NoTax', status: 'AUTHORISED')... add line items ...xero_invoice.save

My workaround is to save the new invoice without an invoice number, assign
the invoice, and then re-save (adding some extraneous API calls):

x_invoice.savex_invoice.invoice_number = '1001'x_invoice.save


Reply to this email directly or view it on GitHubhttps://github.com//issues/125
.

@waynerobinson
Copy link
Owner

Still happy to receive a PR on this issue. Sounds like something Xeroizer should have but I haven't run into this problem specifically myself.

@CloCkWeRX
Copy link
Collaborator

There's a lot of other calls that have foo_id vs foo_number

  • Receipt
  • CreditNote
  • Invoice
  • Contact

Is raising an exception on update the only purpose of possible_primary_keys?

        def update
          if self.class.possible_primary_keys && self.class.possible_primary_keys.all? { | possible_key | self[possible_key].nil? }
            raise RecordKeyMustBeDefined.new(self.class.possible_primary_keys)
          end

          request = to_xml

          log "[UPDATE SENT] (#{__FILE__}:#{__LINE__}) \r\n#{request}"

          response = parent.http_post(request)

          log "[UPDATE RECEIVED] (#{__FILE__}:#{__LINE__}) \r\n#{response}"

          parse_save_response(response)
        end

@CloCkWeRX
Copy link
Collaborator

@sinad can you maybe do up a test case that demonstrates the behaviour?

Under the hood, methods like find are just doing:

          response_xml = @application.http_get(@application.client, "#{url}/#{CGI.escape(id)}", options)

and all is doing

          response_xml = http_get(parse_params(options))

Unless the underlying API is returning the wrong invoices by ID (ie: looking up invoice number) or you are calling things a bit funny, it's hard to see how invoice_number could be returning the wrong records after create/first save.

InvoiceTest

... has some pretty straight forward example tests to copy

@rjaus
Copy link
Collaborator

rjaus commented Oct 19, 2017

This seems to have been resolved, invoices are created as expected.

xero_invoice = xero.Invoice.build(type: 'ACCPAY', date: Date.today, due_date: 30.days.from_now, invoice_number: '1001', line_amount_types: 'NoTax', status: 'AUTHORISED')
xero_invoice.add_line_item(:description => 'test', :unit_amount => '200.00', :quantity => '1', :account_code => '200')
xero_invoice.build_contact(:name => "test123")
xero_invoice.save

another_xero_invoice = xero.Invoice.build(type: 'ACCPAY', date: Date.today, due_date: 30.days.from_now, invoice_number: '1001', line_amount_types: 'NoTax', status: 'AUTHORISED')
another_xero_invoice.add_line_item(:description => 'test', :unit_amount => '200.00', :quantity => '1', :account_code => '200')
another_xero_invoice.build_contact(:name => "test123")
another_xero_invoice.save

Returning an invoice by invoice_number works, but if there are multiple invoices with that invoice_number, it will only return one. You should use the invoice_id instead.

# find by invoice_number (possible duplicates)
invoice = xero.Invoice.find("1001")
# find by invoice_id (no possible duplicates)
invoice = xero.Invoice.find("6a5f03b1-7307-4554-b236-1f7c33589e06")

If I'm missing the point here, please re-open the issue.

@rjaus rjaus closed this as completed Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants