On 3.9.2014 21:44, Endi Sukma Dewata wrote:
On 8/22/2014 3:31 AM, Petr Vobornik wrote:
On 12.8.2014 17:59, Endi Sukma Dewata wrote:
On 8/5/2014 6:31 AM, Petr Vobornik wrote:>>
ticket: https://fedorahosted.org/freeipa/ticket/4402
snip
[PATCH] 727 webui: hide empty fields and sections
Hide widgets without a value. Must be explicitly turned on. In
widget by
`hidden_if_empty` flag. Or globally by `hide_empty_widgets` flag.
Global
hiding can be individually turned off by `ignore_empty_hiding` flag.
In item #2 of ticket #4402 I was originally thinking the widget
visibility would be determined by the token type.
Originally I wrote it that way but with this patch it became redundant.
Any plan to add the
token type field in the future?
maybe, I don't have any strong feelings about it. Will users benefit
from adding it? If yes, it should be also added to CLI.
Can the users tell which type of token they have based on the existing
fields? There is a model field which is populated with the type (e.g.
totp or hotp). But since the value can be changed to anything it's not a
reliable way to determine the type. I don't think it's very
user-friendly to ask the user to see whether the type-specific fields
are shown in order to determine the type.
I can't say there's a big benefit by adding the field, but maybe some
admins might find it useful, and it can be used to sort/filter out
search results.
Atm. token type is derived from object classes. It exists only in add
operation as a virtual attribute. We can check a presence of
ipatokenhotp or ipatokentotp object classes to simulate the field.
Yes, if people can add an attribute usually they will expect to be able
to see what they added.
The issue is that it's not an attribute per se.
Nathaniel: do you think we should add a virtual output param(computed
from object class) for the token type since 'model' could be easily
overwritten? I would prefer to do it both in Web UI and CLI.
Will the "counter" field strictly have a
value with HOTP only and "clock offset & interval" fields strictly have
value with TOTP only? Do these fields contain the configured values or
the effective values? I was just thinking maybe in the future some of
these fields might be configured empty and they will use the default
values instead. If that's not a problem then ACK.
Respective fields are present only in corresponding object classes ->
there won't be an HTOP token with 'clock offset'. If they are present,
they have a default value. -> No false hiding -> Shouldn't be a problem.
Btw: Other of my other original intents was to hide it based on ACI
rights. The issues is that the rights are not present without
corresponding OC. Hiding in such case is dangerous - explicitly disabled
in patch 728.
What do you think about setting `hide_empty_widgets` global setting to
True?
On a read-only page I think it's OK to hide empty fields. But on edit
page I don't think we should do that by default.
Maybe we should first clarify what is a read-only page. All details
pages are writable if user has the rights or read-only if he doesn't.
But mostly it depends on individual fields/attributes.
Does "empty" mean "unset", or "blank" like empty string/array? Does
"unset" always mean "non-editable and invisible" and "blank" always mean
"editable and visible but it's just empty"? If this definition isn't
strictly followed there could be an editable field that accidentally
gets hidden because it's empty.
My intent was to show everything what user can edit. And hide fields:
1. for which he doesn't have read rights
2. have no (undefined) or empty('') value
3. are explicitly hidden by other logic - not related to this patch
When thinking about #1. Maybe we should base it on a presence or rather
not presence of a specific objectclass, rather than on a value. That way
it would be less confusing for newcomers. The logic is: if fields is
bound to an object class and the class is not present (that also means
we don't have attribute level rights) -> hide it.
#2 is questionable. IMHO it would require user tests. Also hiding on ''
value might not be always desirable. Nevertheless I like the behavior
but it may be caused by the fact that I already know what to expect.
Generally if a field is supposed to be hidden in an edit page because of
other condition (e.g. token type), not because of the value, the code
should also use that condition instead of relying on the value being
unset. In this particular case, instead of:
if (field !== undefined) {
// display field
}
we probably should do:
var type = ...
if (type == 'hotp') {
// display hotp fields
} else { // totp
// display totp fields
}
I agree - use case #3.
For now the type can be determined by the availability of certain
type-specific fields, but in the future we may add the type itself.
btw somewhere is a rebase issue which leads too:
if (that.readable !== undefined) {
visible = visible && that.readable;
}
if (that.readable !== undefined) {
visible = visible && that.readable;
}
I'll fix that when we agree on an approach.
snip
4. If no "Owner" is specified when adding a token, it will default to
admin. Is this the intended behavior?
Sadly yes.
Maybe the adder dialog should show admin (or the current user?) as the
default owner instead of empty?
Note: all patches except for 727 are ACKed.
--
Petr Vobornik
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
