Skip to content

Conversation

@SAMurai-16
Copy link

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

Closes #565 .

Description of Changes

Please describe these changes.

Replaced django-compress-staticfiles with the new
django-minify-compress-staticfiles package for static
file minification and compression.

The new package provides:

  • CSS/JS minification using rjsmin and rcssmin
  • Both Gzip and Brotli compression support
  • Better security with path traversal protection
  • Configurable compression levels and file size limits

Updated imports in storage.py and documentation
references accordingly.

Fixes #565

SAMurai-16 and others added 4 commits February 1, 2026 10:40
Replaced django-compress-staticfiles with the new
django-minify-compress-staticfiles package for static
file minification and compression.

The new package provides:
- CSS/JS minification using rjsmin and rcssmin
- Both Gzip and Brotli compression support
- Better security with path traversal protection
- Configurable compression levels and file size limits

Updated imports in storage.py and documentation
references accordingly.

Fixes openwisp#565
Copilot AI review requested due to automatic review settings February 1, 2026 10:48
@coderabbitai
Copy link

coderabbitai bot commented Feb 1, 2026

Walkthrough

The PR replaces the staticfiles compressor dependency from django-compress-staticfiles to django-minify-compress-staticfiles (updates in setup, imports, docs, and storage class reference), exposes excluded_patterns as a computed @property on FileHashedNameMixin, modifies CompressStaticFilesStorage to inherit from the Minicompress-based base, adds a non-fatal git pull --tags step before invoking git-cliff in the releaser and corresponding tests, adjusts tests/config to skip AI summaries by default, and changes Selenium retry logic to treat skipped tests as final.

Sequence Diagram(s)

sequenceDiagram
    participant Releaser as Releaser.run_git_cliff
    participant Git as Git CLI
    participant GitCliff as git-cliff
    participant Stderr as stderr

    Releaser->>Git: git pull --tags
    alt pull succeeds
        Git-->>Releaser: exit 0
    else pull fails
        Git-->>Releaser: non-zero exit
        Releaser->>Stderr: print warning
    end
    Releaser->>GitCliff: git-cliff --unreleased ...
    GitCliff-->>Releaser: changelog output / exit
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately describes the main change: switching from django-compress-staticfiles to django-minify-compress-staticfiles.
Description check ✅ Passed The description includes the required issue reference (#565) and explains the changes made, though some checklist items remain unchecked.
Linked Issues check ✅ Passed The PR directly addresses issue #565 by replacing the problematic staticfiles compressor with django-minify-compress-staticfiles, which resolves the malformed CSS error.
Out of Scope Changes check ✅ Passed All changes are focused on replacing the staticfiles dependency and updating related imports and documentation; no unrelated changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7fd77d9 and 6a8dd0a.

📒 Files selected for processing (3)
  • tests/openwisp2/settings.py
  • tests/test_project/tests/test_admin.py
  • tests/test_project/tests/test_dashboard.py
✅ Files skipped from review due to trivial changes (1)
  • tests/openwisp2/settings.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/test_project/tests/test_admin.py
  • tests/test_project/tests/test_dashboard.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: Python==3.13 | django~=5.1.0
  • GitHub Check: Python==3.13 | django~=5.2.0
  • GitHub Check: Python==3.11 | django~=5.2.0
  • GitHub Check: Python==3.11 | django~=5.1.0
  • GitHub Check: Python==3.12 | django~=5.0.0
  • GitHub Check: Python==3.12 | django~=5.2.0
  • GitHub Check: Python==3.12 | django~=4.2.0
  • GitHub Check: Python==3.10 | django~=5.1.0
  • GitHub Check: Python==3.11 | django~=5.0.0
  • GitHub Check: Python==3.10 | django~=5.0.0
  • GitHub Check: Python==3.11 | django~=4.2.0
  • GitHub Check: Python==3.10 | django~=5.2.0
  • GitHub Check: Python==3.12 | django~=5.1.0
  • GitHub Check: Python==3.10 | django~=4.2.0

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 1, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR primarily replaces django-compress-staticfiles with django-minify-compress-staticfiles for staticfile minification/compression, updating the storage backend integration and documentation, while also introducing unrelated changes to the releaser changelog flow and selenium test runner behavior.

Changes:

  • Switched dependency to django-minify-compress-staticfiles and updated CompressStaticFilesStorage base class accordingly.
  • Updated test project settings to use Django’s STORAGES setting for staticfiles storage.
  • Modified releaser run_git_cliff to run git pull --tags first and added/adjusted tests around this behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/openwisp2/settings.py Configures STATIC_ROOT and STORAGES to use CompressStaticFilesStorage in tests.
setup.py Replaces django-compress-staticfiles with django-minify-compress-staticfiles.
openwisp_utils/tests/selenium.py Adjusts retry logic to propagate skips instead of treating them as successes.
openwisp_utils/storage.py Switches storage backend base class import to MinicompressStorage.
openwisp_utils/releaser/tests/test_changelog.py Updates and expands tests to account for the new git pull --tags call.
openwisp_utils/releaser/tests/conftest.py Mocks OPENAI_CHATGPT_TOKEN via os.environ.get to skip AI summary by default.
openwisp_utils/releaser/changelog.py Adds a pre-step to pull tags before running git cliff.
docs/developer/other-utilities.rst Updates documentation reference to the new staticfiles package and base class.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

capture_output=True,
text=True,
check=True,
)
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_git_cliff now runs subprocess.run(["git", "pull", "--tags"], ...) outside the try/except FileNotFoundError block. If git is missing, this will raise FileNotFoundError and bypass the existing user-friendly error handling (and likely produce a stack trace). Wrap this call in the same FileNotFoundError handling or move it inside the main try/except so missing git is reported consistently.

