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

Issue 49501002: party: Add subdivision type restriction on address

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months, 2 weeks ago by pokoli
Modified:
3 weeks, 4 days ago
Reviewers:
dave, ced, reviewbot
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use Array field #

Total comments: 3

Patch Set 3 : Add missing copyright and remove trailing coma #

Patch Set 4 : Add type on array field #

Total comments: 4

Patch Set 5 : Use MatchMixin, support country instance and improve model name #

Patch Set 6 : Improve changelog #

Patch Set 7 : Update to tip #

Total comments: 3

Patch Set 8 : Use fields.Char instead of fields.Many2One for array field #

Unified diffs Side-by-side diffs Delta from patch set Stats (+161 lines, -4 lines) Patch
M CHANGELOG View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M __init__.py View 1 1 chunk +1 line, -0 lines 0 comments Download
M address.py View 1 2 3 4 5 6 7 4 chunks +72 lines, -4 lines 0 comments Download
M address.xml View 1 2 chunks +69 lines, -0 lines 0 comments Download
A view/address_subdivision_type_form.xml View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
A view/address_subdivision_type_list.xml View 1 2 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 27
pokoli
7 months, 2 weeks ago (2018-07-05 11:58:17 UTC) #1
reviewbot
flake8 OK URL: https://codereview.tryton.org/49501002
7 months, 2 weeks ago (2018-07-05 12:15:58 UTC) #2
ced
I do not find very robust to use context to add blindly a domain clause. ...
7 months, 1 week ago (2018-07-11 13:23:20 UTC) #3
pokoli
The main problem with this desing is that we should accept a list of types ...
6 months, 2 weeks ago (2018-08-02 11:08:21 UTC) #4
ced
On 2018/08/02 11:08:21, pokoli wrote: > The main problem with this desing is that we ...
6 months, 2 weeks ago (2018-08-02 11:39:37 UTC) #5
pokoli
On 2018/08/02 11:39:37, ced wrote: > On 2018/08/02 11:08:21, pokoli wrote: > > The main ...
6 months, 2 weeks ago (2018-08-02 11:53:45 UTC) #6
ced
On 2018/08/02 11:53:45, pokoli wrote: > On 2018/08/02 11:39:37, ced wrote: > > On 2018/08/02 ...
6 months, 2 weeks ago (2018-08-02 14:02:38 UTC) #7
ced
On 2018/08/02 14:02:38, ced wrote: > On 2018/08/02 11:53:45, pokoli wrote: > > On 2018/08/02 ...
6 months, 2 weeks ago (2018-08-03 06:42:14 UTC) #8
pokoli
Use Array field
3 months ago (2018-11-11 20:34:22 UTC) #9
reviewbot
flake8 OK URL: https://codereview.tryton.org/49501002
3 months ago (2018-11-11 20:38:27 UTC) #10
ced
https://codereview.tryton.org/49501002/diff/20001/address.py File address.py (right): https://codereview.tryton.org/49501002/diff/20001/address.py#newcode52 address.py:52: (), This trailing coma is not really useful https://codereview.tryton.org/49501002/diff/20001/view/address_subdivision_type_form.xml ...
2 months, 4 weeks ago (2018-11-19 20:43:25 UTC) #11
pokoli
Add missing copyright and remove trailing coma
2 months, 4 weeks ago (2018-11-20 17:28:19 UTC) #12
reviewbot
flake8 OK URL: https://codereview.tryton.org/49501002
2 months, 4 weeks ago (2018-11-20 17:46:51 UTC) #13
pokoli
Add type on array field
2 months, 3 weeks ago (2018-11-21 11:14:51 UTC) #14
reviewbot
flake8 OK URL: https://codereview.tryton.org/49501002
2 months, 3 weeks ago (2018-11-21 11:43:27 UTC) #15
dave
https://codereview.tryton.org/49501002/diff/60001/CHANGELOG File CHANGELOG (right): https://codereview.tryton.org/49501002/diff/60001/CHANGELOG#newcode1 CHANGELOG:1: * Add per country subdivision type restriction What about ...
2 months, 3 weeks ago (2018-11-21 12:02:37 UTC) #16
ced
https://codereview.tryton.org/49501002/diff/60001/address.py File address.py (right): https://codereview.tryton.org/49501002/diff/60001/address.py#newcode231 address.py:231: return [] I find it will be more flexible ...
2 months, 3 weeks ago (2018-11-26 21:36:40 UTC) #17
pokoli
Use MatchMixin, support country instance and improve model name
2 months, 1 week ago (2018-12-10 11:26:12 UTC) #18
pokoli
Improve changelog
2 months, 1 week ago (2018-12-10 11:28:06 UTC) #19
reviewbot
flake8 OK URL: https://codereview.tryton.org/49501002
2 months, 1 week ago (2018-12-10 11:40:57 UTC) #20
pokoli
Update to tip
1 month, 2 weeks ago (2018-12-30 12:31:11 UTC) #21
reviewbot
flake8 OK URL: https://codereview.tryton.org/49501002
1 month, 2 weeks ago (2018-12-30 12:43:54 UTC) #22
ced
https://codereview.tryton.org/49501002/diff/120001/address.py File address.py (right): https://codereview.tryton.org/49501002/diff/120001/address.py#newcode47 address.py:47: fields.Array(fields.Many2One, "Country Subdivision Types"), Can you use a One2Many ...
3 weeks, 4 days ago (2019-01-22 10:16:35 UTC) #23
pokoli
Use fields.Char instead of fields.Many2One for array field
3 weeks, 4 days ago (2019-01-22 10:32:43 UTC) #24
pokoli
https://codereview.tryton.org/49501002/diff/120001/address.py File address.py (right): https://codereview.tryton.org/49501002/diff/120001/address.py#newcode47 address.py:47: fields.Array(fields.Many2One, "Country Subdivision Types"), On 2019/01/22 10:16:35, ced wrote: ...
3 weeks, 4 days ago (2019-01-22 10:35:39 UTC) #25
reviewbot
flake8 OK URL: https://codereview.tryton.org/49501002
3 weeks, 4 days ago (2019-01-22 10:39:50 UTC) #26
ced
3 weeks, 4 days ago (2019-01-22 11:23:35 UTC) #27
https://codereview.tryton.org/49501002/diff/120001/address.py
File address.py (right):

https://codereview.tryton.org/49501002/diff/120001/address.py#newcode47
address.py:47: fields.Array(fields.Many2One, "Country Subdivision Types"),
On 2019/01/22 10:35:39, pokoli wrote:
> On 2019/01/22 10:16:35, ced wrote:
> > Can you use a One2Many instead of Array?
> 
> No, as the array is a Char instead of a Many2One (I have fixed it on next
> patchset). It references the country.subdivision's type field which is a
> selection. 
> 
> If we want to use a One2Many we should convert the selection into a Many2One
and
> define the values on xml. Not sure if it's worth it. 

Then I think it is better to use MultiSelection than Array (see my comments on
Array issue).
Sign in to reply to this message.

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