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

:arrow_up::one::two: pos_invoice_pay #880

Merged
merged 2 commits into from Jan 28, 2019

Conversation

Projects
None yet
5 participants
@GabbasovDinar
Copy link
Member

GabbasovDinar commented Dec 25, 2018

No description provided.

@GabbasovDinar

This comment has been minimized.

Copy link
Member Author

GabbasovDinar commented Dec 25, 2018

I ported the pos_invoice_pay module, you can check @janbec

@ilmir-k
Copy link
Member

ilmir-k left a comment

Travis is red

@GabbasovDinar GabbasovDinar force-pushed the GabbasovDinar:12.0-pos_invoice_pay branch from 0f4885e to 036c858 Dec 26, 2018

@ilmir-k ilmir-k requested a review from yelizariev Dec 26, 2018

@Ramil-Mukhametzyanov
Copy link
Member

Ramil-Mukhametzyanov left a comment

Clicking 'Create Invoice' on invoiced order causes the 'event is not defined' error.

@GabbasovDinar

This comment has been minimized.

Copy link
Member Author

GabbasovDinar commented Dec 27, 2018

Clicking 'Create Invoice' on invoiced order causes the 'event is not defined' error.

I cannot reproduce this error

@ilmir-k

This comment has been minimized.

Copy link
Member

ilmir-k commented Dec 27, 2018

What's the next step for this PR?

@janbec

This comment has been minimized.

Copy link

janbec commented Dec 27, 2018

@GabbasovDinar GabbasovDinar force-pushed the GabbasovDinar:12.0-pos_invoice_pay branch 2 times, most recently from a2335b4 to 6aa9e48 Dec 28, 2018

@janbec

This comment has been minimized.

Copy link

janbec commented Jan 7, 2019

@GabbasovDinar
I finally managed to take a look at it. Saw that there are some additional improvements as well, nice!
I noticed the following singleton errors occuring:

  1. If you have two taxes in one invoice line, singleton error is caused on loading touch interface
  2. If you create a credit note from an invoice and choose option create refund, reconcile, and create new invoice causes singleton error when opening refund/invoice. To solve this in v11, I decorated _get_payment_info_JSON with @api.one because it had to be fast. but worked.

Everything else looks good!

@janbec

This comment has been minimized.

Copy link

janbec commented Jan 13, 2019

@GabbasovDinar
Here is a screen showing the line causing the first error:
image

hope it helps

@GabbasovDinar

This comment has been minimized.

Copy link
Member Author

GabbasovDinar commented Jan 14, 2019

I will update soon

@GabbasovDinar GabbasovDinar force-pushed the GabbasovDinar:12.0-pos_invoice_pay branch from 6aa9e48 to 7ca8b0c Jan 14, 2019

@GabbasovDinar GabbasovDinar force-pushed the GabbasovDinar:12.0-pos_invoice_pay branch 2 times, most recently from 4f087a6 to 639194c Jan 22, 2019

@GabbasovDinar

This comment has been minimized.

Copy link
Member Author

GabbasovDinar commented Jan 22, 2019

@janbec I fixed the indicated errors, you can check again

@janbec

This comment has been minimized.

Copy link

janbec commented Jan 22, 2019

@GabbasovDinar My tests passed succesfully, thank you for taking care of this! :)

@Ramil-Mukhametzyanov
Copy link
Member

Ramil-Mukhametzyanov left a comment

The POS invoice payment generates a web request with same parameters and gets the same response more 10 or more times after clicking "Create Invoice" button.
In my case, it took 10 second passed before the payment screen is displayed.

@Ramil-Mukhametzyanov
Copy link
Member

Ramil-Mukhametzyanov left a comment

Slow Invoice generation speed is independent of migration

@Ramil-Mukhametzyanov
Copy link
Member

Ramil-Mukhametzyanov left a comment

@ilmir-k Please check the PR

@janbec

This comment has been minimized.

Copy link

janbec commented Jan 23, 2019

@GabbasovDinar
Hi Dinar,
I just noticed that invoices are not printed anymore when created from sale order. Currently, as soon as we create an invoice, the option Invoice on the payment screen is pre-selected and the user can then easily print the invoice. Do you know if you removed that feature at some point of time?

image

@ilmir-k
Copy link
Member

ilmir-k left a comment

It seems there is rounding issue. When Difference account is empty, in some cases we have the warning about a change even if amount due = amount tendered

@GabbasovDinar GabbasovDinar force-pushed the GabbasovDinar:12.0-pos_invoice_pay branch from 639194c to da043f3 Jan 24, 2019

@GabbasovDinar

This comment has been minimized.

Copy link
Member Author

GabbasovDinar commented Jan 24, 2019

@janbec Hi Jan, yes, you're right. I fixed the error 👍

@yelizariev
Copy link
Member

yelizariev left a comment

Code LGTM

@ilmir-k ilmir-k merged commit 6795f68 into it-projects-llc:12.0 Jan 28, 2019

5 of 6 checks passed

coverage/coveralls Coverage decreased (-0.4%) to 79.074%
Details
Hound No violations found. Woof!
ci/branches Branch names are correct
Details
ci/runbot runbot build 27853-880-089309 (runtime 532s)
Details
codecov/patch/backend Coverage not affected when comparing cddac13...089309d
Details
codecov/patch/tests Coverage not affected when comparing cddac13...089309d
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.