fix: enable Stremio NNTP service during template import #568
Conversation
WalkthroughTemplate input handling now recognises a new 'custom-nntp-servers' input type. The modal imports and renders Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![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 (1)
packages/frontend/src/components/shared/config-templates-modal.tsx (1)
973-978: Use strict equality operators.The filter uses loose equality (
==) instead of strict equality (===). While this works, strict equality is preferred for type safety and consistency with TypeScript best practices.🔎 Proposed fix
serviceMeta.credentials .filter( (cred) => - cred.type == 'string' || - cred.type == 'password' || - cred.type == 'custom-nntp-servers' + cred.type === 'string' || + cred.type === 'password' || + cred.type === 'custom-nntp-servers' )
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/frontend/src/components/shared/config-templates-modal.tsxpackages/frontend/src/components/shared/template-option.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/frontend/src/components/shared/config-templates-modal.tsx (1)
packages/frontend/src/components/shared/template-option.tsx (1)
NNTPServersInput(447-666)
🔇 Additional comments (6)
packages/frontend/src/components/shared/template-option.tsx (1)
439-447: LGTM! Exports enable proper consumption.The interface and component exports are correct and enable the NNTPServersInput to be consumed by the config-templates-modal component.
packages/frontend/src/components/shared/config-templates-modal.tsx (5)
31-31: LGTM! Import is correct.The import statement correctly references the newly exported component.
77-77: LGTM! Type extension is appropriate.The addition of
'custom-nntp-servers'to the TemplateInput type union correctly supports the new input type.
985-988: LGTM! Type mapping is correct.The ternary operator correctly maps 'custom-nntp-servers' credentials to the corresponding input type, while treating other credential types as password inputs for security.
1747-1758: LGTM! Component rendering is correct.The NNTPServersInput component is properly rendered with the correct props, and the state management for the input value is handled appropriately.
1762-1767: LGTM! Conditional rendering avoids duplication.Skipping the description for
'custom-nntp-servers'is correct since the NNTPServersInput component handles its own description rendering internally.
packages/frontend/src/components/shared/config-templates-modal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/frontend/src/components/shared/config-templates-modal.tsx (1)
973-978: Consider using strict equality (===) instead of loose equality (==).The filter logic correctly includes the new
custom-nntp-serverstype. However, using==instead of===is inconsistent with TypeScript best practices, even though it's functionally equivalent when comparing string literals.🔎 Proposed fix
serviceMeta.credentials .filter( (cred) => - cred.type == 'string' || - cred.type == 'password' || - cred.type == 'custom-nntp-servers' + cred.type === 'string' || + cred.type === 'password' || + cred.type === 'custom-nntp-servers' )
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/frontend/src/components/shared/config-templates-modal.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/frontend/src/components/shared/config-templates-modal.tsx (2)
packages/frontend/src/components/shared/template-option.tsx (1)
NNTPServersInput(447-666)packages/frontend/src/components/ui/password-input/password-input.tsx (1)
PasswordInput(40-146)
🔇 Additional comments (5)
packages/frontend/src/components/shared/config-templates-modal.tsx (5)
31-31: LGTM!Import correctly brings in the
NNTPServersInputcomponent needed for the new custom NNTP servers input type.
77-77: LGTM!Type union correctly extended to support the new NNTP servers input type.
1194-1221: Good improvement: services are now disabled instead of removed.This change correctly addresses the previous concern about data loss. Pre-configured services in the template are now preserved with their settings, and unselected services are simply disabled rather than removed.
Minor type safety note: Line 1207 uses
as anytype assertion which bypasses type checking. Consider using the proper service ID type if available.🔎 Optional: remove type assertion if service types are available
services.push({ - id: serviceId as any, + id: serviceId, enabled: true, credentials: {}, });If the service array expects a specific union type for
id, you may need to properly typeselectedServicesor use a type guard instead ofas any.
1750-1761: LGTM!The
NNTPServersInputcomponent is correctly integrated with proper prop wiring. The component handles its own description rendering internally, justifying the conditional description logic below.Note: The
disabledprop is available onNNTPServersInputbut not passed here. This is acceptable if the input should never be disabled in this context, but consider adding it if disabling is needed during loading states.
1765-1770: LGTM!Correctly excludes description rendering for
custom-nntp-serversinputs since theNNTPServersInputcomponent renders its own description internally.
packages/frontend/src/components/shared/config-templates-modal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/frontend/src/components/shared/config-templates-modal.tsx (1)
973-978: Consider using strict equality operators.The filter uses loose equality (
==) instead of strict equality (===). While functionally equivalent for string comparisons, strict equality is the recommended practice in TypeScript/JavaScript to avoid unexpected type coercion.🔎 Suggested fix
serviceMeta.credentials .filter( (cred) => - cred.type == 'string' || - cred.type == 'password' || - cred.type == 'custom-nntp-servers' + cred.type === 'string' || + cred.type === 'password' || + cred.type === 'custom-nntp-servers' )
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/frontend/src/components/shared/config-templates-modal.tsx
🔇 Additional comments (6)
packages/frontend/src/components/shared/config-templates-modal.tsx (6)
31-31: LGTM!The import is correctly added and the component is properly used later in the rendering logic.
77-77: LGTM!The type union correctly includes the new
'custom-nntp-servers'input type.
985-990: LGTM!The type mapping correctly preserves the original credential types:
'custom-nntp-servers'maps to itself,'password'maps to'password', and all other types (including'string') correctly default to'string'.
1196-1223: Good fix addressing the previous review concern.The logic now correctly:
- Enables selected services (creating them if missing)
- Disables unselected services instead of removing them
This preserves pre-configured service settings and credentials, preventing data loss that was flagged in the previous review.
1752-1763: LGTM!The
NNTPServersInputcomponent is correctly integrated with appropriate props and state handling.
1767-1772: LGTM!Correctly suppresses the external description rendering for
'custom-nntp-servers'inputs since theNNTPServersInputcomponent handles its own description display internally.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.