Suggested change
)
)
except FileNotFoundError:
print("Error: 'git' or 'git-cliff' command not found.", file=sys.stderr)
sys.exit(1)

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +55
subprocess.run(
["git", "pull", "--tags"],
capture_output=True,
text=True,
check=True,
)
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using git pull --tags in a release utility can unexpectedly merge remote branch changes into the local checkout (and can fail if the working tree isn’t clean). If the goal is only to update tags for changelog generation, prefer git fetch --tags (optionally with --force) to avoid altering branch state.

Copilot uses AI. Check for mistakes.
# First call (git pull --tags) succeeds, second call (git cliff) raises FileNotFoundError
mock_run.side_effect = [
subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr=""),
FileNotFoundError,
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mock_run.side_effect uses FileNotFoundError (the class) as the second element. With unittest.mock, iterable side_effect items are only raised when they are exception instances; the class will be returned as a value, leading to an unexpected AttributeError instead of the intended SystemExit. Use FileNotFoundError() (or FileNotFoundError("...")) for the second call.

Suggested change
FileNotFoundError,
FileNotFoundError(),

Copilot uses AI. Check for mistakes.
Comment on lines +48 to +58
# Pull latest tags before calculating changelog
try:
subprocess.run(
["git", "pull", "--tags"],
capture_output=True,
text=True,
check=True,
)
except subprocess.CalledProcessError as e:
print(f"Warning: Failed to pull tags: {e.stderr}", file=sys.stderr)
# Run git-cliff to calculate changelog
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description focuses on switching the staticfiles minification/compression dependency, but this PR also changes the release/changelog workflow (adding a git pull --tags call) and modifies selenium test retry/skip behavior. Please update the PR description to cover these additional changes, or split them into separate PRs to keep scope aligned with #565.

Copilot uses AI. Check for mistakes.
Replaced django-compress-staticfiles with the new
django-minify-compress-staticfiles package for static
file minification and compression.

The new package provides:
- CSS/JS minification using rjsmin and rcssmin
- Both Gzip and Brotli compression support
- Better security with path traversal protection
- Configurable compression levels and file size limits

Updated imports in storage.py and documentation
references accordingly.

Fixes openwisp#565
coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 1, 2026
Replaced django-compress-staticfiles with the new
django-minify-compress-staticfiles package for static
file minification and compression.

The new package provides:
- CSS/JS minification using rjsmin and rcssmin
- Both Gzip and Brotli compression support
- Better security with path traversal protection
- Configurable compression levels and file size limits

Updated imports in storage.py and documentation
references accordingly.

Fixes openwisp#565
coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 1, 2026
Replaced django-compress-staticfiles with the new
django-minify-compress-staticfiles package for static
file minification and compression.

The new package provides:
- CSS/JS minification using rjsmin and rcssmin
- Both Gzip and Brotli compression support
- Better security with path traversal protection
- Configurable compression levels and file size limits

Updated imports in storage.py and documentation
references accordingly.

Fixes openwisp#565
@coveralls
Copy link

Coverage Status

coverage: 97.144% (+0.002%) from 97.142%
when pulling 1b2ba1d on SAMurai-16:issues/565-collectstatic-error
into 8a56903 on openwisp:master.

@SAMurai-16
Copy link
Author

@nemesifier kindly review

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the PR contains commits from other PRs, please cleanup, I suggest to start again from master and cherry-pick.

@SAMurai-16
Copy link
Author

Sure, I'll just close this PR and start again from master

@SAMurai-16 SAMurai-16 closed this Feb 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] CompressStaticFilesStorage: "malformed css" error on collectstatic

4 participants