-
Notifications
You must be signed in to change notification settings - Fork 594
feat(lib): add dry-run checkbox #3327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe changes introduce a "dry run" feature for library uploads. Twenty-two localization files add three new message keys for the feature (label, help text, success confirmation). The upload form UI now includes a checkbox for dry-run mode, the validation schema supports the new field with a default value, and the server handler routes dry-run requests to a separate endpoint while adjusting success flash messages accordingly. Changes
Sequence DiagramsequenceDiagram
participant User
participant Form as Upload Form
participant Server as Server Handler
participant Endpoint as Backend API
User->>Form: Check "Dry run" checkbox
User->>Form: Submit upload with file
Form->>Form: Validate (schema + dry_run flag)
Form->>Server: POST with form data
Server->>Server: Extract dry_run from form
alt dry_run = true
Server->>Endpoint: POST /endpoint?dry_run=true
Endpoint->>Endpoint: Validate file (no save)
Endpoint-->>Server: Success (validation only)
Server->>Server: Set flash type: info<br/>message: libraryDryRunSuccess
else dry_run = false
Server->>Endpoint: POST /endpoint
Endpoint->>Endpoint: Validate & save file
Endpoint-->>Server: Success (saved)
Server->>Server: Set flash type: success<br/>message: libraryUploadSuccess
end
Server-->>User: Redirect with flash message
User->>User: View result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/lib/utils/schemas.ts`:
- Around line 104-107: The LibraryUploadSchema currently sets dry_run to
default(true) which overrides form initialization and causes unchecked checkbox
submissions to be treated as dry-run; update the schema definition
(LibraryUploadSchema and its dry_run z.coerce.boolean().optional().default(...))
to use default(false) so missing/omitted dry_run values align with the form's
initial state and expected behavior.
| export const LibraryUploadSchema = z.object({ | ||
| file: z.instanceof(File).optional() | ||
| file: z.instanceof(File).optional(), | ||
| dry_run: z.coerce.boolean().optional().default(true) | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for dry_run usage in the upload form and server handler
rg -n "dry_run" -C 4 frontend/src/lib/components/Modals/UploadLibraryModal.svelte frontend/src/routes/\(app\)/\(internal\)/libraries/+page.server.tsRepository: intuitem/ciso-assistant-community
Length of output: 2892
Defaulting dry_run to true forces dry‑run mode when checkbox is unchecked.
The form initializes with dry_run: false (line 47, +page.server.ts), but the schema defaults to true for missing values. When an unchecked checkbox is omitted from form submission, the schema applies the .default(true), causing the server to treat it as dry-run (line 70). Change the default to false to align with form initialization and expected behavior.
Suggested fix
export const LibraryUploadSchema = z.object({
file: z.instanceof(File).optional(),
- dry_run: z.coerce.boolean().optional().default(true)
+ dry_run: z.coerce.boolean().optional().default(false)
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const LibraryUploadSchema = z.object({ | |
| file: z.instanceof(File).optional() | |
| file: z.instanceof(File).optional(), | |
| dry_run: z.coerce.boolean().optional().default(true) | |
| }); | |
| export const LibraryUploadSchema = z.object({ | |
| file: z.instanceof(File).optional(), | |
| dry_run: z.coerce.boolean().optional().default(false) | |
| }); |
🤖 Prompt for AI Agents
In `@frontend/src/lib/utils/schemas.ts` around lines 104 - 107, The
LibraryUploadSchema currently sets dry_run to default(true) which overrides form
initialization and causes unchecked checkbox submissions to be treated as
dry-run; update the schema definition (LibraryUploadSchema and its dry_run
z.coerce.boolean().optional().default(...)) to use default(false) so
missing/omitted dry_run values align with the form's initial state and expected
behavior.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.