Skip to content

Commit fd922d7

Browse files
pmachapmanpmachapman
committed
SF-3637 Backend changes
1 parent 558b548 commit fd922d7

File tree

6 files changed

+89
-69
lines changed

6 files changed

+89
-69
lines changed

‎src/SIL.XForge.Scripture/Services/MachineApiService.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -488,12 +488,13 @@ await hubContext.NotifyDraftApplyProgress(
488488
continue;
489489
}
490490

491-
bool canWriteChapter =
492-
targetProjectDoc
493-
.Data.Texts[textIndex]
494-
.Chapters[chapterIndex]
495-
.Permissions.TryGetValue(curUserId, out string chapterPermission)
496-
&& chapterPermission == TextInfoPermission.Write;
491+
// If a user does not have permission set at chapter level, use the book level permission
492+
bool canWriteChapter = targetProjectDoc
493+
.Data.Texts[textIndex]
494+
.Chapters[chapterIndex]
495+
.Permissions.TryGetValue(curUserId, out string chapterPermission)
496+
? chapterPermission == TextInfoPermission.Write
497+
: canWriteBook;
497498
if (!canWriteChapter)
498499
{
499500
// Remove the chapter from the project if we created it, and proceed to add the next chapter

‎src/SIL.XForge.Scripture/Services/ParatextService.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -876,7 +876,8 @@ public async Task<Dictionary<string, string>> GetPermissionsAsync(
876876
|| scrText!.Permissions.GetRole(userName) == UserRoles.None
877877
)
878878
{
879-
permissions.Add(uid, TextInfoPermission.None);
879+
// The user will either have read only access or no access
880+
// This will be handled by their role on the project (or lack of role) on the project
880881
}
881882
else
882883
{
@@ -1775,11 +1776,13 @@ await projectDoc.SubmitJson0OpAsync(op =>
17751776
{
17761777
int chapterIndex = j;
17771778
int chapterNumber = text.Chapters[chapterIndex].Number;
1778-
if (editableChapters.Contains(chapterNumber) || currentUserIsAdministrator)
1779+
1780+
// Only record chapter level permissions that contradict the book level permissions
1781+
if (!(editableChapters.Contains(chapterNumber) || currentUserIsAdministrator))
17791782
{
17801783
op.Set(
17811784
p => p.Texts[textIndex].Chapters[chapterIndex].Permissions[user.SFUserId],
1782-
TextInfoPermission.Write
1785+
TextInfoPermission.Read
17831786
);
17841787
}
17851788
}

‎src/SIL.XForge.Scripture/Services/ParatextSyncRunner.cs‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,6 +1150,22 @@ internal async Task<Dictionary<int, string>> GetChapterAuthorsAsync(
11501150
SortedList<int, IDocument<TextData>> textDocs
11511151
)
11521152
{
1153+
bool hasWritePermission(string userId, Chapter chapter)
1154+
{
1155+
// If the user has a chapter permission, use that, otherwise fall back to the book permission
1156+
if (!text.Permissions.TryGetValue(userId, out string bookPermission))
1157+
{
1158+
bookPermission = TextInfoPermission.None;
1159+
}
1160+
1161+
if (!chapter.Permissions.TryGetValue(userId, out string chapterPermission))
1162+
{
1163+
chapterPermission = bookPermission;
1164+
}
1165+
1166+
return chapterPermission == TextInfoPermission.Write;
1167+
}
1168+
11531169
// Get all of the last editors for the chapters.
11541170
var chapterAuthors = new Dictionary<int, string>();
11551171
foreach (Chapter chapter in text.Chapters)
@@ -1167,11 +1183,7 @@ SortedList<int, IDocument<TextData>> textDocs
11671183
userSFId = await _realtimeService.GetLastModifiedUserIdAsync<TextData>(textId, version);
11681184

11691185
// Check that this user still has write permissions
1170-
if (
1171-
string.IsNullOrEmpty(userSFId)
1172-
|| !chapter.Permissions.TryGetValue(userSFId, out string permission)
1173-
|| permission != TextInfoPermission.Write
1174-
)
1186+
if (string.IsNullOrEmpty(userSFId) || !hasWritePermission(userSFId, chapter))
11751187
{
11761188
// They no longer have write access, so reset the user id, and find it below
11771189
userSFId = null;
@@ -1182,18 +1194,17 @@ SortedList<int, IDocument<TextData>> textDocs
11821194
if (string.IsNullOrEmpty(userSFId))
11831195
{
11841196
// See if the current user has permissions
1185-
if (
1186-
chapter.Permissions.TryGetValue(_userSecret.Id, out string permission)
1187-
&& permission == TextInfoPermission.Write
1188-
)
1197+
if (hasWritePermission(_userSecret.Id, chapter))
11891198
{
11901199
userSFId = _userSecret.Id;
11911200
}
11921201
else
11931202
{
11941203
// Get the first user with write permission
11951204
// NOTE: As a KeyValuePair is a struct, we do not need a null-conditional (key will be null)
1196-
userSFId = chapter.Permissions.FirstOrDefault(p => p.Value == TextInfoPermission.Write).Key;
1205+
userSFId =
1206+
chapter.Permissions.FirstOrDefault(p => p.Value == TextInfoPermission.Write).Key
1207+
?? text.Permissions.FirstOrDefault(p => p.Value == TextInfoPermission.Write).Key;
11971208

11981209
// If the userId is still null, find a project administrator, as they can escalate privilege
11991210
if (string.IsNullOrEmpty(userSFId))

‎src/SIL.XForge.Scripture/Services/SFProjectService.cs‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1405,19 +1405,23 @@ public async Task UpdatePermissionsAsync(
14051405
}
14061406
Models.TextInfo text = projectDoc.Data.Texts[textIndex];
14071407
List<Chapter> chapters = text.Chapters;
1408-
Dictionary<string, string> bookPermissions = null;
1408+
Dictionary<string, string> bookPermissions;
14091409
IEnumerable<(
14101410
int bookIndex,
14111411
int chapterIndex,
14121412
Dictionary<string, string> chapterPermissions
1413-
)> chapterPermissionsInBook = null;
1413+
)> chapterPermissionsInBook;
14141414

14151415
if (isResource)
14161416
{
1417-
bookPermissions = resourcePermissions;
1418-
// Prepare to write the same resource permission for each chapter in the book/text.
1417+
// Do not write any read permissions for resources
1418+
bookPermissions = resourcePermissions
1419+
.Where(kvp => kvp.Value != TextInfoPermission.Read)
1420+
.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
1421+
1422+
// Chapter permissions will not vary from book permissions for resources
14191423
chapterPermissionsInBook = chapters.Select(
1420-
(Chapter chapter, int chapterIndex) => (textIndex, chapterIndex, bookPermissions)
1424+
(_, chapterIndex) => (textIndex, chapterIndex, new Dictionary<string, string>())
14211425
);
14221426
}
14231427
else
@@ -1444,11 +1448,24 @@ Dictionary<string, string> chapterPermissions
14441448
chapter.Number,
14451449
token
14461450
);
1451+
1452+
// Only save chapter permissions where they differ from the book permissions
1453+
chapterPermissions = chapterPermissions
1454+
.Where(kvp =>
1455+
!bookPermissions.ContainsKey(kvp.Key) || bookPermissions[kvp.Key] != kvp.Value
1456+
)
1457+
.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
14471458
return (textIndex, chapterIndex, chapterPermissions);
14481459
}
14491460
)
14501461
);
1462+
1463+
// Only save write permissions - all other users will have read if they are on the project
1464+
bookPermissions = bookPermissions
1465+
.Where(kvp => kvp.Value != TextInfoPermission.Read)
1466+
.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
14511467
}
1468+
14521469
projectChapterPermissions.AddRange(chapterPermissionsInBook);
14531470
projectBookPermissions.Add((textIndex, bookPermissions));
14541471
}

