Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-03-22 Thread Martin Kosek
On 02/08/2013 02:27 PM, Petr Vobornik wrote:
 Checkbox for NONE option was added.
 
 https://fedorahosted.org/freeipa/ticket/3404
 
 Patches for master and 3.1 branch attached.
 

I pushed this version to ipa-3-1 branch only as a hotfix for upcoming 3.1
release so that users can set NONE option via UI. The right solution should
be chosen and pushed to master branch.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-03-15 Thread Endi Sukma Dewata

On 3/7/2013 7:37 AM, Petr Vobornik wrote:

Ideally it should be generic enough to combine any widgets. This might
be a common scenario somewhere else:

Something: ( ) Option 1
   ( ) Option 2
   (o) Other: [something else   ]


This design has a flaw:
https://fedorahosted.org/freeipa/ticket/3404#comment:5


I think this one makes the most sense to me:

PAC Types: ( ) Inherited setting: ... inherited values ...
   (o) Override inherited setting
   [ ] MS-PAC
   [ ] PAD

It looks like the NONE option is identical to not using the inherited 
values but also not selecting any new values, so we don't actually need 
a separate radio button for NONE because it can be represented by the 
above UI without redundancy. We just need better labels to explain the 
radio buttons. Maybe someone can come up with better labels than these.



I implemented following design:
https://fedorahosted.org/freeipa/ticket/3404#comment:7


It works but I can't imagine how it would look if we have more than two 
PAC types. I don't think we want to list every possible combinations. 
The above design is more future proof.



Patch attached (255-1).

I have a dilemma. I practically implemented the previous design (and
then I've found the flaw..). Patches attached as wip-fre... I wonder if
we can use it somehow or we should ditch it.


--
Endi S. Dewata

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-03-07 Thread Petr Vobornik

On 02/14/2013 04:56 PM, Endi Sukma Dewata wrote:

On 2/14/2013 6:30 AM, Petr Vobornik wrote:

If they are mutually exclusive, they probably should be separated using
radio buttons like this:

   PAC: ( ) None
(o) Type:
[x] MS-PAC
[ ] PAD


You missed one option: nothing selected. It can be solved by adding   '(
) Inherited' radio.


I wouldn't have guessed that :) I agree we should add the 'Inherited'
option.


Anyway, this design seems more user friendly for more general audience
than mine so I will implement it. The only problem with it is that one
have to come with new label for each group and empty value - can't be
inferred from metadata.


Is there any issue adding new labels at this point? Worst case we could
hard code the label now and add a translation later.


It might be better to use a composite widget of radio buttons and
checkboxes so we can reuse the code. Probably the definition will look
something like this:

