feat(cms): TACC Site Card plugin#1191
Merged
Merged
Conversation
Editors get a Style-based Card plugin that always outputs c-card with stackable skin and image-layout modifiers, without new migrations.
Use c-card class names, plain default skin, and tag-type form UX; nest test pages under /test/ with a dedicated card layout grid command.
Use CARD_SKIN_CLASS_NAME_CHOICES (Base, Plain, Standard) instead of raw class strings in the admin.
Drop legacy card/card-- mapping; default new plugins to plain when class_name is not a valid skin choice.
Keep constants.py for choices only; class and attribute helpers live in taccsite_card/utils.py.
Centralize tag choices and CARD_STYLE_INHERITED_TAG; default new plugins when tag is unknown or still Style’s inherited value.
PR Summary by QodoAdd TACC Site Card CMS plugin (c-card) with QA test page command
AI Description
Diagram
High-Level Assessment
Files changed (18)
|
Code Review by Qodo
1. Attributes string autoescaped
|
Comment on lines
+31
to
+40
| def attributes_str_without_class(instance): | ||
| """Render attributes except class (class is composed on the wrapper element).""" | ||
| attributes = dict(instance.attributes or {}) | ||
| attributes.pop('class', None) | ||
| if not attributes: | ||
| return '' | ||
| return ' '.join( | ||
| f'{key}="{value}"' | ||
| for key, value in attributes.items() | ||
| ) |
There was a problem hiding this comment.
1. Attributes string autoescaped 🐞 Bug ≡ Correctness
attributes_str_without_class() manually formats HTML attributes into a plain string and
base.html renders it via {{ attributes_str }} under autoescape, causing quotes to be
entity-escaped (e.g. data-x="bar" becomes data-x="bar") and changing the effective
attribute values compared to the rest of the codebase’s instance.attributes_str usage.
Agent Prompt
### Issue description
`taccsite_card.utils.attributes_str_without_class()` builds an HTML attribute fragment as a plain Python string and passes it to the template, where it is autoescaped. This causes the rendered HTML to differ from other plugins that use `instance.attributes_str` (safe, properly escaped), and can change attribute values (notably quotes).
### Issue Context
The project consistently renders attributes via `{{ instance.attributes_str }}` (from `djangocms-attributes-field`), implying the attribute fragment is already safe and correctly escaped. The Card plugin bypasses that mechanism.
### Fix Focus Areas
- taccsite_card/utils.py[31-40]
- taccsite_card/templates/taccsite_card/base.html[1-7]
### Suggested fix
Prefer one of:
1) **Reuse the existing `AttributesField` rendering path**:
- Build the final class string (including `c-card`, skin, layout, additional classes) and assign it into `instance.attributes['class']`.
- Then render `{{ instance.attributes_str }}` in the template (and remove `attributes_str_without_class`).
2) **If you must render a custom attribute fragment**:
- Remove `class` from the attributes dict.
- Use `django.forms.utils.flatatt(attributes)` (or `format_html_join` + `conditional_escape`) to produce a SafeString attribute fragment with correct escaping.
- Ensure the result includes the needed leading space or the template adds it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+2
to
+3
| <{{ instance.tag_type }} | ||
| {% if instance.id_name %}id="{{ instance.id_name }}"{% endif %} |
There was a problem hiding this comment.
2. Unvalidated dynamic tag_type 🐞 Bug ☼ Reliability
taccsite_card/base.html interpolates instance.tag_type into the element name; if an unexpected value reaches the DB (bypassing form choices), it will generate invalid markup and may break page rendering.
Agent Prompt
### Issue description
The template uses `instance.tag_type` directly as the HTML tag name. While the form sets choices, render-time code does not defensively enforce the allowlist, so malformed DB values can produce invalid HTML.
### Issue Context
The app already defines an allowlist of tag types (article/aside/div). This is best enforced at render time as well to prevent corrupted content from taking down page rendering.
### Fix Focus Areas
- taccsite_card/templates/taccsite_card/base.html[2-3]
- taccsite_card/constants.py[27-32]
- taccsite_card/forms.py[62-93]
### Suggested fix
In `TaccsiteCardPlugin.render()` (or `get_render_template()`), normalize/validate `instance.tag_type` against `CARD_TAG_TYPE_CHOICES` (or a set derived from it). If not allowed, fall back to `CARD_TAG_TYPE_DEFAULT` before the template renders.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+57
to
+62
| def get_render_template(self, context, instance, placeholder): | ||
| layout = instance.template or 'default' | ||
| if layout == 'default': | ||
| return 'taccsite_card/base.html' | ||
| return f'taccsite_card/layouts/{layout}.html' | ||
|
|
There was a problem hiding this comment.
3. Layout template not guarded 🐞 Bug ☼ Reliability
TaccsiteCardPlugin.get_render_template() interpolates instance.template into the template path without validating it against the known layout choices, so unexpected DB values can raise TemplateDoesNotExist at render time.
Agent Prompt
### Issue description
`get_render_template()` constructs `taccsite_card/layouts/{layout}.html` from `instance.template` without guarding. If a record has an invalid value (older data, manual DB edits, etc.), rendering will fail.
### Issue Context
You already define valid layouts in `CARD_LAYOUT_TEMPLATES` and set them as form choices, but render-time code trusts the stored value.
### Fix Focus Areas
- taccsite_card/cms_plugins.py[57-62]
- taccsite_card/constants.py[19-25]
- taccsite_card/forms.py[84-90]
### Suggested fix
Build an allowlist from `CARD_LAYOUT_TEMPLATES` (e.g., `{key for key, _ in CARD_LAYOUT_TEMPLATES}`) and:
- if `instance.template` is falsy or not in the allowlist -> return `taccsite_card/base.html`
- else -> return the corresponding layout template path.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+4
to
+11
| STYLE_TAGS = getattr( | ||
| settings, | ||
| 'DJANGOCMS_STYLE_TAGS', | ||
| ('div', 'article', 'section'), | ||
| ) | ||
| # djangocms_style uses the first DJANGOCMS_STYLE_TAGS entry as Style.tag_type default. | ||
| CARD_STYLE_INHERITED_TAG = STYLE_TAGS[0] | ||
|
|
There was a problem hiding this comment.
4. Empty style_tags crash 🐞 Bug ☼ Reliability
CARD_STYLE_INHERITED_TAG = STYLE_TAGS[0] will raise IndexError at import time if DJANGOCMS_STYLE_TAGS is configured as an empty sequence, preventing Django startup.
Agent Prompt
### Issue description
`taccsite_card.constants` indexes `STYLE_TAGS[0]` at import time without ensuring the tuple/list is non-empty.
### Issue Context
If a deployment sets `DJANGOCMS_STYLE_TAGS=()` (or `[]`), importing this app will raise `IndexError`, breaking startup.
### Fix Focus Areas
- taccsite_card/constants.py[4-11]
### Suggested fix
Handle the empty-setting case safely, for example:
- `CARD_STYLE_INHERITED_TAG = STYLE_TAGS[0] if STYLE_TAGS else 'div'` (or `CARD_TAG_TYPE_DEFAULT`)
- optionally, add a system check / warning if the setting is empty.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
1 participant
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.
Overview
Editors deserve a simple Card plugin.
class_name)template)c-card's BEMWhy not use Bootstrap Card plugin?
cardstyles are not yet skinned by Core-Styles.c-cardfor now, and render Bootstrap when we support it.Changes
taccsite_cms.contrib.taccsite_cardINSTALLED_APPScreate_test_page_section_styleTesting
make startdocker exec core_cms python manage.py test taccsite_cms.contrib.taccsite_card --no-inputdocker exec core_cms python manage.py create_test_page_card_style --replacec-cardplus expected skin/layout classes in the DOM.UI
create.test.pages.mov
review.card.plugin.mov