Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

Commit

Permalink
notification: Fix emails with same message-id
Browse files Browse the repository at this point in the history
Notification emails for updated comment had same message-id with the
event when the comment was created because event type for updated
comment was NEW_COMMENT. It was incorrect behavior which possibily
causes that MUAs trash the emails.

Add COMMENT_UPDATED event and use it for updated comments.
  • Loading branch information
Yi EungJun committed Mar 11, 2015
1 parent 7cdea2e commit 01eb2fc
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 25 deletions.
14 changes: 3 additions & 11 deletions app/controllers/AbstractPostingApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,30 +51,22 @@ public SearchCondition() {
}
}

public static Result saveComment(final Comment comment, Form<? extends Comment> commentForm, final Call toView, Runnable containerUpdater) {
if (commentForm.hasErrors()) {
flash(Constants.WARNING, "post.comment.empty");
return redirect(toView);
}

public static Comment saveComment(final Comment comment, Runnable containerUpdater) {
containerUpdater.run(); // this updates comment.issue or comment.posting;

if(comment.id != null && AccessControl.isAllowed(UserApp.currentUser(), comment.asResource(), Operation.UPDATE)) {
comment.update();
} else {
comment.setAuthor(UserApp.currentUser());
comment.save();
}


// Attach all of the files in the current user's temporary storage.
attachUploadFilesToPost(comment.asResource());

String urlToView = RouteUtil.getUrl(comment);
NotificationEvent.afterNewComment(comment);
return redirect(urlToView);
return comment;
}


protected static Result delete(Model target, Resource resource, Call redirectTo) {
if (!AccessControl.isAllowed(UserApp.currentUser(), resource, Operation.DELETE)) {
return forbidden(ErrorViews.Forbidden.render("error.forbidden", resource.getProject()));
Expand Down
21 changes: 14 additions & 7 deletions app/controllers/BoardApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,7 @@
import playRepository.BareCommit;
import playRepository.BareRepository;
import playRepository.RepositoryService;
import utils.AccessControl;
import utils.ErrorViews;
import utils.JodaDateUtil;
import utils.MenuType;
import utils.*;
import views.html.board.create;
import views.html.board.edit;
import views.html.board.list;
Expand Down Expand Up @@ -272,7 +269,6 @@ public static Result deletePost(String owner, String projectName, Long number) {
public static Result newComment(String owner, String projectName, Long number) throws IOException {
Project project = Project.findByOwnerAndProjectName(owner, projectName);
final Posting posting = Posting.findByNumber(project, number);
Call redirectTo = routes.BoardApp.post(project.owner, project.name, number);
Form<PostingComment> commentForm = new Form<>(PostingComment.class)
.bindFromRequest();

Expand All @@ -287,12 +283,23 @@ public static Result newComment(String owner, String projectName, Long number) t

final PostingComment comment = commentForm.get();
PostingComment existingComment = PostingComment.find.where().eq("id", comment.id).findUnique();

if (commentForm.hasErrors()) {
flash(Constants.WARNING, "common.comment.empty");
return redirect(routes.BoardApp.post(project.owner, project.name, number));
}

Comment savedComment;
if (existingComment != null) {
existingComment.contents = comment.contents;
return saveComment(existingComment, commentForm, redirectTo, getContainerUpdater(posting, comment));
savedComment = saveComment(existingComment, getContainerUpdater(posting, comment));
NotificationEvent.afterCommentUpdated(savedComment);
} else {
return saveComment(comment, commentForm, redirectTo, getContainerUpdater(posting, comment));
savedComment = saveComment(comment, getContainerUpdater(posting, comment));
NotificationEvent.afterNewComment(savedComment);
}

return redirect(RouteUtil.getUrl(savedComment));
}

private static Runnable getContainerUpdater(final Posting posting, final PostingComment comment) {
Expand Down
17 changes: 14 additions & 3 deletions app/controllers/IssueApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -602,12 +602,23 @@ public static Result newComment(String ownerName, String projectName, Long numbe
final IssueComment comment = commentForm.get();

IssueComment existingComment = IssueComment.find.where().eq("id", comment.id).findUnique();
if( existingComment != null){

if (commentForm.hasErrors()) {
flash(Constants.WARNING, "common.comment.empty");
return redirect(routes.IssueApp.issue(project.owner, project.name, number));
}

Comment savedComment;
if (existingComment != null) {
existingComment.contents = comment.contents;
return saveComment(existingComment, commentForm, redirectTo, getContainerUpdater(issue, comment));
savedComment = saveComment(existingComment, getContainerUpdater(issue, comment));
NotificationEvent.afterCommentUpdated(savedComment);
} else {
return saveComment(comment, commentForm, redirectTo, getContainerUpdater(issue, comment));
savedComment = saveComment(comment, getContainerUpdater(issue, comment));
NotificationEvent.afterNewComment(savedComment);
}

return redirect(RouteUtil.getUrl(savedComment));
}

private static Runnable getContainerUpdater(final Issue issue, final IssueComment comment) {
Expand Down
17 changes: 15 additions & 2 deletions app/models/NotificationEvent.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ public String getMessage(Lang lang) {
case NEW_PULL_REQUEST:
case NEW_COMMIT:
case ISSUE_BODY_CHANGED:
case COMMENT_UPDATED:
return newValue;
case NEW_REVIEW_COMMENT:
try {
Expand Down Expand Up @@ -535,20 +536,28 @@ public static void afterNewComment(Comment comment) {
NotificationEvent.add(forNewComment(comment, UserApp.currentUser()));
}

public static NotificationEvent forNewComment(Comment comment, User author) {
public static NotificationEvent forComment(Comment comment, User author, EventType eventType) {
AbstractPosting post = comment.getParent();

NotificationEvent notiEvent = createFrom(author, comment);
notiEvent.title = formatReplyTitle(post);
notiEvent.receivers = getReceivers(post, author);
notiEvent.eventType = NEW_COMMENT;
notiEvent.eventType = eventType;
notiEvent.oldValue = null;
notiEvent.newValue = comment.contents;
notiEvent.resourceType = comment.asResource().getType();
notiEvent.resourceId = comment.asResource().getId();
return notiEvent;
}

public static NotificationEvent forUpdatedComment(Comment comment, User author) {
return forComment(comment, author, COMMENT_UPDATED);
}

public static NotificationEvent forNewComment(Comment comment, User author) {
return forComment(comment, author, NEW_COMMENT);
}

public static void afterNewCommentWithState(Comment comment, State state) {
AbstractPosting post = comment.getParent();

Expand Down Expand Up @@ -1097,4 +1106,8 @@ public static int getNotificationsCount(User user) {

return find.setRawSql(RawSqlBuilder.parse(sql).create()).findList().size();
}

public static void afterCommentUpdated(Comment comment) {
NotificationEvent.add(forUpdatedComment(comment, UserApp.currentUser()));
}
}
3 changes: 2 additions & 1 deletion app/models/enumeration/EventType.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public enum EventType {
ISSUE_BODY_CHANGED("notification.type.issue.body.changed", 17),
ISSUE_REFERRED_FROM_PULL_REQUEST("notification.type.issue.referred.from.pullrequest", 16),
REVIEW_THREAD_STATE_CHANGED("notification.type.review.state.changed", 18),
ORGANIZATION_MEMBER_ENROLL_REQUEST("notification.organization.type.member.enroll",19);
ORGANIZATION_MEMBER_ENROLL_REQUEST("notification.organization.type.member.enroll",19),
COMMENT_UPDATED("notification.type.comment.updated", 20);

private String descr;

Expand Down
2 changes: 1 addition & 1 deletion conf/messages
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ common.comment.beforeunload.confirm = Would you like to exit this page without
common.comment.delete = Delete comment
common.comment.delete.confirm = Once you delete this comment, you won''t be able to recover it. Are you sure you want to delete this comment?
common.comment.edit = Edit comment
common.comment.empty = Comment is a required field.
common.comment.unvote = Withdraw
common.comment.vote = Agree
common.comment.vote.agreement = {0} Agreement
Expand Down Expand Up @@ -448,7 +449,6 @@ organization.settingFrom = Setting
organization.you.may.want.to.be.a.member = You may want to be a member of {0} group.
organization.you.want.to.be.a.member = You want to be a member of {0} group.
post.author = Author
post.comment.empty = Comment is a required field.
post.createdDate = Created date
post.delete.confirm = Once you delete the post, you won''t be able to recover it. Do you still want to delete this post?
post.error.beforeunload = This post has not been saved yet. Would you like to exit this page without saving?
Expand Down

0 comments on commit 01eb2fc

Please sign in to comment.