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.

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

added missing /repos/{owner}/{repo}/pulls/... handlers (#546) #605

Merged
merged 12 commits into from
Jul 29, 2024
Merged
1 change: 0 additions & 1 deletion src/api/checks.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use chrono::{DateTime, Utc};
use hyper::body::Body;

use crate::models::checks::{AutoTriggerCheck, CheckSuite, CheckSuitePreferences};
use crate::models::{AppId, CheckRunId, CheckSuiteId};
Expand Down
47 changes: 40 additions & 7 deletions src/api/pulls.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,26 @@
//! The pull request API.

mod comment;
mod create;
mod list;
mod merge;
mod update;

use http::request::Builder;
use http::{Method, Uri};

use snafu::ResultExt;

use crate::error::HttpSnafu;
use crate::models::CommentId;
use crate::pulls::specific_pr::SpecificPullRequestBuilder;
use crate::{Octocrab, Page};

pub use self::{
create::CreatePullRequestBuilder, list::ListPullRequestsBuilder,
update::UpdatePullRequestBuilder,
};

mod comment;
mod create;
mod list;
mod merge;
mod specific_pr;
mod update;

/// A client to GitHub's pull request API.
///
/// Created with [`Octocrab::pulls`].
Expand Down Expand Up @@ -369,6 +371,37 @@ impl<'octo> PullRequestHandler<'octo> {
comment::ListCommentsBuilder::new(self, pr)
}

///creates a new `CommentBuilder` for GET/PATCH/DELETE requests
/// to the `/repos/{owner}/{repo}/pulls/{pr}/comments/{comment_id}` endpoint
/// ```no_run
/// use octocrab::models::CommentId;
/// use octocrab::models::pulls::Comment;
/// async fn run() -> octocrab::Result<Comment> {
/// let octocrab = octocrab::Octocrab::default();
/// let _ = octocrab.pulls("owner", "repo").comment(CommentId(21)).delete();
/// let _ = octocrab.pulls("owner", "repo").comment(CommentId(42)).update("new comment");
/// let comment = octocrab.pulls("owner", "repo").comment(CommentId(42)).get().await;
///
/// comment
/// }
/// ```
pub fn comment(&self, comment_id: CommentId) -> comment::CommentBuilder {
comment::CommentBuilder::new(self, comment_id)
}

/// creates a builder for the `/repos/{owner}/{repo}/pulls/{pull_number}/......` endpoint
/// working with particular pull request, e.g.
/// * /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/events
/// * /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}
/// * /repos/{owner}/{repo}/pulls/{pull_number}/commits
/// * /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/comments
/// * /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/dismissals
/// * /repos/{owner}/{repo}/pulls/{pull_number}/comments/{comment_id}/replies
///
pub fn pull_number(&self, pull_nr: u64) -> SpecificPullRequestBuilder {
SpecificPullRequestBuilder::new(self, pull_nr)
}
Copy link
Owner

Choose a reason for hiding this comment

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

I think instead of having a specific builder for this, we should instead mimic octokit here, and have each of these methods attached to the pulls builder, and have the pull number as a required argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So far it is looking like

octocrab.pulls("owner", "repo")
    .pull_number(42)
    .reviews()
        .review(42).get()

octocrab.pulls("owner", "repo")
    .pull_number(42)
    .reviews()
        .review(42).update("this is a new body")

octocrab.pulls("owner", "repo")
    .pull_number(42)
    .comment(CommentId(42))
    .reply("new comment")

octocrab.pulls("owner", "repo")
    .pull_number(42)
    .reviews().review(42)
    .list_comments()
        .per_page(10)
        .page(3u32)
        .send()

How do you prefer it? Octokit does it like

[ManualRoute("GET", "/repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/comments")]
        public Task<IReadOnlyList<PullRequestReviewComment>> GetAllComments(string owner, string name, int number, long reviewId, ApiOptions options)

so we would have
-=1=-

octocrab.pulls("owner", "repo")
        .get_all_comments(/*pr_nr*/ 42, /*review_nr*/ 42)
        .per_page(10)
        .page(3u32)
        .send()

or even
-=2=-

octocrab.pulls("owner", "repo")
        .get_all_comments(/*pr_nr*/ 42, /*review_nr*/ 42, /*pagination*/ PagingOptions{...})

?

IMHO, in absence of named arguments, the following options are more readable:

  • Builder pattern;
  • Struct parameter.

Anyways, I sense we go with -=1=- (to retain Octocrab pagination style)? TIA @XAMPPRocky

Copy link
Owner

@XAMPPRocky XAMPPRocky Mar 29, 2024

Choose a reason for hiding this comment

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

Yes, 1, I just mean removing the pull_number builder and having the pr number as the first argument in these calls. Keeping the other builders.

Copy link
Contributor Author

@dmgorsky dmgorsky Apr 5, 2024

Choose a reason for hiding this comment

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

@XAMPPRocky please check if it suits: 377342b


/// Creates a new `MergePullRequestsBuilder` that can be configured used to
/// merge a pull request.
/// ```no_run
Expand Down
74 changes: 74 additions & 0 deletions src/api/pulls/comment.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use serde_json::json;

use crate::models::pulls::Comment;

use super::*;

/// A builder pattern struct for listing comments.
Expand Down Expand Up @@ -84,6 +88,76 @@ impl<'octo, 'b> ListCommentsBuilder<'octo, 'b> {
}
}

/// A builder pattern struct for working with specific comment.
///
/// created by [`PullRequestHandler::comment`]
///
/// [`PullRequestHandler::comment`]: ./struct.PullRequestHandler.html#method.comment
#[derive(serde::Serialize)]
pub struct CommentBuilder<'octo, 'b> {
#[serde(skip)]
handler: &'b PullRequestHandler<'octo>,
comment_id: CommentId,
}

impl<'octo, 'b> CommentBuilder<'octo, 'b> {
pub(crate) fn new(handler: &'b PullRequestHandler<'octo>, comment_id: CommentId) -> Self {
Self {
handler,
comment_id,
}
}

///https://docs.github.com/en/rest/pulls/comments?apiVersion=2022-11-28#get-a-review-comment-for-a-pull-request
pub async fn get(self) -> crate::Result<Comment> {
self.handler
.crab
.get(
format!(
"/repos/{owner}/{repo}/pulls/comments/{comment_id}",
owner = self.handler.owner,
repo = self.handler.repo,
comment_id = self.comment_id
),
None::<&Comment>,
)
.await
}

///https://docs.github.com/en/rest/pulls/comments?apiVersion=2022-11-28#update-a-review-comment-for-a-pull-request
pub async fn update(self, comment: &str) -> crate::Result<Comment> {
self.handler
.crab
.patch(
format!(
"/repos/{owner}/{repo}/pulls/comments/{comment_id}",
owner = self.handler.owner,
repo = self.handler.repo,
comment_id = self.comment_id
),
Some(&json!({ "body": comment })),
)
.await
}

///https://docs.github.com/en/rest/pulls/comments?apiVersion=2022-11-28#delete-a-review-comment-for-a-pull-request
pub async fn delete(self) -> crate::Result<()> {
self.handler
.crab
._delete(
format!(
"/repos/{owner}/{repo}/pulls/comments/{comment_id}",
owner = self.handler.owner,
repo = self.handler.repo,
comment_id = self.comment_id
),
None::<&()>,
)
.await?;
Ok(())
}
}

