Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(339)

Issue 37371002: sale_payment: Initial commit

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by mshayden
Modified:
1 month, 1 week ago
Reviewers:
pokoli, ced, reviewbot
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 44

Patch Set 2 : Clean up code issues reported by reviewbot #

Total comments: 10

Patch Set 3 : Address issues from initial review by ced #

Total comments: 1

Patch Set 4 : WIP simplify and use origin #

Patch Set 5 : WIP: Modify tests, fix typos and add minimal code to allow tests to execute #

Total comments: 5

Patch Set 6 : Many improvement: enforce domain, support clearing, payment many lines etc. #

Total comments: 17

Patch Set 7 : Skip failed payment #

Patch Set 8 : Override create/write to trigger authorized instead of workflow which does not catch all changes #

Total comments: 9

Patch Set 9 : Fix pokoli's comments #

Patch Set 10 : Do not try to pay already reconciled lines #

Patch Set 11 : Do not copy payments when duplicate sales #

Patch Set 12 : Create clearing and reconcile only succeeded payment #

Total comments: 3

Patch Set 13 : Add scenario test about failing a payment #

Patch Set 14 : Add comment and ensure payment line is not reconciled #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1500 lines, -0 lines) Patch
A .drone.yml View 1 2 3 4 5 1 chunk +9 lines, -0 lines 0 comments Download
A COPYRIGHT View 1 2 3 5 1 chunk +17 lines, -0 lines 0 comments Download
A INSTALL View 1 2 3 4 5 1 chunk +32 lines, -0 lines 0 comments Download
A LICENSE View 1 2 3 5 1 chunk +674 lines, -0 lines 0 comments Download
A MANIFEST.in View 1 2 3 4 5 1 chunk +13 lines, -0 lines 0 comments Download
A README View 1 2 3 4 5 1 chunk +35 lines, -0 lines 0 comments Download
A __init__.py View 1 2 3 4 5 1 chunk +14 lines, -0 lines 0 comments Download
A account.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +178 lines, -0 lines 0 comments Download
A doc/index.rst View 1 2 3 4 5 6 7 8 1 chunk +15 lines, -0 lines 0 comments Download
A sale.py View 1 2 3 4 5 6 7 8 9 10 1 chunk +91 lines, -0 lines 0 comments Download
A sale.xml View 1 2 3 4 5 1 chunk +10 lines, -0 lines 0 comments Download
A setup.py View 1 2 3 4 5 1 chunk +128 lines, -0 lines 0 comments Download
A tests/__init__.py View 1 2 3 5 1 chunk +9 lines, -0 lines 0 comments Download
A tests/scenario_sale_payment.rst View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +212 lines, -0 lines 0 comments Download
A tests/test_sale_payment.py View 1 2 3 5 1 chunk +24 lines, -0 lines 0 comments Download
A tox.ini View 1 2 3 5 1 chunk +18 lines, -0 lines 0 comments Download
A tryton.cfg View 1 2 3 4 5 1 chunk +12 lines, -0 lines 0 comments Download
A view/sale_form.xml View 1 2 3 4 5 6 7 8 1 chunk +9 lines, -0 lines 0 comments Download

Messages

Total messages: 45
mshayden
3 months ago (2017-04-27 06:23:57 UTC) #1
reviewbot
https://codereview.tryton.org/37371002/diff/1/__init__.py#newcode4 __init__.py:4: F403 'from invoice import *' used; unable to detect undefined names https://codereview.tryton.org/37371002/diff/1/__init__.py#newcode5 __init__.py:5: ...
3 months ago (2017-04-27 06:35:08 UTC) #2
mshayden
2 months, 4 weeks ago (2017-04-27 07:09:44 UTC) #3
ced
This is just few remarks about design from an quick review. https://tryton-rietveld.appspot.com/37371002/diff/1/invoice.py File invoice.py (right): ...
2 months, 4 weeks ago (2017-04-27 07:19:11 UTC) #4
ced
Commit message for new module is: Initial commit. https://tryton-rietveld.appspot.com/37371002/diff/1/sale.py File sale.py (right): https://tryton-rietveld.appspot.com/37371002/diff/1/sale.py#newcode16 sale.py:16: payments ...
2 months, 4 weeks ago (2017-04-27 07:34:37 UTC) #5
reviewbot
https://codereview.tryton.org/37371002/diff/20001/__init__.py#newcode4 __init__.py:4: F403 'from invoice import *' used; unable to detect undefined names https://codereview.tryton.org/37371002/diff/20001/__init__.py#newcode5 __init__.py:5: ...
2 months, 4 weeks ago (2017-04-27 07:37:09 UTC) #6
mshayden
2 months, 4 weeks ago (2017-04-27 08:19:36 UTC) #7
reviewbot
https://codereview.tryton.org/37371002/diff/20002/__init__.py#newcode4 __init__.py:4: F403 'from invoice import *' used; unable to detect undefined names https://codereview.tryton.org/37371002/diff/20002/__init__.py#newcode5 __init__.py:5: ...
2 months, 4 weeks ago (2017-04-27 08:30:53 UTC) #8
mshayden
I have addressed most of the comments and will submit another update after I get ...
2 months, 4 weeks ago (2017-04-27 08:47:50 UTC) #9
ced
https://tryton-rietveld.appspot.com/37371002/diff/1/sale.py File sale.py (right): https://tryton-rietveld.appspot.com/37371002/diff/1/sale.py#newcode25 sale.py:25: depends=['currency_digits']), 'get_payments') On 2017/04/27 08:47:50, mshayden wrote: > On ...
2 months, 4 weeks ago (2017-04-27 15:37:50 UTC) #10
mshayden
Noticed that you have done a bit of work but not an update...are you working ...
2 months, 4 weeks ago (2017-04-27 17:00:44 UTC) #11
ced
On 2017/04/27 17:00:44, mshayden wrote: > Noticed that you have done a bit of work ...
2 months, 4 weeks ago (2017-04-27 20:21:47 UTC) #12
ced
WIP simplify and use origin
2 months, 4 weeks ago (2017-04-27 20:22:51 UTC) #13
mshayden
On 2017/04/27 20:22:51, ced wrote: > WIP simplify and use origin Thanks it is appreciated! ...
2 months, 4 weeks ago (2017-04-27 20:29:37 UTC) #14
reviewbot
https://codereview.tryton.org/37371002/diff/50001/__init__.py#newcode4 __init__.py:4: F403 'from invoice import *' used; unable to detect undefined names https://codereview.tryton.org/37371002/diff/50001/__init__.py#newcode5 __init__.py:5: ...
2 months, 4 weeks ago (2017-04-27 20:38:36 UTC) #15
mshayden
2 months, 4 weeks ago (2017-04-28 07:31:05 UTC) #16
mshayden
Decided to see if I could make this WIP work--I like the simplified approach; it ...
2 months, 4 weeks ago (2017-04-28 07:43:10 UTC) #17
reviewbot
https://codereview.tryton.org/37371002/diff/70001/__init__.py#newcode4 __init__.py:4: F403 'from invoice import *' used; unable to detect undefined names https://codereview.tryton.org/37371002/diff/70001/__init__.py#newcode5 __init__.py:5: ...
2 months, 4 weeks ago (2017-04-28 08:09:56 UTC) #18
ced
https://tryton-rietveld.appspot.com/37371002/diff/70001/sale.py File sale.py (right): https://tryton-rietveld.appspot.com/37371002/diff/70001/sale.py#newcode17 sale.py:17: if not all((p.state == 'draft' or p.state == 'failed' ...
2 months, 4 weeks ago (2017-04-28 10:25:43 UTC) #19
ced
Many improvement: enforce domain, support clearing, payment many lines etc.
2 months, 4 weeks ago (2017-04-28 15:10:26 UTC) #20
mshayden
I addressed your comments and will submit one more time along with modified tests and ...
2 months, 4 weeks ago (2017-04-28 15:24:40 UTC) #21
mshayden
Disregard--looks like you have things in pretty good shape! https://tryton-rietveld.appspot.com/37371002/diff/90001/sale.py File sale.py (right): https://tryton-rietveld.appspot.com/37371002/diff/90001/sale.py#newcode72 sale.py:72: ...
2 months, 4 weeks ago (2017-04-28 15:27:33 UTC) #22
reviewbot
https://codereview.tryton.org/37371002/diff/90001/setup.py#newcode89 setup.py:89: E501 line too long (85 > 79 characters) URL: https://codereview.tryton.org/37371002
2 months, 4 weeks ago (2017-04-28 15:41:26 UTC) #23
pokoli
https://tryton-rietveld.appspot.com/37371002/diff/90001/account.py File account.py (right): https://tryton-rietveld.appspot.com/37371002/diff/90001/account.py#newcode77 account.py:77: def process(cls, payments, group): why confirm sales on process? ...
2 months, 3 weeks ago (2017-05-04 16:01:50 UTC) #24
ced
Skip failed payment
2 months, 3 weeks ago (2017-05-05 15:27:05 UTC) #25
reviewbot
https://codereview.tryton.org/37371002/diff/110001/setup.py#newcode89 setup.py:89: E501 line too long (85 > 79 characters) URL: https://codereview.tryton.org/37371002
2 months, 3 weeks ago (2017-05-05 15:36:45 UTC) #26
ced
Override create/write to trigger authorized instead of workflow which does not catch all changes
2 months, 3 weeks ago (2017-05-05 16:41:38 UTC) #27
reviewbot
https://codereview.tryton.org/37371002/diff/110002/setup.py#newcode89 setup.py:89: E501 line too long (85 > 79 characters) URL: https://codereview.tryton.org/37371002
2 months, 3 weeks ago (2017-05-05 17:10:55 UTC) #28
pokoli
https://tryton-rietveld.appspot.com/37371002/diff/110002/doc/index.rst File doc/index.rst (right): https://tryton-rietveld.appspot.com/37371002/diff/110002/doc/index.rst#newcode5 doc/index.rst:5: of an invoice. of an -> of any https://tryton-rietveld.appspot.com/37371002/diff/110002/doc/index.rst#newcode11 ...
2 months, 2 weeks ago (2017-05-10 10:43:40 UTC) #29
ced
Fix pokoli's comments
2 months, 1 week ago (2017-05-18 07:00:53 UTC) #30
reviewbot
https://codereview.tryton.org/37371002/diff/130001/setup.py#newcode89 setup.py:89: E501 line too long (85 > 79 characters) URL: https://codereview.tryton.org/37371002
2 months, 1 week ago (2017-05-18 07:08:59 UTC) #31
ced
Do not try to pay already reconciled lines
2 months ago (2017-05-24 14:23:54 UTC) #32
ced
https://tryton-rietveld.appspot.com/37371002/diff/90001/account.py File account.py (right): https://tryton-rietveld.appspot.com/37371002/diff/90001/account.py#newcode77 account.py:77: def process(cls, payments, group): On 2017/05/04 16:01:49, pokoli wrote: ...
2 months ago (2017-05-24 14:25:15 UTC) #33
reviewbot
https://codereview.tryton.org/37371002/diff/150001/setup.py#newcode89 setup.py:89: E501 line too long (85 > 79 characters) URL: https://codereview.tryton.org/37371002
2 months ago (2017-05-24 14:32:48 UTC) #34
ced
Do not copy payments when duplicate sales
1 month, 1 week ago (2017-06-12 09:15:54 UTC) #35
reviewbot
https://codereview.tryton.org/37371002/diff/170001/setup.py#newcode89 setup.py:89: E501 line too long (85 > 79 characters) URL: https://codereview.tryton.org/37371002
1 month, 1 week ago (2017-06-12 09:39:32 UTC) #36
ced
Create clearing and reconcile only succeeded payment
1 month, 1 week ago (2017-06-13 13:19:05 UTC) #37
reviewbot
https://codereview.tryton.org/37371002/diff/190001/setup.py#newcode89 setup.py:89: E501 line too long (85 > 79 characters) URL: https://codereview.tryton.org/37371002
1 month, 1 week ago (2017-06-13 13:28:46 UTC) #38
pokoli
https://tryton-rietveld.appspot.com/37371002/diff/190001/account.py File account.py (right): https://tryton-rietveld.appspot.com/37371002/diff/190001/account.py#newcode166 account.py:166: move = payment.create_clearing_move() This is already done by the ...
1 month, 1 week ago (2017-06-13 13:35:11 UTC) #39
ced
Add scenario test about failing a payment
1 month, 1 week ago (2017-06-13 13:48:51 UTC) #40
reviewbot
https://codereview.tryton.org/37371002/diff/210001/setup.py#newcode89 setup.py:89: E501 line too long (85 > 79 characters) URL: https://codereview.tryton.org/37371002
1 month, 1 week ago (2017-06-13 14:02:26 UTC) #41
ced
Add comment and ensure payment line is not reconciled
1 month, 1 week ago (2017-06-13 14:15:54 UTC) #42
ced
https://tryton-rietveld.appspot.com/37371002/diff/190001/account.py File account.py (right): https://tryton-rietveld.appspot.com/37371002/diff/190001/account.py#newcode166 account.py:166: move = payment.create_clearing_move() On 2017/06/13 13:35:11, pokoli wrote: > ...
1 month, 1 week ago (2017-06-13 14:16:11 UTC) #43
reviewbot
https://codereview.tryton.org/37371002/diff/230001/setup.py#newcode89 setup.py:89: E501 line too long (85 > 79 characters) URL: https://codereview.tryton.org/37371002
1 month, 1 week ago (2017-06-13 14:28:46 UTC) #44
pokoli
1 month, 1 week ago (2017-06-13 15:19:27 UTC) #45
https://tryton-rietveld.appspot.com/37371002/diff/190001/account.py
File account.py (right):

https://tryton-rietveld.appspot.com/37371002/diff/190001/account.py#newcode166
account.py:166: move = payment.create_clearing_move()
On 2017/06/13 14:16:11, ced wrote:
> On 2017/06/13 13:35:11, pokoli wrote:
> > This is already done by the succeed transition on account_payment_clearing
> > module. Why we need to duplicate it here? 
> > 
> 
> Added a comment.

Acknowledged.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cd18842