Skip to content

Commit

Permalink
fix: sorting problem using email comments (#3458)
Browse files Browse the repository at this point in the history
#### What type of PR is this?
/kind bug
/area core
/milestone 2.3.x

#### What this PR does / why we need it:
修复使用邮箱评论时 `spec.creationTime` 没有填充导致排序不正确

how to test it?
1. 迁移数据,看是否 `spec.creationTime` 与 `spec.approvedTime` 相等
2. 使用邮箱评论看是否在 Console 评论列表排在第一位

#### Which issue(s) this PR fixes:
Fixes #3456

#### Does this PR introduce a user-facing change?
```release-note
修复使用邮箱评论时的排序问题
```
  • Loading branch information
guqing committed Mar 8, 2023
1 parent 6ace362 commit 633c489
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,22 @@ public Mono<Comment> create(Comment comment) {
}
comment.getSpec()
.setApproved(Boolean.FALSE.equals(commentSetting.getRequireReviewForNew()));

if (BooleanUtils.isTrue(comment.getSpec().getApproved())
&& comment.getSpec().getApprovedTime() == null) {
comment.getSpec().setApprovedTime(Instant.now());
}

if (comment.getSpec().getCreationTime() == null) {
comment.getSpec().setCreationTime(Instant.now());
}

comment.getSpec().setHidden(false);

// return if the comment owner is not null
if (comment.getSpec().getOwner() != null) {
return Mono.just(comment);
}
if (comment.getSpec().getCreationTime() == null) {
comment.getSpec().setCreationTime(Instant.now());
}
// populate owner from current user
return fetchCurrentUser()
.map(this::toCommentOwner)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,6 @@ public static class BaseCommentSpec {

@Schema(required = true, defaultValue = "false")
private Boolean hidden;

/**
* If the creation time is null, use approvedTime.
*
* @return if creationTime is null returned approved time, otherwise creationTime.
*/
public Instant getCreationTime() {
return defaultIfNull(creationTime, approvedTime);
}
}

@Data
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package run.halo.app.core.extension.reconciler;

import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;

import java.time.Instant;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.springframework.lang.Nullable;
import org.springframework.stereotype.Component;
import run.halo.app.content.comment.ReplyService;
Expand Down Expand Up @@ -51,6 +53,7 @@ public Result reconcile(Request request) {
return;
}
addFinalizerIfNecessary(comment);
compatibleCreationTime(request.name());
reconcileStatus(request.name());
updateSameSubjectRefCommentCounter(comment.getSpec().getSubjectRef());
});
Expand All @@ -64,6 +67,28 @@ public Controller setupWith(ControllerBuilder builder) {
.build();
}

/**
* If the comment creation time is null, set it to the approved time or the current time.
* TODO remove this method in the future and fill in attributes in hook mode instead.
*
* @param name comment name
*/
void compatibleCreationTime(String name) {
client.fetch(Comment.class, name).ifPresent(comment -> {
Instant creationTime = comment.getSpec().getCreationTime();
Instant oldCreationTime =
creationTime == null ? null : Instant.ofEpochMilli(creationTime.toEpochMilli());
if (creationTime == null) {
creationTime = defaultIfNull(comment.getSpec().getApprovedTime(), Instant.now());
comment.getSpec().setCreationTime(creationTime);
}

if (!Objects.equals(oldCreationTime, comment.getSpec().getCreationTime())) {
client.update(comment);
}
});
}

private boolean isDeleted(Comment comment) {
return comment.getMetadata().getDeletionTimestamp() != null;
}
Expand All @@ -89,7 +114,7 @@ private void reconcileStatus(String name) {
client.fetch(Comment.class, name).ifPresent(comment -> {
Comment oldComment = JsonUtils.deepCopy(comment);
Comment.CommentStatus status = comment.getStatusOrDefault();
status.setHasNewReply(ObjectUtils.defaultIfNull(status.getUnreadReplyCount(), 0) > 0);
status.setHasNewReply(defaultIfNull(status.getUnreadReplyCount(), 0) > 0);
updateUnReplyCountIfNecessary(comment);
if (!oldComment.equals(comment)) {
client.update(comment);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.time.Instant;
import java.util.HashSet;
Expand Down Expand Up @@ -85,6 +86,29 @@ void reconcileDelete() {
.contains(CommentReconciler.FINALIZER_NAME)).isFalse();
}

@Test
void compatibleCreationTime() {
Comment comment = new Comment();
comment.setMetadata(new Metadata());
comment.getMetadata().setName("fake-comment");
comment.setSpec(new Comment.CommentSpec());
comment.getSpec().setApprovedTime(Instant.now());
comment.getSpec().setCreationTime(null);
when(client.fetch(eq(Comment.class), eq("fake-comment")))
.thenReturn(Optional.of(comment));

commentReconciler.compatibleCreationTime("fake-comment");

verify(client, times(1)).fetch(eq(Comment.class), eq("fake-comment"));

ArgumentCaptor<Comment> captor = ArgumentCaptor.forClass(Comment.class);
verify(client, times(1)).update(captor.capture());
Comment updated = captor.getValue();
assertThat(updated.getSpec().getCreationTime()).isNotNull();
assertThat(updated.getSpec().getCreationTime())
.isEqualTo(updated.getSpec().getApprovedTime());
}

private static Ref getRef() {
Ref ref = new Ref();
ref.setGroup("content.halo.run");
Expand Down

0 comments on commit 633c489

Please sign in to comment.