‎test/SIL.XForge.Scripture.Tests/Services/ParatextServiceTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ public async Task GetPermissionsAsync_AllBooksAndAutomaticBooks_HasBookLevelPerm
469469
ptUsernameMapping,
470470
40
471471
);
472-
string[] expected = [TextInfoPermission.Write, TextInfoPermission.None, TextInfoPermission.None];
472+
string[] expected = [TextInfoPermission.Write];
473473
Assert.That(permissions.Values, Is.EquivalentTo(expected));
474474
// User01 permission to Mark explicitly and automatically
475475
permissions = await env.Service.GetPermissionsAsync(user01Secret, project, ptUsernameMapping, 41);
@@ -503,8 +503,8 @@ public async Task GetPermissionsAsync_AllBooksAndAutomaticBooks_IsConsultant()
503503
40
504504
);
505505

506-
// Ensure the user has read only access to Matthew and Mark
507-
string[] expected = [TextInfoPermission.Read, TextInfoPermission.None, TextInfoPermission.None];
506+
// Ensure the user has read only access to Matthew
507+
string[] expected = [TextInfoPermission.Read];
508508
Assert.That(permissions.Values, Is.EquivalentTo(expected));
509509
}
510510

@@ -533,11 +533,11 @@ public async Task GetPermissionsAsync_AutomaticBooks_HasBookLevelPermission()
533533
ptUsernameMapping,
534534
40
535535
);
536-
string[] expected = [TextInfoPermission.Read, TextInfoPermission.None, TextInfoPermission.None];
536+
string[] expected = [TextInfoPermission.Read];
537537
Assert.That(permissions.Values, Is.EquivalentTo(expected));
538538
// User01 has permission to Mark automatically
539539
permissions = await env.Service.GetPermissionsAsync(user01Secret, project, ptUsernameMapping, 41);
540-
expected = [TextInfoPermission.Write, TextInfoPermission.None, TextInfoPermission.None];
540+
expected = [TextInfoPermission.Write];
541541
Assert.That(permissions.Values, Is.EquivalentTo(expected));
542542
}
543543

