Fix error in update check condition

paulscherrerinstitute · Jan 31, 2024 · 40b2c04 · 40b2c04
1 parent 97034df
commit 40b2c04
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 46 deletions.
diff --git a/scilog/src/app/logbook/core/snippet/snippet.component.spec.ts b/scilog/src/app/logbook/core/snippet/snippet.component.spec.ts
@@ -303,18 +303,6 @@ describe('SnippetComponent', () => {
     expect(component.setLocked).toHaveBeenCalledTimes(1);
   });
 
-  [
-    {input: {v: true, isNotExpired: true}, output: true},
-    {input: {v: true, isNotExpired: false}, output: false},
-    {input: {v: false, isNotExpired: true}, output: false},
-    {input: {v: false, isNotExpired: false}, output: false},
-  ].forEach((t, i) => {
-    it(`should test commonCondition ${i}`, () => {
-      spyOn(component['isActionAllowed'], "isNotExpired").and.returnValue(t.input.isNotExpired);
-      expect(component['commonCondition'](t.input.v)).toEqual(t.output);
-    })
-  });
-
   [
     {input: {v: true, canUpdate: true}, output: true},
     {input: {v: true, canUpdate: false}, output: false},
@@ -324,7 +312,6 @@ describe('SnippetComponent', () => {
     it(`should test enableEdit ${i}`, () => {
       spyOn(component['isActionAllowed'], "canUpdate").and.returnValue(t.input.canUpdate);
       spyOn(component['isActionAllowed'], "canDelete").and.returnValue(true);
-      spyOn<any>(component, "commonCondition").and.returnValue(t.input.v);
       component.enableEdit = t.input.v;
       expect(component._enableEdit.update).toEqual(t.output);
     })

diff --git a/scilog/src/app/logbook/core/snippet/snippet.component.ts b/scilog/src/app/logbook/core/snippet/snippet.component.ts
@@ -28,7 +28,8 @@ import { IsAllowedService } from 'src/app/overview/is-allowed.service';
       transition('highlight => default', animate('2000ms ease-out')),
       transition('default => highlight', animate('200ms ease-in'))
     ])
-  ]
+  ],
+  providers: [IsAllowedService],
 })
 export class SnippetComponent implements OnInit {
 
@@ -146,6 +147,7 @@ export class SnippetComponent implements OnInit {
       this._hideMetadata = true;
     }
     // enable edit for snippet
+    this.isActionAllowed.snippet = this.snippet;
     this.enableEdit = true;
     this._subsnippets = new BehaviorSubject(this.snippet.subsnippets);
   }
@@ -165,19 +167,13 @@ export class SnippetComponent implements OnInit {
       });
   }
 
