Skip to content

feat: CSP support #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 16, 2025
Merged

feat: CSP support #58

merged 11 commits into from
Jul 16, 2025

Conversation

fsbraun
Copy link
Member

@fsbraun fsbraun commented Jul 15, 2025

Description

Replace inline CSS and JS by media files if djangocms-attributes-field is installed in INSTALLED_APPS. Otherwise keep the inline code for backward compatibility.

Related resources

  • #...
  • #...

Checklist

  • I have opened this pull request against master
  • I have added or modified the tests when changing logic
  • I have followed the conventional commits guidelines to add meaningful information into the changelog
  • I have read the contribution guidelines and I have joined #workgroup-pr-review on
    Slack to find a “pr review buddy” who is going to review my pull request.

Summary by Sourcery

Enable Content Security Policy compatibility by extracting inline assets, refactor widget asset loading, update version and metadata, modernize CI with Ruff, and add publish workflows

New Features:

  • Provide CSP support by using external JS/CSS media files when the app is installed, falling back to inline code otherwise
  • Add static assets for widget JS/CSS to support CSP restrictions
  • Add GitHub Actions workflows for publishing distributions to TestPyPI and PyPI

Enhancements:

  • Extract inline CSS/JS into static widget.js and widget.css and refactor widget to conditionally include them
  • Bump version to 3.0.0 and update setup.py classifiers for Python 3.12/3.13, Django 5.x, and Django CMS 5.0
  • Add pyproject.toml with configurations for Black, djlint, and Ruff

CI:

  • Replace flake8 and isort checks with Ruff and enable workflow concurrency
  • Expand CI test matrix to cover Python 3.12/3.13 and additional Django and Django CMS versions

Deployment:

  • Introduce publish-to-test-pypi and publish-to-live-pypi GitHub workflows

Copy link
Contributor

sourcery-ai bot commented Jul 15, 2025

Reviewer's Guide

This PR externalizes inline widget CSS/JS into static assets and conditionally inlines them to support CSP, revamps CI workflows (linting with Ruff, expanded test matrix, concurrency), introduces publish pipelines for TestPyPI and PyPI, bumps the package version and supported platforms, and adds project lint/format configs.

Class diagram for AttributesWidget CSP support

classDiagram
    class AttributesWidget {
        +render(name, value, attrs=None, renderer=None)
        +value_from_datadict(data, files, name)
    }
    class Widget
    AttributesWidget --|> Widget
    class apps {
        +is_installed(app_name)
    }
    AttributesWidget ..> apps : uses
    class _inline_code
    AttributesWidget ..> _inline_code : uses
    class _read_static_files {
        +_read_static_files()
    }
    _inline_code ..> _read_static_files : calls if not installed
Loading

File-Level Changes

Change Details Files
Implement CSP support by externalizing inline CSS and JS and inlining conditionally
  • Detect if the app is installed and set _inline_code accordingly
  • Read and format static CSS/JS at module load when not installed
  • Replace the hardcoded inline <style>/<script> block in render() with module‐level _inline_code
djangocms_attributes_field/widgets.py
Add static asset files for widget behavior and styling
  • Add widget.js containing the JS initialization logic
  • Add widget.css containing the styling rules
djangocms_attributes_field/static/djangocms_attributes_field/widget.js
djangocms_attributes_field/static/djangocms_attributes_field/widget.css
Revise GitHub Actions workflows for linting and testing
  • Replace separate flake8/isort jobs with a single ruff job
  • Update action versions and add concurrency grouping
  • Expand test matrix to Python 3.12/3.13 and more Django/DjangoCMS combos
.github/workflows/lint.yml
.github/workflows/test.yml
Add CI/CD workflows for publishing to TestPyPI and PyPI
  • Create publish-to-test-pypi.yml to build and upload to TestPyPI
  • Create publish-to-live-pypi.yml to build and upload on release to PyPI
.github/workflows/publish-to-test-pypi.yml
.github/workflows/publish-to-live-pypi.yml
Bump package version and update supported platforms
  • Update version to 3.0.0
  • Extend setup.py classifiers for Python 3.12/3.13, Django 5.x, and DjangoCMS 5.x
djangocms_attributes_field/__init__.py
setup.py
Introduce project‐level configuration in pyproject.toml
  • Add tool.black, tool.djlint, and tool.ruff settings
  • Define excludes, ignores, and lint rules
pyproject.toml
Minor cleanup and documentation updates
  • Remove extraneous blank line in fields.py
  • Update CHANGELOG.rst and README.rst for the version bump and CSP feature
djangocms_attributes_field/fields.py
CHANGELOG.rst
README.rst

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @fsbraun - I've reviewed your changes - here's some feedback:

  • Loading the JS/CSS files via a hard-coded open() on a relative path is brittle—consider using Django’s staticfiles finders or importlib.resources to reliably locate package assets.
  • When djangocms_attributes_field is installed you currently suppress inline code but haven’t added a Media class on AttributesWidget, so the external CSS/JS won’t be enqueued—please define class Media with the correct asset paths.
  • In your CI matrix (.github/workflows/test.yml) the dj52_cms50.txt entry is duplicated, remove the extra line to avoid running identical test jobs twice.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Loading the JS/CSS files via a hard-coded open() on a relative path is brittle—consider using Django’s staticfiles finders or importlib.resources to reliably locate package assets.
- When `djangocms_attributes_field` is installed you currently suppress inline code but haven’t added a Media class on `AttributesWidget`, so the external CSS/JS won’t be enqueued—please define `class Media` with the correct asset paths.
- In your CI matrix (`.github/workflows/test.yml`) the `dj52_cms50.txt` entry is duplicated, remove the extra line to avoid running identical test jobs twice.

## Individual Comments

### Comment 1
<location> `djangocms_attributes_field/widgets.py:17` </location>
<code_context>
+# inline scripts/styles, but also support projects that historically do not use
+# djangocms_attributes_field as an app, but still want to use the widget.
+
+if apps.is_installed('djangocms_attributes_field'):
+    _inline_code = ""
+else:
</code_context>

<issue_to_address>
The use of apps.is_installed at import time may cause issues in some Django setups.

Checking apps.is_installed at import time can trigger ImportError or AppRegistryNotReady if the app registry isn't ready. Move this check to runtime, such as within the render method, to prevent these issues.
</issue_to_address>

### Comment 2
<location> `djangocms_attributes_field/widgets.py:21` </location>
<code_context>
+    _inline_code = ""
+else:
+    def _read_static_files():
+        with open('./static/djangocms_attributes_field/widget.js', 'r', encoding='utf-8') as f:
+            js_code = f.read()
+        with open('./static/djangocms_attributes_field/widget.css', 'r', encoding='utf-8') as f:
+            css_code = f.read()
+        return css_code, js_code
</code_context>

<issue_to_address>
Hardcoding static file paths may break in various deployment scenarios.

Relative paths like './static/djangocms_attributes_field/widget.js' may not work reliably in production or with collectstatic. Use Django's staticfiles finders to ensure correct file resolution.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

fsbraun and others added 3 commits July 15, 2025 16:27
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Copy link

codecov bot commented Jul 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.44%. Comparing base (47864a0) to head (99174d8).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
+ Coverage   90.36%   91.44%   +1.08%     
==========================================
  Files           4        4              
  Lines         166      187      +21     
  Branches       26       29       +3     
==========================================
+ Hits          150      171      +21     
  Misses         10       10              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fsbraun fsbraun requested a review from vinitkumar July 15, 2025 15:20
@vinitkumar vinitkumar merged commit 41d95f8 into master Jul 16, 2025
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants