Version 0.8.3

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## v0.8.3
+
+- Upgrade to Rust edition 2021.
+- Add --requested option.
+- Add the Requested (explicitly) score.
+- Simplify request query removing -author: and -reviewed-by: conditions to avoid GH bug to return an empty result.
+- Improve parsing of code owner team members.
+
 ## v0.8.2
 
 - Enable --include-reviewed-by-me when using --only-mine.

Cargo.toml

@@ -1,14 +1,14 @@
 [package]
 name = "ateam"
-version = "0.8.2"
+version = "0.8.3"
 authors = ["Andrea Frigido"]
 license = "MIT"
 description = "The tool that helps optimize the code review process."
 documentation = "https://github.com/frisoft/ateam"
 homepage = "https://github.com/frisoft/ateam"
 repository = "https://github.com/frisoft/ateam"
 readme = "README.md"
-edition = "2018"
+edition = "2021"
 keywords = ["cli", "github", "code-review", "pull-request", "command-line"]
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

README.md

@@ -13,6 +13,8 @@ The tool that helps optimize the code review process.
 
 Create a GitHub API token and store it in the GITHUB_API_TOKEN env variable. You can also use a .env file.
 
+The token needs `repo` and `read:org` selected scopes.
+
 ## Configuration
 
 A-team needs to connect to GitHub's API using your GitHub API token.
@@ -71,7 +73,7 @@ To see all the possible options, you can use `--help`:
 ```
 ❯ ateam pr --help
 
-ateam-pr 0.8.2
+ateam-pr 0.8.3
 
 USAGE:
     ateam pr [FLAGS] [OPTIONS]
@@ -89,6 +91,7 @@ FLAGS:
         --include-tests-pending     Include pull requests with pending tests
         --json                      Output in JSON
         --only-mine                 select only my pull requests (enables --include-reviewed-by-me automatically)
+        --requested                 Select pull requests I have been requested to review, explicitly or as a code owner
     -s, --short                     Short version. No table
     -V, --version                   Prints version information
 
@@ -136,6 +139,7 @@ The ranking algorithm is based on several pieces of information:
    - deletions * 0.1
    + based_on_main_branch * 200.0
    + blame * 400.0
+   + requested * 800.0
    + codeowner * 400.0
 ```
 
@@ -165,6 +169,8 @@ It is best reviewing first pull request based on the main branch.
 
 `blame` is 1 if you changed in the past one of the first 5 files changed by the pull requiest.
 
+`requested` is 1 if somebody requested your review explicity, not because you are a code owner.
+
 `codeowner` is 1 if you are one of the [code owners](https://docs.github.com/en/free-pro-team@latest/github/creating-cloning-and-archiving-repositories/about-code-owners) for this pull request.
 
 ## ateam followup

src/cli.rs

@@ -58,6 +58,8 @@ pub struct Pr {
         help = "select only my pull requests (enables --include-reviewed-by-me automatically)"
     )]
     pub only_mine: bool,
+    #[structopt(long, help = "Select pull requests I have been requested to review, explicitly or as a code owner")]
+    pub requested: bool,
     #[structopt(long, help = "Include draft pull requests")]
     pub include_drafts: bool,
     #[structopt(long, help = "Include pull requests with pending tests")]

src/client/blame.rs

@@ -49,19 +49,19 @@ fn is_file_author(response_data: &blame::ResponseData, login: &str) -> bool {
     // println!("\n\nRanges: {:?}\n\n", v);
 
     let authors = match v {
-        Some(ranges) => ranges.iter().map(|range|
+        Some(ranges) => ranges.iter().flat_map(|range|
               match &range.commit.authors.nodes {
-                 Some(nodes) => nodes.iter().map(|node|
+                 Some(nodes) => nodes.iter().flat_map(|node|
                      match node {
                         Some(blame::BlameRepositoryDefaultBranchRefTargetOnCommitBlameRangesCommitAuthorsNodes{
                             user: Some(blame::BlameRepositoryDefaultBranchRefTargetOnCommitBlameRangesCommitAuthorsNodesUser {login})
                         }) => Some(login),
                         _ => None
                      }
-                 ).flatten().collect(),
+                 ).collect(),
                  _ => vec!()
               }
-        ).flatten().collect(),
+        ).collect(),
         _ => vec!(),
     };
 

src/client/followup.rs

@@ -53,8 +53,7 @@ fn parse(response_data: &followup::ResponseData, login: &str) -> Vec<Review> {
         } => prs
             .iter()
             .flatten()
-            .map(|pr| parse_pr(pr, login))
-            .flatten()
+            .filter_map(|pr| parse_pr(pr, login))
             .collect(),
         _ => vec![],
     }
@@ -95,8 +94,7 @@ fn last_dismissed_or_addressed_review(
     reviews
         .iter()
         .flatten()
-        .map(|review| parse_review(review, has_unaddressed_review_threads, title))
-        .flatten()
+        .filter_map(|review| parse_review(review, has_unaddressed_review_threads, title))
         .next()
 }
 

src/client/mod.rs

@@ -101,11 +101,8 @@ fn github_query(username: &str, options: &cli::Pr) -> String {
         // "is:pr is:open draft:false -status:progess -status:failure {}{}{}{}",
         "is:pr is:open {}{}{}{}{}{}{}",
         query_drafts(options.include_drafts),
-        query_mine(username, options.include_mine, options.only_mine),
-        query_include_reviewed_by_me(
-            username,
-            options.include_reviewed_by_me || options.only_mine
-        ),
+        query_mine(username, options.only_mine),
+        query_requested(username, options.requested),
         query_labels(&options.label, &options.exclude_label),
         query_repos(&options.repo),
         query_org(&options.org),
@@ -121,24 +118,19 @@ fn query_drafts(include_drafts: bool) -> &'static str {
     }
 }
 
-fn query_mine(username: &str, include_mine: bool, only_mine: bool) -> String {
+fn query_mine(username: &str, only_mine: bool) -> String {
     if only_mine {
         format!("author:{} ", username)
-    } else if include_mine {
-        "".to_string()
     } else {
-        format!("-author:{} ", username)
+        "".to_string()
     }
 }
 
-fn query_include_reviewed_by_me(
-    username: &str,
-    include_reviewed_by_me_or_only_mine: bool,
-) -> String {
-    if include_reviewed_by_me_or_only_mine {
-        "".to_string()
+fn query_requested(username: &str, requested: bool) -> String {
+    if requested {
+        format!("review-requested:{} ", username)
     } else {
-        format!("-reviewed-by:{} ", username)
+        "".to_string()
     }
 }
 
@@ -175,19 +167,10 @@ pub fn ranked_prs<'a>(
     options: &cli::Pr,
     response_data: &'a repo_view::ResponseData,
 ) -> Vec<ScoredPr<'a>> {
-    let re = regex(&options.regex);
-    let re_not = regex(&options.regex_not);
-    let sprs: Vec<ScoredPr> = prs(
-        github_api_token,
-        username,
-        &re,
-        &re_not,
-        options,
-        response_data,
-    )
-    .into_par_iter()
-    .map(|pr| scored_pr(required_approvals, pr))
-    .collect();
+    let sprs: Vec<ScoredPr> = prs(github_api_token, username, options, response_data)
+        .into_par_iter()
+        .map(|pr| scored_pr(required_approvals, pr))
+        .collect();
     sprs
 }
 
@@ -213,11 +196,11 @@ fn scored_pr(required_approvals: u8, pr: Pr) -> ScoredPr {
 fn prs<'a>(
     github_api_token: &str,
     username: &str,
-    regex: &Option<Regex>,
-    regex_not: &Option<Regex>,
     options: &cli::Pr,
     response_data: &'a repo_view::ResponseData,
 ) -> Vec<Pr<'a>> {
+    let re = regex(&options.regex);
+    let re_not = regex(&options.regex_not);
     response_data
         .search
         .edges
@@ -232,17 +215,41 @@ fn prs<'a>(
             _ => None,
         }) // <-- Refactor
         .flatten() // Extract value from Some(value) and remove the Nones
-        .filter(move |i| {
-            !is_empty(i)
-                && !has_conflicts(i)
-                && regex_match(regex, true, i)
-                && !regex_match(regex_not, false, i)
+        .filter(|i| {
+            include_pr(
+                i,
+                &re,
+                &re_not,
+                username,
+                options.include_mine,
+                options.only_mine,
+                options.include_reviewed_by_me,
+            )
         })
         .map(move |i| pr_stats(github_api_token, username, options, i)) // <-- Refactor
         .flatten() // Extract value from Some(value) and remove the Nones
         .collect()
 }
 
+fn include_pr(
+    pr: &repo_view::RepoViewSearchEdgesNodeOnPullRequest,
+    regex: &Option<Regex>,
+    regex_not: &Option<Regex>,
+    username: &str,
+    include_mine: bool,
+    only_mine: bool,
+    include_reviewed_by_me: bool,
+) -> bool {
+    !is_empty(pr)
+        && !has_conflicts(pr)
+        && regex_match(regex, true, pr)
+        && !regex_match(regex_not, false, pr)
+        && (include_mine || only_mine || author(pr) != username)
+        && (include_reviewed_by_me
+            || only_mine
+            || review_states(&pr.reviews, username, true).is_empty()) // not reviewed by me
+}
+
 fn regex_match(
     regex: &Option<Regex>,
     or: bool,
@@ -322,7 +329,8 @@ fn pr_stats<'a>(
     };
 
     let author = author(pr);
-    let reviews = review_states(&pr.reviews, &author);
+    let reviews = review_states(&pr.reviews, &author, false);
+    let review_requested = review_requested(&pr.review_requests, username);
 
     Some(Pr {
         title: pr.title.clone(),
@@ -339,7 +347,8 @@ fn pr_stats<'a>(
         files,
         blame,
         labels: pr_labels(&pr.labels),
-        codeowner: is_codeowner(&pr.review_requests, username),
+        requested: matches!(review_requested, ReviewRequested::RequestedNotAsCodeOwner),
+        codeowner: matches!(review_requested, ReviewRequested::RequestedAsCodeOwner),
     })
 }
 
