Re: [Freeipa-devel] [PATCH] Big webUI patch.
Of all this feedback, the only things I consider necessary prior to a
checkin are:
CSS
Facet list
Everything else can wait, I just wanted to get the full analysis recorded.
On 09/13/2010 10:24 PM, Adam Young wrote:
Feedback:
First of all, let me say that this is a tremendous effort. I'm
impressed. Lots of good work here.
Don't include the full state of the application, just the current
tab. The URL gets too long, and the application becomes confused.
When transitioning betwen tabs, if you want to keep the state of other
tabs, save the whole pre hashchange state in a hashtable, keyed on the
tab name. It won't be bookmarked, but it will live as long as the
user doesn't do a reload.
Facets are specific to the entity, not a generalizable list. The code
that manages the set of facets should take a list from the entity
object. Take a look at how the most recent netgroup.js file manages them:
this.setup = function(facet){
http://git.fedorahosted.org/cgi-bin/gitweb.cgi#l100
if (this[facet]){
http://git.fedorahosted.org/cgi-bin/gitweb.cgi#l101
this[facet].setup();
http://git.fedorahosted.org/cgi-bin/gitweb.cgi#l102 }else{
http://git.fedorahosted.org/cgi-bin/gitweb.cgi#l103
this.unspecified.setup();
http://git.fedorahosted.org/cgi-bin/gitweb.cgi#l104 }
http://git.fedorahosted.org/cgi-bin/gitweb.cgi#l105 }
But we also maintain an array:
this.facets = [details,users,groups,hosts,hostgroups];
(I've removed the assignentity factets, as they are going to be
modals just like 'add' is for 'details')
This is one place where JavaScript falls down. We should be able to
enumerate through the property names of the object, but JavaScript
enumeration does not honor order.
The CSS is broken and needs to be redone for:
toplevel tabs
subtabs
facets
list tables
As you mentioned, we need to add services back in.
I get an error on an undefined variable associationsList. Need to
hunt that code down and remove it.
In navigation.js
I'm not a fan of the template approach. I prefer the $jquery
way, as it at least validates the nodes. Please replace code like
var _nav_li_tab_template = 'lia href=#IN/a/li';
function nav_insert_tab_li(jobj, id, name)
{
jobj.append(_nav_li_tab_template.replace('I', id).replace('N', name));
}
with $(li {
html = $(a/,{
id=id,
href=name
}
Don't prepend functions with ipa_ or nav_. We should not be creating
new global variables. Instead, create a single global var ipa= {};
And then the global variables and functions go under that as:
ipa.entity={
search_list: {};
set_search_definition: function(obj_name, data)
{
search_list[obj_name] = data;
}
function tab_on_click(obj)
{
var jobj = $(this);
var state = {};
var id = jobj.closest('.tabs').attr('id');
var index = jobj.parent().prevAll().length;
state[id] = index;
$.bbq.pushState(state);
}
}
functions like tab_on_click that you want to be local should not be
exposed in the public interface, just leave them like this and the
other members of ipa.entity have access to them, but nothing else.
Don't repeat long parameter lists. Create a spec object, and pass it
in to the functions. thus:
function nav_create(nls, container, tabclass)
becomes
/ *spec must have nls, container, tabclass*/
function nav_create(spec);
Then it can be called
nav_create({nls : blah, container : that, tabclass: tabclass});
Ideally this is done for factories and Constructors.
webui.js has the javascript function that kicks off all of the loigic,
but it might get executed too early. It gets executed when the
webui.js file is parsed, which might be before the index.xhtml file is
fully loaded. It doesn't seem to be a problem, but one way to make
sure is to put it at the end of the index.xhtml file, or to put an
onload event hander lin the index.xhtml file that then calls the code
in webui.xhtml. It is OK to start JS processing prior to the load of
the main page, so long as it doesn't modify the dom of the main page.
I suspect that the reason this works so far is because of the
additional json calls for init and for whoami.
Again, delegate the code of the form if (facet)... to the tab
object, just like the setup code above.
add.js: Add/ Add Edit should be Add and Edit /Add and Add Another.
The logic looks OK, just the labels are off, I think
associate.js : The H1 tag is rendereing both above and below the
enrollments.
We should change obj_name to entity, but not in this patch.
groups.js: f_posix should probably be if_posix
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
Re: [Freeipa-devel] Proposed Javascript coding standards
On 2010-09-13 23:46, Simo Sorce wrote:
On Mon, 13 Sep 2010 17:02:19 -0400
Adam Youngayo...@redhat.com wrote:
The is a really nasty bug that the same line policy avoids.
Javascript often attempts to guess where you meant to put semicolons,
and puts the in for you, without telling you.
return
{
status: true;
};
actually returns undefined. I fully acknowledge that this is brain
dead. There are some really brain-dead features in JavaScript.
It is easier to be consistent here, hence the rule always put it on
the opening statement line.
Ok, in that case please note the rationale in the coding style.
Also though in that case I think function() { (with the space) is
better than function(){, unless the sapce is what causes javascript to
put in the automatic ';'. If that's the case I hate it :)
For functions I also prefer:
func()
{
}
but only use it for file scope (thus global) functions in Javascript.
For nested functions and every other compound statement:
func() {
}
I wouldn't mind switching to the second variant for everything for
consistency.
We also banned C++ style comments in C code, /* */ is preferred and
should never be added on the same line of code but only on the
previous line.
I'm OK with that rule. C++ style comments are only to be used for
commenting out code, which probably shouldn't get checked in anyway.
Given space matters in javascript I say that the git history is where
you put unused code, not in comments :)
Simo.
I don't like the 'spec' object to be used instead of naming each
variable separately for parameter lists of functions. I think it's very
artificial. I do agree, that being able to do this:
function some_func(spec) {
return (spec.param1 + spec.param2);
}
var some_var = some_func({'param1': 'value1', 'param2': 'value2'})
is nice, but it makes the code less readable. You can't tell directly
what parameters the function takes.
The rest is almost 100% compatible with my coding style, so naturally I
agree. :D
Pavel
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-freeipa-0023-user-whoami.patch
On 09/14/2010 12:49 PM, Endi Sukma Dewata wrote:
- Adam Youngayo...@redhat.com wrote:
user whoami
Added a whoami option to the user, allows the user to query their
own information based on their Kerberos principal
https://fedorahosted.org/freeipa/attachment/ticket/47/admiyo-freeipa-0023-user-whoami.patch
This will be used to return the users principal and rolegroups.
Test with :
curl -H Content-Type:application/json -H
Accept:applicaton/json -H Accept-Language:es--negotiate -u
: --cacert /etc/ipa/ca.crt -d
'{method:user_find,params:[[],{ all:true,whoami:True }
],id:0}' -X POST http://127.0.0.1:/ipa/json
as well as
ipa user-find --whoami --all
ACK, but as we discussed there's an existing bug with the whoami operation
which causes it to fetch the wrong principal:
[r...@dev scripts]# kdestroy
[r...@dev scripts]# klist
klist: No credentials cache found (ticket cache FILE:/tmp/krb5cc_0)
[r...@dev scripts]# kinit edewata
Password for edew...@dev.example.com:
[r...@dev scripts]# klist
Ticket cache: FILE:/tmp/krb5cc_0
Default principal: edew...@dev.example.com
Valid starting ExpiresService principal
09/14/10 14:42:02 09/15/10 14:41:59 krbtgt/dev.example@dev.example.com
[r...@dev scripts]# ipa user-find --whoami
--
1 user matched
--
User login: admin
Last name: Administrator
Home directory: /home/admin
Login shell: /bin/bash
Groups: admins
Rolegroups: replicaadmin
Taskgroups: managereplica, deletereplica
Number of entries returned 1
[r...@dev scripts]# ipa user-find --whoami
--
1 user matched
--
User login: edewata
First name: Endi
Last name: Dewata
Home directory: /home/edewata
Login shell: /bin/sh
Groups: ipausers
Number of entries returned 1
--
Endi S. Dewata
pushed to master
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-freeipa-0024-user-whoami.patch
Adam Young wrote:
admiyo-freeipa-0024-user-whoami.patch broke the user-find, due to a
missing return statement. It has been reverted. Here is the corrected one.
NACK.
I think you want to use false for options.get:
if options.get('whoami', False):
Otherwise it will always return the whoami version.
I'm not sure which is most efficient when building a string but it is
easier to read the filter this way IMHO:
return ((objectclass=posixaccount)(krbprincipalname=%s))%\
util.get_current_principal()
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] Adding quick links in user and group search results.
Hi,
Please review the attached patch. This patch requires
pzuna-freeipa-0022-2-BIG.patch.
Should we create a branch in the main repository for this redesign? I
think we will need to make a number of changes before we could merge
this to master. Once the redesigned code is the same level as the old
one we could merge it back to master.
Thanks!
Patch description:
The render_call() signature has been modified to pass the entry_attrs
so each callback function can construct the appropriate quick links
using any attributes from the search results.
The callback function has been implemented for user and group entities.
--
Endi S. Dewata
From bef6aa95bdfd13d360e4a0420d467915330db833 Mon Sep 17 00:00:00 2001
From: Endi Sukma Dewata edew...@redhat.com
Date: Tue, 14 Sep 2010 22:26:15 -0400
Subject: [PATCH] Adding quick links in user and group search results.
The render_call() signature has been modified to pass the entry_attrs
so each callback function can construct the appropriate quick links
using any attributes from the search results.
The callback function has been implemented for user and group entities.
---
install/static/group.js | 44 ++
install/static/search.js |6 ++--
install/static/user.js | 58 +-
3 files changed, 104 insertions(+), 4 deletions(-)
diff --git a/install/static/group.js b/install/static/group.js
index 113c1f1..685c5be 100644
--- a/install/static/group.js
+++ b/install/static/group.js
@@ -24,6 +24,7 @@ ipa_entity_set_search_definition('group', [
['cn', 'Name', null],
['gidnumber', 'GID', null],
['description', 'Description', null],
+['actions', 'Actions', group_render_actions]
]);
ipa_entity_set_add_definition('group', [
@@ -64,3 +65,46 @@ function f_posix(dlg, mode)
}
}
+function group_render_actions(tr, attr, value, entry_attrs) {
+
+var td = $(td/);
+tr.append(td);
+
+$(a/, {
+href: jslink,
+html: [D],
+click: function() {
+var state = {};
+state['group-facet'] = 'details';
+state['group-pkey'] = entry_attrs['cn'][0];
+$.bbq.pushState(state);
+return false;
+}
+}).appendTo(td);
+
+$(a/, {
+href: jslink,
+html: [U],
+click: function() {
+var state = {};
+state['group-facet'] = 'associate';
+state['group-enroll'] = 'user';
+state['group-pkey'] = entry_attrs['cn'][0];
+$.bbq.pushState(state);
+return false;
+}
+}).appendTo(td);
+
+$(a/, {
+href: jslink,
+html: [N],
+click: function() {
+var state = {};
+state['group-facet'] = 'associate';
+state['group-enroll'] = 'netgroup';
+state['group-pkey'] = entry_attrs['cn'][0];
+$.bbq.pushState(state);
+return false;
+}
+}).appendTo(td);
+}
diff --git a/install/static/search.js b/install/static/search.js
index 6a7c40b..7347dfc 100644
--- a/install/static/search.js
+++ b/install/static/search.js
@@ -113,9 +113,9 @@ function search_generate_tr(thead, tbody, entry_attrs)
var value = entry_attrs[attr];
var render_call = window[jobj.attr('title')];
-if (typeof render_call == 'function')
-render_call(tr, attr, value);
-else
+if (typeof render_call == 'function') {
+render_call(tr, attr, value, entry_attrs);
+} else
search_generate_td(tr, attr, value);
}
diff --git a/install/static/user.js b/install/static/user.js
index a8954be..2d77fac 100644
--- a/install/static/user.js
+++ b/install/static/user.js
@@ -27,7 +27,7 @@ ipa_entity_set_search_definition('user', [
['mail', 'EMAIL', null],
['telephonenumber', 'Phone', null],
['title', 'Job Title', null],
-['actions', 'Actions', null]
+['actions', 'Actions', user_render_actions]
]);
ipa_entity_set_add_definition('user', [
@@ -209,3 +209,59 @@ function a_manager(jobj, result, mode)
{
}
+function user_render_actions(tr, attr, value, entry_attrs) {
+
+var td = $(td/);
+tr.append(td);
+
+$(a/, {
+href: jslink,
+html: [D],
+click: function() {
+var state = {};
+state['user-facet'] = 'details';
+state['user-pkey'] = entry_attrs['uid'][0];
+$.bbq.pushState(state);
+return false;
+}
+}).appendTo(td);
+
+$(a/, {
+href: jslink,
+html: [G],
+click: function() {
+var state = {};
+state['user-facet'] = 'associate';
+state['user-enroll'] = 'group';
+state['user-pkey'] = entry_attrs['uid'][0];
+$.bbq.pushState(state);
+return false;
+}
+}).appendTo(td);
+
+$(a/, {
+href: jslink,
+html: [N],
+click:
[Freeipa-devel] [www.transifex.net] Team Creation Requested: French
Hello freeipa, this is Transifex at http://www.transifex.net. A translation team for 'French' has been required to the 'FreeIPA' project. Please, visit Transifex at http://www.transifex.net/projects/p/freeipa/teams/ in order to manage the teams of the project. Always at your service. -- Transifex -- Open Translation Platform To change your notification settings, please visit your profile page at http://www.transifex.net/notices/. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 528 make some hbac options mutually exclusive
Rob Crittenden wrote: If an HBAC category is 'all' don't allow individual objects to be added. Basically, make 'all' mutually exclusive. This makes debugging lots easier. If say usercat='all' there is no point adding specific users to the rule because it will always apply to everyone. ticket 164 Comparison to 'all' should be case insensitive. I do not know Python syntax but from general experience I assume it is a NACK. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Thank you, Dmitri Pal Engineering Manager IPA project, Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-freeipa-0024-user-whoami.patch
On 09/14/2010 05:57 PM, Rob Crittenden wrote:
Adam Young wrote:
admiyo-freeipa-0024-user-whoami.patch broke the user-find, due to a
missing return statement. It has been reverted. Here is the corrected
one.
NACK.
I think you want to use false for options.get:
if options.get('whoami', False):
Otherwise it will always return the whoami version.
Doesn't seem to be working that way.
If I kinit as kfrog:
ipa user-find pdawn
--
1 user matched
--
User login: pdawn
First name: Prairie
Last name: Dawn
Home directory: /home/pdawn
Login shell: /bin/sh
Groups: ipausers, muppets
Number of entries returned 1
[ayo...@ipa ~]$ ipa user-find
---
7 users matched
---
...
I'm not sure which is most efficient when building a string but it is
easier to read the filter this way IMHO:
return ((objectclass=posixaccount)(krbprincipalname=%s))%\
util.get_current_principal()
If you still NACK after the previous comment, I'll do the printf style.
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Adding quick links in user and group search results.
On 09/14/2010 06:45 PM, Endi Sukma Dewata wrote: Hi, Please review the attached patch. This patch requires pzuna-freeipa-0022-2-BIG.patch. Should we create a branch in the main repository for this redesign? I think we will need to make a number of changes before we could merge this to master. Once the redesigned code is the same level as the old one we could merge it back to master. Thanks! Patch description: The render_call() signature has been modified to pass the entry_attrs so each callback function can construct the appropriate quick links using any attributes from the search results. The callback function has been implemented for user and group entities. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK, but change the Column heading to Quick Links first ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
