From 2220abb4c2ed33971f512c81da6cc51e71b5267e Mon Sep 17 00:00:00 2001 From: Andrey Kutejko Date: Thu, 9 May 2013 14:09:22 +0300 Subject: [PATCH] cleanup auth admin forms --- ipf/admin/media/css/forms.css | 3 + ipf/admin/templates/admin/base.html | 10 +- ipf/auth/forms/widget/permissions.php | 13 -- ipf/auth/models.yml | 12 +- ipf/auth/models/Permission.php | 1 + ipf/auth/models/Role.php | 49 ++++-- ipf/auth/models/RolePermission.php | 5 +- ipf/auth/models/User.php | 142 ++++++++++++------ ipf/auth/models/UserPermission.php | 5 +- ipf/auth/models/UserRole.php | 5 +- ipf/form/db/boolean.php | 8 +- ipf/form/db/manytomany.php | 1 + ipf/form/extra/addjs.php | 21 --- ipf/form/extra/checkgroup.php | 51 ------- ipf/form/extra/widget/checkboxgroupinput.php | 30 ---- ipf/form/extra/widget/checkboxinput.php | 18 --- ipf/form/field/modelmultiplechoice.php | 7 +- ipf/form/model.php | 16 +- ipf/form/widget/checkboxinput.php | 12 +- ipf/form/widget/selectmultipleinput.php | 10 +- .../widget/selectmultipleinputcheckbox.php | 16 +- 21 files changed, 187 insertions(+), 248 deletions(-) delete mode 100644 ipf/auth/forms/widget/permissions.php delete mode 100644 ipf/form/extra/addjs.php delete mode 100644 ipf/form/extra/checkgroup.php delete mode 100644 ipf/form/extra/widget/checkboxgroupinput.php delete mode 100644 ipf/form/extra/widget/checkboxinput.php diff --git a/ipf/admin/media/css/forms.css b/ipf/admin/media/css/forms.css index adc1249..dc1c469 100644 --- a/ipf/admin/media/css/forms.css +++ b/ipf/admin/media/css/forms.css @@ -27,6 +27,9 @@ form ul.radiolist label { float:none; display:inline; } form ul.inline { margin-left:0; padding:0; } form ul.inline li { float:left; padding-right:7px; } +.checkgroup_master li {list-style: none outside none} +.checkgroup_master li label {width:auto;float:none} + /* ALIGNED FIELDSETS */ .aligned label { display:block; padding:0 1em 3px 0; float:left; width:8em; } .aligned label.inline { display:inline; float:none; } diff --git a/ipf/admin/templates/admin/base.html b/ipf/admin/templates/admin/base.html index 3c8a35e..6b4f810 100644 --- a/ipf/admin/templates/admin/base.html +++ b/ipf/admin/templates/admin/base.html @@ -23,13 +23,9 @@ {block commonjs} - {if isset($form) && method_exists($form, 'render_commonjs')} - {$form.render_commonjs()|safe} - {else} - - - - {/if} + + + {/block} {block scripts}{/block} diff --git a/ipf/auth/forms/widget/permissions.php b/ipf/auth/forms/widget/permissions.php deleted file mode 100644 index 736cbf9..0000000 --- a/ipf/auth/forms/widget/permissions.php +++ /dev/null @@ -1,13 +0,0 @@ -name); } } + diff --git a/ipf/auth/models/Role.php b/ipf/auth/models/Role.php index 6f6c3e9..e600af9 100644 --- a/ipf/auth/models/Role.php +++ b/ipf/auth/models/Role.php @@ -8,6 +8,42 @@ class Role extends BaseRole } } +class IPFAdminRoleForm extends IPF_Form_Model +{ + public function add__Permissions__field() + { + if (!IPF_Auth_App::ArePermissionsEnabled()) + return; + + $choices = array(); + foreach (IPF_ORM::getTable('Permission')->findAll() as $o) + $choices[$o->__toString()] = $o->id; + ksort($choices); + + $field = new IPF_Form_Field_ModelMultipleChoice(array( + 'required' => false, + 'label' => 'Permissions', + 'help_text' => '', + 'type' => 'manytomany', + 'editable' => true, + 'model' => 'Permission', + 'widget' => 'IPF_Form_Widget_SelectMultipleInputCheckbox', + 'choices' => $choices, + 'widget_attrs' => array('class' => 'checkgroup'), + )); + + $this->fields['Permissions'] = $field; + } + + public function extra_js() + { + $extra_js = parent::extra_js(); + if (IPF_Auth_App::ArePermissionsEnabled()) + $extra_js[] = ''; + return array_unique($extra_js); + } +} + class AdminRole extends IPF_Admin_Model { public function list_display() @@ -35,19 +71,12 @@ class AdminRole extends IPF_Admin_Model protected function _getForm($model_obj, $data, $extra) { $extra['model'] = $model_obj; - $extra['checkgroup_fields'] = array( - 'Permissions' => array('widget'=>'IPF_Auth_Forms_Widget_Permissions'), - ); - return new IPF_Form_Extra_CheckGroup($data, $extra); - } - - protected function _setupForm($form) - { - IPF_Form_Extra_CheckGroup::SetupForm($form); + return new IPFAdminRoleForm($data, $extra); } public function page_title() { return 'Group'; } public function verbose_name() { return 'Group'; } } -IPF_Admin_Model::register('Role','AdminRole'); +IPF_Admin_Model::register('Role', 'AdminRole'); + diff --git a/ipf/auth/models/RolePermission.php b/ipf/auth/models/RolePermission.php index 9707d3e..c407a8c 100644 --- a/ipf/auth/models/RolePermission.php +++ b/ipf/auth/models/RolePermission.php @@ -1,9 +1,6 @@ fields['email']->label = 'E-mail'; - + $this->fields['is_active']->label = 'Active'; $this->fields['is_staff']->label = 'Staff status'; $this->fields['is_superuser']->label = 'Superuser status'; @@ -15,20 +15,19 @@ class IPFAuthAdminUserForm extends IPF_Form_Extra_CheckGroup $this->fields['is_active']->help_text = 'Designates whether this user should be treated as active. Unselect this instead of deleting accounts.'; $this->fields['is_staff']->help_text = 'Designates whether the user can log into this admin site.'; $this->fields['is_superuser']->help_text = 'Designates that this user has all permissions without explicitly assigning them.'; - + $this->fields['username']->help_text = 'Required. 32 characters or less. Alphanumeric characters only (letters, digits and underscores).'; - - if (!$this->model->id) - { + + if (!$this->model->id) { unset($this->fields['password']); - + $this->fields['password1'] = new IPF_Form_Field_Varchar(array( 'label' => 'Password', 'required' => true, 'max_length' => 32, 'widget' => 'IPF_Form_Widget_PasswordInput' )); - + $this->fields['password2'] = new IPF_Form_Field_Varchar(array( 'label' => 'Password (again)', 'required' => true, @@ -36,13 +35,11 @@ class IPFAuthAdminUserForm extends IPF_Form_Extra_CheckGroup 'widget' => 'IPF_Form_Widget_PasswordInput', 'help_text' => 'Enter the same password as above, for verification.' )); - + $account = array('username', 'password1', 'password2'); - } - else - { + } else { $this->fields['password']->help_text = "Use '[algo]$[salt]$[hexdigest]' or use the change password form."; - + $account = array('username', 'password'); } @@ -50,14 +47,6 @@ class IPFAuthAdminUserForm extends IPF_Form_Extra_CheckGroup if (IPF_Auth_App::ArePermissionsEnabled()) { $permissions[] = 'Permissions'; $permissions[] = 'Roles'; - - $this->fields['Roles']->label = 'Groups'; - $this->fields['Roles']->help_text = 'In addition to the permissions manually assigned, this user will also get all permissions granted to each group he/she is in.'; - - parent::SetupForm($this); - } else { - unset($this->fields['Permissions']); - unset($this->fields['Roles']); } $this->field_groups = array( @@ -66,24 +55,79 @@ class IPFAuthAdminUserForm extends IPF_Form_Extra_CheckGroup array('fields' => $permissions, 'label' => 'Permissions'), ); } - + + public function add__Permissions__field() + { + if (!IPF_Auth_App::ArePermissionsEnabled()) + return; + + $choices = array(); + foreach (IPF_ORM::getTable('Permission')->findAll() as $o) + $choices[$o->__toString()] = $o->id; + ksort($choices); + + $field = new IPF_Form_Field_ModelMultipleChoice(array( + 'required' => false, + 'label' => 'Permissions', + 'help_text' => '', + 'type' => 'manytomany', + 'editable' => true, + 'model' => 'Permission', + 'widget' => 'IPF_Form_Widget_SelectMultipleInputCheckbox', + 'choices' => $choices, + 'widget_attrs' => array('class' => 'checkgroup'), + )); + + $this->fields['Permissions'] = $field; + } + + public function add__Roles__field() + { + if (!IPF_Auth_App::ArePermissionsEnabled()) + return; + + $choices = array(); + foreach (IPF_ORM::getTable('Role')->findAll() as $o) + $choices[$o->__toString()] = $o->id; + + $field = new IPF_Form_Field_ModelMultipleChoice(array( + 'required' => false, + 'label' => 'Groups', + 'help_text' => 'In addition to the permissions manually assigned, this user will also get all permissions granted to each group he/she is in.', + 'type' => 'manytomany', + 'editable' => true, + 'model' => 'Role', + 'widget' => 'IPF_Form_Widget_SelectMultipleInputCheckbox', + 'choices' => $choices, + 'widget_attrs' => array('class' => 'checkgroup'), + )); + + $this->fields['Roles'] = $field; + } + + public function extra_js() + { + $extra_js = parent::extra_js(); + if (IPF_Auth_App::ArePermissionsEnabled()) + $extra_js[] = ''; + return array_unique($extra_js); + } + function isValid() { $ok = parent::isValid(); - if ($ok===true && !$this->model->id) - { - if ($this->cleaned_data['password1'] != $this->cleaned_data['password2']) - { + if ($ok===true && !$this->model->id) { + if ($this->cleaned_data['password1'] != $this->cleaned_data['password2']) { $this->is_valid = false; $this->errors['password2'][] = "The two password fields didn't match."; - + return false; } - + $this->cleaned_data['password'] = User::SetPassword2($this->cleaned_data['password1']); } - + return $ok; } } @@ -104,7 +148,7 @@ class AdminUser extends IPF_Admin_Model public function fields() { - return array( + $fields = array( 'username', 'password', 'email', @@ -113,9 +157,12 @@ class AdminUser extends IPF_Admin_Model 'is_active', 'is_staff', 'is_superuser', - 'Permissions', - 'Roles', ); + if (IPF_Auth_App::ArePermissionsEnabled()) { + $fields[] = 'Permissions'; + $fields[] = 'Roles'; + } + return $fields; } function _searchFields() @@ -129,10 +176,6 @@ class AdminUser extends IPF_Admin_Model protected function _getForm($model_obj, $data, $extra) { $extra['model'] = $model_obj; - $extra['checkgroup_fields'] = array( - 'Permissions' => array('widget'=>'IPF_Auth_Forms_Widget_Permissions'), - 'Roles' => array(), - ); return new IPFAuthAdminUserForm($data, $extra); } } @@ -142,14 +185,16 @@ class User extends BaseUser const UNUSABLE_PASSWORD = '!'; public $session_key = 'IPF_User_auth'; - public function __toString() { + public function __toString() + { $s = $this->username; if ($s===null) return 'Anonymous'; return $s; } - public function smartName() { + public function smartName() + { $username = $this->username; if ($username===null) return __('Anonymous'); @@ -197,20 +242,24 @@ class User extends BaseUser return $user; } - function setUnusablePassword(){ + function setUnusablePassword() + { $this->password = UNUSABLE_PASSWORD; } - static function SetPassword2($raw_password){ + static function SetPassword2($raw_password) + { $salt = IPF_Utils::randomString(5); return 'sha1:'.$salt.':'.sha1($salt.$raw_password); } - function setPassword($raw_password){ + function setPassword($raw_password) + { $this->password = self::SetPassword2($raw_password); } - function checkPassword($password){ + function checkPassword($password) + { if ( ($this->password=='') || ($this->password==User::UNUSABLE_PASSWORD) ) return false; list($algo, $salt, $hash) = explode(':', $this->password); @@ -222,9 +271,7 @@ class User extends BaseUser function isAnonymous() { - if (0===(int)$this->id) - return true; - return false; + return 0 === (int)$this->id; } function checkCreditentials($username, $password) @@ -240,4 +287,5 @@ class User extends BaseUser } } -IPF_Admin_Model::register('User','AdminUser'); +IPF_Admin_Model::register('User', 'AdminUser'); + diff --git a/ipf/auth/models/UserPermission.php b/ipf/auth/models/UserPermission.php index 58111cc..7af37b9 100644 --- a/ipf/auth/models/UserPermission.php +++ b/ipf/auth/models/UserPermission.php @@ -1,9 +1,6 @@ add_js[] = ''; - - if (array_key_exists('add_js', $extra) && is_array($extra['add_js'])) - $this->add_js = array_merge($this->add_js, $extra['add_js']); - } - - public function render_commonjs() - { - return implode('', $this->add_js); - } -} \ No newline at end of file diff --git a/ipf/form/extra/checkgroup.php b/ipf/form/extra/checkgroup.php deleted file mode 100644 index e9b0d26..0000000 --- a/ipf/form/extra/checkgroup.php +++ /dev/null @@ -1,51 +0,0 @@ -checkgroup_fields = $extra['checkgroup_fields']; - } - - static function SetupForm($form, $checkgroup_fields=null) - { - if (!$checkgroup_fields) - $checkgroup_fields = &$form->checkgroup_fields; - - foreach ($checkgroup_fields as $fieldName=>&$params) - { - $field = $form->fields[$fieldName]; - - if (array_key_exists('label', $params)) - $field->label = $params['label']; - - $widget = null; - if (array_key_exists('widget', $params)) - $widget = $params['widget']; - if (!$widget) - $widget = 'IPF_Form_Extra_Widget_CheckboxGroupInput'; - - $field->widget = new $widget(array( - 'choices' => $field->widget->choices, - )); - - if (array_key_exists('class', $field->widget->attrs) && $field->widget->attrs['class']) - $field->widget->attrs['class'] .= ' checkgroup'; - else $field->widget->attrs['class'] = 'checkgroup'; - } - } - - public function render_commonjs() - { - if (count($this->checkgroup_fields)) - $result = ''; - else $result = ''; - - return implode('', $this->add_js).$result; - } -} diff --git a/ipf/form/extra/widget/checkboxgroupinput.php b/ipf/form/extra/widget/checkboxgroupinput.php deleted file mode 100644 index cca37f1..0000000 --- a/ipf/form/extra/widget/checkboxgroupinput.php +++ /dev/null @@ -1,30 +0,0 @@ -buildAttrs($extra_attrs); - $output[] = '
'; - return new IPF_Template_SafeString(implode("\n", $output), true); - } -} diff --git a/ipf/form/extra/widget/checkboxinput.php b/ipf/form/extra/widget/checkboxinput.php deleted file mode 100644 index 7ecd19f..0000000 --- a/ipf/form/extra/widget/checkboxinput.php +++ /dev/null @@ -1,18 +0,0 @@ -_model = $params['model']; } diff --git a/ipf/form/model.php b/ipf/form/model.php index 80e1ee6..cf3a32a 100644 --- a/ipf/form/model.php +++ b/ipf/form/model.php @@ -25,20 +25,20 @@ class IPF_Form_Model extends IPF_Form else $exclude = array(); - foreach($db_columns as $name=>$col) { - if (array_search($name,$exclude)!==false) + foreach ($db_columns as $name => $col) { + if (array_search($name, $exclude) !== false) continue; - $this->addDBField($name,$col); + $this->addDBField($name, $col); } - foreach($db_relations as $name => $relation) { - if (array_search($name,$exclude)!==false) + foreach ($db_relations as $name => $relation) { + if (array_search($name, $exclude) !== false) continue; - $this->addDBRelation($name,$relation,$col); + $this->addDBRelation($name, $relation, $col); } } else { - foreach($user_fields as $uname) { + foreach ($user_fields as $uname) { $add_method = 'add__'.$uname.'__field'; - if (method_exists($this,$add_method)) { + if (method_exists($this, $add_method)) { $this->$add_method(); continue; } diff --git a/ipf/form/widget/checkboxinput.php b/ipf/form/widget/checkboxinput.php index 4b55f2c..f147a76 100644 --- a/ipf/form/widget/checkboxinput.php +++ b/ipf/form/widget/checkboxinput.php @@ -6,18 +6,16 @@ class IPF_Form_Widget_CheckboxInput extends IPF_Form_Widget_Input public function render($name, $value, $extra_attrs=array()) { - if ((bool)$value) { + if ($value) $extra_attrs['checked'] = 'checked'; - } - $extra_attrs['value'] = '1'; + if (!array_key_exists('value', $extra_attrs)) + $extra_attrs['value'] = '1'; return parent::render($name, '', $extra_attrs); } public function valueFromFormData($name, &$data) { - if (!isset($data[$name]) or false === $data[$name] or (string)$data[$name] === '0' or (string)$data[$name] === 'off') { - return false; - } - return true; + return isset($data[$name]) && false !== $data[$name] && (string)$data[$name] !== '0' && (string)$data[$name] !== 'off'; } } + diff --git a/ipf/form/widget/selectmultipleinput.php b/ipf/form/widget/selectmultipleinput.php index 86b39b0..43133c2 100644 --- a/ipf/form/widget/selectmultipleinput.php +++ b/ipf/form/widget/selectmultipleinput.php @@ -6,10 +6,10 @@ class IPF_Form_Widget_SelectMultipleInput extends IPF_Form_Widget public function __construct($attrs=array()) { - if (isset($attrs['choices'])){ - $this->choices = $attrs['choices']; - unset($attrs['choices']); - } + if (isset($attrs['choices'])) { + $this->choices = $attrs['choices']; + unset($attrs['choices']); + } parent::__construct($attrs); } @@ -45,5 +45,5 @@ class IPF_Form_Widget_SelectMultipleInput extends IPF_Form_Widget } return null; } - } + diff --git a/ipf/form/widget/selectmultipleinputcheckbox.php b/ipf/form/widget/selectmultipleinputcheckbox.php index d2b18f6..3f4e614 100644 --- a/ipf/form/widget/selectmultipleinputcheckbox.php +++ b/ipf/form/widget/selectmultipleinputcheckbox.php @@ -2,26 +2,23 @@ class IPF_Form_Widget_SelectMultipleInputCheckbox extends IPF_Form_Widget_SelectMultipleInput { - public function render($name, $value, $extra_attrs=array(), - $choices=array()) + public function render($name, $value, $extra_attrs=array(), $choices=array()) { $output = array(); - if ($value === null or $value == '') { + if ($value === null || $value == '') $value = array(); - } $final_attrs = $this->buildAttrs($extra_attrs); $output[] = '