chore: Prepare release 4.1.0 #60
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Reviewer's GuideThis PR prepares the 4.1.0 release by refactoring widget media loading to support CSPs (inlining CSS/JS or falling back to external files), adding static assets, updating packaging to PEP 621 (pyproject.toml), bumping the version, and updating tests and CI matrix accordingly. Class diagram for refactored AttributesWidget media loadingclassDiagram
class AttributesWidget {
+__init__(*args, **kwargs)
+media
+_render_row(key, value, field_name, key_attrs, val_attrs)
+render(name, value, attrs=None, renderer=None)
+value_from_datadict(data, files, name)
}
class Widget
AttributesWidget --|> Widget
class Media
AttributesWidget ..> Media : uses
class apps
AttributesWidget ..> apps : uses
class os
AttributesWidget ..> os : uses
class _read_inline_code {
+_read_inline_code()
}
AttributesWidget ..> _read_inline_code : calls
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @fsbraun - I've reviewed your changes - here's some feedback:
- Reading widget CSS and JS directly from the filesystem can break when the package is installed as a zipped wheel or in environments without a local file path—consider using pkg_resources or Django’s staticfiles finder API instead.
- The module‐level _inline_code cache may hold stale data across different settings configurations in long‐running processes; consider using a per‐widget cached_property or clearing the cache more explicitly to avoid unexpected reuse.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Reading widget CSS and JS directly from the filesystem can break when the package is installed as a zipped wheel or in environments without a local file path—consider using pkg_resources or Django’s staticfiles finder API instead.
- The module‐level _inline_code cache may hold stale data across different settings configurations in long‐running processes; consider using a per‐widget cached_property or clearing the cache more explicitly to avoid unexpected reuse.
## Individual Comments
### Comment 1
<location> `djangocms_attributes_field/widgets.py:19` </location>
<code_context>
+# djangocms_attributes_field as an app, but still want to use the widget.
+_inline_code = None
+
+def _read_inline_code():
+ if apps.is_installed('djangocms_attributes_field'):
+ _inline_code = ""
+ else:
+ def _read_static_files():
+ base_dir = os.path.dirname(os.path.abspath(__file__))
+ with open(os.path.join(base_dir, 'static/djangocms_attributes_field/widget.js'), 'r', encoding='utf-8') as f:
+ js_code = f.read()
+ with open(os.path.join(base_dir, 'static/djangocms_attributes_field/widget.css'), 'r', encoding='utf-8') as f:
+ css_code = f.read()
+ return css_code, js_code
+
+ _inline_code = "<style>{}</style><script>{}</script>".format(*_read_static_files())
+ return _inline_code
+
</code_context>
<issue_to_address>
Assignment to _inline_code inside _read_inline_code() does not use the 'global' keyword.
Without declaring 'global _inline_code', assignments in _read_inline_code() will not update the module-level variable, breaking caching and causing unnecessary file reads.
</issue_to_address>
### Comment 2
<location> `djangocms_attributes_field/widgets.py:142` </location>
<code_context>
- }(django.jQuery));
- </script>
- """
+ output += f'</div>{_inline_code}'
return mark_safe(output)
</code_context>
<issue_to_address>
Directly appending _inline_code to output may result in None being rendered.
If _inline_code is None, 'None' will appear in the HTML. Ensure _inline_code is always a string or default to an empty string when it's None.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
19
to
28
| def _read_inline_code(): | ||
| if apps.is_installed('djangocms_attributes_field'): | ||
| _inline_code = "" | ||
| else: | ||
| def _read_static_files(): | ||
| base_dir = os.path.dirname(os.path.abspath(__file__)) | ||
| with open(os.path.join(base_dir, 'static/djangocms_attributes_field/widget.js'), 'r', encoding='utf-8') as f: | ||
| js_code = f.read() | ||
| with open(os.path.join(base_dir, 'static/djangocms_attributes_field/widget.css'), 'r', encoding='utf-8') as f: | ||
| css_code = f.read() |
There was a problem hiding this comment.
issue (bug_risk): Assignment to _inline_code inside _read_inline_code() does not use the 'global' keyword.
Without declaring 'global _inline_code', assignments in _read_inline_code() will not update the module-level variable, breaking caching and causing unnecessary file reads.
| }(django.jQuery)); | ||
| </script> | ||
| """ | ||
| output += f'</div>{_inline_code}' |
There was a problem hiding this comment.
issue (bug_risk): Directly appending _inline_code to output may result in None being rendered.
If _inline_code is None, 'None' will appear in the HTML. Ensure _inline_code is always a string or default to an empty string when it's None.
Comment on lines
3
to
10
| function fixUpIds (fieldGroup) { | ||
| fieldGroup.find('.attributes-pair').each(function (idx, value) { | ||
| $(value).find('.attributes-key').attr('id', 'field-key-row-' + idx) | ||
| .siblings('label').attr('for', 'field-key-row-' + idx); | ||
| $(value).find('.attributes-value').attr('id', 'field-value-row-' + idx) | ||
| .siblings('label').attr('for', 'field-value-row-' + idx); | ||
| }); | ||
| } |
There was a problem hiding this comment.
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #60 +/- ##
=======================================
Coverage 91.44% 91.44%
=======================================
Files 4 4
Lines 187 187
Branches 29 29
=======================================
Hits 171 171
Misses 10 10
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This prepares the 4.1.0 release featuring CSP. It also removes test files from the distributed wheel fixing #59
Related resources
Checklist
masterSlack to find a “pr review buddy” who is going to review my pull request.
Summary by Sourcery
Prepare for the 4.1.0 release by adding CSP-friendly widget media loading, updating packaging metadata, refining CI tests, and bumping the version.
New Features:
Enhancements:
Build:
CI:
Tests:
Chores: