fix: grace period not updated

jimmykane · jimmykane · commit 3ba45b9fd36f · 2026-02-01T10:32:09.000+02:00
diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts
@@ -105,7 +105,7 @@ describe('AppComponent', () => {
                     provide: AppUserService, useValue: {
                         updateUserProperties: vi.fn().mockReturnValue(Promise.resolve()),
                         getSubscriptionRole: vi.fn().mockReturnValue(Promise.resolve('free')),
-                        getGracePeriodUntil: vi.fn().mockReturnValue(of(null)),
+                        gracePeriodUntil: signal(null),
                         isAdmin: vi.fn().mockReturnValue(Promise.resolve(false))
                     }
                 },
diff --git a/src/app/components/grace-period-banner/grace-period-banner.component.html b/src/app/components/grace-period-banner/grace-period-banner.component.html
@@ -1,4 +1,4 @@
-<ng-container *ngIf="(gracePeriodUntil$ | async) as gracePeriodUntil">
+<ng-container *ngIf="gracePeriodUntil() as gracePeriodUntil">
     <div #bannerElement *ngIf="!isDismissed" class="grace-period-banner mat-typography">
         <mat-icon class="banner-warning-icon">warning</mat-icon>
         <span class="banner-text">
diff --git a/src/app/components/grace-period-banner/grace-period-banner.component.spec.ts b/src/app/components/grace-period-banner/grace-period-banner.component.spec.ts
@@ -3,6 +3,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest';
 import { GracePeriodBannerComponent } from './grace-period-banner.component';
 import { AppUserService } from '../../services/app.user.service';
 import { of } from 'rxjs';
+import { signal } from '@angular/core';
 import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
 import { RouterTestingModule } from '@angular/router/testing';
 
@@ -11,7 +12,7 @@ describe('GracePeriodBannerComponent', () => {
     let fixture: ComponentFixture<GracePeriodBannerComponent>;
 
     const mockUserService = {
-        getGracePeriodUntil: vi.fn().mockReturnValue(of(null))
+        gracePeriodUntil: signal<Date | null>(null)
     };
 
     beforeEach(async () => {
@@ -41,9 +42,8 @@ describe('GracePeriodBannerComponent', () => {
 
     it('should show banner when grace period date is present', async () => {
         const mockDate = new Date();
-        mockUserService.getGracePeriodUntil.mockReturnValue(of(mockDate));
+        mockUserService.gracePeriodUntil.set(mockDate);
 
-        component.ngOnInit();
         fixture.detectChanges();
         await fixture.whenStable();
 
@@ -53,9 +53,8 @@ describe('GracePeriodBannerComponent', () => {
     });
 
     it('should hide banner when grace period date is null', () => {
-        mockUserService.getGracePeriodUntil.mockReturnValue(of(null));
+        mockUserService.gracePeriodUntil.set(null);
 
-        component.ngOnInit();
         fixture.detectChanges();
 
         const banner = fixture.nativeElement.querySelector('.grace-period-banner');
@@ -64,9 +63,8 @@ describe('GracePeriodBannerComponent', () => {
 
     it('should hide banner when dismissed', async () => {
         const mockDate = new Date();
-        mockUserService.getGracePeriodUntil.mockReturnValue(of(mockDate));
+        mockUserService.gracePeriodUntil.set(mockDate);
 
-        component.ngOnInit();
         fixture.detectChanges();
         await fixture.whenStable();
 
diff --git a/src/app/components/grace-period-banner/grace-period-banner.component.ts b/src/app/components/grace-period-banner/grace-period-banner.component.ts
@@ -1,4 +1,4 @@
-import { Component, OnInit, Output, EventEmitter, ViewChild, ElementRef, AfterViewInit, OnDestroy } from '@angular/core';
+import { Component, OnInit, Output, EventEmitter, ViewChild, ElementRef, AfterViewInit, OnDestroy, effect, inject } from '@angular/core';
 import { Observable, Subscription } from 'rxjs';
 import { AppUserService } from '../../services/app.user.service';
 import { LoggerService } from '../../services/logger.service';
@@ -29,21 +29,18 @@ export class GracePeriodBannerComponent implements OnInit, AfterViewInit, OnDest
         return this._bannerElement;
     }
 
-    gracePeriodUntil$!: Observable<Date | null>;
+    private userService = inject(AppUserService);
+    private logger = inject(LoggerService);
+
+    gracePeriodUntil = this.userService.gracePeriodUntil;
     isDismissed = false;
     private subscription: Subscription | null = null;
     private resizeObserver: ResizeObserver | null = null;
 
-    constructor(private userService: AppUserService, private logger: LoggerService) { }
-
-    ngOnInit(): void {
-        this.logger.info('[GracePeriodBanner] ngOnInit - Getting grace period observable');
-        this.gracePeriodUntil$ = this.userService.getGracePeriodUntil();
-    }
-
-    ngAfterViewInit(): void {
-        // Subscribe to grace period changes to update height when banner appears
-        this.subscription = this.gracePeriodUntil$.subscribe(() => {
+    constructor() {
+        effect(() => {
+            const date = this.gracePeriodUntil();
+            this.logger.info('[GracePeriodBanner] Effect triggered - gracePeriodUntil:', date);
             // Use setTimeout to ensure the DOM is updated before measuring
             setTimeout(() => {
                 this.updateHeight();
@@ -52,6 +49,18 @@ export class GracePeriodBannerComponent implements OnInit, AfterViewInit, OnDest
         });
     }
 
+    ngOnInit(): void {
+        this.logger.info('[GracePeriodBanner] ngOnInit - Signal already initialized in service');
+    }
+
+    ngAfterViewInit(): void {
+        // Use an effect or similar to update height when signal changes
+        // Since we are in EXECUTION and the component might be simple, 
+        // using effect() in constructor is cleaner for signals.
+        // But for this refactor, I'll just change the subscription to an effect-like behavior if needed.
+        // Actually, let's use effect in the constructor for the height update.
+    }
+
     ngOnDestroy(): void {
         this.subscription?.unsubscribe();
         this.resizeObserver?.disconnect();
diff --git a/src/app/services/app.user.service.spec.ts b/src/app/services/app.user.service.spec.ts
@@ -20,10 +20,11 @@ vi.mock('@angular/fire/auth', async (importOriginal) => {
 
 vi.mock('@angular/fire/firestore', async (importOriginal) => {
     const actual: any = await importOriginal();
+    const { of } = await import('rxjs');
     return {
         ...actual,
         doc: vi.fn().mockReturnValue({}),
-        docData: vi.fn(),
+        docData: vi.fn().mockReturnValue(of({})),
         setDoc: vi.fn().mockResolvedValue(undefined),
         updateDoc: vi.fn().mockResolvedValue(undefined),
     };
@@ -63,10 +64,10 @@ describe('AppUserService', () => {
                 { provide: AppWindowService, useValue: {} }
             ]
         });
-        service = TestBed.inject(AppUserService);
     });
 
     it('should be created', () => {
+        service = TestBed.inject(AppUserService);
         expect(service).toBeTruthy();
     });
 
@@ -79,6 +80,7 @@ describe('AppUserService', () => {
 
             // Note: because authState is mocked to return the user, we need to ensure firstValueFrom works
             // But AppUserService.getSubscriptionRole uses authState(this.auth)
+            service = TestBed.inject(AppUserService);
         });
 
         it('should return basic role', async () => {
@@ -108,43 +110,47 @@ describe('AppUserService', () => {
             mockAuth.currentUser.getIdTokenResult.mockReturnValue(Promise.resolve({
                 claims: { stripeRole: 'pro' }
             }));
+            service = TestBed.inject(AppUserService);
             const hasAccess = await service.hasPaidAccess();
             expect(hasAccess).toBe(true);
         });
     });
 
-    describe('getGracePeriodUntil', () => {
+    describe('gracePeriodUntil signal', () => {
         it('should return null if user is not logged in', async () => {
-            mockAuth.currentUser = null;
-            const res = await firstValueFrom(service.getGracePeriodUntil());
-            expect(res).toBeNull();
+            (authState as any).mockReturnValue(of(null));
+            service = TestBed.inject(AppUserService);
+            expect(service.gracePeriodUntil()).toBeNull();
         });
 
         it('should return null if no grace period is set', async () => {
-            mockAuth.currentUser = {
+            (authState as any).mockReturnValue(of({
                 uid: 'u1',
                 getIdTokenResult: vi.fn().mockResolvedValue({ claims: {} })
-            };
+            }));
             (docData as any).mockReturnValue(of({}));
-            const res = await firstValueFrom(service.getGracePeriodUntil());
-            expect(res).toBeNull();
+            service = TestBed.inject(AppUserService);
+            expect(service.gracePeriodUntil()).toBeNull();
         });
 
         it('should return Date if grace period is set', async () => {
             const mockDate = new Date();
-            mockAuth.currentUser = {
+            (authState as any).mockReturnValue(of({
                 uid: 'u1',
                 getIdTokenResult: vi.fn().mockResolvedValue({ claims: {} })
-            };
+            }));
             (docData as any).mockReturnValue(of({
                 gracePeriodUntil: { toDate: () => mockDate }
             }));
 
-            const res = await firstValueFrom(service.getGracePeriodUntil());
-            expect(res).toEqual(mockDate);
+            service = TestBed.inject(AppUserService);
+            expect(service.gracePeriodUntil()).toEqual(mockDate);
         });
     });
     describe('updateUserProperties', () => {
+        beforeEach(() => {
+            service = TestBed.inject(AppUserService);
+        });
         it('should split settings and other properties', async () => {
             const user = { uid: 'u1' } as any;
             const settings = { theme: 'dark' };
@@ -216,6 +222,9 @@ describe('AppUserService', () => {
     });
 
     describe('static user role checks', () => {
+        beforeEach(() => {
+            service = TestBed.inject(AppUserService);
+        });
         const mockUser = { uid: 'u1' } as any;
 
         describe('isProUser', () => {
@@ -302,6 +311,9 @@ describe('AppUserService', () => {
         });
     });
     describe('deleteAllUserData', () => {
+        beforeEach(() => {
+            service = TestBed.inject(AppUserService);
+        });
         it('should call deleteSelf cloud function and sign out', async () => {
             await service.deleteAllUserData({ uid: 'u1' } as any);
 
@@ -319,7 +331,10 @@ describe('AppUserService', () => {
         });
     });
 
-    describe('Service Integrations', () => {
+    describe('Service Integration', () => {
+        beforeEach(() => {
+            service = TestBed.inject(AppUserService);
+        });
         const startDate = new Date('2023-01-01');
         const endDate = new Date('2023-01-31');
 
diff --git a/src/app/services/app.user.service.ts b/src/app/services/app.user.service.ts
@@ -1,12 +1,13 @@
 import { inject, Injectable, OnDestroy, EnvironmentInjector, runInInjectionContext } from '@angular/core';
+import { toSignal } from '@angular/core/rxjs-interop';
 
 
 import { Observable, from, firstValueFrom, of, combineLatest, distinctUntilChanged } from 'rxjs';
 import { StripeRole } from '../models/stripe-role.model';
 import { User } from '@sports-alliance/sports-lib';
 import { Privacy } from '@sports-alliance/sports-lib';
 import { AppEventService } from './app.event.service';
-import { catchError, map, take } from 'rxjs/operators';
+import { catchError, map, take, switchMap } from 'rxjs/operators';
 import {
   AppThemes,
   UserAppSettingsInterface
@@ -99,6 +100,33 @@ export class AppUserService implements OnDestroy {
   private auth = inject(Auth);
   private functionsService = inject(AppFunctionsService);
   private injector = inject(EnvironmentInjector);
+  private logger = inject(LoggerService);
+  private eventService = inject(AppEventService);
+  private http = inject(HttpClient);
+  private windowService = inject(AppWindowService);
+
+  public readonly gracePeriodUntil = toSignal(
+    authState(this.auth).pipe(
+      switchMap(user => {
+        this.logger.log('[AppUserService] gracePeriodUntil signal init - Current auth user:', user?.uid || 'null');
+        if (!user) return of(null);
+        const systemDoc = doc(this.firestore, `users/${user.uid}/system/status`);
+        return docData(systemDoc).pipe(
+          map((systemData: any) => {
+            if (systemData?.gracePeriodUntil) {
+              return (systemData.gracePeriodUntil as any).toDate();
+            }
+            return null;
+          }),
+          catchError((error) => {
+            this.logger.error('[AppUserService] gracePeriodUntil signal error:', error);
+            return of(null);
+          })
+        );
+      })
+    ),
+    { initialValue: null, injector: this.injector }
+  );
 
   static getDefaultChartTheme(): ChartThemes {
     return ChartThemes.Material;
@@ -361,12 +389,7 @@ export class AppUserService implements OnDestroy {
     return AppUserService.isProUser(user, isAdmin) || AppUserService.isBasicUser(user);
   }
 
-  constructor(
-    private eventService: AppEventService,
-    private http: HttpClient,
-    private windowService: AppWindowService,
-    private logger: LoggerService
-  ) {
+  constructor() {
     authState(this.auth).subscribe((user) => {
       if (user) {
         this.logger.setUser({ id: user.uid, email: user.email || undefined });
@@ -761,33 +784,6 @@ export class AppUserService implements OnDestroy {
     return role === 'pro' || role === 'basic';
   }
 
-  public getGracePeriodUntil(): Observable<Date | null> {
-    const user = this.auth.currentUser;
-    this.logger.log('[AppUserService] getGracePeriodUntil - Current auth user:', user?.uid || 'null');
-    if (!user) return from([null]);
-
-    return runInInjectionContext(this.injector, () => {
-      // Logic refactored: gracePeriodUntil is now in system/status and merged onto user
-      // so this can technically just call getUserByID, but that's heavy.
-      // Let's read directly from system/status for efficiency
-      const systemDoc = doc(this.firestore, `users/${user.uid}/system/status`);
-      return docData(systemDoc).pipe(
-        map((systemData: any) => {
-          if (systemData?.gracePeriodUntil) {
-            // Firebase Timestamp to Date
-            const date = (systemData.gracePeriodUntil as any).toDate();
-            // this.logger.log('[AppUserService] getGracePeriodUntil - Returning grace period date:', date);
-            return date;
-          }
-          return null;
-        }),
-        catchError((error) => {
-          this.logger.error('[AppUserService] getGracePeriodUntil - Error fetching system document:', error);
-          return from([null]);
-        })
-      );
-    });
-  }
 
   public async deleteAllUserData(_user: User) {
     try {

Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ describe('AppComponent', () => {`
`105`	`105`	`provide: AppUserService, useValue: {`
`106`	`106`	`updateUserProperties: vi.fn().mockReturnValue(Promise.resolve()),`
`107`	`107`	`getSubscriptionRole: vi.fn().mockReturnValue(Promise.resolve('free')),`
`108`		`- getGracePeriodUntil: vi.fn().mockReturnValue(of(null)),`
	`108`	`+ gracePeriodUntil: signal(null),`
`109`	`109`	`isAdmin: vi.fn().mockReturnValue(Promise.resolve(false))`
`110`	`110`	`}`
`111`	`111`	`},`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-<ng-container *ngIf="(gracePeriodUntil$ \| async) as gracePeriodUntil">`
	`1`	`+<ng-container *ngIf="gracePeriodUntil() as gracePeriodUntil">`
`2`	`2`	`<div #bannerElement *ngIf="!isDismissed" class="grace-period-banner mat-typography">`
`3`	`3`	`<mat-icon class="banner-warning-icon">warning</mat-icon>`
`4`	`4`	`<span class="banner-text">`