From b451bd664e53ead17bc2eb0699a8e90cccfcd6ff Mon Sep 17 00:00:00 2001 From: Kiran Patel <7103956+kiran94@users.noreply.github.com> Date: Sat, 27 May 2023 18:17:03 +0100 Subject: [PATCH] feat(main): support pr review collection options (#40) * chore(deps): add cloup * feat(collection): support pr review options * feat(main): add pr review collection options * style(main): remove whitespace * docs(readme): add user reviewed and user requested review sections * docs(readme): updated learn more about date filters note * docs(readme): remove space --- README.md | 26 ++++++++++++++-- poetry.lock | 18 ++++++++++- prfiesta/__main__.py | 44 ++++++++++++++++++--------- prfiesta/collectors/github.py | 22 ++++++++++---- pyproject.toml | 1 + tests/collectors/test_github.py | 16 ++++++++++ tests/test_main.py | 53 ++++++++++++++++++++++++++++----- 7 files changed, 150 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 047100b..40ff165 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,12 @@ prfiesta -u kiran94 -dc events_url -dc comments_url -dc node_id # Get all pull requests where the user was involved (as opposed to just authored) prfiesta -u kiran94 --use-involves +# Get all pull requests where the user reviewed it rather then being the author +prfiesta -u charliermarsh --after 2023-05-01 --use-reviewed-by + +# Get all pull requests where the user was requested a review rather then being the author +prfiesta -u charliermarsh --after 2023-05-01 --use-review-requested + # Get help prfiesta --help @@ -85,7 +91,11 @@ You can also customize the output file name using the `--output` option. By default, `prfiesta` will take the users provided in the `--user` option and search the Git provider for any pull requests that the user **authored**. Within more collaborative environments, this may not be what you want as you may want to also gain some visibility into all secondary contributions a user made (e.g commenting on others pull requests). -To help with this, `prfiesta` exposes the `--use-involves` flag which will search for pull requests that were: +*The options listed here are mutually exclusive.* + +#### User Involvement + +`prfiesta` exposes the `--use-involves` flag which will search for pull requests that were: - Created by a certain user - Assigned to that user @@ -94,11 +104,23 @@ To help with this, `prfiesta` exposes the `--use-involves` flag which will searc Learn more about `involves` [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-a-user-thats-involved-in-an-issue-or-pull-request). +#### User Reviewed + +`prfiesta` exposes a `--use-reviewed-by` flag which will collect pull requests where the user has reviewed others pull requests. + +Learn more about searching review requests [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-pull-request-review-status-and-reviewer) + +#### User Requested Review + +`prfiesta` exposes a `--use-review-requested` flag which will collect pull requests where the user was *requested* a review from other collaborators. + +Learn more about searching review requests [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-pull-request-review-status-and-reviewer) + ### Date Filter When using the `--after` and `--before` date filters, by default `prfiesta` will use the `created` date dimension with these filters on the Git provider (e.g GitHub). This may not fit your use case and you may want to filter on when a pull request was updated instead. To do this you can use the `--use-updated` flag. -See more information [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-when-an-issue-or-pull-request-was-created-or-last-updated). +Learn more about date filters [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-when-an-issue-or-pull-request-was-created-or-last-updated). ## Analysis diff --git a/poetry.lock b/poetry.lock index d103c36..57dbbb8 100644 --- a/poetry.lock +++ b/poetry.lock @@ -502,6 +502,22 @@ files = [ [package.dependencies] colorama = {version = "*", markers = "platform_system == \"Windows\""} +[[package]] +name = "cloup" +version = "2.1.0" +description = "Adds features to Click: option groups, constraints, subcommand sections and help themes." +category = "main" +optional = false +python-versions = ">=3.7" +files = [ + {file = "cloup-2.1.0-py2.py3-none-any.whl", hash = "sha256:cf93e4c4d22482d64be49d931342c997f100b9633600d59d061d0b449033754d"}, + {file = "cloup-2.1.0.tar.gz", hash = "sha256:dd42c7c9cd493fadc53a4236f96178a3f118bbbd945a7f8adafcd5034d826971"}, +] + +[package.dependencies] +click = ">=8.0,<9.0" +typing-extensions = {version = "*", markers = "python_version <= \"3.8\""} + [[package]] name = "colorama" version = "0.4.6" @@ -3618,4 +3634,4 @@ testing = ["big-O", "flake8 (<5)", "jaraco.functools", "jaraco.itertools", "more [metadata] lock-version = "2.0" python-versions = "^3.8" -content-hash = "b6a09ab8c097148f041d2f0561f420df1c5d4da8c5d32b21c3e9b3b4297a14fb" +content-hash = "6944cdc778620b40530e933152f0bcd7145535ab76957ffec9c89eb13e8c521c" diff --git a/prfiesta/__main__.py b/prfiesta/__main__.py index 6715f91..d4ec5da 100644 --- a/prfiesta/__main__.py +++ b/prfiesta/__main__.py @@ -2,6 +2,7 @@ from datetime import datetime import click +import cloup from rich.live import Live from rich.spinner import Spinner from rich.text import Text @@ -15,19 +16,25 @@ github_environment = GitHubEnvironment() - -@click.command() -@click.option('-u', '--users', required=True, multiple=True, help='The GitHub Users to search for. Can be multiple') -@click.option('-a', '--after', type=click.DateTime(formats=['%Y-%m-%d']), help='Only search for pull requests after this date e.g 2023-01-01') -@click.option('-b', '--before', type=click.DateTime(formats=['%Y-%m-%d']), help='Only search for pull requests before this date e.g 2023-04-30') -@click.option('-d', '--use-updated', is_flag=True, default=False, help='filter on when the pr was last updated rather then created') -@click.option('-i', '--use-involves', is_flag=True, default=False, help='include prs where the author was the author or assignee or mentioned or commented') -@click.option('-dc', '--drop-columns', multiple=True, help='Drop columns from the output dataframe') -@click.option('-x', '--url', help='The URL of the Git provider to use') -@click.option('-t', '--token', help='The Authentication token to use') -@click.option('-o', '--output', default=None, help='The output location') -@click.option('-ot', '--output-type', type=click.Choice(['csv', 'parquet']), default='csv', show_default=True, show_choices=True, help='The output format') -@click.version_option(__version__) +@cloup.command() +@cloup.option('-u', '--users', required=True, multiple=True, help='The GitHub Users to search for. Can be multiple') +@cloup.option('-a', '--after', type=click.DateTime(formats=['%Y-%m-%d']), help='Only search for pull requests after this date e.g 2023-01-01') +@cloup.option('-b', '--before', type=click.DateTime(formats=['%Y-%m-%d']), help='Only search for pull requests before this date e.g 2023-04-30') +@cloup.option('-d', '--use-updated', is_flag=True, default=False, help='filter on when the pr was last updated rather then created') +@cloup.option_group( + 'Author Filter', + cloup.option('-i', '--use-involves', is_flag=True, default=False, help='collect prs where the users are the author or assignee or mentioned or commented'), + cloup.option('-r', '--use-reviewed-by', is_flag=True, default=False, help='collect prs where the users reviewed them'), + cloup.option('-rr', '--use-review-requested', is_flag=True, default=False, help='collect prs where the users were requested a review'), + constraint=cloup.constraints.mutually_exclusive, + help='Collect alternative details for the users. If omitted then just collect the prs that a user has authored.', +) +@cloup.option('-dc', '--drop-columns', multiple=True, help='Drop columns from the output dataframe') +@cloup.option('-x', '--url', help='The URL of the Git provider to use') +@cloup.option('-t', '--token', help='The Authentication token to use') +@cloup.option('-o', '--output', default=None, help='The output location') +@cloup.option('-ot', '--output-type', type=click.Choice(['csv', 'parquet']), default='csv', show_default=True, show_choices=True, help='The output format') +@cloup.version_option(__version__) def main(**kwargs) -> None: users: tuple[str] = kwargs.get('users') @@ -40,6 +47,8 @@ def main(**kwargs) -> None: drop_columns: list[str] = list(kwargs.get('drop_columns')) use_updated: bool = kwargs.get('use_updated') use_involves: bool = kwargs.get('use_involves') + use_reviewed_by: bool = kwargs.get('use_reviewed_by') + use_review_requested: bool = kwargs.get('use_review_requested') logger.info('[bold green]PR Fiesta 🦜🥳') @@ -48,7 +57,14 @@ def main(**kwargs) -> None: with Live(spinner, refresh_per_second=20, transient=True): collector = GitHubCollector(token=token, url=url, spinner=spinner, drop_columns=drop_columns) - pr_frame = collector.collect(*users, after=after, before=before, use_updated=use_updated, use_involves=use_involves) + pr_frame = collector.collect( + *users, + after=after, + before=before, + use_updated=use_updated, + use_involves=use_involves, + use_reviewed_by=use_reviewed_by, + use_review_requested=use_review_requested) if not pr_frame.empty: logger.info('Found [bold green]%s[/bold green] pull requests!', pr_frame.shape[0]) diff --git a/prfiesta/collectors/github.py b/prfiesta/collectors/github.py index 6098d5e..52d1c37 100644 --- a/prfiesta/collectors/github.py +++ b/prfiesta/collectors/github.py @@ -50,9 +50,11 @@ def collect( before: Optional[datetime] = None, use_updated: Optional[bool] = False, use_involves: Optional[bool] = False, + use_reviewed_by: Optional[bool] = False, + use_review_requested: Optional[bool] = False, ) -> pd.DataFrame: - query = self._construct_query(users, after, before, use_updated, use_involves) + query = self._construct_query(users, after, before, use_updated, use_involves, use_reviewed_by, use_review_requested) update_spinner(f'Searching {self._url} with[bold blue] {query}', self._spinner, logger) @@ -92,6 +94,8 @@ def _construct_query( before: Optional[datetime] = None, use_updated: Optional[bool] = False, use_involves: Optional[bool] = False, + use_reviewed_by: Optional[bool] = False, + use_review_requested: Optional[bool] = False, ) -> str: """ Constructs a GitHub Search Query @@ -100,8 +104,12 @@ def _construct_query( Examples -------- type:pr author:user1 - type:pr author:user2 updated:<=2021-01-01 - type:pr author:user1 author:user2 updated:2021-01-01..2021-03-01 + type:pr author:user2 created:<=2021-01-01 + type:pr author:user1 author:user2 created:2021-01-01..2021-03-01 + type:pr author:user2 updated:>=2021-01-01 + type:pr involves:user2 + type:pr reviewed-by:user1 + type:pr review-requested:user1 All dates are inclusive. See GitHub Docs for full options https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests @@ -112,12 +120,16 @@ def _construct_query( author_filter = 'author' if use_involves: author_filter = 'involves' + elif use_reviewed_by: + author_filter = 'reviewed-by' + elif use_review_requested: + author_filter = 'review-requested' + + logger.debug('using author filter %s', author_filter) for u in users: query.append(f'{author_filter}:{u}') - logger.debug('using author filter %s', author_filter) - time_filter = 'created' if use_updated: time_filter = 'updated' diff --git a/pyproject.toml b/pyproject.toml index 5fd0a83..c6ce08f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,6 +21,7 @@ classifiers = [ python = "^3.8" pygithub = "^1.58.1" click = "^8.1.3" +cloup = "^2.1.0" pandas = "^2.0.1" pyarrow = "^11.0.0" rich = "^13.3.5" diff --git a/tests/collectors/test_github.py b/tests/collectors/test_github.py index 4e12ad0..1cdab83 100644 --- a/tests/collectors/test_github.py +++ b/tests/collectors/test_github.py @@ -143,6 +143,22 @@ 'type:pr involves:user1 updated:2009-01-01..2010-01-01', id='involves_updated_before_and_after_returned_results', ), + pytest.param + ( + ('user1', 'user2'), + { 'use_reviewed_by': True }, + [ _mock_issue1, _mock_issue2 ], + 'type:pr reviewed-by:user1 reviewed-by:user2', + id='reviewed_by_user', + ), + pytest.param + ( + ('user1', 'user2'), + { 'use_review_requested': True }, + [ _mock_issue1, _mock_issue2 ], + 'type:pr review-requested:user1 review-requested:user2', + id='review_requested_user', + ), ]) @patch('prfiesta.collectors.github.Github') def test_collect( diff --git a/tests/test_main.py b/tests/test_main.py index 2b652b3..8dd063e 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -19,12 +19,26 @@ def test_main_missing_users() -> None: assert "Missing option '-u' / '--users'" in result.output assert result.exit_code == FAILURE_CODE + +@pytest.mark.parametrize('arguments', [ + pytest.param(['--users', 'user', '--use-involves', '--use-reviewed-by', '--use-review-requested']), + pytest.param(['--users', 'user', '--use-involves', '--use-reviewed-by']), + pytest.param(['--users', 'user', '--use-reviewed-by', '--use-review-requested']), + pytest.param(['--users', 'user', '--use-involves', '--use-review-requested']), +]) +def test_main_author_filters_mutually_exclusive(arguments: List[str]) -> None: + runner = CliRunner() + result = runner.invoke(main, arguments) + + assert 'the following parameters are mutually exclusive' in result.output + assert result.exit_code == FAILURE_CODE + @pytest.mark.parametrize(('params', 'expected_collect_params', 'collect_response', 'expected_output_type'), [ pytest.param ( ['--users', 'test_user'], - [call('test_user', after=None, before=None, use_updated=False, use_involves=False)], + [call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)], pd.DataFrame(), 'csv', id='user_provided', @@ -32,7 +46,7 @@ def test_main_missing_users() -> None: pytest.param ( ['--users', 'test_user', '--after', '2020-01-01'], - [call('test_user', after=datetime(2020, 1, 1), before=None, use_updated=False, use_involves=False)], + [call('test_user', after=datetime(2020, 1, 1), before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)], pd.DataFrame(), 'csv', id='with_after', @@ -40,7 +54,7 @@ def test_main_missing_users() -> None: pytest.param ( ['--users', 'test_user', '--before', '2020-01-01'], - [call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=False, use_involves=False)], + [call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)], pd.DataFrame(), 'csv', id='with_before', @@ -48,7 +62,14 @@ def test_main_missing_users() -> None: pytest.param ( ['--users', 'test_user', '--before', '2020-01-01', '--after', '2009-01-01'], - [call('test_user', before=datetime(2020, 1, 1), after=datetime(2009, 1, 1), use_updated=False, use_involves=False)], + [call( + 'test_user', + before=datetime(2020, 1, 1), + after=datetime(2009, 1, 1), + use_updated=False, + use_involves=False, + use_reviewed_by=False, + use_review_requested=False)], pd.DataFrame(), 'csv', id='with_before_and_after', @@ -56,7 +77,7 @@ def test_main_missing_users() -> None: pytest.param ( ['--users', 'test_user'], - [call('test_user', after=None, before=None, use_updated=False, use_involves=False)], + [call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)], pd.DataFrame( data=[(1, 2, 3)], columns=['col1', 'col2', 'col3'], @@ -67,7 +88,7 @@ def test_main_missing_users() -> None: pytest.param ( ['--users', 'test_user', '--output-type', 'parquet'], - [call('test_user', after=None, before=None, use_updated=False, use_involves=False)], + [call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)], pd.DataFrame( data=[(1, 2, 3)], columns=['col1', 'col2', 'col3'], @@ -78,7 +99,7 @@ def test_main_missing_users() -> None: pytest.param ( ['--users', 'test_user', '--before', '2020-01-01', '--use-updated'], - [call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=True, use_involves=False)], + [call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=True, use_involves=False, use_reviewed_by=False, use_review_requested=False)], pd.DataFrame(), 'csv', id='with_use_updated', @@ -86,11 +107,27 @@ def test_main_missing_users() -> None: pytest.param ( ['--users', 'test_user', '--before', '2020-01-01', '--use-involves'], - [call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=False, use_involves=True)], + [call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=False, use_involves=True, use_reviewed_by=False, use_review_requested=False)], pd.DataFrame(), 'csv', id='with_use_involves', ), + pytest.param + ( + ['--users', 'test_user', '--use-reviewed-by'], + [call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=True, use_review_requested=False)], + pd.DataFrame(), + 'csv', + id='user_reviewed_by', + ), + pytest.param + ( + ['--users', 'test_user', '--use-review-requested'], + [call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=True)], + pd.DataFrame(), + 'csv', + id='user_review_requested', + ), ]) @patch('prfiesta.__main__.Spinner') @patch('prfiesta.__main__.Live')