Disable sensitivity for One2Many gui controls where appropriate per issue1406

[ced, 2010-08-20 18:18:06]

http://codereview.appspot.com/1974045/diff/1/2
File tryton/gui/window/view_form/view/form_gtk/one2many.py (right):

http://codereview.appspot.com/1974045/diff/1/2#newcode323
tryton/gui/window/view_form/view/form_gtk/one2many.py:323: if signal_data[0]+1 <
signal_data[1]:
space between +

[ced, 2010-08-20 18:18:41]

Add also an entry if CHANGELOG

[ced, 2010-08-20 18:43:08]

Looks ok for me
But let's some more time to have other comments.

[yangoon1, 2010-08-20 20:00:36]

Thx a lot.

Tested on neso tip the delete button is activated on fresh open forms like Lines
on Account Moves. It is disabled when switching view of the o2m after deleting
the new record.

http://codereview.appspot.com/1974045/diff/6001/7002
File tryton/gui/window/view_form/view/form_gtk/one2many.py (right):

http://codereview.appspot.com/1974045/diff/6001/7002#newcode319
tryton/gui/window/view_form/view/form_gtk/one2many.py:319: if signal_data[0] >=
0:
Could perhaps made be shorter like (untested)

self.but_open.set_sensitive(False)
self.but_del.set_sensitive(False)
self.but_pre.set_sensitive(False)
self.but_next.set_sensitive(False)
if signal_data[0] >= 0:
	name = str(signal_data[0] + 1)
	self.but_open.set_sensitive(True)
	self.but_del.set_sensitive(True)
	if signal_data[0] + 1 < signal_data[1]:
		self.but_next.set_sensitive(True)
	if signal_data[0] > 0:
		self.but_pre.set_sensitive(True)

[yangoon1, 2010-08-20 20:03:30]

http://codereview.appspot.com/1974045/diff/6001/7002
File tryton/gui/window/view_form/view/form_gtk/one2many.py (right):

http://codereview.appspot.com/1974045/diff/6001/7002#newcode317
tryton/gui/window/view_form/view/form_gtk/one2many.py:317: def _sig_label(self,
screen, signal_data):
Perhaps rename function to a better suitable name, since now it doesn't only set
label?

[pheller, 2010-08-20 22:05:59]

Added a _readonly variable within o2m view, which is set on calls to
_readonly_set.  Subsequently, updates to the o2m controls via calls to
_sig_label respect readonly status.

This should fix the issue yangoon reported.  Please give it a try and let me
know.

I don't mind renaming _sig_label to something else; perhaps _sig_controls or
_sig_record, but I want some input first -- and it would probably make sense to
then update this name for all views.

[ced, 2010-08-20 22:16:34]

http://codereview.appspot.com/1974045/diff/14001/15002
File tryton/gui/window/view_form/view/form_gtk/one2many.py (right):

http://codereview.appspot.com/1974045/diff/14001/15002#newcode211
tryton/gui/window/view_form/view/form_gtk/one2many.py:211: if
self.screen.current_record is not None:
Why this test?

http://codereview.appspot.com/1974045/diff/14001/15002#newcode322
tryton/gui/window/view_form/view/form_gtk/one2many.py:322:
self.but_open.set_sensitive(False)
I don't like to put all insensitive (it could have side effect).
I prefer to have state change only when needed like in previous version.

http://codereview.appspot.com/1974045/diff/14001/15002#newcode329
tryton/gui/window/view_form/view/form_gtk/one2many.py:329:
self.but_del.set_sensitive(not self._readonly)
You must also test that signal_data[0] is not equal to 0

http://codereview.appspot.com/1974045/diff/14001/15002#newcode329
tryton/gui/window/view_form/view/form_gtk/one2many.py:329:
self.but_del.set_sensitive(not self._readonly)
You must have the same behavior for but_remove

[yangoon1, 2010-08-21 10:00:53]

On 2010/08/20 22:16:34, ced wrote:
> http://codereview.appspot.com/1974045/diff/14001/15002#newcode322
> tryton/gui/window/view_form/view/form_gtk/one2many.py:322:
> self.but_open.set_sensitive(False)
> I don't like to put all insensitive (it could have side effect).

Which side effect are you thinking of? Could you please explain a little bit
more?

