fix: update autocomplete on applied controls creation from suggestions #1599
Conversation
WalkthroughThis pull request updates both backend and frontend logic for creating and processing applied controls. In the backend, the methods in the assessment view sets now capture and serialize the results of control creation, returning detailed data instead of a simple success. On the frontend, the response of the POST handler is modified to return JSON data rather than an empty body. Additionally, the third-party assessment edit flow now includes improved error handling and updates the UI state to handle multiple new controls at once. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant V as ViewSet
participant M as Control Creator
participant S as Serializer
C->>V: POST /create_suggested_applied_controls
V->>M: create_applied_controls_from_suggestions()
M-->>V: List of applied controls
V->>S: Serialize applied controls
S-->>V: JSON data payload
V-->>C: HTTP 200 with applied controls JSON
sequenceDiagram
participant U as User
participant PS as +page.server.ts Handler
participant F as +server.ts (Internal API)
participant UI as +page.svelte (Frontend)
U->>PS: Trigger createSuggestedControls action
PS->>F: Fetch applied controls
F-->>PS: JSON response (or error)
PS->>U: Return { form, newControls } or fail response
UI->>UI: Reactive statement processes form.newControls
UI->>UI: Append new controls to $formStore.applied_controls
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🪛 Ruff (0.8.2)backend/core/views.py4073-4073: (F405) 4211-4211: (F405) ⏰ Context from checks skipped due to timeout of 90000ms (10)
🔇 Additional comments (2)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
frontend/src/routes/(app)/(internal)/requirement-assessments/[id=uuid]/suggestions/applied-controls/+server.ts (1)
23-23: Enhanced response structure with control data.The change improves the API response by returning the parsed JSON data instead of an empty response. This allows clients to access details of the newly created applied controls, facilitating better UI updates and state management.
Consider adding error handling around the
res.json()call in case the response isn't valid JSON:- return new Response(JSON.stringify(await res.json()), { + try { + const responseData = await res.json(); + return new Response(JSON.stringify(responseData), { + headers: { + 'Content-Type': 'application/json' + } + }); + } catch (error) { + console.error('Error parsing response as JSON:', error); + return new Response(JSON.stringify({ error: 'Failed to parse response' }), { + status: 500, + headers: { + 'Content-Type': 'application/json' + } + }); + }frontend/src/routes/(app)/(third-party)/requirement-assessments/[id=uuid]/edit/+page.server.ts (1)
305-306: Enhanced response handling for created controls.The updated code now processes the JSON response to extract control IDs and returns them as
newControls. This works with the new reactive block in the page component to update the UI with multiple new controls.Consider adding error handling around the JSON parsing and data mapping:
-const newControls = await response.json().then((data) => data.map((e) => e.id)); -return { form, newControls }; +try { + const responseData = await response.json(); + // Validate that responseData is an array + if (!Array.isArray(responseData)) { + console.error('Expected array response but got:', responseData); + return { form, newControls: [] }; + } + const newControls = responseData.map((e) => e?.id).filter(Boolean); + return { form, newControls }; +} catch (error) { + console.error('Error processing response:', error); + return { form, newControls: [] }; +}This provides better error resilience if the API response format changes unexpectedly.
backend/core/views.py (2)
4067-4075: Improved return value handling for multiple control creation.The changes correctly enhance the endpoint to return the newly created applied controls rather than a simple success response. This allows the frontend to immediately access and display the created controls without requiring an additional API call.
A few observations:
- The code assumes
create_applied_controls_from_suggestions()always returns a valid object that can be appended to a list.- There's no error handling if the control creation fails.
Consider adding error handling to make the endpoint more robust:
def create_suggested_applied_controls(request, pk): compliance_assessment = ComplianceAssessment.objects.get(id=pk) if not RoleAssignment.is_access_allowed( user=request.user, perm=Permission.objects.get(codename="add_appliedcontrol"), folder=compliance_assessment.folder, ): return Response(status=status.HTTP_403_FORBIDDEN) requirement_assessments = compliance_assessment.requirement_assessments.all() controls = [] for requirement_assessment in requirement_assessments: - controls.append( - requirement_assessment.create_applied_controls_from_suggestions() - ) + try: + created_controls = requirement_assessment.create_applied_controls_from_suggestions() + if created_controls: + controls.append(created_controls) + except Exception as e: + logger.error(f"Error creating controls for requirement {requirement_assessment.id}: {str(e)}") return Response( AppliedControlReadSerializer(controls, many=True).data, status=status.HTTP_200_OK, )🧰 Tools
🪛 Ruff (0.8.2)
4073-4073:
AppliedControlReadSerializermay be undefined, or defined from star imports(F405)
4209-4213: Improved return value handling for individual control creation.This change correctly enhances the endpoint to return the newly created applied controls rather than a simple success response. This allows frontend code to immediately use the new control data without making additional API calls.
Consider adding error handling and validation of results:
def create_suggested_applied_controls(request, pk): requirement_assessment = RequirementAssessment.objects.get(id=pk) if not RoleAssignment.is_access_allowed( user=request.user, perm=Permission.objects.get(codename="add_appliedcontrol"), folder=requirement_assessment.folder, ): return Response(status=status.HTTP_403_FORBIDDEN) - controls = requirement_assessment.create_applied_controls_from_suggestions() - return Response( - AppliedControlReadSerializer(controls, many=True).data, - status=status.HTTP_200_OK, - ) + try: + controls = requirement_assessment.create_applied_controls_from_suggestions() + return Response( + AppliedControlReadSerializer(controls, many=True).data, + status=status.HTTP_200_OK, + ) + except Exception as e: + logger.error(f"Error creating controls: {str(e)}") + return Response( + {"error": "Failed to create suggested controls"}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR + )🧰 Tools
🪛 Ruff (0.8.2)
4211-4211:
AppliedControlReadSerializermay be undefined, or defined from star imports(F405)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/core/views.py(2 hunks)frontend/src/routes/(app)/(internal)/requirement-assessments/[id=uuid]/suggestions/applied-controls/+server.ts(1 hunks)frontend/src/routes/(app)/(third-party)/requirement-assessments/[id=uuid]/edit/+page.server.ts(1 hunks)frontend/src/routes/(app)/(third-party)/requirement-assessments/[id=uuid]/edit/+page.svelte(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
backend/core/views.py
4073-4073: AppliedControlReadSerializer may be undefined, or defined from star imports
(F405)
4211-4211: AppliedControlReadSerializer may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: ruff (3.12)
- GitHub Check: test (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: build (3.12)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
frontend/src/routes/(app)/(third-party)/requirement-assessments/[id=uuid]/edit/+page.svelte (1)
209-214:✅ Verification successful
Added support for handling multiple new controls.
Good enhancement that handles an array of control IDs returned from the
createSuggestedControlsaction. This allows multiple controls to be added to the UI state at once when creating controls from suggestions.Let's check if there are other instances in the codebase that might need similar handling of multiple items:
🏁 Script executed:
#!/bin/bash # Search for patterns where a single item is added to an array in form stores rg "formStore.*push\(.*\.new" --type svelteLength of output: 76
I'll now re-run the search using an updated command to target Svelte files by their extension:
This command should help us verify if there are other locations in the codebase using a similar pattern for adding controls. Please run this script and share the output so we can confirm whether additional changes are necessary or if the current handling can be considered complete.
🏁 Script executed:
#!/bin/bash # Search for instances where formStore.applied_controls is updated with "push" in Svelte files rg "formStore\.applied_controls\.push\(" -g "*.svelte"Length of output: 461
Enhancement Verified: Multiple Controls Handling Implemented Correctly
The update correctly iterates over
form.newControlsto add each control to$formStore.applied_controls. Our search shows that aside from the unchanged single control update (form.newControl), no other parts of the codebase require similar adjustments. This confirms that the implementation adequately supports adding multiple controls without unintended side effects.frontend/src/routes/(app)/(third-party)/requirement-assessments/[id=uuid]/edit/+page.server.ts (1)
303-303: Added proper error handling for failed API requests.Good improvement to return a proper fail response with the form data when the API request to create suggested controls fails. This ensures the UI can handle the error state correctly.
nas-tabchiche
force-pushed
the
CA-848-create-controls-from-suggestions-in-requirement-assessments-does-not-update-the-auto-complete
branch
from
19a5716 to
316e8af
Compare
Summary by CodeRabbit