SAK-51027 Discussions Date read-only option toggles Lock Topic, requi…

[ottenhoff]

…res instructor to disable in order to update dates

Summary by CodeRabbit

• Bug Fixes

  • Consistent lock icon/display: UI now reads the selected forum/topic locked flags so icons render reliably.
  • Safer lock-state evaluation: null-safe logic now considers availability, close dates, and timing to avoid incorrect locked status and NPEs.
  • Permissions now respect the revised lock determination, preventing improper access.

• Behavior

  • Defensive sync: editing availability-restricted topics keeps in-memory lock state aligned with persisted close-date changes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Refactored locking: bean getters/setters now use direct boolean checks and null-safe comparisons; JSPs read lock state from selected beans; locking/availability/close-date logic centralized into manager/permissions helpers; added defensive unlock when saving topic settings with changed close dates.

Changes

Cohort / File(s)	Summary
Bean Logic Updates msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionForumBean.java, msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/ui/DiscussionTopicBean.java	Replaced string/lazy lock state with direct Boolean checks, synchronized setters to update local state, and tightened null-safe comparisons in handleLockedAfterClosedCondition.
View Updates msgcntr/messageforums-app/src/webapp/jsp/discussionForum/forum/dfForumDetail.jsp, msgcntr/messageforums-app/src/webapp/jsp/discussionForum/message/dfAllMessages.jsp, msgcntr/messageforums-app/src/webapp/jsp/discussionForum/topic/dfTopicSettings.jsp	Changed lock icon rendering to use selectedForum.locked / selectedTopic.locked (bean-level properties) instead of nested .forum.locked / .topic.locked.
Manager & Permissions Refactor msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsMessageManagerImpl.java, msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java	Consolidated lock checks into new isLocked(...) and isLockedAfterClose(...) helpers, adjusted message manager to load OpenForum/OpenTopic in-session, and updated permission checks to use centralized date-aware locking logic.
Tool Defensive Unlock msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java	Added logic in saveTopicSettings to detect and clear a persisted locked state when an availability-restricted topic's persisted close date is past but the new close date is future (defensive unlock).

Suggested reviewers

• ern

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: addressing an issue where the 'Date read-only' option toggles 'Lock Topic', requiring instructors to disable the lock to update dates.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.40.5)

msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/MessageForumsMessageManagerImpl.java (1)

1756-1791: Replace debug logging with SLF4J parameterized format.

Line 1762 uses string concatenation in logging: log.debug("isForumLocked executing with forumId: " + forumId + ":: topicId: " + topicId);. Use parameterized logging instead: log.debug("isForumLocked executing with forumId: {} :: topicId: {}", forumId, topicId); per the Java coding guidelines.

🧹 Nitpick comments (1)

msgcntr/messageforums-component-impl/src/java/org/sakaiproject/component/app/messageforums/ui/UIPermissionsManagerImpl.java (1)

307-320: Redundant isLocked(topic) check.

Line 311 already returns false if the topic is locked, so the !isLocked(topic) check on line 316 is always true when reached and can be removed. The same redundancy appears in isDeleteAny, isDeleteOwn, and isMarkAsRead.

♻️ Suggested simplification

 public boolean isReviseOwn(DiscussionTopic topic, DiscussionForum forum, String userId, String contextId) {
     if (checkBaseConditions(topic, forum, userId, contextId)) return true;

     if (isLocked(topic)) return false;

     if (!forum.getDraft()
             && !isLocked(forum)
-            && !topic.getDraft()
-            && !isLocked(topic)) {
+            && !topic.getDraft()) {
         return getTopicItemsByUser(topic, userId, contextId).stream().anyMatch(ifReviseOwn);
     }
     return false;
 }

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In
`@msgcntr/messageforums-app/src/java/org/sakaiproject/tool/messageforums/DiscussionForumTool.java`:
- Around line 2123-2133: The current block in DiscussionForumTool
unconditionally clears topic locks when editing a close date into the future,
which also clears instructor-set manual locks; instead, load the
persisted/original topic close date (e.g., fetch the stored Topic by id as
persistedTopic or existingTopic before applying edits) and only auto-unlock if
the persisted closeDate existed and was in the past (persistedCloseDate != null
&& persistedCloseDate.before(new Date())) and the topic has lockedAfterClosed
true; otherwise leave topic.getLocked() untouched so manual locks are preserved.
Ensure null checks for persistedCloseDate and use the same
selectedTopic.setTopicLocked(false) only in the auto-unlock case.