From 568c4cc8f34bc0efa2ee1d197352c0ac03249f66 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Germ=C3=A1n=20Mart=C3=ADn?= <github@gmartin.net>
Date: Sun, 28 Sep 2025 23:47:28 +0200
Subject: [PATCH] Refactor access management: update access handling and
 permissions, enhance UI interactions for editing access

---
 apps/api/src/app/access/access.controller.ts  | 45 ++++-----
 apps/api/src/app/access/access.service.ts     | 15 ++-
 apps/api/src/app/access/update-access.dto.ts  |  3 +
 .../access-table/access-table.component.html  | 15 +--
 .../access-table/access-table.component.ts    |  6 +-
 ...reate-or-update-access-dialog.component.ts | 99 +++++++++----------
 .../create-or-update-access-dialog.html       |  8 +-
 .../interfaces/interfaces.ts                  |  1 -
 .../user-account-access.component.ts          | 68 ++++++-------
 .../user-account-access.html                  |  2 +-
 apps/client/src/app/services/data.service.ts  | 12 +--
 libs/common/src/lib/permissions.ts            |  3 +
 12 files changed, 135 insertions(+), 142 deletions(-)

diff --git a/apps/api/src/app/access/access.controller.ts b/apps/api/src/app/access/access.controller.ts
index 372c7f34a..836bd49c0 100644
--- a/apps/api/src/app/access/access.controller.ts
+++ b/apps/api/src/app/access/access.controller.ts
@@ -34,21 +34,6 @@ export class AccessController {
     @Inject(REQUEST) private readonly request: RequestWithUser
   ) {}
 
-  @Get(':id')
-  @UseGuards(AuthGuard('jwt'), HasPermissionGuard)
-  public async getAccess(@Param('id') id: string): Promise<AccessModel> {
-    const access = await this.accessService.access({ id });
-
-    if (!access || access.userId !== this.request.user.id) {
-      throw new HttpException(
-        getReasonPhrase(StatusCodes.FORBIDDEN),
-        StatusCodes.FORBIDDEN
-      );
-    }
-
-    return access;
-  }
-
   @Get()
   @UseGuards(AuthGuard('jwt'), HasPermissionGuard)
   public async getAllAccesses(): Promise<Access[]> {
@@ -56,7 +41,10 @@ export class AccessController {
       include: {
         granteeUser: true
       },
-      orderBy: { granteeUserId: 'asc' },
+      orderBy: [
+        { granteeUserId: 'desc' }, // NULL values first (public access), then user IDs
+        { createdAt: 'asc' } // Within each group, order by creation time
+      ],
       where: { userId: this.request.user.id }
     });
 
