Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis set of changes extensively refactors and expands the auto-update feature across backend and frontend. It unifies update logic into a single, DTO-driven service, introduces persistent update records, adds endpoints for update history and status, and standardizes API responses. Frontend components and services are updated to support these new APIs and data structures, while code editors are unified and enhanced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant APIHandler
participant AutoUpdateService
participant DB
User->>Frontend: Initiates update check (all/containers/stacks)
Frontend->>APIHandler: POST /updates/check (with DTO)
APIHandler->>AutoUpdateService: CheckForUpdates(ctx, DTO)
AutoUpdateService->>AutoUpdateService: Concurrently check containers/stacks
AutoUpdateService->>DB: Record update results for each resource
AutoUpdateService-->>APIHandler: Return AutoUpdateResultDto
APIHandler-->>Frontend: JSON response (success/data)
Frontend-->>User: Display update results
sequenceDiagram
participant User
participant Frontend
participant APIHandler
participant AutoUpdateService
participant DB
User->>Frontend: Requests update history
Frontend->>APIHandler: GET /updates/history?limit=50
APIHandler->>AutoUpdateService: GetAutoUpdateHistory(ctx, limit)
AutoUpdateService->>DB: Query auto_update_records
AutoUpdateService-->>APIHandler: Return records
APIHandler-->>Frontend: JSON response (success/data)
Frontend-->>User: Display update history
Possibly related PRs
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 review |
✅ Actions performedReview triggered.
|
kmendell
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (2)
frontend/src/routes/compose/[composeId]/+page.svelte (2)
391-422: Remove duplicate exposed ports section.This entire section is an exact duplicate of the exposed ports card above (lines 357-388). This appears to be a copy-paste error.
-{#if servicePorts && Object.keys(servicePorts).length > 0} - {@const allUniquePorts = [...new Set(Object.values(servicePorts).flat())]} - <Card.Root class="border"> - <Card.Header class="pb-4"> - <Card.Title>Exposed Ports</Card.Title> - </Card.Header> - <Card.Content> - <div class="flex flex-wrap gap-2"> - {#each allUniquePorts as port (port)} - {@const portValue = - typeof port === 'string' || - typeof port === 'number' || - (typeof port === 'object' && port !== null) - ? port - : String(port)} - {@const serviceWithPort = stack.services?.find((s) => - s.ports?.includes(String(port)) - ) || { container_id: '', name: '', status: '' }} - <a - href={getServicePortUrl(serviceWithPort, portValue)} - target="_blank" - rel="noopener noreferrer" - class="inline-flex items-center rounded-md bg-blue-500/10 px-3 py-2 font-medium text-blue-600 transition-colors hover:bg-blue-500/20 dark:text-blue-400" - > - Port {port} - <ExternalLink class="ml-2 size-4" /> - </a> - {/each} - </div> - </Card.Content> - </Card.Root> -{/if}
160-165: Incorrect protocol assignment for non-TCP ports.The logic on line 164 assigns 'https' for non-TCP protocols, which doesn't make sense for UDP ports. UDP is not an HTTP-based protocol.
if (port.Type) { - protocol = port.Type.toLowerCase() === 'tcp' ? 'http' : 'https'; + protocol = port.Type.toLowerCase() === 'tcp' ? 'http' : port.Type.toLowerCase(); }Consider also handling the case where the protocol might be 'udp' differently, as UDP URLs aren't typically accessed via browser.
🧹 Nitpick comments (5)
backend/internal/job/auto_update_job.go (1)
104-104: Clarify the boolean parameter purpose.The
falseparameter added toRegisterJoblacks context. Consider adding a comment to explain what this parameter controls for better code maintainability.return scheduler.RegisterJob( ctx, "auto-update", jobDefinition, job.Execute, - false, + false, // runImmediately )frontend/src/lib/types/auto-update.type.ts (1)
33-47: Consider strengthening type definitions for consistencyThe
AutoUpdateRecordinterface correctly mirrors the backend model but could benefit from stronger typing for better consistency with the resource result interface.export interface AutoUpdateRecord { id: string; resourceId: string; - resourceType: string; + resourceType: 'container' | 'stack'; resourceName: string; - status: string; + status: 'pending' | 'checking' | 'updating' | 'completed' | 'failed' | 'skipped'; startTime: string; endTime?: string; updateAvailable: boolean; updateApplied: boolean; oldImageVersions?: Record<string, string>; newImageVersions?: Record<string, string>; error?: string; details?: Record<string, any>; }frontend/src/routes/compose/new/+page.svelte (1)
154-159: Potential CSS class conflict on example buttons.The
truncateclass on line 158 may not work as intended when combined withbreak-all whitespace-normalclasses. Consider using either truncation or word breaking, but not both.-class="h-auto w-full justify-start p-2 text-left font-mono text-xs break-all whitespace-normal" +class="h-auto w-full justify-start p-2 text-left font-mono text-xs"And update the span:
-<span class="truncate">{command}</span> +<span class="block truncate">{command}</span>backend/internal/api/auto_update_handler.go (1)
23-27: Consider logging JSON binding errors for debugging.While defaulting to
{Type: "all"}on binding failure provides good backward compatibility, consider logging the error to help debug client issues.var req dto.AutoUpdateCheckDto if err := c.ShouldBindJSON(&req); err != nil { + // Log the error for debugging but continue with default + c.Set("binding_error", err.Error()) req = dto.AutoUpdateCheckDto{Type: "all"} }frontend/src/lib/services/api/autoupdate-api-service.ts (1)
25-34: Consider adding type guards for wrapped responses.The code correctly handles the backend's wrapped response format for
getUpdateHistoryandgetUpdateStatus. Consider adding type safety for these wrapped responses.interface WrappedResponse<T> { success: boolean; data: T; } async getUpdateHistory(limit?: number): Promise<AutoUpdateRecord[]> { const params = limit ? { limit } : {}; const response = await this.api.get<WrappedResponse<AutoUpdateRecord[]>>('/updates/history', { params }); return response.data.data; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (23)
backend/internal/api/auto_update_handler.go(2 hunks)backend/internal/api/image_handler.go(2 hunks)backend/internal/api/network_handler.go(2 hunks)backend/internal/api/routes.go(1 hunks)backend/internal/bootstrap/services_bootstrap.go(1 hunks)backend/internal/dto/auto_update_dto.go(1 hunks)backend/internal/job/auto_update_job.go(3 hunks)backend/internal/models/auto_update.go(1 hunks)backend/internal/services/auto_update_service.go(3 hunks)backend/internal/services/image_maturity_service.go(16 hunks)frontend/package.json(1 hunks)frontend/src/lib/components/editor.svelte(2 hunks)frontend/src/lib/components/env-editor.svelte(0 hunks)frontend/src/lib/components/ui/button/button.svelte(1 hunks)frontend/src/lib/components/ui/copy-button/copy-button.svelte(1 hunks)frontend/src/lib/components/universal-table.svelte(7 hunks)frontend/src/lib/models/image.type.ts(1 hunks)frontend/src/lib/services/api/autoupdate-api-service.ts(1 hunks)frontend/src/lib/types/auto-update.type.ts(1 hunks)frontend/src/routes/compose/+page.svelte(1 hunks)frontend/src/routes/compose/[composeId]/+page.svelte(17 hunks)frontend/src/routes/compose/new/+page.svelte(5 hunks)frontend/src/routes/images/[imageId]/+page.svelte(9 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/lib/components/env-editor.svelte
🧰 Additional context used
🧠 Learnings (7)
frontend/src/lib/components/ui/copy-button/copy-button.svelte (1)
Learnt from: kmendell
PR: ofkm/arcane#86
File: src/lib/components/table-cells/user-table-actions.svelte:15-15
Timestamp: 2025-04-30T01:20:17.798Z
Learning: This project uses Svelte 5 with the runes syntax, where props are declared using `$props()` rather than the older `export let` pattern.
frontend/src/lib/components/ui/button/button.svelte (1)
Learnt from: kmendell
PR: ofkm/arcane#86
File: src/lib/components/table-cells/user-table-actions.svelte:15-15
Timestamp: 2025-04-30T01:20:17.798Z
Learning: This project uses Svelte 5 with the runes syntax, where props are declared using `$props()` rather than the older `export let` pattern.
frontend/src/lib/components/editor.svelte (1)
Learnt from: kmendell
PR: ofkm/arcane#86
File: src/lib/components/table-cells/user-table-actions.svelte:15-15
Timestamp: 2025-04-30T01:20:17.798Z
Learning: This project uses Svelte 5 with the runes syntax, where props are declared using `$props()` rather than the older `export let` pattern.
frontend/src/lib/components/universal-table.svelte (1)
Learnt from: kmendell
PR: ofkm/arcane#86
File: src/lib/components/table-cells/user-table-actions.svelte:15-15
Timestamp: 2025-04-30T01:20:17.798Z
Learning: This project uses Svelte 5 with the runes syntax, where props are declared using `$props()` rather than the older `export let` pattern.
frontend/package.json (1)
Learnt from: kmendell
PR: ofkm/arcane#86
File: src/lib/components/table-cells/user-table-actions.svelte:15-15
Timestamp: 2025-04-30T01:20:17.798Z
Learning: This project uses Svelte 5 with the runes syntax, where props are declared using `$props()` rather than the older `export let` pattern.
frontend/src/routes/compose/[composeId]/+page.svelte (1)
Learnt from: kmendell
PR: ofkm/arcane#86
File: src/lib/components/table-cells/user-table-actions.svelte:15-15
Timestamp: 2025-04-30T01:20:17.798Z
Learning: This project uses Svelte 5 with the runes syntax, where props are declared using `$props()` rather than the older `export let` pattern.
frontend/src/routes/compose/new/+page.svelte (1)
Learnt from: kmendell
PR: ofkm/arcane#86
File: src/lib/components/table-cells/user-table-actions.svelte:15-15
Timestamp: 2025-04-30T01:20:17.798Z
Learning: This project uses Svelte 5 with the runes syntax, where props are declared using `$props()` rather than the older `export let` pattern.
🧬 Code Graph Analysis (5)
backend/internal/job/auto_update_job.go (1)
backend/internal/dto/auto_update_dto.go (1)
AutoUpdateCheckDto(3-8)
frontend/src/lib/types/auto-update.type.ts (2)
backend/internal/dto/auto_update_dto.go (1)
AutoUpdateResourceResult(22-33)backend/internal/models/auto_update.go (3)
AutoUpdateRecord(18-33)AutoUpdateRecord(35-37)AutoUpdateStatus(7-7)
backend/internal/models/auto_update.go (2)
frontend/src/lib/types/auto-update.type.ts (2)
AutoUpdateStatus(49-54)AutoUpdateRecord(33-47)backend/internal/models/base.go (2)
JSON(17-17)BaseModel(11-14)
backend/internal/dto/auto_update_dto.go (1)
frontend/src/lib/types/auto-update.type.ts (1)
AutoUpdateResourceResult(20-31)
backend/internal/api/auto_update_handler.go (1)
backend/internal/dto/auto_update_dto.go (1)
AutoUpdateCheckDto(3-8)
🔇 Additional comments (45)
frontend/src/lib/components/ui/copy-button/copy-button.svelte (1)
9-11: LGTM! Import style updated consistently.The change from named imports to default imports from specific icon paths aligns with the package update pattern mentioned in the AI summary. This is a clean refactor that maintains functionality while potentially improving tree-shaking.
frontend/src/lib/components/ui/button/button.svelte (1)
70-70: LGTM! Consistent import style update.The LoaderCircleIcon import change follows the same pattern as other UI components, maintaining consistency across the codebase.
backend/internal/api/image_handler.go (1)
62-62: Method signature update handled correctly.The removal of the final
nilargument from bothCheckImageMaturitycalls aligns with the updated signature. I didn’t spot any remaining invocations using the old five-parameter form, but please double-check across the repository to be sure:# List all invocations of CheckImageMaturity and verify they use the new signature rg --color=never 'CheckImageMaturity' -n .frontend/src/routes/compose/+page.svelte (1)
156-156: Confirmed – checkStacks() is implemented and no old checkStack() remains.
- In frontend/src/lib/services/api/autoupdate-api-service.ts (line 20):
async checkStacks(): Promise<AutoUpdateResult> { … }- No remaining references to
checkStack()across the codebase.- The call at frontend/src/routes/compose/+page.svelte line 156 is correct.
backend/internal/api/routes.go (1)
255-258: Well-structured API route refactoring.The new route organization provides both unified and specific update endpoints, along with useful history functionality. The routing structure is clean and follows REST conventions.
frontend/package.json (1)
29-29: Good dependency additions for enhanced code editing.The new dependencies support improved code editing features:
@uiw/codemirror-theme-githubfor GitHub-style theming@shikijs/langsand@shikijs/themeswithshikifor advanced syntax highlightingisomorphic-dompurifyfor content sanitization (good security practice)svelte-toolbeltfor additional Svelte utilitiesThese align well with the PR's frontend UX improvement objectives.
Also applies to: 45-46, 58-58, 62-62, 66-66
backend/internal/job/auto_update_job.go (2)
39-45: Excellent refactoring to unified DTO-based approach.The change to use
dto.AutoUpdateCheckDtowith a singleCheckForUpdatesmethod is much cleaner than separate calls. The DTO structure allows for flexible update types and options like dry runs.
51-70: Enhanced logging provides excellent visibility.The detailed logging of update results, including counts and individual resource outcomes, will be very helpful for monitoring and debugging auto-update operations.
backend/internal/bootstrap/services_bootstrap.go (1)
25-25: Correct dependency injection updates.The updated service constructors properly inject the required dependencies:
imageMaturityServicenow receivescontainerRegistryfor credential managementautoUpdateservice now receivesdbfor persistent update recordsThese changes are consistent with the service refactoring.
Also applies to: 29-29
frontend/src/lib/models/image.type.ts (1)
4-31: Comprehensive image metadata type expansion.The expanded
ServiceImagetype now includes extensive Docker image metadata:
- Architecture and OS information
- Detailed configuration (ports, environment, labels)
- Storage and filesystem details
- Metadata and descriptors
All properties are appropriately optional for backward compatibility. This enhancement supports the richer image functionality mentioned in the PR objectives.
frontend/src/lib/components/editor.svelte (4)
13-22: LGTM - Well-structured TypeScript interface and props declarationThe
CodeLanguageunion type and props declaration using Svelte 5 runes syntax are implemented correctly. The bindablevalueprop and default values for all props provide good defaults and flexibility.
26-42: LGTM - Excellent dynamic language extension implementationThe
getLanguageExtensionfunction provides clean separation of concerns for different language modes:
- YAML mode includes syntax highlighting and conditional linting for editable editors
- ENV mode uses legacy properties mode without linting
- Proper conditional logic prevents linting in read-only mode
This approach is maintainable and easily extensible for additional languages.
69-70: LGTM - Clean template implementation with dynamic extensionsThe template correctly uses the new
githubDarktheme and dynamically applies language extensions via thegetLanguageExtensionfunction. The binding to height and fontSize props provides proper customization.
51-52: Fix potential off-by-one error in YAML linterThe end position calculation may cause issues when
err.mark.positionis 0, asMath.max(start + 1, err.mark.position + 1)would result inMath.max(1, 1) = 1, creating a zero-length range.- const end = - err.mark?.position !== undefined ? Math.max(start + 1, err.mark.position + 1) : start + 1; + const end = err.mark?.position !== undefined ? Math.max(start + 1, start + 1) : start + 1;Or more simply:
- const end = - err.mark?.position !== undefined ? Math.max(start + 1, err.mark.position + 1) : start + 1; + const end = start + 1;Likely an incorrect or invalid review comment.
frontend/src/lib/components/universal-table.svelte (5)
2-21: LGTM - Improved import organization and readabilityThe multi-line import formatting significantly improves readability and makes it easier to track dependencies. The alphabetical grouping of TanStack table imports is well organized.
72-89: LGTM - Enhanced destructuring with clear defaultsThe multi-line destructuring with explicit defaults improves code readability and makes the component's configuration options more discoverable. The type annotations and default values are well-defined.
204-216: LGTM - Improved reactive computations formattingThe multi-line formatting of derived values enhances readability while maintaining the same logic. The computed properties are well-structured and easy to understand.
269-312: LGTM - Enhanced table header styling and accessibilityThe explicit CSS classes and improved multi-line formatting provide better control over table layout. The addition of proper
aria-labelattributes enhances accessibility. The conditional styling for sortable columns is well-implemented.
364-380: LGTM - Improved pagination control formattingThe multi-line formatting of the Select component improves readability while maintaining all functionality. The event handling and accessibility attributes are properly preserved.
frontend/src/lib/types/auto-update.type.ts (4)
1-6: LGTM - Well-structured request interfaceThe
AutoUpdateCheckinterface provides comprehensive request parameters with proper optional fields and clear union types for thetypefield. The boolean flags forforceUpdateanddryRunare appropriately optional.
8-18: LGTM - Comprehensive result interfaceThe
AutoUpdateResultinterface captures all essential metrics from an auto-update operation with proper field types. The inclusion of timing information and summary statistics provides good observability.
20-31: LGTM - Detailed resource result interfaceThe
AutoUpdateResourceResultinterface aligns well with the backend DTO structure. The union type forresourceTypeand detailed status enum provide type safety. The optional fields for images and error handling are appropriate.
49-54: LGTM - Clear status interfaceThe
AutoUpdateStatusinterface provides a clear summary of current update operations with proper field types for counts and ID arrays.backend/internal/models/auto_update.go (3)
7-16: LGTM - Well-defined status type with comprehensive constantsThe
AutoUpdateStatusstring type with clear lifecycle constants provides excellent type safety and covers all necessary states for auto-update operations. The naming convention is consistent and self-explanatory.
18-33: LGTM - Comprehensive model with proper database mappingThe
AutoUpdateRecordstruct is well-designed with:
- Proper GORM tags for primary key, indexing, and constraints
- Appropriate field types including pointers for nullable fields
- JSON tags matching the frontend interface naming
- Good use of the JSON type for complex data storage
- Embedding of BaseModel for common timestamp fields
The model provides excellent persistence capabilities for auto-update operations.
35-37: LGTM - Proper table name specificationThe
TableName()method correctly specifies the database table name following Go naming conventions.frontend/src/routes/images/[imageId]/+page.svelte (6)
27-27: LGTM - Safer ID extraction with optional chainingThe use of optional chaining (
image?.Id?.split) with fallback to 'N/A' provides robust null safety for the short ID extraction.
29-42: LGTM - Excellent date parsing with comprehensive error handlingThe enhanced date parsing function provides multiple layers of safety:
- Early return for missing data
- Try-catch for parsing errors
- NaN validation for invalid dates
- Graceful fallback to 'N/A' in all error cases
This is a significant improvement in robustness.
45-46: LGTM - Safer property access for image metadataThe optional chaining for
image?.Architectureandimage?.Oswith fallback values provides consistent null safety across the component.
199-218: LGTM - Updated labels access path and improved layoutThe change from
image.Labelstoimage.Config?.Labelscorrectly reflects the updated data structure. The enhanced flex layout with separators improves visual presentation and readability.
220-242: LGTM - Excellent addition of Environment Variables displayThe new Environment Variables card provides valuable information with:
- Proper conditional rendering
- Safe environment variable parsing with split/join logic
- Consistent styling matching the Labels card
- Proper separator handling
The implementation handles edge cases well, such as environment variables containing multiple '=' characters.
94-100: LGTM - Consistent component usage with ArcaneButtonThe transition to using
ArcaneButtonwith proper props provides consistent UI patterns across the application. The loading and disabled state handling is appropriate.frontend/src/routes/compose/new/+page.svelte (3)
1-18: Import changes look good!The import updates align well with the UI refactoring - removing unused components and adding the necessary Dialog components and converterAPI.
26-26: Dialog state management implemented correctly.Good UX decision to close the converter dialog on successful conversion.
Also applies to: 68-68, 74-74
216-244: Clean grid layout implementation with unified editors.The switch to CodeEditor components with language-specific configurations is well implemented. The fixed height provides consistent editing experience.
backend/internal/dto/auto_update_dto.go (3)
3-8: Well-structured request DTO.The
AutoUpdateCheckDtoprovides good flexibility with clear field documentation and appropriate use ofomitemptytags.
10-33: Comprehensive result DTOs with good structure.The separation between overall results (
AutoUpdateResultDto) and per-resource results (AutoUpdateResourceResult) is well-designed. The JSON field names properly follow camelCase convention and match the frontend TypeScript interfaces.
35-44: Well-designed configuration DTO.Good structure with clear field names and helpful comment indicating
CheckIntervalis in minutes.frontend/src/lib/services/api/autoupdate-api-service.ts (1)
36-42: Clean implementation of dry run functionality.The
dryRunmethod provides a convenient way to perform update checks without applying changes. Good use of default parameter.frontend/src/routes/compose/[composeId]/+page.svelte (2)
72-81: Well-implemented scroll-based header visibility.Good use of Svelte's
$effectwith proper cleanup and browser check. The opacity and pointer-events transitions create a smooth UX.Also applies to: 203-208
431-503: Excellent services grid implementation.The responsive grid layout with distinct styling for created vs not-created services provides clear visual feedback. The hover effects and port count badges enhance the user experience.
backend/internal/services/image_maturity_service.go (2)
20-32: Good use of dependency injection pattern.The integration of
ContainerRegistryServicethrough constructor injection is well implemented, following SOLID principles.
637-649: Correct handling of GORM result type.The updated implementation properly handles the
*gorm.DBresult with appropriate error checking before accessingRowsAffected.backend/internal/services/auto_update_service.go (2)
418-444: Well-implemented image update detection logic.Good practices:
- Correctly skips immutable digest-based images
- Compares image IDs for accurate update detection
- Provides helpful logging for debugging
598-625: Excellent handling of different label formats.The method properly handles both map-based and list-based label formats, ensuring compatibility with different Docker Compose versions.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores