Skip to content

Commit 8612d93

Browse files
pmachapmanpmachapman
committed
SF-3637 Frontend changes
1 parent 12c8136 commit 8612d93

File tree

11 files changed

+80
-61
lines changed

11 files changed

+80
-61
lines changed

‎src/SIL.XForge.Scripture/ClientApp/src/app/core/permissions.service.spec.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,16 @@ describe('PermissionsService', () => {
105105

106106
const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 1 };
107107

108-
expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(true);
108+
expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(true);
109109
}));
110110

111111
it('allows access to text if user is on project and does not have chapter permission', fakeAsync(async () => {
112112
const env = new TestEnvironment();
113-
env.setupProjectData(undefined);
113+
env.setupProjectData(undefined, false);
114114

115115
const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 1 };
116116

117-
expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(true);
117+
expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(true);
118118
}));
119119

120120
it('does not allow access to text if user is not on project', fakeAsync(async () => {
@@ -123,7 +123,7 @@ describe('PermissionsService', () => {
123123

124124
const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 1 };
125125

126-
expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(false);
126+
expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(false);
127127
}));
128128

129129
it('does not allow access to text if user has no access', fakeAsync(async () => {
@@ -132,7 +132,7 @@ describe('PermissionsService', () => {
132132

133133
const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 1 };
134134

135-
expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(false);
135+
expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(false);
136136
}));
137137

138138
it('does not allow access to text if the chapter does not exist', fakeAsync(async () => {
@@ -141,7 +141,7 @@ describe('PermissionsService', () => {
141141

142142
const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 2 };
143143

144-
expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(false);
144+
expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(false);
145145
}));
146146

147147
it('checks current user doc to determine if user is on project', fakeAsync(async () => {
@@ -264,7 +264,7 @@ class TestEnvironment {
264264
this.setupUserData();
265265
}
266266

267-
setupProjectData(textPermission?: TextInfoPermission | undefined): void {
267+
setupProjectData(textPermission?: TextInfoPermission | undefined, chapterPermissions: boolean = true): void {
268268
const projectId: string = 'project01';
269269
const permission: TextInfoPermission = textPermission ?? TextInfoPermission.Write;
270270

@@ -284,10 +284,12 @@ class TestEnvironment {
284284
number: 1,
285285
lastVerse: 3,
286286
isValid: true,
287-
permissions: {
288-
user01: permission,
289-
user02: permission
290-
}
287+
permissions: chapterPermissions
288+
? {
289+
user01: permission,
290+
user02: permission
291+
}
292+
: {}
291293
}
292294
],
293295
hasSource: true,

‎src/SIL.XForge.Scripture/ClientApp/src/app/core/permissions.service.ts‎

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { Injectable } from '@angular/core';
22
import { Operation } from 'realtime-server/lib/esm/common/models/project-rights';
3+
import { SFProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project';
34
import { SF_PROJECT_RIGHTS, SFProjectDomain } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-rights';
45
import { isParatextRole, SFProjectRole } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-role';
5-
import { Chapter } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
6+
import { Chapter, TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
67
import { TextInfoPermission } from 'realtime-server/lib/esm/scriptureforge/models/text-info-permission';
78
import { UserDoc } from 'xforge-common/models/user-doc';
89
import { UserService } from 'xforge-common/user.service';
@@ -58,25 +59,26 @@ export class PermissionsService {
5859
return isParatextRole(projectDoc.data?.userRoles[currentUserDoc.id] ?? SFProjectRole.None);
5960
}
6061

61-
async canAccessText(textDocId: TextDocId): Promise<boolean> {
62-
// Get the project doc, if the user is on that project
63-
let projectDoc: SFProjectProfileDoc | undefined;
64-
if (textDocId.projectId != null) {
65-
const isUserOnProject = await this.isUserOnProject(textDocId.projectId);
66-
projectDoc = isUserOnProject ? await this.projectService.getProfile(textDocId.projectId) : undefined;
67-
}
68-
62+
/**
63+
* Determines if a user can access the text in the specified project.
64+
* @param textDocId The text document id.
65+
* @param project The project.
66+
* @returns A boolean value.
67+
*/
68+
canAccessText(textDocId?: TextDocId, project?: SFProjectProfile): boolean {
6969
// Ensure the user has project level permission to view the text
7070
if (
71-
projectDoc?.data != null &&
72-
SF_PROJECT_RIGHTS.hasRight(projectDoc.data, this.userService.currentUserId, SFProjectDomain.Texts, Operation.View)
71+
textDocId != null &&
72+
project != null &&
73+
SF_PROJECT_RIGHTS.hasRight(project, this.userService.currentUserId, SFProjectDomain.Texts, Operation.View)
7374
) {
7475
// Check chapter permissions
75-
const chapter: Chapter | undefined = projectDoc.data.texts
76-
.find(t => t.bookNum === textDocId.bookNum)
77-
?.chapters.find(c => c.number === textDocId.chapterNum);
78-
if (chapter != null) {
79-
const chapterPermission: string | undefined = chapter.permissions[this.userService.currentUserId];
76+
const text: TextInfo | undefined = project.texts.find(t => t.bookNum === textDocId.bookNum);
77+
const chapter: Chapter | undefined = text?.chapters.find(c => c.number === textDocId.chapterNum);
78+
if (text != null && chapter != null) {
79+
// If the chapter permission is not present, use the book permission instead
80+
const chapterPermission: string | undefined =
81+
chapter.permissions[this.userService.currentUserId] ?? text.permissions[this.userService.currentUserId];
8082
// If there is no chapter permission, they will have access to the chapter as they have access to the project.
8183
// We should only deny access if there is an explicit "None" permission.
8284
return chapterPermission !== TextInfoPermission.None;
@@ -86,6 +88,23 @@ export class PermissionsService {
8688
return false;
8789
}
8890

91+
/**
92+
* Determines if a user can access a text.
93+
* @param textDocId The text document id.
94+
* @returns A promise for a boolean value.
95+
*/
96+
async canAccessTextAsync(textDocId: TextDocId): Promise<boolean> {
97+
// Get the project doc, if the user is on that project
98+
let projectDoc: SFProjectProfileDoc | undefined;
99+
if (textDocId.projectId != null) {
100+
const isUserOnProject = await this.isUserOnProject(textDocId.projectId);
101+
projectDoc = isUserOnProject ? await this.projectService.getProfile(textDocId.projectId) : undefined;
102+
return this.canAccessText(textDocId, projectDoc?.data);
103+
}
104+
105+
return false;
106+
}
107+
89108
canSync(projectDoc: SFProjectProfileDoc, userId?: string): boolean {
90109
if (projectDoc.data == null) {
91110
return false;

‎src/SIL.XForge.Scripture/ClientApp/src/app/core/text-doc.service.spec.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,9 +419,10 @@ describe('TextDocService', () => {
419419
expect(actual).toBeUndefined();
420420
});
421421

422-
it('should return false if the user does not have the edit permission', () => {
422+
it('should return false if the user does not have the write permission for the chapter', () => {
423423
const env = new TestEnvironment();
424424
const text: Partial<TextInfo> = {
425+
permissions: { user01: TextInfoPermission.Write },
425426
chapters: [
426427
{ number: 1, permissions: { user01: TextInfoPermission.Read }, lastVerse: 0, isValid: true } as Chapter
427428
]
@@ -432,7 +433,7 @@ describe('TextDocService', () => {
432433
expect(actual).toBe(false);
433434
});
434435

435-
it('should return true if the user has the write permission', () => {
436+
it('should return true if the user has the write permission for the chapter', () => {
436437
const env = new TestEnvironment();
437438
const text: Partial<TextInfo> = {
438439
chapters: [
@@ -444,6 +445,18 @@ describe('TextDocService', () => {
444445
const actual: boolean | undefined = env.textDocService.hasChapterEditPermissionForText(text as TextInfo, 1);
445446
expect(actual).toBe(true);
446447
});
448+
449+
it('should return true if the user has the write permission for the book and no chapter permission set', () => {
450+
const env = new TestEnvironment();
451+
const text: Partial<TextInfo> = {
452+
permissions: { user01: TextInfoPermission.Write },
453+
chapters: [{ number: 1, lastVerse: 0, isValid: true } as Chapter]
454+
};
455+
456+
// SUT
457+
const actual: boolean | undefined = env.textDocService.hasChapterEditPermissionForText(text as TextInfo, 1);
458+
expect(actual).toBe(true);
459+
});
447460
});
448461
});
449462

‎src/SIL.XForge.Scripture/ClientApp/src/app/core/text-doc.service.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,8 @@ export class TextDocService {
189189
const chapter: Chapter | undefined = text?.chapters.find(c => c.number === chapterNum);
190190
// Even though permissions is guaranteed to be there in the model, its not in IndexedDB the first time the project
191191
// is accessed after migration
192-
const permission: string | undefined = chapter?.permissions?.[this.userService.currentUserId];
192+
const permission: string | undefined =
193+
chapter?.permissions?.[this.userService.currentUserId] ?? text?.permissions?.[this.userService.currentUserId];
193194
return permission == null ? undefined : permission === TextInfoPermission.Write;
194195
}
195196

‎src/SIL.XForge.Scripture/ClientApp/src/app/shared/cache-service/cache.service.spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ describe('cache service', () => {
2424
describe('load all texts', () => {
2525
it('does not get texts from project service if no permission', fakeAsync(async () => {
2626
const env = new TestEnvironment();
27-
when(mockedPermissionService.canAccessText(anything())).thenResolve(false);
27+
when(mockedPermissionService.canAccessTextAsync(anything())).thenResolve(false);
2828
await env.service.cache(env.projectDoc);
2929
env.wait();
3030

@@ -72,9 +72,9 @@ describe('cache service', () => {
7272

7373
it('gets the source texts if they are present and the user can access', fakeAsync(async () => {
7474
const env = new TestEnvironment();
75-
when(mockedPermissionService.canAccessText(deepEqual(new TextDocId('sourceId', 0, 0, 'target')))).thenResolve(
76-
false
77-
); //remove access for one source doc
75+
when(
76+
mockedPermissionService.canAccessTextAsync(deepEqual(new TextDocId('sourceId', 0, 0, 'target')))
77+
).thenResolve(false); //remove access for one source doc
7878

7979
await env.service.cache(env.projectDoc);
8080
env.wait();
@@ -106,7 +106,7 @@ class TestEnvironment {
106106
});
107107

108108
when(mockedProjectDoc.data).thenReturn(data);
109-
when(mockedPermissionService.canAccessText(anything())).thenResolve(true);
109+
when(mockedPermissionService.canAccessTextAsync(anything())).thenResolve(true);
110110
}
111111

112112
createTexts(): TextInfo[] {

‎src/SIL.XForge.Scripture/ClientApp/src/app/shared/cache-service/cache.service.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ export class CacheService {
3232
}
3333

3434
const textDocId = new TextDocId(project.id, text.bookNum, chapter.number, 'target');
35-
if (await this.permissionsService.canAccessText(textDocId)) {
35+
if (await this.permissionsService.canAccessTextAsync(textDocId)) {
3636
await this.projectService.getText(textDocId);
3737
}
3838

3939
if (text.hasSource && sourceId != null) {
4040
const sourceTextDocId = new TextDocId(sourceId, text.bookNum, chapter.number, 'target');
41-
if (await this.permissionsService.canAccessText(sourceTextDocId)) {
41+
if (await this.permissionsService.canAccessTextAsync(sourceTextDocId)) {
4242
await this.projectService.getText(sourceTextDocId);
4343
}
4444
}

‎src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ describe('progress service', () => {
139139

140140
it('cannot train suggestions if no source permission', fakeAsync(async () => {
141141
const env = new TestEnvironment();
142-
when(mockPermissionService.canAccessText(anything())).thenCall((textDocId: TextDocId) => {
142+
when(mockPermissionService.canAccessTextAsync(anything())).thenCall((textDocId: TextDocId) => {
143143
return Promise.resolve(textDocId.projectId !== 'sourceId');
144144
});
145145
tick();
@@ -192,7 +192,7 @@ class TestEnvironment {
192192
when(this.mockProject.id).thenReturn('project01');
193193
when(mockProjectService.changes$).thenReturn(this.project$);
194194

195-
when(mockPermissionService.canAccessText(anything())).thenResolve(true);
195+
when(mockPermissionService.canAccessTextAsync(anything())).thenResolve(true);
196196
when(mockSFProjectService.getProfile('project01')).thenResolve({
197197
data,
198198
id: 'project01',

‎src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ export class ProgressService extends DataLoadingComponent implements OnDestroy {
304304

305305
// Only retrieve the source text if the user has permission
306306
let sourceNonEmptyVerses: string[] = [];
307-
if (await this.permissionsService.canAccessText(sourceTextDocId)) {
307+
if (await this.permissionsService.canAccessTextAsync(sourceTextDocId)) {
308308
const sourceChapterText: TextDoc = await this.projectService.getText(sourceTextDocId);
309309
sourceNonEmptyVerses = sourceChapterText.getNonEmptyVerses();
310310
}

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4920,6 +4920,7 @@ class TestEnvironment {
49204920
when(mockedDraftGenerationService.getLastCompletedBuild(anything())).thenReturn(of({} as BuildDto));
49214921
when(mockedDraftOptionsService.areFormattingOptionsAvailableButUnselected(anything())).thenReturn(false);
49224922
when(mockedPermissionsService.isUserOnProject(anything())).thenResolve(true);
4923+
when(mockedPermissionsService.canAccessText(anything(), anything())).thenReturn(true);
49234924
when(mockedFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(false));
49244925
when(mockedLynxWorkspaceService.rawInsightSource$).thenReturn(of([]));
49254926

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ import { isParatextRole, SFProjectRole } from 'realtime-server/lib/esm/scripture
5858
import { TextAnchor } from 'realtime-server/lib/esm/scriptureforge/models/text-anchor';
5959
import { TextType } from 'realtime-server/lib/esm/scriptureforge/models/text-data';
6060
import { Chapter, TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
61-
import { TextInfoPermission } from 'realtime-server/lib/esm/scriptureforge/models/text-info-permission';
6261
import { TranslateSource } from 'realtime-server/lib/esm/scriptureforge/models/translate-config';
6362
import { fromVerseRef } from 'realtime-server/lib/esm/scriptureforge/models/verse-ref-data';
6463
import { DeltaOperation } from 'rich-text';
@@ -324,7 +323,6 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy,
324323
private isParatextUserRole: boolean = false;
325324
private projectUserConfigChangesSub?: Subscription;
326325
private text?: TextInfo;
327-
private sourceText?: TextInfo;
328326
sourceProjectDoc?: SFProjectProfileDoc;
329327
private _unsupportedTags = new Set<string>();
330328
private sourceLoaded: boolean = false;
@@ -535,20 +533,7 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy,
535533
return this.isParatextUserRole;
536534
}
537535

538-
if (
539-
SF_PROJECT_RIGHTS.hasRight(sourceProject, this.userService.currentUserId, SFProjectDomain.Texts, Operation.View)
540-
) {
541-
// Check for chapter rights
542-
const chapter = this.sourceText?.chapters.find(c => c.number === this.chapter);
543-
// Even though permissions is guaranteed to be there in the model, its not in IndexedDB the first time the project
544-
// is accessed after migration
545-
if (chapter != null && chapter.permissions != null && !this.isParatextUserRole) {
546-
const chapterPermission: string = chapter.permissions[this.userService.currentUserId];
547-
return chapterPermission === TextInfoPermission.Write || chapterPermission === TextInfoPermission.Read;
548-
}
549-
}
550-
551-
return this.isParatextUserRole;
536+
return this.isParatextUserRole && this.permissionsService.canAccessText(this.source?.id, sourceProject);
552537
}
553538

554539
get canInsertNote(): boolean {
@@ -846,10 +831,6 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy,
846831
return;
847832
}
848833

849-
if (this.sourceProjectDoc?.data != null) {
850-
this.sourceText = this.sourceProjectDoc.data.texts.find(t => t.bookNum === bookNum);
851-
}
852-
853834
this.chapters = this.text.chapters.map(c => c.number);
854835

855836
this.updateVerseNumber();

0 commit comments

Comments
 (0)