{
 name: 'ipakrbauthzdata',
 type: 'radio',


Not sure if it should be radio, more like something new.


Right, probably the current radio widget can't do this. So either we
improve the radio widget or create something new.


 label: ...,
 options: [
 {
 label: ...,
 value: 'NONE'
 },
 {
 label: ...,
 type: 'checkboxes',


Do you expect to be there something different than checkboxes, or do you
want it to do it this way for possible future customization.


Ideally it should be generic enough to combine any widgets. This might
be a common scenario somewhere else:

Something: ( ) Option 1
( ) Option 2
(o) Other: [something else   ]



This design has a flaw: 
https://fedorahosted.org/freeipa/ticket/3404#comment:5


I implemented following design: 
https://fedorahosted.org/freeipa/ticket/3404#comment:7


Patch attached (255-1).

I have a dilemma. I practically implemented the previous design (and 
then I've found the flaw..). Patches attached as wip-fre... I wonder if 
we can use it somehow or we should ditch it.


--
Petr Vobornik
From 854fb91a1fdc45f4d1ea45b9dc18f82b52a83e6d Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 7 Mar 2013 14:16:21 +0100
Subject: [PATCH] Added Web UI support for service PAC type option: NONE

ipakrbauthzdata accepts [null, 'NONE', 'MS-PAC, 'PAD']. 'MS-PAC' and 'PAD' can be combined together but they are mutually exclusive with 'NONE'. Hence, we can't use normal radio buttons nor checkboxes.

New multivalued radio widget was developed to solve the problem. One can define option with multiple values. Thus following layout is possible:

PAC type:  ( ) Inherited
   ( ) None
   (o) MS-PAC
   ( ) PAD
   ( ) MS-PAC + PAD

https://fedorahosted.org/freeipa/ticket/3404
---
 install/ui/src/freeipa/field.js|  1 +
 install/ui/src/freeipa/service.js  | 20 +++--
 install/ui/src/freeipa/widget.js   | 44 ++
 install/ui/test/data/ipa_init.json |  5 +
 ipalib/plugins/internal.py |  5 +
 5 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/install/ui/src/freeipa/field.js b/install/ui/src/freeipa/field.js
index f705ef7b899f502b717daa384c01763c96aea664..efa9568124c0ebc59fdfc93c55a2bb888a616f90 100644
--- a/install/ui/src/freeipa/field.js
+++ b/install/ui/src/freeipa/field.js
@@ -921,6 +921,7 @@ IPA.field_factories['entity_select'] = IPA.field;
 IPA.field_factories['field'] = IPA.field;
 IPA.field_factories['link'] = IPA.link_field;
 IPA.field_factories['multivalued'] = IPA.multivalued_field;
+IPA.field_factories['multivalued_radio'] = IPA.radio_field;
 IPA.field_factories['password'] = IPA.field;
 IPA.field_factories['radio'] = IPA.radio_field;
 IPA.field_factories['select'] = IPA.select_field;
diff --git a/install/ui/src/freeipa/service.js b/install/ui/src/freeipa/service.js
index ecb8ce9b30518bd620dbd6ffab05342cff7a946b..8e21a9a225e2fcf5b34512c85bd471dd2cf71704 100644
--- a/install/ui/src/freeipa/service.js
+++ b/install/ui/src/freeipa/service.js
@@ -54,8 +54,24 @@ IPA.service.entity = function(spec) {
 },
 {
 name: 'ipakrbauthzdata',
-type: 'checkboxes',
-options: IPA.create_options(['MS-PAC', 'PAD'])
+type: 'multivalued_radio',
+direction: 'vertical',
+options: IPA.create_options([
+{
+label: IPA.messages.krbauthzdata.inherited,
+value: ''
+},
+{
+label: IPA.messages.krbauthzdata.none,
+value: 'NONE'
+},
+   

Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-02-19 Thread Sumit Bose
On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote:
 On 2/14/2013 6:30 AM, Petr Vobornik wrote:
 If they are mutually exclusive, they probably should be separated using
 radio buttons like this:
 
PAC: ( ) None
 (o) Type:
 [x] MS-PAC
 [ ] PAD
 
 You missed one option: nothing selected. It can be solved by adding   '(
 ) Inherited' radio.
 
 I wouldn't have guessed that :) I agree we should add the
 'Inherited' option.
 

Would it be possible to print the inherited values here, i.e. the
default values given in the ipakrbauthzdata attribute of ipaGuiConfig
object? I think this would improve the user experience because you do
not have to remember/look up the default values.

bye,
Sumit

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-02-19 Thread Petr Vobornik

On 02/19/2013 01:40 PM, Sumit Bose wrote:

On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote:

On 2/14/2013 6:30 AM, Petr Vobornik wrote:

If they are mutually exclusive, they probably should be separated using
radio buttons like this:

   PAC: ( ) None
(o) Type:
[x] MS-PAC
[ ] PAD


You missed one option: nothing selected. It can be solved by adding   '(
) Inherited' radio.


I wouldn't have guessed that :) I agree we should add the
'Inherited' option.



Would it be possible to print the inherited values here, i.e. the
default values given in the ipakrbauthzdata attribute of ipaGuiConfig
object? I think this would improve the user experience because you do
not have to remember/look up the default values.

bye,
Sumit



It is possible, but not straightforward.

I see two options:
  1) Currently Web UI loads config at start so we can use it. The 
disadvantage is that the value might be changed and so the displayed 
value might be incorrect.
  2) Other option is to run config-show along with service-show in a 
batch to get the up-to-date value.


I'm not a big fan of #2 but it is probably better solution.
--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-02-19 Thread Sumit Bose
On Tue, Feb 19, 2013 at 02:01:24PM +0100, Petr Vobornik wrote:
 On 02/19/2013 01:40 PM, Sumit Bose wrote:
 On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote:
 On 2/14/2013 6:30 AM, Petr Vobornik wrote:
 If they are mutually exclusive, they probably should be separated using
 radio buttons like this:
 
PAC: ( ) None
 (o) Type:
 [x] MS-PAC
 [ ] PAD
 
 You missed one option: nothing selected. It can be solved by adding   '(
 ) Inherited' radio.
 
 I wouldn't have guessed that :) I agree we should add the
 'Inherited' option.
 
 
 Would it be possible to print the inherited values here, i.e. the
 default values given in the ipakrbauthzdata attribute of ipaGuiConfig
 object? I think this would improve the user experience because you do
 not have to remember/look up the default values.
 
 bye,
 Sumit
 
 
 It is possible, but not straightforward.
 
 I see two options:
   1) Currently Web UI loads config at start so we can use it. The
 disadvantage is that the value might be changed and so the displayed
 value might be incorrect.
   2) Other option is to run config-show along with service-show in a
 batch to get the up-to-date value.
 
 I'm not a big fan of #2 but it is probably better solution.
I agree with both :-). Since it is not straightforward I guess it would
be better not to add this with this ticket but open a new one. Do you
agree?

