From 1581698055d069de94739301669e78a113640bef Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Sun, 28 Sep 2025 23:23:36 +0200 Subject: [PATCH 1/4] #170 - Add logic for fetching sub issues from external repositories - cross-repo sub-issues are now visible in release notes. --- release_notes_generator/filter.py | 8 +++-- release_notes_generator/generator.py | 12 +++---- release_notes_generator/miner.py | 36 +++++++++++++------ .../model/hierarchy_issue_record.py | 15 ++++++-- release_notes_generator/model/issue_record.py | 3 ++ release_notes_generator/model/mined_data.py | 20 ++++++++++- release_notes_generator/model/record.py | 19 ++++++++++ .../model/sub_issue_record.py | 18 +++++----- .../record/factory/default_record_factory.py | 35 +++++++++++++----- .../factory/issue_hierarchy_record_factory.py | 23 ++++++++---- .../factory/test_default_record_factory.py | 10 +++--- .../test_issue_hierarchy_record_factory.py | 16 ++++----- tests/test_filter.py | 6 ++-- tests/test_miner.py | 14 ++++---- 14 files changed, 165 insertions(+), 70 deletions(-) diff --git a/release_notes_generator/filter.py b/release_notes_generator/filter.py index a8491935..b3dd5778 100644 --- a/release_notes_generator/filter.py +++ b/release_notes_generator/filter.py @@ -61,7 +61,7 @@ def filter(self, data: MinedData) -> MinedData: Returns: MinedData: The filtered mined data. """ - md = MinedData(data.repository) + md = MinedData(data.home_repository) md.release = data.release md.since = data.since @@ -123,7 +123,8 @@ def _filter_issues(self, data: MinedData) -> list: logger.debug("Used default issue filtering logic.") return self._filter_issues_default(data) - def _filter_issues_default(self, data: MinedData) -> list: + @staticmethod + def _filter_issues_default(data: MinedData) -> list: """ Default filtering for issues: filter out closed issues before the release date. @@ -135,7 +136,8 @@ def _filter_issues_default(self, data: MinedData) -> list: """ return [issue for issue in data.issues if (issue.closed_at is None) or (issue.closed_at >= data.since)] - def _filter_issues_issue_hierarchy(self, data: MinedData) -> list: + @staticmethod + def _filter_issues_issue_hierarchy(data: MinedData) -> list: """ Hierarchy filtering for issues: include issues closed since the release date or still open at generation time. diff --git a/release_notes_generator/generator.py b/release_notes_generator/generator.py index c0d483e1..a9abb6ca 100644 --- a/release_notes_generator/generator.py +++ b/release_notes_generator/generator.py @@ -92,15 +92,14 @@ def generate(self) -> Optional[str]: changelog_url: str = get_change_url( tag_name=ActionInputs.get_tag_name(), - repository=data_filtered_by_release.repository, + repository=data_filtered_by_release.home_repository, git_release=data_filtered_by_release.release, ) - assert data_filtered_by_release.repository is not None, "Repository must not be None" + assert data_filtered_by_release.home_repository is not None, "Repository must not be None" rls_notes_records: dict[str, Record] = self._get_record_factory( - github=self._github_instance, - home_repository=data_filtered_by_release.repository, + github=self._github_instance, home_repository=data_filtered_by_release.home_repository, miner=miner ).generate(data=data_filtered_by_release) return ReleaseNotesBuilder( @@ -109,12 +108,13 @@ def generate(self) -> Optional[str]: changelog_url=changelog_url, ).build() - def _get_record_factory(self, github: Github, home_repository: Repository) -> DefaultRecordFactory: + @staticmethod + def _get_record_factory(github: Github, home_repository: Repository) -> DefaultRecordFactory: """ Determines and returns the appropriate RecordFactory instance based on the action inputs. Parameters: - github (Github): An instance of the Github class. + github (GitHub): An instance of the GitHub class. home_repository (Repository): The home repository for which records are to be generated. Returns: diff --git a/release_notes_generator/miner.py b/release_notes_generator/miner.py index aca2a5ec..0855f99e 100644 --- a/release_notes_generator/miner.py +++ b/release_notes_generator/miner.py @@ -60,24 +60,36 @@ def check_repository_exists(self) -> bool: return False return True + def get_repository(self, full_name: str) -> Optional[Repository]: + """ + Retrieves the specified GitHub repository. + + Returns: + Optional[Repository]: The GitHub repository if found, None otherwise. + """ + repo: Repository = self._safe_call(self.github_instance.get_repo)(full_name) + if repo is None: + logger.error("Repository not found: %s", full_name) + return None + return repo + def mine_data(self) -> MinedData: """ Mines data from GitHub, including repository information, issues, pull requests, commits, and releases. """ logger.info("Starting data mining from GitHub...") - repo: Repository = self._safe_call(self.github_instance.get_repo)(ActionInputs.get_github_repository()) + repo: Optional[Repository] = self.get_repository(ActionInputs.get_github_repository()) if repo is None: - logger.error("Repository not found: %s", ActionInputs.get_github_repository()) raise ValueError("Repository not found") data = MinedData(repo) - data.release = self.get_latest_release(data.repository) + data.release = self.get_latest_release(repo) self._get_issues(data) # pulls and commits, and then reduce them by the latest release since time - data.pull_requests = list(self._safe_call(data.repository.get_pulls)(state=PullRequestRecord.PR_STATE_CLOSED)) - data.commits = list(self._safe_call(data.repository.get_commits)()) + data.pull_requests = list(self._safe_call(repo.get_pulls)(state=PullRequestRecord.PR_STATE_CLOSED)) + data.commits = list(self._safe_call(repo.get_commits)()) logger.info("Data mining from GitHub completed.") @@ -136,11 +148,11 @@ def _get_issues(self, data: MinedData): - If release exists: fetch issues updated since the release timestamp AND all currently open issues (to include long-lived open issues not updated recently). De-duplicate by issue.number. """ - assert data.repository is not None, "Repository must not be None" + assert data.home_repository is not None, "Repository must not be None" logger.info("Fetching issues from repository...") if data.release is None: - data.issues = list(self._safe_call(data.repository.get_issues)(state=IssueRecord.ISSUE_STATE_ALL)) + data.issues = list(self._safe_call(data.home_repository.get_issues)(state=IssueRecord.ISSUE_STATE_ALL)) logger.info("Fetched %d issues", len(data.issues)) return @@ -151,11 +163,11 @@ def _get_issues(self, data: MinedData): if prefer_published and getattr(data.release, "published_at", None) else data.release.created_at ) - issues_since = self._safe_call(data.repository.get_issues)( + issues_since = self._safe_call(data.home_repository.get_issues)( state=IssueRecord.ISSUE_STATE_ALL, since=data.since, ) - open_issues = self._safe_call(data.repository.get_issues)( + open_issues = self._safe_call(data.home_repository.get_issues)( state=IssueRecord.ISSUE_STATE_OPEN, ) @@ -175,7 +187,8 @@ def _get_issues(self, data: MinedData): data.issues = list(by_number.values()) logger.info("Fetched %d issues (deduplicated).", len(data.issues)) - def __get_latest_semantic_release(self, releases) -> Optional[GitRelease]: + @staticmethod + def __get_latest_semantic_release(releases) -> Optional[GitRelease]: published_releases = [release for release in releases if not release.draft and not release.prerelease] latest_version: Optional[semver.Version] = None rls: Optional[GitRelease] = None @@ -199,7 +212,8 @@ def __get_latest_semantic_release(self, releases) -> Optional[GitRelease]: return rls - def __filter_duplicated_issues(self, data: MinedData) -> "MinedData": + @staticmethod + def __filter_duplicated_issues(data: MinedData) -> "MinedData": """ Filters out duplicated issues from the list of issues. This method address problem in output of GitHub API where issues list contains PR values. diff --git a/release_notes_generator/model/hierarchy_issue_record.py b/release_notes_generator/model/hierarchy_issue_record.py index 889b2764..e292b0a8 100644 --- a/release_notes_generator/model/hierarchy_issue_record.py +++ b/release_notes_generator/model/hierarchy_issue_record.py @@ -61,10 +61,16 @@ def pull_requests_count(self) -> int: count = super().pull_requests_count() for sub_issue in self._sub_issues.values(): - count += sub_issue.pull_requests_count() + if sub_issue.is_cross_repo: + count += 1 + else: + count += sub_issue.pull_requests_count() for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): - count += sub_hierarchy_issue.pull_requests_count() + if sub_hierarchy_issue.is_cross_repo: + count += 1 + else: + count += sub_hierarchy_issue.pull_requests_count() return count @@ -85,6 +91,7 @@ def get_labels(self) -> list[str]: # methods - override ancestor methods def to_chapter_row(self, add_into_chapters: bool = True) -> str: + logger.debug("Rendering hierarchy issue row for issue #%s", self.issue.number) if add_into_chapters: self.added_into_chapters() row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.present_in_chapters() > 1 else "" @@ -120,19 +127,23 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: # add sub-hierarchy issues for sub_hierarchy_issue in self._sub_hierarchy_issues.values(): + logger.debug("Rendering hierarchy issue row for sub-issue #%s", sub_hierarchy_issue.issue.number) if sub_hierarchy_issue.contains_change_increment(): + logger.debug("Sub-hierarchy issue #%s contains change increment", sub_hierarchy_issue.issue.number) row = f"{row}\n{sub_hierarchy_issue.to_chapter_row()}" # add sub-issues if len(self._sub_issues) > 0: sub_indent = " " * (self._level + 1) for sub_issue in self._sub_issues.values(): + logger.debug("Rendering sub-issue row for issue #%d", sub_issue.issue.number) if sub_issue.is_open: continue # only closed issues are reported in release notes if not sub_issue.contains_change_increment(): continue # skip sub-issues without change increment + logger.debug("Sub-issue #%s contains change increment", sub_issue.issue.number) sub_issue_block = "- " + sub_issue.to_chapter_row() ind_child_block = "\n".join( f"{sub_indent}{line}" if line else "" for line in sub_issue_block.splitlines() diff --git a/release_notes_generator/model/issue_record.py b/release_notes_generator/model/issue_record.py index fca5b0ce..e093a35c 100644 --- a/release_notes_generator/model/issue_record.py +++ b/release_notes_generator/model/issue_record.py @@ -123,6 +123,9 @@ def to_chapter_row(self, add_into_chapters: bool = True) -> str: return row def contains_change_increment(self) -> bool: + if self.is_cross_repo: + return True + return self.pull_requests_count() > 0 def get_rls_notes(self, line_marks: Optional[list[str]] = None) -> str: diff --git a/release_notes_generator/model/mined_data.py b/release_notes_generator/model/mined_data.py index 2306128d..f9d4c083 100644 --- a/release_notes_generator/model/mined_data.py +++ b/release_notes_generator/model/mined_data.py @@ -39,13 +39,31 @@ class MinedData: """Class for keeping track of mined GitHub data.""" def __init__(self, repository: Repository): - self.repository: Repository = repository + self._home_repository_full_name: str = repository.full_name + self._repositories: dict[str, Repository] = {repository.full_name: repository} self.release: Optional[GitRelease] = None self.issues: list[Issue] = [] self.pull_requests: list[PullRequest] = [] self.commits: list[Commit] = [] self.since = datetime(1970, 1, 1) # Default to epoch start + @property + def home_repository(self) -> Repository: + """Get the home repository.""" + return self._repositories[self._home_repository_full_name] + + def add_repository(self, repository: Repository) -> None: + """Add a repository to the mined data if not already present.""" + if repository.full_name not in self._repositories: + self._repositories[repository.full_name] = repository + logger.debug(f"Added repository {repository.full_name} to mined data.") + + def get_repository(self, full_name: str) -> Optional[Repository]: + if full_name not in self._repositories: + return None + + return self._repositories[full_name] + def is_empty(self): """ Check if the mined data is empty (no issues, pull requests, or commits). diff --git a/release_notes_generator/model/record.py b/release_notes_generator/model/record.py index 14d5c8e5..e9a8cfda 100644 --- a/release_notes_generator/model/record.py +++ b/release_notes_generator/model/record.py @@ -36,6 +36,7 @@ class Record(metaclass=ABCMeta): def __init__(self, labels: Optional[list[str]] = None, skip: bool = False): self._present_in_chapters = 0 self._skip = skip + self._is_cross_repo: bool = False self._is_release_note_detected: Optional[bool] = None self._labels: Optional[list[str]] = labels self._rls_notes: Optional[str] = None # single annotation here @@ -50,6 +51,24 @@ def is_present_in_chapters(self) -> bool: """ return self._present_in_chapters > 0 + @property + def is_cross_repo(self) -> bool: + """ + Checks if the record is a cross-repo record. + Returns: + bool: True if the record is a cross-repo record, False otherwise. + """ + return self._is_cross_repo + + @is_cross_repo.setter + def is_cross_repo(self, value: bool) -> None: + """ + Sets the cross-repo status of the record. + Parameters: + value (bool): The cross-repo status to set. + """ + self._is_cross_repo = value + @property def skip(self) -> bool: """Check if the record should be skipped during output generation process.""" diff --git a/release_notes_generator/model/sub_issue_record.py b/release_notes_generator/model/sub_issue_record.py index 7f95390e..25569f75 100644 --- a/release_notes_generator/model/sub_issue_record.py +++ b/release_notes_generator/model/sub_issue_record.py @@ -23,14 +23,14 @@ def __init__(self, sub_issue: SubIssue | Issue, issue_labels: Optional[list[str] # properties - override IssueRecord properties - @property - def issue(self) -> SubIssue: - """ - Gets the issue associated with the record. - Returns: The issue associated with the record. - """ - if not isinstance(self._issue, SubIssue): - raise TypeError("Expected SubIssue") - return self._issue + # @property + # def issue(self) -> SubIssue: + # """ + # Gets the issue associated with the record. + # Returns: The issue associated with the record. + # """ + # if not isinstance(self._issue, SubIssue): + # raise TypeError("Expected SubIssue") + # return self._issue # properties - specific to IssueRecord diff --git a/release_notes_generator/record/factory/default_record_factory.py b/release_notes_generator/record/factory/default_record_factory.py index 3d98842e..b24ee598 100644 --- a/release_notes_generator/record/factory/default_record_factory.py +++ b/release_notes_generator/record/factory/default_record_factory.py @@ -49,6 +49,7 @@ class DefaultRecordFactory(RecordFactory): """ def __init__(self, github: Github, home_repository: Repository) -> None: + self._github = github rate_limiter = GithubRateLimiter(github) self._safe_call = safe_call_decorator(rate_limiter) self._home_repository = home_repository @@ -84,7 +85,20 @@ def _(self, pull_request: PullRequest) -> str: @get_id.register def _(self, commit: Commit) -> str: - return f"{commit.repository.full_name}@{commit.sha}" + return f"{commit.sha}" + + def get_repository(self, full_name: str) -> Optional[Repository]: + """ + Retrieves the specified GitHub repository. + + Returns: + Optional[Repository]: The GitHub repository if found, None otherwise. + """ + repo: Repository = self._safe_call(self._github.get_repo)(full_name) + if repo is None: + logger.error("Repository not found: %s", full_name) + return None + return repo def generate(self, data: MinedData) -> dict[str, Record]: """ @@ -96,10 +110,10 @@ def generate(self, data: MinedData) -> dict[str, Record]: """ def register_pull_request(pr: PullRequest, skip_rec: bool) -> None: - pid = self.get_id(pr) - pull_labels = [label.name for label in pr.get_labels()] + l_pid = self.get_id(pr) + l_pull_labels = [label.name for label in pr.get_labels()] attached_any = False - detected_issues = extract_issue_numbers_from_body(pr, repository=data.repository) + detected_issues = extract_issue_numbers_from_body(pr, repository=data.home_repository) logger.debug("Detected issues - from body: %s", detected_issues) linked = self._safe_call(get_issues_for_pr)(pull_number=pr.number) if linked: @@ -116,9 +130,12 @@ def register_pull_request(pr: PullRequest, skip_rec: bool) -> None: parent_issue_id, ) # dev note: here we expect that PR links to an issue in the same repository !!! - parent_issue_number = int(parent_issue_id.split("#")[1]) + pi_repo_name, pi_number = parent_issue_id.split("#") + parent_repository = data.get_repository(pi_repo_name) parent_issue = ( - self._safe_call(data.repository.get_issue)(parent_issue_number) if data.repository else None + self._safe_call(parent_repository.get_issue)(pi_number) + if parent_repository is not None + else None ) if parent_issue is not None: self._create_record_for_issue(parent_issue) @@ -136,8 +153,8 @@ def register_pull_request(pr: PullRequest, skip_rec: bool) -> None: ) if not attached_any: - self._records[pid] = PullRequestRecord(pr, pull_labels, skip=skip_rec) - logger.debug("Created stand-alone PR record %s: %s (fallback)", pid, pr.title) + self._records[l_pid] = PullRequestRecord(pr, l_pull_labels, skip=skip_rec) + logger.debug("Created stand-alone PR record %s: %s (fallback)", l_pid, pr.title) logger.debug("Registering issues to records...") for issue in data.issues: @@ -150,7 +167,7 @@ def register_pull_request(pr: PullRequest, skip_rec: bool) -> None: skip_record: bool = any(item in pull_labels for item in ActionInputs.get_skip_release_notes_labels()) linked_from_api = self._safe_call(get_issues_for_pr)(pull_number=pull.number) or set() - linked_from_body = extract_issue_numbers_from_body(pull, data.repository) + linked_from_body = extract_issue_numbers_from_body(pull, data.home_repository) if not linked_from_api and not linked_from_body: self._records[pid] = PullRequestRecord(pull, pull_labels, skip=skip_record) logger.debug("Created record for PR %s: %s", pid, pull.title) diff --git a/release_notes_generator/record/factory/issue_hierarchy_record_factory.py b/release_notes_generator/record/factory/issue_hierarchy_record_factory.py index b78c7707..e7ed52c4 100644 --- a/release_notes_generator/record/factory/issue_hierarchy_record_factory.py +++ b/release_notes_generator/record/factory/issue_hierarchy_record_factory.py @@ -136,14 +136,18 @@ def generate(self, data: MinedData) -> dict[str, Record]: ) return self._records - def _register_pull_and_its_commits_to_issue(self, pull: PullRequest, data: MinedData) -> None: + def _register_pull_and_its_commits_to_issue( + self, pull: PullRequest, data: MinedData, target_repository: Optional[Repository] = None + ) -> None: pull_labels = [label.name for label in pull.get_labels()] skip_record: bool = any(item in pull_labels for item in ActionInputs.get_skip_release_notes_labels()) related_commits = [c for c in data.commits if c.sha == pull.merge_commit_sha] self.__registered_commits.update(c.sha for c in related_commits) + pr_repo = target_repository if target_repository is not None else data.home_repository + linked_from_api = self._safe_call(get_issues_for_pr)(pull_number=pull.number) or set() - linked_from_body = extract_issue_numbers_from_body(pull, data.repository) + linked_from_body = extract_issue_numbers_from_body(pull, pr_repo) pull_issues: list[str] = list(linked_from_api.union(linked_from_body)) attached_any = False if len(pull_issues) > 0: @@ -156,10 +160,9 @@ def _register_pull_and_its_commits_to_issue(self, pull: PullRequest, data: Mined issue_id, ) # dev note: here we expect that PR links to an issue in the same repository !!! - # TODO - handle cross-repo issue linking - the split[0] can contain different repo name - # this problem is present on multiple places - issue_number = int(issue_id.split("#")[1]) - parent_issue = self._safe_call(data.repository.get_issue)(issue_number) if data.repository else None + i_repo_name, i_number = issue_id.split("#") + parent_repository = data.get_repository(i_repo_name) + parent_issue = self._safe_call(parent_repository.get_issue)(i_number) if parent_repository else None if parent_issue is not None: self._create_issue_record_using_sub_issues_existence(parent_issue, data) @@ -298,6 +301,11 @@ def _create_record_for_sub_issue(self, issue: Issue, issue_labels: Optional[list self.__registered_issues.add(iid) self._records[iid] = SubIssueRecord(issue, issue_labels, skip_record) + if iid.split("#")[0] == self._home_repository.full_name: + return + + self._records[iid].is_cross_repo = True + def _re_register_hierarchy_issues(self): logger.debug("Re-registering hierarchy issues ...") @@ -315,17 +323,20 @@ def _re_register_hierarchy_issues(self): parent_issue_id: str = self.__sub_issue_parents[sub_issue_id] parent_rec = cast(HierarchyIssueRecord, self._records[parent_issue_id]) sub_rec = self._records[sub_issue_id] + # TODO - check this localtion - is possiblem that there is saved another type than SubIssueRecord if isinstance(sub_rec, SubIssueRecord): parent_rec.sub_issues[sub_issue_id] = sub_rec # add to parent as SubIssueRecord self._records.pop(sub_issue_id) # remove from main records as it is sub-one self.__sub_issue_parents.pop(sub_issue_id) # remove from sub-parents as it is now sub-one made_progress = True + logger.debug("Added sub-issue %s to parent %s", sub_issue_id, parent_issue_id) elif isinstance(sub_rec, HierarchyIssueRecord): parent_rec.sub_hierarchy_issues[sub_issue_id] = sub_rec # add to parent as 'Sub' HierarchyIssueRecord self._records.pop(sub_issue_id) # remove from main records as it is sub-one self.__sub_issue_parents.pop(sub_issue_id) # remove from sub-parents as it is now sub-one made_progress = True + logger.debug("Added sub-hierarchy-issue %s to parent %s", sub_issue_id, parent_issue_id) else: logger.error( "Detected IssueRecord in position of SubIssueRecord - leaving as standalone and dropping mapping" diff --git a/tests/release_notes/record/factory/test_default_record_factory.py b/tests/release_notes/record/factory/test_default_record_factory.py index 7cf9250b..dec01249 100644 --- a/tests/release_notes/record/factory/test_default_record_factory.py +++ b/tests/release_notes/record/factory/test_default_record_factory.py @@ -241,12 +241,12 @@ def test_generate_with_issues_and_pulls_and_commits_with_skip_labels(mocker, moc assert 'org/repo#1' in records assert 'org/repo#2' in records - assert 'org/repo@ghi789' in records + assert 'ghi789' in records # Verify the record creation assert isinstance(records['org/repo#1'], IssueRecord) assert isinstance(records['org/repo#2'], IssueRecord) - assert isinstance(records['org/repo@ghi789'], CommitRecord) + assert isinstance(records['ghi789'], CommitRecord) assert records['org/repo#1'].skip # skip label applied to issue as the record was created from issue assert not records['org/repo#2'].skip # skip label is present only on inner PR but record create from issues (leading) @@ -264,7 +264,7 @@ def test_generate_with_issues_and_pulls_and_commits_with_skip_labels(mocker, moc # Verify that commits are registered assert commit1 == rec_i1.get_commit(101, 'abc123') assert commit2 == rec_i2.get_commit(102, 'def456') - assert commit3 == cast(CommitRecord, records['org/repo@ghi789']).commit + assert commit3 == cast(CommitRecord, records['ghi789']).commit def test_generate_with_no_commits(mocker, mock_repo): @@ -352,7 +352,7 @@ def test_generate_with_no_issues(mocker, mock_repo, request): pr1, pr2, commit1, commit2 = setup_no_issues_pulls_commits(mocker) data.pull_requests = [pr1, pr2] data.commits = [commit1, commit2] - data.repository = request.getfixturevalue("mock_repo") + data.home_repository.return_value = request.getfixturevalue("mock_repo") data.issues = [] # No issues records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) @@ -394,7 +394,7 @@ def test_generate_with_no_issues_skip_labels(mocker, mock_repo, request): data.pull_requests = [pr1, pr2] data.commits = [commit1, commit2] - data.repository = request.getfixturevalue("mock_repo") + data.home_repository.return_value = request.getfixturevalue("mock_repo") data.issues = [] # No issues records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) diff --git a/tests/release_notes/record/factory/test_issue_hierarchy_record_factory.py b/tests/release_notes/record/factory/test_issue_hierarchy_record_factory.py index 3aa659f6..3be9dfa1 100644 --- a/tests/release_notes/record/factory/test_issue_hierarchy_record_factory.py +++ b/tests/release_notes/record/factory/test_issue_hierarchy_record_factory.py @@ -60,7 +60,7 @@ def test_generate_isolated_record_types_no_labels_no_type_defined(mocker, mock_r result = factory.generate(mined_data_isolated_record_types_no_labels_no_type_defined) assert 8 == len(result) - assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "org/repo@merge_commit_sha_direct"}.issubset(result.keys()) + assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "merge_commit_sha_direct"}.issubset(result.keys()) assert isinstance(result['org/repo#121'], IssueRecord) assert isinstance(result['org/repo#301'], HierarchyIssueRecord) @@ -69,7 +69,7 @@ def test_generate_isolated_record_types_no_labels_no_type_defined(mocker, mock_r assert isinstance(result['org/repo#304'], HierarchyIssueRecord) assert isinstance(result['org/repo#123'], PullRequestRecord) assert isinstance(result['org/repo#124'], PullRequestRecord) - assert isinstance(result["org/repo@merge_commit_sha_direct"], CommitRecord) + assert isinstance(result["merge_commit_sha_direct"], CommitRecord) rec_i = cast(IssueRecord, result['org/repo#121']) assert 0 == rec_i.pull_requests_count() @@ -130,7 +130,7 @@ def test_generate_isolated_record_types_with_labels_no_type_defined(mocker, mock result = factory.generate(mined_data_isolated_record_types_with_labels_no_type_defined) assert 8 == len(result) - assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "org/repo@merge_commit_sha_direct"}.issubset(result.keys()) + assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "merge_commit_sha_direct"}.issubset(result.keys()) assert isinstance(result['org/repo#121'], IssueRecord) assert isinstance(result['org/repo#301'], HierarchyIssueRecord) @@ -139,7 +139,7 @@ def test_generate_isolated_record_types_with_labels_no_type_defined(mocker, mock assert isinstance(result['org/repo#304'], HierarchyIssueRecord) assert isinstance(result['org/repo#123'], PullRequestRecord) assert isinstance(result['org/repo#124'], PullRequestRecord) - assert isinstance(result["org/repo@merge_commit_sha_direct"], CommitRecord) + assert isinstance(result["merge_commit_sha_direct"], CommitRecord) rec_i = cast(IssueRecord, result['org/repo#121']) assert 0 == rec_i.pull_requests_count() @@ -193,7 +193,7 @@ def test_generate_isolated_record_types_no_labels_with_type_defined(mocker, mock result = factory.generate(mined_data_isolated_record_types_no_labels_with_type_defined) assert 8 == len(result) - assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "org/repo@merge_commit_sha_direct"}.issubset(result.keys()) + assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "merge_commit_sha_direct"}.issubset(result.keys()) assert isinstance(result['org/repo#121'], IssueRecord) assert isinstance(result['org/repo#301'], HierarchyIssueRecord) @@ -202,7 +202,7 @@ def test_generate_isolated_record_types_no_labels_with_type_defined(mocker, mock assert isinstance(result['org/repo#304'], HierarchyIssueRecord) assert isinstance(result['org/repo#123'], PullRequestRecord) assert isinstance(result['org/repo#124'], PullRequestRecord) - assert isinstance(result["org/repo@merge_commit_sha_direct"], CommitRecord) + assert isinstance(result["merge_commit_sha_direct"], CommitRecord) rec_i = cast(IssueRecord, result['org/repo#121']) assert 0 == rec_i.pull_requests_count() @@ -256,7 +256,7 @@ def test_generate_isolated_record_types_with_labels_with_type_defined(mocker, mo result = factory.generate(mined_data_isolated_record_types_with_labels_with_type_defined) assert 8 == len(result) - assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "org/repo@merge_commit_sha_direct"}.issubset(result.keys()) + assert {'org/repo#121', 'org/repo#301', 'org/repo#302', 'org/repo#303', 'org/repo#304', 'org/repo#123', 'org/repo#124', "merge_commit_sha_direct"}.issubset(result.keys()) assert isinstance(result['org/repo#121'], IssueRecord) assert isinstance(result['org/repo#301'], HierarchyIssueRecord) @@ -265,7 +265,7 @@ def test_generate_isolated_record_types_with_labels_with_type_defined(mocker, mo assert isinstance(result['org/repo#304'], HierarchyIssueRecord) assert isinstance(result['org/repo#123'], PullRequestRecord) assert isinstance(result['org/repo#124'], PullRequestRecord) - assert isinstance(result["org/repo@merge_commit_sha_direct"], CommitRecord) + assert isinstance(result["merge_commit_sha_direct"], CommitRecord) rec_i = cast(IssueRecord, result['org/repo#121']) assert 0 == rec_i.pull_requests_count() diff --git a/tests/test_filter.py b/tests/test_filter.py index 7baa7b85..0f652b9e 100644 --- a/tests/test_filter.py +++ b/tests/test_filter.py @@ -15,10 +15,10 @@ # -import pytest from unittest.mock import MagicMock from datetime import datetime +from github.Repository import Repository from release_notes_generator.filter import FilterByRelease from release_notes_generator.model.mined_data import MinedData @@ -30,7 +30,7 @@ def test_filter_no_release(mocker): # Mock MinedData data = MagicMock(spec=MinedData) - data.repository = MagicMock() + data.home_repository.return_value = MagicMock(spec=Repository) data.since = datetime(2023, 1, 1) data.release = None data.issues = [MagicMock(closed_at=None), MagicMock(closed_at=None)] @@ -55,7 +55,7 @@ def test_filter_with_release(mocker): # Mock MinedData data = MagicMock(spec=MinedData) - data.repository = MagicMock() + data.home_repository.return_value = MagicMock(spec=Repository) data.release = MagicMock() data.since = datetime(2023, 1, 1) diff --git a/tests/test_miner.py b/tests/test_miner.py index c446a6a2..d741f5bd 100644 --- a/tests/test_miner.py +++ b/tests/test_miner.py @@ -74,7 +74,7 @@ def test_get_latest_release_from_tag_name_not_defined_2_releases_type_error(mock release_notes_miner._safe_call = decorator_mock mocker.patch("semver.Version.parse", side_effect=TypeError) - latest_release = release_notes_miner.get_latest_release(data.repository) + latest_release = release_notes_miner.get_latest_release(data.home_repository) assert latest_release is None assert ('Getting latest release by semantic ordering (could not be the last one by time).',) == mock_log_info.call_args_list[0][0] @@ -101,9 +101,9 @@ def test_get_latest_release_from_tag_name_not_defined_2_releases_value_error(moc release_notes_miner._safe_call = decorator_mock mocker.patch("semver.Version.parse", side_effect=ValueError) - data.repository.get_releases = mocker.MagicMock(return_value=mock_git_releases) + data.home_repository.get_releases = mocker.MagicMock(return_value=mock_git_releases) - latest_release = release_notes_miner.get_latest_release(data.repository) + latest_release = release_notes_miner.get_latest_release(data.home_repository) assert latest_release is None assert ('Getting latest release by semantic ordering (could not be the last one by time).',) == mock_log_info.call_args_list[0][0] @@ -128,7 +128,7 @@ def test_get_latest_release_from_tag_name_not_defined_2_releases(mocker, mock_re release_notes_miner = DataMiner(github_mock, mock_rate_limit) release_notes_miner._safe_call = decorator_mock - latest_release = release_notes_miner.get_latest_release(data.repository) + latest_release = release_notes_miner.get_latest_release(data.home_repository) assert latest_release is not None assert ('Getting latest release by semantic ordering (could not be the last one by time).',) == mock_log_info.call_args_list[0][0] @@ -150,7 +150,7 @@ def test_get_latest_release_from_tag_name_not_defined_no_release(mocker, mock_re release_notes_miner = DataMiner(github_mock, mock_rate_limit) release_notes_miner._safe_call = decorator_mock - latest_release = release_notes_miner.get_latest_release(data.repository) + latest_release = release_notes_miner.get_latest_release(data.home_repository) assert latest_release is None assert 2 == len(mock_log_info.call_args_list) @@ -176,7 +176,7 @@ def test_get_latest_release_from_tag_name_defined_release_exists(mocker, mock_re release_notes_miner = DataMiner(github_mock, mock_rate_limit) release_notes_miner._safe_call = decorator_mock - latest_release = release_notes_miner.get_latest_release(data.repository) + latest_release = release_notes_miner.get_latest_release(data.home_repository) assert rls_mock == latest_release mock_exit.assert_not_called() @@ -203,7 +203,7 @@ def test_get_latest_release_from_tag_name_defined_no_release(mocker, mock_repo): release_notes_miner = DataMiner(github_mock, mock_rate_limit) release_notes_miner._safe_call = decorator_mock - latest_release = release_notes_miner.get_latest_release(data.repository) + latest_release = release_notes_miner.get_latest_release(data.home_repository) assert latest_release is None mock_exit.assert_called_once_with(1) From ec0e619ed5bd1693cb79e6a84813baa213b1c92b Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Sun, 28 Sep 2025 23:25:58 +0200 Subject: [PATCH 2/4] Removed not used input. --- release_notes_generator/generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_notes_generator/generator.py b/release_notes_generator/generator.py index a9abb6ca..07ced620 100644 --- a/release_notes_generator/generator.py +++ b/release_notes_generator/generator.py @@ -99,7 +99,7 @@ def generate(self) -> Optional[str]: assert data_filtered_by_release.home_repository is not None, "Repository must not be None" rls_notes_records: dict[str, Record] = self._get_record_factory( - github=self._github_instance, home_repository=data_filtered_by_release.home_repository, miner=miner + github=self._github_instance, home_repository=data_filtered_by_release.home_repository ).generate(data=data_filtered_by_release) return ReleaseNotesBuilder( From 536175d3bb3e4b3c43ae61955913a9ce87b64dfb Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Sep 2025 08:48:23 +0200 Subject: [PATCH 3/4] Addressed Rabbit review comments. --- release_notes_generator/miner.py | 2 +- .../model/sub_issue_record.py | 10 --------- .../record/factory/default_record_factory.py | 22 +++++++++++++------ .../factory/issue_hierarchy_record_factory.py | 17 +++++++++++--- .../factory/test_default_record_factory.py | 2 -- tests/test_filter.py | 4 ++-- 6 files changed, 32 insertions(+), 25 deletions(-) diff --git a/release_notes_generator/miner.py b/release_notes_generator/miner.py index 0855f99e..8e34434d 100644 --- a/release_notes_generator/miner.py +++ b/release_notes_generator/miner.py @@ -67,7 +67,7 @@ def get_repository(self, full_name: str) -> Optional[Repository]: Returns: Optional[Repository]: The GitHub repository if found, None otherwise. """ - repo: Repository = self._safe_call(self.github_instance.get_repo)(full_name) + repo: Optional[Repository] = self._safe_call(self.github_instance.get_repo)(full_name) if repo is None: logger.error("Repository not found: %s", full_name) return None diff --git a/release_notes_generator/model/sub_issue_record.py b/release_notes_generator/model/sub_issue_record.py index 25569f75..bd62baf3 100644 --- a/release_notes_generator/model/sub_issue_record.py +++ b/release_notes_generator/model/sub_issue_record.py @@ -23,14 +23,4 @@ def __init__(self, sub_issue: SubIssue | Issue, issue_labels: Optional[list[str] # properties - override IssueRecord properties - # @property - # def issue(self) -> SubIssue: - # """ - # Gets the issue associated with the record. - # Returns: The issue associated with the record. - # """ - # if not isinstance(self._issue, SubIssue): - # raise TypeError("Expected SubIssue") - # return self._issue - # properties - specific to IssueRecord diff --git a/release_notes_generator/record/factory/default_record_factory.py b/release_notes_generator/record/factory/default_record_factory.py index b24ee598..b6caa0b5 100644 --- a/release_notes_generator/record/factory/default_record_factory.py +++ b/release_notes_generator/record/factory/default_record_factory.py @@ -130,13 +130,21 @@ def register_pull_request(pr: PullRequest, skip_rec: bool) -> None: parent_issue_id, ) # dev note: here we expect that PR links to an issue in the same repository !!! - pi_repo_name, pi_number = parent_issue_id.split("#") - parent_repository = data.get_repository(pi_repo_name) - parent_issue = ( - self._safe_call(parent_repository.get_issue)(pi_number) - if parent_repository is not None - else None - ) + pi_repo_name, pi_number_str = parent_issue_id.split("#", 1) + try: + pi_number = int(pi_number_str) + except ValueError: + logger.error("Invalid parent issue id: %s", parent_issue_id) + continue + parent_repository = data.get_repository(pi_repo_name) or self.get_repository(pi_repo_name) + if parent_repository is not None: + # cache for subsequent lookups + if data.get_repository(pi_repo_name) is None: + data.add_repository(parent_repository) + parent_issue = self._safe_call(parent_repository.get_issue)(pi_number) + else: + parent_issue = None + if parent_issue is not None: self._create_record_for_issue(parent_issue) diff --git a/release_notes_generator/record/factory/issue_hierarchy_record_factory.py b/release_notes_generator/record/factory/issue_hierarchy_record_factory.py index e7ed52c4..360e8934 100644 --- a/release_notes_generator/record/factory/issue_hierarchy_record_factory.py +++ b/release_notes_generator/record/factory/issue_hierarchy_record_factory.py @@ -160,9 +160,20 @@ def _register_pull_and_its_commits_to_issue( issue_id, ) # dev note: here we expect that PR links to an issue in the same repository !!! - i_repo_name, i_number = issue_id.split("#") - parent_repository = data.get_repository(i_repo_name) - parent_issue = self._safe_call(parent_repository.get_issue)(i_number) if parent_repository else None + i_repo_name, i_number_str = issue_id.split("#", 1) + try: + i_number = int(i_number_str) + except ValueError: + logger.error("Invalid issue id: %s", issue_id) + continue + parent_repository = data.get_repository(i_repo_name) or self.get_repository(i_repo_name) + if parent_repository is not None: + if data.get_repository(i_repo_name) is None: + data.add_repository(parent_repository) + parent_issue = self._safe_call(parent_repository.get_issue)(i_number) + else: + parent_issue = None + if parent_issue is not None: self._create_issue_record_using_sub_issues_existence(parent_issue, data) diff --git a/tests/release_notes/record/factory/test_default_record_factory.py b/tests/release_notes/record/factory/test_default_record_factory.py index dec01249..093e239f 100644 --- a/tests/release_notes/record/factory/test_default_record_factory.py +++ b/tests/release_notes/record/factory/test_default_record_factory.py @@ -352,7 +352,6 @@ def test_generate_with_no_issues(mocker, mock_repo, request): pr1, pr2, commit1, commit2 = setup_no_issues_pulls_commits(mocker) data.pull_requests = [pr1, pr2] data.commits = [commit1, commit2] - data.home_repository.return_value = request.getfixturevalue("mock_repo") data.issues = [] # No issues records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) @@ -394,7 +393,6 @@ def test_generate_with_no_issues_skip_labels(mocker, mock_repo, request): data.pull_requests = [pr1, pr2] data.commits = [commit1, commit2] - data.home_repository.return_value = request.getfixturevalue("mock_repo") data.issues = [] # No issues records = DefaultRecordFactory(mock_github_client, mock_repo).generate(data) diff --git a/tests/test_filter.py b/tests/test_filter.py index 0f652b9e..30c58809 100644 --- a/tests/test_filter.py +++ b/tests/test_filter.py @@ -30,7 +30,7 @@ def test_filter_no_release(mocker): # Mock MinedData data = MagicMock(spec=MinedData) - data.home_repository.return_value = MagicMock(spec=Repository) + data.home_repository = MagicMock(spec=Repository) data.since = datetime(2023, 1, 1) data.release = None data.issues = [MagicMock(closed_at=None), MagicMock(closed_at=None)] @@ -55,7 +55,7 @@ def test_filter_with_release(mocker): # Mock MinedData data = MagicMock(spec=MinedData) - data.home_repository.return_value = MagicMock(spec=Repository) + data.home_repository = MagicMock(spec=Repository) data.release = MagicMock() data.since = datetime(2023, 1, 1) From a0f9219e62078604ca43945bf7d5722815dfc415 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 29 Sep 2025 09:04:22 +0200 Subject: [PATCH 4/4] Addressed Rabbit review comments. --- .../record/factory/default_record_factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_notes_generator/record/factory/default_record_factory.py b/release_notes_generator/record/factory/default_record_factory.py index b6caa0b5..0eb020b9 100644 --- a/release_notes_generator/record/factory/default_record_factory.py +++ b/release_notes_generator/record/factory/default_record_factory.py @@ -94,7 +94,7 @@ def get_repository(self, full_name: str) -> Optional[Repository]: Returns: Optional[Repository]: The GitHub repository if found, None otherwise. """ - repo: Repository = self._safe_call(self._github.get_repo)(full_name) + repo: Optional[Repository] = self._safe_call(self._github.get_repo)(full_name) if repo is None: logger.error("Repository not found: %s", full_name) return None