feat: add all backend settings to the forms #184
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes changes across multiple files, primarily enhancing form functionalities and updating configuration settings. The Changes
Possibly related PRs
Suggested reviewers
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 (
|
davidemarcoli
force-pushed
the
feature/extend-forms
branch
from
5f86b3e to
87f3f22
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 10
🧹 Outside diff range comments (5)
src/lib/forms/media-server-form.svelte (1)
Line range hint
1-202: Consider enhancing error handling and type safety.
- The error handling in the form submission could be more specific about which media server failed
- Consider adding TypeScript interfaces for the server responses
Example implementation:
interface MediaServerResponse { success: boolean; error?: string; data?: { version: string; serverName: string; }; } async function validateServer(url: string, token: string, type: 'emby' | 'jellyfin'): Promise<MediaServerResponse> { try { // Implementation } catch (error) { return { success: false, error: `Failed to connect to ${type} server: ${error.message}` }; } }src/lib/forms/general-form.svelte (2)
Line range hint
140-224: Add URL validation for proxy fieldsThe proxy URL fields for Real-Debrid, AllDebrid, and TorBox should validate the input to ensure they are valid URLs.
+function isValidUrl(url: string): boolean { + try { + new URL(url); + return true; + } catch { + return false; + } +} <TextField {form} name="realdebrid_proxy_url" label="Real-Debrid Proxy URL" {formData} + validate={(value) => { + if (value && !isValidUrl(value)) { + return 'Please enter a valid URL'; + } + }} />Apply similar validation to
alldebrid_proxy_urlandtorbox_proxy_urlfields.
Line range hint
1-24: Add test coverage for form validationConsider adding unit tests to verify:
- Form validation logic
- State management for conditional fields
- Error handling scenarios
Example test structure:
import { render, fireEvent } from '@testing-library/svelte'; import GeneralForm from './general-form.svelte'; describe('GeneralForm', () => { test('validates video extensions format', async () => { const { getByLabelText } = render(GeneralForm); const input = getByLabelText('Add Video Extension'); await fireEvent.input(input, { target: { value: 'mp4' } }); expect(screen.getByText('Extension must start with "."')).toBeInTheDocument(); }); });src/lib/forms/helpers.ts (2)
Line range hint
7-15: Missingvideo_extensionsandprefer_speed_over_qualityingeneralSettingsToGetThe properties
video_extensionsandprefer_speed_over_qualityare defined in the schema but are not included ingeneralSettingsToGet(lines 7-15). This omission may result in these settings not being retrieved correctly.Consider adding these properties to the
generalSettingsToGetarray to ensure they are fetched appropriately.Apply this diff to include the missing properties:
export const generalSettingsToGet: string[] = [ 'debug', 'debug_database', 'log', 'force_refresh', 'map_metadata', 'tracemalloc', + 'video_extensions', + 'prefer_speed_over_quality', 'symlink', 'downloaders', 'database', 'indexer', 'notifications', 'post_processing' ];
Line range hint
160-160: Correct Property Name forepisode_filesize_mb_maxIn the function
generalSettingsToPass, at line 160, the propertyepisode_filesize_mb_maxis being assigned fromdata.downloaders.episode_filesize, which appears to be incorrect. The corresponding property indata.downloadersshould likely beepisode_filesize_mb_maxto match the schema and maintain consistency.This mismatch can lead to incorrect data being passed or runtime errors.
Apply this diff to correct the property:
- episode_filesize_mb_max: data.downloaders.episode_filesize, + episode_filesize_mb_max: data.downloaders.episode_filesize_mb_max,
🛑 Comments failed to post (10)
src/lib/forms/media-server-form.svelte (2)
194-201: 🛠️ Refactor suggestion
Consider code reuse and adding URL validation.
- The Emby implementation is very similar to Jellyfin, suggesting an opportunity for component reuse
- Like Jellyfin, consider adding URL validation and connection testing
Consider creating a reusable component:
// MediaServerFields.svelte <script lang="ts"> export let serverType: 'emby' | 'jellyfin'; export let form: any; export let formData: any; const urlField = `${serverType}_url`; const tokenField = `${serverType}_token`; const testConnection = async () => { const endpoint = serverType === 'emby' ? '/emby/System/Info/Public' : '/jellyfin/System/Info/Public'; // ... implementation }; </script> <div transition:slide> <TextField {form} name={urlField} label="{serverType} URL" {formData} /> </div> <div transition:slide> <TextField {form} name={tokenField} label="{serverType} Token" {formData} isProtected={true} /> </div> <Button on:click={testConnection}>Test Connection</Button>Then use it like:
-{#if $formData.emby_enabled} - <div transition:slide> - <TextField {form} name="emby_url" label="Emby URL" {formData} /> - </div> - <div transition:slide> - <TextField {form} name="emby_token" label="Emby Token" {formData} isProtected={true} /> - </div> -{/if} +{#if $formData.emby_enabled} + <MediaServerFields serverType="emby" {form} {formData} /> +{/if}
179-192: 🧹 Nitpick (assertive)
Consider adding URL validation and API connectivity check.
While the implementation is solid, consider these enhancements:
- Add URL format validation for Jellyfin server URLs (typically ends with /jellyfin)
- Add a "Test Connection" button to verify the URL and token are valid
Example implementation:
<TextField {form} name="jellyfin_url" label="Jellyfin URL" {formData} /> +<Button + type="button" + variant="outline" + size="sm" + on:click={async () => { + try { + const response = await fetch(`${$formData.jellyfin_url}/System/Info/Public`); + if (response.ok) { + toast.success('Successfully connected to Jellyfin server'); + } else { + toast.error('Failed to connect to Jellyfin server'); + } + } catch (error) { + toast.error('Failed to connect to Jellyfin server'); + } + }} +> + Test Connection +</Button>Committable suggestion skipped: line range outside the PR's diff.
src/lib/forms/scrapers-form.svelte (2)
74-89: 🧹 Nitpick (assertive)
LGTM! Consider adding min/max validation for bucket_limit.
The new fields are well-structured and properly documented. The enable_aliases checkbox provides a clear example which improves usability.
Consider adding min/max validation for the bucket_limit field to prevent unreasonable values:
<NumberField {form} name="bucket_limit" {formData} stepValue={1} + min={1} + max={100} fieldDescription="Limit the number of torrents that get taken per resolution bucket in each run" />📝 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.<CheckboxField {form} name="enable_aliases" label="Enable aliases" {formData} fieldDescription="Enable title aliases (e.g. Harry Potter and the Philosopher's Stone -> Harry Potter und der Stein der Weisen)" /> <NumberField {form} name="bucket_limit" {formData} stepValue={1} min={1} max={100} fieldDescription="Limit the number of torrents that get taken per resolution bucket in each run" />
172-182:
⚠️ Potential issueFix typos in field names and enhance field descriptions.
There are several issues to address:
- Field names contain typos: "orioniod" should be "orionoid"
- The video quality field lacks a description of expected format/values
Apply these corrections:
- <CheckboxField {form} name="orioniod_parameters_video3d" {formData} /> + <CheckboxField {form} name="orionoid_parameters_video3d" {formData} /> - <TextField {form} name="orioniod_parameters_videoquality" {formData} isProtected={true} /> + <TextField + {form} + name="orionoid_parameters_videoquality" + {formData} + isProtected={true} + fieldDescription="Comma-separated list of allowed video qualities (e.g. hd1080,hd720)" + />📝 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.<div transition:slide> <CheckboxField {form} name="orionoid_cached_results_only" {formData} /> </div> <div transition:slide> <CheckboxField {form} name="orionoid_parameters_video3d" {formData} /> </div> <div transition:slide> <TextField {form} name="orionoid_parameters_videoquality" {formData} isProtected={true} fieldDescription="Comma-separated list of allowed video qualities (e.g. hd1080,hd720)" /> </div>src/lib/forms/general-form.svelte (2)
53-60: 🧹 Nitpick (assertive)
Consider adding confirmation dialogs for critical settings
These new debug settings can significantly impact the system (database debugging, forced refresh, metadata remapping). Consider adding confirmation dialogs before applying these changes to prevent accidental activation.
Example implementation:
<CheckboxField {form} name="debug_database" {formData} fieldDescription="Requires restart" + on:change={(e) => { + if (e.target.checked) { + const confirmed = confirm('Enabling database debug mode requires a restart and may impact performance. Continue?'); + if (!confirmed) e.preventDefault(); + } + }} />Committable suggestion skipped: line range outside the PR's diff.
82-125: 🛠️ Refactor suggestion
Add validation for video extensions
The current implementation allows any input without validation. Consider adding:
- Extension format validation (should start with '.')
- Duplicate prevention
- Empty value validation
Example implementation:
function addField(name: string) { + if (name === 'video_extensions') { + const newExt = ''; + if ($formData.video_extensions.includes(newExt)) return; + } // @ts-expect-error eslint-disable-next-line $formData[name] = [...$formData[name], '']; } +function validateExtension(value: string): boolean { + return value.startsWith('.') && value.length > 1; +} <Input type="text" spellcheck="false" autocomplete="false" {...attrs} bind:value={$formData.video_extensions[i]} + on:input={(e) => { + const value = e.target.value; + if (value && !validateExtension(value)) { + form.errors.video_extensions = 'Extension must start with "."'; + } else if ($formData.video_extensions.filter(ext => ext === value).length > 1) { + form.errors.video_extensions = 'Extension already exists'; + } + }} />Committable suggestion skipped: line range outside the PR's diff.
src/lib/forms/content-form.svelte (2)
479-480: 🧹 Nitpick (assertive)
Consider adding validation for the period field
The
trakt_most_wanted_periodis implemented as a text field but might benefit from validation to ensure valid period values (e.g., "weekly", "monthly", "yearly").Consider:
- Adding input validation in the schema
- Using a select/dropdown component instead of a text field to restrict input to valid values
477-482:
⚠️ Potential issueFix incorrect conditional rendering
There's a critical bug in the conditional rendering. The "most wanted" fields are being shown based on
trakt_fetch_popularinstead oftrakt_fetch_most_wanted.Apply this diff to fix the conditional:
- {#if $formData.trakt_fetch_popular} + {#if $formData.trakt_fetch_most_wanted} <div transition:slide> <NumberField {form} name="trakt_most_wanted_count" {formData} stepValue={1} /> <TextField {form} name="trakt_most_wanted_period" {formData} stepValue={1} /> </div> {/if}📝 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.{#if $formData.trakt_fetch_most_wanted} <div transition:slide> <NumberField {form} name="trakt_most_wanted_count" {formData} stepValue={1} /> <TextField {form} name="trakt_most_wanted_period" {formData} stepValue={1} /> </div> {/if}src/lib/forms/helpers.ts (2)
79-80:
⚠️ Potential issueInconsistent Access and Assignment of
video_extensionsandprefer_speed_over_qualityThe properties
video_extensionsandprefer_speed_over_qualityare defined at the top level ingeneralSettingsSchema(lines 32-33), but ingeneralSettingsToPass(lines 79-80), they are accessed fromdata.downloaders, and ingeneralSettingsToSet(lines 164-165), they are nested underdownloadersin the configuration object. This inconsistency may lead to data mapping issues.To maintain consistency, consider one of the following approaches:
Option 1: Move the definitions of
video_extensionsandprefer_speed_over_qualityunder adownloadersobject ingeneralSettingsSchema.Option 2: Adjust the access paths in
generalSettingsToPassand nesting ingeneralSettingsToSetto align with the schema structure.Apply the following diff for Option 1 to adjust the schema:
export const generalSettingsSchema = z.object({ + downloaders: z.object({ + video_extensions: z.string().array().optional().default([]), + prefer_speed_over_quality: z.boolean().default(true) + }), debug: z.boolean().default(true), debug_database: z.boolean().default(false), log: z.boolean().default(true), force_refresh: z.boolean().default(false), map_metadata: z.boolean().default(false), tracemalloc: z.boolean().default(false), - video_extensions: z.string().array().optional().default([]), - prefer_speed_over_quality: z.boolean().default(true), // ... other fields });Alternatively, for Option 2, adjust
generalSettingsToPassandgeneralSettingsToSetto access these properties at the top level:In
generalSettingsToPass(lines 79-80):- video_extensions: data.downloaders.video_extensions, - prefer_speed_over_quality: data.downloaders.prefer_speed_over_quality, + video_extensions: data.video_extensions, + prefer_speed_over_quality: data.prefer_speed_over_quality,In
generalSettingsToSet(lines 164-165):- video_extensions: form.data.video_extensions, - prefer_speed_over_quality: form.data.prefer_speed_over_quality, + // Move these properties outside the 'downloaders' object + // and include them at the top level if necessaryEnsure that the structure is consistent across the schema, data access, and configuration setting functions.
Also applies to: 32-33, 164-165
327-329:
⚠️ Potential issueCorrect Typographical Error:
orioniodvsorionoidThere is an inconsistency in the spelling of
orionoidin the property names. In several instances, it is misspelled asorioniod(with transposed letters), specifically in:
orioniod_parameters_video3d(lines 327, 383, 444)orioniod_parameters_videoquality(lines 328, 384, 445)This inconsistency can lead to errors in data mapping and access.
Please correct the spelling to
orionoidto match the intended property names.Apply this diff to fix the typos:
In
scrapersSettingsSchema:orionoid_enabled: z.boolean().default(false), orionoid_api_key: z.string().optional().default(''), orionoid_timeout: z.coerce.number().gte(0).int().optional().default(10), orionoid_ratelimit: z.boolean().default(true), - orioniod_parameters_video3d: z.boolean().default(false), - orioniod_parameters_videoquality: z.string().optional().default('sd_hd8k'), + orionoid_parameters_video3d: z.boolean().default(false), + orionoid_parameters_videoquality: z.string().optional().default('sd_hd8k'), orionoid_parameters_limitcount: z.coerce.number().gte(0).int().optional().default(5),In
scrapersSettingsToPass(lines 383-385):orionoid_cached_results_only: data.scraping.orionoid.cached_results_only, - orioniod_parameters_video3d: data.scraping.orionoid.parameters.video3d, - orioniod_parameters_videoquality: data.scraping.orionoid.parameters.videoquality, + orionoid_parameters_video3d: data.scraping.orionoid.parameters.video3d, + orionoid_parameters_videoquality: data.scraping.orionoid.parameters.videoquality, orionoid_parameters_limitcount: data.scraping.orionoid.parameters.limitcount,In
scrapersSettingsToSet(lines 444-446):parameters: { - video3d: form.data.orioniod_parameters_video3d, - videoquality: form.data.orioniod_parameters_videoquality, + video3d: form.data.orionoid_parameters_video3d, + videoquality: form.data.orionoid_parameters_videoquality, limitcount: form.data.orionoid_parameters_limitcount },Ensure that all instances of the property are updated to maintain consistency.
Also applies to: 383-385, 444-446
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range comments (1)
src/lib/forms/media-server-form.svelte (1)
Line range hint
1-220: Consider standardizing media server authentication approaches.The implementation shows an architectural inconsistency in how authentication is handled across different media servers:
- Plex has an OAuth-like flow with PIN-based authentication
- Jellyfin and Emby use direct token input
Consider implementing similar automated authentication flows for Jellyfin and Emby to provide a consistent user experience across all media servers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (7)
.gitignore(1 hunks)src/lib/forms/content-form.svelte(1 hunks)src/lib/forms/general-form.svelte(4 hunks)src/lib/forms/helpers.ts(19 hunks)src/lib/forms/media-server-form.svelte(2 hunks)src/lib/forms/scrapers-form.svelte(2 hunks)src/routes/browse/+page.svelte(0 hunks)
💤 Files with no reviewable changes (1)
- src/routes/browse/+page.svelte
🔇 Additional comments (5)
.gitignore (1)
14-14: LGTM! Appropriate additions for lock and log files.
The additions of bun.lockb and openapi*.log are correct as:
- Lock file from Bun should be ignored since the project uses npm (evidenced by ignored package-lock.json)
- Log files should not be tracked in version control
Also applies to: 18-18
src/lib/forms/media-server-form.svelte (1)
125-132: LGTM! Clean implementation of new media server options.
The new checkbox fields for Jellyfin and Emby follow the existing pattern and maintain consistency with the Plex implementation.
src/lib/forms/scrapers-form.svelte (1)
74-89: LGTM! New global settings are well-implemented.
The new fields for aliases and bucket limits are well-structured with clear descriptions. The implementation follows the established patterns in the form.
Let's verify the schema definitions for these new fields:
✅ Verification successful
Schema definitions for new fields are properly implemented
The verification confirms that both fields are correctly defined across the codebase:
- Type definitions in
types.gen.ts - JSON schema in
schemas.gen.ts - Zod validation schema in
forms/helpers.tswith appropriate constraints:enable_aliases: boolean with defaultfalsebucket_limit: number between 0-20 with default5
- Form data mapping logic in
forms/helpers.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify schema definitions for new fields
# Expected: Find schema definitions for enable_aliases and bucket_limit
rg -A 2 "enable_aliases|bucket_limit" --type ts
Length of output: 1515
src/lib/forms/content-form.svelte (1)
472-482: LGTM! Clean implementation of the most watched feature
The implementation follows the established patterns:
- Consistent conditional rendering
- Proper field grouping
- Matches the UI/UX of other similar features
src/lib/forms/helpers.ts (1)
244-250: Ensure secure handling of media server credentials
The added fields for jellyfin_token, emby_token, and other media server tokens (lines 244-250, 260-266, 280-289) involve sensitive information. It's crucial to handle these tokens securely to prevent unauthorized access.
[security_issue]
Recommendations:
- Avoid Exposing Tokens in Client-Side Code: Ensure that these tokens are not inadvertently sent to the client or exposed in any client-side scripts.
- Secure Storage: Consider storing tokens securely on the server side, using environment variables or a secure vault, instead of in configuration files that may be checked into version control.
- Validation and Sanitization: Implement proper validation and sanitization when handling these tokens to prevent injection attacks.
- Access Controls: Apply strict access controls to functions and endpoints that utilize these tokens.
Would you like assistance in enhancing the security measures for handling these credentials?
Also applies to: 260-266, 280-289
| server-config.json |
There was a problem hiding this comment.
💡 Codebase verification
Configuration setup is well documented, but example files are missing
The project has clear documentation in README.md about environment variables and configuration through docker-compose.yml, but it would benefit from example configuration files. Consider:
- Adding an example config directory (e.g.,
config.example/) with template files - Including a
server-config.example.jsonto demonstrate the expected format
🔗 Analysis chain
Verify availability of example configuration files.
Ignoring both /config and server-config.json might make it difficult for new developers to set up the project. Please ensure that:
- Example/template configuration files are provided
- Setup instructions are documented in README
Let's verify the existence of example configurations:
Let me gather more information about configuration-related files and documentation.
Let me check the complete environment variables documentation in the README to ensure proper setup instructions are available.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for example configuration files and documentation
# Check for example config files
echo "Checking for example configuration files..."
fd -t f -e json -e yaml -e yml -e example "config|server-config"
# Check for setup documentation in README
echo "Checking README for configuration setup instructions..."
rg -i "config|configuration|setup" "README.md"
Length of output: 571
Script:
#!/bin/bash
# Check for example/template configuration files with different extensions and naming patterns
echo "Checking for example/template configuration files..."
fd -t f "example|sample|template|default" --exec grep -l "config"
# Check docker-compose.yml for configuration details
echo "\nChecking docker-compose.yml..."
cat docker-compose.yml
# Look for configuration setup in documentation files
echo "\nChecking documentation files..."
fd -t f -e md | xargs rg -i "configuration setup|config setup|environment variables"
Length of output: 1175
Script:
#!/bin/bash
# Get the complete environment variables section from README
echo "Checking environment variables documentation..."
rg -A 10 "### Environment variables" README.md
# Check if there's a sample/example config directory that might be ignored
echo "\nChecking for example config directory..."
fd "example.*config|sample.*config|template.*config" -t d
Length of output: 882
| {#if $formData.jellyfin_enabled} | ||
| <div transition:slide> | ||
| <TextField {form} name="jellyfin_url" label="Jellyfin URL" {formData} /> | ||
| </div> | ||
| <div transition:slide> | ||
| <TextField | ||
| {form} | ||
| name="jellyfin_token" | ||
| label="Jellyfin Token" | ||
| {formData} | ||
| isProtected={true} | ||
| /> | ||
| </div> | ||
| {/if} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
💡 Codebase verification
Consider adding URL validation for Jellyfin server URL.
Looking at the schema definitions, while there is URL validation for other services, the Jellyfin URL field lacks proper URL validation. Consider adding URL validation in the mediaServerSettingsSchema to ensure valid server URLs are provided.
Example locations that need attention:
src/lib/forms/helpers.ts: Add URL validation tojellyfin_urlinmediaServerSettingsSchema
🔗 Analysis chain
Consider adding URL validation and authentication flow.
While the implementation is clean, consider these improvements:
- Add URL validation to ensure valid Jellyfin server URLs.
- Consider implementing an authentication flow similar to Plex for obtaining the Jellyfin token automatically.
Let's check if URL validation exists in the schema:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for URL validation in the schema
rg -A 5 "jellyfin_url.*zod" src/lib/forms/
Length of output: 44
Script:
#!/bin/bash
# Let's search for schema validation related to Jellyfin
rg -l "jellyfin.*schema|schema.*jellyfin" src/
# Let's also check for any URL validation patterns in the codebase
rg -A 3 "url.*zod|zod.*url" src/
Length of output: 83
Script:
#!/bin/bash
# Let's look for any schema or validation related files
fd "schema|validation" src/
# Let's also check the content of the media-server-form file for any validation
cat src/lib/forms/media-server-form.svelte
Length of output: 6435
Script:
#!/bin/bash
# Let's check the schema definition for URL validation
cat src/lib/forms/helpers.ts
# Also check the generated schema file
cat src/lib/client/schemas.gen.ts
Length of output: 108415
src/lib/forms/scrapers-form.svelte
Outdated
| <div transition:slide> | ||
| <CheckboxField {form} name="orionoid_cached_results_only" {formData} /> | ||
| </div> | ||
|
|
||
| <div transition:slide> | ||
| <CheckboxField {form} name="orioniod_parameters_video3d" {formData} /> | ||
| </div> | ||
|
|
||
| <div transition:slide> | ||
| <TextField {form} name="orioniod_parameters_videoquality" {formData} isProtected={true} /> | ||
| </div> | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
Fix typos in Orionoid field names and schema definitions
The schema definitions in src/lib/forms/helpers.ts confirm the typos in field names:
orioniod_parameters_video3d(incorrect)orioniod_parameters_videoquality(incorrect)
These need to be fixed in both the form component and schema definitions to match the correct spelling "orionoid". The review comment was accurate about the typos and missing field descriptions.
Required changes:
- Fix typo in schema:
orioniod_parameters_video3d→orionoid_parameters_video3d - Fix typo in schema:
orioniod_parameters_videoquality→orionoid_parameters_videoquality - Fix corresponding typos in the form component as suggested in the review
- Add field descriptions as suggested in the review
The naming pattern with "parameters_" prefix is actually consistent with the schema structure, as these fields are nested under the parameters object in the final form data structure.
🔗 Analysis chain
Fix typos and inconsistencies in Orionoid field names.
Several issues were found in the new Orionoid fields:
- Typos in field names ('orioniod' instead of 'orionoid')
- Missing field descriptions
- Inconsistent naming pattern with the 'parameters_' prefix
Apply these fixes:
<div transition:slide>
- <CheckboxField {form} name="orioniod_parameters_video3d" {formData} />
+ <CheckboxField
+ {form}
+ name="orionoid_parameters_video3d"
+ {formData}
+ fieldDescription="Enable 3D video support"
+ />
</div>
<div transition:slide>
- <TextField {form} name="orioniod_parameters_videoquality" {formData} isProtected={true} />
+ <TextField
+ {form}
+ name="orionoid_parameters_videoquality"
+ {formData}
+ isProtected={true}
+ fieldDescription="Specify video quality parameters"
+ />
</div>
<div transition:slide>
<CheckboxField
{form}
name="orionoid_cached_results_only"
{formData}
+ fieldDescription="Only show cached results"
/>
</div>Let's verify the schema definitions for these fields:
Also applies to: 187-187
Let me gather more information about the schema definitions for all Orionoid fields.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify schema definitions for Orionoid fields
# Expected: Find schema definitions with correct field names
rg -A 2 "orionoid_parameters" --type ts
Length of output: 754
Script:
#!/bin/bash
# Search for all Orionoid-related schema definitions and their usage
rg "orionoid" --type ts -A 3 src/lib/forms/helpers.ts
Length of output: 1874
src/lib/forms/helpers.ts
Outdated
| video_extensions: z.string().array().optional().default([]), | ||
| prefer_speed_over_quality: z.boolean().default(true), |
There was a problem hiding this comment.
Fix inconsistency in 'video_extensions' and 'prefer_speed_over_quality' mapping
The properties video_extensions and prefer_speed_over_quality are defined at the root level in the generalSettingsSchema (lines 32-33). However, in generalSettingsToPass (lines 79-80) and generalSettingsToSet (lines 164-165), they are accessed and assigned under the downloaders object. This inconsistency can lead to issues in data mapping and form validation.
Proposed Fix:
Update the generalSettingsToPass and generalSettingsToSet functions to access these properties directly from the root level, ensuring consistency with the schema.
In generalSettingsToPass:
79 video_extensions: data.downloaders.video_extensions,
80 prefer_speed_over_quality: data.downloaders.prefer_speed_over_quality,
---
79 video_extensions: data.video_extensions,
80 prefer_speed_over_quality: data.prefer_speed_over_quality,In generalSettingsToSet:
164 value: {
165 video_extensions: form.data.video_extensions,
166 prefer_speed_over_quality: form.data.prefer_speed_over_quality,
---
164 value: {
165 // Remove 'video_extensions' and 'prefer_speed_over_quality' from the 'downloaders' object
166 // They will be set at the root level insteadAdd them as separate entries at the root level:
+ {
+ key: 'video_extensions',
+ value: form.data.video_extensions
+ },
+ {
+ key: 'prefer_speed_over_quality',
+ value: form.data.prefer_speed_over_quality
+ },Alternatively, if these properties are intended to be under downloaders, update the generalSettingsSchema to reflect that structure.
Also applies to: 79-80, 164-165
src/lib/forms/helpers.ts
Outdated
| orioniod_parameters_video3d: z.boolean().default(false), | ||
| orioniod_parameters_videoquality: z.string().optional().default('sd_hd8k'), |
There was a problem hiding this comment.
Correct typo in 'orionoid' parameter names
There is a typo in the parameter names orioniod_parameters_video3d and orioniod_parameters_videoquality in the schema (lines 327-328), scrapersSettingsToPass (lines 383-384), and scrapersSettingsToSet (lines 444-445). The correct spelling should be orionoid_parameters_video3d and orionoid_parameters_videoquality.
Proposed Fix:
In scrapersSettingsSchema:
327 orioniod_parameters_video3d: z.boolean().default(false),
328 orioniod_parameters_videoquality: z.string().optional().default('sd_hd8k'),
---
327 orionoid_parameters_video3d: z.boolean().default(false),
328 orionoid_parameters_videoquality: z.string().optional().default('sd_hd8k'),In scrapersSettingsToPass:
383 orioniod_parameters_video3d: data.scraping.orionoid.parameters.video3d,
384 orioniod_parameters_videoquality: data.scraping.orionoid.parameters.videoquality,
---
383 orionoid_parameters_video3d: data.scraping.orionoid.parameters.video3d,
384 orionoid_parameters_videoquality: data.scraping.orionoid.parameters.videoquality,In scrapersSettingsToSet:
444 video3d: form.data.orioniod_parameters_video3d,
445 videoquality: form.data.orioniod_parameters_videoquality,
---
444 video3d: form.data.orionoid_parameters_video3d,
445 videoquality: form.data.orionoid_parameters_videoquality,Ensure that all references to these parameters throughout the code are corrected accordingly.
Also applies to: 383-384, 444-445
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
src/lib/forms/helpers.ts(19 hunks)src/lib/forms/scrapers-form.svelte(2 hunks)
🔇 Additional comments (15)
src/lib/forms/scrapers-form.svelte (2)
74-89: LGTM! Well-structured new global settings.
The new fields are well-implemented with clear descriptions and appropriate input controls:
enable_aliasescheckbox includes a helpful example in its descriptionbucket_limituses correct step value for integer input
187-187: LGTM! Consistent field renaming.
The field rename to orionoid_parameters_limitcount follows the established pattern of grouping parameters while maintaining the existing functionality.
src/lib/forms/helpers.ts (13)
7-11: Addition of new general settings properties is appropriate
The new properties debug_database, force_refresh, map_metadata, and tracemalloc are correctly added to the generalSettingsToGet array. This enhances the configuration retrieval process as intended.
22-26: New general settings schema entries are well-defined
The schema definitions for debug_database, force_refresh, map_metadata, and tracemalloc are properly declared with default boolean values. This ensures correct validation and default behaviors.
32-33: Fix inconsistency in 'video_extensions' and 'prefer_speed_over_quality' mapping
The properties video_extensions and prefer_speed_over_quality are defined at the root level in the generalSettingsSchema, but in the generalSettingsToPass function, they are accessed under the downloaders object. This inconsistency can lead to issues in data mapping and form validation.
Apply this diff to correct the mapping in generalSettingsToPass:
- video_extensions: data.downloaders.video_extensions,
- prefer_speed_over_quality: data.downloaders.prefer_speed_over_quality,
+ video_extensions: data.video_extensions,
+ prefer_speed_over_quality: data.prefer_speed_over_quality,74-78: Ensure new general settings are correctly mapped in data passing function
The properties debug_database, force_refresh, map_metadata, and tracemalloc are properly mapped in the generalSettingsToPass function, ensuring they are correctly passed from the data object.
164-165: Ensure 'video_extensions' and 'prefer_speed_over_quality' are set at the correct level
In the generalSettingsToSet function, video_extensions and prefer_speed_over_quality are currently assigned under the downloaders object. However, in generalSettingsSchema, these properties are defined at the root level. This inconsistency may cause issues when setting or retrieving the configuration values.
Apply this diff to correct the assignment:
Remove them from the downloaders object and add them as separate entries at the root level.
// Within 'downloaders' object
{
key: 'downloaders',
value: {
- video_extensions: form.data.video_extensions,
- prefer_speed_over_quality: form.data.prefer_speed_over_quality,
// ... other properties
}
},
// Add them as separate entries at the root level
+{
+ key: 'video_extensions',
+ value: form.data.video_extensions
+},
+{
+ key: 'prefer_speed_over_quality',
+ value: form.data.prefer_speed_over_quality
+},244-250: Proper inclusion of Jellyfin and Emby settings
The added properties for jellyfin_enabled, jellyfin_token, jellyfin_url, emby_enabled, emby_token, and emby_url are appropriately defined in the mediaServerSettingsSchema. This extends support for additional media servers.
260-266: Correct mapping of media server settings in data passing function
The mediaServerSettingsToPass function correctly extracts and maps the Jellyfin and Emby settings from the data object, ensuring accurate data handling.
280-289: Accurate setting of media server configurations
The mediaServerSettingsToSet function properly constructs the updaters object with the new Jellyfin and Emby settings, facilitating correct configuration updates.
304-305: Addition of 'enable_aliases' and 'bucket_limit' settings
The properties enable_aliases and bucket_limit are correctly added to the scrapersSettingsSchema with appropriate validations and default values.
366-367: Proper mapping of new scrapers settings
The scrapersSettingsToPass function correctly maps enable_aliases and bucket_limit from the data object, ensuring they are available for use in the application.
526-532: Appropriate addition of new Trakt settings
The properties trakt_popular_count, trakt_fetch_most_watched, trakt_most_watched_count, and trakt_most_watched_period are correctly defined in the contentSettingsSchema, enhancing the Trakt integration capabilities.
565-568: Correct mapping of new Trakt settings in data passing function
The function contentSettingsToPass appropriately extracts and assigns the new Trakt settings from the data object.
612-615: Accurate setting of new Trakt configuration parameters
The contentSettingsToSet function properly includes the new Trakt properties within the content configuration, ensuring they are correctly updated.
| <div transition:slide> | ||
| <CheckboxField {form} name="orionoid_cached_results_only" {formData} /> | ||
| </div> | ||
|
|
||
| <div transition:slide> | ||
| <CheckboxField {form} name="orionoid_parameters_video3d" {formData} /> | ||
| </div> | ||
|
|
||
| <div transition:slide> | ||
| <TextField {form} name="orionoid_parameters_videoquality" {formData} isProtected={true} /> | ||
| </div> | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add field descriptions to new Orionoid fields.
The new fields should include descriptive text to help users understand their purpose:
<div transition:slide>
- <CheckboxField {form} name="orionoid_cached_results_only" {formData} />
+ <CheckboxField
+ {form}
+ name="orionoid_cached_results_only"
+ {formData}
+ fieldDescription="Only fetch results from Orionoid's cache for faster response times"
+ />
</div>
<div transition:slide>
- <CheckboxField {form} name="orionoid_parameters_video3d" {formData} />
+ <CheckboxField
+ {form}
+ name="orionoid_parameters_video3d"
+ {formData}
+ fieldDescription="Include 3D video content in search results"
+ />
</div>
<div transition:slide>
- <TextField {form} name="orionoid_parameters_videoquality" {formData} isProtected={true} />
+ <TextField
+ {form}
+ name="orionoid_parameters_videoquality"
+ {formData}
+ isProtected={true}
+ fieldDescription="Filter results by video quality (e.g., HD, SD, 4K)"
+ />
</div>📝 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.
| <div transition:slide> | |
| <CheckboxField {form} name="orionoid_cached_results_only" {formData} /> | |
| </div> | |
| <div transition:slide> | |
| <CheckboxField {form} name="orionoid_parameters_video3d" {formData} /> | |
| </div> | |
| <div transition:slide> | |
| <TextField {form} name="orionoid_parameters_videoquality" {formData} isProtected={true} /> | |
| </div> | |
| <div transition:slide> | |
| <CheckboxField | |
| {form} | |
| name="orionoid_cached_results_only" | |
| {formData} | |
| fieldDescription="Only fetch results from Orionoid's cache for faster response times" | |
| /> | |
| </div> | |
| <div transition:slide> | |
| <CheckboxField | |
| {form} | |
| name="orionoid_parameters_video3d" | |
| {formData} | |
| fieldDescription="Include 3D video content in search results" | |
| /> | |
| </div> | |
| <div transition:slide> | |
| <TextField | |
| {form} | |
| name="orionoid_parameters_videoquality" | |
| {formData} | |
| isProtected={true} | |
| fieldDescription="Filter results by video quality (e.g., HD, SD, 4K)" | |
| /> | |
| </div> |
davidemarcoli
force-pushed
the
feature/extend-forms
branch
from
b1957f8 to
f3d2b2f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (7)
.gitignore(1 hunks)src/lib/forms/content-form.svelte(1 hunks)src/lib/forms/general-form.svelte(4 hunks)src/lib/forms/helpers.ts(19 hunks)src/lib/forms/media-server-form.svelte(2 hunks)src/lib/forms/scrapers-form.svelte(2 hunks)src/routes/browse/+page.svelte(0 hunks)
💤 Files with no reviewable changes (1)
- src/routes/browse/+page.svelte
🔇 Additional comments (20)
.gitignore (2)
16-17: Ensure example configuration files are provided
While ignoring sensitive configuration files is good practice, new developers might struggle with setup. The previous concern about missing example files remains valid.
Let's verify if example configuration files exist:
#!/bin/bash
# Description: Check for example configuration files and documentation
# Check for example config files
echo "Checking for example configuration files..."
fd -t f "example|template|sample" -e json -e yml -e yaml
# Check documentation
echo "\nChecking setup documentation..."
rg -i "configuration|setup" "README.md"14-14: LGTM: Package manager and logging configurations
Good practices:
- Ignoring
bun.lockbis correct as it's machine-specific - Ignoring OpenAPI logs prevents cluttering the repository
Also applies to: 18-18
src/lib/forms/media-server-form.svelte (3)
125-132: LGTM! Clean implementation of new updater options.
The new checkbox fields for Jellyfin and Emby updaters are well-integrated into the existing group structure and follow consistent naming patterns.
179-192: Consider adding URL validation for Jellyfin server URL.
Looking at the schema definitions, while there is URL validation for other services, the Jellyfin URL field lacks proper URL validation.
179-192: Consider refactoring duplicated media server field logic.
The Jellyfin and Emby sections contain identical patterns. Consider creating a reusable component to reduce duplication.
src/lib/forms/scrapers-form.svelte (3)
74-89: LGTM! Well-structured new global settings.
The new fields are properly implemented with clear descriptions and appropriate types:
enable_aliases: Good example in the description helps users understand the featurebucket_limit: Clear purpose with appropriate step value of 1 for integer input
172-183: Add descriptive text to the new Orionoid fields.
The fields need descriptions to help users understand their purpose:
orionoid_cached_results_only: Missing description about caching behaviororionoid_parameters_video3d: Missing description about 3D video supportorionoid_parameters_videoquality: Missing description about quality parameters
<div transition:slide>
- <CheckboxField {form} name="orionoid_cached_results_only" {formData} />
+ <CheckboxField
+ {form}
+ name="orionoid_cached_results_only"
+ {formData}
+ fieldDescription="Only fetch results from Orionoid's cache for faster response times"
+ />
</div>
<div transition:slide>
- <CheckboxField {form} name="orionoid_parameters_video3d" {formData} />
+ <CheckboxField
+ {form}
+ name="orionoid_parameters_video3d"
+ {formData}
+ fieldDescription="Include 3D video content in search results"
+ />
</div>
<div transition:slide>
- <TextField {form} name="orionoid_parameters_videoquality" {formData} isProtected={true} />
+ <TextField
+ {form}
+ name="orionoid_parameters_videoquality"
+ {formData}
+ isProtected={true}
+ fieldDescription="Filter results by video quality (e.g., HD, SD, 4K)"
+ />
</div>187-187: LGTM! Good field renaming.
The rename of orionoid_limitcount to orionoid_parameters_limitcount maintains consistency with other parameter fields while retaining a clear description.
src/lib/forms/general-form.svelte (4)
55-60: Improve field description clarity
The description for map_metadata contains informal language.
Apply this diff to improve clarity:
- fieldDescription="Requires restart, inits an empty database based on the symlinked files"
+ fieldDescription="Requires restart, initializes an empty database based on the symlinked files"90-90: Fix invalid value for 'autocomplete' attribute
Apply this diff to fix the attribute value:
- autocomplete="false"
+ autocomplete="off"138-141:
Enforce single downloader selection
The implementation still allows multiple downloaders to be enabled simultaneously, which conflicts with the field description.
Consider using radio buttons instead of checkboxes as previously suggested.
146-152: Verify secure handling of API keys
The API keys for all downloaders are marked as protected, but we should verify the secure handling of these sensitive values.
Also applies to: 174-180, 202-208
✅ Verification successful
Based on the search results, I can now provide a comprehensive verification of the API key handling. Let me generate the final response:
API keys are handled securely with appropriate protections in place
The codebase demonstrates secure handling of API keys through multiple security measures:
- All sensitive API key fields are consistently marked with
isProtected={true}attribute, which ensures the values are masked in the UI - API keys are only transmitted with HTTPS (verified through nginx configuration using auth_basic)
- No logging of API keys found in the codebase - the only debug/log statements are for form validation status
- API keys are properly scoped:
- Server-side: Keys are stored in server configuration and transmitted via secure headers
- Client-side: Keys are protected in forms and not exposed to client-side storage or state management
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential API key leaks in logs or client-side code
# Search for API key logging
rg -i "api.?key|debug|log" --type svelte
# Check for API key exposure in client-side state management
rg -i "store.*api.?key" --type svelte --type typescript
Length of output: 162
Script:
#!/bin/bash
# Search for API key handling patterns in all files
rg -i "api.?key|debug|log"
# Search for API key storage patterns
rg -i "store.*api.?key"
# Look for form handling components and their usage of protected fields
ast-grep --pattern 'isProtected={true}'
Length of output: 40104
src/lib/forms/content-form.svelte (2)
479-480: Add validation and documentation for the period field
The trakt_most_watched_period field needs clarification on the expected format.
479-481: 🧹 Nitpick (assertive)
Improve field validation and spacing
- Add min/max validation for
trakt_most_watched_countto prevent unreasonable values - Consider adding spacing between the count and period fields for better visual separation
<div transition:slide>
- <NumberField {form} name="trakt_most_watched_count" {formData} stepValue={1} />
- <TextField {form} name="trakt_most_watched_period" {formData} />
+ <NumberField
+ {form}
+ name="trakt_most_watched_count"
+ {formData}
+ stepValue={1}
+ min={1}
+ max={100}
+ />
+ <div class="mt-2">
+ <TextField
+ {form}
+ name="trakt_most_watched_period"
+ {formData}
+ placeholder="weekly, monthly, or yearly"
+ />
+ </div>
</div>Likely invalid or redundant comment.
src/lib/forms/helpers.ts (6)
383-385: Ensure consistency in accessing 'orionoid' parameters
The orionoid parameters are correctly named in the schema and functions. However, please verify that all references to these parameters throughout the codebase use the correct spelling orionoid to prevent any potential issues.
Also applies to: 444-447
526-532: Confirm default values and enum options for 'trakt' settings
The new properties related to trakt_most_watched have been added with default values and an enum for trakt_most_watched_period. Ensure that these defaults align with the expected behavior and that the enum options ('daily', 'weekly', 'monthly', 'yearly', 'all') cover all necessary periods.
Also applies to: 565-568, 612-615
244-250: Verify media server settings mapping
New media server settings for jellyfin and emby have been added. Ensure that the enabled, token, and url properties are correctly mapped in the mediaServerSettingsToPass and mediaServerSettingsToSet functions, and that these settings are handled appropriately in other parts of the application.
Also applies to: 260-266, 280-289
304-305: Validate new 'enable_aliases' and 'bucket_limit' settings
The properties enable_aliases and bucket_limit have been added to the scrapersSettingsSchema. Confirm that the bucket_limit value is within the specified range (0 to 20) and that these settings function as intended within the application.
Also applies to: 366-368, 424-425
22-22: New general settings added correctly
The new general settings (debug_database, force_refresh, map_metadata, tracemalloc) have been added to the schema and are correctly passed and set in the corresponding functions. Ensure these settings are utilized properly in the application logic.
Also applies to: 24-26, 74-74, 76-78, 131-134, 139-142, 143-146, 147-150
32-33:
Fix inconsistency in 'video_extensions' and 'prefer_speed_over_quality' mapping
The properties video_extensions and prefer_speed_over_quality are defined at the root level in the generalSettingsSchema (lines 32-33). However, in generalSettingsToPass (lines 79-80) and generalSettingsToSet (lines 164-165), they are accessed and assigned under the downloaders object. This inconsistency can lead to issues in data mapping and form validation.
Also applies to: 79-80, 164-165
| {#if $formData.emby_enabled} | ||
| <div transition:slide> | ||
| <TextField {form} name="emby_url" label="Emby URL" {formData} /> | ||
| </div> | ||
| <div transition:slide> | ||
| <TextField {form} name="emby_token" label="Emby Token" {formData} isProtected={true} /> | ||
| </div> | ||
| {/if} |
There was a problem hiding this comment.
💡 Codebase verification
Add URL validation for Emby server URL
The schema in src/lib/forms/helpers.ts shows that the Emby URL field (emby_url) only has basic string validation with an optional empty default value. This should be enhanced with proper URL validation, similar to how other service URLs in the codebase should be validated.
- Add URL validation to
emby_urlinmediaServerSettingsSchema:
emby_url: z.string().url().optional().default('')🔗 Analysis chain
Add URL validation for Emby server URL.
Similar to Jellyfin, ensure proper URL validation is added for the Emby server URL field in the mediaServerSettingsSchema.
Let's verify the current schema validation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for URL validation in the schema for Emby
rg "emby_url.*schema|schema.*emby_url" src/lib/forms/
Length of output: 55
Script:
#!/bin/bash
# Let's first find the schema file
fd "schema" src/lib/forms/
# Then let's look for any validation related to Emby
rg -A 5 "emby" src/lib/forms/
Length of output: 2634
Script:
#!/bin/bash
# Let's check if there's any URL validation for other media servers that we can compare with
rg "url.*z\." src/lib/forms/helpers.ts -A 2
Length of output: 2674
davidemarcoli
force-pushed
the
feature/extend-forms
branch
from
f3d2b2f to
37a32a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (7)
.gitignore(1 hunks)src/lib/forms/content-form.svelte(1 hunks)src/lib/forms/general-form.svelte(3 hunks)src/lib/forms/helpers.ts(17 hunks)src/lib/forms/media-server-form.svelte(2 hunks)src/lib/forms/scrapers-form.svelte(2 hunks)src/routes/browse/+page.svelte(0 hunks)
💤 Files with no reviewable changes (1)
- src/routes/browse/+page.svelte
🔇 Additional comments (9)
src/lib/forms/helpers.ts (2)
32-32: Fix inconsistency in 'video_extensions' mapping
The property video_extensions is defined at the root level in the generalSettingsSchema (line 32). However, in generalSettingsToPass (line 73), it's accessed under the downloaders object. This inconsistency can lead to issues in data mapping and form validation.
Also applies to: 73-74
84-84:
Correct property name for 'episode_filesize_mb_max'
In generalSettingsToPass, the property episode_filesize_mb_max is incorrectly mapped to data.downloaders.episode_filesize (line 84). It should be mapped to data.downloaders.episode_filesize_mb_max to match the schema definition.
Apply this diff to fix the mapping:
- episode_filesize_mb_max: data.downloaders.episode_filesize,
+ episode_filesize_mb_max: data.downloaders.episode_filesize_mb_max,Likely invalid or redundant comment.
.gitignore (1)
16-18: Potential issue with ignoring configuration files
Ignoring /config and server-config.json (lines 16-17) might make it difficult for new developers to set up the project due to missing configuration files. Consider providing example configuration files or a template config directory (e.g., config.example/) to aid in the initial setup process.
src/lib/forms/media-server-form.svelte (2)
181-182: Consider adding URL validation for 'jellyfin_url'
The jellyfin_url field should have URL validation to ensure users enter valid URLs. This can be enforced by updating the schema in mediaServerSettingsSchema.
196-197: Consider adding URL validation for 'emby_url'
The emby_url field should have URL validation to ensure users enter valid URLs. Updating the schema in mediaServerSettingsSchema can enforce this.
src/lib/forms/scrapers-form.svelte (1)
173-183: Add field descriptions to new Orionoid fields
To enhance user understanding, please add fieldDescription to the new Orionoid fields:
orionoid_cached_results_only: Describe what this option does.orionoid_parameters_video3d: Explain the purpose of this setting.orionoid_parameters_videoquality: Provide guidance on acceptable values and the effect of this parameter.
src/lib/forms/general-form.svelte (2)
82-93:
Add validation for video extension format
The input fields for video extensions should validate the format to ensure proper file extensions (e.g., .mp4, .mkv).
Apply this diff to add validation:
<Input
type="text"
spellcheck="false"
autocomplete="false"
+ pattern="^\.[\w\d]+$"
+ title="Extension must start with a dot followed by alphanumeric characters"
{...attrs}
bind:value={$formData.video_extensions[i]}
/>138-141:
Convert checkboxes to radio buttons for exclusive selection
The field description states "Enable only one downloader at a time" but uses checkboxes which allow multiple selections.
Convert the checkbox group to radio buttons to enforce single selection.
src/lib/forms/content-form.svelte (1)
479-480:
Add validation and documentation for most watched period
The trakt_most_watched_period field needs validation and clarification on the expected format.
Add validation and placeholder to indicate valid period values (weekly, monthly, yearly).
| plex_url: z.string().optional().default(''), | ||
| jellyfin_enabled: z.boolean().default(false), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add URL validation for 'plex_url'
In mediaServerSettingsSchema, the field plex_url (line 227) is defined as a string without URL validation. Consider adding .url() to ensure that a valid URL is provided.
Apply this diff to enhance URL validation:
- plex_url: z.string().optional().default(''),
+ plex_url: z.string().url().optional().default(''),📝 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.
| plex_url: z.string().optional().default(''), | |
| jellyfin_enabled: z.boolean().default(false), | |
| plex_url: z.string().url().optional().default(''), | |
| jellyfin_enabled: z.boolean().default(false), |
| jellyfin_url: z.string().optional().default(''), | ||
| emby_enabled: z.boolean().default(false), | ||
| emby_token: z.string().optional().default(''), | ||
| emby_url: z.string().optional().default('') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add URL validation for 'jellyfin_url' and 'emby_url'
The fields jellyfin_url (line 230) and emby_url (line 233) are defined as strings without URL validation. Adding .url() will ensure valid URLs are entered.
Apply this diff to enhance URL validation:
- jellyfin_url: z.string().optional().default(''),
+ jellyfin_url: z.string().url().optional().default(''),
- emby_url: z.string().optional().default(''),
+ emby_url: z.string().url().optional().default(''),📝 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.
| jellyfin_url: z.string().optional().default(''), | |
| emby_enabled: z.boolean().default(false), | |
| emby_token: z.string().optional().default(''), | |
| emby_url: z.string().optional().default('') | |
| jellyfin_url: z.string().url().optional().default(''), | |
| emby_enabled: z.boolean().default(false), | |
| emby_token: z.string().optional().default(''), | |
| emby_url: z.string().url().optional().default('') |
| <CheckboxField | ||
| {form} | ||
| name="map_metadata" | ||
| {formData} | ||
| fieldDescription="Requires restart, inits an empty database based on the symlinked files" | ||
| /> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance map_metadata field description
The description could be clearer about the relationship between the database and symlinked files.
Consider updating the description to: "Requires restart. Initializes an empty database and maps metadata from the existing symlinked media files."
| </div> | ||
| </ArrayField> | ||
|
|
||
| <TextField {form} name="proxy_url" label="Downloader Proxy URL" {formData} /> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add URL validation and description for proxy field
The proxy URL field should include format validation and a description of supported proxy protocols.
Consider updating the field to:
-<TextField {form} name="proxy_url" label="Downloader Proxy URL" {formData} />
+<TextField
+ {form}
+ name="proxy_url"
+ label="Downloader Proxy URL"
+ {formData}
+ pattern="^(http|https|socks4|socks5)://.*$"
+ fieldDescription="Supported formats: http://, https://, socks4://, socks5://"
+/>📝 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.
| <TextField {form} name="proxy_url" label="Downloader Proxy URL" {formData} /> | |
| <TextField | |
| {form} | |
| name="proxy_url" | |
| label="Downloader Proxy URL" | |
| {formData} | |
| pattern="^(http|https|socks4|socks5)://.*$" | |
| fieldDescription="Supported formats: http://, https://, socks4://, socks5://" | |
| /> |
Summary by CodeRabbit
New Features
Bug Fixes
Chores
.gitignorefile to include additional ignored files and directories for better project management.