[Phamm] [PATCH] phamm-0.5.15: Use TEXTAREA instead of confusing checkbox to edit multiple valued attribute
kabe at sra-tohoku.co.jp
kabe at sra-tohoku.co.jp
Fri May 1 12:34:06 CEST 2009
My previous patch sent on 2009-02-07 didn't correctly
handle inputs like
_forward at to.domain__
_<blank line>_______
_forward at to.domain__
which should be common when deleting a forward target.
(Stock Phamm doesn't handle this as it doesn't use TEXTAREA for deleting an entry)
Please discard the 2009-02-07 "[PATCH] phamm-0.5.15: Use TEXTAREA instead of confusing checkbox to unset multiple valued attribute" patch.
For multiple valued LDAP attribute, current Phamm presents
(e.g for alias user modification)
___________________________
(o) ALIAS Destination ___<blank textarea>________
___________________________
[ ] current at setting1.com
[ ] current at setting2.com
which the checkbox is *checked to delete* the existing value,
which was unnecessarily confusing.
This patch will change the interface to present
current at setting1.com________
(o) ALIAS Destination current at setting2.com________
____________________________
which should be much more comprehensible and easy to edit.
Bonus: You can replace the value in single step by editing the TEXTAREA.
(previously, for LDAP MUST attributes like maildrop:,
you had to fill new value in textarea, [Modify account],
check the old values to delete, [Modify account])
After examining the CVS history, using the TEXTAREA and CHECKBOX
seems to be a patchwork legacy, which textarea was first intended
for adding entries, not for editing. Editing can get rid of the
confusing checkbox altogether.
The overall logic had became a lot clearer by eliminating the
disaffirmitive checkbox, and getting rid of
$values_multi_del[$pn][$attib] CGI data array.
Although not invoked in current plugins, the patch will (correctly?)
display the <multiple> field by uneditable <PRE> if the
login user didn't have a privilege.
--
kabe
Index: lib/xhtml.php
===================================================================
RCS file: /admin/sra01/CVSroot/phamm/lib/xhtml.php,v
retrieving revision 1.1.1.2
retrieving revision 1.3.4.2
diff -c -r1.1.1.2 -r1.3.4.2
*** lib/xhtml.php 5 Feb 2009 13:26:01 -0000 1.1.1.2
--- lib/xhtml.php 6 Feb 2009 16:30:25 -0000 1.3.4.2
***************
*** 584,610 ****
elseif (isset($attr["MULTIPLE"]) && !isset($hidden))
{
! // Show box to add multiple values
! $tag .= '<textarea name="values_multi['.$p_name.']['.$name.']" cols="35" rows="5">';
! if (isset($attr["TEXTAREA"]))
! $tag .= stripslashes($value);
! $tag .= "</textarea><br/>";
!
! // Show the values
! if (is_array($value))
{
! for ($i=0; $i < $value["count"]; $i++)
{
! $tag .= '<input type="checkbox" name="values_multi_del['.$p_name.']['.$name.'][]" value="'.$value[$i].'" />';
! $tag .= $value[$i]."<br/>";
}
}
! }
! elseif (isset($attr["SUBORDINATEDDELETE"]) && $value)
{
! $tag .= '<input type="checkbox" name="values_multi_del['.$p_name.']['.$name.']" value="'.$value.'" id="subordinatedelete" />';
! $tag .= $value."<br/>";
}
--- 584,632 ----
elseif (isset($attr["MULTIPLE"]) && !isset($hidden))
{
! if ($_SESSION["login"]["level"] < $minAuthLevel)
{
! $tag .= '<PRE>';
! if (is_array($value))
! {
! for ($i=0; $i < $value["count"]; $i++)
! $tag .= htmlspecialchars($value[$i])."\n";
! }
! else // plain value
{
! $tag .= htmlspecialchars($value);
}
+ $tag .= "</PRE>";
}
! else
! {
! // Show box to edit multiple values
! $tag .= '<textarea name="values_multi['.$p_name.']['.$name.']" cols="35" rows="5">';
! if (isset($attr["TEXTAREA"]))
! {
! $tag .= stripslashes($value);
! }
! elseif (is_array($value))
! {
! for ($i=0; $i < $value["count"]; $i++)
! {
! if ($i) $tag .= "\n";
! $tag .= htmlspecialchars($value[$i]);
! }
! }
! else
! {
! $tag .= htmlspecialchars($value);
! }
! $tag .= "</textarea><br />";
! }
! }//MULTIPLE
! elseif (isset($attr["SUBORDINATEDDELETE"]) && $value && !$hidden)
{
! // use $values_multi to erase the attribute if blank
! $tag .= '<input type="text" name="values_multi['.$p_name.']['.$name.']" value="'.$value.'" id="subordinatedelete" />';
! $tag .= "<br />";
}
***************
*** 923,929 ****
if ($_SESSION["login"]["level"] >= $minAuthLevel)
{
! $js_string = ($field == 'vacationactive' ? 'onchange="subordinatedelete.checked=true;"' : '');
$tag .= '<select name="'.$table.'['.$field.']" '.$js_string.'>';
--- 945,951 ----
if ($_SESSION["login"]["level"] >= $minAuthLevel)
{
! $js_string = ($field == 'vacationactive' ? 'onchange="subordinatedelete.value=\'\';"' : '');
$tag .= '<select name="'.$table.'['.$field.']" '.$js_string.'>';
Index: www-data/main.php
===================================================================
RCS file: /admin/sra01/CVSroot/phamm/www-data/main.php,v
retrieving revision 1.1.1.1
retrieving revision 1.2.4.3
diff -c -r1.1.1.1 -r1.2.4.3
*** www-data/main.php 5 Feb 2009 12:30:53 -0000 1.1.1.1
--- www-data/main.php 1 May 2009 09:35:48 -0000 1.2.4.3
***************
*** 552,560 ****
$values_multi_array = explode (",",$values_multi_string);
! for ($i=0; $i < count($values_multi_array); $i++)
! $entry[$mv_name][$i] = $values_multi_array[$i];
! }
}
$r = @phamm_add ('mail='.$mail.',vd='.$domain.','.LDAP_BASE,$entry);
--- 552,566 ----
$values_multi_array = explode (",",$values_multi_string);
! foreach ($values_multi_array as $mv_val)
! {
! // LDAP cannot have valueless attribute; ignore null string.
! // Totally blank input will (correctly) erase the attribute
! // since $entry[<attrib>] will be an empty array.
! if ($mv_val != "")
! $entry[$mv_name][] = $mv_val;
! }//$values_multi_array
! }//$values_multi_purged
}
$r = @phamm_add ('mail='.$mail.',vd='.$domain.','.LDAP_BASE,$entry);
***************
*** 622,628 ****
$values = $_POST["values"];
$values_date = (isset($_POST["values_date"]) ? $_POST["values_date"] : null);
$values_multi = (isset($_POST["values_multi"]) ? $_POST["values_multi"] : null);
- $values_multi_del = (isset($_POST["values_multi_del"]) ? $_POST["values_multi_del"] : null);
$confirm = $_POST["confirm"];
$account = (isset($_POST["account_new"]) ? $_POST["account_new"] : null);
--- 628,633 ----
***************
*** 637,642 ****
--- 642,648 ----
$values_date_purged_one = array();
$values_multi_purged = array();
$values_date_purged = array();
+ $entry_add = array();
$entry["sn"] = $sn;
$entry["givenname"] = $givenName;
***************
*** 665,670 ****
--- 671,688 ----
{
foreach ( $confirm as $p_name )
{
+ // Scan for possible empty attributes marked by
+ // existance of $values_multi, and initialize
+ // $entry_add[<attr>] = array() to delete the attribute
+ // if there was no value
+ if (count($values_multi[$p_name]) > 0)
+ {
+ foreach ($values_multi[$p_name] as $mv_name => $mv_val)
+ {
+ $entry_add[$mv_name] = array(); //empty array to delete attrib
+ }
+ }
+
// Purge empty values
if (isset($pv[$p_name]["ACCOUNT"]))
$values_purged = purge_empty_values($values[$p_name],$pv[$p_name]["ACCOUNT"]["ATTRIBUTES"]);
***************
*** 672,677 ****
--- 690,696 ----
$values_purged = purge_empty_values($values[$p_name]);
if (isset($values_multi[$p_name]))
+ // $values_multi[$p_name][<attrname>] = "string[,string]"
$values_multi_purged_one = purge_empty_values($values_multi[$p_name]);
if (isset($values_date[$p_name]))
$values_date_purged_one = purge_empty_values($values_date[$p_name]);
***************
*** 680,692 ****
$entry = array_merge($entry,$values_purged);
$values_multi_purged = array_merge($values_multi_purged,$values_multi_purged_one);
$values_date_purged = array_merge($values_date_purged,$values_date_purged_one);
!
! // Del Values from checkbox
! if (isset($values_multi_del[$p_name]))
! {
! $rd = phamm_mod_del ('mail='.$mail.',vd='.$domain.','.LDAP_BASE,$values_multi_del[$p_name]);
! }
! }
}
$mail = $account;
--- 699,706 ----
$entry = array_merge($entry,$values_purged);
$values_multi_purged = array_merge($values_multi_purged,$values_multi_purged_one);
$values_date_purged = array_merge($values_date_purged,$values_date_purged_one);
!
! }//$p_name
}
$mail = $account;
***************
*** 713,730 ****
$values_multi_array = explode (",",$values_multi_string);
! for ($i=0; $i < count($values_multi_array); $i++)
! $entry_add[$mv_name][$i] = $values_multi_array[$i];
! }
!
! if (count($values_multi_array) > 0)
{
! $ra = phamm_mod_add ('mail='.$mail.',vd='.$domain.','.LDAP_BASE,$entry_add);
! }
!
}
! // This swith action to change mail or postmaster value
if ($mail)
{
$entry["lastChange"] = time();
--- 727,755 ----
$values_multi_array = explode (",",$values_multi_string);
! foreach ($values_multi_array as $mv_val)
! {
! // LDAP cannot have valueless attribute; ignore null string.
! // Totally blank input will (correctly) erase the attribute
! // since $entry_add[<attrib>] will be an empty array.
! if ($mv_val != "")
! $entry_add[$mv_name][] = $mv_val;
! }//$values_multi_array
! }//$values_multi_purged
! }
! if (count($entry_add) > 0)
! {
! $ra = phamm_modify ('mail='.$mail.',vd='.$domain.','.LDAP_BASE,$entry_add);
! if (!$ra)
{
! phamm_print_message('error',sprintf(_("account %s not updated!"),$account));
! print phamm_error();
! break;
! }
}
+ unset($entry_add);
! // This switch action to change mail or postmaster value
if ($mail)
{
$entry["lastChange"] = time();
More information about the Phamm
mailing list