[ced, 2010-08-21 10:06:40]

On 2010/08/21 10:00:53, yangoon wrote:
> On 2010/08/20 22:16:34, ced wrote:
> > http://codereview.appspot.com/1974045/diff/14001/15002#newcode322
> > tryton/gui/window/view_form/view/form_gtk/one2many.py:322:
> > self.but_open.set_sensitive(False)
> > I don't like to put all insensitive (it could have side effect).
> 
> Which side effect are you thinking of? Could you please explain a little bit
> more?

Perhaps blinking but I don't know.
In a general way, it generates more calls to set_sensitive which is not very
optimal.

PS: when you answer to a comment, please do it under the comment and not in
global message because it is harder to follow.

[pheller, 2010-08-23 16:13:40]

See comment responses, will now submit another patch.

http://codereview.appspot.com/1974045/diff/14001/15002
File tryton/gui/window/view_form/view/form_gtk/one2many.py (right):

http://codereview.appspot.com/1974045/diff/14001/15002#newcode211
tryton/gui/window/view_form/view/form_gtk/one2many.py:211: if
self.screen.current_record is not None:
On 2010/08/20 22:16:34, ced wrote:
> Why this test?

When a new record containing the one2many is created, it is not readonly, thus
the sensitivity of but_del is set to "not False", though no one2many relations
exist on this new record.

http://codereview.appspot.com/1974045/diff/14001/15002#newcode329
tryton/gui/window/view_form/view/form_gtk/one2many.py:329:
self.but_del.set_sensitive(not self._readonly)
On 2010/08/20 22:16:34, ced wrote:
> You must also test that signal_data[0] is not equal to 0

But, by the original logic, the current_record is "1" if signal_data[0] = 0....

[ced, 2010-08-23 17:03:10]

http://codereview.appspot.com/1974045/diff/14001/15002
File tryton/gui/window/view_form/view/form_gtk/one2many.py (right):

http://codereview.appspot.com/1974045/diff/14001/15002#newcode211
tryton/gui/window/view_form/view/form_gtk/one2many.py:211: if
self.screen.current_record is not None:
On 2010/08/23 16:13:41, pheller wrote:
> On 2010/08/20 22:16:34, ced wrote:
> > Why this test?
> 
> When a new record containing the one2many is created, it is not readonly, thus
> the sensitivity of but_del is set to "not False", though no one2many relations
> exist on this new record.

The test is not correct because it is not change the sensitive for all cases.

http://codereview.appspot.com/1974045/diff/18001/12005#newcode73
tryton/gui/window/view_form/view/form_gtk/one2many.py:73: 
Why initializing all buttons to sensitive False ?

http://codereview.appspot.com/1974045/diff/18001/12005#newcode343
tryton/gui/window/view_form/view/form_gtk/one2many.py:343: new_group =
field.get_client(record)
And what about but_next and but_pre

[pheller, 2010-10-06 03:29:45]

I've been generally testing against parties and the party contact one2many.  One
thing to note is that it seems if you delete all contacts from a party, then
save, the last contact is still there.  

I'll look more at this tomorrow, unless this is intended operation.

[ced, 2010-10-06 10:02:05]

On 2010/10/06 03:29:45, pheller wrote:
> I've been generally testing against parties and the party contact one2many. 
One
> thing to note is that it seems if you delete all contacts from a party, then
> save, the last contact is still there.

It is not linked to this change.

[pheller, 2010-10-06 12:09:47]

On 2010/10/06 10:02:05, ced wrote:
> On 2010/10/06 03:29:45, pheller wrote:
> > I've been generally testing against parties and the party contact one2many. 
> One
> > thing to note is that it seems if you delete all contacts from a party, then
> > save, the last contact is still there.
> 
> It is not linked to this change.

Ok, good, then I believe this patchset implements the feature.

[ced, 2010-10-06 12:21:16]

On 2010/10/06 10:02:05, ced wrote:
> On 2010/10/06 03:29:45, pheller wrote:
> > I've been generally testing against parties and the party contact one2many. 
> One
> > thing to note is that it seems if you delete all contacts from a party, then
> > save, the last contact is still there.
> 
> It is not linked to this change.

http://codereview.appspot.com/2377041/

[ced, 2010-10-06 14:37:25]

Ok for me