Permalink
Browse files

feature(groups): Only submitted group profile fields are updated

Group edit forms made by 3rd parties no longer need to submit data for all
group features.

BREAKING CHANGE:
Fields not submitted to the groups/edit action will no longer be acted
upon. Previously they would be set to empty or default values.
  • Loading branch information...
mrclay committed Jun 30, 2015
1 parent 3ed70fb commit c3d11285cb257fb36a4f79231fb8047d92101847
Showing with 76 additions and 51 deletions.
  1. +5 −1 engine/classes/ElggGroup.php
  2. +71 −50 mod/groups/actions/groups/edit.php
@@ -391,12 +391,16 @@ public function getContentAccessMode() {
/**
* Set the content access mode used by group_gatekeeper()
*
- * @param string $mode One of CONTENT_ACCESS_MODE_* constants
+ * @param string $mode One of CONTENT_ACCESS_MODE_* constants. If empty string, mode will not be changed.
* @return void
* @access private
* @since 1.9.0
*/
public function setContentAccessMode($mode) {
+ if (!$mode && $this->content_access_mode) {
+ return;
+ }
+
// only support two modes for now
if ($mode !== self::CONTENT_ACCESS_MODE_MEMBERS_ONLY) {
$mode = self::CONTENT_ACCESS_MODE_UNRESTRICTED;
@@ -2,26 +2,31 @@
/**
* Elgg groups plugin edit action.
*
+ * If editing an existing group, only the "group_guid" must be submitted. All other form
+ * elements may be omitted and the corresponding data will be left as is.
+ *
* @package ElggGroups
*/
elgg_make_sticky_form('groups');
-/**
- * wrapper for recursive array walk decoding
- */
-function profile_array_decoder(&$v) {
- $v = _elgg_html_decode($v);
-}
-
// Get group fields
$input = array();
foreach (elgg_get_config('group') as $shortname => $valuetype) {
- $input[$shortname] = get_input($shortname);
+ $value = get_input($shortname);
+
+ if ($value === null) {
+ // only submitted fields should be updated
+ continue;
+ }
+
+ $input[$shortname] = $value;
// @todo treat profile fields as unescaped: don't filter, encode on output
if (is_array($input[$shortname])) {
- array_walk_recursive($input[$shortname], 'profile_array_decoder');
+ array_walk_recursive($input[$shortname], function (&$v) {
+ $v = _elgg_html_decode($v);
+ });
} else {
$input[$shortname] = _elgg_html_decode($input[$shortname]);
}
@@ -31,7 +36,11 @@ function profile_array_decoder(&$v) {
}
}
-$input['name'] = htmlspecialchars(get_input('name', '', false), ENT_QUOTES, 'UTF-8');
+// only set if submitted
+$name = get_input('name', null, false);
+if ($name !== null) {
+ $input['name'] = htmlspecialchars($name, ENT_QUOTES, 'UTF-8');
+}
$user = elgg_get_logged_in_user_entity();
@@ -52,32 +61,30 @@ function profile_array_decoder(&$v) {
}
// Assume we can edit or this is a new group
-if (sizeof($input) > 0) {
- foreach($input as $shortname => $value) {
- // update access collection name if group name changes
- if (!$is_new_group && $shortname == 'name' && $value != $group->name) {
- $group_name = html_entity_decode($value, ENT_QUOTES, 'UTF-8');
- $ac_name = sanitize_string(elgg_echo('groups:group') . ": " . $group_name);
- $acl = get_access_collection($group->group_acl);
- if ($acl) {
- // @todo Elgg api does not support updating access collection name
- $db_prefix = elgg_get_config('dbprefix');
- $query = "UPDATE {$db_prefix}access_collections SET name = '$ac_name'
- WHERE id = $group->group_acl";
- update_data($query);
- }
- }
-
- if ($value === '') {
- // The group profile displays all profile fields that have a value.
- // We don't want to display fields with empty string value, so we
- // remove the metadata completely.
- $group->deleteMetadata($shortname);
- continue;
+foreach ($input as $shortname => $value) {
+ // update access collection name if group name changes
+ if (!$is_new_group && $shortname == 'name' && $value != $group->name) {
+ $group_name = html_entity_decode($value, ENT_QUOTES, 'UTF-8');
+ $ac_name = sanitize_string(elgg_echo('groups:group') . ": " . $group_name);
+ $acl = get_access_collection($group->group_acl);
+ if ($acl) {
+ // @todo Elgg api does not support updating access collection name
+ $db_prefix = elgg_get_config('dbprefix');
+ $query = "UPDATE {$db_prefix}access_collections SET name = '$ac_name'
+ WHERE id = $group->group_acl";
+ update_data($query);
}
+ }
- $group->$shortname = $value;
+ if ($value === '' && !in_array($shortname, ['name', 'description'])) {
+ // The group profile displays all profile fields that have a value.
+ // We don't want to display fields with empty string value, so we
+ // remove the metadata completely.
+ $group->deleteMetadata($shortname);
+ continue;
}
+
+ $group->$shortname = $value;
}
// Validate create
@@ -86,29 +93,40 @@ function profile_array_decoder(&$v) {
forward(REFERER);
}
-
// Set group tool options
$tool_options = elgg_get_config('group_tool_options');
if ($tool_options) {
foreach ($tool_options as $group_option) {
$option_toggle_name = $group_option->name . "_enable";
$option_default = $group_option->default_on ? 'yes' : 'no';
- $group->$option_toggle_name = get_input($option_toggle_name, $option_default);
+ $value = get_input($option_toggle_name);
+
+ // if already has option set, don't change if no submission
+ if ($group->$option_toggle_name && $value === null) {
+ continue;
+ }
+
+ $group->$option_toggle_name = $value ? $value : $option_default;
}
}
// Group membership - should these be treated with same constants as access permissions?
-$is_public_membership = (get_input('membership') == ACCESS_PUBLIC);
-$group->membership = $is_public_membership ? ACCESS_PUBLIC : ACCESS_PRIVATE;
+$value = get_input('membership');
+if ($group->membership === null || $value !== null) {
+ $is_public_membership = ($value == ACCESS_PUBLIC);
+ $group->membership = $is_public_membership ? ACCESS_PUBLIC : ACCESS_PRIVATE;
+}
-$group->setContentAccessMode(get_input('content_access_mode'));
+$group->setContentAccessMode((string)get_input('content_access_mode'));
if ($is_new_group) {
$group->access_id = ACCESS_PUBLIC;
}
$old_owner_guid = $is_new_group ? 0 : $group->owner_guid;
-$new_owner_guid = (int) get_input('owner_guid');
+
+$value = get_input('owner_guid');
+$new_owner_guid = ($value === null) ? $old_owner_guid : (int)$value;
$owner_has_changed = false;
$old_icontime = null;
@@ -158,19 +176,22 @@ function profile_array_decoder(&$v) {
// is an odd requirement and should be removed. Either the acl creation happens
// in the action or the visibility moves to a plugin hook
if (elgg_get_plugin_setting('hidden_groups', 'groups') == 'yes') {
- $visibility = (int)get_input('vis');
-
- if ($visibility == ACCESS_PRIVATE) {
- // Make this group visible only to group members. We need to use
- // ACCESS_PRIVATE on the form and convert it to group_acl here
- // because new groups do not have acl until they have been saved once.
- $visibility = $group->group_acl;
+ $value = get_input('vis');
+ if ($is_new_group || $value !== null) {
+ $visibility = (int)$value;
+
+ if ($visibility == ACCESS_PRIVATE) {
+ // Make this group visible only to group members. We need to use
+ // ACCESS_PRIVATE on the form and convert it to group_acl here
+ // because new groups do not have acl until they have been saved once.
+ $visibility = $group->group_acl;
+
+ // Force all new group content to be available only to members
+ $group->setContentAccessMode(ElggGroup::CONTENT_ACCESS_MODE_MEMBERS_ONLY);
+ }
- // Force all new group content to be available only to members
- $group->setContentAccessMode(ElggGroup::CONTENT_ACCESS_MODE_MEMBERS_ONLY);
+ $group->access_id = $visibility;
}
-
- $group->access_id = $visibility;
}
if (!$group->save()) {

0 comments on commit c3d1128

Please sign in to comment.