bye,
Sumit

 -- 
 Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-02-19 Thread Petr Vobornik

On 02/19/2013 02:08 PM, Sumit Bose wrote:

On Tue, Feb 19, 2013 at 02:01:24PM +0100, Petr Vobornik wrote:

On 02/19/2013 01:40 PM, Sumit Bose wrote:

On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote:

On 2/14/2013 6:30 AM, Petr Vobornik wrote:

If they are mutually exclusive, they probably should be separated using
radio buttons like this:

   PAC: ( ) None
(o) Type:
[x] MS-PAC
[ ] PAD


You missed one option: nothing selected. It can be solved by adding   '(
) Inherited' radio.


I wouldn't have guessed that :) I agree we should add the
'Inherited' option.



Would it be possible to print the inherited values here, i.e. the
default values given in the ipakrbauthzdata attribute of ipaGuiConfig
object? I think this would improve the user experience because you do
not have to remember/look up the default values.

bye,
Sumit



It is possible, but not straightforward.

I see two options:
   1) Currently Web UI loads config at start so we can use it. The
disadvantage is that the value might be changed and so the displayed
value might be incorrect.
   2) Other option is to run config-show along with service-show in a
batch to get the up-to-date value.

I'm not a big fan of #2 but it is probably better solution.

I agree with both :-). Since it is not straightforward I guess it would
be better not to add this with this ticket but open a new one. Do you
agree?


Yes



bye,
Sumit


--
Petr Vobornik



--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-02-19 Thread Sumit Bose
On Tue, Feb 19, 2013 at 02:12:24PM +0100, Petr Vobornik wrote:
 On 02/19/2013 02:08 PM, Sumit Bose wrote:
 On Tue, Feb 19, 2013 at 02:01:24PM +0100, Petr Vobornik wrote:
 On 02/19/2013 01:40 PM, Sumit Bose wrote:
 On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote:
 On 2/14/2013 6:30 AM, Petr Vobornik wrote:
 If they are mutually exclusive, they probably should be separated using
 radio buttons like this:
 
PAC: ( ) None
 (o) Type:
 [x] MS-PAC
 [ ] PAD
 
 You missed one option: nothing selected. It can be solved by adding   '(
 ) Inherited' radio.
 
 I wouldn't have guessed that :) I agree we should add the
 'Inherited' option.
 
 
 Would it be possible to print the inherited values here, i.e. the
 default values given in the ipakrbauthzdata attribute of ipaGuiConfig
 object? I think this would improve the user experience because you do
 not have to remember/look up the default values.
 
 bye,
 Sumit
 
 
 It is possible, but not straightforward.
 
 I see two options:
1) Currently Web UI loads config at start so we can use it. The
 disadvantage is that the value might be changed and so the displayed
 value might be incorrect.
2) Other option is to run config-show along with service-show in a
 batch to get the up-to-date value.
 
 I'm not a big fan of #2 but it is probably better solution.
 I agree with both :-). Since it is not straightforward I guess it would
 be better not to add this with this ticket but open a new one. Do you
 agree?
 
 Yes

