✨ (collaboration): Add issue/MR comments and per-commit diff view
Changes
24 files changed, +1124 -10
MODIFY
README.md
+2 -0
@@ -28,6 +28,8 @@
28
28
- Dashboard notifications: the signed-in home page (`/`) surfaces the open issues and merge requests you are involved in — ones you authored, are assigned to, or (for merge requests) are asked to review — each linking straight to the item and labelled with its repository; items in a repository you can no longer read never appear. Built on a pluggable `NotificationSource` aggregation, so future item types can contribute without touching the dashboard
29
29
- The merge request detail page renders the live diff of the source branch relative to the merge base with the target (three-dot diff), file by file with per-line add/delete coloring and a changed-files / +additions / −deletions summary — always computed live from git, never duplicated into the database
30
30
- The owner or a collaborator can Merge or Close an open merge request from the detail page; merging runs entirely in-core against the bare repository (no working tree), fast-forwarding when possible or else recording a two-parent merge commit authored by the acting user and advancing the target branch ref. An automatic merge that would conflict is rejected; a source branch already contained in the target is treated as already merged
31
+- Comments on issues and merge requests: any authenticated user who can read the repository can post a free-text comment to an issue's or a merge request's discussion thread (a flat, chronological list showing each comment's author and time), from the item's detail page. A comment can be deleted by its author, the repository owner, or a collaborator. These general discussion comments are distinct from a merge request's per-line review comments below
32
+- Per-commit diff view: from the commits tab, clicking a commit opens its detail page showing the diff that commit introduced — file by file with add/delete coloring and a changed-files / +additions / −deletions summary, computed live from git (diff against the commit's first parent, or the empty tree for a root commit)
31
33
- Line-level review comments on a merge request's diff: any authenticated user who can read the repository can comment on a specific diff line (added, deleted, or context) from the merge-request detail page; comments render inline beneath the line they anchor to. A comment can be deleted by its author, the repository owner, or a collaborator. Comments are anchored to a file plus the diff line's old/new line numbers and must land on a line that's part of the current diff. Hovering a commentable line reveals a comment icon on the right; clicking it opens the form inline — a progressive-enhancement disclosure that works without JavaScript
32
34
- OIDC login (authorization code flow) via `GET /login`; on first login the user account is created without a username and the browser is redirected to `/onboarding`, where the user picks a URL-safe handle (`^[a-z0-9][a-z0-9-]{0,38}$`, unique across users and organisations). The chosen handle — not the OIDC `preferred_username` claim (which is an SPN form in kanidm and not URL-safe) — is used in all repo, SSH, ActivityPub, and webfinger URLs. The `name` claim becomes an editable display name; both can be changed later at `/settings/profile`. A request filter blocks all app pages until a handle is chosen. The code-flow callback is pinned to `/login` (`redirect-path` + `restore-path-after-redirect`, so strict-`redirect_uri` IdPs like kanidm register one URI) and after login the user lands back on the page they came from — the header's Log in button carries `?redirect=<current page>` (local paths only, open-redirect guarded). Expired ID tokens are refreshed silently with the refresh token (`refresh-expired=true`, 60 s proactive skew, session cookie usable 12 h past expiry) instead of logging the user out. Logout is local-session only via `POST /logout` (the kanidm provider advertises no `end_session_endpoint`, so RP-Initiated Logout is disabled)
33
35
- Profile pictures: users can upload a PNG/JPEG/GIF/WebP avatar (≤ 2 MB, content-type and magic bytes both validated) at `/settings/profile`, stored on the filesystem keyed by user UUID and served publicly at `GET /users/{username}/avatar`; shown wherever a local user is rendered (header nav, repo lists, repo sidebar, issue/MR/comment authors) via a reusable Qute avatar tag, removable, and falling back to an initials badge when absent. Git commit authors and remote federation actors are not local users and keep their existing pseudo-avatars
MODIFY
docs/README.md
+8 -0
@@ -8,6 +8,11 @@
8
8
9
9
### For users
10
10
11
+- **[Comments](users/comments.md)** — leave and delete discussion comments on
12
+ issues and merge requests, who may comment, and how discussion differs from
13
+ per-line review comments.
14
+- **[Viewing a commit's changes](users/commits.md)** — open a commit from the
15
+ Commits tab to see the diff it introduced, including root commits.
11
16
- **[Forking](users/forking.md)** — make your own copy of any repository you can
12
17
read, what a fork carries, and the one-fork-per-name rule.
13
18
- **[Search](users/search.md)** — the header search box: case-insensitive
@@ -61,6 +66,9 @@
61
66
62
67
### For maintainers
63
68
69
+- **[Comments and per-commit diffs](maintainers/comments.md)** — how issue and
70
+ merge-request discussion comments and the per-commit diff view reuse the
71
+ existing comment and diff machinery, with their tables and endpoints.
64
72
- **[Forking](maintainers/forking.md)** — how forks are created (clone vs copy),
65
73
the `parent` model, visibility rules, and what works versus what is still to do.
66
74
- **[Avatars](maintainers/avatars.md)** — profile-picture storage, validation,
MODIFY
docs/admins/persistent-data.md
+1 -1
@@ -12,7 +12,7 @@
12
12
13
13
| Store | Configured by | Default (in container) | Contents | If you lose it |
14
14
|---|---|---|---|---|
15
-| PostgreSQL | `QUARKUS_DATASOURCE_*` | external service | Users, repository records, issues, merge requests, comments, SSH public keys, access-token hashes, push-mirror and federation state, CI runner registration-token and runner records (hashed secrets). Also each avatar's and repository image's content type and update timestamp — but **not** the image bytes. | Everything except the raw git objects and images. Total loss. |
15
+| PostgreSQL | `QUARKUS_DATASOURCE_*` | external service | Users, repository records, issues, merge requests, comments (issue discussion in `issue_comments`; merge-request discussion and per-line review comments share `merge_request_comments`), SSH public keys, access-token hashes, push-mirror and federation state, CI runner registration-token and runner records (hashed secrets). Also each avatar's and repository image's content type and update timestamp — but **not** the image bytes. | Everything except the raw git objects and images. Total loss. |
16
16
| Repositories | `GITSHARK_STORAGE_ROOT` | `data/repositories` | The bare git repositories (all commits, branches, tags). | All hosted code. The DB rows survive but point at nothing. |
17
17
| Avatars | `GITSHARK_AVATAR_ROOT` | `data/avatars` | Uploaded profile pictures, one file per user, named by user UUID. | Profile pictures render as broken images (the DB still says the user has one, but `GET /users/{username}/avatar` returns 404). Users must re-upload. |
18
18
| Repository images | `GITSHARK_REPO_IMAGE_ROOT` | `data/repo-images` | Uploaded per-repository images, one file per repository, named by repository UUID. | Repository images render as broken images (the DB still says the repo has one, but `GET /repos/{owner}/{name}/image` returns 404); repos fall back to the owner's avatar once the DB row is also cleared. Owners must re-upload. |
ADD
docs/maintainers/comments.md
+67 -0
@@ -0,0 +1,67 @@
1
+# Comments and per-commit diffs — implementation notes
2
+
3
+Two collaboration/inspection features that share the existing diff and comment
4
+machinery rather than adding parallel copies of it.
5
+
6
+## Issue and merge-request comments
7
+
8
+General (non-line) discussion comments on issues and merge requests.
9
+
10
+### Data model
11
+
12
+- **Issues** — a dedicated `IssueComment` entity / `issue_comments` table
13
+ (`V21__issue_comments.sql`): `id`, `issue_id` (FK, `ON DELETE CASCADE`),
14
+ `author_id` (FK, `ON DELETE CASCADE`), `body`, `created_at`. Mirrors
15
+ `MergeRequestComment` minus the diff-anchor columns.
16
+- **Merge requests** — general comments **reuse** the existing
17
+ `merge_request_comments` table. `V22__merge_request_discussion_comments.sql`
18
+ drops the `NOT NULL` on `file_path`; a general comment has `file_path = null`
19
+ and `old_line = new_line = -1`, while a per-line review comment keeps its path
20
+ and line anchor. This avoids a second MR-comment table and lets one delete
21
+ endpoint serve both kinds.
22
+
23
+### Services
24
+
25
+- `IssueCommentService` — `add` / `list` / `delete`. Any reader may comment;
26
+ author, owner, or collaborator may delete (`AccessPolicy.canWrite`). Empty
27
+ bodies are rejected with `InvalidIssueException` (→ 400).
28
+- `MergeRequestCommentService.addGeneral` — sibling of the line-anchored `add`,
29
+ but with no diff-anchor check: it sets `filePath = null` and both line numbers
30
+ to `-1`.
31
+
32
+### Web layer
33
+
34
+- `IssueResource`: `POST {number}/comments` and
35
+ `POST {number}/comments/{commentId}/delete`; the detail view passes the comment
36
+ list plus `loggedIn` / `currentUserId` to `issue.html`.
37
+- `MergeRequestResource`: `POST {number}/discussion` for general comments;
38
+ deletion reuses the existing `{number}/comments/{commentId}/delete`. `detail`
39
+ splits `commentService.list(mr)` into line comments (`filePath != null`) and
40
+ discussion (`filePath == null`) — the line-matching stream now filters on the
41
+ line-comment sublist, so a general comment's null `filePath` never reaches the
42
+ `.equals(...)` match (which would NPE).
43
+
44
+Rendering is shared markup: both `issue.html` and `mergeRequest.html` render the
45
+thread with the same `comment` / `comment-head` / `comment-body` classes already
46
+used by per-line comments, so no new CSS is required.
47
+
48
+## Per-commit diff view
49
+
50
+Reuses the merge-request diff core instead of introducing a new differ.
51
+
52
+- `GitMergeService.commitDiff(barePath, commitId)` — resolves the commit and its
53
+ first parent (or an `EmptyTreeIterator` for a root commit), scans the two trees
54
+ with a `DiffFormatter`, and runs each `DiffEntry` through the same private
55
+ `formatEntry` used by branch diffs, returning the existing `DiffView` record. A
56
+ well-formed but unknown id (including the all-zero id) surfaces as
57
+ `MissingObjectException` during `resolve` and is mapped to `Optional.empty()`
58
+ (→ 404) rather than a 500.
59
+- `GitBrowseService.commit(barePath, rev)` — single-commit metadata
60
+ (`CommitInfo`) for the page header, with the same `MissingObjectException` →
61
+ empty handling.
62
+- `RepositoryResource`: `GET commit/{id}` renders `commit.html` (read-only diff,
63
+ no comment affordances). Rows in `commits.html` link to it. The route prefix
64
+ `commit/` does not collide with the existing `commits/{ref}` listing.
65
+
66
+The diff is always computed live from git; nothing is duplicated into the
67
+database, matching the merge-request diff decision.
ADD
docs/users/comments.md
+38 -0
@@ -0,0 +1,38 @@
1
+# Comments
2
+
3
+Every issue and merge request has a **discussion thread** where anyone who can
4
+read the repository can leave a comment.
5
+
6
+## Leaving a comment
7
+
8
+Open an issue (`…/issues/1`) or a merge request (`…/merge-requests/1`) and scroll
9
+to the **Comments** (issues) or **Discussion** (merge requests) section. Type into
10
+the box and press **Comment**. You must be logged in; visitors see a *Log in to
11
+comment* note instead of the form.
12
+
13
+Comments are plain text and appear in the order they were posted, each showing the
14
+author's avatar, username, and the time it was posted.
15
+
16
+Any authenticated user who can read the repository may comment — you do not need to
17
+be the owner or a collaborator. On a private repository, only users who can read it
18
+can see or add comments.
19
+
20
+## Deleting a comment
21
+
22
+A comment can be removed by:
23
+
24
+- its **author**, or
25
+- the repository **owner** or a **collaborator**.
26
+
27
+Use the trash icon next to the comment. Deletion is immediate and cannot be undone.
28
+
29
+## Discussion vs. per-line review comments
30
+
31
+A merge request has two kinds of comment:
32
+
33
+- **Discussion** — the general thread described here, not tied to any code.
34
+- **Per-line review comments** — anchored to a specific line of the diff. See the
35
+ [merge request line comments](../../README.md#features) in the feature list;
36
+ those are added from the diff itself by hovering a changed line.
37
+
38
+Issues only have the general discussion thread.
ADD
docs/users/commits.md
+25 -0
@@ -0,0 +1,25 @@
1
+# Viewing a commit's changes
2
+
3
+The **Commits** tab of a repository lists every commit on a branch. Click a
4
+commit's short id or its message to open the **commit detail page**, which shows
5
+exactly what that commit changed.
6
+
7
+## What the page shows
8
+
9
+- The commit's full id, author, and date, and its message as the heading.
10
+- A **Changes** section: every file the commit touched, file by file, with
11
+ added lines in green and removed lines in red, plus per-file and total
12
+ *files changed / +additions / −deletions* counts.
13
+
14
+The diff is the change the commit introduced on its own — that is, the difference
15
+between the commit and its parent. For the very first commit in a repository (a
16
+*root* commit, which has no parent) every file shows as an addition, because there
17
+was nothing before it.
18
+
19
+The diff is always computed live from git; nothing is stored in the database.
20
+
21
+## Getting there
22
+
23
+- From the repository sidebar, open **Commits**, then click any row.
24
+- Or go straight to `…/commit/<id>`, where `<id>` is the commit's full or
25
+ abbreviated hash (e.g. `…/commit/1a2b3c4`).
MODIFY
src/main/java/de/workaround/git/GitBrowseService.java
+28 -0
@@ -10,6 +10,7 @@
10
10
import java.util.Optional;
11
11
12
12
import org.eclipse.jgit.diff.RawText;
13
+import org.eclipse.jgit.errors.MissingObjectException;
13
14
import org.eclipse.jgit.lib.Constants;
14
15
import org.eclipse.jgit.lib.ObjectId;
15
16
import org.eclipse.jgit.lib.Ref;
@@ -199,6 +200,33 @@
199
200
}
200
201
}
201
202
203
+ public Optional<CommitInfo> commit(Path barePath, String rev)
204
+ {
205
+ try (Repository repo = open(barePath); RevWalk revWalk = new RevWalk(repo))
206
+ {
207
+ RevCommit commit = resolveCommit(repo, revWalk, rev);
208
+ if (commit == null)
209
+ {
210
+ return Optional.empty();
211
+ }
212
+ return Optional.of(new CommitInfo(
213
+ commit.name(),
214
+ commit.abbreviate(8).name(),
215
+ commit.getShortMessage(),
216
+ commit.getAuthorIdent().getName(),
217
+ commit.getAuthorIdent().getWhenAsInstant()));
218
+ }
219
+ catch (MissingObjectException notFound)
220
+ {
221
+ // a well-formed but unknown id (or the all-zero id) resolves to a missing object rather than null
222
+ return Optional.empty();
223
+ }
224
+ catch (IOException e)
225
+ {
226
+ throw new UncheckedIOException(e);
227
+ }
228
+ }
229
+
202
230
public int commitCount(Path barePath, String ref)
203
231
{
204
232
try (Repository repo = open(barePath); RevWalk revWalk = new RevWalk(repo))
MODIFY
src/main/java/de/workaround/git/GitMergeService.java
+52 -0
@@ -13,6 +13,7 @@
13
13
14
14
import org.eclipse.jgit.diff.DiffEntry;
15
15
import org.eclipse.jgit.diff.DiffFormatter;
16
+import org.eclipse.jgit.errors.MissingObjectException;
16
17
import org.eclipse.jgit.lib.CommitBuilder;
17
18
import org.eclipse.jgit.lib.Constants;
18
19
import org.eclipse.jgit.lib.ObjectId;
@@ -136,6 +137,57 @@
136
137
}
137
138
138
139
/**
140
+ * Computes the changes a single commit introduced, i.e. the diff from its first parent to the commit itself.
141
+ * A root commit (no parent) is diffed against the empty tree, so every file shows as an addition. Returns empty
142
+ * if the commit cannot be resolved.
143
+ */
144
+ public Optional<DiffView> commitDiff(Path barePath, String commitId)
145
+ {
146
+ try (Repository repo = open(barePath); RevWalk walk = new RevWalk(repo))
147
+ {
148
+ ObjectId id = repo.resolve(commitId + "^{commit}");
149
+ if (id == null)
150
+ {
151
+ return Optional.empty();
152
+ }
153
+ RevCommit commit = walk.parseCommit(id);
154
+ RevCommit parent = commit.getParentCount() > 0 ? walk.parseCommit(commit.getParent(0)) : null;
155
+
156
+ List<FileDiff> files = new ArrayList<>();
157
+ int totalAdd = 0;
158
+ int totalDel = 0;
159
+ try (ObjectReader reader = repo.newObjectReader())
160
+ {
161
+ AbstractTreeIterator oldTree = parent == null ? new EmptyTreeIterator() : tree(reader, parent);
162
+ AbstractTreeIterator newTree = tree(reader, commit);
163
+ List<DiffEntry> entries;
164
+ try (DiffFormatter scanner = new DiffFormatter(DisabledOutputStream.INSTANCE))
165
+ {
166
+ scanner.setRepository(repo);
167
+ entries = scanner.scan(oldTree, newTree);
168
+ }
169
+ for (DiffEntry entry : entries)
170
+ {
171
+ FileDiff file = formatEntry(repo, entry);
172
+ files.add(file);
173
+ totalAdd += file.additions();
174
+ totalDel += file.deletions();
175
+ }
176
+ }
177
+ return Optional.of(new DiffView(files, totalAdd, totalDel));
178
+ }
179
+ catch (MissingObjectException notFound)
180
+ {
181
+ // a well-formed but unknown id (or the all-zero id) resolves to a missing object rather than null
182
+ return Optional.empty();
183
+ }
184
+ catch (IOException e)
185
+ {
186
+ throw new UncheckedIOException(e);
187
+ }
188
+ }
189
+
190
+ /**
139
191
* Merges the source branch into the target branch and advances the target ref. Fast-forwards when possible,
140
192
* otherwise records a two-parent merge commit authored by the given identity. Runs entirely in-core, so no
141
193
* working tree is required. Idempotent-ish: an already-merged source reports {@link MergeResult#UP_TO_DATE}.
ADD
src/main/java/de/workaround/git/IssueCommentService.java
+80 -0
@@ -0,0 +1,80 @@
1
+package de.workaround.git;
2
+
3
+import java.util.List;
4
+
5
+import de.workaround.model.Issue;
6
+import de.workaround.model.IssueComment;
7
+import de.workaround.model.User;
8
+import jakarta.enterprise.context.ApplicationScoped;
9
+import jakarta.inject.Inject;
10
+import jakarta.transaction.Transactional;
11
+
12
+/**
13
+ * Manages free-text discussion comments on issues. Any authenticated user who can read the repository may
14
+ * comment; a comment can be removed by its author, the repository owner, or a collaborator.
15
+ */
16
+@ApplicationScoped
17
+public class IssueCommentService
18
+{
19
+ @Inject
20
+ IssueComment.Repo comments;
21
+
22
+ @Inject
23
+ AccessPolicy accessPolicy;
24
+
25
+ @Transactional
26
+ public IssueComment add(User actor, Issue issue, String body)
27
+ {
28
+ requireReader(actor, issue);
29
+ String trimmedBody = body == null ? "" : body.strip();
30
+ if (trimmedBody.isEmpty())
31
+ {
32
+ throw new InvalidIssueException("Comment must not be empty");
33
+ }
34
+ IssueComment comment = new IssueComment();
35
+ comment.issue = issue;
36
+ comment.author = actor;
37
+ comment.body = trimmedBody;
38
+ comment.persist();
39
+ return comment;
40
+ }
41
+
42
+ public List<IssueComment> list(Issue issue)
43
+ {
44
+ return comments.findByIssue(issue);
45
+ }
46
+
47
+ @Transactional
48
+ public void delete(User actor, IssueComment comment)
49
+ {
50
+ if (actor == null)
51
+ {
52
+ throw new ForbiddenOperationException("Authentication required");
53
+ }
54
+ IssueComment managed = comments.findById(comment.id);
55
+ if (managed == null)
56
+ {
57
+ return;
58
+ }
59
+ boolean isAuthor = managed.author.id.equals(actor.id);
60
+ boolean isOwner = accessPolicy.canWrite(actor, managed.issue.repository);
61
+ if (!isAuthor && !isOwner)
62
+ {
63
+ throw new ForbiddenOperationException("Only the comment author, the repository owner or a collaborator can delete it");
64
+ }
65
+ comments.deleteById(managed.id);
66
+ }
67
+
68
+ private void requireReader(User actor, Issue issue)
69
+ {
70
+ if (actor == null)
71
+ {
72
+ throw new ForbiddenOperationException("Authentication required");
73
+ }
74
+ if (!accessPolicy.canRead(actor, issue.repository))
75
+ {
76
+ throw new ForbiddenOperationException("You cannot comment on this issue");
77
+ }
78
+ }
79
+
80
+}
MODIFY
src/main/java/de/workaround/git/MergeRequestCommentService.java
+24 -0
@@ -54,6 +54,30 @@
54
54
return comment;
55
55
}
56
56
57
+ /**
58
+ * Adds a general (non-line) discussion comment to the merge request. Unlike {@link #add}, it is not anchored
59
+ * to a diff line: {@code filePath} is left null and both line numbers stay -1.
60
+ */
61
+ @Transactional
62
+ public MergeRequestComment addGeneral(User actor, MergeRequest mr, String body)
63
+ {
64
+ requireReader(actor, mr);
65
+ String trimmedBody = body == null ? "" : body.strip();
66
+ if (trimmedBody.isEmpty())
67
+ {
68
+ throw new InvalidMergeRequestException("Comment must not be empty");
69
+ }
70
+ MergeRequestComment comment = new MergeRequestComment();
71
+ comment.mergeRequest = mr;
72
+ comment.author = actor;
73
+ comment.filePath = null;
74
+ comment.oldLine = -1;
75
+ comment.newLine = -1;
76
+ comment.body = trimmedBody;
77
+ comment.persist();
78
+ return comment;
79
+ }
80
+
57
81
public List<MergeRequestComment> list(MergeRequest mr)
58
82
{
59
83
return comments.findByMergeRequest(mr);
ADD
src/main/java/de/workaround/model/IssueComment.java
+47 -0
@@ -0,0 +1,47 @@
1
+package de.workaround.model;
2
+
3
+import java.time.Instant;
4
+import java.util.List;
5
+import java.util.UUID;
6
+
7
+import org.hibernate.annotations.processing.HQL;
8
+
9
+import io.quarkus.hibernate.panache.PanacheEntity;
10
+import io.quarkus.hibernate.panache.PanacheRepository;
11
+import jakarta.persistence.Entity;
12
+import jakarta.persistence.GeneratedValue;
13
+import jakarta.persistence.GenerationType;
14
+import jakarta.persistence.Id;
15
+import jakarta.persistence.ManyToOne;
16
+import jakarta.persistence.Table;
17
+
18
+/**
19
+ * A free-text discussion comment on an {@link Issue}. Comments form a flat, chronological thread on the issue
20
+ * detail page and carry only an author, a body and a creation time. Removed with the issue (and thus its
21
+ * repository) via DB-level ON DELETE CASCADE.
22
+ */
23
+@Entity
24
+@Table(name = "issue_comments")
25
+public class IssueComment implements PanacheEntity.Managed
26
+{
27
+ @Id
28
+ @GeneratedValue(strategy = GenerationType.UUID)
29
+ public UUID id;
30
+
31
+ @ManyToOne(optional = false)
32
+ public Issue issue;
33
+
34
+ @ManyToOne(optional = false)
35
+ public User author;
36
+
37
+ public String body;
38
+
39
+ public Instant createdAt = Instant.now();
40
+
41
+ public interface Repo extends PanacheRepository.Managed<IssueComment, UUID>
42
+ {
43
+ @HQL("select c from IssueComment c join fetch c.author where c.issue = :issue order by c.createdAt")
44
+ List<IssueComment> findByIssue(Issue issue);
45
+ }
46
+
47
+}
MODIFY
src/main/java/de/workaround/web/IssueResource.java
+55 -2
@@ -9,8 +9,10 @@
9
9
import de.workaround.git.AccessPolicy;
10
10
import de.workaround.git.CollaboratorService;
11
11
import de.workaround.git.GitRepositoryService;
12
+import de.workaround.git.IssueCommentService;
12
13
import de.workaround.git.IssueService;
13
14
import de.workaround.model.Issue;
15
+import de.workaround.model.IssueComment;
14
16
import de.workaround.model.Repository;
15
17
import de.workaround.model.User;
16
18
import io.quarkus.qute.CheckedTemplate;
@@ -44,7 +46,8 @@
44
46
static native TemplateInstance editIssue(Repository repo, RepoNav nav, Issue issue);
45
47
46
48
static native TemplateInstance issue(Repository repo, RepoNav nav, boolean owner, Issue issue,
47
- String descriptionHtml, List<Issue.Status> statuses, List<User> assignees);
49
+ String descriptionHtml, List<Issue.Status> statuses, List<User> assignees, boolean loggedIn,
50
+ UUID currentUserId, List<IssueComment> comments);
48
51
}
49
52
50
53
@Inject
@@ -60,6 +63,12 @@
60
63
IssueService issueService;
61
64
62
65
@Inject
66
+ IssueCommentService issueCommentService;
67
+
68
+ @Inject
69
+ IssueComment.Repo commentRepo;
70
+
71
+ @Inject
63
72
CollaboratorService collaboratorService;
64
73
65
74
@Inject
@@ -114,9 +123,41 @@
114
123
Issue issue = issueService.find(repo, number).orElseThrow(NotFoundException::new);
115
124
User user = currentUser.get();
116
125
boolean isOwner = accessPolicy.canAdmin(user, repo);
126
+ boolean loggedIn = user != null;
127
+ UUID currentUserId = user == null ? null : user.id;
117
128
String descriptionHtml = issue.description == null ? null : Markdown.render(issue.description);
118
129
return Templates.issue(repo, repoNav.build(repo, uriInfo), isOwner, issue, descriptionHtml,
119
- List.of(Issue.Status.values()), assignableUsers(repo));
130
+ List.of(Issue.Status.values()), assignableUsers(repo), loggedIn, currentUserId,
131
+ issueCommentService.list(issue));
132
+ }
133
+
134
+ @POST
135
+ @jakarta.ws.rs.Path("{number:\\d+}/comments")
136
+ @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
137
+ public Response comment(@PathParam("owner") String owner, @PathParam("name") String name,
138
+ @PathParam("number") int number, @FormParam("body") String body)
139
+ {
140
+ Repository repo = requireReadable(owner, name);
141
+ Issue issue = issueService.find(repo, number).orElseThrow(NotFoundException::new);
142
+ issueCommentService.add(currentUser.require(), issue, body);
143
+ return Response.seeOther(issueUri(repo, issue.number)).build();
144
+ }
145
+
146
+ @POST
147
+ @jakarta.ws.rs.Path("{number:\\d+}/comments/{commentId}/delete")
148
+ @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
149
+ public Response deleteComment(@PathParam("owner") String owner, @PathParam("name") String name,
150
+ @PathParam("number") int number, @PathParam("commentId") String commentId)
151
+ {
152
+ Repository repo = requireReadable(owner, name);
153
+ Issue issue = issueService.find(repo, number).orElseThrow(NotFoundException::new);
154
+ IssueComment comment = commentRepo.findById(parseId(commentId));
155
+ if (comment == null || !comment.issue.id.equals(issue.id))
156
+ {
157
+ throw new NotFoundException();
158
+ }
159
+ issueCommentService.delete(currentUser.require(), comment);
160
+ return Response.seeOther(issueUri(repo, issue.number)).build();
120
161
}
121
162
122
163
/**
@@ -216,6 +257,18 @@
216
257
return URI.create("/repos/" + repo.ownerHandle() + "/" + repo.name + "/issues/" + number);
217
258
}
218
259
260
+ private static UUID parseId(String id)
261
+ {
262
+ try
263
+ {
264
+ return UUID.fromString(id);
265
+ }
266
+ catch (IllegalArgumentException malformed)
267
+ {
268
+ throw new NotFoundException();
269
+ }
270
+ }
271
+
219
272
private static Issue.Status parseStatus(String status)
220
273
{
221
274
try
MODIFY
src/main/java/de/workaround/web/MergeRequestResource.java
+20 -5
@@ -50,7 +50,7 @@
50
50
51
51
static native TemplateInstance mergeRequest(Repository repo, RepoNav nav, boolean owner, boolean loggedIn,
52
52
UUID currentUserId, MergeRequest mr, List<FileDiffView> files, int additions, int deletions,
53
- List<User> assignees);
53
+ List<User> assignees, List<MergeRequestComment> discussion);
54
54
}
55
55
56
56
/**
@@ -143,6 +143,9 @@
143
143
MergeRequest mr = mergeRequestService.find(repo, number).orElseThrow(NotFoundException::new);
144
144
GitMergeService.DiffView diff = mergeRequestService.diff(mr).orElse(null);
145
145
List<MergeRequestComment> comments = commentService.list(mr);
146
+ // line-anchored review comments carry a file path; general discussion comments have none
147
+ List<MergeRequestComment> lineComments = comments.stream().filter(c -> c.filePath != null).toList();
148
+ List<MergeRequestComment> discussion = comments.stream().filter(c -> c.filePath == null).toList();
146
149
User user = currentUser.get();
147
150
boolean loggedIn = user != null;
148
151
UUID currentUserId = user == null ? null : user.id;
@@ -160,14 +163,14 @@
160
163
for (GitMergeService.DiffLine line : file.lines())
161
164
{
162
165
boolean commentable = isContent(line.type());
163
- List<MergeRequestComment> lineComments = commentable
164
- ? comments.stream()
166
+ List<MergeRequestComment> anchored = commentable
167
+ ? lineComments.stream()
165
168
.filter(c -> c.filePath.equals(file.path()) && c.oldLine == line.oldLine()
166
169
&& c.newLine == line.newLine())
167
170
.toList()
168
171
: List.of();
169
172
String anchorId = "cl-" + fileIndex + "-" + lineIndex;
170
- lines.add(new DiffLineView(anchorId, line, commentable && loggedIn, lineComments));
173
+ lines.add(new DiffLineView(anchorId, line, commentable && loggedIn, anchored));
171
174
lineIndex++;
172
175
}
173
176
files.add(new FileDiffView(file.path(), file.changeType(), file.additions(), file.deletions(), lines));
@@ -177,7 +180,7 @@
177
180
}
178
181
}
179
182
return Templates.mergeRequest(repo, repoNav.build(repo, uriInfo), isOwner(repo), loggedIn, currentUserId, mr,
180
- files, additions, deletions, assignableUsers(repo));
183
+ files, additions, deletions, assignableUsers(repo), discussion);
181
184
}
182
185
183
186
/**
@@ -221,6 +224,18 @@
221
224
}
222
225
223
226
@POST
227
+ @jakarta.ws.rs.Path("{number:\\d+}/discussion")
228
+ @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
229
+ public Response discuss(@PathParam("owner") String owner, @PathParam("name") String name,
230
+ @PathParam("number") int number, @FormParam("body") String body)
231
+ {
232
+ Repository repo = requireReadable(owner, name);
233
+ MergeRequest mr = mergeRequestService.find(repo, number).orElseThrow(NotFoundException::new);
234
+ commentService.addGeneral(currentUser.require(), mr, body);
235
+ return Response.seeOther(detailUri(repo, mr.number)).build();
236
+ }
237
+
238
+ @POST
224
239
@jakarta.ws.rs.Path("{number:\\d+}/comments/{commentId}/delete")
225
240
@Consumes(MediaType.APPLICATION_FORM_URLENCODED)
226
241
public Response deleteComment(@PathParam("owner") String owner, @PathParam("name") String name,
MODIFY
src/main/java/de/workaround/web/RepositoryResource.java
+20 -0
@@ -22,6 +22,7 @@
22
22
import de.workaround.account.InvalidImageException;
23
23
import de.workaround.git.AccessPolicy;
24
24
import de.workaround.git.GitBrowseService;
25
+import de.workaround.git.GitMergeService;
25
26
import de.workaround.git.GitRepositoryService;
26
27
import de.workaround.git.RepositoryImageService;
27
28
import de.workaround.git.RepositoryPinService;
@@ -66,6 +67,9 @@
66
67
List<GitBrowseService.CommitInfo> commits, int page, int prevPage, int nextPage, int size,
67
68
boolean hasNext);
68
69
70
+ static native TemplateInstance commit(Repository repo, RepoNav nav, GitBrowseService.CommitInfo commit,
71
+ List<GitMergeService.FileDiff> files, int additions, int deletions);
72
+
69
73
static native TemplateInstance branches(Repository repo, RepoNav nav,
70
74
List<GitBrowseService.BranchInfo> branches);
71
75
@@ -85,6 +89,9 @@
85
89
GitBrowseService browse;
86
90
87
91
@Inject
92
+ GitMergeService mergeService;
93
+
94
+ @Inject
88
95
AccessPolicy accessPolicy;
89
96
90
97
@Inject
@@ -236,6 +243,19 @@
236
243
}
237
244
238
245
@GET
246
+ @jakarta.ws.rs.Path("commit/{id}")
247
+ public TemplateInstance commit(@PathParam("owner") String owner, @PathParam("name") String name,
248
+ @PathParam("id") String id)
249
+ {
250
+ Repository repo = requireReadable(owner, name);
251
+ RepoNav nav = repoNav.build(repo, uriInfo);
252
+ Path path = service.repositoryPath(repo);
253
+ GitBrowseService.CommitInfo info = browse.commit(path, id).orElseThrow(NotFoundException::new);
254
+ GitMergeService.DiffView diff = mergeService.commitDiff(path, id).orElseThrow(NotFoundException::new);
255
+ return Templates.commit(repo, nav, info, diff.files(), diff.additions(), diff.deletions());
256
+ }
257
+
258
+ @GET
239
259
@jakarta.ws.rs.Path("branches")
240
260
public TemplateInstance branches(@PathParam("owner") String owner, @PathParam("name") String name)
241
261
{
ADD
src/main/resources/db/migration/V21__issue_comments.sql
+9 -0
@@ -0,0 +1,9 @@
1
+create table issue_comments
2
+(
3
+ id uuid primary key,
4
+ issue_id uuid not null references issues (id) on delete cascade,
5
+ author_id uuid not null references users (id) on delete cascade,
6
+ body text not null,
7
+ created_at timestamptz not null default now()
8
+);
9
+create index issue_comments_issue_idx on issue_comments (issue_id);
ADD
src/main/resources/db/migration/V22__merge_request_discussion_comments.sql
+4 -0
@@ -0,0 +1,4 @@
1
+-- General (non-line) discussion comments reuse this table with a null file_path; only line-anchored
2
+-- review comments still carry one. old_line/new_line stay -1 for general comments (their -1 default).
3
+alter table merge_request_comments
4
+ alter column file_path drop not null;
MODIFY
src/main/resources/templates/IssueResource/issue.html
+36 -0
@@ -29,6 +29,42 @@
29
29
{#else}
30
30
<p class="issue-nodesc muted">No description provided.</p>
31
31
{/if}
32
+
33
+ <section class="comments">
34
+ <h3 class="comments-title">Comments <span class="ct">{comments.size()}</span></h3>
35
+ {#if comments.isEmpty()}
36
+ <p class="muted">No comments yet.</p>
37
+ {/if}
38
+ {#for c in comments}
39
+ <div class="comment-row">
40
+ <div class="comment">
41
+ <div class="comment-head">
42
+ <span class="who">{#avatar user=c.author /} <strong>{c.author.username}</strong></span>
43
+ <span class="muted comment-when">{c.createdAt}</span>
44
+ </div>
45
+ <div class="comment-body">{c.body}</div>
46
+ </div>
47
+ {#if owner || c.author.id == currentUserId}
48
+ <form class="comment-del-form" method="post"
49
+ action="/repos/{repo.ownerHandle}/{repo.name}/issues/{issue.number}/comments/{c.id}/delete">
50
+ <button type="submit" class="comment-del" title="Delete comment" aria-label="Delete comment">
51
+ <svg viewBox="0 0 24 24" width="17" height="17" aria-hidden="true"><path fill="currentColor" d="M9 3h6l1 2h4v2H4V5h4l1-2Zm-3 6h12l-1 11a2 2 0 0 1-2 2H9a2 2 0 0 1-2-2L6 9Z"/></svg>
52
+ </button>
53
+ </form>
54
+ {/if}
55
+ </div>
56
+ {/for}
57
+ {#if loggedIn}
58
+ <form class="comment-form" method="post" action="/repos/{repo.ownerHandle}/{repo.name}/issues/{issue.number}/comments">
59
+ <textarea name="body" rows="3" placeholder="Leave a comment" required autocomplete="off"></textarea>
60
+ <div class="form-actions">
61
+ <button type="submit" class="btn btn-primary btn-sm">Comment</button>
62
+ </div>
63
+ </form>
64
+ {#else}
65
+ <p class="muted">Log in to comment.</p>
66
+ {/if}
67
+ </section>
32
68
</div>
33
69
34
70
<aside class="issue-side">
MODIFY
src/main/resources/templates/MergeRequestResource/mergeRequest.html
+35 -0
@@ -107,6 +107,41 @@
107
107
</form>
108
108
</div>
109
109
{/if}
110
+<section class="comments">
111
+ <h3 class="comments-title">Discussion <span class="ct">{discussion.size()}</span></h3>
112
+ {#if discussion.isEmpty()}
113
+ <p class="muted">No comments yet.</p>
114
+ {/if}
115
+ {#for c in discussion}
116
+ <div class="comment-row">
117
+ <div class="comment">
118
+ <div class="comment-head">
119
+ <span class="who">{#avatar user=c.author /} <strong>{c.author.username}</strong></span>
120
+ <span class="muted comment-when">{c.createdAt}</span>
121
+ </div>
122
+ <div class="comment-body">{c.body}</div>
123
+ </div>
124
+ {#if owner || c.author.id == currentUserId}
125
+ <form class="comment-del-form" method="post"
126
+ action="/repos/{repo.ownerHandle}/{repo.name}/merge-requests/{mr.number}/comments/{c.id}/delete">
127
+ <button type="submit" class="comment-del" title="Delete comment" aria-label="Delete comment">
128
+ <svg viewBox="0 0 24 24" width="17" height="17" aria-hidden="true"><path fill="currentColor" d="M9 3h6l1 2h4v2H4V5h4l1-2Zm-3 6h12l-1 11a2 2 0 0 1-2 2H9a2 2 0 0 1-2-2L6 9Z"/></svg>
129
+ </button>
130
+ </form>
131
+ {/if}
132
+ </div>
133
+ {/for}
134
+ {#if loggedIn}
135
+ <form class="comment-form" method="post" action="/repos/{repo.ownerHandle}/{repo.name}/merge-requests/{mr.number}/discussion">
136
+ <textarea name="body" rows="3" placeholder="Leave a comment" required autocomplete="off"></textarea>
137
+ <div class="form-actions">
138
+ <button type="submit" class="btn btn-primary btn-sm">Comment</button>
139
+ </div>
140
+ </form>
141
+ {#else}
142
+ <p class="muted">Log in to comment.</p>
143
+ {/if}
144
+</section>
110
145
<h3>Changes</h3>
111
146
{#if files.isEmpty()}
112
147
<p class="muted">No changes to show. The source branch has nothing new over the target.</p>
ADD
src/main/resources/templates/RepositoryResource/commit.html
+44 -0
@@ -0,0 +1,44 @@
1
+{#include layout}
2
+{#title}{commit.shortId} {commit.message} – {repo.name}{/title}
3
+<div class="repo-layout">
4
+ {#include RepositoryResource/sidebar nav=nav active='commits' /}
5
+ <section class="repo-main">
6
+ <p><a href="/repos/{repo.ownerHandle}/{repo.name}/commits/{nav.defaultBranch}">← Commits</a></p>
7
+ <div class="commit-detail">
8
+ <h2 class="commit-subject">{commit.message}</h2>
9
+ <p class="commit-meta muted">
10
+ <code>{commit.id}</code> · {commit.author} · {commit.date}
11
+ </p>
12
+ </div>
13
+ <h3>Changes</h3>
14
+ {#if files.isEmpty()}
15
+ <p class="muted">This commit introduced no file changes.</p>
16
+ {#else}
17
+ <p class="diffstat-summary">
18
+ <b>{files.size()}</b> file{#if files.size() != 1}s{/if} changed,
19
+ <span class="add">+{additions}</span> <span class="del">-{deletions}</span>
20
+ </p>
21
+ {#for file in files}
22
+ <div class="diff-file">
23
+ <div class="diff-file-head">
24
+ <span class="badge ct-{file.changeType}">{file.changeType}</span>
25
+ <code>{file.path}</code>
26
+ <span class="diffstat"><span class="add">+{file.additions}</span> <span class="del">-{file.deletions}</span></span>
27
+ </div>
28
+ <div class="diff-body">
29
+ {#for line in file.lines}
30
+ <div class="dl {line.type}">
31
+ <div class="dl-line">
32
+ <span class="ln">{#if line.oldLine != -1}{line.oldLine}{/if}</span>
33
+ <span class="ln">{#if line.newLine != -1}{line.newLine}{/if}</span>
34
+ <span class="dl-text">{line.text}</span>
35
+ </div>
36
+ </div>
37
+ {/for}
38
+ </div>
39
+ </div>
40
+ {/for}
41
+ {/if}
42
+ </section>
43
+</div>
44
+{/include}
MODIFY
src/main/resources/templates/RepositoryResource/commits.html
+2 -2
@@ -8,8 +8,8 @@
8
8
<tr><th>Commit</th><th>Message</th><th>Author</th><th>Date</th></tr>
9
9
{#for commit in commits}
10
10
<tr>
11
- <td><code>{commit.shortId}</code></td>
12
- <td>{commit.message}</td>
11
+ <td><a href="/repos/{repo.ownerHandle}/{repo.name}/commit/{commit.id}"><code>{commit.shortId}</code></a></td>
12
+ <td><a href="/repos/{repo.ownerHandle}/{repo.name}/commit/{commit.id}">{commit.message}</a></td>
13
13
<td>{commit.author}</td>
14
14
<td>{commit.date}</td>
15
15
</tr>
ADD
src/test/java/de/workaround/git/IssueCommentServiceTest.java
+158 -0
@@ -0,0 +1,158 @@
1
+package de.workaround.git;
2
+
3
+import java.util.UUID;
4
+
5
+import org.junit.jupiter.api.Test;
6
+
7
+import de.workaround.model.Issue;
8
+import de.workaround.model.IssueComment;
9
+import de.workaround.model.Repository;
10
+import de.workaround.model.User;
11
+import io.quarkus.test.junit.QuarkusTest;
12
+import jakarta.inject.Inject;
13
+import jakarta.transaction.Transactional;
14
+
15
+import static org.junit.jupiter.api.Assertions.assertEquals;
16
+import static org.junit.jupiter.api.Assertions.assertNull;
17
+import static org.junit.jupiter.api.Assertions.assertThrows;
18
+
19
+@QuarkusTest
20
+class IssueCommentServiceTest
21
+{
22
+ @Inject
23
+ GitRepositoryService service;
24
+
25
+ @Inject
26
+ IssueService issues;
27
+
28
+ @Inject
29
+ IssueCommentService comments;
30
+
31
+ @Inject
32
+ IssueComment.Repo commentRepo;
33
+
34
+ @Inject
35
+ User.Repo userRepo;
36
+
37
+ @Test
38
+ void addStoresACommentOnTheIssue()
39
+ {
40
+ User owner = persistUser("icm-alice");
41
+ Repository repository = service.create(owner, "icma", Repository.Visibility.PUBLIC, null);
42
+ Issue issue = issues.create(owner, repository, "Topic", null);
43
+
44
+ IssueComment comment = comments.add(owner, issue, "looks good");
45
+
46
+ assertEquals("looks good", comment.body);
47
+ assertEquals(owner.id, comment.author.id);
48
+ assertEquals(issue.id, comment.issue.id);
49
+ }
50
+
51
+ @Test
52
+ void addRejectsBlankBody()
53
+ {
54
+ User owner = persistUser("icm-bob");
55
+ Repository repository = service.create(owner, "icmb", Repository.Visibility.PUBLIC, null);
56
+ Issue issue = issues.create(owner, repository, "Topic", null);
57
+
58
+ assertThrows(InvalidIssueException.class, () -> comments.add(owner, issue, " "));
59
+ }
60
+
61
+ @Test
62
+ void anonymousCannotComment()
63
+ {
64
+ User owner = persistUser("icm-dan");
65
+ Repository repository = service.create(owner, "icmd", Repository.Visibility.PUBLIC, null);
66
+ Issue issue = issues.create(owner, repository, "Topic", null);
67
+
68
+ assertThrows(ForbiddenOperationException.class, () -> comments.add(null, issue, "hi"));
69
+ }
70
+
71
+ @Test
72
+ void anyReaderOfAPublicRepositoryCanComment()
73
+ {
74
+ User owner = persistUser("icm-erin");
75
+ User reader = persistUser("icm-frank");
76
+ Repository repository = service.create(owner, "icme", Repository.Visibility.PUBLIC, null);
77
+ Issue issue = issues.create(owner, repository, "Topic", null);
78
+
79
+ IssueComment comment = comments.add(reader, issue, "drive-by remark");
80
+
81
+ assertEquals(reader.id, comment.author.id);
82
+ }
83
+
84
+ @Test
85
+ void strangerCannotCommentOnAPrivateRepository()
86
+ {
87
+ User owner = persistUser("icm-gwen");
88
+ User stranger = persistUser("icm-hugo");
89
+ Repository repository = service.create(owner, "icmg", Repository.Visibility.PRIVATE, null);
90
+ Issue issue = issues.create(owner, repository, "Topic", null);
91
+
92
+ assertThrows(ForbiddenOperationException.class, () -> comments.add(stranger, issue, "sneaky"));
93
+ }
94
+
95
+ @Test
96
+ void authorOrOwnerCanDeleteButOthersCannot()
97
+ {
98
+ User owner = persistUser("icm-ivy");
99
+ User reader = persistUser("icm-jack");
100
+ User other = persistUser("icm-kate");
101
+ Repository repository = service.create(owner, "icmi", Repository.Visibility.PUBLIC, null);
102
+ Issue issue = issues.create(owner, repository, "Topic", null);
103
+ IssueComment byReader = comments.add(reader, issue, "reader note");
104
+
105
+ // a third party who is neither the author nor an owner/collaborator cannot delete it
106
+ assertThrows(ForbiddenOperationException.class, () -> comments.delete(other, byReader));
107
+
108
+ // the author can delete their own comment
109
+ comments.delete(reader, byReader);
110
+ assertNull(commentRepo.findById(byReader.id));
111
+
112
+ // the repo owner can delete anyone's comment
113
+ IssueComment byReader2 = comments.add(reader, issue, "another");
114
+ comments.delete(owner, byReader2);
115
+ assertNull(commentRepo.findById(byReader2.id));
116
+ }
117
+
118
+ @Test
119
+ void listReturnsCommentsForTheIssue()
120
+ {
121
+ User owner = persistUser("icm-liam");
122
+ Repository repository = service.create(owner, "icml", Repository.Visibility.PUBLIC, null);
123
+ Issue issue = issues.create(owner, repository, "Topic", null);
124
+ comments.add(owner, issue, "one");
125
+ comments.add(owner, issue, "two");
126
+
127
+ assertEquals(2, comments.list(issue).size());
128
+ }
129
+
130
+ @Test
131
+ void deletingARepositoryRemovesItsIssueComments()
132
+ {
133
+ User owner = persistUser("icm-mia");
134
+ Repository repository = service.create(owner, "icmm", Repository.Visibility.PUBLIC, null);
135
+ Issue issue = issues.create(owner, repository, "Topic", null);
136
+ IssueComment comment = comments.add(owner, issue, "bye");
137
+ UUID commentId = comment.id;
138
+
139
+ service.delete(owner, repository);
140
+
141
+ assertNull(commentRepo.findById(commentId), "comment must cascade-delete with its repository");
142
+ }
143
+
144
+ @Transactional
145
+ User persistUser(String name)
146
+ {
147
+ User existing = userRepo.findByOidcSubOptional(name).orElse(null);
148
+ if (existing != null)
149
+ {
150
+ return existing;
151
+ }
152
+ User user = new User();
153
+ user.oidcSub = name;
154
+ user.username = name;
155
+ user.persist();
156
+ return user;
157
+ }
158
+}
ADD
src/test/java/de/workaround/web/CommitDiffUiTest.java
+112 -0
@@ -0,0 +1,112 @@
1
+package de.workaround.web;
2
+
3
+import java.nio.charset.StandardCharsets;
4
+import java.nio.file.Path;
5
+import java.util.Map;
6
+
7
+import org.junit.jupiter.api.Test;
8
+
9
+import de.workaround.git.GitBrowseService;
10
+import de.workaround.git.GitRepositoryService;
11
+import de.workaround.git.GitTestSeeder;
12
+import de.workaround.model.Repository;
13
+import de.workaround.model.User;
14
+import io.quarkus.test.junit.QuarkusTest;
15
+import io.quarkus.test.security.TestSecurity;
16
+import jakarta.inject.Inject;
17
+import jakarta.transaction.Transactional;
18
+
19
+import static io.restassured.RestAssured.given;
20
+import static org.hamcrest.CoreMatchers.containsString;
21
+
22
+@QuarkusTest
23
+class CommitDiffUiTest
24
+{
25
+ @Inject
26
+ GitRepositoryService service;
27
+
28
+ @Inject
29
+ GitBrowseService browse;
30
+
31
+ @Inject
32
+ User.Repo userRepo;
33
+
34
+ @Test
35
+ @TestSecurity(user = "cd-owner")
36
+ void commitDetailShowsTheDiffAndTheCommitsListLinksToIt() throws Exception
37
+ {
38
+ User owner = persistUser("cd-owner");
39
+ Repository repo = service.create(owner, "board", Repository.Visibility.PUBLIC, null);
40
+ Path bare = service.repositoryPath(repo);
41
+ // two commits: the second rewrites counter.txt to "revision 2\n"
42
+ GitTestSeeder.seed(bare, Map.of("base.txt", "base\n".getBytes(StandardCharsets.UTF_8)), 2);
43
+ String head = browse.commits(bare, "main", 0, 10).orElseThrow().commits().get(0).id();
44
+
45
+ String detail = "/repos/" + owner.username + "/board/commit/" + head;
46
+ given().when().get(detail)
47
+ .then().statusCode(200)
48
+ .body(containsString("counter.txt"))
49
+ .body(containsString("revision 2"));
50
+
51
+ // the commits list links each row to its per-commit diff page
52
+ given().when().get("/repos/" + owner.username + "/board/commits/main")
53
+ .then().statusCode(200)
54
+ .body(containsString("/board/commit/" + head));
55
+ }
56
+
57
+ @Test
58
+ @TestSecurity(user = "cd-root")
59
+ void rootCommitShowsItsFilesAsAdditions() throws Exception
60
+ {
61
+ User owner = persistUser("cd-root");
62
+ Repository repo = service.create(owner, "seed", Repository.Visibility.PUBLIC, null);
63
+ Path bare = service.repositoryPath(repo);
64
+ GitTestSeeder.seed(bare, Map.of("hello.txt", "hello world\n".getBytes(StandardCharsets.UTF_8)));
65
+ String root = browse.commits(bare, "main", 0, 10).orElseThrow().commits().get(0).id();
66
+
67
+ given().when().get("/repos/" + owner.username + "/seed/commit/" + root)
68
+ .then().statusCode(200)
69
+ .body(containsString("hello.txt"))
70
+ .body(containsString("hello world"));
71
+ }
72
+
73
+ @Test
74
+ void unknownCommitIs404()
75
+ {
76
+ User owner = persistUser("cd-404-owner");
77
+ service.create(owner, "empty", Repository.Visibility.PUBLIC, null);
78
+
79
+ given().when().get("/repos/" + owner.username + "/empty/commit/0000000000000000000000000000000000000000")
80
+ .then().statusCode(404);
81
+ }
82
+
83
+ @Test
84
+ @TestSecurity(user = "cd-stranger")
85
+ void privateRepoCommitIsHiddenFromStrangers() throws Exception
86
+ {
87
+ persistUser("cd-stranger");
88
+ User owner = persistUser("cd-priv-owner");
89
+ Repository repo = service.create(owner, "secret", Repository.Visibility.PRIVATE, null);
90
+ Path bare = service.repositoryPath(repo);
91
+ GitTestSeeder.seed(bare, Map.of("base.txt", "base\n".getBytes(StandardCharsets.UTF_8)));
92
+ String head = browse.commits(bare, "main", 0, 10).orElseThrow().commits().get(0).id();
93
+
94
+ given().when().get("/repos/" + owner.username + "/secret/commit/" + head)
95
+ .then().statusCode(404);
96
+ }
97
+
98
+ @Transactional
99
+ User persistUser(String name)
100
+ {
101
+ User existing = userRepo.findByOidcSubOptional(name).orElse(null);
102
+ if (existing != null)
103
+ {
104
+ return existing;
105
+ }
106
+ User user = new User();
107
+ user.oidcSub = name;
108
+ user.username = name;
109
+ user.persist();
110
+ return user;
111
+ }
112
+}
ADD
src/test/java/de/workaround/web/IssueCommentUiTest.java
+142 -0
@@ -0,0 +1,142 @@
1
+package de.workaround.web;
2
+
3
+import java.util.UUID;
4
+
5
+import org.junit.jupiter.api.Test;
6
+
7
+import de.workaround.git.GitRepositoryService;
8
+import de.workaround.git.IssueCommentService;
9
+import de.workaround.git.IssueService;
10
+import de.workaround.model.Issue;
11
+import de.workaround.model.IssueComment;
12
+import de.workaround.model.Repository;
13
+import de.workaround.model.User;
14
+import io.quarkus.test.junit.QuarkusTest;
15
+import io.quarkus.test.security.TestSecurity;
16
+import jakarta.inject.Inject;
17
+import jakarta.transaction.Transactional;
18
+
19
+import static io.restassured.RestAssured.given;
20
+import static org.hamcrest.CoreMatchers.containsString;
21
+import static org.hamcrest.CoreMatchers.not;
22
+
23
+@QuarkusTest
24
+class IssueCommentUiTest
25
+{
26
+ @Inject
27
+ GitRepositoryService service;
28
+
29
+ @Inject
30
+ IssueService issueService;
31
+
32
+ @Inject
33
+ IssueCommentService comments;
34
+
35
+ @Inject
36
+ User.Repo userRepo;
37
+
38
+ @Test
39
+ @TestSecurity(user = "ic-owner")
40
+ void ownerCanCommentOnAnIssueAndSeeItInline()
41
+ {
42
+ User owner = persistUser("ic-owner");
43
+ Repository repo = service.create(owner, "board", Repository.Visibility.PUBLIC, null);
44
+ Issue issue = issueService.create(owner, repo, "Discuss me", null);
45
+ String detail = "/repos/" + owner.username + "/board/issues/" + issue.number;
46
+
47
+ // the detail page carries a comment thread section and a compose form
48
+ given().when().get(detail)
49
+ .then().statusCode(200)
50
+ .body(containsString("Comments"))
51
+ .body(containsString(detail + "/comments"));
52
+
53
+ given().redirects().follow(false).contentType("application/x-www-form-urlencoded")
54
+ .formParam("body", "first thought")
55
+ .when().post(detail + "/comments")
56
+ .then().statusCode(303);
57
+
58
+ given().when().get(detail)
59
+ .then().statusCode(200)
60
+ .body(containsString("first thought"))
61
+ .body(containsString("ic-owner"));
62
+ }
63
+
64
+ @Test
65
+ @TestSecurity(user = "ic-reader")
66
+ void aReaderWhoIsNotTheOwnerCanCommentOnAPublicIssue()
67
+ {
68
+ persistUser("ic-reader");
69
+ User owner = persistUser("ic-owner2-" + UUID.randomUUID().toString().substring(0, 8));
70
+ Repository repo = service.create(owner, "board", Repository.Visibility.PUBLIC, null);
71
+ Issue issue = issueService.create(owner, repo, "Open to all", null);
72
+ String detail = "/repos/" + owner.username + "/board/issues/" + issue.number;
73
+
74
+ given().redirects().follow(false).contentType("application/x-www-form-urlencoded")
75
+ .formParam("body", "outside remark")
76
+ .when().post(detail + "/comments")
77
+ .then().statusCode(303);
78
+
79
+ given().when().get(detail).then().statusCode(200).body(containsString("outside remark"));
80
+ }
81
+
82
+ @Test
83
+ void anonymousCannotComment()
84
+ {
85
+ User owner = persistUser("ic-anon-" + UUID.randomUUID().toString().substring(0, 8));
86
+ Repository repo = service.create(owner, "board", Repository.Visibility.PUBLIC, null);
87
+ Issue issue = issueService.create(owner, repo, "No anon", null);
88
+ String detail = "/repos/" + owner.username + "/board/issues/" + issue.number;
89
+
90
+ given().contentType("application/x-www-form-urlencoded").formParam("body", "sneaky")
91
+ .when().post(detail + "/comments")
92
+ .then().statusCode(403);
93
+ }
94
+
95
+ @Test
96
+ @TestSecurity(user = "ic-empty")
97
+ void emptyCommentIsRejected()
98
+ {
99
+ User owner = persistUser("ic-empty");
100
+ Repository repo = service.create(owner, "board", Repository.Visibility.PUBLIC, null);
101
+ Issue issue = issueService.create(owner, repo, "Blank", null);
102
+ String detail = "/repos/" + owner.username + "/board/issues/" + issue.number;
103
+
104
+ given().contentType("application/x-www-form-urlencoded").formParam("body", " ")
105
+ .when().post(detail + "/comments")
106
+ .then().statusCode(400);
107
+ }
108
+
109
+ @Test
110
+ @TestSecurity(user = "ic-del")
111
+ void authorCanDeleteTheirComment()
112
+ {
113
+ User owner = persistUser("ic-del");
114
+ Repository repo = service.create(owner, "board", Repository.Visibility.PUBLIC, null);
115
+ Issue issue = issueService.create(owner, repo, "Deletable", null);
116
+ IssueComment comment = comments.add(owner, issue, "temporary note");
117
+ String detail = "/repos/" + owner.username + "/board/issues/" + issue.number;
118
+
119
+ given().when().get(detail).then().statusCode(200).body(containsString("temporary note"));
120
+
121
+ given().redirects().follow(false).contentType("application/x-www-form-urlencoded")
122
+ .when().post(detail + "/comments/" + comment.id + "/delete")
123
+ .then().statusCode(303);
124
+
125
+ given().when().get(detail).then().statusCode(200).body(not(containsString("temporary note")));
126
+ }
127
+
128
+ @Transactional
129
+ User persistUser(String name)
130
+ {
131
+ User existing = userRepo.findByOidcSubOptional(name).orElse(null);
132
+ if (existing != null)
133
+ {
134
+ return existing;
135
+ }
136
+ User user = new User();
137
+ user.oidcSub = name;
138
+ user.username = name;
139
+ user.persist();
140
+ return user;
141
+ }
142
+}
ADD
src/test/java/de/workaround/web/MergeRequestDiscussionUiTest.java
+115 -0
@@ -0,0 +1,115 @@
1
+package de.workaround.web;
2
+
3
+import java.nio.charset.StandardCharsets;
4
+import java.nio.file.Files;
5
+import java.nio.file.Path;
6
+import java.util.Map;
7
+import java.util.UUID;
8
+
9
+import org.eclipse.jgit.api.Git;
10
+import org.eclipse.jgit.transport.RefSpec;
11
+import org.junit.jupiter.api.Test;
12
+
13
+import de.workaround.git.GitRepositoryService;
14
+import de.workaround.git.GitTestSeeder;
15
+import de.workaround.git.MergeRequestService;
16
+import de.workaround.model.MergeRequest;
17
+import de.workaround.model.Repository;
18
+import de.workaround.model.User;
19
+import io.quarkus.test.junit.QuarkusTest;
20
+import io.quarkus.test.security.TestSecurity;
21
+import jakarta.inject.Inject;
22
+import jakarta.transaction.Transactional;
23
+
24
+import static io.restassured.RestAssured.given;
25
+import static org.hamcrest.CoreMatchers.containsString;
26
+
27
+@QuarkusTest
28
+class MergeRequestDiscussionUiTest
29
+{
30
+ @Inject
31
+ GitRepositoryService service;
32
+
33
+ @Inject
34
+ MergeRequestService mergeRequests;
35
+
36
+ @Inject
37
+ User.Repo userRepo;
38
+
39
+ @Test
40
+ @TestSecurity(user = "mrd-owner")
41
+ void ownerCanPostAGeneralDiscussionCommentAndSeeIt()
42
+ {
43
+ User owner = persistUser("mrd-owner");
44
+ MergeRequest mr = seededMr(owner, "board");
45
+ String detail = "/repos/" + owner.username + "/board/merge-requests/" + mr.number;
46
+
47
+ // a general discussion thread (distinct from per-line review comments) with a compose form
48
+ given().when().get(detail)
49
+ .then().statusCode(200)
50
+ .body(containsString("Discussion"))
51
+ .body(containsString(detail + "/discussion"));
52
+
53
+ given().redirects().follow(false).contentType("application/x-www-form-urlencoded")
54
+ .formParam("body", "general remark on this MR")
55
+ .when().post(detail + "/discussion")
56
+ .then().statusCode(303);
57
+
58
+ given().when().get(detail)
59
+ .then().statusCode(200)
60
+ .body(containsString("general remark on this MR"))
61
+ .body(containsString("mrd-owner"));
62
+ }
63
+
64
+ @Test
65
+ void anonymousCannotPostDiscussion()
66
+ {
67
+ User owner = persistUser("mrd-anon-" + UUID.randomUUID().toString().substring(0, 8));
68
+ MergeRequest mr = seededMr(owner, "board");
69
+ String detail = "/repos/" + owner.username + "/board/merge-requests/" + mr.number;
70
+
71
+ given().contentType("application/x-www-form-urlencoded").formParam("body", "sneaky")
72
+ .when().post(detail + "/discussion")
73
+ .then().statusCode(403);
74
+ }
75
+
76
+ private MergeRequest seededMr(User owner, String name)
77
+ {
78
+ try
79
+ {
80
+ Repository repo = service.create(owner, name, Repository.Visibility.PUBLIC, null);
81
+ Path bare = service.repositoryPath(repo);
82
+ GitTestSeeder.seed(bare, Map.of("base.txt", "base\n".getBytes(StandardCharsets.UTF_8)));
83
+ Path work = Files.createTempDirectory("mrdseed");
84
+ try (Git git = Git.cloneRepository().setURI(bare.toUri().toString()).setDirectory(work.toFile()).call())
85
+ {
86
+ git.checkout().setCreateBranch(true).setName("feature").call();
87
+ Files.writeString(work.resolve("feature.txt"), "feature\n");
88
+ git.add().addFilepattern(".").call();
89
+ git.commit().setMessage("feature work").setSign(false)
90
+ .setAuthor("dev", "dev@example.com").setCommitter("dev", "dev@example.com").call();
91
+ git.push().setRefSpecs(new RefSpec("feature:refs/heads/feature")).call();
92
+ }
93
+ return mergeRequests.create(owner, repo, "Review me", null, "feature", "main");
94
+ }
95
+ catch (Exception e)
96
+ {
97
+ throw new RuntimeException(e);
98
+ }
99
+ }
100
+
101
+ @Transactional
102
+ User persistUser(String name)
103
+ {
104
+ User existing = userRepo.findByOidcSubOptional(name).orElse(null);
105
+ if (existing != null)
106
+ {
107
+ return existing;
108
+ }
109
+ User user = new User();
110
+ user.oidcSub = name;
111
+ user.username = name;
112
+ user.persist();
113
+ return user;
114
+ }
115
+}