Skip to content
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

Jump to bottom

Integrate Merge PR API #8232

Merged
merged 15 commits into from
Sep 29, 2024
Merged

Integrate Merge PR API #8232

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d41ffad
Update src-dest UI in Pulls list
itaigilo Sep 25, 2024
499f6aa
Show closed/merged timestamp in the Pulls list
itaigilo Sep 25, 2024
11dd9c8
Show merged commit link in merged PR details
itaigilo Sep 25, 2024
db1a1e3
Cleanup
itaigilo Sep 25, 2024
dcda563
Handle empty diffs
itaigilo Sep 25, 2024
7dd22d9
Handle deleted branch errors
itaigilo Sep 25, 2024
62e0477
Disable button when branch not found
itaigilo Sep 25, 2024
f6bb51c
Merge branch 'master' into task/prfd-ux-ui-improvements
itaigilo Sep 25, 2024
d3d85eb
Update field name
itaigilo Sep 25, 2024
b55484e
Fix PR comments
itaigilo Sep 26, 2024
b39de22
Integrate Merge PR API
itaigilo Sep 26, 2024
3be755f
Remove log
itaigilo Sep 26, 2024
01f76ad
Merge branch 'master' into task/prfd-integrate-pr-merge-api
itaigilo Sep 26, 2024
e66757b
Merge branch 'master' into task/prfd-integrate-pr-merge-api
itaigilo Sep 27, 2024
94cee56
Update error message
itaigilo Sep 29, 2024
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
7 changes: 4 additions & 3 deletions webui/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion webui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
"bootstrap": "^5.3.2",
"csstype": "^3.1.3",
"dayjs": "^1.11.10",
"exponential-backoff": "^3.1.1",
"lodash": "^4.17.21",
"p-map": "^7.0.0",
"prismjs": "^1.29.0",
Expand Down
20 changes: 20 additions & 0 deletions webui/src/lib/api/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,26 @@ class Pulls {
return response.json()
}

async merge(repoId, pullId) {
const response = await apiRequest(`/repositories/${encodeURIComponent(repoId)}/pulls/${encodeURIComponent(pullId)}/merge`, {
method: 'PUT',
});
if (response.status !== 200) {
const baseMessage = 'Could not merge pull request';
switch (response.status) {
case 400:
case 401:
case 403:
case 404:
case 409:
case 412:
throw new Error(`${baseMessage}: ${(await response.json()).message}`);
default:
throw new Error(`${baseMessage} (status = ${response.status}).`);
}
}
}

async update(repoId, pullId, pullDetails) {
const response = await apiRequest(`/repositories/${encodeURIComponent(repoId)}/pulls/${encodeURIComponent(pullId)}`, {
method: 'PATCH',
Expand Down
2 changes: 1 addition & 1 deletion webui/src/pages/repositories/repository/pulls/createPull.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ const CreatePullForm = ({repo, reference, compare}) => {
{error && <AlertError error={error} onDismiss={() => setError(null)}/>}
<div>
<Button variant="success"
disabled={!title || !description || loading || isEmptyDiff}
disabled={!title || loading || isEmptyDiff}
onClick={submitForm}>
{loading && <><span className="spinner-border spinner-border-sm text-light" role="status"/> {""}</>}
Create Pull Request
Expand Down
11 changes: 2 additions & 9 deletions webui/src/pages/repositories/repository/pulls/pullDetails.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ import Card from "react-bootstrap/Card";
import {GitMergeIcon, GitPullRequestClosedIcon, GitPullRequestIcon} from "@primer/octicons-react";
import dayjs from "dayjs";
import Markdown from 'react-markdown'
import {backOff} from "exponential-backoff";

import {AlertError, Loading} from "../../../../lib/components/controls";
import {useRefs} from "../../../../lib/hooks/repo";
import {useRouter} from "../../../../lib/hooks/router";
import {RepoError} from "../error";
import {pulls as pullsAPI, refs as refsAPI} from "../../../../lib/api";
import {pulls as pullsAPI} from "../../../../lib/api";
import {useAPI} from "../../../../lib/hooks/api";
import {Link} from "../../../../lib/components/nav";
import CompareBranches from "../../../../lib/components/repository/compareBranches";
Expand All @@ -32,18 +31,12 @@ const PullDetailsContent = ({repo, pull}) => {
setError(null);
setLoading(true);
try {
await refsAPI.merge(repo.id, pull.source_branch, pull.destination_branch);
await pullsAPI.merge(repo.id, pull.id);
} catch (error) {
setError(`Failed to merge pull request: ${error.message}`);
Copy link
Member

Choose a reason for hiding this comment

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

I think the error here may be confusing. The merge might have been successful and the failure was on the update.
Since we are returning a verbose error message from the server, lets just use it as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right! It's there since we had two different errors (merge + status change), and now it isn't relevant anymore.
Changed.

setLoading(false);
return;
}
try {
await backOff(() => pullsAPI.update(repo.id, pull.id, {status: PullStatus.merged}));
} catch (error) {
setError(`Failed to update pull request status: ${error.message}`);
setLoading(false);
}
window.location.reload(); // TODO (gilo): replace with a more elegant solution
}

Expand Down
Loading