doortts doortts 2018-03-02
noti: Polish notification features
- Refactor methods
- Change messages
- Reduce notification scope of new commit codes
@ff95d5e5ec15dc9091e8a8d3fd1bd238afa50350
app/actors/CommitsNotificationActor.java
--- app/actors/CommitsNotificationActor.java
+++ app/actors/CommitsNotificationActor.java
@@ -1,23 +1,10 @@
 /**
- * Yobi, Project Hosting SW
- *
- * Copyright 2013 NAVER Corp.
- * http://yobi.io
- *
- * @author Keesun Baik
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+ * Yona, 21st Century Project Hosting SW
+ * <p>
+ * Copyright Yona & Yobi Authors & NAVER Corp. & NAVER LABS Corp.
+ * https://yona.io
+ **/
+
 package actors;
 
 import models.*;
@@ -29,8 +16,6 @@
 
 /**
  * Creates new commit notifications.
- *
- * @author Keesun Baik
  */
 public class CommitsNotificationActor extends PostReceiveActor {
 
@@ -50,10 +35,7 @@
             title = Messages.get("notification.pushed.commits", project.name, commits.size());
         }
 
-        Set<User> watchers = Watch.findWatchers(project.asResource());
-        watchers.remove(sender);
-
-        NotificationEvent.afterNewCommits(commits, refNames, project, sender, title, watchers);
+        NotificationEvent.afterNewCommits(commits, refNames, project, sender, title);
     }
 
 }
