Conversation
- Added support for Altcha CAPTCHA in the form builder. - Updated README with configuration instructions for integrating django-altcha. - Introduced new CSS file for Altcha styling. - Modified templates to include Altcha CSS conditionally. - Updated settings to allow customization of Altcha field options.
Reviewer's GuideAdds Altcha captcha as an additional supported captcha backend, wires it into field creation and template rendering, and introduces minimal CSS styling and configuration options for the Altcha field. Sequence diagram for Altcha captcha rendering in formsequenceDiagram
actor User
participant Browser
participant DjangoView
participant Template_ajax_form
participant TemplateTagModule as form_builder_tags
participant RecaptchaModule as recaptcha
participant FormInstance as form
participant AltchaField
participant AltchaWidget
User->>Browser: Request form page
Browser->>DjangoView: HTTP GET /form
DjangoView->>RecaptchaModule: get_recaptcha_field(instance)
RecaptchaModule->>AltchaField: create_with_ALTCHA_FIELD_OPTIONS
AltchaField-->>RecaptchaModule: AltchaField_instance
RecaptchaModule-->>DjangoView: form_with_captcha_field
DjangoView-->>Browser: Rendered HTML with form
User->>Browser: Submit form via AJAX
Browser->>Template_ajax_form: Render response template
Template_ajax_form->>FormInstance: access fields.captcha_field
Template_ajax_form-->>Browser: Include altcha.css when captcha_field
Template_ajax_form->>TemplateTagModule: render_recaptcha_widget(form)
TemplateTagModule->>RecaptchaModule: check installed and field_name
TemplateTagModule->>FormInstance: captcha_field = fields[field_name]
FormInstance-->>TemplateTagModule: AltchaField_instance
TemplateTagModule->>AltchaField: get widget (AltchaWidget)
TemplateTagModule->>AltchaWidget: render(name=field_name, value="", attrs={})
AltchaWidget-->>TemplateTagModule: HTML widget_markup
TemplateTagModule-->>Template_ajax_form: widget_markup
Template_ajax_form-->>Browser: Response with Altcha widget HTML
Class diagram for Altcha captcha integrationclassDiagram
class RecaptchaModule {
+bool installed
+str field_name
+dict CAPTCHA_FIELDS
+dict CAPTCHA_WIDGETS
+tuple CAPTCHA_CHOICES
+get_recaptcha_field(instance)
}
class AltchaConfig {
+dict ALTCHA_FIELD_OPTIONS
}
class FormInstance {
+dict fields
+Config config
}
class Config {
+str captcha_widget
}
class AltchaField {
}
class AltchaWidget {
}
class ReCaptchaField {
}
class ReCaptchaWidget {
}
class HCaptchaField {
}
class HCaptchaWidget {
}
class TemplateTagModule {
+render_widget(form, form_field, kwargs)
+render_recaptcha_widget(form)
}
RecaptchaModule --> AltchaField : creates_when_captcha_widget_altcha
RecaptchaModule --> ReCaptchaField : creates_for_recaptcha
RecaptchaModule --> HCaptchaField : creates_for_hcaptcha
ReCaptchaField --> ReCaptchaWidget : uses_widget
HCaptchaField --> HCaptchaWidget : uses_widget
AltchaField --> AltchaWidget : uses_widget
AltchaConfig <.. RecaptchaModule : reads_ALTCHA_FIELD_OPTIONS
TemplateTagModule --> RecaptchaModule : reads_installed_and_field_name
TemplateTagModule --> FormInstance : accesses_fields
TemplateTagModule --> AltchaWidget : calls_render_for_altcha
FormInstance --> Config : has_config
FormInstance --> AltchaField : may_contain
FormInstance --> ReCaptchaField : may_contain
FormInstance --> HCaptchaField : may_contain
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The assignment to
CAPTCHA_WIDGETS["hcaptcha"] = hCaptchaWidgetwas removed, butget_recaptcha_fieldstill relies onCAPTCHA_WIDGETS[instance.captcha_widget], which will now raise a KeyError for hCaptcha; restore or rework this mapping. - In
render_recaptcha_widget, checkingcaptcha_field.widget.__class__.__name__ == "AltchaWidget"is brittle; consider usingisinstanceor another more robust way to detect the Altcha widget. - The Altcha CSS in
ajax_form.htmlis included whenever acaptcha_fieldexists, even for non-Altcha captchas; you may want to gate this oninstance.config.captcha_widget == "altcha"(or similar) to avoid loading unnecessary styles.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The assignment to `CAPTCHA_WIDGETS["hcaptcha"] = hCaptchaWidget` was removed, but `get_recaptcha_field` still relies on `CAPTCHA_WIDGETS[instance.captcha_widget]`, which will now raise a KeyError for hCaptcha; restore or rework this mapping.
- In `render_recaptcha_widget`, checking `captcha_field.widget.__class__.__name__ == "AltchaWidget"` is brittle; consider using `isinstance` or another more robust way to detect the Altcha widget.
- The Altcha CSS in `ajax_form.html` is included whenever a `captcha_field` exists, even for non-Altcha captchas; you may want to gate this on `instance.config.captcha_widget == "altcha"` (or similar) to avoid loading unnecessary styles.
## Individual Comments
### Comment 1
<location path="djangocms_form_builder/recaptcha.py" line_range="36-38" />
<code_context>
from hcaptcha.widgets import hCaptchaWidget # NOQA
CAPTCHA_FIELDS["hcaptcha"] = hCaptchaField
- CAPTCHA_WIDGETS["hcaptcha"] = hCaptchaWidget
CAPTCHA_CHOICES += (("hcaptcha", _("hCaptcha")),)
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Restoring `CAPTCHA_WIDGETS["hcaptcha"]` mapping to avoid runtime errors
This change removes `CAPTCHA_WIDGETS[
</issue_to_address>
### Comment 2
<location path="djangocms_form_builder/templates/djangocms_form_builder/ajax_form.html" line_range="29-30" />
<code_context>
+ {% endaddtoblock %}
+{% endif %}
+
{% if instance.config.captcha_widget %}
{% addtoblock 'js' %}<script src="https://www.google.com/recaptcha/api.js?onload=reCaptchaOnLoadCallback&render=explicit" async defer></script>{% endaddtoblock %}
{% endif %}
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid loading Google reCAPTCHA script when using Altcha
With Altcha enabled, `instance.config.captcha_widget` is still truthy, so this block will always load the Google reCAPTCHA script. Please gate this include on the widget type (e.g. only when the widget is `recaptcha`/`hcaptcha`) to avoid an unnecessary external script request and possible confusion.
</issue_to_address>
### Comment 3
<location path="djangocms_form_builder/templatetags/form_builder_tags.py" line_range="152-157" />
<code_context>
@register.simple_tag(takes_context=False)
def render_recaptcha_widget(form):
if recaptcha.installed:
+ captcha_field = form.fields[recaptcha.field_name]
+ if captcha_field.widget.__class__.__name__ == "AltchaWidget":
+ return form.fields["captcha_field"].widget.render(
+ name=recaptcha.field_name, value="", attrs={}
+ )
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use the bound field’s rendering instead of calling `widget.render` directly
This path bypasses `BoundField` rendering by calling `widget.render` with hard-coded `name`, `value`, and empty `attrs`, so you may lose attributes like `id`, CSS classes, other form-level customizations, and any bound value. Use `form[recaptcha.field_name]` or `render_widget` (or an Altcha-specific helper) so this field is rendered consistently with the rest of the form.
```suggestion
captcha_field = form.fields[recaptcha.field_name]
if captcha_field.widget.__class__.__name__ == "AltchaWidget":
return render_widget(form, recaptcha.field_name)
return render_widget(form, recaptcha.field_name)
```
</issue_to_address>
### Comment 4
<location path="djangocms_form_builder/templatetags/form_builder_tags.py" line_range="153" />
<code_context>
def render_recaptcha_widget(form):
if recaptcha.installed:
+ captcha_field = form.fields[recaptcha.field_name]
+ if captcha_field.widget.__class__.__name__ == "AltchaWidget":
+ return form.fields["captcha_field"].widget.render(
+ name=recaptcha.field_name, value="", attrs={}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Prefer a more robust check than comparing `__class__.__name__` to detect Altcha widgets
Using `captcha_field.widget.__class__.__name__ == "AltchaWidget"` is fragile and can break if the widget is subclassed, renamed, or imported differently. Where possible, compare against the actual Altcha widget class (or a shared base/interface) rather than the class name string.
Suggested implementation:
```python
if recaptcha.installed:
captcha_field = form.fields[recaptcha.field_name]
if isinstance(captcha_field.widget, AltchaWidget):
return captcha_field.widget.render(
name=recaptcha.field_name, value="", attrs={}
)
return render_widget(form, recaptcha.field_name)
```
You will also need to import the concrete `AltchaWidget` class at the top of this file. For example, if the widget lives in `djangocms_form_builder.widgets`, add:
```python
from djangocms_form_builder.widgets import AltchaWidget
```
If `AltchaWidget` is defined elsewhere, adjust the import path accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
djangocms_form_builder/templates/djangocms_form_builder/ajax_form.html
Outdated
Show resolved
Hide resolved
|
@pierreben Interesting PR! Can you address the comments by sourcery? Also, can you give me an idea how altcha works? |
…lder into feat/altcha
|
@fsbraun , Altcha is a GDPR-compliant, Proof-of-Work CAPTCHA solution. It can be used with self-hosted instances of their "Sentinel" system, or with a simple Django view directly included in your Django project, as provided by django-altcha You can have more information on these links : The minimum setup to get it working is to install The For example, you can set a |
|
@pierreben Thanks!
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
==========================================
+ Coverage 81.88% 82.43% +0.55%
==========================================
Files 21 21
Lines 1496 1503 +7
Branches 188 190 +2
==========================================
+ Hits 1225 1239 +14
+ Misses 216 208 -8
- Partials 55 56 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…lder into feat/altcha
|
Hi @fsbraun, |
I've added the altcha captcha support in addition to hcaptcha and recaptcha.
Summary by Sourcery
Add optional Altcha captcha integration alongside existing reCAPTCHA and hCaptcha support in the form builder.
New Features:
Enhancements: