Skip to content

plg_groups_members: order member roles and save canned deny replies #1928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
denphi wants to merge 2 commits into
base: 2.4-main
Choose a base branch
from
Open

plg_groups_members: order member roles and save canned deny replies #1928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
229c091
plg_groups_members: order member roles and save canned deny replies
denphi Sep 16, 2026
b98326f
plg_groups_members: harden deny-response saves and role reordering
nkissebe Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/bootstrap/Install/sql/mysql/schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5955,6 +5955,7 @@ CREATE TABLE `#__xgroups_roles` (
`id` int(11) NOT NULL AUTO_INCREMENT,
`gidNumber` int(11) DEFAULT NULL,
`name` varchar(150) DEFAULT NULL,
`ordering` int(11) NOT NULL DEFAULT 0,
`permissions` text DEFAULT NULL,
PRIMARY KEY (`id`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8mb3 COLLATE=utf8mb3_general_ci;
Expand Down
49 changes: 49 additions & 0 deletions core/components/com_groups/migrations/Migration20260915000000ComGroups.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php
/**
* @package hubzero-cms
* @copyright Copyright (c) 2005-2026 The Regents of the University of California.
* @license http://opensource.org/licenses/MIT MIT
*/

use Hubzero\Content\Migration\Base;

// No direct access
defined('_HZEXEC_') or die();

/**
* Migration script for manager-ordered group member roles
*
* Every existing role starts at 0, so a group that has never been reordered
* keeps listing its roles by name.
**/
class Migration20260915000000ComGroups extends Base
{
/**
* Up
**/
public function up()
{
if ($this->db->tableExists('#__xgroups_roles')
&& !$this->db->tableHasField('#__xgroups_roles', 'ordering'))
{
$query = "ALTER TABLE `#__xgroups_roles`
ADD COLUMN `ordering` int(11) NOT NULL DEFAULT 0 AFTER `name`";

@nkissebe nkissebe Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: low — the matching column is missing from core/bootstrap/Install/sql/mysql/schema.sql, which still declares #__xgroups_roles as (id, gidNumber, name, permissions) (around line 5954).

That file is the canonical fresh-install schema, and the convention in this tree is to change both: the sibling feature in the same area (45f83df, PR #1915) shipped Migration20260810000000ComGroups.php and a schema.sql update in the same commit. Leaving it out means the file no longer describes a current database, so anything that builds or diffs a schema straight from it (test fixtures, a restored baseline, the next regeneration) is missing ordering, and every query added by this PR — getRoles(), the per-member query in views/browse/tmpl/default.php, and plg_groups_messages — is a fatal Unknown column 'ordering' until the migration runs against it.

Adding `ordering` int(11) NOT NULL DEFAULT 0 after `name` in schema.sql keeps the two in step.

$this->db->setQuery($query);
$this->db->query();
}
}

/**
* Down
**/
public function down()
{
if ($this->db->tableExists('#__xgroups_roles')
&& $this->db->tableHasField('#__xgroups_roles', 'ordering'))
{
$query = "ALTER TABLE `#__xgroups_roles` DROP COLUMN `ordering`";
$this->db->setQuery($query);
$this->db->query();
}
}
}
13 changes: 13 additions & 0 deletions core/components/com_groups/models/role.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,19 @@ public function save()
$this->set('permissions', json_encode($this->get('permissions')));
}

// A new role goes to the end of its group's list rather than jumping
// ahead of roles a manager has already put in order
if ($this->isNew() && !$this->get('ordering'))
{
$last = self::blank()
->whereEquals('gidNumber', (int) $this->get('gidNumber'))
->order('ordering', 'desc')
->limit(1)
->row();

$this->set('ordering', (int) $last->get('ordering') + 1);

@nkissebe nkissebe Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: low — this breaks the "roles list by name until a manager sorts them" promise the migration docblock makes, the first time a role is added.

After the migration every role sits at ordering = 0, so getRoles()'s ORDER BY ordering ASC, name ASC lists a never-reordered group alphabetically. The moment a manager adds one more role, the max here is 0, the new role gets 1, and it lands below every existing role while the rest stay alphabetical — a group that never touched ordering now has one role stuck at the bottom, with no UI hint why. Concrete: a group with roles "Alumni", "Faculty", "Staff" (all at 0) adds "Advisors"; the sidebar shows Alumni, Faculty, Staff, Advisors.

The method comment says a new role should not jump ahead of "roles a manager has already put in order" — in a group where nothing has been ordered there is no arranged list to protect. Only appending when the group actually has an order satisfies both intents:

$last = self::blank()
	->whereEquals('gidNumber', (int) $this->get('gidNumber'))
	->order('ordering', 'desc')
	->limit(1)
	->row();

// A group whose roles have never been ordered keeps listing by name
$this->set('ordering', ((int) $last->get('ordering') > 0)
	? (int) $last->get('ordering') + 1
	: 0);

Flagging rather than asserting — if new-roles-always-last is the deliberate choice, then the migration docblock is the thing that needs rewording.

}

return parent::save();
}

Expand Down
49 changes: 49 additions & 0 deletions core/plugins/groups/members/assets/css/members.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,55 @@
.aside a.edit-role:after {
content: '\270E';
}
.aside .roles-order-hint {
color: #777;
font-size: 0.85em;
}
.aside .roles-sort {
margin: 0 0 0.5em;
}
.aside .roles-sort .btn {
font-size: 0.85em;
padding: 0.2em 0.6em;
}
.aside ul.roles.sortable li {
padding-left: 1.5em;
}
.aside ul.roles.sortable li.ui-sortable-helper {
background-color: #fff;
box-shadow: 0 1px 4px rgba(0, 0, 0, 0.2);
}
.aside .role-mover {
position: absolute;
top: 0;
bottom: 0;
left: 0;
width: 1.5em;
cursor: move;
color: #999;
text-align: center;
}
.aside .role-mover:after {
content: '\2630';
position: absolute;
top: 50%;
left: 0;
right: 0;
transform: translateY(-50%);
}
.aside .role-mover:hover {
color: #333;
}

/* Canned deny responses */
.deny-responses .deny-response {
border-bottom: 1px solid #ddd;
margin-bottom: 1em;
padding-bottom: 0.5em;
}
.deny-responses .deny-response-remove {
text-align: right;
}

/* Message AJAX */
#sbox-content .hub-mail {
Expand Down
87 changes: 87 additions & 0 deletions core/plugins/groups/members/assets/js/members.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,91 @@ jQuery(document).ready(function(jq){
}
});
//end assign role pop ups