app/controllers/IssueApp.java
--- app/controllers/IssueApp.java
+++ app/controllers/IssueApp.java
@@ -900,16 +900,16 @@
     private static Comment saveComment(Project project, Issue issue, IssueComment comment) {
         Comment savedComment;
         IssueComment existingComment = IssueComment.find.where().eq("id", comment.id).findUnique();
-        if (existingComment != null) {
+        if (existingComment == null) {
+            comment.projectId = project.id;
+            savedComment = saveComment(comment, getContainerUpdater(issue, comment));
+            NotificationEvent.afterNewComment(savedComment);
+        } else {
             existingComment.contents = comment.contents;
             savedComment = saveComment(existingComment, getContainerUpdater(issue, comment));
             if(isSelectedToSendNotificationMail() || !existingComment.isAuthoredBy(UserApp.currentUser())){
                 NotificationEvent.afterCommentUpdated(savedComment);
             }
-        } else {
-            comment.projectId = project.id;
-            savedComment = saveComment(comment, getContainerUpdater(issue, comment));
-            NotificationEvent.afterNewComment(savedComment);
         }
         return savedComment;
     }
app/controllers/WatchProjectApp.java
--- app/controllers/WatchProjectApp.java
+++ app/controllers/WatchProjectApp.java
@@ -60,12 +60,12 @@
         UserProjectNotification userProjectNotification = findOne(user, project, notiType);
         if(userProjectNotification == null) { // not specified yet
             if (isNotifiedByDefault(notiType)) {
-                watchExplictly(user, project, notiType);
-            } else {
                 unwatchExplictly(user, project, notiType);
+            } else {
+                watchExplictly(user, project, notiType);
             }
         } else {
-            userProjectNotification.toggle();
+            userProjectNotification.toggle(notiType);
         }
 
         return ok();
app/models/NotificationEvent.java
--- app/models/NotificationEvent.java
+++ app/models/NotificationEvent.java
@@ -39,15 +39,14 @@
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
-import java.util.Date;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static models.UserProjectNotification.findEventUnwatchersByEventType;
 import static models.UserProjectNotification.findEventWatchersByEventType;
+import static models.Watch.findUnwatchers;
 import static models.Watch.findWatchers;
 import static models.enumeration.EventType.*;
 
@@ -733,11 +732,7 @@
         NotificationEvent notiEvent = createFrom(author, comment);
         notiEvent.title = formatReplyTitle(post);
         notiEvent.eventType = eventType;
-        Set<User> receivers = getCommentReceivers(comment, author);
-        receivers.addAll(findEventWatchersByEventType(comment.projectId, eventType));
-        receivers.addAll(getMentionedUsers(comment.contents));
-        receivers.remove(author);
-        notiEvent.receivers = receivers;
+        notiEvent.receivers = getMandatoryReceivers(comment, eventType);
         notiEvent.oldValue = null;
         notiEvent.newValue = comment.contents;
         notiEvent.resourceType = comment.asResource().getType();
@@ -776,7 +771,7 @@
 
         NotificationEvent notiEvent = createFromCurrentUser(issue);
         notiEvent.title = formatReplyTitle(issue);
-        notiEvent.receivers = getMandatoryReceivers(issue);
+        notiEvent.receivers = getMandatoryReceivers(issue, EventType.ISSUE_STATE_CHANGED);
         notiEvent.eventType = ISSUE_STATE_CHANGED;
         notiEvent.oldValue = oldState != null ? oldState.state() : null;
         notiEvent.newValue = issue.state.state();
@@ -842,7 +837,7 @@
     }
 
     private static Set<User> getReceiversWhenAssigneeChanged(User oldAssignee, Issue issue) {
-        Set<User> receivers = getMandatoryReceivers(issue);
+        Set<User> receivers = getMandatoryReceivers(issue, ISSUE_ASSIGNEE_CHANGED);
 
         if (oldAssignee != null && !oldAssignee.isAnonymous()
                 && !oldAssignee.loginId.equals(UserApp.currentUser().loginId)) {
@@ -953,7 +948,7 @@
 
         NotificationEvent notiEvent = createFromCurrentUser(issue);
 
-        Set<User> receivers = getMandatoryReceivers(issue);
+        Set<User> receivers = getMandatoryReceivers(issue, ISSUE_MILESTONE_CHANGED);
 
         notiEvent.title = formatReplyTitle(issue);
         notiEvent.receivers = receivers;
@@ -966,7 +961,7 @@
         return notiEvent;
     }
 
-    private static Set<User> getMandatoryReceivers(Issue issue) {
+    private static Set<User> getMandatoryReceivers(Issue issue, EventType eventType) {
         Set<User> receivers = findWatchers(issue.asResource());
         receivers.add(issue.getAuthor());
 
@@ -978,22 +973,74 @@
             receivers.add(issue.assignee.user);
         }
 
+        receivers.addAll(findWatchers(issue.asResource()));
+        receivers.addAll(findEventWatchersByEventType(issue.project.id, eventType));
+
+        receivers.removeAll(findUnwatchers(issue.asResource()));
+        receivers.removeAll(findEventUnwatchersByEventType(issue.project.id, eventType));
         receivers.remove(UserApp.currentUser());
 
         return receivers;
     }
 
-    private static Set<User> getMandatoryReceivers(Posting posting) {
+    private static Set<User> getMandatoryReceivers(Posting posting, EventType eventType) {
         Set<User> receivers = findWatchers(posting.asResource());
         receivers.add(posting.getAuthor());
+        receivers.addAll(findWatchers(posting.asResource()));
+        receivers.addAll(findEventWatchersByEventType(posting.project.id, eventType));
 
+        receivers.removeAll(findUnwatchers(posting.asResource()));
+        receivers.removeAll(findEventUnwatchersByEventType(posting.project.id, eventType));
         receivers.remove(UserApp.currentUser());
 
         return receivers;
+    }
+
+    private static Set<User> getMandatoryReceivers(Comment comment, EventType eventType) {
+        AbstractPosting parent = comment.getParent();
+        Set<User> receivers = findWatchers(parent.asResource());
+        receivers.add(parent.getAuthor());
+        receivers.addAll(findEventWatchersByEventType(comment.projectId, eventType));
+        receivers.addAll(getMentionedUsers(comment.contents));
+        includeAssigneeIfExist(comment, receivers);
+
+        receivers.removeAll(findUnwatchers(parent.asResource()));
+        receivers.removeAll(findEventUnwatchersByEventType(comment.projectId, eventType));
+        receivers.remove(UserApp.currentUser());
+
+        return receivers;
+    }
+
+    private static Set<User> getProjectCommitReceivers(Project project, EventType eventType) {
+        Set<User> receivers = findMembersOnlyFromWatchers(project);
+        receivers.removeAll(findUnwatchers(project.asResource()));
+        receivers.removeAll(findEventUnwatchersByEventType(project.id, eventType));
+        receivers.remove(UserApp.currentUser());
+
+        return receivers;
+    }
+
+    private static Set<User> findMembersOnlyFromWatchers(Project project) {
+        Set<User> receivers = new HashSet<>();
+        Set<User> projectMembers = extractMembers(project);
+        for (User watcher : findWatchers(project.asResource())) {
+            if (projectMembers.contains(watcher)) {
+                receivers.add(watcher);
+            }
+        }
+        return receivers;
+    }
+
+    private static Set<User> extractMembers(Project project) {
+        Set<User> projectMembers = new HashSet<>();
+        for (ProjectUser projectUser : project.members()) {
+            projectMembers.add(projectUser.user);
+        }
+        return projectMembers;
     }
 
     private static Set<User> getReceiversForIssueBodyChanged(String oldBody, Issue issue) {
-        Set<User> receivers = getMandatoryReceivers(issue);
+        Set<User> receivers = getMandatoryReceivers(issue, ISSUE_BODY_CHANGED);
         receivers.addAll(getNewMentionedUsers(oldBody, issue.body));
         receivers.remove(UserApp.currentUser());
         return receivers;
@@ -1020,7 +1067,7 @@
     public static NotificationEvent forUpdatePosting(String oldValue, Posting post, User author) {
         NotificationEvent notiEvent = createFrom(author, post);
         notiEvent.title = formatNewTitle(post);
-        notiEvent.receivers = getMandatoryReceivers(post);
+        notiEvent.receivers = getMandatoryReceivers(post, EventType.POSTING_BODY_CHANGED);
         notiEvent.eventType = POSTING_BODY_CHANGED;
         notiEvent.oldValue = oldValue;
         notiEvent.newValue = post.body;
@@ -1133,10 +1180,10 @@
         NotificationEvent.add(notiEvent);
     }
 
-    public static void afterNewCommits(List<RevCommit> commits, List<String> refNames, Project project, User sender, String title, Set<User> watchers) {
+    public static void afterNewCommits(List<RevCommit> commits, List<String> refNames, Project project, User sender, String title) {
         NotificationEvent notiEvent = createFrom(sender, project);
         notiEvent.title = title;
-        notiEvent.receivers = watchers;
+        notiEvent.receivers = getProjectCommitReceivers(project, NEW_COMMIT);
         notiEvent.eventType = NEW_COMMIT;
         notiEvent.oldValue = null;
         notiEvent.newValue = newCommitsMessage(commits, refNames, project);
@@ -1230,17 +1277,6 @@
         Set<User> receivers = abstractPosting.getWatchers();
         receivers.addAll(getMentionedUsers(abstractPosting.body));
         receivers.remove(except);
-        return receivers;
-    }
-
-    private static Set<User> getCommentReceivers(Comment comment, User except) {
-        AbstractPosting parent = comment.getParent();
-
-        Set<User> receivers = new HashSet<>(findWatchers(parent.asResource()));
-        receivers.add(comment.getParent().getAuthor());
-        includeAssigneeIfExist(comment, receivers);
-        receivers.remove(except);
-
         return receivers;
     }
 
app/models/UserProjectNotification.java
--- app/models/UserProjectNotification.java
+++ app/models/UserProjectNotification.java
@@ -13,9 +13,7 @@
 import java.util.*;
 
 /**
- * User this class when someone want to know whether a user is receiving notification alarm from the project or not
- *
- * @author Keesun Baik
+ * Project notification subscribing settings with events which are customized by user
  */
 @Entity
 @Table(uniqueConstraints = @UniqueConstraint(columnNames = {"project_id", "user_id", "notification_type"}))
@@ -96,9 +94,13 @@
                 .findUnique();
     }
 
-    public void toggle() {
+    public void toggle(EventType notificationType) {
         this.allowed = !this.allowed;
-        update();
+        if (allowed == isNotifiedByDefault(notificationType)) {
+            delete();
+        } else {
+            update();
+        }
     }
 
     public static void unwatchExplictly(User user, Project project, EventType notiType) {
@@ -144,10 +146,18 @@
     }
 
     public static Set<User> findEventWatchersByEventType(Long projectId, EventType eventType) {
+        return findByEventTypeAndOption(projectId, eventType, true);
+    }
+
+    public static Set<User> findEventUnwatchersByEventType(Long projectId, EventType eventType) {
+        return findByEventTypeAndOption(projectId, eventType, false);
+    }
+
+    private static Set<User> findByEventTypeAndOption(Long projectId, EventType eventType, boolean isAllowd) {
         List<UserProjectNotification> userProjectNotifications = find.where()
                 .eq("project.id", projectId)
                 .eq("notificationType", eventType)
-                .eq("allowed", true)
+                .eq("allowed", isAllowd)
                 .findList();
         Set<User> users = new LinkedHashSet<>();
         for (UserProjectNotification notification : userProjectNotifications) {
conf/messages
--- conf/messages
+++ conf/messages
@@ -483,7 +483,7 @@
 notification.type.pullrequest.reviewed = Pull request review completed.
 notification.type.pullrequest.state.changed = Pull request Status changed.
 notification.type.pullrequest.unreviewed = Pull request review is canceled.
-notification.type.resource.deleted = Deleted
+notification.type.resource.deleted = Issue/Posting deletion
 notification.type.review.state.changed = Review Thread State Change
 notification.unwatch = Unwatch
 notification.watch = Watch
conf/messages.ko-KR
--- conf/messages.ko-KR
+++ conf/messages.ko-KR
@@ -484,7 +484,7 @@
 notification.type.pullrequest.reviewed = 코드보내기 리뷰가 완료되었습니다.
 notification.type.pullrequest.state.changed = 코드보내기 상태 변경
 notification.type.pullrequest.unreviewed = 코드보내기 리뷰가 취소되었습니다.
-notification.type.resource.deleted = 삭제되었습니다
+notification.type.resource.deleted = 이슈/게시글 삭제
 notification.type.review.state.changed = 리뷰 스레드 상태 변경
 notification.unwatch = 그만 지켜보기
 notification.watch = 지켜보기
Add a comment
List