From adff2a719407a3ba4aad21caf3960a985936b361 Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Fri, 24 Feb 2023 18:09:57 +0800 Subject: [PATCH] refactor: default is approved to reply to comments on the console-side (#3363) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind improvement /area core /milestone 2.3.x #### What this PR does / why we need it: Console 端创建回复不再需要审核 但需要注意的是目前无法区分是否为管理员，所以如果具有评论管理权限的用户在主题端登录回复还是需要审核。 #### Which issue(s) this PR fixes: Fixes #3353 #### Special notes for your reviewer: how to test it? 1. 在主题端创建评论和回复都需要审核 2. 在 console 端回复不需要审核 /cc @halo-dev/sig-halo #### Does this PR introduce a user-facing change? ```release-note Console 端创建回复不再需要审核 ``` --- .../app/content/comment/ReplyServiceImpl.java | 53 ++++------------- .../extension/endpoint/CommentEndpoint.java | 8 +++ .../theme/endpoint/CommentFinderEndpoint.java | 58 +++++++++++++------ .../endpoint/CommentFinderEndpointTest.java | 6 ++ 4 files changed, 65 insertions(+), 60 deletions(-) diff --git a/src/main/java/run/halo/app/content/comment/ReplyServiceImpl.java b/src/main/java/run/halo/app/content/comment/ReplyServiceImpl.java index 9282f12e98..d98025a9e8 100644 --- a/src/main/java/run/halo/app/content/comment/ReplyServiceImpl.java +++ b/src/main/java/run/halo/app/content/comment/ReplyServiceImpl.java @@ -18,8 +18,6 @@ import run.halo.app.extension.Extension; import run.halo.app.extension.ListResult; import run.halo.app.extension.ReactiveExtensionClient; -import run.halo.app.infra.SystemConfigurableEnvironmentFetcher; -import run.halo.app.infra.exception.AccessDeniedException; /** * A default implementation of {@link ReplyService}. @@ -32,13 +30,12 @@ public class ReplyServiceImpl implements ReplyService { private final ReactiveExtensionClient client; - private final SystemConfigurableEnvironmentFetcher environmentFetcher; private final UserService userService; @Override public Mono create(String commentName, Reply reply) { - return client.fetch(Comment.class, commentName) - .flatMap(comment -> { + return client.get(Comment.class, commentName) + .map(comment -> { // Boolean allowNotification = reply.getSpec().getAllowNotification(); // TODO send notification if allowNotification is true reply.getSpec().setCommentName(commentName); @@ -51,28 +48,14 @@ public Mono create(String commentName, Reply reply) { if (reply.getSpec().getCreationTime() == null) { reply.getSpec().setCreationTime(Instant.now()); } - return environmentFetcher.fetchComment() - .map(commentSetting -> { - if (Boolean.FALSE.equals(commentSetting.getEnable())) { - throw new AccessDeniedException( - "The comment function has been turned off.", - "problemDetail.comment.turnedOff", null); - } - if (checkReplyOwner(reply, commentSetting.getSystemUserOnly())) { - throw new AccessDeniedException("Allow only system users to comment.", - "problemDetail.comment.systemUsersOnly", null); - } - reply.getSpec().setApproved( - Boolean.FALSE.equals(commentSetting.getRequireReviewForNew())); - // fix https://github.com/halo-dev/halo/issues/2951 - reply.getSpec().setHidden(false); - - if (BooleanUtils.isTrue(reply.getSpec().getApproved()) - && reply.getSpec().getApprovedTime() == null) { - reply.getSpec().setApprovedTime(Instant.now()); - } - return reply; - }); + if (reply.getSpec().getApproved() == null) { + reply.getSpec().setApproved(false); + } + if (BooleanUtils.isTrue(reply.getSpec().getApproved()) + && reply.getSpec().getApprovedTime() == null) { + reply.getSpec().setApprovedTime(Instant.now()); + } + return reply; }) .flatMap(replyToUse -> { if (replyToUse.getSpec().getOwner() != null) { @@ -85,21 +68,9 @@ public Mono create(String commentName, Reply reply) { return replyToUse; }) .switchIfEmpty( - Mono.error(new IllegalStateException("Reply owner must not be null."))); + Mono.error(new IllegalArgumentException("Reply owner must not be null."))); }) - .flatMap(client::create) - .switchIfEmpty(Mono.error( - new IllegalArgumentException( - String.format("Comment not found for name [%s].", commentName))) - ); - } - - private boolean checkReplyOwner(Reply reply, Boolean onlySystemUser) { - Comment.CommentOwner owner = reply.getSpec().getOwner(); - if (Boolean.TRUE.equals(onlySystemUser)) { - return owner != null && Comment.CommentOwner.KIND_EMAIL.equals(owner.getKind()); - } - return false; + .flatMap(client::create); } @Override diff --git a/src/main/java/run/halo/app/core/extension/endpoint/CommentEndpoint.java b/src/main/java/run/halo/app/core/extension/endpoint/CommentEndpoint.java index e1751b684f..ff75e4cce5 100644 --- a/src/main/java/run/halo/app/core/extension/endpoint/CommentEndpoint.java +++ b/src/main/java/run/halo/app/core/extension/endpoint/CommentEndpoint.java @@ -6,6 +6,7 @@ import static org.springdoc.core.fn.builders.requestbody.Builder.requestBodyBuilder; import io.swagger.v3.oas.annotations.enums.ParameterIn; +import java.time.Instant; import org.springdoc.core.fn.builders.schema.Builder; import org.springdoc.webflux.core.fn.SpringdocRouteBuilder; import org.springframework.http.MediaType; @@ -116,8 +117,15 @@ Mono createReply(ServerRequest request) { return request.bodyToMono(ReplyRequest.class) .flatMap(replyRequest -> { Reply reply = replyRequest.toReply(); + // Create via console without audit + reply.getSpec().setApproved(true); + reply.getSpec().setApprovedTime(Instant.now()); reply.getSpec().setIpAddress(IpAddressUtils.getIpAddress(request)); reply.getSpec().setUserAgent(HaloUtils.userAgentFrom(request)); + // fix gh-2951 + if (reply.getSpec().getHidden() == null) { + reply.getSpec().setHidden(false); + } return replyService.create(commentName, reply); }) .flatMap(comment -> ServerResponse.ok().bodyValue(comment)); diff --git a/src/main/java/run/halo/app/theme/endpoint/CommentFinderEndpoint.java b/src/main/java/run/halo/app/theme/endpoint/CommentFinderEndpoint.java index 654b5c44d7..8f0316f066 100644 --- a/src/main/java/run/halo/app/theme/endpoint/CommentFinderEndpoint.java +++ b/src/main/java/run/halo/app/theme/endpoint/CommentFinderEndpoint.java @@ -1,5 +1,8 @@ package run.halo.app.theme.endpoint; +import static io.swagger.v3.oas.annotations.media.Schema.RequiredMode.REQUIRED; +import static org.apache.commons.lang3.BooleanUtils.isFalse; +import static org.apache.commons.lang3.BooleanUtils.isTrue; import static org.springdoc.core.fn.builders.apiresponse.Builder.responseBuilder; import static org.springdoc.core.fn.builders.content.Builder.contentBuilder; import static org.springdoc.core.fn.builders.parameter.Builder.parameterBuilder; @@ -9,6 +12,7 @@ import io.swagger.v3.oas.annotations.enums.ParameterIn; import io.swagger.v3.oas.annotations.media.Schema; import java.util.List; +import lombok.RequiredArgsConstructor; import org.apache.commons.lang3.StringUtils; import org.springdoc.core.fn.builders.schema.Builder; import org.springdoc.webflux.core.fn.SpringdocRouteBuilder; @@ -32,6 +36,8 @@ import run.halo.app.extension.Ref; import run.halo.app.extension.router.IListRequest; import run.halo.app.extension.router.QueryParamBuildUtil; +import run.halo.app.infra.SystemConfigurableEnvironmentFetcher; +import run.halo.app.infra.exception.AccessDeniedException; import run.halo.app.infra.utils.HaloUtils; import run.halo.app.infra.utils.IpAddressUtils; import run.halo.app.theme.finders.CommentFinder; @@ -42,25 +48,13 @@ * Endpoint for {@link CommentFinder}. */ @Component +@RequiredArgsConstructor public class CommentFinderEndpoint implements CustomEndpoint { private final CommentFinder commentFinder; private final CommentService commentService; private final ReplyService replyService; - - /** - * Construct a {@link CommentFinderEndpoint} instance. - * - * @param commentFinder comment finder - * @param commentService comment service to create comment - * @param replyService reply service to create reply - */ - public CommentFinderEndpoint(CommentFinder commentFinder, CommentService commentService, - ReplyService replyService) { - this.commentFinder = commentFinder; - this.commentService = commentService; - this.replyService = replyService; - } + private final SystemConfigurableEnvironmentFetcher environmentFetcher; @Override public RouterFunction endpoint() { @@ -158,15 +152,41 @@ Mono createReply(ServerRequest request) { Reply reply = replyRequest.toReply(); reply.getSpec().setIpAddress(IpAddressUtils.getIpAddress(request)); reply.getSpec().setUserAgent(HaloUtils.userAgentFrom(request)); - return replyService.create(commentName, reply); + // fix gh-2951 + reply.getSpec().setHidden(false); + return environmentFetcher.fetchComment() + .map(commentSetting -> { + if (isFalse(commentSetting.getEnable())) { + throw new AccessDeniedException( + "The comment function has been turned off.", + "problemDetail.comment.turnedOff", null); + } + if (checkReplyOwner(reply, commentSetting.getSystemUserOnly())) { + throw new AccessDeniedException("Allow only system users to comment.", + "problemDetail.comment.systemUsersOnly", null); + } + reply.getSpec() + .setApproved(isFalse(commentSetting.getRequireReviewForNew())); + return reply; + }) + .defaultIfEmpty(reply); }) + .flatMap(reply -> replyService.create(commentName, reply)) .flatMap(comment -> ServerResponse.ok().bodyValue(comment)); } + private boolean checkReplyOwner(Reply reply, Boolean onlySystemUser) { + Comment.CommentOwner owner = reply.getSpec().getOwner(); + if (isTrue(onlySystemUser)) { + return owner != null && Comment.CommentOwner.KIND_EMAIL.equals(owner.getKind()); + } + return false; + } + Mono listComments(ServerRequest request) { CommentQuery commentQuery = new CommentQuery(request.queryParams()); return commentFinder.list(commentQuery.toRef(), commentQuery.getPage(), - commentQuery.getSize()) + commentQuery.getSize()) .flatMap(list -> ServerResponse.ok().bodyValue(list)); } @@ -196,7 +216,7 @@ public String getGroup() { return queryParams.getFirst("group"); } - @Schema(required = true, description = "The comment subject version.") + @Schema(requiredMode = REQUIRED, description = "The comment subject version.") public String getVersion() { return emptyToNull(queryParams.getFirst("version")); } @@ -206,7 +226,7 @@ public String getVersion() { * * @return comment subject ref kind */ - @Schema(required = true, description = "The comment subject kind.") + @Schema(requiredMode = REQUIRED, description = "The comment subject kind.") public String getKind() { String kind = emptyToNull(queryParams.getFirst("kind")); if (kind == null) { @@ -220,7 +240,7 @@ public String getKind() { * * @return comment subject ref name */ - @Schema(required = true, description = "The comment subject name.") + @Schema(requiredMode = REQUIRED, description = "The comment subject name.") public String getName() { String name = emptyToNull(queryParams.getFirst("name")); if (name == null) { diff --git a/src/test/java/run/halo/app/theme/endpoint/CommentFinderEndpointTest.java b/src/test/java/run/halo/app/theme/endpoint/CommentFinderEndpointTest.java index af5773a134..7d5b19e55d 100644 --- a/src/test/java/run/halo/app/theme/endpoint/CommentFinderEndpointTest.java +++ b/src/test/java/run/halo/app/theme/endpoint/CommentFinderEndpointTest.java @@ -4,6 +4,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -26,6 +27,7 @@ import run.halo.app.core.extension.content.Reply; import run.halo.app.extension.ListResult; import run.halo.app.extension.Ref; +import run.halo.app.infra.SystemConfigurableEnvironmentFetcher; import run.halo.app.theme.finders.CommentFinder; /** @@ -42,6 +44,9 @@ class CommentFinderEndpointTest { @Mock private CommentService commentService; + @Mock + private SystemConfigurableEnvironmentFetcher environmentFetcher; + @Mock private ReplyService replyService; @@ -52,6 +57,7 @@ class CommentFinderEndpointTest { @BeforeEach void setUp() { + lenient().when(environmentFetcher.fetchComment()).thenReturn(Mono.empty()); webTestClient = WebTestClient .bindToRouterFunction(commentFinderEndpoint.endpoint()) .build();