Issue 183121: Naming of tab security and groups (Closed)
Patch Set 1 #
Total comments: 18
Patch Set 2 : Modified proposal without migration code yet #
Total comments: 3
Patch Set 3 : Reverting some items #
Total comments: 2
Patch Set 4 : Users -> Members #
MessagesTotal messages: 22
http://codereview.appspot.com/183121/diff/1/2 File trytond/res/group.xml (right): http://codereview.appspot.com/183121/diff/1/2#newcode25 trytond/res/group.xml:25: <page string="Access Permissions" col="1" id="security"> You should also change the id http://codereview.appspot.com/183121/diff/1/3 File trytond/res/ir.xml (right): http://codereview.appspot.com/183121/diff/1/3#newcode358 trytond/res/ir.xml:358: <field name="operand">User/Group Membership</field> You should create a migration script for all ir.rule http://codereview.appspot.com/183121/diff/1/4 File trytond/res/user.py (right): http://codereview.appspot.com/183121/diff/1/4#newcode34 trytond/res/user.py:34: 'uid', 'gid', 'Group Membership') It doesn't look like other naming. http://codereview.appspot.com/183121/diff/1/5 File trytond/res/user.xml (right): http://codereview.appspot.com/183121/diff/1/5#newcode40 trytond/res/user.xml:40: <page string="Group Membership" col="1" id="security"> It must have the same name as in group.xml http://codereview.appspot.com/183121/diff/1/5#newcode79 trytond/res/user.xml:79: <page string="Group Membership" col="1" id="security"> Idem
Sign in to reply to this message.
http://codereview.appspot.com/183121/diff/1/2 File trytond/res/group.xml (right): http://codereview.appspot.com/183121/diff/1/2#newcode25 trytond/res/group.xml:25: <page string="Access Permissions" col="1" id="security"> On 2010/01/06 09:03:30, ced wrote: > You should also change the id Wouldn't changing the ID possible break existing? For my understanding, an ID should never change. The page's name is just for humans, the IDs are for machines. (But maybe this is used different in Tryton.) http://codereview.appspot.com/183121/diff/1/3 File trytond/res/ir.xml (right): http://codereview.appspot.com/183121/diff/1/3#newcode358 trytond/res/ir.xml:358: <field name="operand">User/Group Membership</field> On 2010/01/06 09:03:30, ced wrote: > You should create a migration script for all ir.rule yangoon: I already have some script doing similar stuff. I'll send it to you today. http://codereview.appspot.com/183121/diff/1/5 File trytond/res/user.xml (right): http://codereview.appspot.com/183121/diff/1/5#newcode40 trytond/res/user.xml:40: <page string="Group Membership" col="1" id="security"> Should be the same name as in group.xml to keep the UI consistent.
Sign in to reply to this message.
http://codereview.appspot.com/183121/diff/1/2 File trytond/res/group.xml (right): http://codereview.appspot.com/183121/diff/1/2#newcode25 trytond/res/group.xml:25: <page string="Access Permissions" col="1" id="security"> On 2010/01/06 10:44:59, h.goebel wrote: > On 2010/01/06 09:03:30, ced wrote: > > You should also change the id > > Wouldn't changing the ID possible break existing? For my understanding, an ID > should never change. The page's name is just for humans, the IDs are for > machines. (But maybe this is used different in Tryton.) It is for coherence. We do it when changing the name of field or models.
Sign in to reply to this message.
http://codereview.appspot.com/183121/diff/1/2 File trytond/res/group.xml (right): http://codereview.appspot.com/183121/diff/1/2#newcode25 trytond/res/group.xml:25: <page string="Access Permissions" col="1" id="security"> On 2010/01/06 09:03:30, ced wrote: > You should also change the id So I have to provide migration for those ids, too? http://codereview.appspot.com/183121/diff/1/4 File trytond/res/user.py (right): http://codereview.appspot.com/183121/diff/1/4#newcode34 trytond/res/user.py:34: 'uid', 'gid', 'Group Membership') On 2010/01/06 09:03:30, ced wrote: > It doesn't look like other naming. I don't understand. What would you prefer, since it is mot an interface to generic groups? http://codereview.appspot.com/183121/diff/1/5 File trytond/res/user.xml (right): http://codereview.appspot.com/183121/diff/1/5#newcode40 trytond/res/user.xml:40: <page string="Group Membership" col="1" id="security"> On 2010/01/06 10:44:59, h.goebel wrote: > Should be the same name as in group.xml to keep the UI consistent. That is exactly the problem: While groups in group.py and group.xml are generic groups, they are used in user.py and user.xml as user groups only showing the groups the user belongs to.
Sign in to reply to this message.
http://codereview.appspot.com/183121/diff/1/2 File trytond/res/group.xml (right): http://codereview.appspot.com/183121/diff/1/2#newcode25 trytond/res/group.xml:25: <page string="Access Permissions" col="1" id="security"> On 2010/01/06 13:38:12, yangoon wrote: > On 2010/01/06 09:03:30, ced wrote: > > You should also change the id > > So I have to provide migration for those ids, too? No http://codereview.appspot.com/183121/diff/1/4 File trytond/res/user.py (right): http://codereview.appspot.com/183121/diff/1/4#newcode34 trytond/res/user.py:34: 'uid', 'gid', 'Group Membership') On 2010/01/06 13:38:12, yangoon wrote: > On 2010/01/06 09:03:30, ced wrote: > > It doesn't look like other naming. > > I don't understand. What would you prefer, since it is mot an interface to > generic groups? It is one. http://codereview.appspot.com/183121/diff/1/5 File trytond/res/user.xml (right): http://codereview.appspot.com/183121/diff/1/5#newcode40 trytond/res/user.xml:40: <page string="Group Membership" col="1" id="security"> On 2010/01/06 13:38:12, yangoon wrote: > On 2010/01/06 10:44:59, h.goebel wrote: > > Should be the same name as in group.xml to keep the UI consistent. > > That is exactly the problem: > While groups in group.py and group.xml are generic groups, they are used in > user.py and user.xml as user groups only showing the groups the user belongs to. I mean "Access Permissions"
Sign in to reply to this message.
I have joined some screenshots to http://bugs.tryton.org/roundup/issue1339 to illustrate the current patch: 1) It shows 'Group Membership' where the interface is showing the selection of the groups the user belongs to. 2) It shows 'Access Permissions' on groups, where they are indeed implemented. 3) It shows 'Groups' where generally all groups are related to.
Sign in to reply to this message.
On 2010/01/09 13:03:07, yangoon wrote: > I have joined some screenshots to http://bugs.tryton.org/roundup/issue1339 to > illustrate the current patch: Thanks for this change. This make it much clearer and more understandable.
Sign in to reply to this message.
On 2010/01/09 13:03:07, yangoon wrote: > I have joined some screenshots to http://bugs.tryton.org/roundup/issue1339 to > illustrate the current patch: > 1) It shows 'Group Membership' where the interface is showing the selection of > the groups the user belongs to. > 2) It shows 'Access Permissions' on groups, where they are indeed implemented. > 3) It shows 'Groups' where generally all groups are related to. @ced: Before continuing with the patch I would like to know, if the concept is accepted.
Sign in to reply to this message.
On 2010/01/12 21:20:20, yangoon wrote: > On 2010/01/09 13:03:07, yangoon wrote: > > I have joined some screenshots to http://bugs.tryton.org/roundup/issue1339 to > > illustrate the current patch: > > 1) It shows 'Group Membership' where the interface is showing the selection of > > the groups the user belongs to. Must stay groups like other same kind of fields in Tryton. > > 2) It shows 'Access Permissions' on groups, where they are indeed implemented. Ok and on user. > > 3) It shows 'Groups' where generally all groups are related to. > > @ced: Before continuing with the patch I would like to know, if the concept is > accepted.
Sign in to reply to this message.
cedric.krier@b2ck.com schrieb: >> > 1) It shows 'Group Membership' where the interface is showing the > selection of >> > the groups the user belongs to. > > Must stay groups like other same kind of fields in Tryton. -5 -- Schönen Gruß - Regards Hartmut Goebel Dipl.-Informatiker (univ.), CISSP, CSSLP Goebel Consult Spezialist für IT-Sicherheit in komplexen Umgebungen http://www.goebel-consult.de Monatliche Kolumne: http://www.cissp-gefluester.de/ Goebel Consult mit Mitglied bei http://www.7-it.de
Sign in to reply to this message.
http://codereview.appspot.com/183121/diff/1/4 File trytond/res/user.py (right): http://codereview.appspot.com/183121/diff/1/4#newcode34 trytond/res/user.py:34: 'uid', 'gid', 'Group Membership') On 2010/01/06 14:01:30, ced wrote: > On 2010/01/06 13:38:12, yangoon wrote: > > On 2010/01/06 09:03:30, ced wrote: > > > It doesn't look like other naming. > > I don't understand. What would you prefer, since it is mot an interface to > > generic groups? > It is one. Everything in Tryton is at least an interface to a generic model. In the context of users, the use of the many2many to the generic model groups gives for me enough reason to extend the name "Groups" to "Group Membership". Because the functionality to choose the Groups where the user is a member, is provided with this widget. Or did I misunderstand you, ced? http://codereview.appspot.com/183121/diff/1/5 File trytond/res/user.xml (right): http://codereview.appspot.com/183121/diff/1/5#newcode40 trytond/res/user.xml:40: <page string="Group Membership" col="1" id="security"> Why the string here is "Group Membership" and not "Access Permissions"? When I take a look to http://bugs.tryton.org/roundup/file696/user.png (which is hopefully the screenshot of this view-code), Group Membership seems not an exact wording, because on the same tab are the "Rules", too.
Sign in to reply to this message.
Please take a look to the second set of screenshots at https://bugs.tryton.org/roundup/issue1339 matching the second patch set. It should be a satisfying compromise. (No migration code included yet). http://codereview.appspot.com/183121/diff/1/5 File trytond/res/user.xml (right): http://codereview.appspot.com/183121/diff/1/5#newcode40 trytond/res/user.xml:40: <page string="Group Membership" col="1" id="security"> On 2010/01/13 11:31:21, udo.spallek wrote: > Why the string here is "Group Membership" and not "Access Permissions"? > > When I take a look to http://bugs.tryton.org/roundup/file696/user.png (which is > hopefully the screenshot of this view-code), Group Membership seems not an exact > wording, because on the same tab are the "Rules", too. > > Perfectly right. But you there is no 100% correct naming, because basically Access Permissions are defined on Groups. Changed it to the latter nevertheless, because it describes better the purpose of the tab.
Sign in to reply to this message.
http://codereview.appspot.com/183121/diff/4004/4005 File trytond/res/group.xml (right): http://codereview.appspot.com/183121/diff/4004/4005#newcode21 trytond/res/group.xml:21: <page string="Members" col="2" id="members"> It should be Users http://codereview.appspot.com/183121/diff/4004/4007 File trytond/res/user.py (right): http://codereview.appspot.com/183121/diff/4004/4007#newcode34 trytond/res/user.py:34: 'uid', 'gid', 'Group Membership') We never use this kind of wording for xxx2many. Per example: Categories on party, it is not category membership Companies on user, ... Employees on company, ... Invoices on sale, it is not invoices generated Shipments on sale, ... Taxes on invoice, it is not taxes applied
Sign in to reply to this message.
Changed to the wishes of ced. http://codereview.appspot.com/183121/diff/4004/4007 File trytond/res/user.py (right): http://codereview.appspot.com/183121/diff/4004/4007#newcode34 trytond/res/user.py:34: 'uid', 'gid', 'Group Membership') On 2010/01/13 13:34:08, ced wrote: > We never use this kind of wording for xxx2many. > Per example: > Categories on party, it is not category membership > Companies on user, ... > Employees on company, ... > Invoices on sale, it is not invoices generated > Shipments on sale, ... > Taxes on invoice, it is not taxes applied Once again we are hitting usability questions. Models are for developers, practicability and intuitivity are for the user (who is caring 0% for models, but who wants to work his workflow in the fastest way possible). A more descriptive way would often help the user.
Sign in to reply to this message.
mathias.behrle@gmx.de schrieb: > Once again we are hitting usability questions. > Models are for developers, practicability and intuitivity are for the > user (who is caring 0% for models, but who wants to work his workflow in > the fastest way possible). A more descriptive way would often help the > user. +2 -- Schönen Gruß - Regards Hartmut Goebel Dipl.-Informatiker (univ.), CISSP, CSSLP Goebel Consult Spezialist für IT-Sicherheit in komplexen Umgebungen http://www.goebel-consult.de Monatliche Kolumne: http://www.cissp-gefluester.de/ Goebel Consult mit Mitglied bei http://www.7-it.de
Sign in to reply to this message.
On 2010/01/13 13:34:08, ced wrote: > http://codereview.appspot.com/183121/diff/4004/4005 > File trytond/res/group.xml (right): > > http://codereview.appspot.com/183121/diff/4004/4005#newcode21 > trytond/res/group.xml:21: <page string="Members" col="2" id="members"> > It should be Users > > http://codereview.appspot.com/183121/diff/4004/4007 > File trytond/res/user.py (right): > > http://codereview.appspot.com/183121/diff/4004/4007#newcode34 > trytond/res/user.py:34: 'uid', 'gid', 'Group Membership') > We never use this kind of wording for xxx2many. > Per example: > Categories on party, it is not category membership > Companies on user, ... > Employees on company, ... > Invoices on sale, it is not invoices generated > Shipments on sale, ... > Taxes on invoice, it is not taxes applied @ced: now ok?
Sign in to reply to this message.
http://codereview.appspot.com/183121/diff/3004/4015 File trytond/res/user.xml (right): http://codereview.appspot.com/183121/diff/3004/4015#newcode79 trytond/res/user.xml:79: <page string="Group Membership" col="1" id="membership"> I don't see the logical: On Group the tab with users is named "Users" And on User the tab with groups is named "Group Membership"
Sign in to reply to this message.
http://codereview.appspot.com/183121/diff/3004/4015 File trytond/res/user.xml (right): http://codereview.appspot.com/183121/diff/3004/4015#newcode79 trytond/res/user.xml:79: <page string="Group Membership" col="1" id="membership"> On 2010/01/16 11:04:27, ced wrote: > I don't see the logical: > > On Group the tab with users is named "Users" > And on User the tab with groups is named "Group Membership" It is a tab description, not a model. Look above. Tab for groups and rule_groups is named 'Access Permissions'. With your logic you would need instead two different tabs 'Groups' and 'Rules'.
Sign in to reply to this message.
On 2010/01/16 22:22:00, yangoon wrote: > http://codereview.appspot.com/183121/diff/3004/4015 > File trytond/res/user.xml (right): > > http://codereview.appspot.com/183121/diff/3004/4015#newcode79 > trytond/res/user.xml:79: <page string="Group Membership" col="1" > id="membership"> > On 2010/01/16 11:04:27, ced wrote: > > I don't see the logical: > > > > On Group the tab with users is named "Users" > > And on User the tab with groups is named "Group Membership" > > It is a tab description, not a model. > > Look above. Tab for groups and rule_groups is named 'Access Permissions'. With > your logic you would need instead two different tabs 'Groups' and 'Rules'. That is not what I said. I talk only about Tabs: On Group the *tab* with users is named "Users" And on User the *tab* with groups is named "Group Membership"
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