@@ -6689,7 +6689,7 @@ public async Task UpdateParatextPermissionsForNewBooksAsync_UpdatesMongo()
66896689
SyncMetricInfo expected = new SyncMetricInfo { Updated = 1 };
66906690
Assert.AreEqual(expected, actual);
66916691
Assert.AreEqual(TextInfoPermission.Write, projectDoc.Data.Texts[0].Permissions[env.User01]);
6692-
Assert.AreEqual(TextInfoPermission.Write, projectDoc.Data.Texts[0].Chapters[0].Permissions[env.User01]);
6692+
Assert.IsFalse(projectDoc.Data.Texts[0].Chapters[0].Permissions.ContainsKey(env.User01));
66936693
}
66946694

66956695
[Test]

‎test/SIL.XForge.Scripture.Tests/Services/SFProjectServiceTests.cs‎

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -970,7 +970,7 @@ public async Task JoinWithShareKeyAsync_PTUserHasPTPermissions()
970970
user = env.GetUser(User03);
971971
Assert.That(user.Sites[SiteId].Projects, Contains.Item(Project05));
972972

973-
Assert.That(project.Texts.First().Permissions[User03], Is.EqualTo(TextInfoPermission.Read));
973+
Assert.That(project.Texts.First().Permissions.ContainsKey(User03), Is.False);
974974
Assert.That(project.Texts.First().Chapters.First().Permissions[User03], Is.EqualTo(TextInfoPermission.Write));
975975

976976
resource = env.GetProject(Resource01);
@@ -981,11 +981,9 @@ public async Task JoinWithShareKeyAsync_PTUserHasPTPermissions()
981981
);
982982
Assert.That(resourceUserRole, Is.EqualTo(SFProjectRole.PTObserver), "user role not set correctly on resource");
983983
Assert.That(user.Sites[SiteId].Projects, Contains.Item(Resource01), "user not added to resource correctly");
984-
Assert.That(resource.Texts.First().Permissions[User03], Is.EqualTo(userDBLPermissionForResource));
985-
Assert.That(
986-
resource.Texts.First().Chapters.First().Permissions[User03],
987-
Is.EqualTo(userDBLPermissionForResource)
988-
);
984+
// Book and chapter level read permissions are not written for resources
985+
Assert.That(resource.Texts.First().Permissions.ContainsKey(User03), Is.False);
986+
Assert.That(resource.Texts.First().Chapters.First().Permissions.ContainsKey(User03), Is.False);
989987
}
990988

