fix: endpoints not removable from finding via Edit Finding form#14460
Open
valentijnscholten wants to merge 3 commits intoDefectDojo:bugfixfrom
Open
fix: endpoints not removable from finding via Edit Finding form#14460valentijnscholten wants to merge 3 commits intoDefectDojo:bugfixfrom
valentijnscholten wants to merge 3 commits intoDefectDojo:bugfixfrom
Conversation
Two bugs introduced in the locations refactor (PR DefectDojo#14198): 1. FindingForm did not set `endpoints.initial` for the non-V3 path, because `endpoints` is in Meta.exclude and Django won't auto-populate excluded fields from the model instance. Added explicit initial assignment so existing endpoints are pre-selected in the edit form. 2. add_locations() merged the submitted endpoint selection with the pre-existing set (| finding.endpoints.all()), making it impossible to remove an endpoint by deselecting it. Removed the union with the existing set so the submitted selection replaces the current one. Adds unit tests covering add, keep, remove, and switch scenarios for both add_locations() directly and the EditFinding view.
3 tasks
The previous fix applied replace semantics unconditionally, breaking the add_finding_from_template flow which copies template endpoints onto the finding before calling add_locations(). Replace is now opt-in via a keyword argument (replace=False by default). Only EditFinding passes replace=True; all other callers (add from template, promote to finding, add finding) keep the original union behaviour so that pre-populated endpoints are not wiped by an empty form submission. Unit tests updated to pass replace=True when testing the remove/replace scenarios that are specific to the EditFinding path.
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
2 participants
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
FindingFormnot pre-selecting existing endpoints in the Edit Finding form:endpointsis listed inMeta.exclude, so Django does not auto-populate it from the model instance — added an explicitself.fields["endpoints"].initial = self.instance.endpoints.all()for the non-V3 path (same pattern already used for the V3/Locations path)add_locations()merging the submitted endpoint selection with the pre-existing set (| finding.endpoints.all()), which made it impossible to remove an endpoint by deselecting it — the submitted queryset now replaces the existing set rather than being unioned with itadd_locations()directly and theEditFindingviewFixes #14454