Skip to content

feat(application-server): sync pull request review threads #522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
FelixTJDietrich merged 4 commits into from
Oct 30, 2025
Merged

feat(application-server): sync pull request review threads #522

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7ac022f
feat(application-server): sync pull request review threads
FelixTJDietrich Oct 30, 2025
efeba8f
fix migration
FelixTJDietrich Oct 30, 2025
7031fbf
add length
FelixTJDietrich Oct 30, 2025
5078548
fix ci
FelixTJDietrich Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/contributor/erd/schema.mmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,18 @@ erDiagram
BIGINT author_id FK
BIGINT pull_request_id FK
BIGINT review_id FK
BIGINT thread_id FK "NOT NULL"
BIGINT in_reply_to_id FK
}

PullRequestReviewThread {
BIGINT id PK
TIMESTAMPTZ created_at
TIMESTAMPTZ updated_at
VARCHAR(255) state "NOT NULL"
TIMESTAMPTZ resolved_at
BIGINT pull_request_id FK "NOT NULL"
BIGINT root_comment_id FK,UK
}

PullRequestBadPractice {
Expand Down Expand Up @@ -320,6 +332,7 @@ erDiagram
%% One-to-One relationships
ChatMessage ||--|| ChatMessagePart : has
ChatMessage ||--|| ChatThread : references
PullRequestReviewComment ||--|| PullRequestReviewThread : reviewed
Organization ||--|| Workspace : has

%% One-to-Many relationships
Expand All @@ -341,8 +354,11 @@ erDiagram
User ||--o{ PullRequestReview : reviewed
Issue ||--o{ PullRequestReview : reviewed
User ||--o{ PullRequestReviewComment : commented_on
PullRequestReviewComment ||--o{ PullRequestReviewComment : commented_on
Issue ||--o{ PullRequestReviewComment : commented_on
PullRequestReview ||--o{ PullRequestReviewComment : commented_on
PullRequestReviewThread ||--o{ PullRequestReviewComment : commented_on
Issue ||--o{ PullRequestReviewThread : reviewed
BadPracticeDetection ||--o{ PullRequestBadPractice : has
Issue ||--o{ PullRequestBadPractice : references
Organization ||--o{ Repository : has
Expand Down
5 changes: 5 additions & 0 deletions ...n-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/pullrequest/PullRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import de.tum.in.www1.hephaestus.gitprovider.issue.Issue;
import de.tum.in.www1.hephaestus.gitprovider.pullrequestreview.PullRequestReview;
import de.tum.in.www1.hephaestus.gitprovider.pullrequestreviewcomment.PullRequestReviewComment;
import de.tum.in.www1.hephaestus.gitprovider.pullrequestreviewthread.PullRequestReviewThread;
import de.tum.in.www1.hephaestus.gitprovider.user.User;
import jakarta.persistence.*;
import java.time.Instant;
Expand Down Expand Up @@ -63,6 +64,10 @@ public class PullRequest extends Issue {
@ToString.Exclude
private Set<PullRequestReviewComment> reviewComments = new HashSet<>();

@OneToMany(mappedBy = "pullRequest", cascade = CascadeType.REMOVE, orphanRemoval = true)
@ToString.Exclude
private Set<PullRequestReviewThread> reviewThreads = new HashSet<>();

@Lob
private String badPracticeSummary;

Expand Down
18 changes: 18 additions & 0 deletions ...tum/in/www1/hephaestus/gitprovider/pullrequestreviewcomment/PullRequestReviewComment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@
import de.tum.in.www1.hephaestus.gitprovider.common.BaseGitServiceEntity;
import de.tum.in.www1.hephaestus.gitprovider.pullrequest.PullRequest;
import de.tum.in.www1.hephaestus.gitprovider.pullrequestreview.PullRequestReview;
import de.tum.in.www1.hephaestus.gitprovider.pullrequestreviewthread.PullRequestReviewThread;
import de.tum.in.www1.hephaestus.gitprovider.user.User;
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.Lob;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.OneToMany;
import jakarta.persistence.Table;
import java.util.HashSet;
import java.util.Set;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
Expand Down Expand Up @@ -96,6 +100,20 @@ public class PullRequestReviewComment extends BaseGitServiceEntity {
@ToString.Exclude
private PullRequest pullRequest;

@ManyToOne
@JoinColumn(name = "thread_id", nullable = false)
@ToString.Exclude
private PullRequestReviewThread thread;

@ManyToOne
@JoinColumn(name = "in_reply_to_id")
@ToString.Exclude
private PullRequestReviewComment inReplyTo;

@OneToMany(mappedBy = "inReplyTo")
@ToString.Exclude
private Set<PullRequestReviewComment> replies = new HashSet<>();

public enum Side {
LEFT,
RIGHT,
Expand Down
6 changes: 5 additions & 1 deletion ...1/hephaestus/gitprovider/pullrequestreviewcomment/PullRequestReviewCommentRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@

import org.springframework.data.jpa.repository.JpaRepository;

public interface PullRequestReviewCommentRepository extends JpaRepository<PullRequestReviewComment, Long> {}
public interface PullRequestReviewCommentRepository extends JpaRepository<PullRequestReviewComment, Long> {
boolean existsByThreadId(Long threadId);

long countByThreadId(Long threadId);
}
21 changes: 16 additions & 5 deletions .../gitprovider/pullrequestreviewcomment/github/GitHubPullRequestReviewCommentConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,28 @@ public PullRequestReviewComment update(
comment.setBody(source.getBody());
comment.setHtmlUrl(source.getHtmlUrl().toString());
comment.setAuthorAssociation(authorAssociationConverter.convert(source.getAuthorAssociation()));
comment.setStartLine(source.getPosition());
comment.setOriginalStartLine(source.getOriginalPosition());
comment.setLine(source.getPosition());
comment.setOriginalLine(source.getOriginalPosition());
comment.setStartSide(convertSide(source.getStartSide()));
comment.setStartLine(nullIfZero(source.getStartLine()));
comment.setOriginalStartLine(nullIfZero(source.getOriginalStartLine()));
comment.setLine(source.getLine());
comment.setOriginalLine(source.getOriginalLine());
comment.setStartSide(convertNullableSide(source.getStartSide()));
comment.setSide(convertSide(source.getSide()));
Comment on lines +43 to 48
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix NullPointerException when start_ values are absent.*

source.getStartLine() and source.getOriginalStartLine() are often null (e.g. single-line comments, replies). Auto-unboxing them to call nullIfZero(int) throws a NullPointerException, which will break synchronization on those payloads. Please let the helper accept Integer and guard against null.

-        comment.setStartLine(nullIfZero(source.getStartLine()));
-        comment.setOriginalStartLine(nullIfZero(source.getOriginalStartLine()));
+        comment.setStartLine(nullIfZero(source.getStartLine()));
+        comment.setOriginalStartLine(nullIfZero(source.getOriginalStartLine()));
@@
-    private Integer nullIfZero(int value) {
-        return value == 0 ? null : value;
+    private Integer nullIfZero(Integer value) {
+        if (value == null || value == 0) {
+            return null;
+        }
+        return value;
     }

Also applies to: 54-63

🤖 Prompt for AI Agents 
In
server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/pullrequestreviewcomment/github/GitHubPullRequestReviewCommentConverter.java
around lines 43-48 (and similarly 54-63), the code auto-unboxes potentially null
Integer values via nullIfZero(int), causing NPEs when start_* fields are absent;
update the nullIfZero helper to accept Integer and return null when the input is
null or zero, and change the calls here to pass the Integer values directly (no
pre-unboxing) so null values are preserved instead of throwing.

comment.setPosition(source.getPosition());
comment.setOriginalPosition(source.getOriginalPosition());
return comment;
}

private Integer nullIfZero(int value) {
return value == 0 ? null : value;
}

private PullRequestReviewComment.Side convertNullableSide(Side side) {
if (side == null) {
return null;
}
return convertSide(side);
}

private PullRequestReviewComment.Side convertSide(Side side) {
switch (side) {
case LEFT:
Expand Down
6 changes: 3 additions & 3 deletions ...rovider/pullrequestreviewcomment/github/GitHubPullRequestReviewCommentMessageHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class GitHubPullRequestReviewCommentMessageHandler
private final GitHubPullRequestSyncService pullRequestSyncService;
private final GitHubRepositorySyncService repositorySyncService;

private GitHubPullRequestReviewCommentMessageHandler(
public GitHubPullRequestReviewCommentMessageHandler(
PullRequestReviewCommentRepository pullRequestReviewCommentRepository,
GitHubPullRequestReviewCommentSyncService pullRequestReviewCommentSyncService,
GitHubPullRequestSyncService pullRequestSyncService,
Expand Down Expand Up @@ -51,9 +51,9 @@ protected void handleEvent(GHEventPayload.PullRequestReviewComment eventPayload)
pullRequestSyncService.processPullRequest(pullRequest);

if (action.equals("deleted")) {
pullRequestReviewCommentRepository.deleteById(comment.getId());
pullRequestReviewCommentSyncService.deletePullRequestReviewComment(comment.getId());
} else {
pullRequestReviewCommentSyncService.processPullRequestReviewComment(comment);
pullRequestReviewCommentSyncService.processPullRequestReviewComment(comment, pullRequest);
}
}

Expand Down
Loading
Loading