SAK-43953 PA System: Banner Dismissal Resets Upon Any Form of Update (#…

…11977) (cherry picked from commit 9ca81e5)
sakaiproject · Oct 31, 2023 · 097e81f · 097e81f
1 parent 6e1403b
commit 097e81f
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 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
@@ -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<Banner> getForId(String uuid);
 }

diff --git a/...impl/src/java/org/sakaiproject/pasystem/impl/acknowledgements/AcknowledgementStorage.java b/...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<Void>() {
                             @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/...tem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/banners/BannerStorage.java b/...tem/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/...system-impl/impl/src/java/org/sakaiproject/pasystem/impl/rest/PASystemEntityProvider.java b/...system-impl/impl/src/java/org/sakaiproject/pasystem/impl/rest/PASystemEntityProvider.java
@@ -146,7 +146,7 @@ public String clearBannerAcknowledgements(EntityView view, Map<String, Object> 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
@@ -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<String, List<String>> messages) {
-        Session session = SessionManager.getCurrentSession();
+        Session session = sessionManager.getCurrentSession();
         session.setAttribute(FLASH_MESSAGE_KEY, messages);
     }
 
     private Map<String, List<String>> loadFlashMessages() {
-        Session session = SessionManager.getCurrentSession();
+        Session session = sessionManager.getCurrentSession();
 
         if (session.getAttribute(FLASH_MESSAGE_KEY) != null) {
             Map<String, List<String>> flashErrors = (Map<String, List<String>>) session.getAttribute(FLASH_MESSAGE_KEY);

diff --git a/...m/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/handlers/BannersHandler.java b/...m/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<String, Obje
         String uuid = extractId(request);
         BannerForm bannerForm = BannerForm.fromRequest(uuid, request);
 
+        String userId = sessionManager.getCurrentSessionUserId();
+        if (userId == null) {
+            log.warn("No user ID found for current session");
+            return;
+        }
+
         this.addErrors(bannerForm.validate());
 
         if (hasErrors()) {
@@ -101,6 +110,7 @@ protected void handleCreateOrUpdate(HttpServletRequest request, Map<String, Obje
             flash("info", "banner_created");
         } else {
             paSystem.getBanners().updateBanner(bannerForm.toBanner());
+            paSystem.getBanners().clearAcknowledgementForUser(userId);
             flash("info", "banner_updated");
         }