@@ -120,9 +108,12 @@ export class AccessController {
   @HasPermission(permissions.deleteAccess)
   @UseGuards(AuthGuard('jwt'), HasPermissionGuard)
   public async deleteAccess(@Param('id') id: string): Promise<AccessModel> {
-    const access = await this.accessService.access({ id });
+    const originalAccess = await this.accessService.access({
+      id,
+      userId: this.request.user.id
+    });
 
-    if (!access || access.userId !== this.request.user.id) {
+    if (!originalAccess) {
       throw new HttpException(
         getReasonPhrase(StatusCodes.FORBIDDEN),
         StatusCodes.FORBIDDEN
@@ -134,6 +125,7 @@ export class AccessController {
     });
   }
 
+  @HasPermission(permissions.updateAccess)
   @Put(':id')
   @UseGuards(AuthGuard('jwt'), HasPermissionGuard)
   public async updateAccess(
@@ -150,9 +142,12 @@ export class AccessController {
       );
     }
 
-    const access = await this.accessService.access({ id });
+    const originalAccess = await this.accessService.access({
+      id,
+      userId: this.request.user.id
+    });
 
-    if (!access || access.userId !== this.request.user.id) {
+    if (!originalAccess) {
       throw new HttpException(
         getReasonPhrase(StatusCodes.FORBIDDEN),
         StatusCodes.FORBIDDEN
@@ -160,16 +155,16 @@ export class AccessController {
     }
 
     try {
-      return this.accessService.updateAccess(
-        { id },
-        {
+      return this.accessService.updateAccess({
+        data: {
           alias: data.alias,
           granteeUser: data.granteeUserId
             ? { connect: { id: data.granteeUserId } }
             : { disconnect: true },
           permissions: data.permissions
-        }
-      );
+        },
+        where: { id }
+      });
     } catch {
       throw new HttpException(
         getReasonPhrase(StatusCodes.BAD_REQUEST),
diff --git a/apps/api/src/app/access/access.service.ts b/apps/api/src/app/access/access.service.ts
index 995e27ae9..4c4534811 100644
--- a/apps/api/src/app/access/access.service.ts
+++ b/apps/api/src/app/access/access.service.ts
@@ -25,7 +25,9 @@ export class AccessService {
     take?: number;
     cursor?: Prisma.AccessWhereUniqueInput;
     where?: Prisma.AccessWhereInput;
-    orderBy?: Prisma.AccessOrderByWithRelationInput;
+    orderBy?:
+      | Prisma.AccessOrderByWithRelationInput
+      | Prisma.AccessOrderByWithRelationInput[];
   }): Promise<AccessWithGranteeUser[]> {
     const { include, skip, take, cursor, where, orderBy } = params;
 
@@ -53,10 +55,13 @@ export class AccessService {
     });
   }
 
-  public async updateAccess(
-    where: Prisma.AccessWhereUniqueInput,
-    data: Prisma.AccessUpdateInput
-  ): Promise<Access> {
+  public async updateAccess({
+    data,
+    where
+  }: {
+    data: Prisma.AccessUpdateInput;
+    where: Prisma.AccessWhereUniqueInput;
+  }): Promise<Access> {
     return this.prismaService.access.update({
       where,
       data
diff --git a/apps/api/src/app/access/update-access.dto.ts b/apps/api/src/app/access/update-access.dto.ts
index 5e8799747..26b6df687 100644
--- a/apps/api/src/app/access/update-access.dto.ts
+++ b/apps/api/src/app/access/update-access.dto.ts
@@ -2,6 +2,9 @@ import { AccessPermission } from '@prisma/client';
 import { IsEnum, IsOptional, IsString, IsUUID } from 'class-validator';
 
 export class UpdateAccessDto {
+  @IsString()
+  id: string;
+
   @IsOptional()
   @IsString()
   alias?: string;
diff --git a/apps/client/src/app/components/access-table/access-table.component.html b/apps/client/src/app/components/access-table/access-table.component.html
index 71e01f180..ded172acd 100644
--- a/apps/client/src/app/components/access-table/access-table.component.html
+++ b/apps/client/src/app/components/access-table/access-table.component.html
@@ -65,12 +65,14 @@
           <ion-icon name="ellipsis-horizontal" />
         </button>
         <mat-menu #transactionMenu="matMenu" xPosition="before">
-          <button mat-menu-item (click)="onEditAccess(element.id)">
-            <span class="align-items-center d-flex">
-              <ion-icon class="mr-2" name="create-outline" />
-              <span i18n>Edit</span>
-            </span>
-          </button>
+          @if (user?.settings?.isExperimentalFeatures) {
+            <button mat-menu-item (click)="onUpdateAccess(element.id)">
+              <span class="align-items-center d-flex">
+                <ion-icon class="mr-2" name="create-outline" />
+                <span i18n>Edit</span>
+              </span>
+            </button>
+          }
           @if (element.type === 'PUBLIC') {
             <button mat-menu-item (click)="onCopyUrlToClipboard(element.id)">
               <span class="align-items-center d-flex">
@@ -79,7 +81,6 @@
               </span>
             </button>
           }
-          <hr class="my-0" />
           <button mat-menu-item (click)="onDeleteAccess(element.id)">
             <span class="align-items-center d-flex">
               <ion-icon class="mr-2" name="remove-circle-outline" />
diff --git a/apps/client/src/app/components/access-table/access-table.component.ts b/apps/client/src/app/components/access-table/access-table.component.ts
index 19eb61d10..645d43253 100644
--- a/apps/client/src/app/components/access-table/access-table.component.ts
+++ b/apps/client/src/app/components/access-table/access-table.component.ts
@@ -54,7 +54,7 @@ export class GfAccessTableComponent implements OnChanges {
   @Input() user: User;
 
   @Output() accessDeleted = new EventEmitter<string>();
-  @Output() accessEdited = new EventEmitter<string>();
+  @Output() accessToUpdate = new EventEmitter<string>();
 
   public baseUrl = window.location.origin;
   public dataSource: MatTableDataSource<Access>;
@@ -116,7 +116,7 @@ export class GfAccessTableComponent implements OnChanges {
     });
   }
 
-  public onEditAccess(aId: string) {
-    this.accessEdited.emit(aId);
+  public onUpdateAccess(aId: string) {
+    this.accessToUpdate.emit(aId);
   }
 }
diff --git a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts
index 84d62c3f9..1937d0930 100644
--- a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts
+++ b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.component.ts
@@ -49,9 +49,9 @@ import { CreateOrUpdateAccessDialogParams } from './interfaces/interfaces';
   styleUrls: ['./create-or-update-access-dialog.scss'],
   templateUrl: 'create-or-update-access-dialog.html'
 })
-export class GfCreateOrUpdateAccessDialog implements OnInit, OnDestroy {
+export class GfCreateOrUpdateAccessDialog implements OnDestroy, OnInit {
   public accessForm: FormGroup;
-  public isEditMode: boolean;
+  public mode: 'create' | 'update';
 
   private unsubscribeSubject = new Subject<void>();
 
@@ -63,44 +63,11 @@ export class GfCreateOrUpdateAccessDialog implements OnInit, OnDestroy {
     private formBuilder: FormBuilder,
     private notificationService: NotificationService
   ) {
-    this.isEditMode = !!data.accessId;
+    this.mode = this.data.access?.id ? 'update' : 'create';
   }
 
-  private async createAccess() {
-    console.log('Creating access...');
-    const access: CreateAccessDto = {
-      alias: this.accessForm.get('alias').value,
-      granteeUserId: this.accessForm.get('granteeUserId').value,
-      permissions: [this.accessForm.get('permissions').value]
-    };
-
-    try {
-      await validateObjectForForm({
-        classDto: CreateAccessDto,
-        form: this.accessForm,
-        object: access
-      });
-
-      this.dataService
-        .postAccess(access)
-        .pipe(
-          catchError((error) => {
-            if (error.status === StatusCodes.BAD_REQUEST) {
-              this.notificationService.alert({
-                title: $localize`Oops! Could not grant access.`
-              });
-            }
-
-            return EMPTY;
-          }),
-          takeUntil(this.unsubscribeSubject)
-        )
-        .subscribe(() => {
-          this.dialogRef.close(access);
-        });
-    } catch (error) {
-      console.error(error);
-    }
+  public onCancel() {
+    this.dialogRef.close();
   }
 
   public ngOnDestroy() {
@@ -114,7 +81,7 @@ export class GfCreateOrUpdateAccessDialog implements OnInit, OnDestroy {
       granteeUserId: [this.data.access.grantee, Validators.required],
       permissions: [this.data.access.permissions[0], Validators.required],
       type: [
-        { value: this.data.access.type, disabled: this.isEditMode },
+        { value: this.data.access.type, disabled: this.mode === 'update' },
         Validators.required
       ]
     });
@@ -145,34 +112,58 @@ export class GfCreateOrUpdateAccessDialog implements OnInit, OnDestroy {
     }
   }
 
-  public onCancel() {
-    this.dialogRef.close();
-  }
-
   public async onSubmit() {
-    if (!this.accessForm.valid) {
-      console.error('Form is invalid:', this.accessForm.errors);
-      return;
-    }
-
-    if (this.isEditMode) {
+    if (this.mode === 'update') {
       await this.updateAccess();
     } else {
       await this.createAccess();
     }
   }
 
+  private async createAccess() {
+    const access: CreateAccessDto = {
+      alias: this.accessForm.get('alias').value,
+      granteeUserId: this.accessForm.get('granteeUserId').value,
+      permissions: [this.accessForm.get('permissions').value]
+    };
+
+    try {
+      await validateObjectForForm({
+        classDto: CreateAccessDto,
+        form: this.accessForm,
+        object: access
+      });
+
+      this.dataService
+        .postAccess(access)
+        .pipe(
+          catchError((error) => {
+            if (error.status === StatusCodes.BAD_REQUEST) {
+              this.notificationService.alert({
+                title: $localize`Oops! Could not grant access.`
+              });
+            }
+
+            return EMPTY;
+          }),
+          takeUntil(this.unsubscribeSubject)
+        )
+        .subscribe(() => {
+          this.dialogRef.close(access);
+        });
+    } catch (error) {
+      console.error(error);
+    }
+  }
+
   private async updateAccess() {
-    console.log('Updating access...');
     const access: UpdateAccessDto = {
+      id: this.data.access.id,
       alias: this.accessForm.get('alias').value,
       granteeUserId: this.accessForm.get('granteeUserId').value,
       permissions: [this.accessForm.get('permissions').value]
     };
 
-    console.log('Access data:', access);
-    console.log('Access ID:', this.data.accessId);
-
     try {
       await validateObjectForForm({
         classDto: UpdateAccessDto,
@@ -181,7 +172,7 @@ export class GfCreateOrUpdateAccessDialog implements OnInit, OnDestroy {
       });
 
       this.dataService
-        .putAccess(this.data.accessId, access)
+        .putAccess(access)
         .pipe(
           catchError((error) => {
             if (error.status === StatusCodes.BAD_REQUEST) {
diff --git a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.html b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.html
index cf31545d9..940c7b52b 100644
--- a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.html
+++ b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/create-or-update-access-dialog.html
@@ -5,7 +5,7 @@
   (ngSubmit)="onSubmit()"
 >
   <h1 mat-dialog-title>
-    @if (isEditMode) {
+    @if (mode === 'update') {
       <span i18n>Edit access</span>
     } @else {
       <span i18n>Grant access</span>
@@ -73,10 +73,12 @@
       mat-flat-button
       type="submit"
       [disabled]="
-        isEditMode ? !accessForm.valid : !(accessForm.dirty && accessForm.valid)
+        mode === 'update'
+          ? !accessForm.valid
+          : !(accessForm.dirty && accessForm.valid)
       "
     >
-      @if (isEditMode) {
+      @if (mode === 'update') {
         <ng-container i18n>Update</ng-container>
       } @else {
         <ng-container i18n>Save</ng-container>
diff --git a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/interfaces/interfaces.ts b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/interfaces/interfaces.ts
index 653ee66fd..b7850fb38 100644
--- a/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/interfaces/interfaces.ts
+++ b/apps/client/src/app/components/user-account-access/create-or-update-access-dialog/interfaces/interfaces.ts
@@ -2,5 +2,4 @@ import { Access } from '@ghostfolio/common/interfaces';
 
 export interface CreateOrUpdateAccessDialogParams {
   access: Access;
-  accessId?: string;
 }
diff --git a/apps/client/src/app/components/user-account-access/user-account-access.component.ts b/apps/client/src/app/components/user-account-access/user-account-access.component.ts
index c6a66ed89..55acbe1fc 100644
--- a/apps/client/src/app/components/user-account-access/user-account-access.component.ts
+++ b/apps/client/src/app/components/user-account-access/user-account-access.component.ts
@@ -115,6 +115,8 @@ export class GfUserAccountAccessComponent implements OnDestroy, OnInit {
       .subscribe((params) => {
         if (params['createDialog']) {
           this.openCreateAccessDialog();
+        } else if (params['editDialog']) {
+          this.openUpdateAccessDialog(params['editDialog']);
         }
       });
 
@@ -138,8 +140,8 @@ export class GfUserAccountAccessComponent implements OnDestroy, OnInit {
       });
   }
 
-  public onEditAccess(aId: string) {
-    this.openEditAccessDialog(aId);
+  public onUpdateAccess(aId: string) {
+    this.openUpdateAccessDialog(aId);
   }
 
   public onGenerateAccessToken() {
@@ -204,39 +206,39 @@ export class GfUserAccountAccessComponent implements OnDestroy, OnInit {
     });
   }
 
-  private openEditAccessDialog(accessId: string) {
-    // Fetch the access details first
-    this.dataService
-      .fetchAccess(accessId)
-      .pipe(takeUntil(this.unsubscribeSubject))
-      .subscribe({
-        next: (accessDetails) => {
-          const dialogRef = this.dialog.open(GfCreateOrUpdateAccessDialog, {
-            data: {
-              access: {
-                alias: accessDetails.alias,
-                permissions: accessDetails.permissions,
-                type: accessDetails.granteeUser ? 'PRIVATE' : 'PUBLIC',
-                grantee: accessDetails.granteeUser?.id || null
-              },
-              accessId: accessId
-            },
-            height: this.deviceType === 'mobile' ? '98vh' : undefined,
-            width: this.deviceType === 'mobile' ? '100vw' : '50rem'
-          });
+  private openUpdateAccessDialog(accessId: string) {
+    // Find the access details in the already loaded data
+    const accessDetails = this.accessesGive.find(
+      (access) => access.id === accessId
+    );
 
-          dialogRef.afterClosed().subscribe((result) => {
-            if (result) {
-              this.update();
-            }
-          });
-        },
-        error: () => {
-          this.notificationService.alert({
-            title: $localize`Oops! Could not load access details.`
-          });
-        }
+    if (!accessDetails) {
+      this.notificationService.alert({
+        title: $localize`Oops! Could not find access details.`
       });
+      return;
+    }
+
+    const dialogRef = this.dialog.open(GfCreateOrUpdateAccessDialog, {
+      data: {
+        access: {
+          id: accessDetails.id,
+          alias: accessDetails.alias,
+          permissions: accessDetails.permissions,
+          type: accessDetails.type,
+          grantee:
+            accessDetails.grantee === 'Public' ? null : accessDetails.grantee
+        }
+      },
+      height: this.deviceType === 'mobile' ? '98vh' : undefined,
+      width: this.deviceType === 'mobile' ? '100vw' : '50rem'
+    });
+
+    dialogRef.afterClosed().subscribe((result) => {
+      if (result) {
+        this.update();
+      }
+    });
   }
 
   private update() {
diff --git a/apps/client/src/app/components/user-account-access/user-account-access.html b/apps/client/src/app/components/user-account-access/user-account-access.html
index c0e2fd741..8160c2c8e 100644
--- a/apps/client/src/app/components/user-account-access/user-account-access.html
+++ b/apps/client/src/app/components/user-account-access/user-account-access.html
@@ -64,7 +64,7 @@
     [showActions]="hasPermissionToDeleteAccess"
     [user]="user"
     (accessDeleted)="onDeleteAccess($event)"
-    (accessEdited)="onEditAccess($event)"
+    (accessToUpdate)="onUpdateAccess($event)"
   />
   @if (hasPermissionToCreateAccess) {
     <div class="fab-container">
diff --git a/apps/client/src/app/services/data.service.ts b/apps/client/src/app/services/data.service.ts
index ef89094b9..c2678924b 100644
--- a/apps/client/src/app/services/data.service.ts
+++ b/apps/client/src/app/services/data.service.ts
@@ -54,7 +54,6 @@ import {
 } from '@ghostfolio/common/interfaces';
 import { filterGlobalPermissions } from '@ghostfolio/common/permissions';
 import type {
-  AccessWithGranteeUser,
   AccountWithValue,
   AiPromptMode,
   DateRange,
@@ -336,10 +335,6 @@ export class DataService {
     return this.http.delete<any>(`/api/v1/watchlist/${dataSource}/${symbol}`);
   }
 
-  public fetchAccess(id: string) {
-    return this.http.get<AccessWithGranteeUser>(`/api/v1/access/${id}`);
-  }
-
   public fetchAccesses() {
     return this.http.get<Access[]>('/api/v1/access');
   }
@@ -798,11 +793,8 @@ export class DataService {
     return this.http.post('/api/v1/watchlist', watchlistItem);
   }
 
-  public putAccess(id: string, aAccess: UpdateAccessDto) {
-    return this.http.put<AccessWithGranteeUser>(
-      `/api/v1/access/${id}`,
-      aAccess
-    );
+  public putAccess(aAccess: UpdateAccessDto) {
+    return this.http.put<Access>(`/api/v1/access/${aAccess.id}`, aAccess);
   }
 
   public putAccount(aAccount: UpdateAccountDto) {
diff --git a/libs/common/src/lib/permissions.ts b/libs/common/src/lib/permissions.ts
index 52794f7dc..51f327d32 100644
--- a/libs/common/src/lib/permissions.ts
+++ b/libs/common/src/lib/permissions.ts
@@ -49,6 +49,7 @@ export const permissions = {
   syncDemoUserAccount: 'syncDemoUserAccount',
   toggleReadOnlyMode: 'toggleReadOnlyMode',
   updateAccount: 'updateAccount',
+  updateAccess: 'updateAccess',
   updateAuthDevice: 'updateAuthDevice',
   updateMarketData: 'updateMarketData',
   updateMarketDataOfOwnAssetProfile: 'updateMarketDataOfOwnAssetProfile',
@@ -93,6 +94,7 @@ export function getPermissions(aRole: Role): string[] {
         permissions.readTags,
         permissions.readWatchlist,
         permissions.updateAccount,
+        permissions.updateAccess,
         permissions.updateAuthDevice,
         permissions.updateMarketData,
         permissions.updateMarketDataOfOwnAssetProfile,
@@ -133,6 +135,7 @@ export function getPermissions(aRole: Role): string[] {
         permissions.readMarketDataOfOwnAssetProfile,
         permissions.readWatchlist,
         permissions.updateAccount,
+        permissions.updateAccess,
         permissions.updateAuthDevice,
         permissions.updateMarketDataOfOwnAssetProfile,
         permissions.updateOrder,