|
| 1 | +# Consolidate Metadata Commands & Fix Auto-Triggers |
| 2 | + |
| 3 | +## Implementation Status: COMPLETED |
| 4 | + |
| 5 | +All items have been implemented. See detailed report at the end of this file. |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## Desired Metadata Flow |
| 10 | + |
| 11 | +``` |
| 12 | +PULL from SolidWorks (Drawings are source of truth): |
| 13 | + - WHEN: User right-clicks drawing and selects "Sync Metadata" |
| 14 | + - WHAT: Read part number, description, revision FROM drawing -> update pendingMetadata |
| 15 | +
|
| 16 | +PUSH to SolidWorks (BluePLM is source of truth): |
| 17 | + - WHEN: User right-clicks part/assembly and selects "Sync Metadata" |
| 18 | + - WHAT: Write part number, description FROM BluePLM -> into the SW file |
| 19 | +
|
| 20 | +REQUIREMENTS: |
| 21 | + - Only works on checked-out files (need edit access) |
| 22 | + - NEVER auto-trigger on check-in, checkout, move, navigation, FileWatcher, downloads |
| 23 | +``` |
| 24 | + |
| 25 | +## Problem: Two Confusing Commands |
| 26 | + |
| 27 | +Currently there are two separate commands: |
| 28 | + |
| 29 | +- `refresh-local-metadata` - Reads SW -> updates pendingMetadata |
| 30 | +- `sync-sw-metadata` - Reads SW -> updates database |
| 31 | + |
| 32 | +This is confusing. We need ONE command: **sync-metadata** that: |
| 33 | + |
| 34 | +- For **drawings**: PULL (read from SW file -> update pendingMetadata) |
| 35 | +- For **parts/assemblies**: PUSH (write from pendingMetadata/pdmData -> into SW file) |
| 36 | +- Only works on **checked-out files** |
| 37 | + |
| 38 | +## Implementation |
| 39 | + |
| 40 | +### 1. Create new consolidated sync-metadata command ✅ |
| 41 | + |
| 42 | +Create `src/lib/commands/handlers/syncMetadata.ts`: |
| 43 | + |
| 44 | +The command should: |
| 45 | + |
| 46 | +- Validate files are SolidWorks files (.sldprt, .sldasm, .slddrw) |
| 47 | +- Validate files are checked out by current user |
| 48 | +- For each file: |
| 49 | + - If **drawing** (.slddrw): PULL - call `getProperties()` -> `updatePendingMetadata()` |
| 50 | + - If **part/assembly** (.sldprt/.sldasm): PUSH - call `setProperties()` with values from pendingMetadata or pdmData |
| 51 | + |
| 52 | +Can reuse extraction logic from existing `refreshLocalMetadata.ts` for the PULL path (drawings). |
| 53 | + |
| 54 | +Can reuse the `setProperties` pattern from `DetailsPanel.tsx` for the PUSH path (parts/assemblies). |
| 55 | + |
| 56 | +### 2. Delete old command files ✅ |
| 57 | + |
| 58 | +- [x] Delete `src/lib/commands/handlers/refreshLocalMetadata.ts` |
| 59 | +- [x] Delete `src/lib/commands/handlers/syncSwMetadata.ts` |
| 60 | + |
| 61 | +### 3. Update command registration ✅ |
| 62 | + |
| 63 | +In `src/lib/commands/index.ts`: |
| 64 | + |
| 65 | +- Remove imports for old commands |
| 66 | +- Add import for new `syncMetadataCommand` |
| 67 | +- Update `initializeCommands()` to register `sync-metadata` |
| 68 | +- Update convenience export function |
| 69 | + |
| 70 | +### 4. Update command types ✅ |
| 71 | + |
| 72 | +In `src/lib/commands/types.ts`: |
| 73 | + |
| 74 | +- [x] Remove `RefreshLocalMetadataParams` interface |
| 75 | +- [x] Remove `SyncSwMetadataParams` interface |
| 76 | +- [x] Add `SyncMetadataParams` interface |
| 77 | +- [x] Update `CommandId` union: remove old IDs, add `'sync-metadata'` |
| 78 | +- [x] Update `CommandMap` type |
| 79 | + |
| 80 | +### 5. Update context menus ✅ |
| 81 | + |
| 82 | +**MetadataActions.tsx:** |
| 83 | + |
| 84 | +- Change `executeCommand('refresh-local-metadata', ...)` to `executeCommand('sync-metadata', ...)` |
| 85 | +- Add validation that files are checked out |
| 86 | + |
| 87 | +**CollaborationActions.tsx:** |
| 88 | + |
| 89 | +- Change `executeCommand('sync-sw-metadata', ...)` to `executeCommand('sync-metadata', ...)` |
| 90 | +- This section may need consolidation with MetadataActions |
| 91 | + |
| 92 | +### 6. Update FileTree vault context menu ✅ |
| 93 | + |
| 94 | +In `src/features/source/explorer/FileTree.tsx`: |
| 95 | + |
| 96 | +- [x] Change `executeCommand('sync-sw-metadata', ...)` to `executeCommand('sync-metadata', ...)` |
| 97 | +- [x] Filter to only files checked out by current user |
| 98 | + |
| 99 | +### 7. Update ServiceTab sync button ✅ |
| 100 | + |
| 101 | +In `src/features/settings/integrations/solidworks/tabs/ServiceTab.tsx`: |
| 102 | + |
| 103 | +- Change `executeCommand('sync-sw-metadata', ...)` to `executeCommand('sync-metadata', ...)` |
| 104 | + |
| 105 | +### 8. Remove auto-refresh from FileWatcher ✅ |
| 106 | + |
| 107 | +In `src/app/App.tsx`: |
| 108 | + |
| 109 | +- [x] Deleted the auto-refresh block that triggered `refresh-local-metadata` on file changes |
| 110 | + |
| 111 | +### 9. Remove metadata extraction from checkout ✅ |
| 112 | + |
| 113 | +In `src/lib/commands/handlers/checkout.ts`: |
| 114 | + |
| 115 | +- [x] Remove the `extractSolidWorksMetadata` function |
| 116 | +- [x] Remove the call to it |
| 117 | +- [x] Keep `SW_EXTENSIONS` constant (still used for other purposes) |
| 118 | + |
| 119 | +### 10. Add guard in updatePendingMetadata ✅ |
| 120 | + |
| 121 | +In `src/stores/slices/filesSlice.ts`, add guard at start of `updatePendingMetadata`: |
| 122 | + |
| 123 | +```typescript |
| 124 | +updatePendingMetadata: (path, metadata) => { |
| 125 | + const state = get() |
| 126 | + const file = state.files.find(f => f.path === path) |
| 127 | + |
| 128 | + // Guard: Never set pendingMetadata on non-editable files |
| 129 | + if (file?.pdmData?.id) { |
| 130 | + const checkedOutBy = file.pdmData.checked_out_by |
| 131 | + const currentUserId = state.user?.id |
| 132 | + |
| 133 | + if (!checkedOutBy || checkedOutBy !== currentUserId) { |
| 134 | + console.warn('[filesSlice] updatePendingMetadata: Skipping non-editable file', path) |
| 135 | + window.electronAPI?.log('warn', '[filesSlice] Attempted to set pendingMetadata on non-editable file', { |
| 136 | + path, |
| 137 | + checkedOutBy, |
| 138 | + currentUserId, |
| 139 | + reason: !checkedOutBy ? 'not_checked_out' : 'checked_out_by_other' |
| 140 | + }) |
| 141 | + return |
| 142 | + } |
| 143 | + } |
| 144 | + |
| 145 | + // ... rest of existing logic |
| 146 | +} |
| 147 | +``` |
| 148 | + |
| 149 | +### 11. Remove autoRefreshMetadataOnSave setting ✅ |
| 150 | + |
| 151 | +**src/stores/types.ts:** |
| 152 | + |
| 153 | +- [x] Remove `autoRefreshMetadataOnSave: boolean` from SettingsSlice |
| 154 | +- [x] Remove `setAutoRefreshMetadataOnSave` action |
| 155 | + |
| 156 | +**src/stores/slices/settingsSlice.ts:** |
| 157 | + |
| 158 | +- [x] Remove initial state: `autoRefreshMetadataOnSave: true,` |
| 159 | +- [x] Remove setter action |
| 160 | + |
| 161 | +**src/features/settings/integrations/solidworks/tabs/SettingsTab.tsx:** |
| 162 | + |
| 163 | +- [x] Remove the "Auto-refresh on file save" toggle UI section |
| 164 | + |
| 165 | +**src/features/settings/integrations/solidworks/hooks/useSolidWorksSettings.ts:** |
| 166 | + |
| 167 | +- [x] Remove from destructured state and return object |
| 168 | + |
| 169 | +## Files to Modify (Complete List) |
| 170 | + |
| 171 | +**New file:** |
| 172 | + |
| 173 | +- `src/lib/commands/handlers/syncMetadata.ts` - CREATE new consolidated command |
| 174 | + |
| 175 | +**Delete files:** |
| 176 | + |
| 177 | +- `src/lib/commands/handlers/refreshLocalMetadata.ts` - DELETE |
| 178 | +- `src/lib/commands/handlers/syncSwMetadata.ts` - DELETE |
| 179 | + |
| 180 | +**Modify files:** |
| 181 | + |
| 182 | +- `src/lib/commands/index.ts` - Update registrations and exports |
| 183 | +- `src/lib/commands/types.ts` - Update command types |
| 184 | +- `src/app/App.tsx` - Remove FileWatcher auto-refresh block |
| 185 | +- `src/lib/commands/handlers/checkout.ts` - Remove metadata extraction |
| 186 | +- `src/stores/slices/filesSlice.ts` - Add guard in updatePendingMetadata |
| 187 | +- `src/stores/types.ts` - Remove autoRefreshMetadataOnSave |
| 188 | +- `src/stores/slices/settingsSlice.ts` - Remove autoRefreshMetadataOnSave |
| 189 | +- `src/features/settings/integrations/solidworks/tabs/SettingsTab.tsx` - Remove toggle + update sync button |
| 190 | +- `src/features/settings/integrations/solidworks/hooks/useSolidWorksSettings.ts` - Remove setting |
| 191 | +- `src/features/source/browser/components/ContextMenu/actions/MetadataActions.tsx` - Use sync-metadata |
| 192 | +- `src/features/source/browser/components/ContextMenu/actions/CollaborationActions.tsx` - Use sync-metadata |
| 193 | +- `src/features/source/explorer/FileTree.tsx` - Use sync-metadata |
| 194 | + |
| 195 | +--- |
| 196 | + |
| 197 | +## Implementation Report |
| 198 | + |
| 199 | +### Summary |
| 200 | + |
| 201 | +All plan items have been successfully implemented. The consolidation replaced two confusing commands (`refresh-local-metadata` and `sync-sw-metadata`) with a single unified `sync-metadata` command that: |
| 202 | + |
| 203 | +- For **drawings** (.slddrw): PULL - reads metadata from the SW file (with PRP resolution from parent models) and updates `pendingMetadata` |
| 204 | +- For **parts/assemblies** (.sldprt, .sldasm): PUSH - writes metadata from `pendingMetadata`/`pdmData` into the SW file |
| 205 | +- **Only works on files checked out by the current user** |
| 206 | + |
| 207 | +### Files Created |
| 208 | + |
| 209 | +1. **`src/lib/commands/handlers/syncMetadata.ts`** - New consolidated command handler (~430 lines) |
| 210 | + - Implements PULL logic for drawings with PRP (Part Reference Property) resolution |
| 211 | + - Implements PUSH logic for parts/assemblies using `setProperties` API |
| 212 | + - Validates files are SolidWorks files and checked out by current user |
| 213 | + - Includes comprehensive logging for debugging |
| 214 | + |
| 215 | +### Files Deleted |
| 216 | + |
| 217 | +1. **`src/lib/commands/handlers/refreshLocalMetadata.ts`** - Old "read SW -> update pendingMetadata" command |
| 218 | +2. **`src/lib/commands/handlers/syncSwMetadata.ts`** - Old "read SW -> update database" command |
| 219 | + |
| 220 | +### Files Modified |
| 221 | + |
| 222 | +1. **`src/lib/commands/types.ts`** - Updated command types |
| 223 | +2. **`src/lib/commands/index.ts`** - Updated command registration and exports |
| 224 | +3. **`src/lib/commands/handlers/index.ts`** - Updated handler exports |
| 225 | +4. **`src/lib/commands/parser.ts`** - Updated terminal command aliases |
| 226 | +5. **`src/lib/permissions.ts`** - Updated permission mapping |
| 227 | +6. **`src/lib/commands/handlers/terminal.ts`** - Updated help text |
| 228 | +7. **`src/lib/commands/handlers/checkout.ts`** - Removed metadata extraction on checkout |
| 229 | +8. **`src/app/App.tsx`** - Removed FileWatcher auto-refresh block |
| 230 | +9. **`src/stores/slices/filesSlice.ts`** - Added guard in `updatePendingMetadata` |
| 231 | +10. **`src/stores/types.ts`** - Removed `autoRefreshMetadataOnSave` setting |
| 232 | +11. **`src/stores/slices/settingsSlice.ts`** - Removed `autoRefreshMetadataOnSave` state and setter |
| 233 | +12. **`src/features/settings/integrations/solidworks/tabs/SettingsTab.tsx`** - Removed auto-refresh toggle UI |
| 234 | +13. **`src/features/settings/integrations/solidworks/tabs/ServiceTab.tsx`** - Updated bulk sync button |
| 235 | +14. **`src/features/settings/integrations/solidworks/hooks/useSolidWorksSettings.ts`** - Removed setting |
| 236 | +15. **`src/features/source/browser/components/ContextMenu/actions/MetadataActions.tsx`** - Updated to use `sync-metadata` |
| 237 | +16. **`src/features/source/browser/components/ContextMenu/actions/CollaborationActions.tsx`** - Updated to use `sync-metadata` |
| 238 | +17. **`src/features/source/explorer/FileTree.tsx`** - Updated vault context menu |
| 239 | +18. **`src/features/source/context-menu/items/PDMItems.tsx`** - Updated permission check and command |
| 240 | + |
| 241 | +### Root Cause Analysis |
| 242 | + |
| 243 | +The original problem stemmed from two distinct issues: |
| 244 | + |
| 245 | +1. **Confusing dual commands**: `refresh-local-metadata` and `sync-sw-metadata` had overlapping functionality but different targets (pendingMetadata vs database). Users didn't know which to use. |
| 246 | + |
| 247 | +2. **Unwanted auto-triggers**: Metadata was being extracted automatically during: |
| 248 | + - File checkout (small batches only, but still unexpected) |
| 249 | + - FileWatcher events (when SW files changed on disk) |
| 250 | + |
| 251 | + This caused issues because: |
| 252 | + - Files that weren't checked out could have their pendingMetadata set |
| 253 | + - Auto-extraction could overwrite intentional user changes |
| 254 | + - Performance impact from unexpected SW service calls |
| 255 | + |
| 256 | +### Potential Gaps / Considerations |
| 257 | + |
| 258 | +1. **Removed bulk database sync functionality**: The old `sync-sw-metadata` command could sync metadata from SW files directly to the database (creating new versions). The new `sync-metadata` command doesn't do this - it only works on checked-out files. The "Sync All Vault Metadata" button in ServiceTab now only works on checked-out files. |
| 259 | + |
| 260 | + **Impact**: If users need to bulk-backfill metadata from SW files to the database, they would need to: |
| 261 | + 1. Check out the files |
| 262 | + 2. Run sync-metadata (which PULLs for drawings, but PUSHes for parts/assemblies) |
| 263 | + 3. Check in to persist to database |
| 264 | + |
| 265 | + For drawings this works well (PULL updates pendingMetadata, check-in saves to DB). |
| 266 | + For parts/assemblies, this is a bit odd - PUSH writes BluePLM data TO the file, not FROM the file. |
| 267 | + |
| 268 | +2. **Drawing revision handling**: The PULL logic for drawings keeps the drawing's own revision (from revision table) while inheriting part_number and description from the parent model. This is correct behavior per the plan. |
| 269 | + |
| 270 | +3. **Legacy aliases preserved**: The old command names (`sync-sw-metadata`, `refresh-local-metadata`, `sw-sync`) are preserved as aliases in the new command, ensuring backward compatibility for users who may have memorized the old commands. |
| 271 | + |
| 272 | +### Verification |
| 273 | + |
| 274 | +- TypeScript compilation: ✅ Passed (`npm run typecheck`) |
| 275 | +- Linter errors: ✅ None found |
0 commit comments