Skip to content

Commit

Permalink
SAK-27980 Removing a group will orphan the group's assignment submiss…
Browse files Browse the repository at this point in the history
  • Loading branch information
josecebe authored and ottenhoff committed Feb 5, 2017
1 parent 950d155 commit 5a3c2bc
Show file tree
Hide file tree
Showing 16 changed files with 360 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8175,7 +8175,7 @@ private void post_save_assignment(RunData data, String postOrSave)
String aOldAccessString = null;

// assignment old group setting
Collection aOldGroups = null;
Collection<String> aOldGroups = null;

// assignment old open date setting
Time oldOpenTime = null;
Expand Down Expand Up @@ -8347,7 +8347,7 @@ private void post_save_assignment(RunData data, String postOrSave)
// set group property
String range = (String) state.getAttribute(NEW_ASSIGNMENT_RANGE);

Collection groups = new ArrayList();
Collection<Group> groups = new ArrayList<Group>();
try
{
Site site = siteService.getSite(siteId);
Expand Down Expand Up @@ -8425,6 +8425,66 @@ else if (groupChoice != null)
commitAssignmentEdit(state, post, ac, a, title, visibleTime, openTime, dueTime, closeTime, enableCloseDate, section, range, groups, isGroupSubmit,
usePeerAssessment,peerPeriodTime, peerAssessmentAnonEval, peerAssessmentStudentViewReviews, peerAssessmentNumReviews, peerAssessmentInstructions);

// Locking and unlocking groups
List<String> lockedGroupsReferences = new ArrayList<String>();
if (isGroupSubmit && !groups.isEmpty()) {
for (Group group : groups) {
String groupAssignmentReference = group.getReference() + "/assignment/" + a.getId();

M_log.debug("Getting groups from reference: {}", groupAssignmentReference);
lockedGroupsReferences.add(group.getReference());
M_log.debug("Adding group: {}", group.getReference());

if (!aOldGroups.contains(group.getReference()) || !group.isLocked(groupAssignmentReference)) {
M_log.debug("locking group: {}", group.getReference());
group.lockGroup(groupAssignmentReference);
M_log.debug("locked group: {}", group.getReference());

try {
siteService.save(group.getContainingSite());
}
catch (IdUnusedException e)
{
M_log.warn(":doUpdate_options Cannot find site with id {}", siteId);
addAlert(state, rb.getFormattedMessage("options_cannotFindSite", new Object[]{siteId}));
}
catch (PermissionException e)
{
M_log.warn(":doUpdate_options Do not have permission to edit site with id {}", siteId);
addAlert(state, rb.getFormattedMessage("options_cannotEditSite", new Object[]{siteId}));
}
}
}
}

if (!aOldGroups.isEmpty()) {
try {
Site site = siteService.getSite(siteId);

for (String reference : aOldGroups) {
if (!lockedGroupsReferences.contains(reference)) {
M_log.debug("Not contains: {}", reference);
Group group = site.getGroup(reference);
if (group != null) {
String groupReferenceAssignment = group.getReference() + "/assignment/" + a.getId();
group.unlockGroup(groupReferenceAssignment);
siteService.save(group.getContainingSite());
}
}
}
}
catch (IdUnusedException e)
{
M_log.warn(":doUpdate_options Cannot find site with id {}", siteId);
addAlert(state, rb.getFormattedMessage("options_cannotFindSite", new Object[]{siteId}));
}
catch (PermissionException e)
{
M_log.warn(":doUpdate_options Do not have permission to edit site with id {}", siteId);
addAlert(state, rb.getFormattedMessage("options_cannotEditSite", new Object[]{siteId}));
}
}

if (post)
{
// we need to update the submission
Expand Down Expand Up @@ -10414,6 +10474,31 @@ public void doDelete_assignment(RunData data)
// we use to check "assignment.delete.cascade.submission" setting. But the implementation now is always remove submission objects when the assignment is removed.
// delete assignment and its submissions altogether
deleteAssignmentObjects(state, aEdit, true);

Collection<String> groups = aEdit.getGroups();
String siteId = (String) state.getAttribute(STATE_CONTEXT_STRING);

try {
Site site = siteService.getSite(siteId);

for (String reference : groups) {
Group group = site.getGroup(reference);
if (group != null) {
group.unlockGroup(group.getReference() + "/assignment/" + aEdit.getId());
siteService.save(group.getContainingSite());
}
}
}
catch (IdUnusedException e)
{
M_log.warn(this + ":doDelete_assignment Cannot find site with id {}", siteId);
addAlert(state, rb.getFormattedMessage("options_cannotFindSite", new Object[]{}));
}
catch (PermissionException e)
{
M_log.warn(this + ":doDelete_assignment Do not have permission to edit site with id {}", siteId);
addAlert(state, rb.getFormattedMessage("options_cannotEditSite", new Object[]{}));
}
}
} // for

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,11 @@ public interface CourseSection extends LearningContext {
* @return
*/
public List<Meeting> getMeetings();

/**
* Gets if the CourseSection is locked or not by some tools.
*
* @return True if it's locked by some tool, false otherwise
*/
public boolean isLocked();
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public class CourseSectionImpl implements CourseSection, Comparable<CourseSectio
protected List<Meeting> meetings;
protected String title;
protected String eid;
protected boolean isLocked;

protected boolean lazy_eid = false;

Expand Down Expand Up @@ -126,6 +127,7 @@ public CourseSectionImpl(Group group) {
this.course = new CourseImpl(group.getContainingSite());
this.title = group.getTitle();
this.description = group.getDescription();
this.isLocked = group.isLocked();

ResourceProperties props = group.getProperties();
this.category = props.getProperty(CourseSectionImpl.CATEGORY);
Expand Down Expand Up @@ -614,6 +616,10 @@ public void setMeetings(List<Meeting> meetings) {
this.meetings = meetings;
}

public boolean isLocked(){
return isLocked;
}

public Integer getMaxEnrollments() {
return maxEnrollments;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class CourseSectionImpl extends LearningContextImpl implements CourseSect
protected Integer maxEnrollments;

protected List meetings;
protected boolean isLocked;

/** Default constructor needed by hibernate */
public CourseSectionImpl() {}
Expand All @@ -55,6 +56,7 @@ public CourseSectionImpl(CourseSection section) {
this.meetings = section.getMeetings();
this.title = section.getTitle();
this.uuid = section.getUuid();
this.isLocked = section.isLocked();
}


Expand Down Expand Up @@ -127,6 +129,9 @@ public List getMeetings() {
public void setMeetings(List meetings) {
this.meetings = meetings;
}
public boolean isLocked() {
return isLocked;
}

/**
* Compares CourseSectionImpls based on their category ID and title. Sections
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.sakaiproject.authz.api.Member;
import org.sakaiproject.authz.api.Role;
import org.sakaiproject.authz.api.RoleAlreadyDefinedException;
import org.sakaiproject.entity.api.Entity;
import org.sakaiproject.entity.api.ResourceProperties;
import org.sakaiproject.entity.api.ResourcePropertiesEdit;
import org.sakaiproject.entitybroker.entityprovider.annotations.EntityFieldRequired;
Expand Down Expand Up @@ -518,4 +519,58 @@ public Site getContainingSite() {
throw new UnsupportedOperationException();
}

public void lockGroup(Entity entity) {
if (group != null) {
group.lockGroup(entity);
return;
}
throw new UnsupportedOperationException();
}

public void lockGroup(String lock) {
if (group != null) {
group.lockGroup(lock);
return;
}
throw new UnsupportedOperationException();
}

public void unlockGroup(Entity entity) {
if (group != null) {
group.unlockGroup(entity);
return;
}
throw new UnsupportedOperationException();
}

public void unlockGroup(String lock) {
if (group != null) {
group.unlockGroup(lock);
return;
}
throw new UnsupportedOperationException();
}

public void unlockGroup() {
if (group != null) {
group.unlockGroup();
return;
}
throw new UnsupportedOperationException();
}

public boolean isLocked(String lock) {
if (group != null) {
return group.isLocked(lock);
}
throw new UnsupportedOperationException();
}

public boolean isLocked() {
if (group != null) {
return group.isLocked();
}
throw new UnsupportedOperationException();
}

}
12 changes: 12 additions & 0 deletions kernel/api/src/main/java/org/sakaiproject/site/api/Group.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import org.sakaiproject.authz.api.AuthzGroup;
import org.sakaiproject.entity.api.Edit;
import org.sakaiproject.entity.api.Entity;

/**
* <p>
Expand All @@ -45,6 +46,9 @@ public interface Group extends Edit, Serializable, AuthzGroup
static final String GROUP_PROP_VIEW_MEMBERS = "group_prop_view_members";
/** The property to indicate whether the joinable group is unjoinable or not*/
static final String GROUP_PROP_JOINABLE_UNJOINABLE = "group_prop_joinable_unjoinable";

static final String GROUP_PROP_LOCKED_BY = "group_prop_locked_by";
static final String GROUP_PROP_SEPARATOR = "#:#";

/** @return a human readable short title of this group. */
String getTitle();
Expand Down Expand Up @@ -74,4 +78,12 @@ public interface Group extends Edit, Serializable, AuthzGroup
* The new description.
*/
void setDescription(String description);

void lockGroup(Entity entity);
void lockGroup(String lock);
void unlockGroup(Entity entity);
void unlockGroup(String lock);
void unlockGroup();
boolean isLocked(String lock);
boolean isLocked();
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,22 @@

package org.sakaiproject.site.impl;

import java.util.Arrays;
import java.util.Date;
import java.util.Set;
import java.util.Stack;
import java.util.Iterator;
import java.util.stream.Collectors;

import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sakaiproject.authz.api.AuthzGroup;
import org.sakaiproject.authz.api.GroupNotDefinedException;
import org.sakaiproject.authz.api.Member;
import org.sakaiproject.authz.api.Role;
import org.sakaiproject.authz.api.RoleAlreadyDefinedException;
import org.sakaiproject.entity.api.Entity;
import org.sakaiproject.entity.api.ResourceProperties;
import org.sakaiproject.entity.api.ResourcePropertiesEdit;
import org.sakaiproject.exception.IdUsedException;
Expand Down Expand Up @@ -412,6 +416,10 @@ protected AuthzGroup getAzg()

public void addMember(String userId, String roleId, boolean active, boolean provided)
{
if(this.isLocked()) {
M_log.error("Error, cannot add {} with role {} into a locked group", userId, roleId);
return;
}
m_azgChanged = true;
getAzg().addMember(userId, roleId, active, provided);
}
Expand Down Expand Up @@ -531,12 +539,20 @@ public boolean isEmpty()

public void removeMember(String userId)
{
if(this.isLocked()) {
M_log.error("Error, can not remove a member from a locked group");
return;
}
m_azgChanged = true;
getAzg().removeMember(userId);
}

public void removeMembers()
{
if(this.isLocked()) {
M_log.error("Error, can not remove members from a locked group");
return;
}
m_azgChanged = true;
getAzg().removeMembers();
}
Expand Down Expand Up @@ -571,4 +587,55 @@ public boolean keepIntersection(AuthzGroup other)
if (changed) m_azgChanged = true;
return changed;
}

public void lockGroup(Entity entity){
lockGroup(entity.getReference());
}

public void lockGroup(String lock){
if(StringUtils.isBlank(lock)) {
M_log.warn("lockGroup: null or empty lock");
return;
}
//TODO : this should be changed by addPropertyToList (When implemented in Kernel)
String prop = this.getProperties().getProperty(GROUP_PROP_LOCKED_BY);
if(StringUtils.isNotBlank(prop)) {
prop += GROUP_PROP_SEPARATOR + lock;
} else {
prop = lock;
}
this.getProperties().addProperty(GROUP_PROP_LOCKED_BY, prop);
}

public void unlockGroup(Entity entity){
unlockGroup(entity.getReference());
}

public void unlockGroup(String lock){
if(StringUtils.isBlank(lock)) {
M_log.warn("unlockGroup: null or empty lock");
return;
}
//TODO : this should be changed by addPropertyToList (When implemented in Kernel)
String prop = this.getProperties().getProperty(GROUP_PROP_LOCKED_BY);
if(StringUtils.isNotBlank(prop)) {
this.getProperties().addProperty(GROUP_PROP_LOCKED_BY, Arrays.stream(prop.split(GROUP_PROP_SEPARATOR)).filter(s -> !lock.equals(s)).collect(Collectors.joining(GROUP_PROP_SEPARATOR)));
}
}

public void unlockGroup(){
this.getProperties().removeProperty(GROUP_PROP_LOCKED_BY);
}

public boolean isLocked(){
return (StringUtils.isNotBlank(this.getProperties().getProperty(GROUP_PROP_LOCKED_BY)));
}

public boolean isLocked(String lock){
String prop = this.getProperties().getProperty(GROUP_PROP_LOCKED_BY);
if (StringUtils.contains(prop, lock)) {
return true;
}
return false;
}
}
Loading

0 comments on commit 5a3c2bc

Please sign in to comment.