Thanks, I've opened https://fedorahosted.org/freeipa/ticket/3436 .

bye,
Sumit

 
 
 bye,
 Sumit
 
 --
 Petr Vobornik
 
 
 -- 
 Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-02-14 Thread Petr Vobornik

On 02/12/2013 11:15 PM, Endi Sukma Dewata wrote:

On 2/12/2013 10:56 AM, Petr Vobornik wrote:

We were discussing to NACK this approach.

The implementation should be improved because of the mutually exclusive
nature of NONE option with [MS-PAC, PAD] options.

I think we should add spec definition (to Web UI only, or into server
plugin as well) of mutex options. Something like:

 mutex_groups: [[['NONE'],['MS-PAC', 'PAD']], [/*another array of
groups*/]];

basically an array of group arrays where group array would contain
arrays of mutually exclusive option. Is it over-complicated? Would one
array or a pair of groups be enough?

That way we can disable and uncheck(should not happen, but to keep data
consistency) checkboxes of options from other groups when user selects a
value of some group.


If they are mutually exclusive, they probably should be separated using
radio buttons like this:

   PAC: ( ) None
(o) Type:
[x] MS-PAC
[ ] PAD


You missed one option: nothing selected. It can be solved by adding   '( 
) Inherited' radio.


Anyway, this design seems more user friendly for more general audience 
than mine so I will implement it. The only problem with it is that one 
have to come with new label for each group and empty value - can't be 
inferred from metadata.




It might be better to use a composite widget of radio buttons and
checkboxes so we can reuse the code. Probably the definition will look
something like this:

{
 name: 'ipakrbauthzdata',
 type: 'radio',


Not sure if it should be radio, more like something new.


 label: ...,
 options: [
 {
 label: ...,
 value: 'NONE'
 },
 {
 label: ...,
 type: 'checkboxes',


Do you expect to be there something different than checkboxes, or do you 
want it to do it this way for possible future customization.



 options: [
 {
 label: ...,
 value: 'MS-PAC'
 },
 {
 label: ...,
 value: 'PAD'
 }
 ]
 }
 ]
}

The composite widget will handle setting the appropriate widgets when
the value of the field on load. It will handle enabling/disabling the
checkboxes when the radio button is selected. It will also compute the
final value of the field from selected radio button/checkboxes on save.


Or just dim (no disable) and uncheck. That way there would still be
visual distinction and one don't have to uncheck all the options from
one group when he wants to select options of mutex group.


That is also possible, but it changes the normal behavior of checkboxes,
so probably users would have to play around with it to understand the
grouping. They could also get confused, e.g. is dimmed checkbox disabled?


As a separate issue, we might want to fetch the options from metadata,
when we want to show the values and don't care about creating different
labels.


If we use radio buttons like above, new labels are necessary to describe
the different groups of checkboxes. For all radio buttons  checkboxes
in general, if we don't want to use labels we probably could simplify
the definition such that we can specify the string values directly
(without nested object):

{
 name: 'ipakrbauthzdata',
 type: 'radio',
 options: [
 'NONE',
 {
 type: 'checkboxes',
 options: ['MS-PAC', 'PAD']
 }
 ]
}



+1 for classic checkboxes or radios. Problem with this example is that, 
it is missing label for the checkbox group which can't be inferred from 
anything.



--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-02-14 Thread Endi Sukma Dewata

On 2/14/2013 6:30 AM, Petr Vobornik wrote:

If they are mutually exclusive, they probably should be separated using
radio buttons like this:

   PAC: ( ) None
(o) Type:
[x] MS-PAC
[ ] PAD


You missed one option: nothing selected. It can be solved by adding   '(
) Inherited' radio.


I wouldn't have guessed that :) I agree we should add the 'Inherited' 
option.



Anyway, this design seems more user friendly for more general audience
than mine so I will implement it. The only problem with it is that one
have to come with new label for each group and empty value - can't be
inferred from metadata.


Is there any issue adding new labels at this point? Worst case we could 
hard code the label now and add a translation later.



It might be better to use a composite widget of radio buttons and
checkboxes so we can reuse the code. Probably the definition will look
something like this:

