Enhance migration templates with critical omission prevention

Add comprehensive guidance to prevent common migration oversights: - Remove unused notification imports - Replace hardcoded timeout values with constants - Remove legacy wrapper functions - Extract long class attributes to computed properties - Replace literal strings with constants Based on lessons learned from ContactQRScanShowView.vue migration. Includes validation commands and specific examples for each pattern.
2025-07-09 04:42:05 +00:00
parent 6210a088dd
commit b1435b0c42
7 changed files with 753 additions and 242 deletions
--- a/docs/migration-templates/COMPLETE_MIGRATION_CHECKLIST.md
+++ b/docs/migration-templates/COMPLETE_MIGRATION_CHECKLIST.md
@@ -214,6 +214,30 @@ git commit -m "[user-approved-message]"
 - [ ] **Confirm**: `this.$notify({type: "confirm"})` → `this.notify.confirm(message, onYes)`
 - [ ] **Standard patterns**: Use `this.notify.confirmationSubmitted()`, `this.notify.sent()`, etc.

+### [ ] 13.1. 🚨 CRITICAL: Replace ALL Hardcoded Timeout Values
+- [ ] **Replace hardcoded timeouts**: `3000`, `5000`, `1000`, `2000` → timeout constants
+- [ ] **Add timeout constants**: `COMPONENT_TIMEOUT_SHORT = 1000`, `COMPONENT_TIMEOUT_MEDIUM = 2000`, `COMPONENT_TIMEOUT_STANDARD = 3000`, `COMPONENT_TIMEOUT_LONG = 5000`
+- [ ] **Import timeout constants**: Import from `@/constants/notifications`
+- [ ] **Validation command**: `grep -n "notify\.[a-z]*(" [file] | grep -E "[0-9]{3,4}"`
+
+### [ ] 13.2. 🚨 CRITICAL: Remove ALL Unused Notification Imports
+- [ ] **Check each import**: Verify every imported `NOTIFY_*` constant is actually used
+- [ ] **Remove unused imports**: Delete any `NOTIFY_*` constants not referenced in component
+- [ ] **Validation command**: `grep -n "import.*NOTIFY_" [file]` then verify usage
+- [ ] **Clean imports**: Only import notification constants that are actually used
+
+### [ ] 13.3. 🚨 CRITICAL: Replace ALL Literal Strings with Constants
+- [ ] **No literal strings**: All static notification messages must use constants
+- [ ] **Add constants**: Create `NOTIFY_*` constants for ALL static messages
+- [ ] **Replace literals**: `"The contact DID is missing."` → `NOTIFY_CONTACT_MISSING_DID.message`
+- [ ] **Validation command**: `grep -n "notify\.[a-z]*(" [file] | grep -v "NOTIFY_\|message"`
+
+### [ ] 13.4. 🚨 CRITICAL: Remove Legacy Wrapper Functions
+- [ ] **Remove legacy functions**: Delete `danger()`, `success()`, `warning()`, `info()` wrapper functions
+- [ ] **Direct usage**: Use `this.notify.error()` instead of `this.danger()`
+- [ ] **Why remove**: Maintains consistency with centralized notification system
+- [ ] **Validation command**: `grep -n "danger\|success\|warning\|info.*(" [file] | grep -v "notify\."`
+
 ### [ ] 14. Constants vs Literal Strings
 - [ ] **Use constants** for static, reusable messages
 - [ ] **Use literal strings** for dynamic messages with variables
@@ -234,6 +258,13 @@ git commit -m "[user-approved-message]"
 - [ ] **Conditional Logic**: Extract complex `v-if` conditions to computed properties
 - [ ] **Dynamic Values**: Convert repeated calculations to cached computed properties

+### [ ] 16.1. 🚨 CRITICAL: Extract ALL Long Class Attributes
+- [ ] **Find long classes**: Search for `class="[^"]{50,}"` (50+ character class strings)
+- [ ] **Extract to computed**: Replace with `:class="computedPropertyName"`
+- [ ] **Name descriptively**: Use names like `nameWarningClasses`, `buttonPrimaryClasses`
+- [ ] **Validation command**: `grep -n "class=\"[^\"]\{50,\}" [file]`
+- [ ] **Benefits**: Improves readability, enables reusability, makes testing easier
+
 ### [ ] 17. Document Computed Properties
 - [ ] **JSDoc Comments**: Add comprehensive comments for all computed properties
 - [ ] **Purpose Documentation**: Explain what template complexity each property solves
@@ -282,6 +313,16 @@ git commit -m "[user-approved-message]"
 - [ ] **ALL** notifications through helper methods with centralized constants
 - [ ] **ALL** complex template logic extracted to computed properties

+### [ ] 22.1. 🚨 CRITICAL: Validate All Omission Fixes
+- [ ] **NO** hardcoded timeout values (`1000`, `2000`, `3000`, `5000`)
+- [ ] **NO** unused notification imports (all `NOTIFY_*` imports are used)
+- [ ] **NO** literal strings in notification calls (all use constants)
+- [ ] **NO** legacy wrapper functions (`danger()`, `success()`, etc.)
+- [ ] **NO** long class attributes (50+ characters) in template
+- [ ] **ALL** timeout values use constants
+- [ ] **ALL** notification messages use centralized constants
+- [ ] **ALL** class styling extracted to computed properties
+
 ## ⏱️ Time Tracking & Commit Phase

 ### [ ] 23. End Time Tracking
--- a/docs/migration-templates/component-migration.md
+++ b/docs/migration-templates/component-migration.md
@@ -273,6 +273,192 @@ This approach provides:
 - **Localization**: Ready for future i18n support  
 - **Testability**: Constants can be imported in tests

+## Critical Migration Omissions to Avoid
+
+### 1. Remove Unused Notification Imports
+
+**❌ COMMON MISTAKE**: Importing notification constants that aren't actually used
+
+```typescript
+// ❌ BAD - Unused imports
+import {
+  NOTIFY_CONTACT_ADDED,           // Not used
+  NOTIFY_CONTACT_ADDED_SUCCESS,   // Not used  
+  NOTIFY_CONTACT_ERROR,           // Actually used
+  NOTIFY_CONTACT_EXISTS,          // Actually used
+} from "@/constants/notifications";
+
+// ✅ GOOD - Only import what's used
+import {
+  NOTIFY_CONTACT_ERROR,
+  NOTIFY_CONTACT_EXISTS,
+} from "@/constants/notifications";
+```
+
+**How to check**: Use IDE "Find Usages" or grep to verify each imported constant is actually used in the file.
+
+### 2. Replace ALL Hardcoded Timeout Values
+
+**❌ COMMON MISTAKE**: Converting `$notify()` calls but leaving hardcoded timeout values
+
+```typescript
+// ❌ BAD - Hardcoded timeout values
+this.notify.error(NOTIFY_CONTACT_ERROR.message, 5000);
+this.notify.success(NOTIFY_CONTACT_ADDED.message, 3000);
+this.notify.warning(NOTIFY_CONTACT_EXISTS.message, 5000);
+this.notify.toast(NOTIFY_URL_COPIED.message, 2000);
+
+// ✅ GOOD - Use timeout constants
+this.notify.error(NOTIFY_CONTACT_ERROR.message, QR_TIMEOUT_LONG);
+this.notify.success(NOTIFY_CONTACT_ADDED.message, QR_TIMEOUT_STANDARD);
+this.notify.warning(NOTIFY_CONTACT_EXISTS.message, QR_TIMEOUT_LONG);
+this.notify.toast(NOTIFY_URL_COPIED.message, QR_TIMEOUT_MEDIUM);
+```
+
+**Add timeout constants to your constants file**:
+```typescript
+// Add to src/constants/notifications.ts
+export const QR_TIMEOUT_SHORT = 1000;      // Short operations
+export const QR_TIMEOUT_MEDIUM = 2000;     // Medium operations  
+export const QR_TIMEOUT_STANDARD = 3000;   // Standard success messages
+export const QR_TIMEOUT_LONG = 5000;       // Error messages and warnings
+```
+
+### 3. Remove Legacy Wrapper Functions
+
+**❌ COMMON MISTAKE**: Keeping legacy notification wrapper functions that are inconsistent with the new system
+
+```typescript
+// ❌ BAD - Legacy wrapper function
+danger(message: string, title: string = "Error", timeout = 5000) {
+  this.notify.error(message, timeout);
+}
+
+// Usage (inconsistent with rest of system)
+this.danger(result.error as string, "Error Setting Visibility");
+
+// ✅ GOOD - Direct usage of notification system
+this.notify.error(result.error as string, QR_TIMEOUT_LONG);
+```
+
+**Why remove legacy wrappers**:
+- Creates inconsistency in the codebase
+- Adds unnecessary abstraction layer
+- Often have unused parameters (like `title` above)
+- Bypasses the centralized notification system benefits
+
+### 4. Extract Long Class Attributes to Computed Properties
+
+**❌ COMMON MISTAKE**: Leaving long class strings in template instead of extracting to computed properties
+
+```typescript
+// ❌ BAD - Long class strings in template
+<template>
+  <div class="bg-amber-200 text-amber-900 border-amber-500 border-dashed border text-center rounded-md overflow-hidden px-4 py-3 my-4">
+    <button class="inline-block text-md uppercase bg-gradient-to-b from-blue-400 to-blue-700 shadow-[inset_0_-1px_0_0_rgba(0,0,0,0.5)] text-white px-4 py-2 rounded-md">
+      Set Name
+    </button>
+  </div>
+</template>
+
+// ✅ GOOD - Extract to computed properties
+<template>
+  <div :class="nameWarningClasses">
+    <button :class="setNameButtonClasses">
+      Set Name
+    </button>
+  </div>
+</template>
+
+// Class methods
+get nameWarningClasses(): string {
+  return "bg-amber-200 text-amber-900 border-amber-500 border-dashed border text-center rounded-md overflow-hidden px-4 py-3 my-4";
+}
+
+get setNameButtonClasses(): string {
+  return "inline-block text-md uppercase bg-gradient-to-b from-blue-400 to-blue-700 shadow-[inset_0_-1px_0_0_rgba(0,0,0,0.5)] text-white px-4 py-2 rounded-md";
+}
+```
+
+**Benefits of extracting long classes**:
+- Improves template readability
+- Enables reusability of styles
+- Makes testing easier
+- Allows for dynamic class computation
+
+### 5. Ensure ALL Literal Strings Use Constants
+
+**❌ COMMON MISTAKE**: Converting `$notify()` calls to helpers but not replacing literal strings with constants
+
+```typescript
+// ❌ BAD - Literal strings in notification calls
+this.notify.error("This QR code does not contain valid contact information.");
+this.notify.warning("The contact DID is missing.");
+this.notify.success("Registration submitted...");
+
+// ✅ GOOD - Use constants for all static messages
+this.notify.error(NOTIFY_QR_INVALID_QR_CODE.message);
+this.notify.warning(NOTIFY_QR_MISSING_DID.message);
+this.notify.success(NOTIFY_QR_REGISTRATION_SUBMITTED.message);
+```
+
+**Add constants for ALL static messages**:
+```typescript
+// Add to src/constants/notifications.ts
+export const NOTIFY_QR_INVALID_QR_CODE = {
+  message: "This QR code does not contain valid contact information.",
+};
+
+export const NOTIFY_QR_MISSING_DID = {
+  message: "The contact DID is missing.",
+};
+
+export const NOTIFY_QR_REGISTRATION_SUBMITTED = {
+  message: "Registration submitted...",
+};
+```
+
+### 6. Validation Checklist for Omissions
+
+**Before marking migration complete, verify these items**:
+
+```bash
+# Check for unused imports
+grep -n "import.*NOTIFY_" src/views/YourComponent.vue
+# Then verify each imported constant is actually used in the file
+
+# Check for hardcoded timeouts
+grep -n "notify\.[a-z]*(" src/views/YourComponent.vue | grep -E "[0-9]{3,4}"
+
+# Check for legacy wrapper functions
+grep -n "danger\|success\|warning\|info.*(" src/views/YourComponent.vue | grep -v "notify\."
+
+# Check for long class attributes (>50 chars)
+grep -n "class=\"[^\"]\{50,\}" src/views/YourComponent.vue
+
+# Check for literal strings in notifications
+grep -n "notify\.[a-z]*(" src/views/YourComponent.vue | grep -v "NOTIFY_\|message"
+```
+
+### 7. Post-Migration Cleanup Commands
+
+**Run these commands after migration to catch omissions**:
+
+```bash
+# Check TypeScript compilation
+npm run lint-fix
+
+# Run validation scripts
+scripts/validate-migration.sh
+scripts/validate-notification-completeness.sh
+
+# Check for any remaining databaseUtil references
+grep -r "databaseUtil" src/views/YourComponent.vue
+
+# Check for any remaining $notify calls
+grep -r "\$notify(" src/views/YourComponent.vue
+```
+
 ## Template Logic Streamlining

 ### Move Complex Template Logic to Class
@@ -432,6 +618,8 @@ get itemCoordinates() {
 - [ ] **Hardcoded timeouts replaced with `TIMEOUTS` constants**
 - [ ] **Static messages use notification constants from `@/constants/notifications`**
 - [ ] **Dynamic messages use literal strings appropriately**
+- [ ] **Unused notification constants removed from imports but these can mean that notifications have been overlooked**
+- [ ] **Legacy wrapper functions removed (e.g., `danger()`, `success()`, etc.)**

 ### Final Validation
 - [ ] Error handling includes component name context