From e9dfbfa773149c57544e5c8e4d87a00fc9960bf1 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Thu, 8 Nov 2007 22:12:42 -0500 Subject: Enable multi-value field support for some attributes on the edit pages Better error reporting in the GUI Include a document describing how multi-valued fields work --- ipa-python/ipaerror.py | 5 + ipa-server/ipa-gui/README.multivalue | 27 +++ ipa-server/ipa-gui/ipagui/forms/group.py | 3 + ipa-server/ipa-gui/ipagui/forms/user.py | 8 + ipa-server/ipa-gui/ipagui/subcontrollers/group.py | 49 ++++- ipa-server/ipa-gui/ipagui/subcontrollers/user.py | 87 +++++++- .../ipa-gui/ipagui/templates/groupeditform.kid | 36 +++- ipa-server/ipa-gui/ipagui/templates/groupshow.kid | 20 +- .../ipa-gui/ipagui/templates/usereditform.kid | 223 ++++++++++++++++----- .../ipa-gui/ipagui/templates/usernewform.kid | 2 + ipa-server/ipa-gui/ipagui/templates/usershow.kid | 96 ++++++++- 11 files changed, 477 insertions(+), 79 deletions(-) create mode 100644 ipa-server/ipa-gui/README.multivalue diff --git a/ipa-python/ipaerror.py b/ipa-python/ipaerror.py index 0106132ca..b10a9a8fc 100644 --- a/ipa-python/ipaerror.py +++ b/ipa-python/ipaerror.py @@ -28,6 +28,11 @@ class IPAError(exceptions.Exception): error.""" self.code = code self.message = message + # Fill this in as an empty LDAP error message so we don't have a lot + # of "if e.detail ..." everywhere + if detail is None: + detail = [] + detail.append({'desc':'','info':''}) self.detail = detail def __str__(self): diff --git a/ipa-server/ipa-gui/README.multivalue b/ipa-server/ipa-gui/README.multivalue new file mode 100644 index 000000000..ba315181d --- /dev/null +++ b/ipa-server/ipa-gui/README.multivalue @@ -0,0 +1,27 @@ +The way multi-valued fields work is this: + - A new widget is added to the form. I name it as the attribute + s. + For example, I use cns for the cn attribute. + - If you need a new validator use a ForEach() so that each value is + checked. + - This attribute is populated from the incoming attribute from the + user or group record. The widget can support multiple fields at once + but I'm using it for just one field. In fact, I don't know if it + will work with more the way I'm using it. + - In the GUI an operator can add/remove values to each multi-valued field. + - Naming is very important in the widget. TurboGears automatically + re-assembles the data into a list of dict entries if you name things + properly. For example, the cns (multiple CN entries) looks like: + cns-0.cn=Rob+Crittenden&cns-1.cn=Robert+Crittenden&cns-2.cn=rcrit + - This gets converted to: + [{'cn': u'Rob Crittenden'}, {'cn': u'Robert Crittenden'}, {'cn': u'rcrit'}] + - I take this list of dicts and pull out each value and append it to a new + list that represents the original multi-valued field + - Then the list/dict version is removed (in this case, kw['cns']). + +When adding a new field you have to update: + +1. The form to add the new ExpandingForm() field and perhaps a validator +2. The edit template to add the boilerplate to display the field +3. The show template to be able to display all the fields separately +4. The new template if you want to be able to enter these on new entries +5. The subcontroller so you can do the input and output conversions diff --git a/ipa-server/ipa-gui/ipagui/forms/group.py b/ipa-server/ipa-gui/ipagui/forms/group.py index 380c904a4..f9ae5e5ea 100644 --- a/ipa-server/ipa-gui/ipagui/forms/group.py +++ b/ipa-server/ipa-gui/ipagui/forms/group.py @@ -1,8 +1,10 @@ import turbogears from turbogears import validators, widgets +from tg_expanding_form_widget.tg_expanding_form_widget import ExpandingForm class GroupFields(): cn = widgets.TextField(name="cn", label="Name") + cns = ExpandingForm(name="cns", label="Common Names", fields=[cn]) gidnumber = widgets.TextField(name="gidnumber", label="GID") description = widgets.TextField(name="description", label="Description") @@ -37,6 +39,7 @@ class GroupNewForm(widgets.Form): class GroupEditValidator(validators.Schema): + cn = validators.ForEach(validators.String(not_empty=True)) gidnumber = validators.Int(not_empty=False) description = validators.String(not_empty=False) diff --git a/ipa-server/ipa-gui/ipagui/forms/user.py b/ipa-server/ipa-gui/ipagui/forms/user.py index 1a35b4e07..b426f8e91 100644 --- a/ipa-server/ipa-gui/ipagui/forms/user.py +++ b/ipa-server/ipa-gui/ipagui/forms/user.py @@ -1,10 +1,12 @@ import turbogears from turbogears import validators, widgets +from tg_expanding_form_widget.tg_expanding_form_widget import ExpandingForm class UserFields(): givenname = widgets.TextField(name="givenname", label="Given Name") sn = widgets.TextField(name="sn", label="Family Name") cn = widgets.TextField(name="cn", label="Common Names") + cns = ExpandingForm(name="cns", label="Common Names", fields=[cn]) title = widgets.TextField(name="title", label="Title") displayname = widgets.TextField(name="displayname", label="Display Name") initials = widgets.TextField(name="initials", label="Initials") @@ -21,11 +23,16 @@ class UserFields(): mail = widgets.TextField(name="mail", label="E-mail Address") telephonenumber = widgets.TextField(name="telephonenumber", label="Work Number") + telephonenumbers = ExpandingForm(name="telephonenumbers", label="Work Numbers", fields=[telephonenumber]) facsimiletelephonenumber = widgets.TextField(name="facsimiletelephonenumber", label="Fax Number") + facsimiletelephonenumbers = ExpandingForm(name="facsimiletelephonenumbers", label="Fax Numbers", fields=[facsimiletelephonenumber]) mobile = widgets.TextField(name="mobile", label="Cell Number") + mobiles = ExpandingForm(name="mobiles", label="Cell Numbers", fields=[mobile]) pager = widgets.TextField(name="pager", label="Pager Number") + pagers = ExpandingForm(name="pagers", label="Pager Numbers", fields=[pager]) homephone = widgets.TextField(name="homephone", label="Home Number") + homephones = ExpandingForm(name="homephones", label="Home Numbers", fields=[homephone]) street = widgets.TextField(name="street", label="Street Address") l = widgets.TextField(name="l", label="City") @@ -102,6 +109,7 @@ class UserEditValidator(validators.Schema): userpassword_confirm = validators.String(not_empty=False) givenname = validators.String(not_empty=True) sn = validators.String(not_empty=True) + cn = validators.ForEach(validators.String(not_empty=True)) mail = validators.Email(not_empty=True) uidnumber = validators.Int(not_empty=False) gidnumber = validators.Int(not_empty=False) diff --git a/ipa-server/ipa-gui/ipagui/subcontrollers/group.py b/ipa-server/ipa-gui/ipagui/subcontrollers/group.py index f0574a21c..8ea87641e 100644 --- a/ipa-server/ipa-gui/ipagui/subcontrollers/group.py +++ b/ipa-server/ipa-gui/ipagui/subcontrollers/group.py @@ -90,7 +90,7 @@ class GroupController(IPAController): # on any error, we redirect to the _edit_ group page. # this code does data setup, similar to groupedit() # - group = client.get_entry_by_cn(kw['cn'], group_fields) + group = client.get_entry_by_cn(kw['cn'][0], group_fields) group_dict = group.toDict() member_dicts = [] @@ -180,6 +180,14 @@ class GroupController(IPAController): group_dict = group.toDict() + # Load potential multi-valued fields + if isinstance(group_dict['cn'], str): + group_dict['cn'] = [group_dict['cn']] + cns = [] + for cn in group_dict['cn']: + cns.append(dict(cn=cn)) + group_dict['cns'] = cns + # # convert members to users, for easier manipulation on the page # @@ -210,14 +218,19 @@ class GroupController(IPAController): self.restrict_post() client = self.get_ipaclient() + # Fix incoming multi-valued form fields + kw['cn'] = [] + for i in range(len(kw['cns'])): + kw['cn'].append(kw['cns'][i]['cn']) + del(kw['cns']) + if kw.get('submit') == 'Cancel Edit': turbogears.flash("Edit group cancelled") - raise turbogears.redirect('/group/show', cn=kw.get('cn')) + raise turbogears.redirect('/group/show', cn=kw.get('cn')[0]) # Decode the member data, in case we need to round trip member_dicts = loads(b64decode(kw.get('member_data'))) - tg_errors, kw = self.groupupdatevalidate(**kw) if tg_errors: turbogears.flash("There were validation errors.
" + @@ -233,6 +246,9 @@ class GroupController(IPAController): try: orig_group_dict = loads(b64decode(kw.get('group_orig'))) + # remove multi-valued form fields + del(orig_group_dict['cns']) + new_group = ipa.group.Group(orig_group_dict) if new_group.description != kw.get('description'): group_modified = True @@ -243,6 +259,14 @@ class GroupController(IPAController): group_modified = True new_group.setValue('gidnumber', new_gid) + # Did any cn entries change? + oldcn = new_group.getValues('cn') + if isinstance(oldcn, str): + oldcn = [oldcn] + if oldcn != kw['cn']: + group_modified = True + new_group.setValue('cn', kw['cn']) + if group_modified: rv = client.update_group(new_group) # @@ -252,7 +276,7 @@ class GroupController(IPAController): # kw['group_orig'] = b64encode(dumps(new_group.toDict())) except ipaerror.IPAError, e: - turbogears.flash("Group update failed: " + str(e)) + turbogears.flash("Group update failed: " + str(e) + "
" + e.detail[0]['desc']) return dict(form=group_edit_form, group=kw, members=member_dicts, tg_template='ipagui.templates.groupedit') @@ -268,8 +292,9 @@ class GroupController(IPAController): failed_adds = client.add_members_to_group( utf8_encode_values(dnadds), new_group.dn) kw['dnadd'] = failed_adds + group_modified = True except ipaerror.IPAError, e: - turbogears.flash("Group update failed: " + str(e)) + turbogears.flash("Group update failed: " + str(e) + "
" + e.detail[0]['desc']) return dict(form=group_edit_form, group=kw, members=member_dicts, tg_template='ipagui.templates.groupedit') @@ -285,8 +310,9 @@ class GroupController(IPAController): failed_dels = client.remove_members_from_group( utf8_encode_values(dndels), new_group.dn) kw['dndel'] = failed_dels + group_modified = True except ipaerror.IPAError, e: - turbogears.flash("Group update failed: " + str(e)) + turbogears.flash("Group update failed: " + str(e) + "
" + e.detail[0]['desc']) return dict(form=group_edit_form, group=kw, members=member_dicts, tg_template='ipagui.templates.groupedit') @@ -308,8 +334,11 @@ class GroupController(IPAController): return dict(form=group_edit_form, group=kw, members=member_dicts, tg_template='ipagui.templates.groupedit') - turbogears.flash("%s updated!" % kw['cn']) - raise turbogears.redirect('/group/show', cn=kw['cn']) + if group_modified == True: + turbogears.flash("%s updated!" % kw['cn'][0]) + else: + turbogears.flash("No modifications requested.") + raise turbogears.redirect('/group/show', cn=kw['cn'][0]) @expose("ipagui.templates.grouplist") @@ -330,7 +359,7 @@ class GroupController(IPAController): turbogears.flash("These results are truncated.
" + "Please refine your search and try again.") except ipaerror.IPAError, e: - turbogears.flash("Find groups failed: " + str(e)) + turbogears.flash("Find groups failed: " + str(e) + "
" + e.detail[0]['desc']) raise turbogears.redirect("/group/list") return dict(groups=groups, criteria=criteria, @@ -358,7 +387,7 @@ class GroupController(IPAController): return dict(group=group_dict, fields=ipagui.forms.group.GroupFields(), members = member_dicts) except ipaerror.IPAError, e: - turbogears.flash("Group show failed: " + str(e)) + turbogears.flash("Group show failed: " + str(e) + "
" + e.detail[0]['desc']) raise turbogears.redirect("/") @expose() diff --git a/ipa-server/ipa-gui/ipagui/subcontrollers/user.py b/ipa-server/ipa-gui/ipagui/subcontrollers/user.py index d328052b1..a33307ae6 100644 --- a/ipa-server/ipa-gui/ipagui/subcontrollers/user.py +++ b/ipa-server/ipa-gui/ipagui/subcontrollers/user.py @@ -61,6 +61,35 @@ class UserController(IPAController): user_new_form.validator.add_field(s['field'], validator) user_edit_form.validator.add_field(s['field'], validator) + def setup_mv_fields(self, field, fieldname): + """Given a field (must be a list) and field name, convert that + field into a list of dictionaries of the form: + [ { fieldname : v1}, { fieldname : v2 }, .. ] + + This is how we pre-fill values for multi-valued fields. + """ + mvlist = [] + if field is not None: + for v in field: + mvlist.append({ fieldname : v } ) + else: + # We need to return an empty value so something can be + # displayed on the edit page. Otherwise only an Add link + # will show, not an empty field. + mvlist.append({ fieldname : '' } ) + return mvlist + + def fix_incoming_fields(self, fields, fieldname, multifieldname): + """This is called by the update() function. It takes the incoming + list of dictionaries and converts it into back into the original + field, then removes the multiple field. + """ + fields[fieldname] = [] + for i in range(len(fields[multifieldname])): + fields[fieldname].append(fields[multifieldname][i][fieldname]) + del(fields[multifieldname]) + + return fields @expose() def index(self): @@ -150,7 +179,7 @@ class UserController(IPAController): return dict(form=user_new_form, user=kw, tg_template='ipagui.templates.usernew') except ipaerror.IPAError, e: - turbogears.flash("User add failed: " + str(e)) + turbogears.flash("User add failed: " + str(e) + "
" + e.detail[0]['desc']) return dict(form=user_new_form, user=kw, tg_template='ipagui.templates.usernew') @@ -259,6 +288,32 @@ class UserController(IPAController): turbogears.flash("User edit failed: No uid or principal provided") raise turbogears.redirect('/') user_dict = user.toDict() + + # Load potential multi-valued fields + if isinstance(user_dict['cn'], str): + user_dict['cn'] = [user_dict['cn']] + user_dict['cns'] = self.setup_mv_fields(user_dict['cn'], 'cn') + + if isinstance(user_dict.get('telephonenumber',''), str): + user_dict['telephonenumber'] = [user_dict.get('telephonenumber'),''] + user_dict['telephonenumbers'] = self.setup_mv_fields(user_dict.get('telephonenumber'), 'telephonenumber') + + if isinstance(user_dict.get('facsimiletelephonenumber',''), str): + user_dict['facsimiletelephonenumber'] = [user_dict.get('facsimiletelephonenumber'),''] + user_dict['facsimiletelephonenumbers'] = self.setup_mv_fields(user_dict.get('facsimiletelephonenumber'), 'facsimiletelephonenumber') + + if isinstance(user_dict.get('mobile',''), str): + user_dict['mobile'] = [user_dict.get('mobile'),''] + user_dict['mobiles'] = self.setup_mv_fields(user_dict.get('mobile'), 'mobile') + + if isinstance(user_dict.get('pager',''), str): + user_dict['pager'] = [user_dict.get('pager'),''] + user_dict['pagers'] = self.setup_mv_fields(user_dict.get('pager'), 'pager') + + if isinstance(user_dict.get('homephone',''), str): + user_dict['homephone'] = [user_dict.get('homephone'),''] + user_dict['homephones'] = self.setup_mv_fields(user_dict.get('homephone'), 'homephone') + # Edit shouldn't fill in the password field. if user_dict.has_key('userpassword'): del(user_dict['userpassword']) @@ -300,7 +355,7 @@ class UserController(IPAController): except ipaerror.IPAError, e: if uid is None: uid = principal - turbogears.flash("User edit failed: " + str(e)) + turbogears.flash("User edit failed: " + str(e) + "
" + e.detail[0]['desc']) raise turbogears.redirect('/user/show', uid=uid) @expose() @@ -314,6 +369,14 @@ class UserController(IPAController): turbogears.flash("Edit user cancelled") raise turbogears.redirect('/user/show', uid=kw.get('uid')) + # Fix incoming multi-valued fields we created for the form + kw = self.fix_incoming_fields(kw, 'cn', 'cns') + kw = self.fix_incoming_fields(kw, 'telephonenumber', 'telephonenumbers') + kw = self.fix_incoming_fields(kw, 'facsimiletelephonenumber', 'facsimiletelephonenumbers') + kw = self.fix_incoming_fields(kw, 'mobile', 'mobiles') + kw = self.fix_incoming_fields(kw, 'pager', 'pagers') + kw = self.fix_incoming_fields(kw, 'homephone', 'homephones') + # Decode the group data, in case we need to round trip user_groups_dicts = loads(b64decode(kw.get('user_groups_data'))) @@ -334,6 +397,14 @@ class UserController(IPAController): try: orig_user_dict = loads(b64decode(kw.get('user_orig'))) + # remove multi-valued fields we created for the form + del(orig_user_dict['cns']) + del(orig_user_dict['telephonenumbers']) + del(orig_user_dict['facsimiletelephonenumbers']) + del(orig_user_dict['mobiles']) + del(orig_user_dict['pagers']) + del(orig_user_dict['homephones']) + new_user = ipa.user.User(orig_user_dict) new_user.setValue('title', kw.get('title')) new_user.setValue('givenname', kw.get('givenname')) @@ -400,7 +471,7 @@ class UserController(IPAController): # too much work to figure out unless someone really screams pass except ipaerror.IPAError, e: - turbogears.flash("User update failed: " + str(e)) + turbogears.flash("User update failed: " + str(e) + "
" + e.detail[0]['desc']) return dict(form=user_edit_form, user=kw, user_groups=user_groups_dicts, tg_template='ipagui.templates.useredit') @@ -412,7 +483,7 @@ class UserController(IPAController): if password_change: rv = client.modifyPassword(kw['krbprincipalname'], "", kw.get('userpassword')) except ipaerror.IPAError, e: - turbogears.flash("User password change failed: " + str(e)) + turbogears.flash("User password change failed: " + str(e) + "
" + e.detail[0]['desc']) return dict(form=user_edit_form, user=kw, user_groups=user_groups_dicts, tg_template='ipagui.templates.useredit') @@ -481,7 +552,7 @@ class UserController(IPAController): turbogears.flash("These results are truncated.
" + "Please refine your search and try again.") except ipaerror.IPAError, e: - turbogears.flash("User list failed: " + str(e)) + turbogears.flash("User list failed: " + str(e) + "
" + e.detail[0]['desc']) raise turbogears.redirect("/user/list") return dict(users=users, uid=uid, fields=ipagui.forms.user.UserFields()) @@ -523,7 +594,7 @@ class UserController(IPAController): user_groups=user_groups, user_reports=user_reports, user_manager=user_manager, user_secretary=user_secretary) except ipaerror.IPAError, e: - turbogears.flash("User show failed: " + str(e)) + turbogears.flash("User show failed: " + str(e) + "
" + e.detail[0]['desc']) raise turbogears.redirect("/") @expose() @@ -539,7 +610,7 @@ class UserController(IPAController): turbogears.flash("user deleted") raise turbogears.redirect('/user/list') except (SyntaxError, ipaerror.IPAError), e: - turbogears.flash("User deletion failed: " + str(e)) + turbogears.flash("User deletion failed: " + str(e) + "
" + e.detail[0]['desc']) raise turbogears.redirect('/user/list') @validate(form=user_new_form) @@ -661,7 +732,7 @@ class UserController(IPAController): users_counter = users[0] users = users[1:] except ipaerror.IPAError, e: - turbogears.flash("search failed: " + str(e)) + turbogears.flash("search failed: " + str(e) + "
" + e.detail[0]['desc']) return dict(users=users, criteria=criteria, which_select=kw.get('which_select'), diff --git a/ipa-server/ipa-gui/ipagui/templates/groupeditform.kid b/ipa-server/ipa-gui/ipagui/templates/groupeditform.kid index cab585fcc..865cdfcc3 100644 --- a/ipa-server/ipa-gui/ipagui/templates/groupeditform.kid +++ b/ipa-server/ipa-gui/ipagui/templates/groupeditform.kid @@ -25,6 +25,8 @@ from ipagui.helpers import ipahelper + @@ -66,15 +68,35 @@ from ipagui.helpers import ipahelper - diff --git a/ipa-server/ipa-gui/ipagui/templates/groupshow.kid b/ipa-server/ipa-gui/ipagui/templates/groupshow.kid index 7a66acdbe..f0d1ddfbb 100644 --- a/ipa-server/ipa-gui/ipagui/templates/groupshow.kid +++ b/ipa-server/ipa-gui/ipagui/templates/groupshow.kid @@ -7,7 +7,7 @@

View Group

@@ -22,7 +22,21 @@ edit_url = tg.url('/group/edit', cn=group.get('cn'))
- + @@ -51,7 +65,7 @@ edit_url = tg.url('/group/edit', cn=group.get('cn')) member_type = "user" view_url = tg.url('/user/show', uid=member_uid) else: - member_cn = "%s" % member.get('cn') + member_cn = "%s" % member.get('cn')[0] member_desc = "[group]" member_type = "group" view_url = tg.url('/group/show', cn=member_cn) diff --git a/ipa-server/ipa-gui/ipagui/templates/usereditform.kid b/ipa-server/ipa-gui/ipagui/templates/usereditform.kid index f6da48870..5afb0d055 100644 --- a/ipa-server/ipa-gui/ipagui/templates/usereditform.kid +++ b/ipa-server/ipa-gui/ipagui/templates/usereditform.kid @@ -26,6 +26,8 @@ from ipagui.helpers import ipahelper src="${tg.url('/static/javascript/dynamicedit.js')}"> + - @@ -364,61 +387,170 @@ from ipagui.helpers import ipahelper - - - - - -
- - - ${value_for(group_fields.cn)} - + + + + + + + + + + + + +
+ + + + Remove (-) +
+ Add ( + )
${group.get("cn")} + + + + + + + +
${values[index]}
+
- - - - + + + + + + + + + + + + +
+ + + + Remove +
+ Add Common Name
- - - + + + + + + + + + + + + +
+ + + + Remove +
+ Add Work Number
- - - + + + + + + + + + + + + +
+ + + + Remove +
+ Add Fax Number
- - - + + + + + + + + + + + + +
+ + + + Remove +
+ Add Cell Number
- - - + + + + + + + + + + + + +
+ + + + Remove +
+ Add Pager Number
- - - + + + + + + + + + + + + +
+ + + + Remove +
+ Add Home Phone
@@ -655,7 +787,7 @@ from ipagui.helpers import ipahelper group_dn = group.get('dn') group_dn_esc = ipahelper.javascript_string_escape(group_dn) - group_name = group.get('cn') + group_name = group.get('cn')[0] group_descr = "[group]" group_type = "group" @@ -685,6 +817,7 @@ from ipagui.helpers import ipahelper div_counter = div_counter + 1 ?> +   diff --git a/ipa-server/ipa-gui/ipagui/templates/usernewform.kid b/ipa-server/ipa-gui/ipagui/templates/usernewform.kid index eeaa87fa4..82a90982d 100644 --- a/ipa-server/ipa-gui/ipagui/templates/usernewform.kid +++ b/ipa-server/ipa-gui/ipagui/templates/usernewform.kid @@ -13,6 +13,8 @@ from ipagui.helpers import ipahelper src="${tg.url('/static/javascript/dynamicedit.js')}"> +