From ae6f5b625549813455c555f85b10b44b0567ce47 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Germ=C3=A1n=20Mart=C3=ADn?= <github@gmartin.net>
Date: Sun, 5 Oct 2025 22:47:00 +0200
Subject: [PATCH] Refactor access management: reorder parameters, enhance
 validation, and improve dialog handling

---
 apps/api/src/app/access/access.controller.ts  |  4 +-
 apps/api/src/app/access/access.service.ts     | 12 +++---
 apps/api/src/app/access/update-access.dto.ts  |  6 +--
 .../access-table/access-table.component.html  |  1 +
 ...reate-or-update-access-dialog.component.ts | 39 +++++++++----------
 .../user-account-access.component.ts          | 30 +++++++-------
 libs/common/src/lib/permissions.ts            |  2 +-
 7 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/apps/api/src/app/access/access.controller.ts b/apps/api/src/app/access/access.controller.ts
index 836bd49c0..177191a6e 100644
--- a/apps/api/src/app/access/access.controller.ts
+++ b/apps/api/src/app/access/access.controller.ts
@@ -129,8 +129,8 @@ export class AccessController {
   @Put(':id')
   @UseGuards(AuthGuard('jwt'), HasPermissionGuard)
   public async updateAccess(
-    @Param('id') id: string,
-    @Body() data: UpdateAccessDto
+    @Body() data: UpdateAccessDto,
+    @Param('id') id: string
   ): Promise<AccessModel> {
     if (
       this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION') &&
diff --git a/apps/api/src/app/access/access.service.ts b/apps/api/src/app/access/access.service.ts
index 4c4534811..70e46dc36 100644
--- a/apps/api/src/app/access/access.service.ts
+++ b/apps/api/src/app/access/access.service.ts
@@ -20,16 +20,14 @@ export class AccessService {
   }
 
   public async accesses(params: {
+    cursor?: Prisma.AccessWhereUniqueInput;
     include?: Prisma.AccessInclude;
+    orderBy?: Prisma.Enumerable<Prisma.AccessOrderByWithRelationInput>;
     skip?: number;
     take?: number;
-    cursor?: Prisma.AccessWhereUniqueInput;
     where?: Prisma.AccessWhereInput;
-    orderBy?:
-      | Prisma.AccessOrderByWithRelationInput
-      | Prisma.AccessOrderByWithRelationInput[];
   }): Promise<AccessWithGranteeUser[]> {
-    const { include, skip, take, cursor, where, orderBy } = params;
+    const { cursor, include, orderBy, skip, take, where } = params;
 
     return this.prismaService.access.findMany({
       cursor,
@@ -63,8 +61,8 @@ export class AccessService {
     where: Prisma.AccessWhereUniqueInput;
   }): Promise<Access> {
     return this.prismaService.access.update({
-      where,
-      data
+      data,
+      where
     });
   }
 }
diff --git a/apps/api/src/app/access/update-access.dto.ts b/apps/api/src/app/access/update-access.dto.ts
index 26b6df687..2850186f9 100644
--- a/apps/api/src/app/access/update-access.dto.ts
+++ b/apps/api/src/app/access/update-access.dto.ts
@@ -2,9 +2,6 @@ import { AccessPermission } from '@prisma/client';
 import { IsEnum, IsOptional, IsString, IsUUID } from 'class-validator';
 
 export class UpdateAccessDto {
-  @IsString()
-  id: string;
-
   @IsOptional()
   @IsString()
   alias?: string;
@@ -13,6 +10,9 @@ export class UpdateAccessDto {
   @IsUUID()
   granteeUserId?: string;
 
+  @IsString()
+  id: string;
+
   @IsEnum(AccessPermission, { each: true })
   @IsOptional()
   permissions?: AccessPermission[];
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 ded172acd..b7001782a 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
@@ -72,6 +72,7 @@
                 <span i18n>Edit</span>
               </span>
             </button>
+            <hr class="my-0" />
           }
           @if (element.type === 'PUBLIC') {
             <button mat-menu-item (click)="onCopyUrlToClipboard(element.id)">
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 8a5f7fc37..1dee92b39 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
@@ -68,22 +68,18 @@ export class GfCreateOrUpdateAccessDialogComponent
     this.mode = this.data.access?.id ? 'update' : 'create';
   }
 
-  public onCancel() {
-    this.dialogRef.close();
-  }
-
-  public ngOnDestroy() {
-    this.unsubscribeSubject.next();
-    this.unsubscribeSubject.complete();
-  }
-
   public ngOnInit() {
+    const isPublic = this.data.access.type === 'PUBLIC';
+
     this.accessForm = this.formBuilder.group({
       alias: [this.data.access.alias],
-      granteeUserId: [this.data.access.grantee, Validators.required],
+      granteeUserId: [
+        this.data.access.grantee,
+        isPublic ? null : Validators.required
+      ],
       permissions: [this.data.access.permissions[0], Validators.required],
       type: [
-        { value: this.data.access.type, disabled: this.mode === 'update' },
+        { disabled: this.mode === 'update', value: this.data.access.type },
         Validators.required
       ]
     });
@@ -104,14 +100,10 @@ export class GfCreateOrUpdateAccessDialogComponent
 
       this.changeDetectorRef.markForCheck();
     });
+  }
 
-    // Initial validation setup based on current type
-    if (this.accessForm.get('type').value === 'PUBLIC') {
-      const granteeUserIdControl = this.accessForm.get('granteeUserId');
-      granteeUserIdControl.clearValidators();
-      granteeUserIdControl.setValue(null);
-      granteeUserIdControl.updateValueAndValidity();
-    }
+  public onCancel() {
+    this.dialogRef.close();
   }
 
   public async onSubmit() {
@@ -122,6 +114,11 @@ export class GfCreateOrUpdateAccessDialogComponent
     }
   }
 
+  public ngOnDestroy() {
+    this.unsubscribeSubject.next();
+    this.unsubscribeSubject.complete();
+  }
+
   private async createAccess() {
     const access: CreateAccessDto = {
       alias: this.accessForm.get('alias').value,
@@ -160,9 +157,9 @@ export class GfCreateOrUpdateAccessDialogComponent
 
   private async updateAccess() {
     const access: UpdateAccessDto = {
-      id: this.data.access.id,
       alias: this.accessForm.get('alias').value,
       granteeUserId: this.accessForm.get('granteeUserId').value,
+      id: this.data.access.id,
       permissions: [this.accessForm.get('permissions').value]
     };
 
@@ -176,8 +173,8 @@ export class GfCreateOrUpdateAccessDialogComponent
       this.dataService
         .putAccess(access)
         .pipe(
-          catchError((error) => {
-            if (error.status === StatusCodes.BAD_REQUEST) {
+          catchError(({ status }) => {
+            if (status.status === StatusCodes.BAD_REQUEST) {
               this.notificationService.alert({
                 title: $localize`Oops! Could not update access.`
               });
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 1bcac204e..2201f6e87 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,8 +115,8 @@ export class GfUserAccountAccessComponent implements OnDestroy, OnInit {
       .subscribe((params) => {
         if (params['createDialog']) {
           this.openCreateAccessDialog();
-        } else if (params['editDialog']) {
-          this.openUpdateAccessDialog(params['editDialog']);
+        } else if (params['editDialog'] && params['accessId']) {
+          this.openUpdateAccessDialog(params['accessId']);
         }
       });
 
@@ -140,10 +140,6 @@ export class GfUserAccountAccessComponent implements OnDestroy, OnInit {
       });
   }
 
-  public onUpdateAccess(aId: string) {
-    this.openUpdateAccessDialog(aId);
-  }
-
   public onGenerateAccessToken() {
     this.notificationService.confirm({
       confirmFn: () => {
@@ -179,6 +175,12 @@ export class GfUserAccountAccessComponent implements OnDestroy, OnInit {
     });
   }
 
+  public onUpdateAccess(aId: string) {
+    this.router.navigate([], {
+      queryParams: { accessId: aId, editDialog: true }
+    });
+  }
+
   public ngOnDestroy() {
     this.unsubscribeSubject.next();
     this.unsubscribeSubject.complete();
@@ -208,9 +210,9 @@ export class GfUserAccountAccessComponent implements OnDestroy, OnInit {
 
   private openUpdateAccessDialog(accessId: string) {
     // Find the access details in the already loaded data
-    const accessDetails = this.accessesGive.find(
-      (access) => access.id === accessId
-    );
+    const accessDetails = this.accessesGive.find(({ id }) => {
+      return id === accessId;
+    });
 
     if (!accessDetails) {
       this.notificationService.alert({
@@ -222,12 +224,12 @@ export class GfUserAccountAccessComponent implements OnDestroy, OnInit {
     const dialogRef = this.dialog.open(GfCreateOrUpdateAccessDialogComponent, {
       data: {
         access: {
-          id: accessDetails.id,
           alias: accessDetails.alias,
-          permissions: accessDetails.permissions,
-          type: accessDetails.type,
+          id: accessDetails.id,
           grantee:
-            accessDetails.grantee === 'Public' ? null : accessDetails.grantee
+            accessDetails.grantee === 'Public' ? null : accessDetails.grantee,
+          permissions: accessDetails.permissions,
+          type: accessDetails.type
         }
       },
       height: this.deviceType === 'mobile' ? '98vh' : undefined,
@@ -238,6 +240,8 @@ export class GfUserAccountAccessComponent implements OnDestroy, OnInit {
       if (result) {
         this.update();
       }
+
+      this.router.navigate(['.'], { relativeTo: this.route });
     });
   }
 
diff --git a/libs/common/src/lib/permissions.ts b/libs/common/src/lib/permissions.ts
index 9539b7031..51f327d32 100644
--- a/libs/common/src/lib/permissions.ts
+++ b/libs/common/src/lib/permissions.ts
@@ -184,7 +184,7 @@ export function hasReadRestrictedAccessPermission({
   impersonationId: string;
   user: UserWithSettings;
 }) {
-  if (!impersonationId || !user) {
+  if (!impersonationId) {
     return false;
   }