From 9babf40801880acad5c9a9705cb5aa9395d71bfb Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Sat, 16 May 2026 11:44:31 -0400 Subject: [PATCH 1/9] SAK-48981 Lessons properly configure forum and topic permission levels when using prerequisites https://sakaiproject.atlassian.net/browse/SAK-48981 --- .../service/ForumEntity.java | 346 ++---------------- .../MessageForumsForumManager.java | 9 +- .../ui/DiscussionForumManager.java | 22 ++ .../MessageForumsForumManagerImpl.java | 17 +- .../ui/DiscussionForumManagerImpl.java | 76 ++++ .../dao/hibernate/OpenForum.hbm.xml | 10 +- .../messageforums/dao/hibernate/Topic.hbm.xml | 2 + 7 files changed, 170 insertions(+), 312 deletions(-) diff --git a/lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/ForumEntity.java b/lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/ForumEntity.java index 3e77f821d46e..4934739365d1 100644 --- a/lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/ForumEntity.java +++ b/lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/ForumEntity.java @@ -23,12 +23,11 @@ package org.sakaiproject.lessonbuildertool.service; -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.Date; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -45,27 +44,23 @@ import org.sakaiproject.api.app.messageforums.MessageForumsForumManager; import org.sakaiproject.api.app.messageforums.MessageForumsMessageManager; import org.sakaiproject.api.app.messageforums.MessageForumsTypeManager; -import org.sakaiproject.api.app.messageforums.PermissionLevel; -import org.sakaiproject.api.app.messageforums.PermissionLevelManager; -import org.sakaiproject.api.app.messageforums.PermissionsMask; import org.sakaiproject.api.app.messageforums.Topic; import org.sakaiproject.api.app.messageforums.ui.DiscussionForumManager; import org.sakaiproject.api.app.messageforums.ui.UIPermissionsManager; import org.sakaiproject.authz.api.AuthzGroupService; import org.sakaiproject.api.app.messageforums.MembershipItem; +import org.sakaiproject.component.api.ServerConfigurationService; import org.sakaiproject.component.cover.ComponentManager; -import org.sakaiproject.component.cover.ServerConfigurationService; -import org.sakaiproject.db.cover.SqlService; -import org.sakaiproject.id.cover.IdManager; +import org.sakaiproject.db.api.SqlService; import org.sakaiproject.lessonbuildertool.SimplePageItem; import org.sakaiproject.lessonbuildertool.model.SimplePageToolDao; import org.sakaiproject.lessonbuildertool.tool.beans.SimplePageBean; import org.sakaiproject.lessonbuildertool.tool.beans.SimplePageBean.UrlItem; import org.sakaiproject.site.api.Group; import org.sakaiproject.site.api.Site; +import org.sakaiproject.site.api.SiteService; import org.sakaiproject.site.api.ToolConfiguration; -import org.sakaiproject.site.cover.SiteService; -import org.sakaiproject.tool.cover.ToolManager; +import org.sakaiproject.tool.api.ToolManager; import org.sakaiproject.util.api.FormattedText; import org.springframework.orm.hibernate5.HibernateTemplate; import org.springframework.orm.hibernate5.support.HibernateDaoSupport; @@ -109,8 +104,6 @@ public class ForumEntity extends HibernateDaoSupport implements LessonEntity, Fo ComponentManager.get("org.sakaiproject.api.app.messageforums.MessageForumsForumManager"); static MessageForumsMessageManager messageManager = (MessageForumsMessageManager) ComponentManager.get("org.sakaiproject.api.app.messageforums.MessageForumsMessageManager"); - static PermissionLevelManager permissionLevelManager = (PermissionLevelManager) - ComponentManager.get("org.sakaiproject.api.app.messageforums.PermissionLevelManager"); static UIPermissionsManager uiPermissionsManager = (UIPermissionsManager) ComponentManager.get("org.sakaiproject.api.app.messageforums.ui.UIPermissionsManager"); static DiscussionForumManager discussionForumManager = (DiscussionForumManager) @@ -119,6 +112,10 @@ public class ForumEntity extends HibernateDaoSupport implements LessonEntity, Fo ComponentManager.get("org.sakaiproject.api.app.messageforums.AreaManager"); static MessageForumsTypeManager typeManager = (MessageForumsTypeManager) ComponentManager.get("org.sakaiproject.api.app.messageforums.MessageForumsTypeManager"); + static ServerConfigurationService serverConfigurationService = ComponentManager.get(ServerConfigurationService.class); + static SiteService siteService = ComponentManager.get(SiteService.class); + static ToolManager toolManager = ComponentManager.get(ToolManager.class); + static SqlService sqlService = ComponentManager.get(SqlService.class); private LessonEntity nextEntity = null; private SimplePageBean simplePageBean; @@ -248,14 +245,9 @@ public List getEntitiesInSite(SimplePageBean bean) { // LSNBLDR-21. If the tool is not in the current site we shouldn't query // for topics owned by the tool. - Site site = null; - try { - site = SiteService.getSite(ToolManager.getCurrentPlacement().getContext()); - } catch (Exception impossible) { - return ret; - } - - ToolConfiguration tool = site.getToolForCommonId("sakai.forums"); + ToolConfiguration tool = siteService.getOptionalSite(toolManager.getCurrentPlacement().getContext()) + .map(site -> site.getToolForCommonId("sakai.forums")) + .orElse(null); if(tool == null) { @@ -387,14 +379,10 @@ public String getUrl() { return "javascript:alert('" + messageLocator.getMessage("simplepage.forumdeleted") + "')"; } - Site site = null; - try { - site = SiteService.getSite(ToolManager.getCurrentPlacement().getContext()); - } catch (Exception impossible) { - return null; - } - ToolConfiguration tool = site.getToolForCommonId("sakai.forums"); - + ToolConfiguration tool = siteService.getOptionalSite(toolManager.getCurrentPlacement().getContext()) + .map(site -> site.getToolForCommonId("sakai.forums")) + .orElse(null); + // LSNBLDR-21. If the tool is not in the current site we shouldn't return a url if(tool == null) { return null; @@ -415,132 +403,6 @@ public Date getDueDate() { return null; } - // The msgcntr permissions model is completely undocumented. Here's a cheat sheet: - - // DBMembershipItem has - // type, which is ALL, ROLE, GROUP or USER - // name which is the specific rolename, groupname or username - // permissionlevelname which is "Owner", "Contributor", "None", etc. - // In addition there is a bitmask that says which specific permissions - // are present, but we always use the default permissions for each level. - - // Here's typical code. First we create a permissionlevel with a given bitmask. - // This permissionlevel controls the detailed permissions. As noted, we always - // default levels. Unfortunately we have to create a new copy of the level - // for every entry. Some fo their internal code uses common permissionlevels, but - // if you don't have a separate level object and database entry for each membershipitem, - // things get very confused. - // PermissionLevel contributorLevel = permissionLevelManager. - // createPermissionLevel("Contributor", typeManager.getContributorLevelType(), contributorMask); - // permissionLevelManager.savePermissionLevel(contributorLevel); - - // Now we create the actual entry. Note that this one says members of the specified - // group are contributors. Then it sets the default contributor bitmask. You can call - // a permission contributor but set any bits you want. However we're going to assume - // that people pick a name that represents what they want, and make minimal changes. - // DBMembershipItem membershipItem = permissionLevelManager. - // createDBMembershipItem(groupName, "Contributor", MembershipItem.TYPE_GROUP); - // membershipItem.setPermissionLevel(contributorLevel); - // permissionLevelManager.saveDBMembershipItem(membershipItem); - - - // How we use it: - - // ACCESS CONTROL: - - // Our model is fairly simple. When we control access, Owner is the maintain role, and - // contributor is the group we control. Everything else is set to none. - // When you decontrol something, we make Owner the maintain role - // and Contributor all the other roles. Once we support groups, we'll put back - // saved group access, but we won't try to put back anything else. - // The tool code makes sure that items are added for all roles, so we don't have to add entries - // in most cases, just change their permission levels. - - // GROUP ACCESS WHEN WE AREN'T CONTROLLING: - - // Now, our group management code, which should only be used when we're not controlling: - // Getgroups returns which groups are contributor. - // If you want something more complex, you'll have to do it in the tool, but if there - // are any groups with contributor, we'll only let students in those groups access - // through our tool. - // Setgroups with a non-null list: we set all contributor entries to none, and then set the - // specified groups to contribtor. By only handling groups, we avoid interfering with - // anything you might do in the tool. But the moment you use access control, we take - // over. Sorry. Once we've done that you could go back into the tool and hack, but I - // don't recommend that. - // Setgroups with a null list: we set all contributor entries to none, and then set all roles - // other than maintain to contributor. - // It may be safer to do changes in the tool. - - - // the following methods all take references. So they're in effect static. - // They ignore the entity from which they're called. - // The reason for not making them a normal method is that many of the - // implementations seem to let you set access control and find submissions - // from a reference, without needing the actual object. So doing it this - // way could save some database activity - - PermissionsMask noneMask = null; - PermissionsMask contributorMask = null; - PermissionsMask ownerMask = null; - - private void setMasks() { - if (noneMask == null) { - noneMask = new PermissionsMask(); - noneMask.put(PermissionLevel.NEW_FORUM, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.NEW_TOPIC, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.NEW_RESPONSE, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.NEW_RESPONSE_TO_RESPONSE, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.MOVE_POSTING, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.CHANGE_SETTINGS,Boolean.valueOf(false)); - noneMask.put(PermissionLevel.POST_TO_GRADEBOOK, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.READ, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.MARK_AS_NOT_READ,Boolean.valueOf(false)); - noneMask.put(PermissionLevel.MODERATE_POSTINGS, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.IDENTIFY_ANON_AUTHORS, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.DELETE_OWN, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.DELETE_ANY, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.REVISE_OWN, Boolean.valueOf(false)); - noneMask.put(PermissionLevel.REVISE_ANY, Boolean.valueOf(false)); - } - if (contributorMask == null) { - contributorMask = new PermissionsMask(); - contributorMask.put(PermissionLevel.NEW_FORUM, Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.NEW_TOPIC, Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.NEW_RESPONSE, Boolean.valueOf(true)); - contributorMask.put(PermissionLevel.NEW_RESPONSE_TO_RESPONSE, Boolean.valueOf(true)); - contributorMask.put(PermissionLevel.MOVE_POSTING, Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.CHANGE_SETTINGS,Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.POST_TO_GRADEBOOK, Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.READ, Boolean.valueOf(true)); - contributorMask.put(PermissionLevel.MARK_AS_NOT_READ,Boolean.valueOf(true)); - contributorMask.put(PermissionLevel.MODERATE_POSTINGS, Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.IDENTIFY_ANON_AUTHORS, Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.DELETE_OWN, Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.DELETE_ANY, Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.REVISE_OWN, Boolean.valueOf(false)); - contributorMask.put(PermissionLevel.REVISE_ANY, Boolean.valueOf(false)); - } - if (ownerMask == null) { - ownerMask = new PermissionsMask(); - ownerMask.put(PermissionLevel.NEW_FORUM, Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.NEW_TOPIC, Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.NEW_RESPONSE, Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.NEW_RESPONSE_TO_RESPONSE, Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.MOVE_POSTING, Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.CHANGE_SETTINGS,Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.POST_TO_GRADEBOOK, Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.READ, Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.MARK_AS_NOT_READ,Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.MODERATE_POSTINGS, Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.IDENTIFY_ANON_AUTHORS, Boolean.valueOf(false)); - ownerMask.put(PermissionLevel.DELETE_OWN, Boolean.valueOf(false)); - ownerMask.put(PermissionLevel.DELETE_ANY, Boolean.valueOf(true)); - ownerMask.put(PermissionLevel.REVISE_OWN, Boolean.valueOf(false)); - ownerMask.put(PermissionLevel.REVISE_ANY, Boolean.valueOf(true)); - } - } - public LessonSubmission getSubmission(String user) { return null; // not used } @@ -557,7 +419,7 @@ public List createNewUrls(SimplePageBean bean) { ArrayList list = new ArrayList(); String tool = bean.getCurrentTool("sakai.forums"); if (tool != null) { - tool = ServerConfigurationService.getToolUrl() + "/" + tool + "/discussionForum/forumsOnly/dfForums"; + tool = serverConfigurationService.getToolUrl() + "/" + tool + "/discussionForum/forumsOnly/dfForums"; list.add(new UrlItem(tool, messageLocator.getMessage("simplepage.create_forums"))); } if (nextEntity != null) @@ -783,14 +645,9 @@ public List getGroups(boolean nocache) { } List ret = new ArrayList(); - Collection groups = null; - - try { - Site site = SiteService.getSite(ToolManager.getCurrentPlacement().getContext()); - groups = site.getGroups(); - } catch (Exception e) { - log.info("Unable to get site info for getGroups " + e); - } + Collection groups = siteService.getOptionalSite(toolManager.getCurrentPlacement().getContext()) + .map(Site::getGroups) + .orElse(Collections.emptyList()); // now change any existing ones into null for (DBMembershipItem item: oldMembershipItemSet) { @@ -812,157 +669,28 @@ public List getGroups(boolean nocache) { // set the item to be accessible only to the specific groups. // null to make it accessible to the whole site - public void setGroups(Collection groups) { - - // Setgroups with a non-null list: we set all contributor entries to none, and then set the - // specified groups to contribtor. By only handling groups, we avoid interfering with - // anything you might do in the tool. But the moment you use access control, we take - // over. Sorry. Once we've done that you could go back into the tool and hack, but I - // don't recommend that. - // Setgroups with a null list: we set all contributor entries to none, and then set all roles - // other than maintain to contributor. - - setMasks(); - Set oldMembershipItemSet = null; - if (type == TYPE_FORUM_TOPIC) { - topic = getTopicById(true, id); - if (topic == null) { - return; - } - uiPermissionsManager.clearMembershipsFromCacheForArea(topic.getBaseForum().getArea()); - oldMembershipItemSet = uiPermissionsManager.getTopicItemsSet((DiscussionTopic)topic); - } else if (type == TYPE_FORUM_FORUM) { - forum = getForumById(true, id); - if (forum == null) { - return; - } - uiPermissionsManager.clearMembershipsFromCacheForArea(forum.getArea()); - oldMembershipItemSet = uiPermissionsManager.getForumItemsSet((DiscussionForum)forum); - } else { + public void setGroups(Collection groups) { + if (type != TYPE_FORUM_TOPIC && type != TYPE_FORUM_FORUM) { return; } - - Site site = null; - try { - site = SiteService.getSite(ToolManager.getCurrentPlacement().getContext()); - } catch (Exception e) { - log.info("Unable to get site info for setGroups " + e); - return; - } - - DBMembershipItem membershipItem = null; - - boolean haveOwner = false; - boolean changed = false; - - if (groups != null && groups.size() > 0) { - - // this is the groups we've been asked to use - // remove groups form this as we see them if they already have access - // so at the end we just add the ones remaining - ListgroupNames = new ArrayList(); - SetaddGroupNames = new HashSet(); - for (String groupId: groups) { - groupNames.add(site.getGroup(groupId).getTitle()); - addGroupNames.add(site.getGroup(groupId).getTitle()); - } - - // delete groups from here as they are done. - - // if we've seen an owner. Otherwise set the maintain role as owner - - // Setgroups with a non-null list: we set all contributor entries to none, and then set the - // specified groups to contribtor. However we don't touch owner. - // By only handling groups, we avoid interfering with - // anything you might do in the tool. But the moment you use access control, we take - // over. Sorry. Once we've done that you could go back into the tool and hack, but I - // don't recommend that. - - for (DBMembershipItem item: oldMembershipItemSet) { - // kill everything except our own groups - // this will leave the owner but remove all other roles - if (item.getType().equals(MembershipItem.TYPE_GROUP) && groupNames.contains(item.getName())) { - addGroupNames.remove(item.getName()); // we've seen it - // if it's one of our groups make it a contributor if it's not already an owner - if (!item.getPermissionLevelName().equals("Contributor") && - !item.getPermissionLevelName().equals("Owner")) { - - PermissionLevel contributorLevel = permissionLevelManager. - createPermissionLevel("Contributor", IdManager.createUuid(), contributorMask); - permissionLevelManager.savePermissionLevel(contributorLevel); - - item.setPermissionLevel(contributorLevel); - item.setPermissionLevelName("Contributor"); - permissionLevelManager.saveDBMembershipItem(item); - } - } else if (!item.getPermissionLevelName().equals("Owner")) { // only group members are contributors - // remove contributor from anything else, both groups and roles - PermissionLevel noneLevel = permissionLevelManager. - createPermissionLevel("None", IdManager.createUuid(), noneMask); - permissionLevelManager.savePermissionLevel(noneLevel); - - item.setPermissionLevel(noneLevel); - item.setPermissionLevelName("None"); - permissionLevelManager.saveDBMembershipItem(item); - } - } - for (String newGroupName: addGroupNames) { - changed = true; - PermissionLevel contributorLevel = permissionLevelManager. - createPermissionLevel("Contributor", IdManager.createUuid(), contributorMask); - permissionLevelManager.savePermissionLevel(contributorLevel); - membershipItem = permissionLevelManager. - createDBMembershipItem(newGroupName, "Contributor", MembershipItem.TYPE_GROUP); - membershipItem.setPermissionLevel(contributorLevel); - membershipItem = permissionLevelManager.saveDBMembershipItem(membershipItem); - oldMembershipItemSet.add(membershipItem); - } - - } else { - // Setgroups with a null list: we set all contributor entries to none, and then set all roles - // to contributor. However we don't touch Owners. - - for (DBMembershipItem item: oldMembershipItemSet) { - if (item.getPermissionLevelName().equals("Owner")) { - haveOwner = true; - } else if (item.getType().equals(MembershipItem.TYPE_ROLE)) { - // default state has all roles except owner as contributor - if (!item.getPermissionLevelName().equals("Contributor")) { - PermissionLevel contributorLevel = permissionLevelManager. - createPermissionLevel("Contributor", IdManager.createUuid(), contributorMask); - permissionLevelManager.savePermissionLevel(contributorLevel); - - item.setPermissionLevel(contributorLevel); - item.setPermissionLevelName("Contributor"); - permissionLevelManager.saveDBMembershipItem(item); - } - } else if (!item.getPermissionLevelName().equals("None")) { - // kill other contributors - PermissionLevel noneLevel = permissionLevelManager. - createPermissionLevel("None", IdManager.createUuid(), noneMask); - permissionLevelManager.savePermissionLevel(noneLevel); - - item.setPermissionLevel(noneLevel); - item.setPermissionLevelName("None"); - permissionLevelManager.saveDBMembershipItem(item); - } - } - } - - if (changed) { + Collection groupNames = new ArrayList<>(); + if (groups != null && !groups.isEmpty()) { + Collection siteGroups = siteService.getOptionalSite(toolManager.getCurrentPlacement().getContext()) + .map(Site::getGroups) + .orElse(Collections.emptyList()); + siteGroups.stream(). + filter(g -> groups.contains(g.getId())) + .map(Group::getTitle) + .forEach(groupNames::add); + } if (type == TYPE_FORUM_TOPIC) { - topic.setMembershipItemSet(oldMembershipItemSet); - forumManager.saveDiscussionForumTopic((DiscussionTopic)topic); - } else if (type == TYPE_FORUM_FORUM) { - forum.setMembershipItemSet(oldMembershipItemSet); - forumManager.saveDiscussionForum((DiscussionForum)forum); + discussionForumManager.setTopicGroupRestrictions(id, groupNames); + } else { + discussionForumManager.setForumGroupRestrictions(id, groupNames); } } - - } - - // only used for topics + // only used for topics public String getObjectId(){ String title = getTitle(); // fetches topic as well @@ -1053,7 +781,7 @@ public String getSiteId() { Object fields[] = new Object[1]; fields[0] = id; - List siteIds = SqlService.dbRead(sql, fields, null); + List siteIds = sqlService.dbRead(sql, fields, null); if (siteIds != null && siteIds.size() > 0) return siteIds.get(0); diff --git a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/MessageForumsForumManager.java b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/MessageForumsForumManager.java index c3f06327dbb4..1e2aeb7a8d31 100644 --- a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/MessageForumsForumManager.java +++ b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/MessageForumsForumManager.java @@ -284,13 +284,20 @@ public interface MessageForumsForumManager { public List getForumByTypeAndContextWithTopicsAllAttachments(final String typeUuid, final String contextId); /** - * + * * @param topicId * @return the Topic with the given id with the DBMembershipItems initialized. * Does not initialize attachments or messages. */ public Topic getTopicByIdWithMemberships(final Long topicId); + /** + * @param forumId + * @return the BaseForum with the given id with the DBMembershipItems initialized. + * Does not initialize attachments or topics. + */ + public BaseForum getForumByIdWithMemberships(final Long forumId); + /** * @param contextId the context in which we are seeking topics * @return all topics within this context diff --git a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/ui/DiscussionForumManager.java b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/ui/DiscussionForumManager.java index 43b1e7b0f0fc..89ad27fba954 100644 --- a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/ui/DiscussionForumManager.java +++ b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/ui/DiscussionForumManager.java @@ -676,4 +676,26 @@ public void saveTopicMessagePermissions(DiscussionTopic topic, public Optional getStatementForGrade(String studentUid, String forumTitle, double score); void setUiPermissionsManager(UIPermissionsManager uiPermissionsManager); + + /** + * Restrict access to a topic to the named groups only. + * Contributor-level role and group items are set to None; items for the specified groups are set + * to Contributor. Owner items are never modified. Passing null or an empty collection restores + * role items to Contributor and group items to None. + * + * @param topicId the id of the DiscussionTopic + * @param groupNames site group titles (not IDs); null/empty means "open to site" + */ + void setTopicGroupRestrictions(Long topicId, Collection groupNames); + + /** + * Restrict access to a forum to the named groups only. + * Contributor-level role and group items are set to None; items for the specified groups are set + * to Contributor. Owner items are never modified. Passing null or an empty collection restores + * role items to Contributor and group items to None. + * + * @param forumId the id of the DiscussionForum + * @param groupNames site group titles (not IDs); null/empty means "open to site" + */ + void setForumGroupRestrictions(Long forumId, Collection groupNames); } diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsForumManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsForumManagerImpl.java index ce22a3461b17..676874bd4347 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsForumManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsForumManagerImpl.java @@ -1588,7 +1588,7 @@ public Topic getTopicByIdWithMemberships(final Long topicId) { if (topicId == null) { throw new IllegalArgumentException("Null Argument"); - } + } HibernateCallback hcb = session -> { Query q = session.getNamedQuery("findTopicByIdWithMemberships"); @@ -1599,6 +1599,21 @@ public Topic getTopicByIdWithMemberships(final Long topicId) { return getHibernateTemplate().execute(hcb); } + public BaseForum getForumByIdWithMemberships(final Long forumId) { + + if (forumId == null) { + throw new IllegalArgumentException("Null Argument"); + } + + HibernateCallback hcb = session -> { + Query q = session.getNamedQuery("findForumByIdWithMemberships"); + q.setParameter("id", forumId, LongType.INSTANCE); + return (BaseForum) q.uniqueResult(); + }; + + return getHibernateTemplate().execute(hcb); + } + public List getTopicsInSite(final String contextId) { return getTopicsInSite(contextId, false); diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java index 5ad4adfefcd8..c70592c2cefe 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java @@ -28,6 +28,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Predicate; @@ -2423,4 +2424,79 @@ public Optional getStatementForUserReadViewed(String subject, Str public Optional getStatementForGrade(String studentUid, String forumTitle, double score) { return LRSDelegate.getStatementForGrade(learningResourceStoreService, userDirectoryService, studentUid, forumTitle, score); } + + @Override + public void setTopicGroupRestrictions(Long topicId, Collection groupNames) { + Topic topic = forumManager.getTopicByIdWithMemberships(topicId); + if (topic == null) return; + topic.setBaseForum(topic.getOpenForum()); + if (topic.getBaseForum() == null || topic.getBaseForum().getArea() == null) return; + uiPermissionsManager.clearMembershipsFromCacheForArea(topic.getBaseForum().getArea()); + applyGroupRestrictions(topic.getMembershipItemSet(), groupNames); + forumManager.saveDiscussionForumTopic((DiscussionTopic) topic); + } + + @Override + public void setForumGroupRestrictions(Long forumId, Collection groupNames) { + BaseForum forum = forumManager.getForumByIdWithMemberships(forumId); + if (forum == null) return; + if (forum.getArea() == null) return; + uiPermissionsManager.clearMembershipsFromCacheForArea(forum.getArea()); + applyGroupRestrictions(forum.getMembershipItemSet(), groupNames); + forumManager.saveDiscussionForum((DiscussionForum) forum); + } + + /** + * Applies group restrictions to a set of membership items by adjusting permission levels based on specified group names. + * + * When group names are provided, this method restricts access by demoting Contributor items to None permission level, + * while promoting the specified groups to Contributor level. When group names are not provided or empty, the method + * clears restrictions by restoring role items with None permission to Contributor level and demoting Contributor + * group items to None level. Items with permission levels other than Contributor or None (such as Owner, Reviewer, + * or Author) are never modified. New membership items are created and added to the set for groups that don't + * already exist in the membership set. + * + * @param membershipItemSet the set of membership items to which restrictions will be applied + * @param groupNames the collection of group names to be granted Contributor access, or null/empty to clear restrictions + */ + private void applyGroupRestrictions(Set membershipItemSet, Collection groupNames) { + if (groupNames != null && !groupNames.isEmpty()) { + // Restricting: demote only Contributor items to None; promote specified groups to Contributor. + Set toAdd = new HashSet<>(groupNames); + for (DBMembershipItem item : membershipItemSet) { + if (Objects.equals(item.getType(), MembershipItem.TYPE_GROUP) && groupNames.contains(item.getName())) { + toAdd.remove(item.getName()); + if (PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE.equals(item.getPermissionLevelName())) { + item.setPermissionLevel(permissionLevelManager.getDefaultContributorPermissionLevel()); + item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR); + } + } else if (PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR.equals(item.getPermissionLevelName())) { + item.setPermissionLevel(permissionLevelManager.getDefaultNonePermissionLevel()); + item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE); + } + } + for (String newGroupName : toAdd) { + DBMembershipItem newItem = permissionLevelManager.createDBMembershipItem( + newGroupName, + PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR, + MembershipItem.TYPE_GROUP); + newItem.setPermissionLevel(permissionLevelManager.getDefaultContributorPermissionLevel()); + newItem = permissionLevelManager.saveDBMembershipItem(newItem); + membershipItemSet.add(newItem); + } + } else { + // Clearing: restore only None role items to Contributor; set only Contributor group items to None. + for (DBMembershipItem item : membershipItemSet) { + if (Objects.equals(item.getType(), MembershipItem.TYPE_ROLE)) { + if (PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE.equals(item.getPermissionLevelName())) { + item.setPermissionLevel(permissionLevelManager.getDefaultContributorPermissionLevel()); + item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR); + } + } else if (PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR.equals(item.getPermissionLevelName())) { + item.setPermissionLevel(permissionLevelManager.getDefaultNonePermissionLevel()); + item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE); + } + } + } + } } diff --git a/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/OpenForum.hbm.xml b/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/OpenForum.hbm.xml index ddcc990a6525..b3c8b0e5b6fe 100644 --- a/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/OpenForum.hbm.xml +++ b/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/OpenForum.hbm.xml @@ -176,7 +176,15 @@ where f.id = :id ]]> - + + + + + From a79660fb28ebb8fdf0144ddacae5b9e3afa3eafd Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Mon, 18 May 2026 14:08:19 -0400 Subject: [PATCH 2/9] some improvements --- .../service/ForumEntity.java | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/ForumEntity.java b/lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/ForumEntity.java index 4934739365d1..f33af1ed3312 100644 --- a/lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/ForumEntity.java +++ b/lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/ForumEntity.java @@ -28,6 +28,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.Date; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -667,26 +668,12 @@ public List getGroups(boolean nocache) { return ret; } - // set the item to be accessible only to the specific groups. - // null to make it accessible to the whole site public void setGroups(Collection groups) { - if (type != TYPE_FORUM_TOPIC && type != TYPE_FORUM_FORUM) { - return; - } - Collection groupNames = new ArrayList<>(); - if (groups != null && !groups.isEmpty()) { - Collection siteGroups = siteService.getOptionalSite(toolManager.getCurrentPlacement().getContext()) - .map(Site::getGroups) - .orElse(Collections.emptyList()); - siteGroups.stream(). - filter(g -> groups.contains(g.getId())) - .map(Group::getTitle) - .forEach(groupNames::add); - } + Set groupIds = (groups == null) ? Collections.emptySet() : new HashSet<>(groups); if (type == TYPE_FORUM_TOPIC) { - discussionForumManager.setTopicGroupRestrictions(id, groupNames); - } else { - discussionForumManager.setForumGroupRestrictions(id, groupNames); + discussionForumManager.setTopicGroupRestrictions(id, groupIds); + } else if (type == TYPE_FORUM_FORUM) { + discussionForumManager.setForumGroupRestrictions(id, groupIds); } } From 3f0e6fee92175e385835364e04fd36bfd7938bdc Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Mon, 18 May 2026 14:09:01 -0400 Subject: [PATCH 3/9] forgot these improvements --- .../ui/DiscussionForumManager.java | 22 ++++--- .../ui/DiscussionForumManagerImpl.java | 62 ++++++++++++++----- 2 files changed, 58 insertions(+), 26 deletions(-) diff --git a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/ui/DiscussionForumManager.java b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/ui/DiscussionForumManager.java index 89ad27fba954..bc2d300ac06f 100644 --- a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/ui/DiscussionForumManager.java +++ b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/ui/DiscussionForumManager.java @@ -680,22 +680,24 @@ public void saveTopicMessagePermissions(DiscussionTopic topic, /** * Restrict access to a topic to the named groups only. * Contributor-level role and group items are set to None; items for the specified groups are set - * to Contributor. Owner items are never modified. Passing null or an empty collection restores - * role items to Contributor and group items to None. + * to Contributor. Owner items are never modified. Passing null or an empty set are treated + * identically and restore role items to Contributor and group items to None (open to site). + * If the site or any group ID cannot be resolved, the operation is aborted and no changes are saved. * - * @param topicId the id of the DiscussionTopic - * @param groupNames site group titles (not IDs); null/empty means "open to site" + * @param topicId the id of the DiscussionTopic + * @param groupIds site group IDs; null or empty means "open to site" */ - void setTopicGroupRestrictions(Long topicId, Collection groupNames); + void setTopicGroupRestrictions(Long topicId, Set groupIds); /** * Restrict access to a forum to the named groups only. * Contributor-level role and group items are set to None; items for the specified groups are set - * to Contributor. Owner items are never modified. Passing null or an empty collection restores - * role items to Contributor and group items to None. + * to Contributor. Owner items are never modified. Passing null or an empty set are treated + * identically and restore role items to Contributor and group items to None (open to site). + * If the site or any group ID cannot be resolved, the operation is aborted and no changes are saved. * - * @param forumId the id of the DiscussionForum - * @param groupNames site group titles (not IDs); null/empty means "open to site" + * @param forumId the id of the DiscussionForum + * @param groupIds site group IDs; null or empty means "open to site" */ - void setForumGroupRestrictions(Long forumId, Collection groupNames); + void setForumGroupRestrictions(Long forumId, Set groupIds); } diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java index c70592c2cefe..35ab0b5ec8d2 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java @@ -92,7 +92,6 @@ import org.sakaiproject.site.api.Group; import org.sakaiproject.site.api.Site; import org.sakaiproject.site.api.SiteService; -import org.sakaiproject.tasks.api.Task; import org.sakaiproject.tasks.api.TaskService; import org.sakaiproject.tool.api.SessionManager; import org.sakaiproject.tool.api.Tool; @@ -2425,25 +2424,56 @@ public Optional getStatementForGrade(String studentUid, String fo return LRSDelegate.getStatementForGrade(learningResourceStoreService, userDirectoryService, studentUid, forumTitle, score); } - @Override - public void setTopicGroupRestrictions(Long topicId, Collection groupNames) { - Topic topic = forumManager.getTopicByIdWithMemberships(topicId); - if (topic == null) return; - topic.setBaseForum(topic.getOpenForum()); - if (topic.getBaseForum() == null || topic.getBaseForum().getArea() == null) return; - uiPermissionsManager.clearMembershipsFromCacheForArea(topic.getBaseForum().getArea()); - applyGroupRestrictions(topic.getMembershipItemSet(), groupNames); - forumManager.saveDiscussionForumTopic((DiscussionTopic) topic); - } + @Override + public void setTopicGroupRestrictions(Long topicId, Set groupIds) { + Topic topic = forumManager.getTopicByIdWithMemberships(topicId); + if (topic == null) return; + topic.setBaseForum(topic.getOpenForum()); + if (topic.getBaseForum() == null || topic.getBaseForum().getArea() == null) return; + uiPermissionsManager.clearMembershipsFromCacheForArea(topic.getBaseForum().getArea()); + String siteId = topic.getBaseForum().getArea().getContextId(); + List groupNames = resolveSiteGroupNames(groupIds, siteId); + if (groupNames != null) { + applyGroupRestrictions(topic.getMembershipItemSet(), groupNames); + forumManager.saveDiscussionForumTopic((DiscussionTopic) topic); + return; + } + log.warn("Failed to resolve group names for topic with ID: {}", topicId); + } - @Override - public void setForumGroupRestrictions(Long forumId, Collection groupNames) { + @Override + public void setForumGroupRestrictions(Long forumId, Set groupIds) { BaseForum forum = forumManager.getForumByIdWithMemberships(forumId); if (forum == null) return; if (forum.getArea() == null) return; uiPermissionsManager.clearMembershipsFromCacheForArea(forum.getArea()); - applyGroupRestrictions(forum.getMembershipItemSet(), groupNames); - forumManager.saveDiscussionForum((DiscussionForum) forum); + String siteId = forum.getArea().getContextId(); + List groupNames = resolveSiteGroupNames(groupIds, siteId); + if (groupNames != null) { + applyGroupRestrictions(forum.getMembershipItemSet(), groupNames); + forumManager.saveDiscussionForum((DiscussionForum) forum); + return; + } + log.warn("Failed to resolve group names for forum with ID: {}", forumId); + } + + private List resolveSiteGroupNames(Set groupIds, String siteId) { + if (groupIds == null || groupIds.isEmpty()) return Collections.emptyList(); + Site site = siteService.getOptionalSite(siteId).orElse(null); + if (site != null) { + List groupNames = new ArrayList<>(); + for (String groupId : groupIds) { + Group group = site.getGroup(groupId); + if (group == null) { + log.debug("Group with ID {} not found in site {}", groupId, siteId); + return null; + } + groupNames.add(group.getTitle()); + } + return groupNames; + } + log.debug("Site with ID {} not found", siteId); + return null; } /** @@ -2459,7 +2489,7 @@ public void setForumGroupRestrictions(Long forumId, Collection groupName * @param membershipItemSet the set of membership items to which restrictions will be applied * @param groupNames the collection of group names to be granted Contributor access, or null/empty to clear restrictions */ - private void applyGroupRestrictions(Set membershipItemSet, Collection groupNames) { + private void applyGroupRestrictions(Set membershipItemSet, List groupNames) { if (groupNames != null && !groupNames.isEmpty()) { // Restricting: demote only Contributor items to None; promote specified groups to Contributor. Set toAdd = new HashSet<>(groupNames); From e37e98b30bce8f9a86c24b0882ab88010163825c Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Wed, 20 May 2026 00:10:06 -0400 Subject: [PATCH 4/9] use default permission level --- .../app/messageforums/DiscussionForumServiceImpl.java | 2 +- .../app/messageforums/ui/DiscussionForumManagerImpl.java | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/DiscussionForumServiceImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/DiscussionForumServiceImpl.java index 2c43ccd7fc47..e92d8dc49ecb 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/DiscussionForumServiceImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/DiscussionForumServiceImpl.java @@ -1478,7 +1478,7 @@ private List getSiteRolesAndGroups(String contextId) { } private DBMembershipItem getMembershipItemCopy(DBMembershipItem itemToCopy) { - DBMembershipItem newItem = permissionManager.createDBMembershipItem(itemToCopy.getName(), itemToCopy.getPermissionLevelName(), + DBMembershipItem newItem = permissionManager.createDBMembershipItem(itemToCopy.getName(), itemToCopy.getPermissionLevelName(), itemToCopy.getType()); PermissionLevel oldPermLevel = itemToCopy.getPermissionLevel(); if (newItem.getPermissionLevelName().equals(PermissionLevelManager.PERMISSION_LEVEL_NAME_CUSTOM)) { diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java index 35ab0b5ec8d2..5ed41237abc6 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java @@ -2497,11 +2497,11 @@ private void applyGroupRestrictions(Set membershipItemSet, Lis if (Objects.equals(item.getType(), MembershipItem.TYPE_GROUP) && groupNames.contains(item.getName())) { toAdd.remove(item.getName()); if (PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE.equals(item.getPermissionLevelName())) { - item.setPermissionLevel(permissionLevelManager.getDefaultContributorPermissionLevel()); + item.setPermissionLevel(null); item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR); } } else if (PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR.equals(item.getPermissionLevelName())) { - item.setPermissionLevel(permissionLevelManager.getDefaultNonePermissionLevel()); + item.setPermissionLevel(null); item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE); } } @@ -2510,7 +2510,6 @@ private void applyGroupRestrictions(Set membershipItemSet, Lis newGroupName, PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR, MembershipItem.TYPE_GROUP); - newItem.setPermissionLevel(permissionLevelManager.getDefaultContributorPermissionLevel()); newItem = permissionLevelManager.saveDBMembershipItem(newItem); membershipItemSet.add(newItem); } @@ -2519,11 +2518,11 @@ private void applyGroupRestrictions(Set membershipItemSet, Lis for (DBMembershipItem item : membershipItemSet) { if (Objects.equals(item.getType(), MembershipItem.TYPE_ROLE)) { if (PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE.equals(item.getPermissionLevelName())) { - item.setPermissionLevel(permissionLevelManager.getDefaultContributorPermissionLevel()); + item.setPermissionLevel(null); item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR); } } else if (PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR.equals(item.getPermissionLevelName())) { - item.setPermissionLevel(permissionLevelManager.getDefaultNonePermissionLevel()); + item.setPermissionLevel(null); item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE); } } From c23e6bc533aad590dbcfec28f66770e75fd19ecd Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Thu, 21 May 2026 15:56:25 -0400 Subject: [PATCH 5/9] address permission level issues in db --- .../config/bundle/default.sakai.properties | 5 + .../api/app/messageforums/Area.java | 116 ++-- .../messageforums/DiscussionForumTool.java | 55 +- .../tool/messageforums/ui/PermissionBean.java | 155 +++--- .../app/messageforums/AreaManagerImpl.java | 206 +++----- .../ui/DiscussionForumManagerImpl.java | 86 ++- .../ui/PrivateMessageManagerImpl.java | 3 - .../ui/UIPermissionsManagerImpl.java | 54 +- .../src/webapp/WEB-INF/components.xml | 2 + .../messageforums/dao/hibernate/AreaImpl.java | 496 +++++------------- 10 files changed, 485 insertions(+), 693 deletions(-) diff --git a/config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties b/config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties index 7aa72b3c895e..661647a4fa45 100644 --- a/config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties +++ b/config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties @@ -3963,6 +3963,11 @@ # DEFAULT: false # mc.disableLongDesc=true +# Messages tool option for sending a copy of a message to recipient email addresses. +# Options are 0 (Never), 1 (Optional), and 2 (Always) +# DEFAULT: 1 +# msgcntr.defaultSendToEmailSetting=2 + # Include a copy of the message to the recipient by default. Works in conjunction with user preference. # DEFAULT: false # mc.messages.ccEmailDefault=true diff --git a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/Area.java b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/Area.java index 250600f727ee..6c55a9718782 100644 --- a/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/Area.java +++ b/msgcntr/messageforums-api/src/java/org/sakaiproject/api/app/messageforums/Area.java @@ -30,47 +30,39 @@ public interface Area extends MutableEntity { * setting for {@link #sendToEmail}. A copy of message is never sent * to recipients' email addresses */ - public static final int EMAIL_COPY_NEVER = 0; + int EMAIL_COPY_NEVER = 0; /** * setting for {@link #sendToEmail}. Sender is given the option of sending * a copy of message to email addresses */ - public static final int EMAIL_COPY_OPTIONAL = 1; + int EMAIL_COPY_OPTIONAL = 1; /** * setting for {@link #sendToEmail}. A copy of message is always sent * to recipients' email addresses */ - public static final int EMAIL_COPY_ALWAYS = 2; + int EMAIL_COPY_ALWAYS = 2; - public void setVersion(Integer version); + void setVersion(Integer version); - public String getContextId(); + String getContextId(); - public void setContextId(String contextId); + void setContextId(String contextId); - public Boolean getHidden(); + Boolean getHidden(); - public void setHidden(Boolean hidden); + void setHidden(Boolean hidden); - public String getName(); + String getName(); - public void setName(String name); + void setName(String name); - public Boolean getEnabled(); + Boolean getEnabled(); - public void setEnabled(Boolean enabled); + void setEnabled(Boolean enabled); - /** - * {@link Deprecated} This option was replaced by sendToEmail via MSGCNTR-708. DO NOT USE. - * @return - */ - public Boolean getSendEmailOut(); + Boolean getSendEmailOut(); - /** - * {@link Deprecated} This option was replaced by sendToEmail via MSGCNTR-708. DO NOT USE. - * @param sendEmailOut - */ - public void setSendEmailOut(Boolean sendEmailOut); + void setSendEmailOut(Boolean sendEmailOut); /** * @@ -78,7 +70,7 @@ public interface Area extends MutableEntity { * email addresses. This may be {@link #EMAIL_COPY_NEVER}, #{@link #EMAIL_COPY_OPTIONAL}, * {@link #EMAIL_COPY_ALWAYS} */ - public int getSendToEmail(); + int getSendToEmail(); /** * set the site-level setting for sending a copy of the message to recipients' @@ -86,81 +78,81 @@ public interface Area extends MutableEntity { * {@link #EMAIL_COPY_ALWAYS} * @param sendToEmail */ - public void setSendToEmail(int sendToEmail); + void setSendToEmail(int sendToEmail); - public List getOpenForums(); + List getOpenForums(); - public Set getOpenForumsSet(); + Set getOpenForumsSet(); - public void setOpenForums(List openForums); + void setOpenForums(List openForums); - public List getPrivateForums(); + List getPrivateForums(); - public Set getPrivateForumsSet(); + Set getPrivateForumsSet(); - public void setPrivateForums(List discussionForums); + void setPrivateForums(List discussionForums); - public List getDiscussionForums(); + List getDiscussionForums(); - public void setDiscussionForums(List discussionForums); + void setDiscussionForums(List discussionForums); - public String getTypeUuid(); + String getTypeUuid(); - public void setTypeUuid(String typeUuid); + void setTypeUuid(String typeUuid); - public void addPrivateForum(BaseForum forum); + void addPrivateForum(PrivateForum forum); - public void removePrivateForum(BaseForum forum); + void removePrivateForum(PrivateForum forum); - public void addDiscussionForum(BaseForum forum); + void addDiscussionForum(DiscussionForum forum); - public void removeDiscussionForum(BaseForum forum); + void removeDiscussionForum(DiscussionForum forum); - public void addOpenForum(BaseForum forum); + void addOpenForum(OpenForum forum); - public void removeOpenForum(BaseForum forum); + void removeOpenForum(OpenForum forum); - public Boolean getLocked(); + Boolean getLocked(); - public void setLocked(Boolean locked); + void setLocked(Boolean locked); - public Boolean getModerated(); + Boolean getModerated(); - public void setModerated(Boolean moderated); + void setModerated(Boolean moderated); - public Boolean getAutoMarkThreadsRead(); + Boolean getAutoMarkThreadsRead(); - public void setAutoMarkThreadsRead(Boolean autoMarkThreadsRead); + void setAutoMarkThreadsRead(Boolean autoMarkThreadsRead); - public Set getMembershipItemSet(); + Set getMembershipItemSet(); - public void setMembershipItemSet(Set membershipItemSet); + void setMembershipItemSet(Set membershipItemSet); - public void addMembershipItem(DBMembershipItem item); + void addMembershipItem(DBMembershipItem item); - public void removeMembershipItem(DBMembershipItem item); + void removeMembershipItem(DBMembershipItem item); - public Boolean getAvailabilityRestricted(); + Boolean getAvailabilityRestricted(); - public void setAvailabilityRestricted(Boolean restricted); + void setAvailabilityRestricted(Boolean restricted); - public Date getOpenDate(); + Date getOpenDate(); - public void setOpenDate(Date openDate); + void setOpenDate(Date openDate); - public Date getCloseDate(); + Date getCloseDate(); - public void setCloseDate(Date closeDate); + void setCloseDate(Date closeDate); - public Boolean getAvailability(); + Boolean getAvailability(); - public void setAvailability(Boolean restricted); + void setAvailability(Boolean restricted); - public Boolean getPostFirst(); + Boolean getPostFirst(); - public void setPostFirst(Boolean postFirst); + void setPostFirst(Boolean postFirst); - public Set getHiddenGroups(); + Set getHiddenGroups(); - public void setHiddenGroups(Set hiddenGroups); + void setHiddenGroups(Set hiddenGroups); } diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java index ef687b646784..c17fd9cc0932 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java @@ -1201,7 +1201,8 @@ public String processActionForumSettings() topicClickCount = 0; setEditMode(true); setPermissionMode(PERMISSION_MODE_FORUM); - + permissions = null; + String forumId = getExternalParameterByKey(FORUM_ID); if (StringUtils.isBlank(forumId) || "null".equals(forumId)) { @@ -1696,7 +1697,12 @@ else if (target instanceof Topic){ { DBMembershipItem oldItem = (DBMembershipItem) iter2.next(); if(permBean.getItem().getId().equals(oldItem.getId())){ - if(permBean.getModeratePostings() != oldItem.getPermissionLevel().getModeratePostings()){ + PermissionLevel oldLevel = oldItem.getPermissionLevel(); + if (oldLevel == null) { + oldLevel = permissionLevelManager.getPermissionLevelByName(oldItem.getPermissionLevelName()); + } + Boolean oldModerate = (oldLevel != null) ? oldLevel.getModeratePostings() : Boolean.FALSE; + if(permBean.getModeratePostings() != Boolean.TRUE.equals(oldModerate)){ update = true; break; } @@ -1793,7 +1799,8 @@ public String processActionReviseTopicSettings() forumClickCount = 0; setPermissionMode(PERMISSION_MODE_TOPIC); setEditMode(true); - + permissions = null; + if(selectedTopic == null) { log.debug("no topic is selected in processActionReviseTopicSettings."); @@ -7263,8 +7270,12 @@ public void setObjectPermissions(Object target){ membershipItemSet.forEach(i -> ((DBMembershipItemImpl) i).setTopic(t)); } permissionLevelManager.deleteMembershipItems(oldMembershipItemSet); + if (area != null) { + uiPermissionsManager.clearMembershipsFromCacheForArea(area); + } } siteMembers = null; + permissions = null; } /** @@ -7274,24 +7285,26 @@ public void setObjectPermissions(Object target){ */ private void setupMembershipItemPermission(DBMembershipItem membershipItem, PermissionBean permBean) { - PermissionsMask mask = new PermissionsMask(); - mask.put(PermissionLevel.NEW_FORUM, Boolean.valueOf(permBean.getNewForum())); - mask.put(PermissionLevel.NEW_TOPIC, Boolean.valueOf(permBean.getNewTopic())); - mask.put(PermissionLevel.NEW_RESPONSE, Boolean.valueOf(permBean.getNewResponse())); - mask.put(PermissionLevel.NEW_RESPONSE_TO_RESPONSE, Boolean.valueOf(permBean.getResponseToResponse())); - mask.put(PermissionLevel.MOVE_POSTING, Boolean.valueOf(permBean.getMovePosting())); - mask.put(PermissionLevel.CHANGE_SETTINGS,Boolean.valueOf(permBean.getChangeSettings())); - mask.put(PermissionLevel.POST_TO_GRADEBOOK, Boolean.valueOf(permBean.getPostToGradebook())); - mask.put(PermissionLevel.READ, Boolean.valueOf(permBean.getRead())); - mask.put(PermissionLevel.MARK_AS_NOT_READ,Boolean.valueOf(permBean.getMarkAsNotRead())); - mask.put(PermissionLevel.MODERATE_POSTINGS, Boolean.valueOf(permBean.getModeratePostings())); - mask.put(PermissionLevel.IDENTIFY_ANON_AUTHORS, Boolean.valueOf(permBean.getIdentifyAnonAuthors())); - mask.put(PermissionLevel.DELETE_OWN, Boolean.valueOf(permBean.getDeleteOwn())); - mask.put(PermissionLevel.DELETE_ANY, Boolean.valueOf(permBean.getDeleteAny())); - mask.put(PermissionLevel.REVISE_OWN, Boolean.valueOf(permBean.getReviseOwn())); - mask.put(PermissionLevel.REVISE_ANY, Boolean.valueOf(permBean.getReviseAny())); - - PermissionLevel level = permissionLevelManager.createPermissionLevel(permBean.getSelectedLevel(), typeManager.getCustomLevelType(), mask); + if (!PermissionLevelManager.PERMISSION_LEVEL_NAME_CUSTOM.equals(permBean.getSelectedLevel())) { + return; + } + PermissionsMask mask = new PermissionsMask(); + mask.put(PermissionLevel.NEW_FORUM, permBean.getNewForum()); + mask.put(PermissionLevel.NEW_TOPIC, permBean.getNewTopic()); + mask.put(PermissionLevel.NEW_RESPONSE, permBean.getNewResponse()); + mask.put(PermissionLevel.NEW_RESPONSE_TO_RESPONSE, permBean.getResponseToResponse()); + mask.put(PermissionLevel.MOVE_POSTING, permBean.getMovePosting()); + mask.put(PermissionLevel.CHANGE_SETTINGS, permBean.getChangeSettings()); + mask.put(PermissionLevel.POST_TO_GRADEBOOK, permBean.getPostToGradebook()); + mask.put(PermissionLevel.READ, permBean.getRead()); + mask.put(PermissionLevel.MARK_AS_NOT_READ, permBean.getMarkAsNotRead()); + mask.put(PermissionLevel.MODERATE_POSTINGS, permBean.getModeratePostings()); + mask.put(PermissionLevel.IDENTIFY_ANON_AUTHORS, permBean.getIdentifyAnonAuthors()); + mask.put(PermissionLevel.DELETE_OWN, permBean.getDeleteOwn()); + mask.put(PermissionLevel.DELETE_ANY, permBean.getDeleteAny()); + mask.put(PermissionLevel.REVISE_OWN, permBean.getReviseOwn()); + mask.put(PermissionLevel.REVISE_ANY, permBean.getReviseAny()); + PermissionLevel level = permissionLevelManager.createPermissionLevel(PermissionLevelManager.PERMISSION_LEVEL_NAME_CUSTOM, typeManager.getCustomLevelType(), mask); membershipItem.setPermissionLevel(level); } diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java index 56e2b9e396b2..160d6e10aed8 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java @@ -41,14 +41,17 @@ public class PermissionBean { private String selectedLevel; private DBMembershipItem item; - private PermissionLevelManager permissionLevelManager; + private PermissionLevelManager permissionLevelManager; + private PermissionLevel displayLevel; public PermissionBean(DBMembershipItem item, PermissionLevelManager permissionLevelManager) { this.permissionLevelManager = permissionLevelManager; this.item = item; - selectedLevel= item.getPermissionLevel().getName(); + this.selectedLevel = item.getPermissionLevelName(); + PermissionLevel level = item.getPermissionLevel(); + this.displayLevel = (level != null) ? level : permissionLevelManager.getPermissionLevelByName(selectedLevel); } /** @@ -72,58 +75,56 @@ public void setSelectedLevel(String selectedLevel) private void setPermissionsForLevel(String selectedLevel) { if (selectedLevel != null) - { - if (!"Custom".equals(selectedLevel)) - { - PermissionLevel permLevel= permissionLevelManager.getPermissionLevelByName(selectedLevel); - this.item.setPermissionLevel(permLevel); - } - else - { - MessageForumsTypeManager typeManager = (MessageForumsTypeManager) ComponentManager.get("org.sakaiproject.api.app.messageforums.MessageForumsTypeManager"); - if(!this.item.getPermissionLevel().getTypeUuid().equals(typeManager.getCustomLevelType())) - { - PermissionLevel permLevel = permissionLevelManager.createPermissionLevel(selectedLevel, typeManager.getCustomLevelType(), new PermissionsMask()); - this.item.setPermissionLevel(permLevel); - } - } + { + if (!"Custom".equals(selectedLevel)) + { + this.displayLevel = permissionLevelManager.getPermissionLevelByName(selectedLevel); + } + else + { + MessageForumsTypeManager typeManager = (MessageForumsTypeManager) ComponentManager.get("org.sakaiproject.api.app.messageforums.MessageForumsTypeManager"); + if (this.displayLevel == null || !this.displayLevel.getTypeUuid().equals(typeManager.getCustomLevelType())) + { + this.displayLevel = permissionLevelManager.createPermissionLevel(selectedLevel, typeManager.getCustomLevelType(), new PermissionsMask()); + } + } } } public boolean getChangeSettings() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getChangeSettings() != null) - return item.getPermissionLevel().getChangeSettings().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getChangeSettings() != null) + return displayLevel.getChangeSettings().booleanValue(); else return false; } public void setChangeSettings(boolean changeSettings) { - this.item.getPermissionLevel().setChangeSettings( + this.displayLevel.setChangeSettings( Boolean.valueOf(changeSettings)); } public boolean getDeleteAny() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getDeleteAny() != null) - return item.getPermissionLevel().getDeleteAny().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getDeleteAny() != null) + return displayLevel.getDeleteAny().booleanValue(); else return false; } public void setDeleteAny(boolean deleteAny) { - this.item.getPermissionLevel().setDeleteAny(Boolean.valueOf(deleteAny)); + this.displayLevel.setDeleteAny(Boolean.valueOf(deleteAny)); } public boolean getDeleteOwn() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getDeleteOwn() != null) - return item.getPermissionLevel().getDeleteOwn().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getDeleteOwn() != null) + return displayLevel.getDeleteOwn().booleanValue(); else return false; @@ -131,176 +132,176 @@ public boolean getDeleteOwn() public void setDeleteOwn(boolean deleteOwn) { - this.item.getPermissionLevel().setDeleteOwn(Boolean.valueOf(deleteOwn)); + this.displayLevel.setDeleteOwn(Boolean.valueOf(deleteOwn)); } public boolean getMarkAsNotRead() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getMarkAsNotRead() != null) - return item.getPermissionLevel().getMarkAsNotRead().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getMarkAsNotRead() != null) + return displayLevel.getMarkAsNotRead().booleanValue(); else return false; } public void setMarkAsNotRead(boolean markAsNotRead) { - this.item.getPermissionLevel().setMarkAsNotRead(Boolean.valueOf(markAsNotRead)); + this.displayLevel.setMarkAsNotRead(Boolean.valueOf(markAsNotRead)); } public boolean getModeratePostings() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getModeratePostings() != null) - return item.getPermissionLevel().getModeratePostings().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getModeratePostings() != null) + return displayLevel.getModeratePostings().booleanValue(); else return false; } public void setModeratePostings(boolean moderatePostings) { - this.item.getPermissionLevel().setModeratePostings( + this.displayLevel.setModeratePostings( Boolean.valueOf(moderatePostings)); } public boolean getIdentifyAnonAuthors() { - return item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getIdentifyAnonAuthors() != null - && item.getPermissionLevel().getIdentifyAnonAuthors(); + return item != null && displayLevel != null + && displayLevel.getIdentifyAnonAuthors() != null + && displayLevel.getIdentifyAnonAuthors(); } public void setIdentifyAnonAuthors(boolean identifyAnonAuthors) { - this.item.getPermissionLevel().setIdentifyAnonAuthors( + this.displayLevel.setIdentifyAnonAuthors( Boolean.valueOf(identifyAnonAuthors)); } public boolean getMovePosting() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getMovePosting() != null) - return item.getPermissionLevel().getMovePosting().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getMovePosting() != null) + return displayLevel.getMovePosting().booleanValue(); else return false; } public void setMovePosting(boolean movePosting) { - this.item.getPermissionLevel().setMovePosting(Boolean.valueOf(movePosting)); + this.displayLevel.setMovePosting(Boolean.valueOf(movePosting)); } public boolean getNewForum() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getNewForum() != null) - return item.getPermissionLevel().getNewForum().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getNewForum() != null) + return displayLevel.getNewForum().booleanValue(); else return false; } public void setNewForum(boolean newForum) { - this.item.getPermissionLevel().setNewForum(Boolean.valueOf(newForum)); + this.displayLevel.setNewForum(Boolean.valueOf(newForum)); } public boolean getNewResponse() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getNewResponse() != null) - return item.getPermissionLevel().getNewResponse().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getNewResponse() != null) + return displayLevel.getNewResponse().booleanValue(); else return false; } public void setNewResponse(boolean newResponse) { - this.item.getPermissionLevel().setNewResponse(Boolean.valueOf(newResponse)); + this.displayLevel.setNewResponse(Boolean.valueOf(newResponse)); } public boolean getNewTopic() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getNewTopic() != null) - return item.getPermissionLevel().getNewTopic().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getNewTopic() != null) + return displayLevel.getNewTopic().booleanValue(); else return false; } public void setNewTopic(boolean newTopic) { - this.item.getPermissionLevel().setNewTopic(Boolean.valueOf(newTopic)); + this.displayLevel.setNewTopic(Boolean.valueOf(newTopic)); } public boolean getPostToGradebook() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getPostToGradebook() != null) - return item.getPermissionLevel().getPostToGradebook() .booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getPostToGradebook() != null) + return displayLevel.getPostToGradebook() .booleanValue(); else return false; } public void setPostToGradebook(boolean postGrades) { - this.item.getPermissionLevel().setPostToGradebook(Boolean.valueOf(postGrades)); + this.displayLevel.setPostToGradebook(Boolean.valueOf(postGrades)); } public boolean getRead() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getRead() != null) - return item.getPermissionLevel().getRead().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getRead() != null) + return displayLevel.getRead().booleanValue(); else return false; } public void setRead(boolean read) { - this.item.getPermissionLevel().setRead(Boolean.valueOf(read)); + this.displayLevel.setRead(Boolean.valueOf(read)); } public boolean getResponseToResponse() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getNewResponseToResponse() != null) - return item.getPermissionLevel().getNewResponseToResponse().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getNewResponseToResponse() != null) + return displayLevel.getNewResponseToResponse().booleanValue(); else return false; } public void setResponseToResponse(boolean responseToResponse) { - this.item.getPermissionLevel().setNewResponseToResponse( + this.displayLevel.setNewResponseToResponse( Boolean.valueOf(responseToResponse)); } public boolean getReviseAny() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getReviseAny() != null) - return item.getPermissionLevel().getReviseAny().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getReviseAny() != null) + return displayLevel.getReviseAny().booleanValue(); else return false; } public void setReviseAny(boolean reviseAny) { - this.item.getPermissionLevel().setReviseAny(Boolean.valueOf(reviseAny)); + this.displayLevel.setReviseAny(Boolean.valueOf(reviseAny)); } public boolean getReviseOwn() { - if (item != null && item.getPermissionLevel() != null - && item.getPermissionLevel().getReviseOwn() != null) - return item.getPermissionLevel().getReviseOwn().booleanValue(); + if (item != null && displayLevel != null + && displayLevel.getReviseOwn() != null) + return displayLevel.getReviseOwn().booleanValue(); else return false; } public void setReviseOwn(boolean reviseOwn) { - this.item.getPermissionLevel().setReviseOwn(Boolean.valueOf(reviseOwn)); + this.displayLevel.setReviseOwn(Boolean.valueOf(reviseOwn)); } /** diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/AreaManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/AreaManagerImpl.java index adcb1723d626..e97714748152 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/AreaManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/AreaManagerImpl.java @@ -21,27 +21,34 @@ package org.sakaiproject.component.app.messageforums; import java.util.Date; -import java.util.Iterator; +import lombok.Setter; import lombok.extern.slf4j.Slf4j; -import org.hibernate.HibernateException; import org.hibernate.query.Query; -import org.hibernate.Session; import org.hibernate.collection.internal.PersistentSet; import org.hibernate.type.StringType; +import org.sakaiproject.api.app.messageforums.OpenForum; import org.springframework.orm.hibernate5.HibernateCallback; import org.springframework.orm.hibernate5.support.HibernateDaoSupport; +import java.util.Collections; +import java.util.Set; + +import org.apache.commons.lang3.StringUtils; import org.sakaiproject.api.app.messageforums.Area; import org.sakaiproject.api.app.messageforums.AreaManager; -import org.sakaiproject.api.app.messageforums.BaseForum; +import org.sakaiproject.api.app.messageforums.DBMembershipItem; import org.sakaiproject.api.app.messageforums.DiscussionForum; import org.sakaiproject.api.app.messageforums.DiscussionTopic; +import org.sakaiproject.api.app.messageforums.MembershipItem; import org.sakaiproject.api.app.messageforums.MessageForumsForumManager; import org.sakaiproject.api.app.messageforums.MessageForumsTypeManager; +import org.sakaiproject.api.app.messageforums.PermissionLevelManager; +import org.sakaiproject.authz.api.AuthzGroupService; +import org.sakaiproject.authz.api.Role; import org.sakaiproject.component.api.ServerConfigurationService; import org.sakaiproject.component.app.messageforums.dao.hibernate.AreaImpl; -import org.sakaiproject.exception.IdUnusedException; +import org.sakaiproject.component.app.messageforums.dao.hibernate.DBMembershipItemImpl; import org.sakaiproject.id.api.IdManager; import org.sakaiproject.site.api.Site; import org.sakaiproject.site.api.SiteService; @@ -56,48 +63,24 @@ public class AreaManagerImpl extends HibernateDaoSupport implements AreaManager { private static final String QUERY_AREA_BY_CONTEXT_AND_TYPE_ID = "findAreaByContextIdAndTypeId"; - private static final String QUERY_AREA_BY_TYPE = "findAreaByType"; - - // TODO: pull titles from bundle private static final String MESSAGECENTER_BUNDLE = "org.sakaiproject.api.app.messagecenter.bundle.Messages"; private static final String MESSAGES_TITLE = "cdfm_message_pvtarea"; private static final String FORUMS_TITLE = "cdfm_discussions"; + private static final String DEFAULT_SEND_TO_EMAIL_PROP = "msgcntr.defaultSendToEmailSetting"; - private ResourceLoader rb; - - private IdManager idManager; - - private MessageForumsForumManager forumManager; - - private SessionManager sessionManager; - - private MessageForumsTypeManager typeManager; + @Setter private AuthzGroupService authzGroupService; + @Setter private MessageForumsForumManager forumManager; + @Setter private IdManager idManager; + @Setter private PermissionLevelManager permissionLevelManager; + @Setter private ServerConfigurationService serverConfigurationService; + @Setter private SessionManager sessionManager; + @Setter private SiteService siteService; + @Setter private ToolManager toolManager; + @Setter private MessageForumsTypeManager typeManager; - private ServerConfigurationService serverConfigurationService; + private ResourceLoader rb; private Boolean DEFAULT_AUTO_MARK_READ = false; - private SiteService siteService; - private ToolManager toolManager; - - /** - * sakai.property for setting the default Messages tool option for sending a copy of a message - * to recipient email addresses. Options are {@link Area#EMAIL_COPY_NEVER}, {@link Area#EMAIL_COPY_OPTIONAL}, - * and {@link Area#EMAIL_COPY_ALWAYS} - */ - private static final String DEFAULT_SEND_TO_EMAIL_PROP = "msgcntr.defaultSendToEmailSetting"; - - public void setServerConfigurationService( - ServerConfigurationService serverConfigurationService) { - this.serverConfigurationService = serverConfigurationService; - } - - public void setSiteService(SiteService siteService) { - this.siteService = siteService; - } - - public void setToolManager(ToolManager toolManager) { - this.toolManager = toolManager; - } public void init() { log.info("init()"); @@ -105,36 +88,6 @@ public void init() { DEFAULT_AUTO_MARK_READ = serverConfigurationService.getBoolean("msgcntr.forums.default.auto.mark.threads.read", false); } - - - public MessageForumsTypeManager getTypeManager() { - return typeManager; - } - - public void setTypeManager(MessageForumsTypeManager typeManager) { - this.typeManager = typeManager; - } - - public void setSessionManager(SessionManager sessionManager) { - this.sessionManager = sessionManager; - } - - public IdManager getIdManager() { - return idManager; - } - - public SessionManager getSessionManager() { - return sessionManager; - } - - public void setIdManager(IdManager idManager) { - this.idManager = idManager; - } - - public void setForumManager(MessageForumsForumManager forumManager) { - this.forumManager = forumManager; - } - public Area getPrivateArea() { return getPrivateArea(getContextId()); } @@ -150,7 +103,7 @@ public Area getPrivateArea(String siteId){ area.setLocked(Boolean.FALSE); area.setModerated(Boolean.FALSE); area.setPostFirst(Boolean.FALSE); - area.setAutoMarkThreadsRead(DEFAULT_AUTO_MARK_READ); + area.setAutoMarkThreadsRead(DEFAULT_AUTO_MARK_READ); area.setSendToEmail(serverConfigurationService.getInt(DEFAULT_SEND_TO_EMAIL_PROP, Area.EMAIL_COPY_OPTIONAL)); area = saveArea(area); } @@ -167,14 +120,14 @@ public Area getDiscussionArea(String contextId) { } public Area getDiscussionArea(String contextId, boolean createDefaultForum) { - log.debug("getDiscussionArea(" + contextId +")"); + log.debug("getDiscussionArea({})", contextId); if (contextId == null) { return getDiscusionArea(); } Area area = this.getAreaByContextIdAndTypeId(contextId, typeManager.getDiscussionForumType()); if (area == null) { - log.info("setting up a new Discussion Area for " + contextId); + log.info("setting up a new Discussion Area for {}", contextId); area = createArea(typeManager.getDiscussionForumType(), contextId); area.setName(getResourceBundleString(FORUMS_TITLE)); area.setEnabled(Boolean.TRUE); @@ -197,49 +150,64 @@ public Area getDiscussionArea(String contextId, boolean createDefaultForum) { return area; } private void setAreaDefaultElements(Area area) { - log.info("setAreaDefaultElements(" + area.getId() + ")"); + String siteId = area.getContextId(); + Site site = siteService.getOptionalSite(siteId).orElse(null); + if (site == null) return; + DiscussionForum forum = forumManager.createDiscussionForum(); forum.setArea(area); - String siteTitle = null; - try { - Site site = siteService.getSite(area.getContextId()); - siteTitle = site.getTitle(); - } catch (IdUnusedException e) { - log.error(e.getMessage(), e); - } - //MSGCNTR-453 forum.setCreatedBy("admin"); - forum.setTitle(getResourceBundleString("default_forum", new Object[]{(Object)siteTitle})); forum.setDraft(false); forum.setModerated(area.getModerated()); forum.setPostFirst(area.getPostFirst()); + forum.setTitle(getResourceBundleString("default_forum", new Object[]{site.getTitle()})); forum = forumManager.saveDiscussionForum(forum); + createDefaultMembershipItems(forum, null, site, "/site/" + siteId); + DiscussionTopic topic = forumManager.createDiscussionForumTopic(forum); topic.setTitle(getResourceBundleString("default_topic")); - //MSGCNTR-453 topic.setCreatedBy("admin"); forumManager.saveDiscussionForumTopic(topic, false); - + createDefaultMembershipItems(null, topic, site, "/site/" + siteId); + } + + private void createDefaultMembershipItems(DiscussionForum forum, DiscussionTopic topic, Site site, String contextSiteId) { + for (Role role : site.getRoles()) { + String roleId = role.getId(); + String levelName = resolveDefaultLevelName(roleId, contextSiteId); + DBMembershipItem item = permissionLevelManager.createDBMembershipItem(roleId, levelName, MembershipItem.TYPE_ROLE); + if (forum != null) { + ((DBMembershipItemImpl) item).setForum(forum); + } else { + ((DBMembershipItemImpl) item).setTopic(topic); + } + permissionLevelManager.saveDBMembershipItem(item); + } + } + + private String resolveDefaultLevelName(String roleId, String contextSiteId) { + String configured = serverConfigurationService.getString("mc.default." + roleId); + if (StringUtils.isNotBlank(configured)) return configured; + Set fns = authzGroupService.getAllowedFunctions(roleId, Collections.singletonList(contextSiteId)); + return fns.contains(SiteService.SECURE_UPDATE_SITE) + ? PermissionLevelManager.PERMISSION_LEVEL_NAME_OWNER + : PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR; } public boolean isPrivateAreaEnabled() { - return getPrivateArea().getEnabled().booleanValue(); + return getPrivateArea().getEnabled(); } public Area createArea(String typeId, String contextParam) { - - if (log.isDebugEnabled()) - { - log.debug("createArea(" + typeId + "," + contextParam + ")"); - } - + log.debug("createArea({},{})", typeId, contextParam); + Area area = new AreaImpl(); area.setUuid(getNextUuid()); area.setTypeUuid(typeId); area.setCreated(new Date()); area.setCreatedBy(getCurrentUser()); - /** compatibility with web services*/ + // compatibility with web services if (contextParam == null){ String contextId = getContextId(); if (contextId == null){ @@ -249,9 +217,9 @@ public Area createArea(String typeId, String contextParam) { } else{ area.setContextId(contextParam); - } - - log.debug("createArea executed with areaId: " + area.getUuid()); + } + + log.debug("createArea executed with areaId: {}", area.getUuid()); return area; } @@ -279,18 +247,16 @@ public Area saveArea(Area area, String currentUser){ if( area.getOpenForumsSet() != null && ((area.getOpenForumsSet() instanceof PersistentSet && ((PersistentSet)area.getOpenForumsSet()).wasInitialized()) || !(area.getOpenForumsSet() instanceof PersistentSet) )) { - for(Iterator i = area.getOpenForums().iterator(); i.hasNext(); ) { - BaseForum forum = (BaseForum)i.next(); - if(forum.getSortIndex().intValue() == 0) { - someForumHasZeroSortIndex = true; - break; - } - } + for (OpenForum openForum : area.getOpenForums()) { + if (openForum.getSortIndex() == 0) { + someForumHasZeroSortIndex = true; + break; + } + } if(someForumHasZeroSortIndex) { - for(Iterator i = area.getOpenForums().iterator(); i.hasNext(); ) { - BaseForum forum = (BaseForum)i.next(); - forum.setSortIndex(Integer.valueOf(forum.getSortIndex().intValue() + 1)); - } + for (OpenForum openForum : area.getOpenForums()) { + openForum.setSortIndex(openForum.getSortIndex() + 1); + } } } @@ -307,7 +273,7 @@ public Area saveArea(Area area, String currentUser){ public void deleteArea(Area area) { getHibernateTemplate().delete(area); - log.debug("deleteArea executed with areaId: " + area.getId()); + log.debug("deleteArea executed with areaId: {}", area.getId()); } /** @@ -318,31 +284,24 @@ private String getContextId() { return "test-context"; } Placement placement = toolManager.getCurrentPlacement(); - String presentSiteId = placement.getContext(); - return presentSiteId; + return placement.getContext(); } public Area getAreaByContextIdAndTypeId(final String typeId) { - log.debug("getAreaByContextIdAndTypeId executing for current user: " + getCurrentUser()); return this.getAreaByContextIdAndTypeId(getContextId(), typeId); } public Area getAreaByContextIdAndTypeId(final String contextId, final String typeId) { - log.debug("getAreaByContextIdAndTypeId executing for current user: " + getCurrentUser()); - HibernateCallback hcb = new HibernateCallback() { - public Object doInHibernate(Session session) throws HibernateException { - Query q = session.getNamedQuery(QUERY_AREA_BY_CONTEXT_AND_TYPE_ID); - q.setParameter("contextId", contextId, StringType.INSTANCE); - q.setParameter("typeId", typeId, StringType.INSTANCE); - return q.uniqueResult(); - } + log.debug("getAreaByContextIdAndTypeId executing for current user: {}", getCurrentUser()); + HibernateCallback hcb = session -> { + Query q = session.getNamedQuery(QUERY_AREA_BY_CONTEXT_AND_TYPE_ID); + q.setParameter("contextId", contextId, StringType.INSTANCE); + q.setParameter("typeId", typeId, StringType.INSTANCE); + return (Area) q.uniqueResult(); }; - - return (Area) getHibernateTemplate().execute(hcb); + return getHibernateTemplate().execute(hcb); } - // helpers - private String getNextUuid() { return idManager.createUuid(); } @@ -354,7 +313,6 @@ private String getCurrentUser() { private String getEventMessage(Object object) { return "/MessageCenter/site/" + getContextId() + "/" + object.toString() + "/" + getCurrentUser(); - //return "MessageCenter::" + getCurrentUser() + "::" + object.toString(); } /** diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java index 5ed41237abc6..061f6ec102b2 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java @@ -1168,6 +1168,9 @@ public DiscussionForum saveForum(DiscussionForum forum, boolean draft, String co area = areaManager.saveArea(area, currentUser); flagAreaCacheForClearing(area); } + if (saveArea && (forumReturn.getMembershipItemSet() == null || forumReturn.getMembershipItemSet().isEmpty())) { + createDefaultMembershipItemsForForum(forumReturn, contextId); + } return forumReturn; } @@ -1232,6 +1235,9 @@ public DiscussionTopic saveTopic(DiscussionTopic topic, boolean draft, ForumsTop forum = forumManager.saveDiscussionForum(forum, forum.getDraft(), false, currentUser); // event already logged by saveDiscussionForumTopic() //sak-5146 forumManager.saveDiscussionForum(forum); } + if (saveForum && (topic.getMembershipItemSet() == null || topic.getMembershipItemSet().isEmpty())) { + createDefaultMembershipItemsForTopic(topic, forum.getArea().getContextId()); + } flagAreaCacheForClearing(forum); if (params != null) @@ -2039,6 +2045,56 @@ public DBMembershipItem getAreaDBMember(Set originalSet, Strin return getDBMember(originalSet, name, type); } + private String resolveDefaultLevelName(String roleId, String contextSiteId) { + String configured = ServerConfigurationService.getString(MC_DEFAULT + roleId); + if (StringUtils.isNotBlank(configured)) return configured; + String cacheId = contextSiteId + "/" + roleId; + Set allowedFunctions = allowedFunctionsCache.get(cacheId); + if (allowedFunctions == null) { + allowedFunctions = authzGroupService.getAllowedFunctions(roleId, Collections.singletonList(contextSiteId)); + allowedFunctionsCache.put(cacheId, allowedFunctions); + } + return allowedFunctions.contains(SiteService.SECURE_UPDATE_SITE) + ? PermissionLevelManager.PERMISSION_LEVEL_NAME_OWNER + : PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR; + } + + private void createDefaultMembershipItemsForForum(DiscussionForum forum, String contextId) { + String contextSiteId = "/site/" + contextId; + Set items = new HashSet<>(); + try { + Site site = siteService.getSite(contextId); + for (Role role : site.getRoles()) { + String levelName = resolveDefaultLevelName(role.getId(), contextSiteId); + DBMembershipItem item = permissionLevelManager.createDBMembershipItem(role.getId(), levelName, MembershipItem.TYPE_ROLE); + ((DBMembershipItemImpl) item).setForum(forum); + item = permissionLevelManager.saveDBMembershipItem(item); + items.add(item); + } + } catch (IdUnusedException e) { + log.warn("Could not fetch site {} to create default forum membership items: {}", contextId, e.toString()); + } + forum.setMembershipItemSet(items); + } + + private void createDefaultMembershipItemsForTopic(DiscussionTopic topic, String contextId) { + String contextSiteId = "/site/" + contextId; + Set items = new HashSet<>(); + try { + Site site = siteService.getSite(contextId); + for (Role role : site.getRoles()) { + String levelName = resolveDefaultLevelName(role.getId(), contextSiteId); + DBMembershipItem item = permissionLevelManager.createDBMembershipItem(role.getId(), levelName, MembershipItem.TYPE_ROLE); + ((DBMembershipItemImpl) item).setTopic(topic); + item = permissionLevelManager.saveDBMembershipItem(item); + items.add(item); + } + } catch (IdUnusedException e) { + log.warn("Could not fetch site {} to create default topic membership items: {}", contextId, e.toString()); + } + topic.setMembershipItemSet(items); + } + @Override public DBMembershipItem getDBMember(Set originalSet, String name, int type) { return getDBMember(originalSet, name, type, getContextSiteId()); @@ -2051,21 +2107,12 @@ public DBMembershipItem getDBMember(Set originalSet, String na Optional membershipItem = Optional.empty(); if (originalSet != null) membershipItem = originalSet.stream().filter(ifTypeAndNameAreEqual).findAny(); - if (membershipItem.isPresent() && membershipItem.get().getPermissionLevel() != null) return membershipItem.get(); + if (membershipItem.isPresent()) return membershipItem.get(); + // Item not in the stored set — build a transient item with a default level from config or site roles. PermissionLevel level = null; - //for groups awareness if (type == MembershipItem.TYPE_ROLE || type == MembershipItem.TYPE_GROUP) { - - String levelName; - if (membershipItem.isPresent()) { - /** use level from stored item */ - levelName = membershipItem.get().getPermissionLevelName(); - } else { - /** get level from config file */ - levelName = ServerConfigurationService.getString(MC_DEFAULT + name); - } - + String levelName = ServerConfigurationService.getString(MC_DEFAULT + name); if (StringUtils.isNotBlank(levelName)) { level = permissionLevelManager.getPermissionLevelByName(levelName); } else if (name == null || ".anon".equals(name)) { @@ -2074,7 +2121,6 @@ public DBMembershipItem getDBMember(Set originalSet, String na if (type == MembershipItem.TYPE_GROUP) { level = permissionLevelManager.getDefaultNonePermissionLevel(); } else { - //check cache first: String cacheId = contextSiteId + "/" + name; Set allowedFunctions = allowedFunctionsCache.get(cacheId); if (allowedFunctions == null) { @@ -2091,11 +2137,10 @@ public DBMembershipItem getDBMember(Set originalSet, String na } PermissionLevel noneLevel = permissionLevelManager.getDefaultNonePermissionLevel(); - DBMembershipItem item = new DBMembershipItemImpl(); + DBMembershipItem item = new DBMembershipItemImpl(); item.setName(name); item.setPermissionLevelName((level == null) ? noneLevel.getName() : level.getName()); item.setType(type); - item.setPermissionLevel((level == null) ? noneLevel : level); return item; } @@ -2338,12 +2383,15 @@ public Set getUsersAllowedForTopic(Long topicId, boolean checkReadPermis // now we have the membership items. let's see which ones can read for (DBMembershipItem membershipItem : revisedMembershipItemSet) { - if ((checkReadPermission && membershipItem.getPermissionLevel().getRead() && !checkModeratePermission) || - (!checkReadPermission && checkModeratePermission && membershipItem.getPermissionLevel().getModeratePostings()) || - (checkReadPermission && membershipItem.getPermissionLevel().getRead() && checkModeratePermission && membershipItem.getPermissionLevel().getModeratePostings())) { + PermissionLevel pl = membershipItem.getPermissionLevel(); + if (pl == null) pl = permissionLevelManager.getPermissionLevelByName(membershipItem.getPermissionLevelName()); + if (pl == null) continue; + if ((checkReadPermission && Boolean.TRUE.equals(pl.getRead()) && !checkModeratePermission) || + (!checkReadPermission && checkModeratePermission && Boolean.TRUE.equals(pl.getModeratePostings())) || + (checkReadPermission && Boolean.TRUE.equals(pl.getRead()) && checkModeratePermission && Boolean.TRUE.equals(pl.getModeratePostings()))) { if (membershipItem.getType() == MembershipItem.TYPE_ROLE) { // add the users who are a member of this role - log.debug("Adding users in role: " + membershipItem.getName() + " with read: " + membershipItem.getPermissionLevel().getRead()); + log.debug("Adding users in role: " + membershipItem.getName() + " with read: " + pl.getRead()); Set usersInRole = currentSite.getUsersHasRole(membershipItem.getName()); usersAllowed.addAll(usersInRole); } else if (membershipItem.getType() == MembershipItem.TYPE_GROUP) { diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/PrivateMessageManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/PrivateMessageManagerImpl.java index e2e5f2f15fcf..b5e70f9a38ec 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/PrivateMessageManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/PrivateMessageManagerImpl.java @@ -114,9 +114,6 @@ import org.sakaiproject.util.ResourceLoader; import org.springframework.transaction.annotation.Transactional; -import java.text.DateFormat; -import java.text.SimpleDateFormat; - @Slf4j @Transactional public class PrivateMessageManagerImpl extends HibernateDaoSupport implements PrivateMessageManager { diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java index 7ef8cb7909f4..7b0d0c295991 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java @@ -24,6 +24,7 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -35,6 +36,7 @@ import org.sakaiproject.api.app.messageforums.DiscussionForum; import org.sakaiproject.api.app.messageforums.DiscussionTopic; import org.sakaiproject.api.app.messageforums.MembershipItem; +import org.sakaiproject.api.app.messageforums.PermissionLevel; import org.sakaiproject.api.app.messageforums.PermissionLevelManager; import org.sakaiproject.api.app.messageforums.ui.DiscussionForumManager; import org.sakaiproject.api.app.messageforums.ui.UIPermissionsManager; @@ -62,20 +64,6 @@ @Slf4j public class UIPermissionsManagerImpl implements UIPermissionsManager { - private static final Predicate ifChangeSettings = item -> item.getPermissionLevel().getChangeSettings(); - private static final Predicate ifDeleteAny = item -> item.getPermissionLevel().getDeleteAny(); - private static final Predicate ifDeleteOwn = item -> item.getPermissionLevel().getDeleteOwn(); - private static final Predicate ifMarkAsNotRead = item -> item.getPermissionLevel().getMarkAsNotRead(); - private static final Predicate ifModeratePostings = item -> item.getPermissionLevel().getModeratePostings(); - private static final Predicate ifMovePosting = item -> item.getPermissionLevel().getMovePosting(); - private static final Predicate ifNewResponse = item -> item.getPermissionLevel().getNewResponse(); - private static final Predicate ifNewResponseToResponse = item -> item.getPermissionLevel().getNewResponseToResponse(); - private static final Predicate ifPostToGradebook = item -> item.getPermissionLevel().getPostToGradebook(); - private static final Predicate ifRead = i -> i.getPermissionLevel().getRead(); - private static final Predicate ifReviseAny = item -> item.getPermissionLevel().getReviseAny(); - private static final Predicate ifReviseOwn = item -> item.getPermissionLevel().getReviseOwn(); - - @Setter private AuthzGroupService authzGroupService; @Setter private DiscussionForumManager forumManager; @Setter private MemoryService memoryService; @@ -89,18 +77,50 @@ public class UIPermissionsManagerImpl implements UIPermissionsManager { private Cache> membershipItemCache; private Cache> userGroupMembershipCache; + private Predicate ifChangeSettings; + private Predicate ifDeleteAny; + private Predicate ifDeleteOwn; + private Predicate ifMarkAsNotRead; + private Predicate ifModeratePostings; + private Predicate ifMovePosting; + private Predicate ifNewResponse; + private Predicate ifNewResponseToResponse; + private Predicate ifPostToGradebook; + private Predicate ifRead; + private Predicate ifReviseAny; + private Predicate ifReviseOwn; + public void init() { log.info("init()"); userGroupMembershipCache = memoryService.getCache("org.sakaiproject.component.app.messageforums.ui.UIPermissionsManagerImpl.userGroupMembershipCache"); membershipItemCache = memoryService.getCache("org.sakaiproject.component.app.messageforums.ui.UIPermissionsManagerImpl.membershipItemCache"); + + ifChangeSettings = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getChangeSettings()); + ifDeleteAny = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getDeleteAny()); + ifDeleteOwn = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getDeleteOwn()); + ifMarkAsNotRead = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getMarkAsNotRead()); + ifModeratePostings = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getModeratePostings()); + ifMovePosting = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getMovePosting()); + ifNewResponse = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getNewResponse()); + ifNewResponseToResponse = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getNewResponseToResponse()); + ifPostToGradebook = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getPostToGradebook()); + ifRead = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getRead()); + ifReviseAny = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getReviseAny()); + ifReviseOwn = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getReviseOwn()); + forumManager.setUiPermissionsManager(this); } + private PermissionLevel resolvePermissionLevel(DBMembershipItem item) { + return Optional.ofNullable(item.getPermissionLevel()) + .orElse(permissionLevelManager.getPermissionLevelByName(item.getPermissionLevelName())); + } + @Override public boolean isNewForum() { if (isSuperUser()) return true; - Predicate ifNewForum = item -> item.getPermissionLevel().getNewForum(); + Predicate ifNewForum = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getNewForum()); return getAreaItemsByCurrentUser().stream().anyMatch(ifNewForum); } @@ -147,7 +167,7 @@ public boolean isNewTopic(DiscussionForum forum) { && isInstructorForAllowedGroup(forum.getId(), true, siteId, getCurrentUserId())) { return true; } - Predicate ifNewTopic = item -> item.getPermissionLevel().getNewTopic(); + Predicate ifNewTopic = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getNewTopic()); return getForumItemsByCurrentUser(forum).stream().anyMatch(ifNewTopic); } @@ -408,7 +428,7 @@ public boolean isIdentifyAnonAuthors(DiscussionTopic topic) { if (isSuperUser(currentUserId)) return true; - Predicate ifIdentifyANonAuthors = i -> i.getPermissionLevel().getIdentifyAnonAuthors(); + Predicate ifIdentifyANonAuthors = i -> Boolean.TRUE.equals(resolvePermissionLevel(i).getIdentifyAnonAuthors()); return getTopicItemsByUser(topic, currentUserId, getContextId()).stream().anyMatch(ifIdentifyANonAuthors); } diff --git a/msgcntr/messageforums-component-impl/src/webapp/WEB-INF/components.xml b/msgcntr/messageforums-component-impl/src/webapp/WEB-INF/components.xml index 655f710cfba8..b84b334594b8 100644 --- a/msgcntr/messageforums-component-impl/src/webapp/WEB-INF/components.xml +++ b/msgcntr/messageforums-component-impl/src/webapp/WEB-INF/components.xml @@ -144,6 +144,8 @@ + + membershipItemSet; - private Set hiddenGroups; - private Date openDate; - private Date closeDate; - - private Boolean postFirst; - - /** - * availabilityRestricted: this is the radio button the users turns on or off this feature with - */ - private Boolean availabilityRestricted = false; - /** - * if availabilityRestricted, then this determines whether the area is disabled or not - */ - private Boolean availability = true; - - public void setVersion(Integer version) - { - this.version = version; - } - - public String getContextId() - { - return contextId; - } - - public void setContextId(String contextId) - { - this.contextId = contextId; - } - - public Boolean getHidden() - { - return hidden; - } - - public void setHidden(Boolean hidden) - { - this.hidden = hidden; - } - - public String getName() - { - return name; - } - - public void setName(String name) - { - this.name = name; - } - - public String getTypeUuid() - { - return typeUuid; - } - - public void setTypeUuid(String typeUuid) - { - this.typeUuid = typeUuid; - } - - public Boolean getEnabled() - { - return enabled; - } - - public void setEnabled(Boolean enabled) - { - this.enabled = enabled; - } - - public int getSendToEmail() { - return sendToEmail; - } - - public void setSendToEmail(int sendToEmail) { - this.sendToEmail = sendToEmail; - } - - public List getOpenForums() - { - return Util.setToList(openForumsSet); - } - - public void setOpenForums(List openForums) - { - this.openForumsSet = Util.listToSet(openForums); - } - - public List getPrivateForums() - { - return Util.setToList(privateForumsSet); - } - - public void setPrivateForums(List privateForums) - { - this.privateForumsSet = Util.listToSet(privateForums); - } - - public List getDiscussionForums() - { - return Util.setToList(discussionForumsSet); - } - - public void setDiscussionForums(List discussionForums) - { - this.discussionForumsSet = Util.listToSet(discussionForums); - } - - public String toString() { - //return "Area.id:" + id; - return "Area/" + id; - } - - public Boolean getLocked() { - return locked; - } - - public void setLocked(Boolean locked) { - this.locked = locked; - } - - public Boolean getModerated() { - return moderated; - } - - public void setModerated(Boolean moderated) { - this.moderated = moderated; - } - - public Boolean getAutoMarkThreadsRead() { - return autoMarkThreadsRead; -} - -public void setAutoMarkThreadsRead(Boolean autoMarkThreadsRead) { - this.autoMarkThreadsRead = autoMarkThreadsRead; -} - -public Set getDiscussionForumsSet() { - return discussionForumsSet; - } - - public void setDiscussionForumsSet(Set discussionForumsSet) { - this.discussionForumsSet = discussionForumsSet; - } - - public Set getOpenForumsSet() { - return openForumsSet; - } - - public void setOpenForumsSet(Set openForumsSet) { - this.openForumsSet = openForumsSet; - } - - public Set getPrivateForumsSet() { - return privateForumsSet; - } - - public void setPrivateForumsSet(Set privateForumsSet) { - this.privateForumsSet = privateForumsSet; - } - - public Set getMembershipItemSet() { - return membershipItemSet; - } - - public void setMembershipItemSet(Set membershipItemSet) { - this.membershipItemSet = membershipItemSet; - } - - //////////////////////////////////////////////////////////////////////// - // helper methods for collections - //////////////////////////////////////////////////////////////////////// - - public void addPrivateForum(BaseForum forum) { - if (log.isDebugEnabled()) { - log.debug("addPrivateForum(forum " + forum + ")"); - } - - if (forum == null) { - throw new IllegalArgumentException("forum == null"); - } - - if (privateForumsSet == null) { - privateForumsSet = new TreeSet(); - } - forum.setArea(this); - privateForumsSet.add(forum); - } - - public void removePrivateForum(BaseForum forum) { - if (log.isDebugEnabled()) { - log.debug("removePrivateForum(forum " + forum + ")"); - } - - if (forum == null) { - throw new IllegalArgumentException("Illegal topic argument passed!"); - } - - forum.setArea(null); - privateForumsSet.remove(forum); - } - - public void addDiscussionForum(BaseForum forum) { - if (log.isDebugEnabled()) { - log.debug("addForum(forum " + forum + ")"); - } - - if (forum == null) { - throw new IllegalArgumentException("forum == null"); - } - - if (discussionForumsSet == null) { - discussionForumsSet = new TreeSet(); - } - forum.setArea(this); - discussionForumsSet.add(forum); - } - - public void removeDiscussionForum(BaseForum forum) { - if (log.isDebugEnabled()) { - log.debug("removeDiscussionForum(forum " + forum + ")"); - } - - if (forum == null) { - throw new IllegalArgumentException("Illegal topic argument passed!"); - } - - forum.setArea(null); - discussionForumsSet.remove(forum); - openForumsSet.remove(forum); - } - - public void addOpenForum(BaseForum forum) { - if (log.isDebugEnabled()) { - log.debug("addOpenForum(forum " + forum + ")"); - } - - if (forum == null) { - throw new IllegalArgumentException("forum == null"); - } - - if (openForumsSet == null) { - openForumsSet = new TreeSet(); - } - forum.setArea(this); - openForumsSet.add(forum); - } +import lombok.Getter; +import lombok.Setter; +import lombok.extern.slf4j.Slf4j; - public void removeOpenForum(BaseForum forum) { - if (log.isDebugEnabled()) { - log.debug("removeOpenForum(forum " + forum + ")"); - } - - if (forum == null) { - throw new IllegalArgumentException("Illegal topic argument passed!"); - } - - forum.setArea(null); - openForumsSet.remove(forum); - } - - public void addMembershipItem(DBMembershipItem item) { - if (log.isDebugEnabled()) { - log.debug("addMembershipItem(item " + item + ")"); - } - - if (item == null) { - throw new IllegalArgumentException("item == null"); +@Slf4j +public class AreaImpl extends MutableEntityImpl implements Area { + + @Getter @Setter private Boolean availabilityRestricted = false; + @Getter @Setter private Set hiddenGroups; + @Setter @Getter private Boolean autoMarkThreadsRead; + @Setter @Getter private Boolean availability = true; + @Setter @Getter private Boolean enabled; + @Setter @Getter private Boolean hidden; + @Setter @Getter private Boolean locked; + @Setter @Getter private Boolean moderated; + @Setter @Getter private Boolean postFirst; + @Setter @Getter private Boolean sendEmailOut = false; + @Setter @Getter private Date closeDate; + @Setter @Getter private Date openDate; + @Setter @Getter private Set discussionForumsSet; + @Setter @Getter private Set openForumsSet; + @Setter @Getter private Set privateForumsSet; + @Setter @Getter private Set membershipItemSet; + @Setter @Getter private String contextId; + @Setter @Getter private String name; + @Setter @Getter private String typeUuid; + @Setter @Getter private int sendToEmail; + + public List getOpenForums() { + return new ArrayList<>(openForumsSet); } - - if (membershipItemSet == null) { - membershipItemSet = new HashSet(); - } - membershipItemSet.add(item); -} - public void removeMembershipItem(DBMembershipItem item) { - if (log.isDebugEnabled()) { - log.debug("removeMembershipItem(item " + item + ")"); + public void setOpenForums(List openForums) { + this.openForumsSet = new HashSet<>(openForums); } - - if (item == null) { - throw new IllegalArgumentException("Illegal level argument passed!"); + + public List getPrivateForums() { + return new ArrayList<>(privateForumsSet); } - - membershipItemSet.remove(item); - } - public Boolean getAvailabilityRestricted() { - return availabilityRestricted; - } - - public void setAvailabilityRestricted(Boolean restricted) { - this.availabilityRestricted = restricted; - - } + public void setPrivateForums(List privateForums) { + this.privateForumsSet = new HashSet<>(privateForums); + } - public Date getOpenDate() { - return openDate; - } + public List getDiscussionForums() { + return new ArrayList<>(discussionForumsSet); + } - public void setOpenDate(Date openDate) { - this.openDate = openDate; - } + public void setDiscussionForums(List discussionForums) { + this.discussionForumsSet = new HashSet<>(discussionForums); + } - public Date getCloseDate() { - return closeDate; - } + public String toString() { + return "Area/" + id; + } - public void setCloseDate(Date closeDate) { - this.closeDate = closeDate; - } + public void addPrivateForum(PrivateForum forum) { + log.debug("addPrivateForum(forum {})", forum); + if (forum == null) { + throw new IllegalArgumentException("forum == null"); + } + if (privateForumsSet == null) { + privateForumsSet = new HashSet<>(); + } + forum.setArea(this); + privateForumsSet.add(forum); + } - public Boolean getAvailability() { - return availability; - } + public void removePrivateForum(PrivateForum forum) { + log.debug("removePrivateForum(forum {})", forum); + if (forum == null) { + throw new IllegalArgumentException("Illegal topic argument passed!"); + } + forum.setArea(null); + privateForumsSet.remove(forum); + } - public void setAvailability(Boolean availability) { - this.availability = availability; - } + public void addDiscussionForum(DiscussionForum forum) { + log.debug("addForum(forum {})", forum); + if (forum == null) { + throw new IllegalArgumentException("forum == null"); + } + if (discussionForumsSet == null) { + discussionForumsSet = new HashSet<>(); + } + forum.setArea(this); + discussionForumsSet.add(forum); + } - public Boolean getPostFirst() { - return postFirst; - } + public void removeDiscussionForum(DiscussionForum forum) { + log.debug("removeDiscussionForum(forum {})", forum); + if (forum == null) { + throw new IllegalArgumentException("Illegal topic argument passed!"); + } + forum.setArea(null); + discussionForumsSet.remove(forum); + openForumsSet.remove(forum); + } - public void setPostFirst(Boolean postFirst) { - this.postFirst = postFirst; - } + public void addOpenForum(OpenForum forum) { + log.debug("addOpenForum(forum {})", forum); + if (forum == null) { + throw new IllegalArgumentException("forum == null"); + } + if (openForumsSet == null) { + openForumsSet = new HashSet<>(); + } + forum.setArea(this); + openForumsSet.add(forum); + } - @Override - public Set getHiddenGroups() { - return hiddenGroups; - } + public void removeOpenForum(OpenForum forum) { + log.debug("removeOpenForum(forum {})", forum); + if (forum == null) { + throw new IllegalArgumentException("Illegal topic argument passed!"); + } + forum.setArea(null); + openForumsSet.remove(forum); + } - @Override - public void setHiddenGroups(Set hiddenGroups) { - this.hiddenGroups = hiddenGroups; - } + public void addMembershipItem(DBMembershipItem item) { + log.debug("addMembershipItem(item {})", item); + if (item == null) { + throw new IllegalArgumentException("item == null"); + } + if (membershipItemSet == null) { + membershipItemSet = new HashSet<>(); + } + membershipItemSet.add(item); + } - /** - * {@link Deprecated} This option was replaced by sendToEmail via MSGCNTR-708 - */ - public Boolean getSendEmailOut() { - return sendEmailOut; - } - - /** - * {@link Deprecated} This option was replaced by sendToEmail via MSGCNTR-708 - */ - public void setSendEmailOut(Boolean sendEmailOut) { - this.sendEmailOut = sendEmailOut; - } + public void removeMembershipItem(DBMembershipItem item) { + log.debug("removeMembershipItem(item {})", item); + if (item == null) { + throw new IllegalArgumentException("Illegal level argument passed!"); + } + membershipItemSet.remove(item); + } } From 0cbe49b4e7eae1a171429f35e603192f327f239e Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Thu, 21 May 2026 16:51:23 -0400 Subject: [PATCH 6/9] coderabbitai suggestions --- .../messageforums/DiscussionForumTool.java | 38 ++++++++++--------- .../tool/messageforums/ui/PermissionBean.java | 7 +++- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java index c17fd9cc0932..ba033929efb6 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java @@ -1647,7 +1647,7 @@ private boolean needToUpdateSynopticOnForumSave(Object target, boolean isDraft){ boolean isDraftOld = false; boolean availabilityChanged = false; - Set oldMembershipItemSet = null; + Set oldMembershipItemSet = null; if (target instanceof DiscussionForum){ DiscussionForum forum = ((DiscussionForum) target); @@ -1691,23 +1691,25 @@ else if (target instanceof Topic){ update = true; break; } - - Iterator iter2 = oldMembershipItemSet.iterator(); - while(iter2.hasNext()) - { - DBMembershipItem oldItem = (DBMembershipItem) iter2.next(); - if(permBean.getItem().getId().equals(oldItem.getId())){ - PermissionLevel oldLevel = oldItem.getPermissionLevel(); - if (oldLevel == null) { - oldLevel = permissionLevelManager.getPermissionLevelByName(oldItem.getPermissionLevelName()); - } - Boolean oldModerate = (oldLevel != null) ? oldLevel.getModeratePostings() : Boolean.FALSE; - if(permBean.getModeratePostings() != Boolean.TRUE.equals(oldModerate)){ - update = true; - break; - } - } - } + + for (DBMembershipItem oldItem : oldMembershipItemSet) { + if (permBean.getItem().getId().equals(oldItem.getId())) { + PermissionLevel oldLevel = oldItem.getPermissionLevel(); + if (oldLevel == null) { + if (PermissionLevelManager.PERMISSION_LEVEL_NAME_CUSTOM.equals(oldItem.getPermissionLevelName())) { + // Custom item with no stored level — permissions are unknown, treat as changed + update = true; + break; + } + oldLevel = permissionLevelManager.getPermissionLevelByName(oldItem.getPermissionLevelName()); + } + Boolean oldModerate = (oldLevel != null) ? oldLevel.getModeratePostings() : Boolean.FALSE; + if (permBean.getModeratePostings() != Boolean.TRUE.equals(oldModerate)) { + update = true; + break; + } + } + } if(update){ break; } diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java index 160d6e10aed8..4124e66b45c7 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java @@ -78,12 +78,15 @@ private void setPermissionsForLevel(String selectedLevel) { if (!"Custom".equals(selectedLevel)) { - this.displayLevel = permissionLevelManager.getPermissionLevelByName(selectedLevel); + PermissionLevel level = permissionLevelManager.getPermissionLevelByName(selectedLevel); + if (level != null) { + this.displayLevel = level; + } } else { MessageForumsTypeManager typeManager = (MessageForumsTypeManager) ComponentManager.get("org.sakaiproject.api.app.messageforums.MessageForumsTypeManager"); - if (this.displayLevel == null || !this.displayLevel.getTypeUuid().equals(typeManager.getCustomLevelType())) + if (this.displayLevel == null || !typeManager.getCustomLevelType().equals(this.displayLevel.getTypeUuid())) { this.displayLevel = permissionLevelManager.createPermissionLevel(selectedLevel, typeManager.getCustomLevelType(), new PermissionsMask()); } From 4625d9caae15a07a56b211bd95ec779f48779d2b Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Thu, 21 May 2026 17:52:53 -0400 Subject: [PATCH 7/9] update to OpenForum type in SiteEntityController --- .../webapi/controllers/SiteEntityController.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/webapi/src/main/java/org/sakaiproject/webapi/controllers/SiteEntityController.java b/webapi/src/main/java/org/sakaiproject/webapi/controllers/SiteEntityController.java index 6a4c9f48bf8f..f0d82258439e 100644 --- a/webapi/src/main/java/org/sakaiproject/webapi/controllers/SiteEntityController.java +++ b/webapi/src/main/java/org/sakaiproject/webapi/controllers/SiteEntityController.java @@ -29,6 +29,7 @@ import org.apache.commons.lang3.StringUtils; import org.sakaiproject.api.app.messageforums.Area; import org.sakaiproject.api.app.messageforums.DiscussionForum; +import org.sakaiproject.api.app.messageforums.OpenForum; import org.sakaiproject.api.app.messageforums.ui.DiscussionForumManager; import org.sakaiproject.assignment.api.AssignmentReferenceReckoner; import org.sakaiproject.assignment.api.AssignmentService; @@ -277,7 +278,7 @@ public ResponseEntity> updateSiteEntities(@PathVariable Area forumArea = discussionForumManager.getDiscussionForumArea(siteId); - Optional optForum = findForumInArea(forumArea, patchEntity.getId()); + Optional optForum = findForumInArea(forumArea, patchEntity.getId()); if (optForum.isEmpty()) { log.debug("Forum with id {} not found", patchEntity.getId()); @@ -513,12 +514,12 @@ private Set timeExceptionSet(PublishedAssessmentFacade as } @SuppressWarnings("unchecked") - private Optional findForumInArea(Area forumArea, String forumId) { + private Optional findForumInArea(Area forumArea, String forumId) { if (forumArea == null) { return Optional.empty(); } - Set forums = (Set) forumArea.getOpenForumsSet(); + Set forums = forumArea.getOpenForumsSet(); return forums.stream() .filter(forum -> Long.valueOf(forumId).equals(forum.getId())) From ae3f07091419089cf8491f06df6c02c9a4324bd5 Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Thu, 21 May 2026 18:23:52 -0400 Subject: [PATCH 8/9] more coderabbitai suggestions --- .../messageforums/DiscussionForumTool.java | 12 ++++-- .../tool/messageforums/ui/PermissionBean.java | 3 ++ .../app/messageforums/AreaManagerImpl.java | 3 +- .../ui/DiscussionForumManagerImpl.java | 20 ++++++++-- .../ui/UIPermissionsManagerImpl.java | 37 ++++++++++--------- .../messageforums/dao/hibernate/AreaImpl.java | 32 ++++++++++------ 6 files changed, 71 insertions(+), 36 deletions(-) diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java index ba033929efb6..f1e61d082a14 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java @@ -1222,6 +1222,8 @@ public String processActionForumSettings() return gotoMain(); } + attachments.clear(); + prepareRemoveAttach.clear(); List attachList = forum.getAttachments(); if (attachList != null) { @@ -1230,7 +1232,7 @@ public String processActionForumSettings() attachments.add(new DecoratedAttachment((Attachment)attachList.get(i))); } } - + selectedForum = new DiscussionForumBean(forum, forumManager, userTimeService); loadForumDataInForumBean(forum, selectedForum); if("true".equalsIgnoreCase(ServerConfigurationService.getString("mc.defaultLongDescription"))) @@ -1837,6 +1839,8 @@ public String processActionReviseTopicSettings() setErrorMessage(getResourceBundleString(INSUFFICIENT_PRIVILEGES_NEW_TOPIC)); return gotoMain(); } + attachments.clear(); + prepareRemoveAttach.clear(); List attachList = selectedTopic.getTopic().getAttachments(); if (attachList != null) { @@ -1844,8 +1848,8 @@ public String processActionReviseTopicSettings() { attachments.add(new DecoratedAttachment((Attachment)attachList.get(i))); } - } - + } + setFromMainOrForumOrTopic(); siteGroups.clear(); return TOPIC_SETTING_REVISE; @@ -2319,6 +2323,8 @@ public String processActionTopicSettings() selectedTopic.setReadFullDesciption(true); } + attachments.clear(); + prepareRemoveAttach.clear(); List attachList = selectedTopic.getTopic().getAttachments(); if (attachList != null) { diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java index 4124e66b45c7..392713d417cb 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/PermissionBean.java @@ -52,6 +52,9 @@ public PermissionBean(DBMembershipItem item, this.selectedLevel = item.getPermissionLevelName(); PermissionLevel level = item.getPermissionLevel(); this.displayLevel = (level != null) ? level : permissionLevelManager.getPermissionLevelByName(selectedLevel); + if (this.displayLevel == null) { + setPermissionsForLevel(selectedLevel); + } } /** diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/AreaManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/AreaManagerImpl.java index e97714748152..4fce22701737 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/AreaManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/AreaManagerImpl.java @@ -195,7 +195,8 @@ private String resolveDefaultLevelName(String roleId, String contextSiteId) { } public boolean isPrivateAreaEnabled() { - return getPrivateArea().getEnabled(); + Area area = getPrivateArea(); + return area != null && Boolean.TRUE.equals(area.getEnabled()); } public Area createArea(String typeId, String contextParam) { diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java index 061f6ec102b2..08683bc74b6f 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/DiscussionForumManagerImpl.java @@ -2482,6 +2482,12 @@ public void setTopicGroupRestrictions(Long topicId, Set groupIds) { String siteId = topic.getBaseForum().getArea().getContextId(); List groupNames = resolveSiteGroupNames(groupIds, siteId); if (groupNames != null) { + if (!groupNames.isEmpty()) { + Set topicItems = topic.getMembershipItemSet(); + if (topicItems == null || topicItems.isEmpty()) { + createDefaultMembershipItemsForTopic((DiscussionTopic) topic, siteId); + } + } applyGroupRestrictions(topic.getMembershipItemSet(), groupNames); forumManager.saveDiscussionForumTopic((DiscussionTopic) topic); return; @@ -2498,6 +2504,12 @@ public void setForumGroupRestrictions(Long forumId, Set groupIds) { String siteId = forum.getArea().getContextId(); List groupNames = resolveSiteGroupNames(groupIds, siteId); if (groupNames != null) { + if (!groupNames.isEmpty()) { + Set forumItems = forum.getMembershipItemSet(); + if (forumItems == null || forumItems.isEmpty()) { + createDefaultMembershipItemsForForum((DiscussionForum) forum, siteId); + } + } applyGroupRestrictions(forum.getMembershipItemSet(), groupNames); forumManager.saveDiscussionForum((DiscussionForum) forum); return; @@ -2539,7 +2551,7 @@ private List resolveSiteGroupNames(Set groupIds, String siteId) */ private void applyGroupRestrictions(Set membershipItemSet, List groupNames) { if (groupNames != null && !groupNames.isEmpty()) { - // Restricting: demote only Contributor items to None; promote specified groups to Contributor. + // Restricting: demote only Contributor group items to None; promote specified groups to Contributor. Set toAdd = new HashSet<>(groupNames); for (DBMembershipItem item : membershipItemSet) { if (Objects.equals(item.getType(), MembershipItem.TYPE_GROUP) && groupNames.contains(item.getName())) { @@ -2548,7 +2560,8 @@ private void applyGroupRestrictions(Set membershipItemSet, Lis item.setPermissionLevel(null); item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR); } - } else if (PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR.equals(item.getPermissionLevelName())) { + } else if (Objects.equals(item.getType(), MembershipItem.TYPE_GROUP) + && PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR.equals(item.getPermissionLevelName())) { item.setPermissionLevel(null); item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE); } @@ -2569,7 +2582,8 @@ private void applyGroupRestrictions(Set membershipItemSet, Lis item.setPermissionLevel(null); item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR); } - } else if (PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR.equals(item.getPermissionLevelName())) { + } else if (Objects.equals(item.getType(), MembershipItem.TYPE_GROUP) + && PermissionLevelManager.PERMISSION_LEVEL_NAME_CONTRIBUTOR.equals(item.getPermissionLevelName())) { item.setPermissionLevel(null); item.setPermissionLevelName(PermissionLevelManager.PERMISSION_LEVEL_NAME_NONE); } diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java index 7b0d0c295991..694dfb0d8bee 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java @@ -95,32 +95,33 @@ public void init() { userGroupMembershipCache = memoryService.getCache("org.sakaiproject.component.app.messageforums.ui.UIPermissionsManagerImpl.userGroupMembershipCache"); membershipItemCache = memoryService.getCache("org.sakaiproject.component.app.messageforums.ui.UIPermissionsManagerImpl.membershipItemCache"); - ifChangeSettings = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getChangeSettings()); - ifDeleteAny = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getDeleteAny()); - ifDeleteOwn = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getDeleteOwn()); - ifMarkAsNotRead = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getMarkAsNotRead()); - ifModeratePostings = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getModeratePostings()); - ifMovePosting = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getMovePosting()); - ifNewResponse = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getNewResponse()); - ifNewResponseToResponse = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getNewResponseToResponse()); - ifPostToGradebook = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getPostToGradebook()); - ifRead = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getRead()); - ifReviseAny = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getReviseAny()); - ifReviseOwn = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getReviseOwn()); + ifChangeSettings = item -> resolvePermissionLevel(item).map(PermissionLevel::getChangeSettings).orElse(false); + ifDeleteAny = item -> resolvePermissionLevel(item).map(PermissionLevel::getDeleteAny).orElse(false); + ifDeleteOwn = item -> resolvePermissionLevel(item).map(PermissionLevel::getDeleteOwn).orElse(false); + ifMarkAsNotRead = item -> resolvePermissionLevel(item).map(PermissionLevel::getMarkAsNotRead).orElse(false); + ifModeratePostings = item -> resolvePermissionLevel(item).map(PermissionLevel::getModeratePostings).orElse(false); + ifMovePosting = item -> resolvePermissionLevel(item).map(PermissionLevel::getMovePosting).orElse(false); + ifNewResponse = item -> resolvePermissionLevel(item).map(PermissionLevel::getNewResponse).orElse(false); + ifNewResponseToResponse = item -> resolvePermissionLevel(item).map(PermissionLevel::getNewResponseToResponse).orElse(false); + ifPostToGradebook = item -> resolvePermissionLevel(item).map(PermissionLevel::getPostToGradebook).orElse(false); + ifRead = item -> resolvePermissionLevel(item).map(PermissionLevel::getRead).orElse(false); + ifReviseAny = item -> resolvePermissionLevel(item).map(PermissionLevel::getReviseAny).orElse(false); + ifReviseOwn = item -> resolvePermissionLevel(item).map(PermissionLevel::getReviseOwn).orElse(false); forumManager.setUiPermissionsManager(this); } - private PermissionLevel resolvePermissionLevel(DBMembershipItem item) { - return Optional.ofNullable(item.getPermissionLevel()) - .orElse(permissionLevelManager.getPermissionLevelByName(item.getPermissionLevelName())); + private Optional resolvePermissionLevel(DBMembershipItem item) { + return Optional.ofNullable( + Optional.ofNullable(item.getPermissionLevel()) + .orElse(permissionLevelManager.getPermissionLevelByName(item.getPermissionLevelName()))); } @Override public boolean isNewForum() { if (isSuperUser()) return true; - Predicate ifNewForum = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getNewForum()); + Predicate ifNewForum = item -> resolvePermissionLevel(item).map(PermissionLevel::getNewForum).orElse(false); return getAreaItemsByCurrentUser().stream().anyMatch(ifNewForum); } @@ -167,7 +168,7 @@ public boolean isNewTopic(DiscussionForum forum) { && isInstructorForAllowedGroup(forum.getId(), true, siteId, getCurrentUserId())) { return true; } - Predicate ifNewTopic = item -> Boolean.TRUE.equals(resolvePermissionLevel(item).getNewTopic()); + Predicate ifNewTopic = item -> resolvePermissionLevel(item).map(PermissionLevel::getNewTopic).orElse(false); return getForumItemsByCurrentUser(forum).stream().anyMatch(ifNewTopic); } @@ -428,7 +429,7 @@ public boolean isIdentifyAnonAuthors(DiscussionTopic topic) { if (isSuperUser(currentUserId)) return true; - Predicate ifIdentifyANonAuthors = i -> Boolean.TRUE.equals(resolvePermissionLevel(i).getIdentifyAnonAuthors()); + Predicate ifIdentifyANonAuthors = i -> resolvePermissionLevel(i).map(PermissionLevel::getIdentifyAnonAuthors).orElse(false); return getTopicItemsByUser(topic, currentUserId, getContextId()).stream().anyMatch(ifIdentifyANonAuthors); } diff --git a/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/AreaImpl.java b/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/AreaImpl.java index 414d2421d37b..3820b8371234 100644 --- a/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/AreaImpl.java +++ b/msgcntr/messageforums-hbm/src/java/org/sakaiproject/component/app/messageforums/dao/hibernate/AreaImpl.java @@ -63,27 +63,27 @@ public class AreaImpl extends MutableEntityImpl implements Area { @Setter @Getter private int sendToEmail; public List getOpenForums() { - return new ArrayList<>(openForumsSet); + return openForumsSet == null ? new ArrayList<>() : new ArrayList<>(openForumsSet); } public void setOpenForums(List openForums) { - this.openForumsSet = new HashSet<>(openForums); + this.openForumsSet = openForums == null ? new HashSet<>() : new HashSet<>(openForums); } public List getPrivateForums() { - return new ArrayList<>(privateForumsSet); + return privateForumsSet == null ? new ArrayList<>() : new ArrayList<>(privateForumsSet); } public void setPrivateForums(List privateForums) { - this.privateForumsSet = new HashSet<>(privateForums); + this.privateForumsSet = privateForums == null ? new HashSet<>() : new HashSet<>(privateForums); } public List getDiscussionForums() { - return new ArrayList<>(discussionForumsSet); + return discussionForumsSet == null ? new ArrayList<>() : new ArrayList<>(discussionForumsSet); } public void setDiscussionForums(List discussionForums) { - this.discussionForumsSet = new HashSet<>(discussionForums); + this.discussionForumsSet = discussionForums == null ? new HashSet<>() : new HashSet<>(discussionForums); } public String toString() { @@ -108,7 +108,9 @@ public void removePrivateForum(PrivateForum forum) { throw new IllegalArgumentException("Illegal topic argument passed!"); } forum.setArea(null); - privateForumsSet.remove(forum); + if (privateForumsSet != null) { + privateForumsSet.remove(forum); + } } public void addDiscussionForum(DiscussionForum forum) { @@ -129,8 +131,12 @@ public void removeDiscussionForum(DiscussionForum forum) { throw new IllegalArgumentException("Illegal topic argument passed!"); } forum.setArea(null); - discussionForumsSet.remove(forum); - openForumsSet.remove(forum); + if (discussionForumsSet != null) { + discussionForumsSet.remove(forum); + } + if (openForumsSet != null) { + openForumsSet.remove(forum); + } } public void addOpenForum(OpenForum forum) { @@ -151,7 +157,9 @@ public void removeOpenForum(OpenForum forum) { throw new IllegalArgumentException("Illegal topic argument passed!"); } forum.setArea(null); - openForumsSet.remove(forum); + if (openForumsSet != null) { + openForumsSet.remove(forum); + } } public void addMembershipItem(DBMembershipItem item) { @@ -170,7 +178,9 @@ public void removeMembershipItem(DBMembershipItem item) { if (item == null) { throw new IllegalArgumentException("Illegal level argument passed!"); } - membershipItemSet.remove(item); + if (membershipItemSet != null) { + membershipItemSet.remove(item); + } } } From 7b9174e39ebf02adab782765b934b7c307eecb36 Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Thu, 21 May 2026 19:33:53 -0400 Subject: [PATCH 9/9] last suggestion --- .../app/messageforums/ui/UIPermissionsManagerImpl.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java index 694dfb0d8bee..9d0feaa84885 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java @@ -112,9 +112,11 @@ public void init() { } private Optional resolvePermissionLevel(DBMembershipItem item) { - return Optional.ofNullable( - Optional.ofNullable(item.getPermissionLevel()) - .orElse(permissionLevelManager.getPermissionLevelByName(item.getPermissionLevelName()))); + if (item == null) return Optional.empty(); + PermissionLevel level = item.getPermissionLevel(); + if (level != null) return Optional.of(level); + return Optional.ofNullable(item.getPermissionLevelName()) + .flatMap(name -> Optional.ofNullable(permissionLevelManager.getPermissionLevelByName(name))); } @Override