From b57b9f812b1569fb7f634d2559283c9b34127ab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Wed, 26 Jul 2023 16:48:56 +0200 Subject: [PATCH 1/3] Listing Merged PRs for Github and Gitlab --- did/plugins/github.py | 16 +++++++++ did/plugins/gitlab.py | 82 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/did/plugins/github.py b/did/plugins/github.py index 0ccf4161..ef49a470 100644 --- a/did/plugins/github.py +++ b/did/plugins/github.py @@ -372,6 +372,19 @@ def fetch(self): Issue(issue, self.parent) for issue in self.parent.github.search(query)] +class PullRequestsMerged(Stats): + """ Pull requests merged """ + + def fetch(self): + log.info("Searching for merged pull requests authored by %s", self.user) + login = self.user.login + since = self.options.since + until = GitHub.until(self.options.until) + query = f"search/issues?q=author:{login}+merged:{since}..{until}+type:pr" + self.stats = [ + Issue(issue, self.parent) for issue in self.parent.github.search(query)] + + # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # Stats Group # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -427,4 +440,7 @@ def __init__(self, option, name=None, parent=None, user=None): PullRequestsReviewed( option=f"{option}-pull-requests-reviewed", parent=self, name=f"Pull requests reviewed on {option}"), + PullRequestsMerged( + option=f"{option}-pull-requests-merged", parent=self, + name=f"Merged pull requests on {option}"), ] diff --git a/did/plugins/gitlab.py b/did/plugins/gitlab.py index b4a02d78..9a0f8279 100644 --- a/did/plugins/gitlab.py +++ b/did/plugins/gitlab.py @@ -81,14 +81,16 @@ def __init__(self, self.project_issues: dict[str, list[dict[str, Any]]] = {} self.timeout = timeout - def _get_gitlab_api_raw(self, url): + def _get_gitlab_api_raw(self, url, params=None): log.debug("Connecting to GitLab API at '%s'.", url) + if params: + log.debug("Query params: %s", params) retries = 0 while True: try: api_raw = requests.get( url, headers=self.headers, verify=self.ssl_verify, - timeout=self.timeout) + params=params, timeout=self.timeout) api_raw.raise_for_status() return api_raw except requests.exceptions.HTTPError as http_err: @@ -115,9 +117,9 @@ def _get_gitlab_api_raw(self, url): ) sleep(GITLAB_INTERVAL) - def _get_gitlab_api(self, endpoint): + def _get_gitlab_api(self, endpoint, params=None): url = f'{self.url}/api/v{GITLAB_API}/{endpoint}' - return self._get_gitlab_api_raw(url) + return self._get_gitlab_api_raw(url, params=params) def _get_gitlab_api_json(self, endpoint): log.debug("Query: %s", endpoint) @@ -126,11 +128,16 @@ def _get_gitlab_api_json(self, endpoint): return result def _get_gitlab_api_list( - self, endpoint, since=None, get_all_results=False): + self, endpoint, + params=None, + since=None, + get_all_results=False + ): results = [] - result = self._get_gitlab_api(endpoint) + result = self._get_gitlab_api(endpoint, params=params) result.raise_for_status() results.extend(result.json()) + log.data(pretty(results)) while ('next' in result.links and 'url' in result.links['next'] and get_all_results): log.debug("-> Fetching more paginated data") @@ -170,6 +177,26 @@ def get_project_mr(self, project_id, mr_id): mr = next(filter(lambda x: x['id'] == mr_id, mrs), None) return mr + def get_user_mr(self, username, state, since, until): + """ + Fetch merge requests by user using GitLab's global + merge_requests endpoint. + + This endpoint is available to all authenticated users + (not just admins) and returns merge requests based on + the user's visibility permissions. + See: https://docs.gitlab.com/ee/api/merge_requests.html + """ + since = since.date.strftime('%Y-%m-%dT%H:%M:%S.000Z') + until = until.date.strftime('%Y-%m-%dT%H:%M:%S.000Z') + endpoint = 'merge_requests' + return self._get_gitlab_api_list(endpoint, params={ + 'author_username': username, + 'state': state, + 'updated_after': since, + 'updated_before': until + }, get_all_results=True) + def get_project_mrs(self, project_id): if project_id not in self.project_mrs: query = f'projects/{project_id}/merge_requests' @@ -230,7 +257,10 @@ def __init__(self, data: dict, parent: "GitLabStats", set_id=None): self.id = set_id if set_id is None: self.id = self.iid() - self.title = data['target_title'] + self.title = self._get_title() + + def _get_title(self): + return self.data['target_title'] def iid(self): return self.gitlabapi.get_project_issue( @@ -294,10 +324,31 @@ def note_iid(self, data, gitlabapi): return "unknown" +class MergedRequest(Issue): + # pylint: disable=too-few-public-methods + + def __init__(self, data, parent): + # For merged requests from the global merge_requests API, + # we get MR objects directly (not events), so iid and title + # are already in the data + set_id = data['iid'] + super().__init__(data, parent, set_id) + + def _get_title(self): + # MR objects from global API have 'title' field, + # unlike event objects which have 'target_title' + return self.data['title'] + + def iid(self): + # Override to avoid unnecessary API call since we already + # have iid + return self.data['iid'] + # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # Stats # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + class IssuesCreated(Stats): """ Issue created """ @@ -398,10 +449,24 @@ def fetch(self): for mr in results] +class MergeRequestsMerged(Stats): + """ Merge requests merged """ + + def fetch(self): + log.info("Searching for Merged requests authored by %s", self.user) + results = self.parent.gitlab.get_user_mr( + self.user.login, 'merged', + self.options.since, self.options.until + ) + self.stats = [ + MergedRequest(mr, self.parent) + for mr in results] + # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # Stats Group # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + class GitLabStats(StatsGroup): """ GitLab work """ @@ -461,4 +526,7 @@ def __init__(self, option, name=None, parent=None, user=None): MergeRequestsClosed( option=f"{option}-merge-requests-closed", parent=self, name=f"Merge requests closed on {option}"), + MergeRequestsMerged( + option=f"{option}-merge-requests-merged", parent=self, + name=f"Merged requests on {option}"), ] From fa0540084fefa201d5c32bdf4ee9906a20c2a6be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Tue, 18 Nov 2025 14:16:33 +0100 Subject: [PATCH 2/3] Fix merge date filtering and add unit tests Filter by merged_at timestamp (not updated_at) and add tests for both GitLab and GitHub merge-requests-merged functionality. --- did/plugins/gitlab.py | 34 +++++++++++++++++++++++++++------- tests/plugins/test_github.py | 11 +++++++++++ tests/plugins/test_gitlab.py | 20 ++++++++++++++++++++ 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/did/plugins/gitlab.py b/did/plugins/gitlab.py index 9a0f8279..46c0ae3b 100644 --- a/did/plugins/gitlab.py +++ b/did/plugins/gitlab.py @@ -177,7 +177,7 @@ def get_project_mr(self, project_id, mr_id): mr = next(filter(lambda x: x['id'] == mr_id, mrs), None) return mr - def get_user_mr(self, username, state, since, until): + def get_user_mr(self, username, state, since): """ Fetch merge requests by user using GitLab's global merge_requests endpoint. @@ -186,15 +186,20 @@ def get_user_mr(self, username, state, since, until): (not just admins) and returns merge requests based on the user's visibility permissions. See: https://docs.gitlab.com/ee/api/merge_requests.html + + Note: GitLab API doesn't support filtering by merge date, + only by update date. We fetch by 'updated_after' to reduce + the dataset, but don't use 'updated_before' to avoid missing + MRs that were merged in the target range but had later updates + (e.g., comments after merge). Callers must filter by merged_at + timestamp client-side. """ - since = since.date.strftime('%Y-%m-%dT%H:%M:%S.000Z') - until = until.date.strftime('%Y-%m-%dT%H:%M:%S.000Z') + since_str = since.date.strftime('%Y-%m-%dT%H:%M:%S.000Z') endpoint = 'merge_requests' return self._get_gitlab_api_list(endpoint, params={ 'author_username': username, 'state': state, - 'updated_after': since, - 'updated_before': until + 'updated_after': since_str }, get_all_results=True) def get_project_mrs(self, project_id): @@ -456,11 +461,26 @@ def fetch(self): log.info("Searching for Merged requests authored by %s", self.user) results = self.parent.gitlab.get_user_mr( self.user.login, 'merged', - self.options.since, self.options.until + self.options.since ) + # Filter by merge date since GitLab API only supports filtering + # by update date. We want MRs merged in the date range, not + # MRs updated in the date range (which could include comments + # after merge). + since_date = self.options.since.date + until_date = self.options.until.date + filtered_results = [] + for mr in results: + if mr.get('merged_at'): + merged_at = dateutil.parser.parse(mr['merged_at']).date() + if since_date <= merged_at <= until_date: + filtered_results.append(mr) + log.debug( + "Filtered %s MRs by merge date, %s remain", + len(results), len(filtered_results)) self.stats = [ MergedRequest(mr, self.parent) - for mr in results] + for mr in filtered_results] # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # Stats Group diff --git a/tests/plugins/test_github.py b/tests/plugins/test_github.py index 1fb42ef1..48bce1f9 100644 --- a/tests/plugins/test_github.py +++ b/tests/plugins/test_github.py @@ -187,3 +187,14 @@ def test_github_issues_commented(): assert any( "teemtee/tmt#1787 - tmt does not run test with local changes applied" in str(stat) for stat in stats) + + +def test_github_pull_requests_merged(): + """ Merged pull requests """ + did.base.Config(CONFIG) + option = "--gh-pull-requests-merged --since 2019-12-09 --until 2019-12-09" + # Note: The stats list index is 7 for PullRequestsMerged + stats = did.cli.main(option)[0][0].stats[0].stats[7].stats + assert any( + "psss/did#214 - Enable copr builds in Packit" in str(stat) + for stat in stats) diff --git a/tests/plugins/test_gitlab.py b/tests/plugins/test_gitlab.py index f818a10c..7388f718 100644 --- a/tests/plugins/test_gitlab.py +++ b/tests/plugins/test_gitlab.py @@ -180,6 +180,26 @@ def test_gitlab_config_invaliad_ssl_verify(caplog: LogCaptureFixture): assert "Invalid ssl_verify option for GitLab" in caplog.text +@pytest.mark.skipif("GITLAB_TOKEN" not in os.environ, + reason="No GITLAB_TOKEN environment variable found") +def test_gitlab_merge_requests_merged(): + """ + Merged merge requests. + + Note: GITLAB_TOKEN must be for did.tester user to match test + data. The global merge_requests API only returns MRs visible to + the token owner. + """ + did.base.Config(CONFIG) + option = "--gitlab-merge-requests-merged " + # Note: The stats list index is 7 for MergeRequestsMerged + # MR #4 was merged on 2023-01-20 at 14:14:01 + stats = did.cli.main(option + INTERVAL)[0][0].stats[0].stats[7].stats + assert any( + "did.tester/test-project#4" in str(stat) and "Update README.md" in str(stat) + for stat in stats) + + @pytest.mark.skipif("GITLAB_TOKEN" not in os.environ, reason="No GITLAB_TOKEN environment variable found") def test_gitlab_config_disabled_ssl_verify(): From eb49e39a143d81a31aad16e79d5e61041ce5eede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= Date: Tue, 18 Nov 2025 20:03:48 +0100 Subject: [PATCH 3/3] Fix markdown formatting for merged requests Transform MR data structure to include target_title and target_type fields expected by parent Issue class, eliminating the need for method overrides. --- did/plugins/gitlab.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/did/plugins/gitlab.py b/did/plugins/gitlab.py index 46c0ae3b..2fffccbe 100644 --- a/did/plugins/gitlab.py +++ b/did/plugins/gitlab.py @@ -333,21 +333,21 @@ class MergedRequest(Issue): # pylint: disable=too-few-public-methods def __init__(self, data, parent): - # For merged requests from the global merge_requests API, - # we get MR objects directly (not events), so iid and title - # are already in the data - set_id = data['iid'] - super().__init__(data, parent, set_id) - - def _get_title(self): - # MR objects from global API have 'title' field, - # unlike event objects which have 'target_title' - return self.data['title'] + # Transform MR data from global API to match event structure + # that parent Issue class expects. MR objects have 'title' + # but Issue expects 'target_title', and MR objects lack + # 'target_type' which Issue.__str__() needs for formatting. + transformed_data = data.copy() + transformed_data['target_title'] = data['title'] + transformed_data['target_type'] = 'MergeRequest' + # Store iid for override below to avoid unnecessary API calls + transformed_data['_iid'] = data['iid'] + super().__init__(transformed_data, parent, data['iid']) def iid(self): # Override to avoid unnecessary API call since we already - # have iid - return self.data['iid'] + # have iid in the transformed data + return self.data['_iid'] # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # Stats