991989
[Test]
@@ -1363,16 +1361,9 @@ await env
13631361
.ParatextService.Received()
13641362
.GetResourcePermissionAsync(Resource01PTId, User03, Arg.Any<CancellationToken>());
13651363
resource = env.GetProject(Resource01);
1366-
Assert.That(
1367-
resource.Texts.First().Permissions[User03],
1368-
Is.EqualTo(userDBLPermissionForResource),
1369-
"resource permissions should have been set for joining project user"
1370-
);
1371-
Assert.That(
1372-
resource.Texts.First().Chapters.First().Permissions[User03],
1373-
Is.EqualTo(userDBLPermissionForResource),
1374-
"resource permissions should have been set for joining project user"
1375-
);
1364+
// Book and chapter level read permissions are not written for resources
1365+
Assert.That(resource.Texts.First().Permissions.ContainsKey(User03), Is.False);
1366+
Assert.That(resource.Texts.First().Chapters.First().Permissions.ContainsKey(User03), Is.False);
13761367
Assert.That(
13771368
resource.UserRoles.TryGetValue(User03, out string resourceUserRole),
13781369
Is.True,
@@ -1905,15 +1896,13 @@ public async Task AddUserAsync_PTUserHasPTPermissions()
19051896
user = env.GetUser(User03);
19061897
Assert.That(user.Sites[SiteId].Projects, Contains.Item(Project05));
19071898

1908-
Assert.That(project.Texts.First().Permissions[User03], Is.EqualTo(TextInfoPermission.Read));
1899+
Assert.That(project.Texts.First().Permissions.ContainsKey(User03), Is.False);
19091900
Assert.That(project.Texts.First().Chapters.First().Permissions[User03], Is.EqualTo(TextInfoPermission.Write));
19101901

19111902
resource = env.GetProject(Resource01);
1912-
Assert.That(resource.Texts.First().Permissions[User03], Is.EqualTo(userDBLPermissionForResource));
1913-
Assert.That(
1914-
resource.Texts.First().Chapters.First().Permissions[User03],
1915-
Is.EqualTo(userDBLPermissionForResource)
1916-
);
1903+
// Book and chapter level read permissions are not written for resources
1904+
Assert.That(resource.Texts.First().Permissions.ContainsKey(User03), Is.False);
1905+
Assert.That(resource.Texts.First().Chapters.First().Permissions.ContainsKey(User03), Is.False);
19171906
Assert.That(
19181907
resource.UserRoles.TryGetValue(User03, out string resourceUserRole),
19191908
Is.True,
@@ -2082,7 +2071,7 @@ public async Task UpdatePermissionsAsync_SkipsBookNotInDB()
20822071

20832072
// Permissions were set for the books and chapters that we were able to handle.
20842073
sfProject = env.GetProject(Project01);
2085-
Assert.That(sfProject.Texts.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Read));
2074+
Assert.That(sfProject.Texts.First().Permissions.ContainsKey(User01), Is.False);
20862075
Assert.That(sfProject.Texts.First().Permissions[User02], Is.EqualTo(TextInfoPermission.Write));
20872076
Assert.That(sfProject.Texts.First().Chapters.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Write));
20882077
Assert.That(sfProject.Texts.First().Chapters.First().Permissions[User02], Is.EqualTo(TextInfoPermission.Read));
@@ -2153,7 +2142,7 @@ public async Task UpdatePermissionsAsync_SetsBookAndChapterPermissions()
21532142
await env.Service.UpdatePermissionsAsync(User01, project01Doc);
21542143

21552144
sfProject = env.GetProject(Project01);
2156-
Assert.That(sfProject.Texts.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Read));
2145+
Assert.That(sfProject.Texts.First().Permissions.ContainsKey(User01), Is.False);
21572146
Assert.That(sfProject.Texts.First().Permissions[User02], Is.EqualTo(TextInfoPermission.Write));
21582147
Assert.That(sfProject.Texts.First().Chapters.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Write));
21592148
Assert.That(sfProject.Texts.First().Chapters.First().Permissions[User02], Is.EqualTo(TextInfoPermission.Read));
@@ -2206,10 +2195,10 @@ public async Task UpdatePermissionsAsync_UserHasNoChapterPermission()
22062195
await env.Service.UpdatePermissionsAsync(User01, project01Doc);
22072196

