Skip to content

Commit 7efc714

Browse files
pmachapmanpmachapman
committed
SF-3637 Backend changes
1 parent 8612d93 commit 7efc714

File tree

6 files changed

+163
-77
lines changed

6 files changed

+163
-77
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: 41 additions & 13 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,25 @@ 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.TryGetValue(kvp.Key, out string bookPermission)
1456+
|| bookPermission != kvp.Value
1457+
)
1458+
.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
14471459
return (textIndex, chapterIndex, chapterPermissions);
14481460
}
14491461
)
14501462
);
1463+
1464+
// Only save write permissions - all other users will have read if they are on the project
1465+
bookPermissions = bookPermissions
1466+
.Where(kvp => kvp.Value != TextInfoPermission.Read)
1467+
.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
14511468
}
1469+
14521470
projectChapterPermissions.AddRange(chapterPermissionsInBook);
14531471
projectBookPermissions.Add((textIndex, bookPermissions));
14541472
}
@@ -1575,19 +1593,24 @@ int lastVerse
15751593
throw new DataNotFoundException("The chapter does not exist.");
15761594
}
15771595

1578-
// Ensure the user has permission for this chapter
1596+
// Ensure the user can write to this chapter
1597+
if (!projectDoc.Data.Texts[textIndex].Permissions.TryGetValue(userId, out string bookPermission))
1598+
{
1599+
bookPermission = TextInfoPermission.None;
1600+
}
1601+
15791602
if (
15801603
!projectDoc
15811604
.Data.Texts[textIndex]
15821605
.Chapters[chapterIndex]
1583-
.Permissions.TryGetValue(userId, out string permission)
1606+
.Permissions.TryGetValue(userId, out string chapterPermission)
15841607
)
15851608
{
1586-
throw new ForbiddenException();
1609+
chapterPermission = bookPermission;
15871610
}
15881611

15891612
// Ensure the user can write to this chapter
1590-
if (permission != TextInfoPermission.Write)
1613+
if (chapterPermission != TextInfoPermission.Write)
15911614
{
15921615
throw new ForbiddenException();
15931616
}
@@ -1644,19 +1667,24 @@ public async Task SetIsValidAsync(string userId, string projectId, int book, int
16441667
throw new DataNotFoundException("The chapter does not exist.");
16451668
}
16461669

1647-
// Ensure the user has permission for this chapter
1670+
// Ensure the user can write to this chapter
1671+
if (!projectDoc.Data.Texts[textIndex].Permissions.TryGetValue(userId, out string bookPermission))
1672+
{
1673+
bookPermission = TextInfoPermission.None;
1674+
}
1675+
16481676
if (
16491677
!projectDoc
16501678
.Data.Texts[textIndex]
16511679
.Chapters[chapterIndex]
1652-
.Permissions.TryGetValue(userId, out string permission)
1680+
.Permissions.TryGetValue(userId, out string chapterPermission)
16531681
)
16541682
{
1655-
throw new ForbiddenException();
1683+
chapterPermission = bookPermission;
16561684
}
16571685

16581686
// Ensure the user can write to this chapter
1659-
if (permission != TextInfoPermission.Write)
1687+
if (chapterPermission != TextInfoPermission.Write)
16601688
{
16611689
throw new ForbiddenException();
16621690
}

‎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]

0 commit comments

Comments
 (0)