{
 name: 'ipakrbauthzdata',
 type: 'radio',


Not sure if it should be radio, more like something new.


Right, probably the current radio widget can't do this. So either we 
improve the radio widget or create something new.



 label: ...,
 options: [
 {
 label: ...,
 value: 'NONE'
 },
 {
 label: ...,
 type: 'checkboxes',


Do you expect to be there something different than checkboxes, or do you
want it to do it this way for possible future customization.


Ideally it should be generic enough to combine any widgets. This might 
be a common scenario somewhere else:


Something: ( ) Option 1
   ( ) Option 2
   (o) Other: [something else   ]

--
Endi S. Dewata

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-02-12 Thread Petr Vobornik

On 02/12/2013 05:14 PM, Endi Sukma Dewata wrote:

On 2/8/2013 7:27 AM, Petr Vobornik wrote:

Checkbox for NONE option was added.

https://fedorahosted.org/freeipa/ticket/3404

Patches for master and 3.1 branch attached.


ACK.



We were discussing to NACK this approach.

The implementation should be improved because of the mutually exclusive 
nature of NONE option with [MS-PAC, PAD] options.


I think we should add spec definition (to Web UI only, or into server 
plugin as well) of mutex options. Something like:


	mutex_groups: [[['NONE'],['MS-PAC', 'PAD']], [/*another array of 
groups*/]];


basically an array of group arrays where group array would contain 
arrays of mutually exclusive option. Is it over-complicated? Would one 
array or a pair of groups be enough?


That way we can disable and uncheck(should not happen, but to keep data 
consistency) checkboxes of options from other groups when user selects a 
value of some group.



As a separate issue, we might want to fetch the options from metadata, 
when we want to show the values and don't care about creating different 
labels.

--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE

2013-02-12 Thread Endi Sukma Dewata

On 2/12/2013 10:56 AM, Petr Vobornik wrote:

We were discussing to NACK this approach.

The implementation should be improved because of the mutually exclusive
nature of NONE option with [MS-PAC, PAD] options.

I think we should add spec definition (to Web UI only, or into server
plugin as well) of mutex options. Something like:

 mutex_groups: [[['NONE'],['MS-PAC', 'PAD']], [/*another array of
groups*/]];

basically an array of group arrays where group array would contain
arrays of mutually exclusive option. Is it over-complicated? Would one
array or a pair of groups be enough?

That way we can disable and uncheck(should not happen, but to keep data
consistency) checkboxes of options from other groups when user selects a
value of some group.


If they are mutually exclusive, they probably should be separated using 
radio buttons like this:


  PAC: ( ) None
   (o) Type:
   [x] MS-PAC
   [ ] PAD

It might be better to use a composite widget of radio buttons and 
checkboxes so we can reuse the code. Probably the definition will look 
something like this:


{
name: 'ipakrbauthzdata',
type: 'radio',
label: ...,
options: [
{
label: ...,
value: 'NONE'
},
{
label: ...,
type: 'checkboxes',
options: [
{
label: ...,
value: 'MS-PAC'
},
{
label: ...,
value: 'PAD'
}
]
}
]
}

The composite widget will handle setting the appropriate widgets when 
the value of the field on load. It will handle enabling/disabling the 
checkboxes when the radio button is selected. It will also compute the 
final value of the field from selected radio button/checkboxes on save.



Or just dim (no disable) and uncheck. That way there would still be
visual distinction and one don't have to uncheck all the options from
one group when he wants to select options of mutex group.


That is also possible, but it changes the normal behavior of checkboxes, 
so probably users would have to play around with it to understand the 
grouping. They could also get confused, e.g. is dimmed checkbox disabled?



As a separate issue, we might want to fetch the options from metadata,
when we want to show the values and don't care about creating different
labels.


If we use radio buttons like above, new labels are necessary to describe 
the different groups of checkboxes. For all radio buttons  checkboxes 
in general, if we don't want to use labels we probably could simplify 
the definition such that we can specify the string values directly 
(without nested object):


{
name: 'ipakrbauthzdata',
type: 'radio',
options: [
'NONE',
{
type: 'checkboxes',
options: ['MS-PAC', 'PAD']
}
]
}

--
Endi S. Dewata

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel