Skip to content

Commit d18d282

Browse files
committed
fix review dismissal and thread comment updating
1 parent e749061 commit d18d282

File tree

4 files changed

+56
-46
lines changed

4 files changed

+56
-46
lines changed

cpp-linter/src/rest_api/github/serde_structs.rs

+3
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ impl From<Suggestion> for ReviewDiffComment {
3636
}
3737
}
3838

39+
/// A constant string used as a payload to dismiss PR reviews.
40+
pub const REVIEW_DISMISSAL: &str = r#"{"event":"DISMISS","message":"outdated suggestion"}"#;
41+
3942
/// A structure for deserializing a single changed file in a CI event.
4043
#[derive(Debug, Deserialize, PartialEq, Clone)]
4144
pub struct GithubChangedFile {

cpp-linter/src/rest_api/github/specific_api.rs

+41-44
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ use crate::{
1919
};
2020

2121
use super::{
22-
serde_structs::{FullReview, PullRequestInfo, ReviewComment, ReviewDiffComment, ThreadComment},
22+
serde_structs::{
23+
FullReview, PullRequestInfo, ReviewComment, ReviewDiffComment, ThreadComment,
24+
REVIEW_DISMISSAL,
25+
},
2326
GithubApiClient, RestApiClient,
2427
};
2528

@@ -206,9 +209,14 @@ impl GithubApiClient {
206209
})?,
207210
);
208211
let repo = format!(
209-
"repos/{}/comments",
212+
"repos/{}{}/comments",
210213
// if we got here, then we know it is on a CI runner as self.repo should be known
211-
self.repo.as_ref().expect("Repo name unknown.")
214+
self.repo.as_ref().expect("Repo name unknown."),
215+
if self.event_name == "pull_request" {
216+
"/issues"
217+
} else {
218+
""
219+
},
212220
);
213221
let base_comment_url = self.api_url.join(&repo).unwrap();
214222
while let Some(ref endpoint) = comments_url {
@@ -454,51 +462,40 @@ impl GithubApiClient {
454462
&& !(["PENDING", "DISMISSED"].contains(&review.state.as_str()))
455463
{
456464
// dismiss outdated review
457-
match url.join("reviews/")?.join(review.id.to_string().as_str()) {
458-
Ok(dismiss_url) => {
459-
if let Ok(req) = Self::make_api_request(
460-
&self.client,
461-
dismiss_url,
462-
Method::PUT,
463-
Some(
464-
serde_json::json!(
465-
{
466-
"message": "outdated suggestion",
467-
"event": "DISMISS"
468-
}
469-
)
470-
.to_string(),
471-
),
472-
None,
473-
) {
474-
match Self::send_api_request(
475-
self.client.clone(),
476-
req,
477-
self.rate_limit_headers.clone(),
478-
0,
479-
)
480-
.await
481-
{
482-
Ok(result) => {
483-
if !result.status().is_success() {
484-
Self::log_response(
485-
result,
486-
"Failed to dismiss outdated review",
487-
)
488-
.await;
489-
}
490-
}
491-
Err(e) => {
492-
log::error!(
493-
"Failed to dismiss outdated review: {e:}"
494-
);
465+
if let Ok(dismiss_url) =
466+
url.join(format!("reviews/{}/dismissals", review.id).as_str())
467+
{
468+
if let Ok(req) = Self::make_api_request(
469+
&self.client,
470+
dismiss_url,
471+
Method::PUT,
472+
Some(REVIEW_DISMISSAL.to_string()),
473+
None,
474+
) {
475+
match Self::send_api_request(
476+
self.client.clone(),
477+
req,
478+
self.rate_limit_headers.clone(),
479+
0,
480+
)
481+
.await
482+
{
483+
Ok(result) => {
484+
if !result.status().is_success() {
485+
Self::log_response(
486+
result,
487+
"Failed to dismiss outdated review",
488+
)
489+
.await;
495490
}
496491
}
492+
Err(e) => {
493+
log::error!(
494+
"Failed to dismiss outdated review: {e:}"
495+
);
496+
}
497497
}
498498
}
499-
Err(e) => {
500-
log::error!("Failed to parse URL for dismissing outdated review: {e:?}");
501-
}
502499
}
503500
}
504501
}

cpp-linter/tests/comments.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,14 @@ async fn setup(lib_root: &Path, test_params: &TestParams) {
146146
);
147147
}
148148
}
149-
let comment_url = format!("/repos/{REPO}/comments/76453652");
149+
let comment_url = format!(
150+
"/repos/{REPO}{}/comments/76453652",
151+
if test_params.event_t == EventType::PullRequest {
152+
"/issues"
153+
} else {
154+
""
155+
}
156+
);
150157

151158
if !test_params.fail_get_existing_comments {
152159
mocks.push(

cpp-linter/tests/reviews.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,10 @@ async fn setup(lib_root: &Path, test_params: &TestParams) {
125125
if !test_params.fail_get_existing_reviews {
126126
mocks.push(
127127
server
128-
.mock("PUT", format!("{reviews_endpoint}/1807607546").as_str())
128+
.mock(
129+
"PUT",
130+
format!("{reviews_endpoint}/1807607546/dismissals").as_str(),
131+
)
129132
.match_body(r#"{"event":"DISMISS","message":"outdated suggestion"}"#)
130133
.match_header("Authorization", format!("token {TOKEN}").as_str())
131134
.with_header(REMAINING_RATE_LIMIT_HEADER, "50")

0 commit comments

Comments
 (0)