docs: Fix IMPLEMENTATION_EXACT.md based on code review
- Remove omitempty tag from AuthToken field (user requirement) - Add complete ListDevices handler modification (was "do same") - Clarify goto validateDevice label placement in kept section - Update template time formatting to use .Format() method - Document breaking change decision in summary section - Rename section for clarity: "Update Device List Handlers" Fixes issues identified during implementation plan review: - Template type mismatch checking (already fixed by user) - AuthToken field ambiguity (Option A: breaking change accepted) - goto label missing (clarified in explanation) - Incomplete ListDevices handler (now shows full modification) - Time formatting in templates (uses templ's .Format())
This commit is contained in:
+93
-73
@@ -159,14 +159,15 @@ func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerF
|
|||||||
```
|
```
|
||||||
|
|
||||||
**Explanation**:
|
**Explanation**:
|
||||||
- `goto validateDevice` is used here as a clean way to jump to common validation code after finding a device
|
- `goto validateDevice` jumps to common validation code (label at line 221 in the complete function below)
|
||||||
- This is Go's idiomatic use of goto for error handling (allowed by PROJECT_GUIDELINES.md)
|
- This is Go's idiomatic use of goto for error handling (allowed by PROJECT_GUIDELINES.md)
|
||||||
- Tries Bearer header first (for KOReader, API clients), then URL path (Kobo), then query param (OPDS)
|
- Tries Bearer header first (for KOReader, API clients), then URL path (Kobo), then query param (OPDS)
|
||||||
- Each device type uses only one method: Kobo→URL path, KOReader→Bearer header
|
- Each device type uses only one method: Kobo→URL path, KOReader→Bearer header
|
||||||
|
- The `validateDevice:` label is preserved in the unchanged section of the function (lines 221-270)
|
||||||
|
|
||||||
**KEEP THE REST OF THE FUNCTION THE SAME** (lines 62-108 remain unchanged)
|
**KEEP THE REST OF THE FUNCTION THE SAME** (lines 62-270 remain unchanged, including the `validateDevice:` label at line 221)
|
||||||
|
|
||||||
**Complete Function After Changes**:
|
**Complete Function After Changes** (reference for verification, shows how `goto validateDevice` connects to the label):
|
||||||
|
|
||||||
```go
|
```go
|
||||||
func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerFunc {
|
func (m *DeviceAuthMiddleware) Authenticate(next echo.HandlerFunc) echo.HandlerFunc {
|
||||||
@@ -491,79 +492,46 @@ func (h *DeviceHandler) RegenerateDeviceToken(c echo.Context) error {
|
|||||||
|
|
||||||
**Location**: Multiple locations
|
**Location**: Multiple locations
|
||||||
|
|
||||||
|
**Overview**: After refactor (REFACTORING_PLAN.md Phase 1.3), templates use `handlers.DeviceInfo` directly - no conversion layer exists. Time fields are `*time.Time` (not strings).
|
||||||
|
|
||||||
**Required Changes**:
|
**Required Changes**:
|
||||||
1. Add `auth_token` field to `DeviceData` struct
|
1. Add `auth_token` field to `handlers.DeviceInfo` struct
|
||||||
2. Add "Copy Sync URL" button for each device
|
2. Add "Copy Sync URL" button for each device
|
||||||
3. Add "Regenerate Token" button for each device
|
3. Add "Regenerate Token" button for each device
|
||||||
4. Add JavaScript functions for copy and regenerate
|
4. Add JavaScript functions for copy and regenerate
|
||||||
|
5. Update time formatting in templates (use `.Format()` method)
|
||||||
|
|
||||||
**ADD FIELD** to struct:
|
#### 1.6.1 Add AuthToken to handlers.DeviceInfo Struct
|
||||||
|
|
||||||
```go
|
|
||||||
type DeviceData struct {
|
|
||||||
ID string
|
|
||||||
DeviceName string
|
|
||||||
DeviceType string
|
|
||||||
LastSync string
|
|
||||||
LastSeen string
|
|
||||||
SyncEnabled bool
|
|
||||||
AutoSync bool
|
|
||||||
SyncFrequency int
|
|
||||||
CreatedAt string
|
|
||||||
DeviceMetadata json.RawMessage
|
|
||||||
AuthToken string // NEW: Device API key for authentication
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
#### 1.6.3 Update Collection Detail Template
|
|
||||||
|
|
||||||
**File**: `templates/collection.templ`
|
|
||||||
|
|
||||||
**Location**: Line 213 (function signature)
|
|
||||||
|
|
||||||
**Current**:
|
|
||||||
```templ
|
|
||||||
templ CollectionDetail(user User, collection CollectionDetailData, books []BookData) {
|
|
||||||
```
|
|
||||||
|
|
||||||
**CHANGE** (use handlers.BookInfo):
|
|
||||||
|
|
||||||
```templ
|
|
||||||
templ CollectionDetail(user User, collection CollectionData, books []handlers.BookInfo) {
|
|
||||||
```
|
|
||||||
|
|
||||||
**ADD FIELD** to struct:
|
|
||||||
|
|
||||||
```go
|
|
||||||
AuthToken string // NEW: Device API key for authentication
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
**ADD FIELD** to struct:
|
|
||||||
|
|
||||||
```go
|
|
||||||
type DeviceData struct {
|
|
||||||
ID string
|
|
||||||
DeviceName string
|
|
||||||
DeviceType string
|
|
||||||
LastSync string
|
|
||||||
LastSeen string
|
|
||||||
SyncEnabled bool
|
|
||||||
AutoSync bool
|
|
||||||
SyncFrequency int
|
|
||||||
CreatedAt string
|
|
||||||
DeviceMetadata json.RawMessage
|
|
||||||
AuthToken string // NEW: Device API key for authentication
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
#### 1.6.2 Update Device List Handler
|
|
||||||
|
|
||||||
**File**: `internal/handlers/devices.go`
|
**File**: `internal/handlers/devices.go`
|
||||||
|
|
||||||
**Location**: `GetDevicesData` function (line 275-313)
|
**Location**: Line 78 (after `DeviceMetadata` field in `DeviceInfo` struct)
|
||||||
|
|
||||||
**ADD FIELD TO RESPONSE** (modify line 298-309):
|
**ADD THIS FIELD**:
|
||||||
|
|
||||||
|
```go
|
||||||
|
type DeviceInfo struct {
|
||||||
|
ID uuid.UUID `json:"id"`
|
||||||
|
DeviceName string `json:"device_name"`
|
||||||
|
DeviceType string `json:"device_type"`
|
||||||
|
LastSync *time.Time `json:"last_sync"`
|
||||||
|
LastSeen *time.Time `json:"last_seen"`
|
||||||
|
SyncEnabled bool `json:"sync_enabled"`
|
||||||
|
AutoSync bool `json:"auto_sync"`
|
||||||
|
SyncFrequency int32 `json:"sync_frequency_minutes"`
|
||||||
|
CreatedAt time.Time `json:"created_at"`
|
||||||
|
DeviceMetadata json.RawMessage `json:"device_metadata,omitempty"`
|
||||||
|
AuthToken string `json:"auth_token"` // NEW: Device API key for authentication
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
#### 1.6.2 Update Device List Handlers
|
||||||
|
|
||||||
|
**File**: `internal/handlers/devices.go`
|
||||||
|
|
||||||
|
**Location**: `GetDevicesData` function (line 275-313) AND `ListDevices` function (line 255-267)
|
||||||
|
|
||||||
|
**GetDevicesData** - ADD FIELD TO RESPONSE (modify line 298-309):
|
||||||
|
|
||||||
```go
|
```go
|
||||||
deviceList = append(deviceList, DeviceInfo{
|
deviceList = append(deviceList, DeviceInfo{
|
||||||
@@ -581,7 +549,25 @@ deviceList = append(deviceList, DeviceInfo{
|
|||||||
})
|
})
|
||||||
```
|
```
|
||||||
|
|
||||||
**Do the same** in `ListDevices` function (line 255-267).
|
**ListDevices** - ADD FIELD TO RESPONSE (modify line around 262):
|
||||||
|
|
||||||
|
Find the loop that constructs `deviceList[i]` and add `AuthToken` field:
|
||||||
|
|
||||||
|
```go
|
||||||
|
deviceList[i] = DeviceInfo{
|
||||||
|
ID: device.ID.Bytes,
|
||||||
|
DeviceName: device.DeviceName,
|
||||||
|
DeviceType: device.DeviceType,
|
||||||
|
LastSync: (*time.Time)(&device.LastSync.Time),
|
||||||
|
LastSeen: (*time.Time)(&device.LastSeen.Time),
|
||||||
|
SyncEnabled: syncEnabled,
|
||||||
|
AutoSync: autoSync,
|
||||||
|
SyncFrequency: syncFreq,
|
||||||
|
CreatedAt: device.CreatedAt.Time,
|
||||||
|
DeviceMetadata: device.DeviceMetadata,
|
||||||
|
AuthToken: device.AuthToken, // NEW: Include auth token
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
#### 1.6.3 Update Device Card Template
|
#### 1.6.3 Update Device Card Template
|
||||||
|
|
||||||
@@ -632,16 +618,16 @@ for _, device := range devices {
|
|||||||
</div>
|
</div>
|
||||||
<div class="flex justify-between">
|
<div class="flex justify-between">
|
||||||
<span style="color: var(--text-secondary)">Last Sync</span>
|
<span style="color: var(--text-secondary)">Last Sync</span>
|
||||||
if device.LastSync != "" {
|
if device.LastSync != nil {
|
||||||
<span style="color: var(--text-primary)">{ device.LastSync }</span>
|
<span style="color: var(--text-primary)">{ device.LastSync.Format("2006-01-02 15:04") }</span>
|
||||||
} else {
|
} else {
|
||||||
<span style="color: var(--text-primary)">Never</span>
|
<span style="color: var(--text-primary)">Never</span>
|
||||||
}
|
}
|
||||||
</div>
|
</div>
|
||||||
<div class="flex justify-between">
|
<div class="flex justify-between">
|
||||||
<span style="color: var(--text-secondary)">Last Seen</span>
|
<span style="color: var(--text-secondary)">Last Seen</span>
|
||||||
if device.LastSeen != "" {
|
if device.LastSeen != nil {
|
||||||
<span style="color: var(--text-primary)">{ device.LastSeen }</span>
|
<span style="color: var(--text-primary)">{ device.LastSeen.Format("2006-01-02 15:04") }</span>
|
||||||
} else {
|
} else {
|
||||||
<span style="color: var(--text-primary)">Never</span>
|
<span style="color: var(--text-primary)">Never</span>
|
||||||
}
|
}
|
||||||
@@ -2285,8 +2271,9 @@ sudo ufw deny 8765
|
|||||||
4. **Router** (`internal/router/device.go`)
|
4. **Router** (`internal/router/device.go`)
|
||||||
- Added: Route for `PUT /:id/regenerate-token`
|
- Added: Route for `PUT /:id/regenerate-token`
|
||||||
|
|
||||||
5. **Handler** (`internal/handlers/devices.go`)
|
5. **Handler** (`internal/handlers/devices.go`)
|
||||||
- Added: `RegenerateDeviceToken` function
|
- Added: `RegenerateDeviceToken` function
|
||||||
|
- Added: `AuthToken` field to `DeviceInfo` struct (breaking change: all clients must accept this field)
|
||||||
- Modified: `GetDevicesData`, `ListDevices` to include `auth_token`
|
- Modified: `GetDevicesData`, `ListDevices` to include `auth_token`
|
||||||
|
|
||||||
6. **Template** (`templates/devices.templ`)
|
6. **Template** (`templates/devices.templ`)
|
||||||
@@ -2346,6 +2333,39 @@ sudo ufw deny 8765
|
|||||||
|
|
||||||
**End of Implementation Document**
|
**End of Implementation Document**
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Updates Applied (2026-02-12)
|
||||||
|
|
||||||
|
Based on code review analysis, the following fixes were applied to this document:
|
||||||
|
|
||||||
|
### Fixed Issues
|
||||||
|
|
||||||
|
1. **AuthToken Field Addition** (Section 1.6.1)
|
||||||
|
- Added `AuthToken` field to `DeviceInfo` struct
|
||||||
|
- Breaking change (Option A): All clients must accept this field
|
||||||
|
|
||||||
|
2. **ListDevices Handler Update** (Section 1.6.2)
|
||||||
|
- Changed from "Do the same" to showing complete code modification
|
||||||
|
- Added explicit code block for `ListDevices` function modification
|
||||||
|
- Renamed section from "Update Device List Handler" to "Update Device List Handlers"
|
||||||
|
|
||||||
|
3. **goto Label Clarification** (Section 1.2)
|
||||||
|
- Updated explanation to clarify that `validateDevice:` label is in the kept section
|
||||||
|
- Added reference to line 221 in complete function
|
||||||
|
- Made it explicit that lines 62-270 remain unchanged
|
||||||
|
|
||||||
|
4. **Time Formatting** (Section 1.6.3)
|
||||||
|
- Updated template code to use `.Format("2006-01-02 15:04")` method
|
||||||
|
- Fixed both `LastSync` and `LastSeen` field display
|
||||||
|
- Both fields now properly format `*time.Time` values
|
||||||
|
|
||||||
|
### Remaining User Decisions
|
||||||
|
|
||||||
|
- ✅ Template type checking: Already fixed by user
|
||||||
|
- ✅ Bruno test variables: User confirmed they exist in environment
|
||||||
|
- ✅ Breaking change on AuthToken: User approved Option A
|
||||||
|
|
||||||
**Next Steps**:
|
**Next Steps**:
|
||||||
1. Review this document completely
|
1. Review this document completely
|
||||||
2. Ask questions if anything is unclear
|
2. Ask questions if anything is unclear
|
||||||
|
|||||||
Reference in New Issue
Block a user