Scarab commit: svn commit: r10738 - trunk/src: conf/classes java/org/tigris/scarab/notification java/org/tigris/scarab/om webapp/WEB-INF/templates/email/notification

Hussayn Dabbous <[email protected]>
Newsgroups gmane.comp.java.scarab.cvs
Message-ID <[email protected]>
Author: dabbous
Date: 2009-05-31 08:15:56-0700
New Revision: 10738

Modified:
   trunk/src/conf/classes/ScarabBundle_de.properties
   trunk/src/conf/classes/ScarabBundle_en.properties
   trunk/src/java/org/tigris/scarab/notification/ActivityType.java
   trunk/src/java/org/tigris/scarab/notification/ScarabNewNotificationManager.java
   trunk/src/java/org/tigris/scarab/om/NotificationStatusPeer.java
   trunk/src/webapp/WEB-INF/templates/email/notification/IssueActivitySubject.vm

Log:
SCB2893: make email notification subject more informative. working solution, but might need improvements

Modified: trunk/src/conf/classes/ScarabBundle_de.properties
Url: http://scarab.tigris.org/source/browse/scarab/trunk/src/conf/classes/ScarabBundle_de.properties?view=diff&pathrev738&r1737&r2738
=============================================================================--- trunk/src/conf/classes/ScarabBundle_de.properties	(original)
+++ trunk/src/conf/classes/ScarabBundle_de.properties	2009-05-31 08:15:56-0700
@@ -1455,12 +1455,31 @@
 ActivityTypeCommentAdded = Kommentar hinzugefügt
 ActivityTypeCommentChanged = Kommentar modifiziert

+NotificationIssueCreated=neu
+NotificationIssueMoved=verschoben
+NotificationIssueCopied=Kopie
+NotificationIssueDeleted=gelöscht
+NotificationCommentAdded=Kommentar
+NotificationCommentChanged=Kommentar
+NotificationAttributeChanged=Attribut
+NotificationUserAttributeChanged=Personal
+NotificationURLAdded=Referenz
+NotificationURLCHanged=Referenz
+NotificationURLDESCCHanged=Referenz
+NotificationURLDeleted=Referenz
+NotificationAttachmentCreated=Anhang
+NotificationAttachmentRemoved=Anhang
+NotificationDependencyCreated�hängigkeit
+NotificationDependencyChanged�hängigkeit
+NotificationDependencyDeleted�hängigkeit
+NotificationIssueOther=?
+
 ## ***********************************************************************
 ## Misc Java Objects
 ## ***********************************************************************

 # ActivitySet
-DefaultModifyIssueEmailSubject=[{0}] Ticket #{1} {2}
+DefaultModifyIssueEmailSubject=[{0}] #{1} {2}
 MinimalModifyIssueEmailSubject=Ticket #{0} geändert

 # AbstractScarabModule

Modified: trunk/src/conf/classes/ScarabBundle_en.properties
Url: http://scarab.tigris.org/source/browse/scarab/trunk/src/conf/classes/ScarabBundle_en.properties?view=diff&pathrev738&r1737&r2738
=============================================================================--- trunk/src/conf/classes/ScarabBundle_en.properties	(original)
+++ trunk/src/conf/classes/ScarabBundle_en.properties	2009-05-31 08:15:56-0700
@@ -242,6 +242,25 @@
 ActivityAt      = at
 ActivityReasons = Reasons for the changes

+NotificationIssueCreated=new
+NotificationIssueMoved=moved
+NotificationIssueCopied=copied
+NotificationIssueDeleted�leted
+NotificationCommentAdded=comment
+NotificationCommentChanged=comment
+NotificationAttributeChanged=attribute
+NotificationUserAttributeChanged=personel
+NotificationURLAdded=reference
+NotificationURLCHanged=reference
+NotificationURLDESCCHanged=reference
+NotificationURLDeleted=reference
+NotificationAttachmentCreated=attachment
+NotificationAttachmentRemoved=attachment
+NotificationDependencyCreated�pendency
+NotificationDependencyChanged�pendency
+NotificationDependencyDeleted�pendency
+NotificationIssueOther=?
+
 ##ButtonLabels
 Filter=Filter
 AssociateSelectedUserWithIssue=Associate selected users with issue