@@ -403,7 +412,7 @@ fn commit_tests_state_from_contexts(
 ) -> TestsState {
     match contexts_nodes {
         Some(nodes) => {
-            let states: Vec<TestsState> = nodes.iter().map(|node|
+            let states: Vec<TestsState> = nodes.iter().filter_map(|node|
                 match node {
                   Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestCommitsNodesCommitStatusCheckRollupContextsNodes::StatusContext(status_context)) => {
                       if tests_re.is_match(&status_context.context) {
@@ -413,7 +422,7 @@ fn commit_tests_state_from_contexts(
                       }
                   },
                   _ => None
-              }).flatten().collect();
+              }).collect();
             match states {
                 v if v.iter().any(|state| matches!(state, TestsState::Failure)) => {
                     TestsState::Failure
@@ -461,6 +470,7 @@ fn pr_open_conversations(
 fn review_states<'a>(
     reviews: &'a std::option::Option<repo_view::RepoViewSearchEdgesNodeOnPullRequestReviews>,
     author: &str,
+    reviewed_by_author: bool,
 ) -> Vec<&'a repo_view::PullRequestReviewState> {
     // println!("{:?}", reviews);
     if let Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviews {
@@ -488,7 +498,13 @@ fn review_states<'a>(
                 }
             })
             .rev() // reverse order: from the newest to the oldest
-            .filter(|review| review.0 != author) // exclude reviews given by the author of the PR
+            .filter(|review| {
+                if reviewed_by_author {
+                    review.0 == author // include only reviews given by the author of the PR
+                } else {
+                    review.0 != author // exclude reviews given by the author of the PR
+                }
+            })
             .unique_by(|review| review.0) // unique by user
             .map(|review| review.1) // take only the state
             .collect()
@@ -526,19 +542,30 @@ fn pr_based_on_main_branch(base_branch_name: &str) -> bool {
     base_branch_name == "main" || base_branch_name == "master"
 }
 
-fn is_codeowner(
+fn review_requested(
     requests: &std::option::Option<repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequests>,
     username: &str,
-) -> bool {
+) -> ReviewRequested {
     match requests {
-        Some(requests) => requests.nodes.iter().flatten().flatten().any(|r| {
-            r.as_code_owner
-                && match &r.requested_reviewer {
-                    Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequestsNodesRequestedReviewer::User(reviewer)) =>
-                        reviewer.login == username,
-                    _ => false,
-                }
-        }),
-        None => false,
+        Some(requests) => {
+            requests.nodes.iter().flatten().flatten().find(|r|
+            match &r.requested_reviewer {
+                Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequestsNodesRequestedReviewer::User(reviewer)) =>
+reviewer.login == username,
+                Some(repo_view::RepoViewSearchEdgesNodeOnPullRequestReviewRequestsNodesRequestedReviewer::Team(team)) =>
+                    team.members.nodes.iter().flatten().flatten().any(|member| member.login == username),
+                _ => false,
+            })
+            .map_or(
+                ReviewRequested::NotRequested,
+                |r|
+                 if r.as_code_owner {
+                       ReviewRequested::RequestedAsCodeOwner
+                 } else {
+                       ReviewRequested::RequestedNotAsCodeOwner
+                 }
+            )
+        },
+        None => ReviewRequested::NotRequested,
     }
 }

src/client/pr.graphql

@@ -76,6 +76,13 @@ query RepoView($query: String!, $first: Int!, $after: String, $num_checks: Int!)
                 ... on User {
                   login
                 }
+                ... on Team {
+                  members(first: 20) {
+                    nodes {
+                      login
+                    }
+                  }
+                }
               }
             }
           }