-  public get enableEdit() {
+  public get enableEdit(): any {
     return this._enableEdit;
   }
 
-  public set enableEdit(v: boolean | {update: boolean, delete: boolean}) {
-    const commonConditions = this.commonCondition(v);
-    this._enableEdit.update = commonConditions && this.isActionAllowed.canUpdate();
-    this._enableEdit.delete = commonConditions && this.isActionAllowed.canDelete();
-  }
-
-  private commonCondition(v: boolean | { update: boolean; delete: boolean; }) {
-    this.isActionAllowed.snippet = this.snippet;
-    return v && this.isActionAllowed.isNotExpired();
+  public set enableEdit(v: boolean) {
+    this._enableEdit.update = this.isActionAllowed.canUpdate() && v;
+    this._enableEdit.delete = this.isActionAllowed.canDelete() && v;
   }
 
   ngAfterViewChecked(): void {

diff --git a/scilog/src/app/overview/add-logbook/add-logbook.component.ts b/scilog/src/app/overview/add-logbook/add-logbook.component.ts
@@ -30,7 +30,8 @@ function groupCreationValidator(control: AbstractControl): { anyAuthGroup: {valu
 @Component({
   selector: 'app-add-logbook',
   templateUrl: './add-logbook.component.html',
-  styleUrls: ['./add-logbook.component.css']
+  styleUrls: ['./add-logbook.component.css'],
+  providers: [IsAllowedService]
 })
 export class AddLogbookComponent implements OnInit {
 

diff --git a/scilog/src/app/overview/is-allowed.service.spec.ts b/scilog/src/app/overview/is-allowed.service.spec.ts
@@ -21,7 +21,7 @@ describe('IsAllowedService', () => {
 
 
 [
-    {input: '2200-12-12T00:00:00Z', output: undefined},
+    {input: '2200-12-12T00:00:00Z', output: ''},
     {input: '2023-12-12T00:00:00Z', output: jasmine.anything()}
   ].forEach((t, i) => {
     it(`should test isNotExpired ${i}`, () => {
@@ -64,17 +64,21 @@ describe('IsAllowedService', () => {
   });
 
   [
-    {input: {expired: 'Expired', check: true}, output: 'Expired'},
-    {input: {expired: 'Expired', check: false}, output: 'Update'},
-    {input: {expired: '', check: true}, output: 'Update'},
+    {input: {expired: undefined, check: true}, output: 'Update'},
+    {input: {expired: undefined, check: false}, output: 'Update'},
     {input: {expired: '', check: false}, output: 'Update'},
+    {input: {expired: '', check: true}, output: 'Update'},
+    {input: {expired: 'Expired', check: false}, output: 'Expired'},
+    {input: {expired: 'Expired', check: true}, output: 'Expired'},
   ].forEach((t, i) => {
     it(`should test cascadeExpiration ${i}`, () => {
-      spyOn(service, 'isUserAllowed');
       service.tooltips.expired = t.input.expired;
+      spyOn(service, 'isNotExpired').and.returnValue(t.input.check);
+      spyOn(service, 'isUserAllowed').and.returnValue(true);
       service.tooltips.update = 'Update';
-      service['cascadeExpiration']('update', t.input.check);
+      expect(service['cascadeExpiration']('update')).toEqual(t.output === 'Expired'? false: true);
       expect(service.tooltips.update).toEqual(t.output);
+      service['cascadeExpiration']('update');
     });
   });
 

diff --git a/scilog/src/app/overview/is-allowed.service.ts b/scilog/src/app/overview/is-allowed.service.ts
@@ -4,9 +4,7 @@ import { UserPreferencesService } from "../core/user-preferences.service";
 import { ChangeStreamNotification } from "../logbook/core/changestreamnotification.model";
 
 
-@Injectable({
-  providedIn: 'root'
-})
+@Injectable()
 export class IsAllowedService {
   tooltips: {
     update?: string,
@@ -21,7 +19,10 @@ export class IsAllowedService {
 
   isNotExpired() {
     const expiresAt =  new Date(this.snippet.expiresAt);
-    if (expiresAt > new Date()) return true
+    if (expiresAt > new Date()) {
+      this.tooltips.expired = '';
+      return true
+    } 
     const expiresString = expiresAt.toLocaleDateString(
         'en-GB', 
         {year: '2-digit', month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit'}
@@ -75,18 +76,20 @@ export class IsAllowedService {
     return false
   }
 
-  canUpdate(checkExpiration = true) {
-    return this.cascadeExpiration('update', checkExpiration)
+  canUpdate() {
+    return this.cascadeExpiration('update')
   }
 
-  private cascadeExpiration(action: string, checkExpiration: boolean) {
-    const allowed = this.isUserAllowed(action);
-    if (checkExpiration && this.tooltips.expired)
-      this.tooltips[action] = this.tooltips.expired;
-    return allowed;
+  private cascadeExpiration(action: string) {
+    if (this.tooltips.expired === undefined && this.isNotExpired())
+      return this.isUserAllowed(action);
+    else if (!this.tooltips.expired)
+      return this.isUserAllowed(action);
+    this.tooltips[action] = this.tooltips.expired;
+    return false;
   }
 
-  canDelete(checkExpiration = true) {
-    return this.cascadeExpiration('delete', checkExpiration)
+  canDelete() {
+    return this.cascadeExpiration('delete')
   }
 }
diff --git a/scilog/src/app/overview/logbook-cover/logbook-cover.component.ts b/scilog/src/app/overview/logbook-cover/logbook-cover.component.ts
@@ -1,5 +1,4 @@
 import { Component, OnInit, EventEmitter, Output, Input, ViewChild, ElementRef } from '@angular/core';
-import { UserPreferencesService } from '@shared/user-preferences.service';
 import { Logbooks } from '@model/logbooks';
 import { LogbookInfoService } from '@shared/logbook-info.service';
 import { LogbookItemDataService } from '@shared/remote-data.service';
@@ -8,7 +7,8 @@ import { IsAllowedService } from '../is-allowed.service';
 @Component({
   selector: 'app-logbook-cover',
   templateUrl: './logbook-cover.component.html',
-  styleUrls: ['./logbook-cover.component.css']
+  styleUrls: ['./logbook-cover.component.css'],
+  providers: [IsAllowedService]
 })
 export class LogbookWidgetComponent implements OnInit {