// Managers drag member roles into the order the sidebar lists them
var roles = $('.aside ul.roles.sortable');
if (roles.length && jQuery.ui && jQuery.ui.sortable) {
roles.sortable({
handle: '.role-mover',
items: '> li',
axis: 'y',
cursor: 'move',
update: function (e, ui) {
var frm = $('form.roles-order');
var data = frm.serialize() + '&' + roles.sortable('serialize', { key: 'roles[]' });

$.post(frm.attr('action'), data, function (response) {
if (!response || !response.success) {
roles.sortable('cancel');
alert((response && response.message) || frm.attr('data-error'));
}
}, 'json').fail(function () {
roles.sortable('cancel');
alert(frm.attr('data-error'));
});
}
});
}

// Sorting replaces an order a manager may have arranged by hand
$('.roles-sort').on('submit', function (e) {
if (!confirm($(this).find('.sort-roles').attr('data-confirm'))) {
e.preventDefault();
}
});

// Fill the deny reply from a canned response
var denyReason = $('#reason');
var lastResponse = '';
$('#deny-response').on('change', function () {
var text = $(this).find('option:selected').attr('data-text');

if (typeof text === 'undefined') {
return;
}

// Don't silently throw away a reply the manager has written or edited
var current = $.trim(denyReason.val());
if (current !== '' && current !== $.trim(lastResponse) && !confirm($(this).attr('data-confirm'))) {
return;
}

denyReason.val(text);
lastResponse = text;
});

// Add and remove rows on the canned response form
$('.deny-responses').on('click', '.deny-response-add', function (e) {
e.preventDefault();

var list = $('.deny-response-list');
var next = parseInt(list.attr('data-next'), 10);
var row = list.find('.deny-response').last().clone();

row.find('input, textarea').each(function () {
var field = $(this);
field.val('');
field.attr('name', field.attr('name').replace(/\[\d+\]/, '[' + next + ']'));
field.attr('id', field.attr('id').replace(/-\d+-/, '-' + next + '-'));
});
row.find('label').each(function () {
$(this).attr('for', $(this).attr('for').replace(/-\d+-/, '-' + next + '-'));
});

list.append(row).attr('data-next', next + 1);
row.find('input').first().focus();
});

