Fix code review issues: RecordId conversion, CLOSE handling, race conditions, and unused imports

Co-authored-by: stritti <184547+stritti@users.noreply.github.com>

Author: Copilot

src/app/component/event-card-list/event-card-list.component.ts

@@ -52,12 +52,18 @@ export class EventCardListComponent {
   constructor() {
     // Effect to trigger data loading when inputs change
     // PERFORMANCE FIX: Don't use async in effects, use queueMicrotask instead
+    // Re-read signals inside microtask to avoid stale values
     effect(() => {
-      const loc = this.location()
-      const currentId = this.currentEventId()
+      // Trigger the effect when inputs change
+      this.location()
+      this.currentEventId()
 
       // Queue the async work to avoid blocking the effect
       queueMicrotask(() => {
+        // Re-read the signals to get current values
+        const loc = this.location()
+        const currentId = this.currentEventId()
+        
         if (loc && currentId) {
           void this.loadEventsFromLocation(loc, currentId)
         } else if (!loc && !currentId) {

src/app/component/event-card/event-card.component.ts

@@ -1,4 +1,4 @@
-import { ChangeDetectionStrategy, Component, input, signal, effect, inject, DestroyRef, computed } from '@angular/core'
+import { ChangeDetectionStrategy, Component, input, signal, effect, inject, computed } from '@angular/core'
 import { CommonModule } from '@angular/common'
 import { RouterModule } from '@angular/router'
 import { Event, EventType } from '../../models/event.interface'
@@ -38,7 +38,6 @@ export class EventCardComponent {
   private readonly locationService = inject(LocationService)
   private readonly localStorageService = inject(LocalStorageService)
   private readonly mediaService = inject(MediaService)
-  private readonly destroyRef = inject(DestroyRef)
 
   // Local state as signals
   protected readonly location = signal<Location | null>(null)
@@ -67,7 +66,7 @@ export class EventCardComponent {
       const ev = this.event()
       if (ev?.id) {
         this.resetResolved()
-        const id = ev.id as unknown as string
+        const id = this.surrealDBService.recordIdToString(ev.id)
         this.isSaved.set(this.localStorageService.isEventSaved(id))
         // Use queueMicrotask to avoid blocking the main thread
         queueMicrotask(() => {
@@ -84,7 +83,7 @@ export class EventCardComponent {
     effect(() => {
       this.localStorageService.savedEventsSignal()
       const ev = this.event()
-      const id = (ev?.id as unknown as string) ?? null
+      const id = ev?.id ? this.surrealDBService.recordIdToString(ev.id) : null
       this.isSaved.set(id ? this.localStorageService.isEventSaved(id) : false)
     })
   }

src/app/component/event-type-pill/event-type-pill.component.ts

@@ -28,6 +28,9 @@ export class EventTypePillComponent {
 
   // Local state
   protected readonly pill = signal<Pill | null>(null)
+  
+  // Version counter to prevent race conditions
+  private buildVersion = 0
 
   constructor() {
     // Effect to rebuild pill when event changes
@@ -47,7 +50,16 @@ export class EventTypePillComponent {
       return
     }
 
+    // Increment version to invalidate any in-flight requests
+    const currentVersion = ++this.buildVersion
+
     const allEventType = await this.eventService.getAllEventTypes()
+    
+    // Check if this is still the latest request
+    if (currentVersion !== this.buildVersion) {
+      return // Ignore stale response
+    }
+
     const eventType = allEventType.find((et) => et.id.id === event.event_type?.id)
 
     if (!eventType?.name) {

src/app/component/snack-bar/snack-bar.component.ts

@@ -85,6 +85,7 @@ export class SnackBarComponent {
     }
     this.visible.set(false)
   }
+
   getTypeClass(): string {
     switch (this.type()) {
       case 'success':
@@ -112,8 +113,4 @@ export class SnackBarComponent {
         return 'M8 16A8 8 0 108 0a8 8 0 000 16zm1-11a1 1 0 10-2 0v4a1 1 0 102 0V5zm-1 9a1 1 0 100-2 1 1 0 000 2z'
     }
   }
-
-  close(): void {
-    this.visible.set(false)
-  }
 }

src/app/pages/favourites/favourites.component.ts

@@ -1,7 +1,7 @@
 import { ChangeDetectionStrategy, Component, OnInit, inject, effect, signal } from '@angular/core'
 
 import { Event } from '../../models/event.interface'
-import { Router, RouterLink } from '@angular/router'
+import { RouterLink } from '@angular/router'
 import { FavoriteService } from '../../services/favorite.service'
 import { EventCardComponent } from '../../component/event-card/event-card.component'
 import { TranslateModule } from '@ngx-translate/core'
@@ -29,7 +29,6 @@ export class FavouritesComponent implements OnInit {
   protected readonly loading = signal(true)
 
   private readonly favoriteService = inject(FavoriteService)
-  private readonly router = inject(Router)
 
   constructor() {
     // Effect um auf Änderungen zu reagieren

src/app/services/surrealdb.service.ts

@@ -35,7 +35,11 @@ export class SurrealdbService extends Surreal {
     super()
   }
 
-  private recordIdToString(recordId: RecordId<string> | StringRecordId): string {
+  /**
+   * Convert a RecordId to a string representation.
+   * Public method to be used by components for consistent RecordId handling.
+   */
+  recordIdToString(recordId: RecordId<string> | StringRecordId): string {
     const asString = (recordId as unknown as { toString?: () => string })?.toString?.()
     if (typeof asString === 'string') {
       return asString
@@ -312,9 +316,19 @@ export class SurrealdbService extends Surreal {
       const queryUuid = await super.live<T>(
         table,
         (action, result) => {
-          const update: LiveQueryUpdate<T> = { 
-            action: action as 'CREATE' | 'UPDATE' | 'DELETE',
-            result: result as T
+          // Handle all possible actions including CLOSE
+          let update: LiveQueryUpdate<T>
+          
+          if (action === 'CLOSE') {
+            update = { action: 'CLOSE' }
+            // Clean up on CLOSE
+            this.liveQueryCallbacks.delete(queryKey)
+            this.liveQueryUuids.delete(queryKey)
+          } else {
+            update = { 
+              action: action as 'CREATE' | 'UPDATE' | 'DELETE',
+              result: result as T
+            }
           }
 
           // Notify all callbacks for this query
@@ -331,6 +345,17 @@ export class SurrealdbService extends Surreal {
       this.liveQueryUuids.set(queryKey, queryUuid)
     } catch (error) {
       console.error('Failed to create live query:', error)
+      // Notify listeners of failure via CLOSE event
+      const update: LiveQueryUpdate<T> = { action: 'CLOSE' }
+      const callbacks = this.liveQueryCallbacks.get(queryKey)
+      if (callbacks) {
+        callbacks.forEach(callback => {
+          (callback as LiveQueryCallback<T>)(update)
+        })
+      }
+      // Clean up
+      this.liveQueryCallbacks.delete(queryKey)
+      this.liveQueryUuids.delete(queryKey)
       throw error
     }
   }