From 9ca81e5623057df533d9734911f1fa10dd02618c Mon Sep 17 00:00:00 2001 From: Kunal Jaykam <50500283+kunaljaykam@users.noreply.github.com> Date: Fri, 20 Oct 2023 15:02:53 +0530 Subject: [PATCH] SAK-43953 PA System: Banner Dismissal Resets Upon Any Form of Update (#11977) --- .../org/sakaiproject/pasystem/api/Banners.java | 2 +- .../acknowledgements/AcknowledgementStorage.java | 9 ++++----- .../pasystem/impl/banners/BannerStorage.java | 4 ++-- .../pasystem/impl/rest/PASystemEntityProvider.java | 2 +- .../pasystem/tool/PASystemServlet.java | 12 +++++++----- .../pasystem/tool/handlers/BannersHandler.java | 14 ++++++++++++-- 6 files changed, 27 insertions(+), 16 deletions(-) diff --git a/pasystem/pasystem-api/api/src/java/org/sakaiproject/pasystem/api/Banners.java b/pasystem/pasystem-api/api/src/java/org/sakaiproject/pasystem/api/Banners.java index 65e351db7de9..03452dd64fa8 100644 --- a/pasystem/pasystem-api/api/src/java/org/sakaiproject/pasystem/api/Banners.java +++ b/pasystem/pasystem-api/api/src/java/org/sakaiproject/pasystem/api/Banners.java @@ -48,7 +48,7 @@ public interface Banners extends Acknowledger { /** * Forget all acknowledgements for the current user. */ - public void clearTemporaryDismissedForUser(String userId); + public void clearAcknowledgementForUser(String userId); public Optional getForId(String uuid); } diff --git a/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/acknowledgements/AcknowledgementStorage.java b/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/acknowledgements/AcknowledgementStorage.java index 3225f4b750cd..7607c327eda7 100644 --- a/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/acknowledgements/AcknowledgementStorage.java +++ b/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/acknowledgements/AcknowledgementStorage.java @@ -78,16 +78,15 @@ public Void call(DBConnection db) throws SQLException { } /** - * Forget all temporary acknowledgements created by a user. + * Forget all acknowledgements created by a user. */ - public void clearTemporaryDismissedForUser(String userId) { + public void clearAcknowledgementForUser(String userId) { DB.transaction - ("Delete all temporarily dismissed banners for a user", + ("Delete all acknowledgement for a user", new DBAction() { @Override public Void call(DBConnection db) throws SQLException { - db.run("DELETE FROM " + tableName + " WHERE state = ? AND user_id = ?") - .param(AcknowledgementType.TEMPORARY.dbValue()) + db.run("DELETE FROM " + tableName + " WHERE user_id = ?") .param(userId) .executeUpdate(); diff --git a/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/banners/BannerStorage.java b/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/banners/BannerStorage.java index e3470d2b3a18..bfbeb321dee1 100644 --- a/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/banners/BannerStorage.java +++ b/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/banners/BannerStorage.java @@ -265,7 +265,7 @@ private AcknowledgementType calculateAcknowledgementType(String uuid) { } @Override - public void clearTemporaryDismissedForUser(String userId) { - new AcknowledgementStorage(AcknowledgementStorage.NotificationType.BANNER).clearTemporaryDismissedForUser(userId); + public void clearAcknowledgementForUser(String userId) { + new AcknowledgementStorage(AcknowledgementStorage.NotificationType.BANNER).clearAcknowledgementForUser(userId); } } diff --git a/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/rest/PASystemEntityProvider.java b/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/rest/PASystemEntityProvider.java index e980254e015d..03cdeeffed18 100644 --- a/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/rest/PASystemEntityProvider.java +++ b/pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/rest/PASystemEntityProvider.java @@ -146,7 +146,7 @@ public String clearBannerAcknowledgements(EntityView view, Map p return result.toJSONString(); } - paSystem.getBanners().clearTemporaryDismissedForUser(userId); + paSystem.getBanners().clearAcknowledgementForUser(userId); result.put("status", "SUCCESS"); return result.toJSONString(); diff --git a/pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/PASystemServlet.java b/pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/PASystemServlet.java index b2f432173f3d..de40f415c30f 100644 --- a/pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/PASystemServlet.java +++ b/pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/PASystemServlet.java @@ -60,7 +60,7 @@ import org.sakaiproject.time.api.Time; import org.sakaiproject.time.cover.TimeService; import org.sakaiproject.tool.api.Session; -import org.sakaiproject.tool.cover.SessionManager; +import org.sakaiproject.tool.api.SessionManager; import org.sakaiproject.tool.cover.ToolManager; import org.sakaiproject.user.cover.PreferencesService; @@ -75,12 +75,14 @@ public class PASystemServlet extends HttpServlet { private PASystem paSystem; private ClusterService clusterService; + private SessionManager sessionManager; public void init(ServletConfig config) throws ServletException { super.init(config); paSystem = ComponentManager.get(PASystem.class); clusterService = ComponentManager.get(ClusterService.class); + sessionManager = ComponentManager.get(SessionManager.class); } private Handler handlerForRequest(HttpServletRequest request) { @@ -93,7 +95,7 @@ private Handler handlerForRequest(HttpServletRequest request) { if (path.contains("/popups/")) { return new PopupsHandler(paSystem); } else if (path.contains("/banners/")) { - return new BannersHandler(paSystem, clusterService); + return new BannersHandler(paSystem, clusterService, sessionManager); } else { return new IndexHandler(paSystem); } @@ -149,18 +151,18 @@ private void checkAccessControl() { String siteId = ToolManager.getCurrentPlacement().getContext(); if (!SecurityService.unlock("pasystem.manage", "/site/" + siteId)) { - log.error("Access denied to PA System management tool for user " + SessionManager.getCurrentSessionUserId()); + log.error("Access denied to PA System management tool for user " + sessionManager.getCurrentSessionUserId()); throw new PASystemException("Access denied"); } } private void storeFlashMessages(Map> messages) { - Session session = SessionManager.getCurrentSession(); + Session session = sessionManager.getCurrentSession(); session.setAttribute(FLASH_MESSAGE_KEY, messages); } private Map> loadFlashMessages() { - Session session = SessionManager.getCurrentSession(); + Session session = sessionManager.getCurrentSession(); if (session.getAttribute(FLASH_MESSAGE_KEY) != null) { Map> flashErrors = (Map>) session.getAttribute(FLASH_MESSAGE_KEY); diff --git a/pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/handlers/BannersHandler.java b/pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/handlers/BannersHandler.java index b17e473335b5..cd2b2ee91dc4 100644 --- a/pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/handlers/BannersHandler.java +++ b/pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/handlers/BannersHandler.java @@ -37,6 +37,7 @@ import org.sakaiproject.pasystem.api.Banner; import org.sakaiproject.pasystem.api.PASystem; import org.sakaiproject.pasystem.tool.forms.BannerForm; +import org.sakaiproject.tool.api.SessionManager; /** * A handler for creating and updating banners in the PA System administration tool. @@ -45,11 +46,13 @@ public class BannersHandler extends CrudHandler { private final PASystem paSystem; - private final ClusterService clusterService; + private final ClusterService clusterService; + private final SessionManager sessionManager; - public BannersHandler(PASystem pasystem, ClusterService clusterService) { + public BannersHandler(PASystem pasystem, ClusterService clusterService, SessionManager sessionManager) { this.paSystem = pasystem; this.clusterService = clusterService; + this.sessionManager = sessionManager; } @Override @@ -89,6 +92,12 @@ protected void handleCreateOrUpdate(HttpServletRequest request, Map