22082197
sfProject = env.GetProject(Project01);
2209-
Assert.That(sfProject.Texts.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Read));
2198+
Assert.That(sfProject.Texts.First().Permissions.ContainsKey(User01), Is.False);
22102199
Assert.That(sfProject.Texts.First().Permissions[User02], Is.EqualTo(TextInfoPermission.None));
2211-
Assert.That(sfProject.Texts.First().Chapters.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Read));
2212-
Assert.That(sfProject.Texts.First().Chapters.First().Permissions[User02], Is.EqualTo(TextInfoPermission.None));
2200+
Assert.That(sfProject.Texts.First().Chapters.First().Permissions.ContainsKey(User01), Is.False);
2201+
Assert.That(sfProject.Texts.First().Chapters.First().Permissions.ContainsKey(User02), Is.False);
22132202
}
22142203

22152204
[Test]
@@ -2229,7 +2218,7 @@ public async Task UpdatePermissionsAsync_SetsResourcePermissions()
22292218
var ptChapterPermissions = new Dictionary<string, string>
22302219
{
22312220
{ User01, TextInfoPermission.Write },
2232-
{ User02, TextInfoPermission.Write },
2221+
{ User02, TextInfoPermission.Read },
22332222
};
22342223
var ptSourcePermissions = new Dictionary<string, string>
22352224
{
@@ -2284,12 +2273,12 @@ public async Task UpdatePermissionsAsync_SetsResourcePermissions()
22842273
resource = env.GetProject(Resource01);
22852274
Assert.That(sfProject.Texts.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Write));
22862275
Assert.That(sfProject.Texts.First().Permissions[User02], Is.EqualTo(TextInfoPermission.Write));
2287-
Assert.That(sfProject.Texts.First().Chapters.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Write));
2288-
Assert.That(sfProject.Texts.First().Chapters.First().Permissions[User02], Is.EqualTo(TextInfoPermission.Write));
2289-
Assert.That(resource.Texts.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Read));
2276+
Assert.That(sfProject.Texts.First().Chapters.First().Permissions.ContainsKey(User01), Is.False);
2277+
Assert.That(sfProject.Texts.First().Chapters.First().Permissions[User02], Is.EqualTo(TextInfoPermission.Read));
2278+
Assert.That(resource.Texts.First().Permissions.ContainsKey(User01), Is.False);
22902279
Assert.That(resource.Texts.First().Permissions[User02], Is.EqualTo(TextInfoPermission.None));
2291-
Assert.That(resource.Texts.First().Chapters.First().Permissions[User01], Is.EqualTo(TextInfoPermission.Read));
2292-
Assert.That(resource.Texts.First().Chapters.First().Permissions[User02], Is.EqualTo(TextInfoPermission.None));
2280+
Assert.That(resource.Texts.First().Chapters.First().Permissions.ContainsKey(User01), Is.False);
2281+
Assert.That(resource.Texts.First().Chapters.First().Permissions.ContainsKey(User02), Is.False);
22932282
}
22942283

22952284
[Test]
@@ -2722,7 +2711,8 @@ await env
27222711
);
27232712
resource = env.GetProject(Resource01);
27242713
Assert.That(resource.UserRoles.ContainsKey(User01), Is.True);
2725-
Assert.That(resource.Texts.All(t => t.Permissions.ContainsKey(User01)), Is.True);
2714+
// Book and chapter level read permissions are not written for resources
2715+
Assert.That(resource.Texts.All(t => t.Permissions.ContainsKey(User01)), Is.False);
27262716
}
27272717

27282718
[Test]
@@ -3179,11 +3169,9 @@ await env
31793169
// But we can show that source resource permissions were set:
31803170

31813171
resource = env.GetProject(Resource01);
3182-
Assert.That(resource.Texts.First().Permissions[User03], Is.EqualTo(userDBLPermissionForResource));
3183-
Assert.That(
3184-
resource.Texts.First().Chapters.First().Permissions[User03],
3185-
Is.EqualTo(userDBLPermissionForResource)
3186-
);
3172+
// Book and chapter level read permissions are not written for resources
3173+
Assert.That(resource.Texts.First().Permissions.ContainsKey(User03), Is.False);
3174+
Assert.That(resource.Texts.First().Chapters.First().Permissions.ContainsKey(User03), Is.False);
31873175
Assert.That(
31883176
resource.UserRoles.TryGetValue(User03, out string resourceUserRole),
31893177
Is.True,

0 commit comments

Comments
 (0)