SF-3443 SF-3494 Filter out Lynx insights for chapters that cannot be edited (#3355)

Author: siltomato

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/lynx-insight-filter.service.spec.ts

@@ -1,13 +1,17 @@
 import { TestBed } from '@angular/core/testing';
 import { LynxInsightFilter } from 'realtime-server/lib/esm/scriptureforge/models/lynx-insight';
+import { TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
+import { anything, mock, verify, when } from 'ts-mockito';
 import { RouteBookChapter } from 'xforge-common/activated-book-chapter.service';
 import { configureTestingModule } from 'xforge-common/test-utils';
 import { TextDocId } from '../../../../core/models/text-doc';
+import { TextDocService } from '../../../../core/text-doc.service';
 import { LynxInsight } from './lynx-insight';
 import { LynxInsightFilterService } from './lynx-insight-filter.service';
 
 describe('LynxInsightFilterService', () => {
   let service: LynxInsightFilterService;
+  const mockTextDocService = mock(TextDocService);
 
   function createMockInsight(
     type: 'info' | 'warning' | 'error' = 'warning',
@@ -25,11 +29,24 @@ describe('LynxInsightFilterService', () => {
     };
   }
 
+  function createMockTextInfo(bookNum: number = 40): TextInfo {
+    return {
+      bookNum,
+      hasSource: false,
+      chapters: [],
+      permissions: {}
+    };
+  }
+
   configureTestingModule(() => ({
-    providers: [LynxInsightFilterService]
+    providers: [LynxInsightFilterService, { provide: TextDocService, useMock: mockTextDocService }]
   }));
 
   beforeEach(() => {
+    // Set up default mocks
+    when(mockTextDocService.hasChapterEditPermissionForText(anything(), anything())).thenReturn(true);
+    when(mockTextDocService.isUsfmValidForText(anything(), anything())).thenReturn(true);
+
     service = TestBed.inject(LynxInsightFilterService);
   });
 
@@ -182,5 +199,184 @@ describe('LynxInsightFilterService', () => {
 
       expect(result).toBe('project');
     });
+
+    it('should return false when permissions check fails', () => {
+      const insight = createMockInsight();
+      const filter: LynxInsightFilter = {
+        types: ['warning'],
+        scope: 'project',
+        includeDismissed: true
+      };
+      const bookChapter: RouteBookChapter = { bookId: 'MAT', chapter: 1 };
+      const dismissedIds: string[] = [];
+      const projectTexts: TextInfo[] = [createMockTextInfo()];
+
+      // Mock permission check to return false
+      when(mockTextDocService.hasChapterEditPermissionForText(anything(), anything())).thenReturn(false);
+
+      const result = service.matchesFilter(insight, filter, bookChapter, dismissedIds, projectTexts);
+
+      expect(result).toBeFalse();
+    });
+
+    it('should return false when USFM validity check fails', () => {
+      const insight = createMockInsight();
+      const filter: LynxInsightFilter = {
+        types: ['warning'],
+        scope: 'project',
+        includeDismissed: true
+      };
+      const bookChapter: RouteBookChapter = { bookId: 'MAT', chapter: 1 };
+      const dismissedIds: string[] = [];
+      const projectTexts: TextInfo[] = [createMockTextInfo()];
+
+      // Mock USFM validity check to return false
+      when(mockTextDocService.isUsfmValidForText(anything(), anything())).thenReturn(false);
+
+      const result = service.matchesFilter(insight, filter, bookChapter, dismissedIds, projectTexts);
+
+      expect(result).toBeFalse();
+    });
+
+    it('should return false when book not found in project texts', () => {
+      const insight = createMockInsight('warning', 42); // Luke
+      const filter: LynxInsightFilter = {
+        types: ['warning'],
+        scope: 'project',
+        includeDismissed: true
+      };
+      const bookChapter: RouteBookChapter = { bookId: 'MAT', chapter: 1 };
+      const dismissedIds: string[] = [];
+      const projectTexts: TextInfo[] = [createMockTextInfo(40)]; // Only Matthew
+
+      const result = service.matchesFilter(insight, filter, bookChapter, dismissedIds, projectTexts);
+
+      expect(result).toBeFalse();
+    });
+
+    it('should return true when permissions check passes', () => {
+      const insight = createMockInsight();
+      const filter: LynxInsightFilter = {
+        types: ['warning'],
+        scope: 'project',
+        includeDismissed: true
+      };
+      const bookChapter: RouteBookChapter = { bookId: 'MAT', chapter: 1 };
+      const dismissedIds: string[] = [];
+      const projectTexts: TextInfo[] = [createMockTextInfo()];
+
+      // Mock permission checks to return true (default setup)
+      when(mockTextDocService.hasChapterEditPermissionForText(anything(), anything())).thenReturn(true);
+      when(mockTextDocService.isUsfmValidForText(anything(), anything())).thenReturn(true);
+
+      const result = service.matchesFilter(insight, filter, bookChapter, dismissedIds, projectTexts);
+
+      expect(result).toBeTrue();
+    });
+
+    it('should return true when projectTexts is undefined (no permission filtering)', () => {
+      const insight = createMockInsight();
+      const filter: LynxInsightFilter = {
+        types: ['warning'],
+        scope: 'project',
+        includeDismissed: true
+      };
+      const bookChapter: RouteBookChapter = { bookId: 'MAT', chapter: 1 };
+      const dismissedIds: string[] = [];
+
+      const result = service.matchesFilter(insight, filter, bookChapter, dismissedIds);
+
+      expect(result).toBeTrue();
+    });
+
+    it('should call permission methods with correct parameters when projectTexts provided', () => {
+      const insight = createMockInsight('warning', 40, 1);
+      const filter: LynxInsightFilter = {
+        types: ['warning'],
+        scope: 'project',
+        includeDismissed: true
+      };
+      const bookChapter: RouteBookChapter = { bookId: 'MAT', chapter: 1 };
+      const dismissedIds: string[] = [];
+      const projectTexts: TextInfo[] = [createMockTextInfo(40)];
+
+      when(mockTextDocService.hasChapterEditPermissionForText(anything(), anything())).thenReturn(true);
+      when(mockTextDocService.isUsfmValidForText(anything(), anything())).thenReturn(true);
+
+      const result = service.matchesFilter(insight, filter, bookChapter, dismissedIds, projectTexts);
+
+      verify(mockTextDocService.hasChapterEditPermissionForText(anything(), 1)).once();
+      verify(mockTextDocService.isUsfmValidForText(anything(), 1)).once();
+      expect(result).toBeTrue();
+    });
+  });
+
+  describe('hasDisplayPermission', () => {
+    it('should return false when text not found', () => {
+      const insight = createMockInsight('warning', 42); // Luke
+      const projectTexts: TextInfo[] = [createMockTextInfo(40)]; // Only Matthew
+
+      const result = service.hasDisplayPermission(insight, projectTexts);
+
+      expect(result).toBeFalse();
+    });
+
+    it('should return false when edit permission is false', () => {
+      const insight = createMockInsight();
+      const projectTexts: TextInfo[] = [createMockTextInfo()];
+
+      when(mockTextDocService.hasChapterEditPermissionForText(anything(), anything())).thenReturn(false);
+
+      const result = service.hasDisplayPermission(insight, projectTexts);
+
+      expect(result).toBeFalse();
+    });
+
+    it('should return false when USFM is invalid', () => {
+      const insight = createMockInsight();
+      const projectTexts: TextInfo[] = [createMockTextInfo()];
+
+      when(mockTextDocService.isUsfmValidForText(anything(), anything())).thenReturn(false);
+
+      const result = service.hasDisplayPermission(insight, projectTexts);
+
+      expect(result).toBeFalse();
+    });
+
+    it('should return true when all checks pass', () => {
+      const insight = createMockInsight();
+      const projectTexts: TextInfo[] = [createMockTextInfo()];
+
+      when(mockTextDocService.hasChapterEditPermissionForText(anything(), anything())).thenReturn(true);
+      when(mockTextDocService.isUsfmValidForText(anything(), anything())).thenReturn(true);
+
+      const result = service.hasDisplayPermission(insight, projectTexts);
+
+      verify(mockTextDocService.hasChapterEditPermissionForText(anything(), 1)).once();
+      verify(mockTextDocService.isUsfmValidForText(anything(), 1)).once();
+      expect(result).toBeTrue();
+    });
+
+    it('should call methods with correct TextInfo and chapter number', () => {
+      const insight = createMockInsight('warning', 42, 3); // Luke 3
+      const matthewText = createMockTextInfo(40);
+      const lukeText = createMockTextInfo(42);
+      const projectTexts: TextInfo[] = [matthewText, lukeText];
+
+      when(mockTextDocService.hasChapterEditPermissionForText(anything(), anything())).thenReturn(true);
+      when(mockTextDocService.isUsfmValidForText(anything(), anything())).thenReturn(true);
+
+      service.hasDisplayPermission(insight, projectTexts);
+
+      // Should call with Luke 3
+      verify(mockTextDocService.hasChapterEditPermissionForText(lukeText, 3)).once();
+      verify(mockTextDocService.isUsfmValidForText(lukeText, 3)).once();
+
+      // Should not call with Matthew
+      verify(mockTextDocService.hasChapterEditPermissionForText(matthewText, anything())).never();
+      verify(mockTextDocService.isUsfmValidForText(matthewText, anything())).never();
+
+      expect(1).toBe(1);
+    });
   });
 });

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/lynx-insight-filter.service.ts

@@ -1,20 +1,23 @@
 import { Injectable } from '@angular/core';
 import { Canon } from '@sillsdev/scripture';
 import { LynxInsightFilter, LynxInsightFilterScope } from 'realtime-server/lib/esm/scriptureforge/models/lynx-insight';
+import { TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
 import { RouteBookChapter } from 'xforge-common/activated-book-chapter.service';
+import { TextDocService } from '../../../../core/text-doc.service';
 import { LynxInsight } from './lynx-insight';
 
 @Injectable({
   providedIn: 'root'
 })
 export class LynxInsightFilterService {
-  constructor() {}
+  constructor(private readonly textDocService: TextDocService) {}
 
   matchesFilter(
     insight: LynxInsight,
     filter: LynxInsightFilter,
     bookChapter: RouteBookChapter,
-    dismissedIds: string[]
+    dismissedIds: string[],
+    projectTexts?: TextInfo[]
   ): boolean {
     const routeBookNum: number | undefined = bookChapter.bookId ? Canon.bookIdToNumber(bookChapter.bookId) : undefined;
     const routeChapter = bookChapter.chapter;
@@ -28,6 +31,11 @@ export class LynxInsightFilterService {
       return false;
     }
 
+    // Check edit permissions and USFM validity if project texts are provided
+    if (projectTexts != null && !this.hasDisplayPermission(insight, projectTexts)) {
+      return false;
+    }
+
     if (filter.scope === 'project') {
       return true;
     }
@@ -58,4 +66,35 @@ export class LynxInsightFilterService {
       return 'project';
     }
   }
+
+  /**
+   * Check if an insight has display permission (edit permission and USFM validity).
+   * @param insight The insight to check.
+   * @param projectTexts The project texts to check permissions against.
+   * @returns True if the insight should be displayed, false otherwise.
+   */
+  hasDisplayPermission(insight: LynxInsight, projectTexts: TextInfo[]): boolean {
+    // Find the text info for this book
+    const text: TextInfo | undefined = projectTexts.find(t => t.bookNum === insight.textDocId.bookNum);
+    if (text == null) {
+      return false;
+    }
+
+    // Check chapter edit permission
+    const hasEditPermission: boolean | undefined = this.textDocService.hasChapterEditPermissionForText(
+      text,
+      insight.textDocId.chapterNum
+    );
+    if (!hasEditPermission) {
+      return false;
+    }
+
+    // Check USFM validity
+    const isUsfmValid: boolean = this.textDocService.isUsfmValidForText(text, insight.textDocId.chapterNum);
+    if (!isUsfmValid) {
+      return false;
+    }
+
+    return true;
+  }
 }

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/lynx-insight-state.service.spec.ts

@@ -1,11 +1,15 @@
 import { TestBed } from '@angular/core/testing';
-import { LynxInsightType } from 'realtime-server/lib/esm/scriptureforge/models/lynx-insight';
+import { LynxInsightFilter, LynxInsightType } from 'realtime-server/lib/esm/scriptureforge/models/lynx-insight';
+import { createTestProject } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-test-data';
+import { createTestProjectUserConfig } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-user-config-test-data';
+import { TextInfo } from 'realtime-server/scriptureforge/models/text-info';
 import { BehaviorSubject, firstValueFrom, Subject } from 'rxjs';
 import { anything, instance, mock, when } from 'ts-mockito';
 import { ActivatedBookChapterService, RouteBookChapter } from 'xforge-common/activated-book-chapter.service';
 import { ActivatedProjectUserConfigService } from 'xforge-common/activated-project-user-config.service';
+import { ActivatedProjectService } from 'xforge-common/activated-project.service';
 import { configureTestingModule } from 'xforge-common/test-utils';
-import { createTestProjectUserConfig } from '../../../../../../../../RealtimeServer/scriptureforge/models/sf-project-user-config-test-data';
+import { SFProjectDoc } from '../../../../core/models/sf-project-doc';
 import { SFProjectUserConfigDoc } from '../../../../core/models/sf-project-user-config-doc';
 import { TextDocId } from '../../../../core/models/text-doc';
 import { LynxInsight } from './lynx-insight';
@@ -33,6 +37,7 @@ describe('LynxInsightStateService', () => {
 
   const mockInsightFilterService = mock<LynxInsightFilterService>();
   const mockActivatedBookChapterService = mock<ActivatedBookChapterService>();
+  const mockActivatedProjectService = mock<ActivatedProjectService>();
   const mockActivatedProjectUserConfigService = mock<ActivatedProjectUserConfigService>();
   const mockLynxWorkspaceService = mock<LynxWorkspaceService>();
   const mockProjectUserConfigDoc = mock(SFProjectUserConfigDoc);
@@ -72,6 +77,7 @@ describe('LynxInsightStateService', () => {
       LynxInsightStateService,
       { provide: LynxInsightFilterService, useMock: mockInsightFilterService },
       { provide: ActivatedBookChapterService, useMock: mockActivatedBookChapterService },
+      { provide: ActivatedProjectService, useMock: mockActivatedProjectService },
       { provide: ActivatedProjectUserConfigService, useMock: mockActivatedProjectUserConfigService },
       { provide: LynxWorkspaceService, useMock: mockLynxWorkspaceService }
     ]
@@ -101,14 +107,27 @@ describe('LynxInsightStateService', () => {
       instance(mockProjectUserConfigDoc)
     );
 
+    const projectDoc$ = new BehaviorSubject<SFProjectDoc | undefined>({
+      id: 'project1',
+      data: createTestProject()
+    } as SFProjectDoc);
+
     when(mockLynxWorkspaceService.rawInsightSource$).thenReturn(rawInsightSource);
     when(mockLynxWorkspaceService.currentInsights).thenReturn(testInsights);
     when(mockActivatedBookChapterService.activatedBookChapter$).thenReturn(activatedBookChapter);
+    when(mockActivatedProjectService.projectDoc$).thenReturn(projectDoc$);
     when(mockActivatedProjectUserConfigService.projectUserConfig$).thenReturn(projectUserConfig$);
     when(mockActivatedProjectUserConfigService.projectUserConfigDoc$).thenReturn(projectUserConfigDoc$);
 
-    when(mockInsightFilterService.matchesFilter(anything(), anything(), anything(), anything())).thenCall(
-      (insight: LynxInsight, filter: any, _bookChapter: RouteBookChapter, dismissedIds: string[]) => {
+    when(mockInsightFilterService.hasDisplayPermission(anything(), anything())).thenReturn(true);
+    when(mockInsightFilterService.matchesFilter(anything(), anything(), anything(), anything(), anything())).thenCall(
+      (
+        insight: LynxInsight,
+        filter: LynxInsightFilter,
+        _bookChapter: RouteBookChapter,
+        dismissedIds: string[],
+        _projectTexts?: TextInfo[]
+      ) => {
         if (!filter.types.includes(insight.type)) {
           return false;
         }