diff --git a/release_notes_generator/chapters/custom_chapters.py b/release_notes_generator/chapters/custom_chapters.py index f216fe37..df39067d 100644 --- a/release_notes_generator/chapters/custom_chapters.py +++ b/release_notes_generator/chapters/custom_chapters.py @@ -99,6 +99,7 @@ def populate(self, records: dict[str, Record]) -> None: # Quick intersection check if any(lbl in ch.labels for lbl in record_labels): if record_id not in ch.rows: + record.add_to_chapter_presence(ch.title) ch.add_row(record_id, record.to_chapter_row(True)) # Track for backward compatibility (not used for gating anymore) if record_id not in self.populated_record_numbers_list: diff --git a/release_notes_generator/chapters/service_chapters.py b/release_notes_generator/chapters/service_chapters.py index 57bee16e..308dd3ec 100644 --- a/release_notes_generator/chapters/service_chapters.py +++ b/release_notes_generator/chapters/service_chapters.py @@ -135,6 +135,7 @@ def populate(self, records: dict[str, Record]) -> None: logger.debug("Skipping open HierarchyIssueRecord %s (pr_count=%d)", record_id, pr_count) elif is_issue_like and pr_count > 0: # Open issue/sub-issue with linked PRs → add to the specific chapter + record.add_to_chapter_presence(MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES) self.chapters[MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES].add_row( record_id, record.to_chapter_row() ) @@ -145,6 +146,7 @@ def populate(self, records: dict[str, Record]) -> None: pass else: if record_id not in self.used_record_numbers: + record.add_to_chapter_presence(OTHERS_NO_TOPIC) self.chapters[OTHERS_NO_TOPIC].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) @@ -164,6 +166,7 @@ def __populate_closed_issues(self, record: IssueRecord, record_id: int | str) -> pulls_count = record.pull_requests_count() if pulls_count == 0: + record.add_to_chapter_presence(CLOSED_ISSUES_WITHOUT_PULL_REQUESTS) self.chapters[CLOSED_ISSUES_WITHOUT_PULL_REQUESTS].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) populated = True @@ -174,6 +177,7 @@ def __populate_closed_issues(self, record: IssueRecord, record_id: int | str) -> if self.__is_row_present(record_id) and not self.duplicity_allowed(): return + record.add_to_chapter_presence(CLOSED_ISSUES_WITHOUT_USER_DEFINED_LABELS) self.chapters[CLOSED_ISSUES_WITHOUT_USER_DEFINED_LABELS].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) populated = True @@ -189,6 +193,7 @@ def __populate_closed_issues(self, record: IssueRecord, record_id: int | str) -> if record_id in self.used_record_numbers: return + record.add_to_chapter_presence(OTHERS_NO_TOPIC) self.chapters[OTHERS_NO_TOPIC].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) @@ -209,6 +214,7 @@ def __populate_pr(self, record: PullRequestRecord, record_id: int | str) -> None if self.__is_row_present(record_id) and not self.duplicity_allowed(): return + record.add_to_chapter_presence(MERGED_PRS_WITHOUT_ISSUE_AND_USER_DEFINED_LABELS) self.chapters[MERGED_PRS_WITHOUT_ISSUE_AND_USER_DEFINED_LABELS].add_row( record_id, record.to_chapter_row() ) @@ -219,6 +225,7 @@ def __populate_pr(self, record: PullRequestRecord, record_id: int | str) -> None if self.__is_row_present(record_id) and not self.duplicity_allowed(): return + record.add_to_chapter_presence(MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES) self.chapters[MERGED_PRS_LINKED_TO_NOT_CLOSED_ISSUES].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) @@ -229,6 +236,7 @@ def __populate_pr(self, record: PullRequestRecord, record_id: int | str) -> None if record_id in self.used_record_numbers: return + record.add_to_chapter_presence(OTHERS_NO_TOPIC) self.chapters[OTHERS_NO_TOPIC].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) @@ -241,6 +249,7 @@ def __populate_pr(self, record: PullRequestRecord, record_id: int | str) -> None if self.__is_row_present(record_id) and not self.duplicity_allowed(): return + record.add_to_chapter_presence(CLOSED_PRS_WITHOUT_ISSUE_AND_USER_DEFINED_LABELS) self.chapters[CLOSED_PRS_WITHOUT_ISSUE_AND_USER_DEFINED_LABELS].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) @@ -252,6 +261,7 @@ def __populate_pr(self, record: PullRequestRecord, record_id: int | str) -> None return # not record.is_present_in_chapters: + record.add_to_chapter_presence(OTHERS_NO_TOPIC) self.chapters[OTHERS_NO_TOPIC].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) @@ -262,6 +272,7 @@ def __populate_direct_commit(self, record: CommitRecord, record_id: int | str) - @param record: The CommitRecord object representing the direct commit. @return: None """ + record.add_to_chapter_presence(DIRECT_COMMITS) self.chapters[DIRECT_COMMITS].add_row(record_id, record.to_chapter_row()) self.used_record_numbers.append(record_id) diff --git a/release_notes_generator/model/record/commit_record.py b/release_notes_generator/model/record/commit_record.py index 95c2c6a1..ccbca748 100644 --- a/release_notes_generator/model/record/commit_record.py +++ b/release_notes_generator/model/record/commit_record.py @@ -61,9 +61,7 @@ def commit(self) -> Commit: # methods - override Record methods def to_chapter_row(self, add_into_chapters: bool = True) -> str: - if add_into_chapters: - self.added_into_chapters() - row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.present_in_chapters() > 1 else "" + row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.chapter_presence_count() > 1 else "" # collecting values for formatting commit_message = self._commit.commit.message.replace("\n", " ") diff --git a/release_notes_generator/model/record/hierarchy_issue_record.py b/release_notes_generator/model/record/hierarchy_issue_record.py index 0e7e2bed..4fadee9f 100644 --- a/release_notes_generator/model/record/hierarchy_issue_record.py +++ b/release_notes_generator/model/record/hierarchy_issue_record.py @@ -116,9 +116,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 "" + row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.chapter_presence_count() > 1 else "" format_values: dict[str, Any] = {} # collect format values diff --git a/release_notes_generator/model/record/issue_record.py b/release_notes_generator/model/record/issue_record.py index 75ffa599..d4b7e943 100644 --- a/release_notes_generator/model/record/issue_record.py +++ b/release_notes_generator/model/record/issue_record.py @@ -128,9 +128,7 @@ def find_issue(self, issue_number: int) -> Optional["IssueRecord"]: return None def to_chapter_row(self, add_into_chapters: bool = True) -> str: - if add_into_chapters: - self.added_into_chapters() - row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.present_in_chapters() > 1 else "" + row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.chapter_presence_count() > 1 else "" format_values: dict[str, Any] = {} # collect format values diff --git a/release_notes_generator/model/record/pull_request_record.py b/release_notes_generator/model/record/pull_request_record.py index a6cad647..17bb7a66 100644 --- a/release_notes_generator/model/record/pull_request_record.py +++ b/release_notes_generator/model/record/pull_request_record.py @@ -122,10 +122,7 @@ def get_labels(self) -> list[str]: return self.labels def to_chapter_row(self, add_into_chapters: bool = True) -> str: - if add_into_chapters: - self.added_into_chapters() - - row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.present_in_chapters() > 1 else "" + row_prefix = f"{ActionInputs.get_duplicity_icon()} " if self.chapter_presence_count() > 1 else "" format_values: dict[str, Any] = {} # collecting values for formatting diff --git a/release_notes_generator/model/record/record.py b/release_notes_generator/model/record/record.py index 8b557382..57b990fb 100644 --- a/release_notes_generator/model/record/record.py +++ b/release_notes_generator/model/record/record.py @@ -15,7 +15,7 @@ # """ -This module contains the BaseChapters class, which is responsible for representing the base chapters. +Defines the abstract base `Record` type used by the release notes generator. """ import logging @@ -37,7 +37,7 @@ class Record(metaclass=ABCMeta): RELEASE_NOTE_LINE_MARKS: list[str] = ["-", "*", "+"] def __init__(self, labels: Optional[list[str]] = None, skip: bool = False): - self._present_in_chapters = 0 + self._chapters_present_in: set[str] = set() self._skip = skip self._is_cross_repo: bool = False self._is_release_note_detected: Optional[bool] = None @@ -48,11 +48,12 @@ def __init__(self, labels: Optional[list[str]] = None, skip: bool = False): @property def is_present_in_chapters(self) -> bool: """ - Checks if the record is present in any chapter. + Checks if the record is present in at least one chapter. + Returns: bool: True if the record is present in at least one chapter, False otherwise. """ - return self._present_in_chapters > 0 + return len(self._chapters_present_in) > 0 @property def is_cross_repo(self) -> bool: @@ -190,21 +191,23 @@ def get_rls_notes(self, line_marks: Optional[list[str]] = None) -> str: # shared methods - def added_into_chapters(self) -> None: + def add_to_chapter_presence(self, chapter_id: str) -> None: """ - Increments the count of chapters in which the record is present. - Returns: None + Marks this record as present in the given chapter. + + Parameters: + chapter_id (str): The unique identifier of the chapter. """ - # TODO - fix in #191 - self._present_in_chapters += 1 + self._chapters_present_in.add(chapter_id) - def present_in_chapters(self) -> int: + def chapter_presence_count(self) -> int: """ - Gets the count of chapters in which the record is present. + Gets the number of unique chapters in which the record is present. + Returns: - int: The count of chapters in which the record is present. + int: The count of unique chapters containing this record. """ - return self._present_in_chapters + return len(self._chapters_present_in) def contains_min_one_label(self, labels: list[str]) -> bool: """ diff --git a/tests/integration/test_release_notes_snapshot.py b/tests/integration/test_release_notes_snapshot.py index b8b8ec8e..98ac6040 100644 --- a/tests/integration/test_release_notes_snapshot.py +++ b/tests/integration/test_release_notes_snapshot.py @@ -11,6 +11,7 @@ def __init__(self, _id: str, _labels: list[str]): self.labels = _labels self.skip = False self.is_present_in_chapters = False + self._chapters = set() def contains_change_increment(self): # matches code expectation in populate return True @@ -18,6 +19,9 @@ def contains_change_increment(self): # matches code expectation in populate def to_chapter_row(self, _include_prs: bool): # simplified row rendering return f"{self.id} row" + def add_to_chapter_presence(self, chapter_id: str): # track chapter additions + self._chapters.add(chapter_id) + return R(record_id, labels) diff --git a/tests/unit/release_notes_generator/model/test_commit_record.py b/tests/unit/release_notes_generator/model/test_commit_record.py index 63dae39a..fb69a19a 100644 --- a/tests/unit/release_notes_generator/model/test_commit_record.py +++ b/tests/unit/release_notes_generator/model/test_commit_record.py @@ -34,11 +34,15 @@ def test_to_chapter_row_duplicate_with_icon(monkeypatch, mock_commit): lambda: "[D]", ) rec = CommitRecord(mock_commit) + # Simulate adding to first chapter + rec.add_to_chapter_presence("chapter1") first = rec.to_chapter_row(True) + # Simulate adding to second chapter + rec.add_to_chapter_presence("chapter2") second = rec.to_chapter_row(True) assert not first.startswith("[D] ") assert second.startswith("[D] ") - assert rec.present_in_chapters() == 2 + assert rec.chapter_presence_count() == 2 def test_to_chapter_row_with_release_notes_injected(monkeypatch, mock_commit): # Force contains_release_notes to True and provide fake release notes diff --git a/tests/unit/release_notes_generator/model/test_record.py b/tests/unit/release_notes_generator/model/test_record.py index e6f418fb..22bf44af 100644 --- a/tests/unit/release_notes_generator/model/test_record.py +++ b/tests/unit/release_notes_generator/model/test_record.py @@ -70,7 +70,7 @@ def contains_change_increment(self) -> bool: def test_is_present_in_chapters(): rec = DummyRecord() assert not rec.is_present_in_chapters - rec.added_into_chapters() + rec.add_to_chapter_presence("chapter1") assert rec.is_present_in_chapters def test_skip_property(): @@ -81,10 +81,20 @@ def test_skip_property(): def test_present_in_chapters_count(): rec = DummyRecord() - assert rec.present_in_chapters() == 0 - rec.added_into_chapters() - rec.added_into_chapters() - assert rec.present_in_chapters() == 2 + assert rec.chapter_presence_count() == 0 + rec.add_to_chapter_presence("chapter1") + rec.add_to_chapter_presence("chapter2") + assert rec.chapter_presence_count() == 2 + +def test_present_in_chapters_unique(): + """Test that adding the same chapter multiple times doesn't increase the count.""" + rec = DummyRecord() + assert rec.chapter_presence_count() == 0 + rec.add_to_chapter_presence("chapter1") + rec.add_to_chapter_presence("chapter1") # Same chapter again + assert rec.chapter_presence_count() == 1 # Should still be 1 + rec.add_to_chapter_presence("chapter2") + assert rec.chapter_presence_count() == 2 def test_contains_min_one_label(): rec = DummyRecord(labels=["bug", "feature"])