diff --git a/openwisp_utils/cliff.toml b/openwisp_utils/cliff.toml index a25978f67..be1aee67f 100644 --- a/openwisp_utils/cliff.toml +++ b/openwisp_utils/cliff.toml @@ -17,6 +17,11 @@ body = """ ~~~~~~~~ {% for commit in commits %} - {{ commit.message | split(pat="\n") | first | split(pat="]") | nth(n=1) | trim | upper_first }} + {% for line in commit.message | split(pat="\n") %} + {%- if not loop.first -%} + OW_CHANGELOG_BODY:{{ line }} + {% endif -%} + {% endfor %} {%- endfor -%} {%- endif -%} {%- endfor -%} @@ -36,6 +41,11 @@ body = """ +++++++++++++++++++++++++++++++++ {% for commit in commits %} - {{ commit.message | split(pat="\n") | first | split(pat="]") | nth(n=1) | trim | upper_first }} + {% for line in commit.message | split(pat="\n") %} + {%- if not loop.first -%} + OW_CHANGELOG_BODY:{{ line }} + {% endif -%} + {% endfor %} {%- endfor -%} {%- endif -%} {%- endfor -%} @@ -46,6 +56,11 @@ body = """ +++++++++++++ {% for commit in commits %} - {{ commit.message | split(pat="\n") | first | split(pat="]") | nth(n=1) | trim | upper_first }} + {% for line in commit.message | split(pat="\n") %} + {%- if not loop.first -%} + OW_CHANGELOG_BODY:{{ line }} + {% endif -%} + {% endfor %} {%- endfor -%} {% endif %} {%- endfor -%} @@ -56,6 +71,11 @@ body = """ ++++++++++++ {% for commit in commits %} - {{ commit.message | split(pat="\n") | first | split(pat="]") | nth(n=1) | trim | upper_first }} + {% for line in commit.message | split(pat="\n") %} + {%- if not loop.first -%} + OW_CHANGELOG_BODY:{{ line }} + {% endif -%} + {% endfor %} {%- endfor -%} {%- endif -%} {%- endfor -%} @@ -67,6 +87,11 @@ body = """ ~~~~~~~~ {% for commit in commits %} - {{ commit.message | split(pat="\n") | first | split(pat="]") | nth(n=1) | trim | upper_first }} + {% for line in commit.message | split(pat="\n") %} + {%- if not loop.first -%} + OW_CHANGELOG_BODY:{{ line }} + {% endif -%} + {% endfor %} {%- endfor -%} {% endif %} {% endfor %} diff --git a/openwisp_utils/releaser/changelog.py b/openwisp_utils/releaser/changelog.py index 225645442..6e1362d62 100644 --- a/openwisp_utils/releaser/changelog.py +++ b/openwisp_utils/releaser/changelog.py @@ -9,6 +9,19 @@ from .utils import _call_docstrfmt +CHANGELOG_BODY_MARKER = "OW_CHANGELOG_BODY:" + + +def _get_changelog_line_content(line): + stripped_line = line.strip() + if stripped_line.startswith(CHANGELOG_BODY_MARKER): + return stripped_line.removeprefix(CHANGELOG_BODY_MARKER).strip() + return stripped_line + + +def _convert_markdown_links_to_rst(text): + return re.sub(r"\[([^\]]+)\]\(([^)]+)\)", r"`\1 <\2>`__", text) + def find_cliff_config(): # Locates the cliff.toml file packaged within 'openwisp_utils'. @@ -76,9 +89,71 @@ def run_git_cliff(version=None): sys.exit(1) -def process_changelog(changelog_text): +def _clean_commit_metadata(lines): + cleaned_lines = [] + skip_dependabot_metadata = False + dependabot_metadata_start = re.compile(r"^\s*---\s*$") + trailer_pattern = re.compile( + r"^\s*(?:Signed-off-by|Co-authored-by):|cherry picked from commit", + re.IGNORECASE, + ) + + for index, line in enumerate(lines): + is_body_line = line.strip().startswith(CHANGELOG_BODY_MARKER) + stripped_line = _get_changelog_line_content(line) + if skip_dependabot_metadata: + if stripped_line == "...": + skip_dependabot_metadata = False + continue + if stripped_line == "updated-dependencies:": + skip_dependabot_metadata = True + continue + if dependabot_metadata_start.match(stripped_line): + has_dependabot_metadata = any( + _get_changelog_line_content(remaining_line) == "updated-dependencies:" + for remaining_index, remaining_line in enumerate(lines) + if remaining_index > index + ) + if has_dependabot_metadata: + skip_dependabot_metadata = True + continue + if is_body_line and len(stripped_line) > 3 and set(stripped_line) == {"-"}: + continue + if trailer_pattern.search(stripped_line): + continue + cleaned_lines.append(line) + return cleaned_lines + + +def _format_commit_body_lines(lines, changelog_format="rst"): + formatted_lines = [] + previous_line_was_body = False + for line in lines: + stripped_line = line.strip() + if stripped_line.startswith(CHANGELOG_BODY_MARKER): + body_line = stripped_line.removeprefix(CHANGELOG_BODY_MARKER).strip() + if body_line: + if changelog_format == "rst": + body_line = _convert_markdown_links_to_rst(body_line) + if not previous_line_was_body and formatted_lines[-1:] != [""]: + formatted_lines.append("") + formatted_lines.append(f" {body_line}") + previous_line_was_body = True + elif previous_line_was_body and formatted_lines[-1:] != [""]: + formatted_lines.append("") + continue + if previous_line_was_body and stripped_line.startswith("- "): + formatted_lines.append("") + formatted_lines.append(line) + previous_line_was_body = False + return formatted_lines + + +def process_changelog(changelog_text, changelog_format="rst"): """Processes raw changelog text to reorder and clean the Dependencies section.""" - lines = changelog_text.splitlines() + lines = _format_commit_body_lines( + _clean_commit_metadata(changelog_text.splitlines()), changelog_format + ) # Iterate through the lines to find the start and end of the section # to isolate it for Dependency processing @@ -111,7 +186,7 @@ def process_changelog(changelog_text): # If no Dependencies section was found, just return if dep_start_index == -1: - return changelog_text.strip() + return "\n".join(lines).strip() lines_before_deps = lines[:dep_start_index] dependency_lines_start = dep_start_index + 2 @@ -127,25 +202,32 @@ def process_changelog(changelog_text): r"-\s*(?:Update|Bump)\s+`?`?([\w-]+)`?`?\s+requirement.*to\s+([~<>=!0-9a-zA-Z.,-]+)" ) + current_entry = None for line in dependency_lines: match = dep_update_pattern.search(line) if match: package_name = match.group(1) version_spec = match.group(2) final_version_spec = version_spec.split(",")[-1] - bumped_dependencies[package_name] = ( - f"- Bumped ``{package_name}{final_version_spec}``" - ) + current_entry = [f"- Bumped ``{package_name}{final_version_spec}``"] + bumped_dependencies[package_name] = current_entry + elif line.startswith("- "): + current_entry = [line] + other_dependencies.append(current_entry) else: - if line.strip() and line not in other_dependencies: - other_dependencies.append(line) + if current_entry is not None: + current_entry.append(line) + elif line.strip(): + other_dependencies.append([line]) final_lines = lines_before_deps final_lines.append(lines[dep_start_index]) final_lines.append(section_separator) - final_lines.extend(sorted(bumped_dependencies.values())) - final_lines.extend(other_dependencies) + for dependency_entry in sorted(bumped_dependencies.values()): + final_lines.extend(dependency_entry) + for dependency_entry in other_dependencies: + final_lines.extend(dependency_entry) # new line after Dependencies section if lines_after_deps: diff --git a/openwisp_utils/releaser/release.py b/openwisp_utils/releaser/release.py index c31768927..da8604666 100644 --- a/openwisp_utils/releaser/release.py +++ b/openwisp_utils/releaser/release.py @@ -219,7 +219,9 @@ def main(): print("No changes found for the new release. Exiting.") sys.exit(0) - processed_block = process_changelog(raw_changelog_block) + processed_block = process_changelog( + raw_changelog_block, changelog_format=config["changelog_format"] + ) formatted_block_rst = format_rst_block(processed_block) changelog_content = formatted_block_rst diff --git a/openwisp_utils/releaser/tests/samples/changelogs/full_changelog.rst b/openwisp_utils/releaser/tests/samples/changelogs/full_changelog.rst index 0906f7bfb..a7528c09b 100644 --- a/openwisp_utils/releaser/tests/samples/changelogs/full_changelog.rst +++ b/openwisp_utils/releaser/tests/samples/changelogs/full_changelog.rst @@ -8,11 +8,23 @@ Features ~~~~~~~~ - ValidatedModelSerializer: added exclude_validation, don't set m2m + + - Allow excluding fields from validation in ValidatedModelSerializer. + - Handled exclusion of direct setting of m2m fields. + - Added retry mechanism to SeleniumTestMixin `#464 `_ + + Retry selenium tests if the tests fails on the first attempt. This + prevents failng the CI build from flaky tests. + + Closes `#464 `_ + - Generate CHANGES.rst automatically `#496 `_ + Closes `#496 `_ + Changes ~~~~~~~ @@ -22,6 +34,10 @@ Backward-incompatible changes - Dropped support for OPENWISP_EMAIL_TEMPLATE setting `#482 `_ + Updated docs to suggest overriding the template. + + Closes `#482 `_ + Other changes +++++++++++++ @@ -30,18 +46,61 @@ Other changes - Switched to prettier for CSS/JS linting `#367 `_ + Closes `#367 `_ + Dependencies ++++++++++++ - Bumped ``djangorestframework<3.16.1`` + + Updates the requirements on `djangorestframework + `__ to permit the + latest version. - `Release notes + `__ - `Commits + `__ + - Bumped ``pytest-asyncio<0.27`` + + Updates the requirements on `pytest-asyncio + `__ to permit the latest + version. - `Release notes + `__ - `Commits + `__ + - Bumped ``selenium<4.35`` + + Updates the requirements on `selenium + `__ to permit the latest + version. - `Release notes + `__ - `Commits + `__ + - Bumped ``swapper~=1.4.0`` + + Updates the requirements on `swapper + `__ to ~=1.4.0. - + `Release notes + `__ - + `Changelog + `__ + - `Commits + `__ + - Updated QA dependencies Bugfixes ~~~~~~~~ - Fixed padding of the email container + + - Removed margin-top on logo container + - Fixed the recipient string in email template - Fixed the height of the logo in email template + + Bug: Setting both heights and width would require overriding the + template when the logo is customized. + + Fix: Setting the height to auto let's the logo adapt to width. If + further customizations are required, then the user will need to override + the email template. diff --git a/openwisp_utils/releaser/tests/test_changelog.py b/openwisp_utils/releaser/tests/test_changelog.py index b0e562a34..2735eaf9f 100644 --- a/openwisp_utils/releaser/tests/test_changelog.py +++ b/openwisp_utils/releaser/tests/test_changelog.py @@ -129,6 +129,153 @@ def _git_commit(message): assert actual_output == expected_output +def _generate_changelog_for_messages(git_repo, messages): + commit_count = 0 + + for message in messages: + with open(f"file_{commit_count}.txt", "w") as f: + f.write(f"This is file number {commit_count}") + subprocess.run(["git", "add", "."], check=True, capture_output=True) + subprocess.run( + ["git", "commit", "--file=-"], + input=message.encode("utf-8"), + check=True, + capture_output=True, + ) + commit_count += 1 + + raw_changelog = run_git_cliff() + processed_changelog = process_changelog(raw_changelog) + processed_changelog = "Changelog\n=========\n\n" + processed_changelog + return format_rst_block(processed_changelog) + + +def test_changelog_generation_includes_commit_body_without_git_trailers(git_repo): + commit_message = """[fix] Fixed admin subnet export multitenancy security issue + +Before this patch, if a specific subnet ID was known, +any staff user with permissions to operate on subnet +objects was allowed to export the subnet contents, +regardless of whether they managed the organization +of the subnet or not. This patch fixes it. + +(cherry picked from commit a4b272461bfa7a1762baf0b1fd76b4f5b681586b) +Signed-off-by: Federico Capoano +Co-authored-by: Test User +""" + actual_output = _generate_changelog_for_messages(git_repo, [commit_message]) + + assert "- Fixed admin subnet export multitenancy security issue" in actual_output + assert "Before this patch" in actual_output + assert "specific subnet ID was known" in actual_output + assert "staff user" in actual_output + assert "permissions" in actual_output + assert "operate on subnet" in actual_output + assert "regardless of whether they managed the organization" in actual_output + assert "This patch fixes it." in actual_output + assert "cherry picked from commit" not in actual_output + assert "Signed-off-by:" not in actual_output + assert "Co-authored-by:" not in actual_output + + +def test_changelog_generation_excludes_dependabot_metadata_block(git_repo): + commit_message = """[deps] Update django-reversion requirement from <5.2 to >=5.1,<6.1 + +Updates the requirements on [django-reversion](https://github.com/etianen/django-reversion) +to permit the latest version. +- [Release notes](https://github.com/etianen/django-reversion/releases) +- [Changelog](https://github.com/etianen/django-reversion/blob/master/CHANGELOG.rst) +- [Commits](https://github.com/etianen/django-reversion/compare/v5.1.0...v6.0.0) + +--- +updated-dependencies: +- dependency-name: django-reversion + dependency-type: direct:production +... + +Signed-off-by: dependabot[bot] +Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> +""" + actual_output = _generate_changelog_for_messages(git_repo, [commit_message]) + + assert "- Bumped ``django-reversion<6.1``" in actual_output + assert "Updates the requirements on" in actual_output + assert "`django-reversion" in actual_output + assert "`__" in actual_output + assert "`Release notes" in actual_output + assert "`__" in actual_output + assert "`Changelog" in actual_output + assert ( + "`__" + in actual_output + ) + assert "`Commits" in actual_output + assert ( + "`__" + in actual_output + ) + assert ( + "[django-reversion](https://github.com/etianen/django-reversion)" + not in actual_output + ) + assert "[Release notes]" not in actual_output + assert "[Changelog]" not in actual_output + assert "[Commits]" not in actual_output + assert "django-reversion" in actual_output + assert "https://github.com/etianen/django-reversion/releases" in actual_output + assert ( + "https://github.com/etianen/django-reversion/blob/master/CHANGELOG.rst" + in actual_output + ) + assert ( + "https://github.com/etianen/django-reversion/compare/v5.1.0...v6.0.0" + in actual_output + ) + assert "\n---\n" not in actual_output + assert "updated-dependencies:" not in actual_output + assert "dependency-name:" not in actual_output + assert "dependency-type:" not in actual_output + assert "\n...\n" not in actual_output + assert "Signed-off-by:" not in actual_output + assert "Co-authored-by:" not in actual_output + + +def test_process_changelog_preserves_markdown_links_for_markdown_output(): + changelog_text = """Dependencies +++++++++++++ +- Update django-reversion requirement from <5.2 to >=5.1,<6.1 + +OW_CHANGELOG_BODY:Updates the requirements on [django-reversion](https://github.com/etianen/django-reversion) +OW_CHANGELOG_BODY:to permit the latest version. +OW_CHANGELOG_BODY:- [Release notes](https://github.com/etianen/django-reversion/releases) +OW_CHANGELOG_BODY:- [Changelog](https://github.com/etianen/django-reversion/blob/master/CHANGELOG.rst) +OW_CHANGELOG_BODY:- [Commits](https://github.com/etianen/django-reversion/compare/v5.1.0...v6.0.0) +""" + + processed_text = process_changelog(changelog_text, changelog_format="md") + + assert ( + "[django-reversion](https://github.com/etianen/django-reversion)" + in processed_text + ) + assert ( + "[Release notes](https://github.com/etianen/django-reversion/releases)" + in processed_text + ) + assert ( + "[Changelog](https://github.com/etianen/django-reversion/blob/master/CHANGELOG.rst)" + in processed_text + ) + assert ( + "[Commits](https://github.com/etianen/django-reversion/compare/v5.1.0...v6.0.0)" + in processed_text + ) + assert ( + "`Release notes `__" + not in processed_text + ) + + @patch("openwisp_utils.releaser.changelog.find_cliff_config", return_value=None) def test_run_git_cliff_no_config(mock_find_config): with pytest.raises(SystemExit):