Add UI toggle for classic Bootstrap fallback and align form field widths#1
Closed
Maffooch wants to merge 4 commits into
Closed
Add UI toggle for classic Bootstrap fallback and align form field widths#1Maffooch wants to merge 4 commits into
Maffooch wants to merge 4 commits into
Conversation
github-actions
Bot
added
docs
docker
helm
New Migration
unittests
integration_tests
settings_changes
apiv2
ui
parser
labels
Review Summary by QodoComprehensive UI/UX redesign with finding verification, Celery monitoring, and batch deduplication improvements
WalkthroughsDescription• **Documentation site redesign**: Complete overhaul of Hugo documentation layouts including new base template, homepage with dynamic release fetching, improved header with accessibility features and social links, comprehensive footer component, and new image render hook • **Finding verification feature**: Added "Verify Finding" action with new template, keyboard shortcuts (v/c), and bulk edit enhancements with conditional V3_FEATURE_LOCATIONS support • **Celery status monitoring**: New template and interface for monitoring Redis broker, Celery workers, queue inspection, and task management with AJAX-driven real-time updates • **PDF report formatting improvements**: Replaced <pre> tags with <div class="report-field"> across all PDF report templates for better text wrapping and rendering • **Batch deduplication refactoring**: Enhanced deduplication module with match-only preview functions, batch processing support, and handling of unsaved findings/locations • **Model refactoring**: JIRA imports migration from dojo.jira_link.helper to dojo.jira.services, added deduplication field constants, improved location/endpoint handling • **Template security fixes**: Removed unsafe |safe filters from endpoint template, fixed field references and display methods • **Filter corrections**: Changed truncatechars_html to truncatechars across multiple templates for proper HTML handling • **UI enhancements**: Added sorting to endpoints and URLs tables, social provider indicators for groups, improved list page card layouts, base template banner system enhancements • **Security audit tests**: Comprehensive permission test suite covering 11 vulnerability scenarios including IDOR, role-based access, and cross-product controls • **Code cleanup**: Removed unused imports, normalized line endings, minified scripts, cleaned up styling approaches Diagramflowchart LR
A["Documentation<br/>Redesign"] --> B["Hugo Layouts<br/>& Templates"]
C["Finding<br/>Verification"] --> D["New Verify<br/>Template"]
E["Celery<br/>Monitoring"] --> F["Status & Queue<br/>Management"]
G["PDF Reports"] --> H["Text Wrapping<br/>Improvements"]
I["Batch<br/>Deduplication"] --> J["Match-Only<br/>Preview"]
K["Model<br/>Refactoring"] --> L["JIRA Migration<br/>& Optimization"]
M["Template<br/>Security"] --> N["Filter & Field<br/>Fixes"]
B --> O["Enhanced UX"]
D --> O
F --> O
H --> O
J --> O
L --> O
N --> O
File Changes1. docs/layouts/home.html
|
Code Review by Qodo
1. dojo.jira.services accepts request
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+95
to
+166
| def link_finding(request, finding, new_jira_issue_key): | ||
| """ | ||
| Link a finding to an existing Jira issue. | ||
|
|
||
| Wraps: jira_helper.finding_link_jira | ||
| """ | ||
| return _get_helper().finding_link_jira(request, finding, new_jira_issue_key) | ||
|
|
||
|
|
||
| def unlink_finding(request, finding): | ||
| """ | ||
| Unlink a finding from its Jira issue. | ||
|
|
||
| Wraps: jira_helper.finding_unlink_jira | ||
| """ | ||
| return _get_helper().finding_unlink_jira(request, finding) | ||
|
|
||
|
|
||
| def link_finding_group(request, finding_group, new_jira_issue_key): | ||
| """ | ||
| Link a finding group to an existing Jira issue. | ||
|
|
||
| Wraps: jira_helper.finding_group_link_jira | ||
| """ | ||
| return _get_helper().finding_group_link_jira(request, finding_group, new_jira_issue_key) | ||
|
|
||
|
|
||
| def unlink(request, obj): | ||
| """ | ||
| Unlink an object from its Jira issue. | ||
|
|
||
| Wraps: jira_helper.unlink_jira | ||
| """ | ||
| return _get_helper().unlink_jira(request, obj) | ||
|
|
||
|
|
||
| def push_status(obj, jira_instance, jira, issue, *, save=False): | ||
| """ | ||
| Push finding status to Jira. | ||
|
|
||
| Wraps: jira_helper.push_status_to_jira | ||
| """ | ||
| return _get_helper().push_status_to_jira(obj, jira_instance, jira, issue, save=save) | ||
|
|
||
|
|
||
| def update_issue(obj, *args, **kwargs): | ||
| """ | ||
| Update a Jira issue. | ||
|
|
||
| Wraps: jira_helper.update_jira_issue | ||
| """ | ||
| return _get_helper().update_jira_issue(obj, *args, **kwargs) | ||
|
|
||
|
|
||
| def process_project_form(request, instance=None, target=None, product=None, engagement=None): | ||
| """ | ||
| Process a Jira project configuration form. | ||
|
|
||
| Wraps: jira_helper.process_jira_project_form | ||
| """ | ||
| return _get_helper().process_jira_project_form(request, instance=instance, target=target, | ||
| product=product, engagement=engagement) | ||
|
|
||
|
|
||
| def process_epic_form(request, engagement=None): | ||
| """ | ||
| Process a Jira epic form. | ||
|
|
||
| Wraps: jira_helper.process_jira_epic_form | ||
| """ | ||
| return _get_helper().process_jira_epic_form(request, engagement=engagement) | ||
|
|
There was a problem hiding this comment.
1. dojo.jira.services accepts request 📘 Rule violation ⚙ Maintainability
New Jira service-layer functions accept Django request objects (and delegate request-driven form processing), coupling services to HTTP/framework concerns. This violates the requirement that services remain framework-agnostic and only accept domain objects/primitives.
Agent Prompt
## Issue description
`dojo/jira/services.py` exposes service functions that accept Django `request` objects (and request-driven form processing), violating the service-layer framework-agnostic requirement.
## Issue Context
Services should accept only domain objects/primitives and must not depend on HTTP concerns. Request/form handling should live in UI/API layers, which then call services with primitives/models.
## Fix Focus Areas
- dojo/jira/services.py[95-166]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+783
to
+825
| @action(detail=True, methods=["get", "post"], permission_classes=(IsAuthenticated, permissions.UserHasRiskAcceptanceRelatedObjectPermission)) | ||
| def notes(self, request, pk=None): | ||
| risk_acceptance = self.get_object() | ||
| if request.method == "POST": | ||
| new_note = serializers.AddNewNoteOptionSerializer(data=request.data) | ||
| if new_note.is_valid(): | ||
| entry = new_note.validated_data["entry"] | ||
| private = new_note.validated_data.get("private", False) | ||
| note_type = new_note.validated_data.get("note_type", None) | ||
| else: | ||
| return Response(new_note.errors, status=status.HTTP_400_BAD_REQUEST) | ||
|
|
||
| notes = risk_acceptance.notes.filter(note_type=note_type).first() | ||
| if notes and note_type and note_type.is_single: | ||
| return Response("Only one instance of this note_type allowed on a risk acceptance.", status=status.HTTP_400_BAD_REQUEST) | ||
|
|
||
| author = request.user | ||
| note = Notes(entry=entry, author=author, private=private, note_type=note_type) | ||
| note.save() | ||
| history = NoteHistory.objects.create(data=note.entry, time=note.date, current_editor=note.author) | ||
| note.history.add(history) | ||
| risk_acceptance.notes.add(note) | ||
| engagement = risk_acceptance.engagement | ||
| if engagement: | ||
| process_tag_notifications( | ||
| request=request, | ||
| note=note, | ||
| parent_url=request.build_absolute_uri( | ||
| reverse("view_risk_acceptance", args=(engagement.id, risk_acceptance.id)), | ||
| ), | ||
| parent_title=f"Risk Acceptance: {risk_acceptance.name}", | ||
| ) | ||
|
|
||
| serialized_note = serializers.NoteSerializer( | ||
| {"author": author, "entry": entry, "private": private}, | ||
| ) | ||
| return Response(serialized_note.data, status=status.HTTP_201_CREATED) | ||
|
|
||
| notes = risk_acceptance.notes.all() | ||
| serialized_notes = serializers.RiskAcceptanceToNotesSerializer( | ||
| {"risk_acceptance_id": risk_acceptance, "notes": notes}, | ||
| ) | ||
| return Response(serialized_notes.data, status=status.HTTP_200_OK) |
There was a problem hiding this comment.
2. riskacceptanceviewset.notes() creates notes 📘 Rule violation ⚙ Maintainability
The new notes action performs multi-step note creation (saving Notes, creating NoteHistory, and triggering process_tag_notifications) directly inside the API view. This is business workflow logic that should be moved into a services.py layer, with the view acting as a thin delegator.
Agent Prompt
## Issue description
`RiskAcceptanceViewSet.notes()` contains business workflow logic (creating notes/history and dispatching tag notifications) inside the API layer.
## Issue Context
Per the architecture rules, views should handle transport concerns (auth, serializer validation, HTTP responses) and delegate domain workflows to a module service function.
## Fix Focus Areas
- dojo/api_v2/views.py[783-825]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+20
| import logging | ||
| import os | ||
| from pathlib import Path | ||
|
|
||
| from django import forms | ||
| from django.conf import settings | ||
| from django.core import validators | ||
| from django.core.exceptions import ValidationError | ||
| from django.urls import reverse | ||
|
|
||
| from dojo.jira import services as jira_services | ||
| from dojo.models import ( | ||
| JIRA_Instance, | ||
| JIRA_Issue, | ||
| JIRA_Project, | ||
| ) | ||
| from dojo.utils import ( | ||
| get_system_setting, | ||
| is_finding_groups_enabled, | ||
| ) |
There was a problem hiding this comment.
3. dojo/jira/forms.py not under ui/ 📘 Rule violation ⚙ Maintainability
Jira module UI forms were extracted into dojo/jira/forms.py instead of dojo/jira/ui/forms.py as required by the module UI layering rules. This breaks the expected module layout and undermines consistent UI separation across modules.
Agent Prompt
## Issue description
Jira UI forms are located at `dojo/jira/forms.py` rather than the required `dojo/jira/ui/forms.py`.
## Issue Context
The module layout standard requires UI-layer code (forms) under `ui/` to keep module roots focused on domain/services and to match the `dojo/url/` canonical structure.
## Fix Focus Areas
- dojo/jira/forms.py[1-404]
- dojo/forms.py[36-49]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+18
| import json | ||
|
|
||
| from django.urls import reverse | ||
| from rest_framework import serializers | ||
|
|
||
| from dojo.jira import services as jira_services | ||
| from dojo.models import ( | ||
| JIRA_Instance, | ||
| JIRA_Issue, | ||
| JIRA_Project, | ||
| ) | ||
|
|
||
|
|
||
| class JIRAIssueSerializer(serializers.ModelSerializer): | ||
| url = serializers.SerializerMethodField(read_only=True) | ||
|
|
||
| class Meta: | ||
| model = JIRA_Issue |
There was a problem hiding this comment.
4. dojo/jira/api/serializers.py wrong path 📘 Rule violation ⚙ Maintainability
Jira API serializers were added under dojo/jira/api/serializers.py rather than the required dojo/jira/api/serializer.py. This deviates from the standardized extraction layout and can break consistency/expectations for module API serializer placement.
Agent Prompt
## Issue description
Jira API serializers were added as `dojo/jira/api/serializers.py`, but the required standardized location is `dojo/jira/api/serializer.py`.
## Issue Context
The compliance standard expects a consistent filename/location across modules for serializer extraction to simplify discovery and re-export conventions.
## Fix Focus Areas
- dojo/jira/api/serializers.py[1-91]
- dojo/api_v2/serializers.py[1377-1381]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| get_authorized_product_type_members, | ||
| get_authorized_product_types, | ||
| ) | ||
| from dojo.query_utils import build_count_subquery |
There was a problem hiding this comment.
5. Query helper nameerror 🐞 Bug ≡ Correctness
dojo/query_utils.build_count_subquery references Subquery, Count, and IntegerField (and annotates -> Subquery) without importing them, which triggers a NameError when the module is imported. Because build_count_subquery is now imported/used by API views, this can break Django startup and any endpoint module import that touches it.
Agent Prompt
### Issue description
`dojo/query_utils.py` uses `Subquery`, `Count`, and `IntegerField` (and annotates `-> Subquery`) but does not import them, causing a `NameError` as soon as the module is imported.
### Issue Context
Multiple view modules import `build_count_subquery`, so this import-time error can prevent the app from starting.
### Fix Focus Areas
- dojo/query_utils.py[2-14]
### Suggested fix
Add the missing imports at module top, e.g.:
```py
from django.db.models import Count, IntegerField, Subquery
from django.db.models.query import QuerySet
```
(Keeping the current implementation otherwise unchanged.)
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Adds a per-user UI preference (UserContactInfo.ui_use_tailwind) so users can opt into the redesigned Tailwind UI while the legacy Bootstrap templates continue to render via a parallel `templates_classic/` tree (loaded by `dojo/template_loaders.py`). Includes the migration, classic copies of dojo.css/index.js/metrics.js, and a context-processor banner inviting non-opted-in users to enable the new UI ahead of it becoming the default on September 8th in the 2.62.0 release. Also makes form widgets uniform width across the New/Edit pages for Product Type, Product, Engagement, Test, Finding, Risk Acceptance, and Finding Group: removes the legacy `width: 70% !important` boilerplate duplicated across ~38 form templates and adds a single rule in dojo.css that sizes EasyMDE, Select2 (incl. Tagulous tag inputs) and Chosen widgets to fill their `.form-group` column at every viewport. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Maffooch
changed the title
Hoist deferred imports to module scope, replace try/except/pass with contextlib.suppress, and collapse an if/else into a ternary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Tailwind login template uses `button.login-btn` while the classic template uses `button.btn-success`; integration tests now match either. Adds the Work Sans woff2 files (referenced by tailwind-out.css) under dojo/static/dojo/css/files/ and a copy:fonts step in components/package.json so future Tailwind builds keep them in sync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Adds a per-user UI toggle that lets people opt into the redesigned Tailwind UI ahead of it becoming the default, with the original Bootstrap UI preserved as a parallel
templates_classic/tree for fallback. Also fixes a long-standing inconsistency where text inputs, markdown editors, and tag selects on form pages rendered at three different widths.What's in this PR
UI toggle infrastructure
UserContactInfo.ui_use_tailwindboolean preference (migration0265_usercontactinfo_ui_use_tailwind).dojo/template_loaders.pythat chooses betweendojo/templates/(Tailwind) anddojo/templates_classic/(Bootstrap) based on the per-request preference.dojo/templates_classic/tree (snapshot of the legacy templates), plus classic-onlydojo/static/dojo/css/classic/anddojo/static/dojo/js/classic/assets so existing users see no visual change until they opt in.Form field width consistency fix
Across New/Edit pages for Product Type, Product, Engagement, Test, Finding, Risk Acceptance, and Finding Group (plus ~30 other forms that share the same partial), every text input, select, textarea, EasyMDE markdown editor, and Tagulous/Select2 tag picker now renders at the same responsive width.
width: 70% !important<style>boilerplate that was duplicated across 38 form templates and one popup (add_related.html).dojo/static/dojo/css/dojo.cssthat scopes EasyMDE, CodeMirror, the EasyMDE toolbar/statusbar,.chosen-containerand.select2-containertowidth: 100%of their.form-groupcolumn —!importantonly on.select2-containerso it beats Tagulous' inlinestyle="width:70%".Test plan
UserContactInfo) sees the classic Bootstrap UI by default and a banner inviting them to opt in.ui_use_tailwindon the profile flips a session over to the Tailwind templates without a migration or restart.templates_classic/).