From 05da64c31b0a68d8356ac486bdf82b75116889d0 Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Sat, 11 Jan 2025 12:44:26 -0500 Subject: [PATCH 1/2] SAK-50853 Gradebook Webcomponents fix messaging to group --- .../business/GradebookNgBusinessService.java | 108 ++++-------------- .../rest/GradebookNgEntityProvider.java | 54 ++++----- .../src/SakaiSubmissionMessager.js | 12 +- 3 files changed, 52 insertions(+), 122 deletions(-) diff --git a/gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.java b/gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.java index c85fae094b9d..bfd010942672 100644 --- a/gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.java +++ b/gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.java @@ -234,14 +234,8 @@ public List getGradeableUsers(final GbGroup groupFilter) { * @return a list of users as uuids or null if none */ public List getGradeableUsers(final String siteId, final GbGroup groupFilter) { - + final String givenSiteId = StringUtils.defaultIfBlank(siteId, getCurrentSiteId()); try { - - String givenSiteId = siteId; - if (StringUtils.isBlank(givenSiteId)) { - givenSiteId = getCurrentSiteId(); - } - // note that this list MUST exclude TAs as it is checked in the // GradingService and will throw a SecurityException if invalid // users are provided @@ -1793,7 +1787,19 @@ public List sortGradeMatrix(Map * @return a list of sections and groups in the current site */ public List getSiteSectionsAndGroups() { - final String siteId = getCurrentSiteId(); + return getSiteSectionsAndGroups(null); + } + + /** + * Get a list of sections and groups in a site + * + * @param siteId the site id to get sections/groups for. If null, uses current site. + * @return a list of sections and groups in the site + */ + public List getSiteSectionsAndGroups(String siteId) { + if (siteId == null) { + siteId = getCurrentSiteId(); + } final List rval = new ArrayList<>(); @@ -1808,7 +1814,9 @@ public List getSiteSectionsAndGroups() { // get groups (handles both groups and sections) try { final Site site = this.siteService.getSite(siteId); - final Collection groups = isSuperUser() || role == GbRole.INSTRUCTOR ? site.getGroups() : site.getGroupsWithMember(userDirectoryService.getCurrentUser().getId()); + final Collection groups = isSuperUser() || role == GbRole.INSTRUCTOR ? + site.getGroups() : + site.getGroupsWithMember(userDirectoryService.getCurrentUser().getId()); for (final Group group : groups) { rval.add(new GbGroup(group.getId(), group.getTitle(), group.getReference(), GbGroup.Type.GROUP)); @@ -1819,9 +1827,7 @@ public List getSiteSectionsAndGroups() { log.error("Error retrieving groups", e); } - - // if user is a TA, get the groups they can see and filter the GbGroup - // list to keep just those + // if user is a TA, get the groups they can see and filter the GbGroup list if (role == GbRole.TA) { final Gradebook gradebook = this.getGradebook(siteId); final User user = getCurrentUser(); @@ -1834,7 +1840,6 @@ public List getSiteSectionsAndGroups() { } // get the ones the TA can actually view - // note that if a group is empty, it will not be included. List viewableGroupIds = this.gradingPermissionService .getViewableGroupsForUser(gradebook.getId(), user.getId(), allGroupIds); @@ -1843,9 +1848,9 @@ public List getSiteSectionsAndGroups() { } //FIXME: Another realms hack. The above method only returns groups from gb_permission_t. If this list is empty, - //need to check realms to see if user has privilege to grade any groups. This is already done in + //need to check realms to see if user has privilege to grade any groups. if (CollectionUtils.isEmpty(viewableGroupIds)) { - List realmsPerms = this.getPermissionsForUser(user.getId()); + List realmsPerms = this.getPermissionsForUser(user.getId(), siteId); if (CollectionUtils.isNotEmpty(realmsPerms)) { for (PermissionDefinition permDef : realmsPerms) { if (permDef.getGroupReference() != null) { @@ -1867,79 +1872,6 @@ public List getSiteSectionsAndGroups() { } } } - - } - - Collections.sort(rval); - - return rval; - } - - private List getSiteSectionsAndGroups(final String siteId) { - - final List rval = new ArrayList<>(); - - // get groups (handles both groups and sections) - try { - final Site site = this.siteService.getSite(siteId); - final Collection groups = site.getGroups(); - - for (final Group group : groups) { - rval.add(new GbGroup(group.getId(), group.getTitle(), group.getReference(), GbGroup.Type.GROUP)); - } - - } catch (final IdUnusedException e) { - // essentially ignore and use what we have - log.error("Error retrieving groups", e); - } - - GbRole role; - try { - role = this.getUserRole(siteId); - } catch (final GbAccessDeniedException e) { - log.warn("GbAccessDeniedException trying to getGradebookCategories", e); - return rval; - } - - // if user is a TA, get the groups they can see and filter the GbGroup - // list to keep just those - if (role == GbRole.TA) { - final Gradebook gradebook = this.getGradebook(siteId); - final User user = getCurrentUser(); - - // need list of all groups as REFERENCES (not ids) - final List allGroupIds = new ArrayList<>(); - for (final GbGroup group : rval) { - allGroupIds.add(group.getReference()); - } - - // get the ones the TA can actually view - // note that if a group is empty, it will not be included. - List viewableGroupIds = this.gradingPermissionService - .getViewableGroupsForUser(gradebook.getId(), user.getId(), allGroupIds); - - //FIXME: Another realms hack. The above method only returns groups from gb_permission_t. If this list is empty, - //need to check realms to see if user has privilege to grade any groups. This is already done in - if(CollectionUtils.isEmpty(viewableGroupIds)){ - List realmsPerms = this.getPermissionsForUser(user.getId(),siteId); - if(CollectionUtils.isNotEmpty(realmsPerms)){ - for(PermissionDefinition permDef : realmsPerms){ - if(permDef.getGroupReference()!=null){ - viewableGroupIds.add(permDef.getGroupReference()); - } - } - } - } - - // remove the ones that the user can't view - final Iterator iter = rval.iterator(); - while (iter.hasNext()) { - final GbGroup group = iter.next(); - if (!viewableGroupIds.contains(group.getReference())) { - iter.remove(); - } - } - } Collections.sort(rval); diff --git a/gradebookng/tool/src/java/org/sakaiproject/gradebookng/rest/GradebookNgEntityProvider.java b/gradebookng/tool/src/java/org/sakaiproject/gradebookng/rest/GradebookNgEntityProvider.java index 5e1997e2ad2a..30b7b48f5d48 100644 --- a/gradebookng/tool/src/java/org/sakaiproject/gradebookng/rest/GradebookNgEntityProvider.java +++ b/gradebookng/tool/src/java/org/sakaiproject/gradebookng/rest/GradebookNgEntityProvider.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -27,9 +28,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.math.NumberUtils; -import org.sakaiproject.authz.api.AuthzGroup; import org.sakaiproject.authz.api.AuthzGroupService; -import org.sakaiproject.authz.api.GroupNotDefinedException; import org.sakaiproject.authz.api.SecurityService; import org.sakaiproject.component.api.ServerConfigurationService; import org.sakaiproject.email.api.EmailService; @@ -50,14 +49,13 @@ import org.sakaiproject.gradebookng.business.GradebookNgBusinessService; import org.sakaiproject.gradebookng.business.exception.GbAccessDeniedException; import org.sakaiproject.gradebookng.business.model.GbGradeCell; -import org.sakaiproject.grading.api.GradingAuthz; +import org.sakaiproject.gradebookng.business.model.GbGroup; import org.sakaiproject.grading.api.GradingService; import org.sakaiproject.grading.api.GradeDefinition; import org.sakaiproject.user.api.User; import org.sakaiproject.user.api.UserDirectoryService; import org.sakaiproject.user.api.UserNotDefinedException; -import org.sakaiproject.site.api.Site; import org.sakaiproject.site.api.SiteService; import org.sakaiproject.tool.api.SessionManager; @@ -279,21 +277,25 @@ private Set getRecipients(Map params) { final Double minScore = (!StringUtils.isEmpty(minScoreString)) ? Double.valueOf(minScoreString) : null; final Double maxScore = (!StringUtils.isEmpty(maxScoreString)) ? Double.valueOf(maxScoreString) : null; - Set recipients = null; - try { - AuthzGroup authzGroup = authzGroupService.getAuthzGroup(groupRef); - Set totalRecipients = authzGroup.getUsers(); - Site site = siteService.getSite(siteId); - recipients = site.getUsersIsAllowed(GbRole.STUDENT.getValue()); - recipients.retainAll(totalRecipients); - } catch (GroupNotDefinedException gnde) { - throw new IllegalArgumentException("No group defined for " + groupRef); - } catch (IdUnusedException idune) { - log.warn("IdUnusedException trying to getRecipients", idune); - } + Set recipients = Set.of(); + + // Get the gradeable users for this one group + if (StringUtils.isNotBlank(groupRef)) { + String groupId = groupRef.substring(groupRef.lastIndexOf("/") + 1); + List groups = this.businessService.getSiteSectionsAndGroups(siteId); + for (GbGroup group : groups) { + if (group.getId().equals(groupId)) { + recipients = new HashSet<>(this.businessService.getGradeableUsers(siteId, group)); + break; + } + } + } + else { + recipients = new HashSet<>(this.businessService.getGradeableUsers(siteId)); + } - List grades - = gradingService.getGradesForStudentsForItem(siteId, assignmentId, new ArrayList(recipients)); + List grades + = gradingService.getGradesForStudentsForItem(siteId, assignmentId, new ArrayList<>(recipients)); if (MESSAGE_GRADED.equals(action)) { // We want to message graded students. Filter by min and max score, if needed. @@ -318,9 +320,13 @@ private Set getRecipients(Map params) { } recipients = grades.stream().filter(g -> g.getDateRecorded() != null) - .map(g -> g.getStudentUid()).collect(Collectors.toSet()); + .map(GradeDefinition::getStudentUid).collect(Collectors.toSet()); } else if (MESSAGE_UNGRADED.equals(action)) { - recipients.removeAll(grades.stream().filter(g -> g.getDateRecorded() != null).map(g -> g.getStudentUid()).collect(Collectors.toSet())); + List finalGrades = grades; + recipients = recipients.stream() + .filter(r -> finalGrades.stream() + .noneMatch(g -> g.getDateRecorded() != null && g.getStudentUid().equals(r))) + .collect(Collectors.toSet()); } return recipients; @@ -332,13 +338,7 @@ public ActionReturn listMessageRecipients(final EntityView view, final Map recipients = getRecipients(params); if (!recipients.isEmpty()) { - List users = recipients.stream().map(s -> { - try { - return userDirectoryService.getUser(s); - } catch (UserNotDefinedException unde) { - return null; - } - }).collect(Collectors.toList()); + List users = userDirectoryService.getUsers(recipients); if (users.contains(null)) { String errorMsg = "At least one of the students to message is null. No messsages sent."; diff --git a/webcomponents/tool/src/main/frontend/packages/sakai-submission-messager/src/SakaiSubmissionMessager.js b/webcomponents/tool/src/main/frontend/packages/sakai-submission-messager/src/SakaiSubmissionMessager.js index a3d72e9a5c46..0d35d500e9d1 100644 --- a/webcomponents/tool/src/main/frontend/packages/sakai-submission-messager/src/SakaiSubmissionMessager.js +++ b/webcomponents/tool/src/main/frontend/packages/sakai-submission-messager/src/SakaiSubmissionMessager.js @@ -29,7 +29,6 @@ export class SakaiSubmissionMessager extends SakaiElement { this.groups = []; this.recipientsToCheck = []; this._i18n = {}; - this.group = `/site/${portal.siteId}`; this.reset(); this.loadTranslations("submission-messager").then(t => this._i18n = t); } @@ -70,10 +69,10 @@ export class SakaiSubmissionMessager extends SakaiElement { ${this._i18n.select_group} + @groups-selected=${this.groupSelected}> @@ -116,14 +115,13 @@ export class SakaiSubmissionMessager extends SakaiElement { } groupSelected(e) { - this.recipientsToCheck = []; - this.group = e.detail.value; + this.groupId = e.detail.value[0]; } reset() { - this.groupId = "any"; + this.groupId = `/site/${portal.siteId}`; this.action = "1"; this.subject = ""; this.body = ""; @@ -139,7 +137,7 @@ export class SakaiSubmissionMessager extends SakaiElement { const formData = new FormData(); formData.set("action", this.action); - formData.set("groupRef", this.group || ""); + formData.set("groupRef", this.groupId || ""); formData.set("minScore", this.minScore || ""); formData.set("maxScore", this.maxScore || ""); formData.set("siteId", portal.siteId); From 3eeaba44079874fa1b611411f7bfcb21663620f8 Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Sat, 11 Jan 2025 12:56:38 -0500 Subject: [PATCH 2/2] fix for the whole site --- .../gradebookng/rest/GradebookNgEntityProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradebookng/tool/src/java/org/sakaiproject/gradebookng/rest/GradebookNgEntityProvider.java b/gradebookng/tool/src/java/org/sakaiproject/gradebookng/rest/GradebookNgEntityProvider.java index 30b7b48f5d48..b21aab5c3330 100644 --- a/gradebookng/tool/src/java/org/sakaiproject/gradebookng/rest/GradebookNgEntityProvider.java +++ b/gradebookng/tool/src/java/org/sakaiproject/gradebookng/rest/GradebookNgEntityProvider.java @@ -280,7 +280,7 @@ private Set getRecipients(Map params) { Set recipients = Set.of(); // Get the gradeable users for this one group - if (StringUtils.isNotBlank(groupRef)) { + if (StringUtils.contains(groupRef, "/group/")) { String groupId = groupRef.substring(groupRef.lastIndexOf("/") + 1); List groups = this.businessService.getSiteSectionsAndGroups(siteId); for (GbGroup group : groups) {