fix: use format_html to avoid pylint mark-safe-interpolation error - #253
Open
marlonkeating wants to merge 2 commits into
Open
fix: use format_html to avoid pylint mark-safe-interpolation error#253marlonkeating wants to merge 2 commits into
marlonkeating wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #253 +/- ##
=======================================
Coverage 87.48% 87.48%
=======================================
Files 157 157
Lines 13356 13356
Branches 1296 1296
=======================================
Hits 11685 11685
Misses 1361 1361
Partials 310 310 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates Enterprise Access to accommodate the pylint-django==2.8.0 mark-safe-interpolation (W5151) rule by switching Django admin HTML construction to format_html, and refreshes the compiled requirements sets produced by pip-tools.
Changes:
- Replace
mark_safe(...format(...))/ f-string HTML in the provisioning Django admin withdjango.utils.html.format_html. - Update compiled requirements pins across environments (base/dev/test/quality/production/validation/docs), including
pylint-django==2.8.0. - Add constraints for
social-auth-*packages inrequirements/common_constraints.txt.
High severity issues to address:
enterprise_customer_admin_linkandsubscription_plan_linkcan raise exceptions in the admin UI when the corresponding workflow step record (or its output) doesn’t exist; they should guard for missing step/output before dereferencing.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
enterprise_access/apps/provisioning/admin.py |
Use format_html for admin links and rename the “trial subscription plan” step link accessor. |
requirements/base.txt |
Refresh compiled base dependency pins. |
requirements/common_constraints.txt |
Add constraints for social-auth-app-django / social-auth-core with rationale. |
requirements/dev.txt |
Refresh compiled dev dependency pins (incl. tooling like diff-cover, pip-tools). |
requirements/django.txt |
Bump pinned Django patch version. |
requirements/doc.txt |
Refresh compiled docs dependency pins (incl. Sphinx theme updates). |
requirements/pip-tools.txt |
Bump pip-tools pin used for compilation. |
requirements/pip.txt |
Bump setuptools pin. |
requirements/production.txt |
Refresh compiled production dependency pins. |
requirements/quality.txt |
Refresh compiled quality dependency pins (incl. pylint-django==2.8.0). |
requirements/test.txt |
Refresh compiled test dependency pins (incl. pylint-django==2.8.0). |
requirements/validation.txt |
Refresh compiled validation dependency pins (incl. pylint-django==2.8.0). |
Comments suppressed due to low confidence (1)
enterprise_access/apps/provisioning/admin.py:151
- subscription_plan_link() dereferences step_record.output_object without checking that the step record (or its output) exists. If the trial subscription plan step hasn't run (or failed), the admin page will raise when rendering this readonly field.
step_record = obj.get_create_trial_subscription_plan_step()
plan_uuid = step_record.output_object.uuid
url = f'{settings.LICENSE_MANAGER_URL}/admin/subscriptions/subscriptionplan/{plan_uuid}/change/'
return format_html('<a href="{}">{}</a>', url, url)
Comment on lines
136
to
+139
| step_record = obj.get_create_customer_step() | ||
| customer_uuid = step_record.output_object.uuid | ||
| url = f'{settings.LMS_URL}/admin/enterprise/enterprisecustomer/{customer_uuid}/change/' | ||
| return mark_safe(f'<a href="{url}">{url}</a>') | ||
| return format_html('<a href="{}">{}</a>', url, url) |
Comment on lines
145
to
147
| """ | ||
| Link to the EnterpriseCustomer Admin record in the LMS service. | ||
| """ |
| # -r requirements/test.txt | ||
| # edx-lint | ||
| pylint-django==2.7.0 | ||
| pylint-django==2.8.0 |
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
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.
Updated fix (from #245) for upgrade to pylint-django 2.8.0 causing quality CI failures due to
mark-safe-interpolation(W5151) rule.