Skip to content
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

SAK-50853 Gradebook Webcomponents fix messaging to group #13183

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

SAK-50853 Gradebook Webcomponents fix messaging to group #13183

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
05da64c
SAK-50853 Gradebook Webcomponents fix messaging to group
ottenhoff Jan 11, 2025
3eeaba4
fix for the whole site
ottenhoff Jan 11, 2025
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
108 changes: 20 additions & 88 deletions ...ookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,14 +234,8 @@ public List<String> getGradeableUsers(final GbGroup groupFilter) {
* @return a list of users as uuids or null if none
*/
public List<String> 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
Expand Down Expand Up @@ -1793,7 +1787,19 @@ public List<GbStudentGradeInfo> sortGradeMatrix(Map<String, GbStudentGradeInfo>
* @return a list of sections and groups in the current site
*/
public List<GbGroup> 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<GbGroup> getSiteSectionsAndGroups(String siteId) {
if (siteId == null) {
siteId = getCurrentSiteId();
}

final List<GbGroup> rval = new ArrayList<>();

Expand All @@ -1808,7 +1814,9 @@ public List<GbGroup> getSiteSectionsAndGroups() {
// get groups (handles both groups and sections)
try {
final Site site = this.siteService.getSite(siteId);
final Collection<Group> groups = isSuperUser() || role == GbRole.INSTRUCTOR ? site.getGroups() : site.getGroupsWithMember(userDirectoryService.getCurrentUser().getId());
final Collection<Group> 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));
Expand All @@ -1819,9 +1827,7 @@ public List<GbGroup> 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();
Expand All @@ -1834,7 +1840,6 @@ public List<GbGroup> getSiteSectionsAndGroups() {
}

// get the ones the TA can actually view
// note that if a group is empty, it will not be included.
List<String> viewableGroupIds = this.gradingPermissionService
.getViewableGroupsForUser(gradebook.getId(), user.getId(), allGroupIds);

Expand All @@ -1843,9 +1848,9 @@ public List<GbGroup> 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<PermissionDefinition> realmsPerms = this.getPermissionsForUser(user.getId());
List<PermissionDefinition> realmsPerms = this.getPermissionsForUser(user.getId(), siteId);
if (CollectionUtils.isNotEmpty(realmsPerms)) {
for (PermissionDefinition permDef : realmsPerms) {
if (permDef.getGroupReference() != null) {
Expand All @@ -1867,79 +1872,6 @@ public List<GbGroup> getSiteSectionsAndGroups() {
}
}
}

}

Collections.sort(rval);

return rval;
}

private List<GbGroup> getSiteSectionsAndGroups(final String siteId) {

final List<GbGroup> rval = new ArrayList<>();

// get groups (handles both groups and sections)
try {
final Site site = this.siteService.getSite(siteId);
final Collection<Group> 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<String> 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<String> 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<PermissionDefinition> 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<GbGroup> iter = rval.iterator();
while (iter.hasNext()) {
final GbGroup group = iter.next();
if (!viewableGroupIds.contains(group.getReference())) {
iter.remove();
}
}

}

Collections.sort(rval);
Expand Down
54 changes: 27 additions & 27 deletions gradebookng/tool/src/java/org/sakaiproject/gradebookng/rest/GradebookNgEntityProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -279,21 +277,25 @@ private Set<String> getRecipients(Map<String, Object> params) {
final Double minScore = (!StringUtils.isEmpty(minScoreString)) ? Double.valueOf(minScoreString) : null;
final Double maxScore = (!StringUtils.isEmpty(maxScoreString)) ? Double.valueOf(maxScoreString) : null;

Set<String> recipients = null;
try {
AuthzGroup authzGroup = authzGroupService.getAuthzGroup(groupRef);
Set<String> 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<String> recipients = Set.of();

// Get the gradeable users for this one group
if (StringUtils.isNotBlank(groupRef)) {
String groupId = groupRef.substring(groupRef.lastIndexOf("/") + 1);
List<GbGroup> 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<GradeDefinition> grades
= gradingService.getGradesForStudentsForItem(siteId, assignmentId, new ArrayList<String>(recipients));
List<GradeDefinition> 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.
Expand All @@ -318,9 +320,13 @@ private Set<String> getRecipients(Map<String, Object> 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<GradeDefinition> finalGrades = grades;
recipients = recipients.stream()
.filter(r -> finalGrades.stream()
.noneMatch(g -> g.getDateRecorded() != null && g.getStudentUid().equals(r)))
.collect(Collectors.toSet());
}

return recipients;
Expand All @@ -332,13 +338,7 @@ public ActionReturn listMessageRecipients(final EntityView view, final Map<Strin
Set<String> recipients = getRecipients(params);

if (!recipients.isEmpty()) {
List<User> users = recipients.stream().map(s -> {
try {
return userDirectoryService.getUser(s);
} catch (UserNotDefinedException unde) {
return null;
}
}).collect(Collectors.toList());
List<User> users = userDirectoryService.getUsers(recipients);

if (users.contains(null)) {
String errorMsg = "At least one of the students to message is null. No messsages sent.";
Expand Down
12 changes: 5 additions & 7 deletions .../tool/src/main/frontend/packages/sakai-submission-messager/src/SakaiSubmissionMessager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -70,10 +69,10 @@ export class SakaiSubmissionMessager extends SakaiElement {
<span id="sm-group-selector-label-${this.assignmentId}" class="sm-label">${this._i18n.select_group}</span>
<sakai-group-picker
site-id="${portal.siteId}"
group-id="${ifDefined(this.groupId)}"
group-ref="${ifDefined(this.groupId)}"
aria-labelledby="sm-group-selector-label-${this.assignmentId}"
class="group-select"
@group-selected=${this.groupSelected}>
@groups-selected=${this.groupSelected}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adrianfish is this correct? note:

tool/src/main/frontend/packages/sakai-permissions/src/SakaiPermissions.js:122: @group-selected=${this._groupSelected}>

</sakai-group-picker>
</div>
<button type="button" class="btn btn-link" id="sm-show-recipients-button" @click=${this.listRecipients}>${this._i18n.show_recipients}</button>
Expand Down Expand Up @@ -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 = "";
Expand All @@ -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);
Expand Down
Loading