#[cfg(test)]
mod tests {
#[tokio::test]
Expand Down
101 changes: 101 additions & 0 deletions src/api/pulls/specific_pr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
use crate::models::repos::RepoCommit;
use crate::models::CommentId;
use crate::pulls::specific_pr::pr_comment::SpecificPullRequestCommentBuilder;
use crate::pulls::specific_pr::pr_reviews::ReviewsBuilder;
use crate::pulls::PullRequestHandler;
use crate::Page;

mod pr_comment;
mod pr_reviews;
/// A builder pattern struct for working with a specific pull request data,
/// e.g. reviews, commits, comments, etc.
///
/// created by [`PullRequestHandler::pull_number`]
///
/// [`PullRequestHandler::pull_number`]: ./struct.PullRequestHandler.html#method.pull_number
#[derive(serde::Serialize)]
pub struct SpecificPullRequestBuilder<'octo, 'b> {
#[serde(skip)]
handler: &'b PullRequestHandler<'octo>,
pr_number: u64,
#[serde(skip_serializing_if = "Option::is_none")]
per_page: Option<u8>,

#[serde(skip_serializing_if = "Option::is_none")]
page: Option<u32>,
}

impl<'octo, 'b> SpecificPullRequestBuilder<'octo, 'b> {
pub(crate) fn new(handler: &'b PullRequestHandler<'octo>, pr_number: u64) -> Self {
Self {
handler,
pr_number,
per_page: None,
page: None,
}
}

/// Results per page (max: 100, default: 30).
pub fn per_page(mut self, per_page: impl Into<u8>) -> Self {
self.per_page = Some(per_page.into());
self
}

/// Page number of the results to fetch. (default: 1)
pub fn page(mut self, page: impl Into<u32>) -> Self {
self.page = Some(page.into());
self
}

///Lists a maximum of 250 commits for a pull request.
/// To receive a complete commit list for pull requests with more than 250 commits,
/// use the [List commits](https://docs.github.com/rest/commits/commits#list-commits) endpoint.
pub async fn commits(&self) -> crate::Result<Page<RepoCommit>> {
let route = format!(
"/repos/{owner}/{repo}/pulls/{pr_number}/commits",
owner = self.handler.owner,
repo = self.handler.repo,
pr_number = self.pr_number
);
self.handler.crab.get(route, Some(&self)).await
}

/// Creates a new `ReviewsBuilder`
/// ```no_run
/// # use octocrab::models::CommentId;
/// async fn run() -> octocrab::Result<()> {
/// # let octocrab = octocrab::Octocrab::default();
/// use octocrab::params;
///
/// let _ = octocrab.pulls("owner", "repo")
/// .pull_number(42)
/// .reviews()
/// .review(42)
/// .get()
/// .await;
/// Ok(())
/// }
/// ```
pub fn reviews(&self) -> ReviewsBuilder<'octo, '_> {
ReviewsBuilder::new(self.handler, self.pr_number)
}

/// Creates a new `SpecificPullRequestCommentBuilder`
/// ```no_run
/// # use octocrab::models::CommentId;
/// async fn run() -> octocrab::Result<()> {
/// # let octocrab = octocrab::Octocrab::default();
/// use octocrab::params;
///
/// let _ = octocrab.pulls("owner", "repo")
/// .pull_number(42)
/// .comment(CommentId(42))
/// .reply("new comment")
/// .await;
/// Ok(())
/// }
/// ```
pub fn comment(&self, comment_id: CommentId) -> SpecificPullRequestCommentBuilder {
SpecificPullRequestCommentBuilder::new(self.handler, self.pr_number, comment_id)
}
}
41 changes: 41 additions & 0 deletions src/api/pulls/specific_pr/pr_comment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
use serde_json::json;

use crate::models::pulls::ReviewComment;
use crate::models::CommentId;
use crate::pulls::PullRequestHandler;

#[derive(serde::Serialize)]
pub struct SpecificPullRequestCommentBuilder<'octo, 'b> {
#[serde(skip)]
handler: &'b PullRequestHandler<'octo>,
pr_number: u64,
comment_id: CommentId,
}

impl<'octo, 'b> SpecificPullRequestCommentBuilder<'octo, 'b> {
pub(crate) fn new(
handler: &'b PullRequestHandler<'octo>,
pr_number: u64,
comment_id: CommentId,
) -> Self {
Self {
handler,
comment_id,
pr_number,
}
}

pub async fn reply(&self, comment: impl Into<String>) -> crate::Result<ReviewComment> {
let route = format!(
"/repos/{owner}/{repo}/pulls/{pull_number}/comments/{comment_id}/replies",
owner = self.handler.owner,
repo = self.handler.repo,
pull_number = self.pr_number,
comment_id = self.comment_id
);
self.handler
.crab
.post(route, Some(&json!({ "body": comment.into() })))
.await
}
}
45 changes: 45 additions & 0 deletions src/api/pulls/specific_pr/pr_reviews.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
use crate::pulls::specific_pr::pr_reviews::specific_review::SpecificReviewBuilder;
use crate::pulls::PullRequestHandler;

mod specific_review;

#[derive(serde::Serialize)]
pub struct ReviewsBuilder<'octo, 'b> {
#[serde(skip)]
handler: &'b PullRequestHandler<'octo>,
#[serde(skip_serializing_if = "Option::is_none")]
per_page: Option<u8>,
#[serde(skip_serializing_if = "Option::is_none")]
page: Option<u32>,
pr_number: u64,
}

impl<'octo, 'b> ReviewsBuilder<'octo, 'b> {
pub(crate) fn new(handler: &'b PullRequestHandler<'octo>, pr_number: u64) -> Self {
Self {
handler,
per_page: None,
page: None,
pr_number,
}
}

/// Creates a new `SpecificReviewBuilder`
/// ```no_run
/// # async fn run() -> octocrab::Result<()> {
/// # let octocrab = octocrab::Octocrab::default();
/// use octocrab::params;
///
/// let _ = octocrab.pulls("owner", "repo")
/// .pull_number(42)
/// .reviews()
/// .review(42)
/// .get() // + update, delete_pending, submit, dismiss, list_comments
/// .await?;
/// # Ok(())
/// # }
/// ```
pub fn review(&self, review_id: u64) -> SpecificReviewBuilder<'octo, '_> {
SpecificReviewBuilder::new(self.handler, self.pr_number, review_id)
}
}
Loading
Loading