mirror of
https://codeberg.org/davrot/forgejo.git
synced 2025-07-07 07:00:01 +02:00

This implements the UI controls and information displays necessary to allow reviewing pull requests by stepping through commits individually. Notable changes: - Within the PR page, commit links now stay in the PR context by navigating to `{owner}/{repo}/pulls/{id}/commits/{sha}` - When showing a single commit in the "Files changed" tab, the commit header containing commit message and metadata is displayed - I dropped the existing buttons, since they make less sense to me in the PR context - The SHA links to the separate, dedicated commit view - "Previous"/"Next" buttons have been added to that header to allow stepping through commits - Reviews can be submitted in "single commit" view Talking points: - The "Showing only changes from" banner made sense when that view was limited (e.g. review submit was disabled). Now that it's on par with the "all commits" view, and visually distinct due to the commit header, this banner could potentially be dropped. Closes: #5670 #5126 #5671 #2281 #8084  ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [x] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7155 Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Reviewed-by: Beowulf <beowulf@beocode.eu> Co-authored-by: Lucas Schwiderski <lucas@lschwiderski.de> Co-committed-by: Lucas Schwiderski <lucas@lschwiderski.de>
114 lines
5 KiB
TypeScript
114 lines
5 KiB
TypeScript
// Copyright 2025 The Forgejo Authors. All rights reserved.
|
|
// SPDX-License-Identifier: GPL-3.0-or-later
|
|
|
|
// @watch start
|
|
// templates/repo/diff/new_review.tmpl
|
|
// web_src/js/features/repo-issue.js
|
|
// @watch end
|
|
|
|
import {expect} from '@playwright/test';
|
|
import {save_visual, test} from './utils_e2e.ts';
|
|
|
|
test.use({user: 'user2'});
|
|
|
|
test('PR: Create review from files', async ({page}) => {
|
|
const response = await page.goto('/user2/repo1/pulls/5/files');
|
|
expect(response?.status()).toBe(200);
|
|
|
|
await expect(page.locator('.tippy-box .review-box-panel')).toBeHidden();
|
|
await save_visual(page);
|
|
|
|
// Review panel should appear after clicking Finish review
|
|
await page.locator('#review-box .js-btn-review').click();
|
|
await expect(page.locator('.tippy-box .review-box-panel')).toBeVisible();
|
|
await save_visual(page);
|
|
|
|
await page.locator('.review-box-panel textarea#_combo_markdown_editor_0')
|
|
.fill('This is a review');
|
|
await page.locator('.review-box-panel button.btn-submit[value="approve"]').click();
|
|
await page.waitForURL(/.*\/user2\/repo1\/pulls\/5#issuecomment-\d+/);
|
|
await save_visual(page);
|
|
});
|
|
|
|
test('PR: Create review from commit', async ({page}) => {
|
|
const response = await page.goto('/user2/repo1/pulls/3/commits/4a357436d925b5c974181ff12a994538ddc5a269');
|
|
expect(response?.status()).toBe(200);
|
|
|
|
await page.locator('button.add-code-comment').click();
|
|
const code_comment = page.locator('.comment-code-cloud form textarea.markdown-text-editor');
|
|
await expect(code_comment).toBeVisible();
|
|
|
|
await code_comment.fill('This is a code comment');
|
|
await save_visual(page);
|
|
|
|
const start_button = page.locator('.comment-code-cloud form button.btn-start-review');
|
|
// Workaround for #7152, where there might already be a pending review state from previous
|
|
// test runs (most likely to happen when debugging tests).
|
|
if (await start_button.isVisible({timeout: 100})) {
|
|
await start_button.click();
|
|
} else {
|
|
await page.locator('.comment-code-cloud form button.btn-add-comment').click();
|
|
}
|
|
|
|
await expect(page.locator('.comment-list .comment-container')).toBeVisible();
|
|
|
|
// We need to wait for the review to be processed. Checking the comment counter
|
|
// conveniently does that.
|
|
await expect(page.locator('#review-box .js-btn-review > span.review-comments-counter')).toHaveText('1');
|
|
|
|
await page.locator('#review-box .js-btn-review').click();
|
|
await expect(page.locator('.tippy-box .review-box-panel')).toBeVisible();
|
|
await save_visual(page);
|
|
|
|
await page.locator('.review-box-panel textarea.markdown-text-editor')
|
|
.fill('This is a review');
|
|
await page.locator('.review-box-panel button.btn-submit[value="approve"]').click();
|
|
await page.waitForURL(/.*\/user2\/repo1\/pulls\/3#issuecomment-\d+/);
|
|
await save_visual(page);
|
|
|
|
// In addition to testing the ability to delete comments, this also
|
|
// performs clean up. If tests are run for multiple platforms, the data isn't reset
|
|
// in-between, and subsequent runs of this test would fail, because when there already is
|
|
// a comment, the on-hover button to start a conversation doesn't appear anymore.
|
|
await page.goto('/user2/repo1/pulls/3/commits/4a357436d925b5c974181ff12a994538ddc5a269');
|
|
await page.locator('.comment-header-right.actions a.context-menu').click();
|
|
|
|
await expect(page.locator('.comment-header-right.actions div.menu').getByText(/Copy link.*/)).toBeVisible();
|
|
// The button to delete a comment will prompt for confirmation using a browser alert.
|
|
page.on('dialog', (dialog) => dialog.accept());
|
|
await page.locator('.comment-header-right.actions div.menu .delete-comment').click();
|
|
|
|
await expect(page.locator('.comment-list .comment-container')).toBeHidden();
|
|
await save_visual(page);
|
|
});
|
|
|
|
test('PR: Navigate by single commit', async ({page}) => {
|
|
const response = await page.goto('/user2/repo1/pulls/3/commits');
|
|
expect(response?.status()).toBe(200);
|
|
|
|
await page.locator('tbody.commit-list td.message a').nth(1).click();
|
|
await page.waitForURL(/.*\/user2\/repo1\/pulls\/3\/commits\/4a357436d925b5c974181ff12a994538ddc5a269/);
|
|
await save_visual(page);
|
|
|
|
let prevButton = page.locator('.commit-header-buttons').getByText(/Prev/);
|
|
let nextButton = page.locator('.commit-header-buttons').getByText(/Next/);
|
|
await prevButton.waitFor();
|
|
await nextButton.waitFor();
|
|
|
|
await expect(prevButton).toHaveClass(/disabled/);
|
|
await expect(nextButton).not.toHaveClass(/disabled/);
|
|
await expect(nextButton).toHaveAttribute('href', '/user2/repo1/pulls/3/commits/5f22f7d0d95d614d25a5b68592adb345a4b5c7fd');
|
|
await nextButton.click();
|
|
|
|
await page.waitForURL(/.*\/user2\/repo1\/pulls\/3\/commits\/5f22f7d0d95d614d25a5b68592adb345a4b5c7fd/);
|
|
await save_visual(page);
|
|
|
|
prevButton = page.locator('.commit-header-buttons').getByText(/Prev/);
|
|
nextButton = page.locator('.commit-header-buttons').getByText(/Next/);
|
|
await prevButton.waitFor();
|
|
await nextButton.waitFor();
|
|
|
|
await expect(prevButton).not.toHaveClass(/disabled/);
|
|
await expect(nextButton).toHaveClass(/disabled/);
|
|
await expect(prevButton).toHaveAttribute('href', '/user2/repo1/pulls/3/commits/4a357436d925b5c974181ff12a994538ddc5a269');
|
|
});
|