From fb27db6dcdfb92216918db8ac704d2dd261455ab Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Wed, 21 Jan 2026 21:02:53 -0500 Subject: [PATCH 1/3] SAK-51027 Discussions Date read-only option toggles Lock Topic, requires instructor to disable in order to update dates --- .../messageforums/ui/DiscussionForumBean.java | 23 +----- .../messageforums/ui/DiscussionTopicBean.java | 21 +----- .../discussionForum/forum/dfForumDetail.jsp | 4 +- .../discussionForum/message/dfAllMessages.jsp | 2 +- .../discussionForum/topic/dfTopicSettings.jsp | 4 +- .../MessageForumsMessageManagerImpl.java | 38 +++++++--- .../ui/UIPermissionsManagerImpl.java | 73 +++++++++++++------ 7 files changed, 92 insertions(+), 73 deletions(-) diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionForumBean.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionForumBean.java index c0a39557ee06..513c13a77b01 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionForumBean.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionForumBean.java @@ -178,21 +178,7 @@ public void setLocked(String locked) public Boolean getForumLocked() { log.debug("getForumLocked()"); - if (locked == null || "".equals(locked)){ - if (forum == null || forum.getLocked() == null - || forum.getLocked().booleanValue() == false) - { - locked = Boolean.FALSE.toString(); - } - else - { - locked = Boolean.TRUE.toString(); - } - } - - handleLockedAfterClosedCondition(); - - return Boolean.parseBoolean(locked); + return forum != null && forum.getLocked() != null && forum.getLocked(); } /** @@ -203,17 +189,16 @@ public void setForumLocked(Boolean locked) { log.debug("setForumLocked(String"+ locked+")"); forum.setLocked(locked); + this.locked = String.valueOf(Boolean.TRUE.equals(locked)); } private void handleLockedAfterClosedCondition() { Boolean availabilityRestricted = getForum().getAvailabilityRestricted(); - if(availabilityRestricted && locked.equals(Boolean.FALSE.toString())) { + if (Boolean.TRUE.equals(availabilityRestricted) && Boolean.FALSE.toString().equals(locked)) { Date closeDate = getForum().getCloseDate(); if(closeDate != null) { - // this seems so dirty. - if (getForum().getLockedAfterClosed() && closeDate.before(new Date())) { - setForumLocked(true); + if (Boolean.TRUE.equals(getForum().getLockedAfterClosed()) && closeDate.before(new Date())) { locked = Boolean.TRUE.toString(); } } diff --git a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionTopicBean.java b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionTopicBean.java index 1eeef839e85a..e9cfc915dfc8 100644 --- a/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionTopicBean.java +++ b/msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionTopicBean.java @@ -385,20 +385,7 @@ public void setLocked(String locked) public Boolean getTopicLocked() { log.debug("getTopicLocked()"); - if (StringUtils.isBlank(locked)){ - if (topic == null || topic.getLocked() == null || !topic.getLocked()) - { - locked = Boolean.FALSE.toString(); - } - else - { - locked = Boolean.TRUE.toString(); - } - } - - handleLockedAfterClosedCondition(); - - return Boolean.parseBoolean(locked); + return topic != null && topic.getLocked() != null && topic.getLocked(); } /** @@ -408,15 +395,15 @@ public void setTopicLocked(Boolean locked) { log.debug("setTopicLocked(Boolean {})", locked); topic.setLocked(locked); + this.locked = String.valueOf(Boolean.TRUE.equals(locked)); } private void handleLockedAfterClosedCondition(){ Boolean availabilityRestricted = getTopic().getAvailabilityRestricted(); - if(availabilityRestricted && locked.equals(Boolean.FALSE.toString())) { + if (Boolean.TRUE.equals(availabilityRestricted) && Boolean.FALSE.toString().equals(locked)) { Date closeDate = getTopic().getCloseDate(); - if (closeDate != null && getTopic().getLockedAfterClosed() && closeDate.before(new Date())) { - setTopicLocked(true); + if (closeDate != null && Boolean.TRUE.equals(getTopic().getLockedAfterClosed()) && closeDate.before(new Date())) { locked = Boolean.TRUE.toString(); } } diff --git a/msgcntr/messageforums-app/src/webapp/jsp/discussionForum/forum/dfForumDetail.jsp b/msgcntr/messageforums-app/src/webapp/jsp/discussionForum/forum/dfForumDetail.jsp index 2242b00f1627..2c5b31646555 100644 --- a/msgcntr/messageforums-app/src/webapp/jsp/discussionForum/forum/dfForumDetail.jsp +++ b/msgcntr/messageforums-app/src/webapp/jsp/discussionForum/forum/dfForumDetail.jsp @@ -40,7 +40,7 @@ - + <%-- Rubrics marker --%> - + <%-- Rubrics marker --%> - + <%-- Rubrics marker --%> diff --git a/msgcntr/messageforums-app/src/webapp/jsp/discussionForum/topic/dfTopicSettings.jsp b/msgcntr/messageforums-app/src/webapp/jsp/discussionForum/topic/dfTopicSettings.jsp index 5f4e21039ce4..20cd06db873a 100644 --- a/msgcntr/messageforums-app/src/webapp/jsp/discussionForum/topic/dfTopicSettings.jsp +++ b/msgcntr/messageforums-app/src/webapp/jsp/discussionForum/topic/dfTopicSettings.jsp @@ -51,10 +51,10 @@

- + - + diff --git a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsMessageManagerImpl.java b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsMessageManagerImpl.java index 85ec06bb3a3e..b13842de1c1e 100644 --- a/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsMessageManagerImpl.java +++ b/msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsMessageManagerImpl.java @@ -54,6 +54,8 @@ import org.sakaiproject.api.app.messageforums.MessageForumsMessageManager; import org.sakaiproject.api.app.messageforums.MessageForumsTypeManager; import org.sakaiproject.api.app.messageforums.MessageMoveHistory; +import org.sakaiproject.api.app.messageforums.OpenForum; +import org.sakaiproject.api.app.messageforums.OpenTopic; import org.sakaiproject.api.app.messageforums.PrivateMessage; import org.sakaiproject.api.app.messageforums.SynopticMsgcntrManager; import org.sakaiproject.api.app.messageforums.Topic; @@ -63,6 +65,8 @@ import org.sakaiproject.component.app.messageforums.dao.hibernate.AttachmentImpl; import org.sakaiproject.component.app.messageforums.dao.hibernate.MessageImpl; import org.sakaiproject.component.app.messageforums.dao.hibernate.MessageMoveHistoryImpl; +import org.sakaiproject.component.app.messageforums.dao.hibernate.OpenForumImpl; +import org.sakaiproject.component.app.messageforums.dao.hibernate.OpenTopicImpl; import org.sakaiproject.component.app.messageforums.dao.hibernate.PrivateMessageImpl; import org.sakaiproject.component.app.messageforums.dao.hibernate.UnreadStatusImpl; import org.sakaiproject.component.app.messageforums.dao.hibernate.Util; @@ -1757,19 +1761,33 @@ private boolean isForumOrTopicLocked(final Long forumId, final Long topicId) { log.debug("isForumLocked executing with forumId: " + forumId + ":: topicId: " + topicId); + final Date now = new Date(); + HibernateCallback hcb = session -> { - Query q = session.getNamedQuery("findForumLockedAttribute"); - q.setParameter("id", forumId, LongType.INSTANCE); - return (Boolean) q.uniqueResult(); + OpenForumImpl forum = session.get(OpenForumImpl.class, forumId); + OpenTopicImpl topic = session.get(OpenTopicImpl.class, topicId); + return isLocked(forum, now) || isLocked(topic, now); }; - HibernateCallback hcb2 = session -> { - Query q = session.getNamedQuery("findTopicLockedAttribute"); - q.setParameter("id", topicId, LongType.INSTANCE); - return (Boolean) q.uniqueResult(); - }; - - return getHibernateTemplate().execute(hcb) || getHibernateTemplate().execute(hcb2); + return getHibernateTemplate().execute(hcb); + } + + private boolean isLocked(OpenForum forum, Date now) { + if (forum == null) return true; + if (Boolean.TRUE.equals(forum.getLocked())) return true; + if (!Boolean.TRUE.equals(forum.getAvailabilityRestricted())) return false; + if (!Boolean.TRUE.equals(forum.getLockedAfterClosed())) return false; + Date closeDate = forum.getCloseDate(); + return closeDate != null && closeDate.before(now); + } + + private boolean isLocked(OpenTopic topic, Date now) { + if (topic == null) return true; + if (Boolean.TRUE.equals(topic.getLocked())) return true; + if (!Boolean.TRUE.equals(topic.getAvailabilityRestricted())) return false; + if (!Boolean.TRUE.equals(topic.getLockedAfterClosed())) return false; + Date closeDate = topic.getCloseDate(); + return closeDate != null && closeDate.before(now); } // helpers 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 9da00d92163b..7404431482e3 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 @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Objects; @@ -161,9 +162,9 @@ public boolean isNewResponse(DiscussionTopic topic, DiscussionForum forum, Strin if (forum != null && !forum.getDraft() - && !forum.getLocked() + && !isLocked(forum) && !topic.getDraft() - && !topic.getLocked()) { + && !isLocked(topic)) { return getTopicItemsByUser(topic, userId, contextId).stream().anyMatch(ifNewResponse); } return false; @@ -180,9 +181,9 @@ public boolean isNewResponseToResponse(DiscussionTopic topic, DiscussionForum fo if (forum != null && !forum.getDraft() - && !forum.getLocked() + && !isLocked(forum) && !topic.getDraft() - && !topic.getLocked()) { + && !isLocked(topic)) { return getTopicItemsByUser(topic, userId, contextId).stream().anyMatch(ifNewResponseToResponse); } return false; @@ -194,9 +195,9 @@ public boolean isMovePostings(DiscussionTopic topic, DiscussionForum forum) { if (forum != null && !forum.getDraft() - && !forum.getLocked() + && !isLocked(forum) && !topic.getDraft() - && !topic.getLocked()) { + && !isLocked(topic)) { return getTopicItemsByCurrentUser(topic).stream().anyMatch(ifMovePosting.or(ifReviseAny).or(ifReviseOwn)); } @@ -292,9 +293,9 @@ public boolean isReviseAny(DiscussionTopic topic, DiscussionForum forum, String if (checkBaseConditions(topic, forum, userId, contextId)) return true; return (forum.getDraft() == null || !forum.getDraft()) - && (forum.getLocked() == null || !forum.getLocked()) + && !isLocked(forum) && (topic.getDraft() == null || !topic.getDraft()) - && (topic.getLocked() == null || !topic.getLocked()) + && !isLocked(topic) && getTopicItemsByUser(topic, userId, contextId).stream().anyMatch(ifReviseAny); } @@ -307,12 +308,12 @@ public boolean isReviseOwn(DiscussionTopic topic, DiscussionForum forum) { public boolean isReviseOwn(DiscussionTopic topic, DiscussionForum forum, String userId, String contextId) { if (checkBaseConditions(topic, forum, userId, contextId)) return true; - if (topic.getLocked() == null || topic.getLocked()) return false; + if (isLocked(topic)) return false; if (!forum.getDraft() - && !forum.getLocked() + && !isLocked(forum) && !topic.getDraft() - && !topic.getLocked()) { + && !isLocked(topic)) { return getTopicItemsByUser(topic, userId, contextId).stream().anyMatch(ifReviseOwn); } return false; @@ -327,12 +328,12 @@ public boolean isDeleteAny(DiscussionTopic topic, DiscussionForum forum) { public boolean isDeleteAny(DiscussionTopic topic, DiscussionForum forum, String userId, String contextId) { if (checkBaseConditions(topic, forum, userId, contextId)) return true; - if (topic.getLocked() == null || topic.getLocked()) return false; + if (isLocked(topic)) return false; if (!forum.getDraft() - && !forum.getLocked() + && !isLocked(forum) && !topic.getDraft() - && !topic.getLocked()) { + && !isLocked(topic)) { return getTopicItemsByUser(topic, userId, contextId).stream().anyMatch(ifDeleteAny); } return false; @@ -347,12 +348,12 @@ public boolean isDeleteOwn(DiscussionTopic topic, DiscussionForum forum) { public boolean isDeleteOwn(DiscussionTopic topic, DiscussionForum forum, String userId, String contextId) { if (checkBaseConditions(topic, forum, userId, contextId)) return true; - if (topic.getLocked() == null || topic.getLocked().equals(Boolean.TRUE)) return false; + if (isLocked(topic)) return false; if (!forum.getDraft() - && !forum.getLocked() + && !isLocked(forum) && !topic.getDraft() - && !topic.getLocked()) { + && !isLocked(topic)) { return getTopicItemsByUser(topic, userId, contextId).stream().anyMatch(ifDeleteOwn); } return false; @@ -362,12 +363,12 @@ public boolean isDeleteOwn(DiscussionTopic topic, DiscussionForum forum, String public boolean isMarkAsRead(DiscussionTopic topic, DiscussionForum forum) { if (checkBaseConditions(topic, forum)) return true; - if (topic.getLocked() == null || topic.getLocked().equals(Boolean.TRUE)) return false; + if (isLocked(topic)) return false; if (!forum.getDraft() - && !forum.getLocked() + && !isLocked(forum) && !topic.getDraft() - && !topic.getLocked()) { + && !isLocked(topic)) { return getTopicItemsByCurrentUser(topic).stream().anyMatch(ifMarkAsRead); } return false; @@ -581,8 +582,8 @@ public BulkPermission getBulkPermissions(DiscussionTopic topic, DiscussionForum } boolean ifTopicOwner = topic != null && forumManager.isTopicOwner(topic, userId); - boolean ifLockedTopic = topic != null && (topic.getLocked() == null || topic.getLocked()); - boolean ifLockedForum = forum != null && (forum.getLocked() == null || forum.getLocked()); + boolean ifLockedTopic = isLocked(topic); + boolean ifLockedForum = isLocked(forum); boolean ifDraftTopic = topic != null && (topic.getDraft() != null && topic.getDraft()); boolean ifDraftForum = forum != null && (forum.getDraft() != null && forum.getDraft()); @@ -692,6 +693,34 @@ private boolean checkBaseConditions(DiscussionTopic topic, DiscussionForum forum || (topic != null && topic.getRestrictPermissionsForGroups() && isInstructorForAllowedGroup(topic.getId(), false, contextSiteId, userId)); } + private boolean isLockedAfterClose(DiscussionForum forum) { + if (forum == null) return false; + if (!Boolean.TRUE.equals(forum.getAvailabilityRestricted())) return false; + if (!Boolean.TRUE.equals(forum.getLockedAfterClosed())) return false; + Date closeDate = forum.getCloseDate(); + return closeDate != null && closeDate.before(new Date()); + } + + private boolean isLockedAfterClose(DiscussionTopic topic) { + if (topic == null) return false; + if (!Boolean.TRUE.equals(topic.getAvailabilityRestricted())) return false; + if (!Boolean.TRUE.equals(topic.getLockedAfterClosed())) return false; + Date closeDate = topic.getCloseDate(); + return closeDate != null && closeDate.before(new Date()); + } + + private boolean isLocked(DiscussionForum forum) { + if (forum == null) return true; + if (Boolean.TRUE.equals(forum.getLocked())) return true; + return isLockedAfterClose(forum); + } + + private boolean isLocked(DiscussionTopic topic) { + if (topic == null) return true; + if (Boolean.TRUE.equals(topic.getLocked())) return true; + return isLockedAfterClose(topic); + } + public void clearMembershipsFromCacheForArea(Area area) { if (area == null || area.getId() == null) return; String areaId = area.getId().toString(); From 4a96d3162ba0616fc06a964abee286d341785592 Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Thu, 22 Jan 2026 15:04:30 -0500 Subject: [PATCH 2/3] one more use case on copy --- .../tool/messageforums/DiscussionForumTool.java | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 840bb975b42e..eaf4dd3240ae 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 @@ -2120,6 +2120,17 @@ private String saveTopicSettings(boolean draft) if(selectedForum.getForum().getRestrictPermissionsForGroups() && ServerConfigurationService.getBoolean("msgcntr.restricted.group.perms", false)){ topic.setRestrictPermissionsForGroups(true); } + + if (!isNew + && Boolean.TRUE.equals(topic.getAvailabilityRestricted()) + && Boolean.TRUE.equals(topic.getLockedAfterClosed()) + && Boolean.TRUE.equals(topic.getLocked())) { + Date closeDate = topic.getCloseDate(); + if (closeDate != null && closeDate.after(new Date())) { + topic.setLocked(false); + selectedTopic.setTopicLocked(false); + } + } if(topic.getCreatedBy()==null&&this.forumManager.getAnonRole()==true){ topic.setCreatedBy(".anon"); } From dfe05ed5575a2ab3c23e1abccd1ad337b5f6a7ee Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Thu, 22 Jan 2026 15:28:29 -0500 Subject: [PATCH 3/3] coderabbit --- .../tool/messageforums/DiscussionForumTool.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 eaf4dd3240ae..0aecf8503bf2 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 @@ -2125,8 +2125,13 @@ private String saveTopicSettings(boolean draft) && Boolean.TRUE.equals(topic.getAvailabilityRestricted()) && Boolean.TRUE.equals(topic.getLockedAfterClosed()) && Boolean.TRUE.equals(topic.getLocked())) { + DiscussionTopic persistedTopic = forumManager.getTopicById(topic.getId()); + Date persistedCloseDate = persistedTopic != null ? persistedTopic.getCloseDate() : null; Date closeDate = topic.getCloseDate(); - if (closeDate != null && closeDate.after(new Date())) { + if (persistedCloseDate != null + && persistedCloseDate.before(new Date()) + && closeDate != null + && closeDate.after(new Date())) { topic.setLocked(false); selectedTopic.setTopicLocked(false); }