fix(forms): avoid DB access at import time in Import/ReImport forms#15233
Open
Maffooch wants to merge 1 commit into
Open
fix(forms): avoid DB access at import time in Import/ReImport forms#15233Maffooch wants to merge 1 commit into
Maffooch wants to merge 1 commit into
Conversation
ImportScanForm and ReImportScanForm declared their finding-group fields (group_by, create_finding_groups_for_all_findings) in the class body guarded by is_finding_groups_enabled(). That helper reads a DB-backed system setting, so merely importing dojo.forms issued a SELECT. When the module is imported during app initialization (e.g. from an AppConfig ready() hook), Django raises: RuntimeWarning: Accessing the database during app initialization is discouraged. Move the conditional field construction into each form's __init__, so importing the module never touches the database. Behavior is unchanged: instances still gain the fields when finding groups are enabled, the fields keep their original position (appended last), and the empty default choice is still inserted. Evaluating the setting per-instance instead of once at import is also more correct if the setting changes. Co-Authored-By: Claude Opus 4.8 (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
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.
[sc-13664]
What
ImportScanFormandReImportScanFormdeclared their finding-group fields (group_by,create_finding_groups_for_all_findings) in the class body, guarded byis_finding_groups_enabled(). That helper reads a DB-backed system setting, so simply importingdojo.formsissued aSELECT ... FROM system_settings.When the module is imported during app initialization (e.g. from an
AppConfig.ready()hook), Django emits:Fix
Move the conditional field construction from the class body into each form's
__init__(aftersuper().__init__()), so importing the module never touches the database.Behavior is unchanged:
group_by/create_finding_groups_for_all_findingswhen finding groups are enabled.__init__appends them last).if "group_by" in self.fields:) still runs.Evaluating the setting per-instance rather than once at import is also slightly more correct: a change to the setting is picked up without a process restart.
Testing
base_fieldsno longer contains the fields; a freshImportScanForm()/ReImportScanForm()instance still has them (with the("", "---------")default first) when finding groups are enabled.python manage.py test unittests.test_importers_importer.TestDojoImporterBatchPushToJira— passes (grouping path).ruff --config ruff.toml dojo/forms.py— clean.Note
Other spots use the same import-time anti-pattern (the
FindingFilterFilterSetclasses indojo/finding/ui/filters.py, viaget_finding_filterset_fields()indojo/filters.py). Those are intentionally out of scope here —Meta.fieldsis consumed by django_filters at class-definition time, so making them import-safe is a larger refactor that deserves its own PR and filter tests.