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

Issue 43211002: analytic_account: Add company domain on AnalyticMixin analytic accounts (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 months, 2 weeks ago by pokoli
Modified:
8 months ago
Reviewers:
ced, reviewbot
Visibility:
Public.

Description

Ensure that the accounts of the analytic mixin are only linked to the company of the parent issue7198

Patch Set 1 #

Total comments: 1

Patch Set 2 : Use company field of the model #

Total comments: 1

Patch Set 3 : Fail if no company field and allow to configure the field name #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -0 lines) Patch
M account.py View 1 2 2 chunks +8 lines, -0 lines 2 comments Download

Messages

Total messages: 10
pokoli
10 months, 2 weeks ago (2018-03-08 10:19:11 UTC) #1
reviewbot
https://codereview.tryton.org/43211002/diff/1/account.py#newcode215 account.py:215: E712 comparison to True should be 'if cond is True:' or 'if cond:' ...
10 months, 2 weeks ago (2018-03-08 10:24:37 UTC) #2
ced
https://codereview.tryton.org/43211002/diff/1/account.py File account.py (right): https://codereview.tryton.org/43211002/diff/1/account.py#newcode507 account.py:507: ], It is wrong to use context for domain. ...
10 months, 2 weeks ago (2018-03-08 10:39:02 UTC) #3
pokoli
Use company field of the model
9 months, 1 week ago (2018-04-18 08:32:40 UTC) #4
ced
https://codereview.tryton.org/43211002/diff/20001/account.py File account.py (right): https://codereview.tryton.org/43211002/diff/20001/account.py#newcode510 account.py:510: if hasattr(cls, 'company'): It does not ensure that company ...
9 months, 1 week ago (2018-04-18 08:55:23 UTC) #5
ced
Also I think the description should explain what the domain addition is fixing.
9 months, 1 week ago (2018-04-18 08:56:00 UTC) #6
reviewbot
https://codereview.tryton.org/43211002/diff/20001/account.py#newcode13 account.py:13: F401 'Bool' imported but unused https://codereview.tryton.org/43211002/diff/20001/account.py#newcode210 account.py:210: E712 comparison to True should be ...
9 months, 1 week ago (2018-04-18 08:56:32 UTC) #7
pokoli
Fail if no company field and allow to configure the field name
8 months ago (2018-05-21 09:57:43 UTC) #8
reviewbot
https://codereview.tryton.org/43211002/diff/40001/account.py#newcode211 account.py:211: E712 comparison to True should be 'if cond is True:' or 'if cond:' ...
8 months ago (2018-05-21 10:19:59 UTC) #9
ced
8 months ago (2018-05-23 21:39:22 UTC) #10
https://codereview.tryton.org/43211002/diff/40001/account.py
File account.py (right):

https://codereview.tryton.org/43211002/diff/40001/account.py#newcode516
account.py:516: 'Missing analytic company on analytic mixin %s' % cls.__name__)
When could this happen? The attribute is defined in the mixin.

https://codereview.tryton.org/43211002/diff/40001/account.py#newcode517
account.py:517: cls.analytic_accounts.domain.append(
You can not happen like that because you do not know if the current list is
AND-ed or OR-ed.
Sign in to reply to this message.

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