$('.deny-responses').on('click', '.deny-response-remove button', function (e) {
e.preventDefault();

var row = $(this).closest('.deny-response');

// Keep one row on the page to clone from; an emptied row isn't saved
if ($('.deny-response-list .deny-response').length > 1) {
row.remove();
} else {
row.find('input, textarea').val('');
}
});
});
22 changes: 22 additions & 0 deletions core/plugins/groups/members/language/en-GB/en-GB.plg_groups_members.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,21 @@ PLG_GROUPS_MEMBERS_DENY_EXPLANATION="If a user is denied membership by mistake,
PLG_GROUPS_MEMBERS_DENY_REASON="Customize reply to the user(s)"
PLG_GROUPS_MEMBERS_DENY_REASON_DEFAULT="Sorry, but we cannot grant you access to this group."
PLG_GROUPS_MEMBERS_DENY_USERS="User(s)"
PLG_GROUPS_MEMBERS_DENY_RESPONSES="Canned Responses"
PLG_GROUPS_MEMBERS_DENY_RESPONSES_EXPLANATION="Save replies you send often when denying membership. Choose one on the deny form to fill in the reply, then edit it before sending if needed. Clear a response's text to remove it."
PLG_GROUPS_MEMBERS_DENY_RESPONSES_MANAGE="Manage canned responses"
PLG_GROUPS_MEMBERS_DENY_RESPONSES_NONE="This group has no canned responses."
PLG_GROUPS_MEMBERS_DENY_RESPONSES_ADD="Add some"
PLG_GROUPS_MEMBERS_DENY_RESPONSES_SAVED="Canned responses saved."
PLG_GROUPS_MEMBERS_DENY_RESPONSES_NOT_SAVED="The canned responses could not be saved. Please try again."
PLG_GROUPS_MEMBERS_DENY_RESPONSES_TOO_LONG="These responses are too long to store. Please shorten or remove some of them and try again."
PLG_GROUPS_MEMBERS_DENY_RESPONSE_CHOOSE="Use a canned response"
PLG_GROUPS_MEMBERS_DENY_RESPONSE_NONE="&#8212; Select a response &#8212;"
PLG_GROUPS_MEMBERS_DENY_RESPONSE_REPLACE="Replace the reply you have written with this response?"
PLG_GROUPS_MEMBERS_DENY_RESPONSE_TITLE="Title"
PLG_GROUPS_MEMBERS_DENY_RESPONSE_TEXT="Response"
PLG_GROUPS_MEMBERS_DENY_RESPONSE_ADD="Add another response"
PLG_GROUPS_MEMBERS_DENY_RESPONSE_REMOVE="Remove"

; Cancel
PLG_GROUPS_MEMBERS_CANCEL_REASON="Customize reply to the user(s)"
Expand Down Expand Up @@ -168,6 +183,13 @@ PLG_GROUPS_MEMBERS_ROLE_SUCCESS="Group role successfully created or updated."

PLG_GROUPS_MEMBERS_ROLE_REMOVE="Remove Role"
PLG_GROUPS_MEMBERS_ROLE_EDIT="Edit Role"
PLG_GROUPS_MEMBERS_ROLE_ORDER_HINT="Drag roles by the handle to change their order."
PLG_GROUPS_MEMBERS_ROLE_ORDER_DRAG="Drag to reorder"
PLG_GROUPS_MEMBERS_ROLE_ORDER_ERROR="The new role order could not be saved. Please reload the page and try again."
PLG_GROUPS_MEMBERS_ROLE_ORDER_NOT_AUTHORIZED="Only group managers can change the order of roles."
PLG_GROUPS_MEMBERS_ROLE_SORT="Sort automatically"
PLG_GROUPS_MEMBERS_ROLE_SORT_CONFIRM="Sort every role: names first in alphabetical order, then years in date order. This replaces the current order."
PLG_GROUPS_MEMBERS_ROLE_SORT_DONE="Member roles sorted: names first, then years."

; Profile
COM_MEMBERS_FIELD_VALUE_NONE="(none)"
Expand Down
Loading
Loading