@@ -1535,7 +1554,7 @@
 ## ***********************************************************************

 # ActivitySet
-DefaultModifyIssueEmailSubject=[{0}] Issue #{1} ({2}) {3}
+DefaultModifyIssueEmailSubject=[{0}] #{1} ({2}) {3}
 MinimalModifyIssueEmailSubject=Issue #{0} modified

 # AbstractScarabModule

Modified: trunk/src/java/org/tigris/scarab/notification/ActivityType.java
Url: http://scarab.tigris.org/source/browse/scarab/trunk/src/java/org/tigris/scarab/notification/ActivityType.java?view=diff&pathrev738&r1737&r2738
=============================================================================--- trunk/src/java/org/tigris/scarab/notification/ActivityType.java	(original)
+++ trunk/src/java/org/tigris/scarab/notification/ActivityType.java	2009-05-31 08:15:56-0700
@@ -6,32 +6,35 @@

 public class ActivityType
 {
-    String code = null;
-    String resourceId = null;
-
-    public static final ActivityType ISSUE_CREATED = new ActivityType("issue_created","ActivityTypeIssueCreated");
-    public static final ActivityType ISSUE_MOVED = new ActivityType("issue_moved","ActivityTypeIssueMoved");
-    public static final ActivityType ISSUE_COPIED = new ActivityType("issue_copied","ActivityTypeIssueCopied");
-    public static final ActivityType ISSUE_DELETED = new ActivityType("issue_deleted","ActivityTypeIssueDeleted");
-    public static final ActivityType ATTRIBUTE_CHANGED = new ActivityType("attribute_changed","ActivityTypeAttributeChanged");
-    public static final ActivityType USER_ATTRIBUTE_CHANGED = new ActivityType("user_attribute_changed","ActivityTypeUserAttributeChanged");
-    public static final ActivityType COMMENT_ADDED = new ActivityType("comment_added","ActivityTypeCommentAdded");
-    public static final ActivityType COMMENT_CHANGED = new ActivityType("comment_changed","ActivityTypeCommentChanged");
-    public static final ActivityType URL_ADDED = new ActivityType("url_added","ActivityTypeURLAdded");
-    public static final ActivityType URL_CHANGED = new ActivityType("url_changed","ActivityTypeURLChanged");
-    public static final ActivityType URL_DESC_CHANGED = new ActivityType("url_desc_changed","ActivityTypeURLDescChanged");
-    public static final ActivityType URL_DELETED = new ActivityType("url_deleted","ActivityTypeURLDeleted");
-    public static final ActivityType ATTACHMENT_CREATED = new ActivityType("attachment_created","ActivityTypeAttachmentCreated");
-    public static final ActivityType ATTACHMENT_REMOVED = new ActivityType("attachment_removed","ActivityTypeAttachmentRemoved");
-    public static final ActivityType DEPENDENCY_CREATED = new ActivityType("dependency_created","ActivityTypeDependencyCreated");
-    public static final ActivityType DEPENDENCY_CHANGED = new ActivityType("dependency_changed","ActivityTypeDependencyChanged");
-    public static final ActivityType DEPENDENCY_DELETED = new ActivityType("dependency_deleted","ActivityTypeDependencyDeleted");
-    public static final ActivityType OTHER = new ActivityType("other","ActivityTypeOther");
+    String code           = null;
+    String resourceId     = null;
+    String notificationId = null;
+    Integer notificationPriority      = null;
+
+    public static final ActivityType ISSUE_CREATED = new ActivityType("issue_created","ActivityTypeIssueCreated", "NotificationIssueCreated", 17);
+    public static final ActivityType ISSUE_MOVED = new ActivityType("issue_moved","ActivityTypeIssueMoved", "NotificationIssueMoved", 16);
+    public static final ActivityType ISSUE_COPIED = new ActivityType("issue_copied","ActivityTypeIssueCopied", "NotificationIssueCopied", 15);
+    public static final ActivityType ISSUE_DELETED = new ActivityType("issue_deleted","ActivityTypeIssueDeleted", "NotificationIssueDeleted", 14);
+    public static final ActivityType COMMENT_ADDED = new ActivityType("comment_added","ActivityTypeCommentAdded", "NotificationCommentAdded", 13);
+    public static final ActivityType COMMENT_CHANGED = new ActivityType("comment_changed","ActivityTypeCommentChanged", "NotificationCommentChanged", 12);
+    public static final ActivityType ATTRIBUTE_CHANGED = new ActivityType("attribute_changed","ActivityTypeAttributeChanged", "NotificationAttributeChanged", 11);
+    public static final ActivityType USER_ATTRIBUTE_CHANGED = new ActivityType("user_attribute_changed","ActivityTypeUserAttributeChanged", "NotificationUserAttributeChanged", 10);
+    public static final ActivityType URL_ADDED = new ActivityType("url_added","ActivityTypeURLAdded", "NotificationURLAdded", 9);
+    public static final ActivityType URL_CHANGED = new ActivityType("url_changed","ActivityTypeURLChanged", "NotificationURLCHanged", 8);
+    public static final ActivityType URL_DESC_CHANGED = new ActivityType("url_desc_changed","ActivityTypeURLDescChanged", "NotificationURLDESCCHanged", 7);
+    public static final ActivityType URL_DELETED = new ActivityType("url_deleted","ActivityTypeURLDeleted", "NotificationURLDeleted", 6);
+    public static final ActivityType ATTACHMENT_CREATED = new ActivityType("attachment_created","ActivityTypeAttachmentCreated", "NotificationAttachmentCreated", 5);
+    public static final ActivityType ATTACHMENT_REMOVED = new ActivityType("attachment_removed","ActivityTypeAttachmentRemoved", "NotificationAttachmentRemoved", 4);
+    public static final ActivityType DEPENDENCY_CREATED = new ActivityType("dependency_created","ActivityTypeDependencyCreated", "NotificationDependencyCreated", 3);
+    public static final ActivityType DEPENDENCY_CHANGED = new ActivityType("dependency_changed","ActivityTypeDependencyChanged", "NotificationDependencyChanged", 2);
+    public static final ActivityType DEPENDENCY_DELETED = new ActivityType("dependency_deleted","ActivityTypeDependencyDeleted", "NotificationDependencyDeleted", 1);
+    public static final ActivityType OTHER = new ActivityType("other","ActivityTypeOther", "NotificationIssueOther", 0);

     private static Map types       = new HashMap();
-
+    private static HashMap activityPriority;
     static
     {
+
         types.put(ISSUE_CREATED.getCode(), ISSUE_CREATED);
         types.put(ISSUE_MOVED.getCode(), ISSUE_MOVED);
         types.put(ISSUE_COPIED.getCode(), ISSUE_COPIED);
@@ -51,6 +54,26 @@
         types.put(DEPENDENCY_DELETED.getCode(), DEPENDENCY_DELETED);

     }
+
+    /**
+     * Return the L10N Id for the Notification hint
+     * @return
+     */
+    public String getHint() {
+        return notificationId;
+    }
+
+    /**
+     * Return the reporting priority.
+     * higher values == higher priority
+     * Currently only used for Notifications.
+     * @param at
+     * @return
+     */
+    public int getPriority()
+    {
+        return notificationPriority;
+    }

     /**
      * Return an iterator over all available ActivityType codes.
@@ -77,10 +100,12 @@
         return this.code;
     }

-    private ActivityType(String desc, String resourceId)
+    private ActivityType(String desc, String resourceId, String notificationId, Integer notificationPriority)
     {
         this.code = desc;
         this.resourceId = resourceId;
+        this.notificationId = notificationId;
+        this.notificationPriority = notificationPriority;
     }

     /**
@@ -107,4 +132,5 @@
     {
         return getActivityType(code).resourceId;
     }
+
 }

Modified: trunk/src/java/org/tigris/scarab/notification/ScarabNewNotificationManager.java
Url: http://scarab.tigris.org/source/browse/scarab/trunk/src/java/org/tigris/scarab/notification/ScarabNewNotificationManager.java?view=diff&pathrev738&r1737&r2738
=============================================================================--- trunk/src/java/org/tigris/scarab/notification/ScarabNewNotificationManager.java	(original)
+++ trunk/src/java/org/tigris/scarab/notification/ScarabNewNotificationManager.java	2009-05-31 08:15:56-0700
@@ -228,43 +228,25 @@
             issueActivities.clear();
             archiverActivities.clear();
             creators.clear();
-            firstNotification        = null;
-            lastNotification         = null;
-            Long issueTime           = null;
-            String changedStatusAttributeValue = "";
+            firstNotification                     = null;
+            lastNotification                      = null;
+            Long issueTime                        = null;
+            NotificationStatus mostRelevantNotification = null;

-            Set notificationSet = (Set)pendingIssueMap.get(issue);
+            List notificationList = (List)pendingIssueMap.get(issue);

             //Process each Notification for current Issue ...
-            for (Iterator it = notificationSet.iterator(); it.hasNext();)
+            for (Iterator it = notificationList.iterator(); it.hasNext();)
             {
                 NotificationStatus currentNotification = (NotificationStatus) it.next();
-
-                ActivityType activityType = currentNotification.getActivityType();
-                if(changedStatusAttributeValue.length() == 0 && activityType.equals(ActivityType.ATTRIBUTE_CHANGED))
+                if(firstNotification == null)
                 {
-                    try
-                    {
-                        Attribute attribute = currentNotification.getActivity().getAttribute();
-                        if (getIsStatusAttribute(attribute, issue))
-                        {
-                            String name = attribute.getName();
-                            AttributeValue av = issue.getAttributeValue(name);
-                            if(av != null)
-                            {
-                                changedStatusAttributeValue = av.getValue();
-                            }
-                        }
-                    }
-                    catch (TorqueException e)
-                    {
-                        Log.get().warn("Database acess error while retrieving status attribute value.(ignored)");
-                        Log.get().warn("db layer reported: ["+e.getMessage()+"]");
-                    }
+                    firstNotification = currentNotification;
                 }
-
-                firstNotification = getOldestNotification(currentNotification, firstNotification);
-                lastNotification  = getYoungestNotification(currentNotification, lastNotification);
+                lastNotification = currentNotification;
+
+                mostRelevantNotification = getMostRelevantNotification(currentNotification, mostRelevantNotification, issue);
+
                 try
                 {
                     issueId = issue.getUniqueId();
@@ -311,7 +293,7 @@
                     ectx.put("creators", creators);
                     ectx.put("firstNotification", firstNotification);
                     ectx.put("lastNotification", lastNotification);
-                    ectx.put("changedStatus",changedStatusAttributeValue);
+                    ectx.put("changeHint",getChangeHint(mostRelevantNotification, issue));

                     Map groupedActivities = (Map) issueActivities.get(user);
                     if(groupedActivities == null)
@@ -344,6 +326,90 @@
     }


+    private Object getChangeHint(NotificationStatus notification, Issue issue)
+    {
+        String result = null;
+        ActivityType activityType = notification.getActivityType();
+        if(activityType.equals(ActivityType.ATTRIBUTE_CHANGED))
+        {
+            try
+            {
+                Attribute attribute = notification.getActivity().getAttribute();
+                if (getIsStatusAttribute(attribute, issue))
+                {
+                    String name = attribute.getName();
+                    AttributeValue av = issue.getAttributeValue(name);
+                    if(av != null)
+                    {
+                        result = av.getValue();
+                    }
+                }
+            }
+            catch (TorqueException e)
+            {
+                Log.get().warn("Database acess error while retrieving status attribute value.(ignored)");
+                Log.get().warn("db layer reported: ["+e.getMessage()+"]");
+            }
+        }
+
+        if(result == null)
+        {
+            result = activityType.getHint();
+        }
+
+        return result;
+    }
+
+
+    private NotificationStatus getMostRelevantNotification(
+            NotificationStatus currentNotification,
+            NotificationStatus mostRelevantNotification,
+            Issue issue)
+    {
+        ActivityType currentActivityType      = currentNotification.getActivityType();
+
+        // ====================================================================+        // Check if the Issue status has changed. This is of highest relevance.
+        // ====================================================================+        if(currentActivityType.equals(ActivityType.ATTRIBUTE_CHANGED))
+        {
+            try
+            {
+                Attribute attribute = currentNotification.getActivity().getAttribute();
+                if (getIsStatusAttribute(attribute, issue))
+                {
+                    String name = attribute.getName();
+                    AttributeValue av = issue.getAttributeValue(name);
+                    if(av != null)
+                    {
+                        return currentNotification; // that is the most relevant notification!
+                    }
+                }
+            }
+            catch (TorqueException e)
+            {
+                Log.get().warn("Database acess error while retrieving status attribute value.(ignored)");
+                Log.get().warn("db layer reported: ["+e.getMessage()+"]");
+            }
+        }
+
+        if(mostRelevantNotification == null)
+        {
+            mostRelevantNotification = currentNotification;
+        }
+        else
+        {
+            ActivityType mostRelevantActivityType = mostRelevantNotification.getActivityType();
+            if  (  currentActivityType.getPriority() > mostRelevantActivityType.getPriority() )
+            {
+                mostRelevantNotification = currentNotification;
+            }
+        }
+        return mostRelevantNotification;
+    }
+
+
+
     /**
      * This method returns true, if the attribute is identified as
      * the "status_attribute" for the current module/issue_type combination.
@@ -416,23 +482,6 @@
     }

     /**
-     * Return the Notification which is the youngest of n1,n2
-     * Note: If one of the notificaitons is null, return the other.
-     *       If both notificaitons are null, return null
-     * @param n1
-     * @param n2
-     * @return
-     */
-    private NotificationStatus getYoungestNotification(NotificationStatus n1, NotificationStatus n2)
-    {
-        if(n1==null) return n2;
-        if(n2==null) return n1;
-        int compare = compareCreationDates(n1, n2);
-        NotificationStatus result = (compare > 0) ? n1:n2;
-        return result;
-    }
-
-    /**
      * Return a list of strings with the reasons for the activities to be
      * notified.
      *
@@ -459,70 +508,6 @@
         return list;
     }

-    /**
-     * Return the Notification which is the oldest of n1, n2
-     * Note: If one of the notificaitons is null, return the other.
-     *       If both notificaitons are null, return null
-     * @param n1
-     * @param n2
-     * @return
-     */
-    private NotificationStatus getOldestNotification(NotificationStatus n1, NotificationStatus n2)
-    {
-        if(n1==null) return n2;
-        if(n2==null) return n1;
-        int compare = compareCreationDates(n1, n2);
-        NotificationStatus result = (compare < 0) ? n1:n2;
-        return result;
-    }
-
-    /*
-     * Compares the creation dates of two notifications.
-     * returns:
-     * -1 : n1.date < n2.date
-     *  0 : n1.date == n2.date
-     * +1 : n1.date > n2.date
-     *
-     * If both entries are null, they are reported as equal (0)
-     * If one of the entries is null, its creation date is
-     * assumed to be "older than everything else".
-     * Thrws a ScarabRuntimeException when one of the entries
-     * has no CreationDate.
-     */
-    private int compareCreationDates(NotificationStatus n1,
-                                     NotificationStatus n2)
-    {
-        // handle null entries:
-        if(n1==n2)   return 0;
-        if(n1==null) return -1;
-        if(n2==null) return +1;
-
-        int result;
-        try
-        {
-            long n1d = n1.getCreationDate().getTime();
-            long n2d = n2.getCreationDate().getTime();
-
-            if (n1d == n2d)
-            {
-                result = 0;
-            }
-            else
-            {
-                result = (n1d > n2d) ? 1 : -1;
-            }
-        }
-        catch (NullPointerException npe)
-        {
-            L10NMessage msg = new L10NMessage(
-                    L10NKeySet.NotificationStatusNoCreationDate);
-            log.warn(msg);
-            throw new ScarabRuntimeException(msg, npe);
-        }
-
-        return result;
-    }
-

     /**
      * Update the Notification status in the database. If exception is
@@ -677,13 +662,13 @@
                 if (notification.getActivity().getIssue().equals(issue))
                 {

-                    Set notificationSet = (Set)issueMap.get(issue);
-                    if(notificationSet == null)
+                    List notificationList = (List)issueMap.get(issue);
+                    if(notificationList == null)
                     {
-                        notificationSet = new HashSet();
-                        issueMap.put(issue,notificationSet);
+                        notificationList = new ArrayList();
+                        issueMap.put(issue,notificationList);
                     }
-                    notificationSet.add(notification);
+                    notificationList.add(notification);
                 }
             }
             catch (TorqueException te)

Modified: trunk/src/java/org/tigris/scarab/om/NotificationStatusPeer.java
Url: http://scarab.tigris.org/source/browse/scarab/trunk/src/java/org/tigris/scarab/om/NotificationStatusPeer.java?view=diff&pathrev738&r1737&r2738
=============================================================================--- trunk/src/java/org/tigris/scarab/om/NotificationStatusPeer.java	(original)
+++ trunk/src/java/org/tigris/scarab/om/NotificationStatusPeer.java	2009-05-31 08:15:56-0700
@@ -60,6 +60,9 @@
 {
     /**
      * Returns the list of pending notifications (those with State=SCHEDULED)
+     * ordered by Creation date. So we can be sure, the first Notification in a set
+     * is always the oldest notification, while the last notification in the set
+     * is always the youngest notification.
      *
      * @return
      */
@@ -68,10 +71,11 @@
 		List pending = null;
 		Criteria crit = new Criteria();
 		crit.add(NotificationStatusPeer.STATUS, NotificationStatus.SCHEDULED, Criteria.EQUAL);
+		crit.addAscendingOrderByColumn(NotificationStatusPeer.CREATION_DATE);
 		try
         {
 			pending = doSelect(crit);
-            Collections.sort(pending);
+            //Collections.sort(pending);
 		} catch (TorqueException e)
         {
 			log.error("getPendingNotifications(): " + e);

Modified: trunk/src/webapp/WEB-INF/templates/email/notification/IssueActivitySubject.vm
Url: http://scarab.tigris.org/source/browse/scarab/trunk/src/webapp/WEB-INF/templates/email/notification/IssueActivitySubject.vm?view=diff&pathrev738&r1737&r2738
=============================================================================--- trunk/src/webapp/WEB-INF/templates/email/notification/IssueActivitySubject.vm	(original)
+++ trunk/src/webapp/WEB-INF/templates/email/notification/IssueActivitySubject.vm	2009-05-31 08:15:56-0700
@@ -1 +1 @@
-#set ($args = [$issue.Module.RealName.toUpperCase(), $issue.UniqueId, $l10n.getIgnoreMissingResource($changedStatus), $issue.defaultText]) $l10n.format("DefaultModifyIssueEmailSubject", $args)
\ No newline at end of file
+#set ($args = [$issue.Module.RealName.toUpperCase(), $issue.UniqueId, $l10n.getIgnoreMissingResource($changeHint), $issue.defaultText]) $l10n.format("DefaultModifyIssueEmailSubject", $args)
\ No newline at end of file

------------------------------------------------------
http://scarab.tigris.org/ds/viewMessage.do?dsForumId=3577&dsMessageId=2357278
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.