Skip to content

Commit

Permalink
When removing from graylist, don't remove subsets (#2670)
Browse files Browse the repository at this point in the history
  • Loading branch information
@imnasnainaec
imnasnainaec authored Oct 10, 2023
1 parent e30eac5 commit b50bb01
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 30 deletions.
54 changes: 32 additions & 22 deletions Backend.Tests/Services/MergeServiceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,6 @@ public void UndoMergeMultiChildTest()
[Test]
public void AddMergeToBlacklistTest()
{
_ = _mergeBlacklistRepo.DeleteAllSets(ProjId).Result;
_ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result;
var wordIds = new List<string> { "1", "2" };

// Adding to blacklist should clear from graylist
Expand All @@ -219,7 +217,6 @@ public void AddMergeToBlacklistTest()
[Test]
public void AddMergeToBlacklistErrorTest()
{
_ = _mergeBlacklistRepo.DeleteAllSets(ProjId).Result;
var wordIds0 = new List<string>();
var wordIds1 = new List<string> { "1" };
Assert.That(
Expand All @@ -231,7 +228,6 @@ public void AddMergeToBlacklistErrorTest()
[Test]
public void IsInMergeBlacklistTest()
{
_ = _mergeBlacklistRepo.DeleteAllSets(ProjId).Result;
var wordIds = new List<string> { "1", "2", "3" };
var subWordIds = new List<string> { "3", "2" };

Expand All @@ -243,7 +239,6 @@ public void IsInMergeBlacklistTest()
[Test]
public void IsInMergeBlacklistErrorTest()
{
_ = _mergeBlacklistRepo.DeleteAllSets(ProjId).Result;
var wordIds0 = new List<string>();
var wordIds1 = new List<string> { "1" };
Assert.That(
Expand Down Expand Up @@ -298,7 +293,6 @@ public void UpdateMergeBlacklistTest()
[Test]
public void AddMergeToGraylistTest()
{
_ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result;
var wordIds = new List<string> { "1", "2" };
_ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds).Result;
var graylist = _mergeGraylistRepo.GetAllSets(ProjId).Result;
Expand All @@ -307,10 +301,24 @@ public void AddMergeToGraylistTest()
Assert.That(expectedEntry.ContentEquals(graylist.First()), Is.True);
}

[Test]
public void AddMergeToGraylistSupersetTest()
{
var wordIds12 = new List<string> { "1", "2" };
var wordIds13 = new List<string> { "1", "3" };
var wordIds123 = new List<string> { "1", "2", "3" };

_ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds12).Result;
_ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds13).Result;
Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(2));

_ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds123).Result;
Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(1));
}

[Test]
public void AddMergeToGraylistErrorTest()
{
_ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result;
var wordIds = new List<string>();
var wordIds1 = new List<string> { "1" };
Assert.That(
Expand All @@ -324,24 +332,28 @@ public void AddMergeToGraylistErrorTest()
[Test]
public void RemoveFromMergeGraylistTest()
{
_ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result;
var wordIds12 = new List<string> { "1", "2" };
var wordIds13 = new List<string> { "1", "3" };
_ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds12).Result;
_ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds13).Result;
var graylist = _mergeGraylistRepo.GetAllSets(ProjId).Result;
Assert.That(graylist, Has.Count.EqualTo(2));
var wordIds123 = new List<string> { "1", "2", "3" };
var removed = _mergeService.RemoveFromMergeGraylist(ProjId, UserId, wordIds123).Result;
Assert.That(removed, Has.Count.EqualTo(2));
graylist = _mergeGraylistRepo.GetAllSets(ProjId).Result;
Assert.That(graylist, Is.Empty);
var wordIds = new List<string> { "1", "2", "3" };
_ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds).Result;
Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(1));
Assert.That(_mergeService.RemoveFromMergeGraylist(ProjId, UserId, wordIds).Result, Is.True);
Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(0));
}

[Test]
public void RemoveFromMergeGraylistSupersetTest()
{
var wordIds = new List<string> { "1", "2" };
_ = _mergeService.AddToMergeGraylist(ProjId, UserId, wordIds).Result;
Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(1));

wordIds.Add("3");
Assert.That(_mergeService.RemoveFromMergeGraylist(ProjId, UserId, wordIds).Result, Is.False);
Assert.That(_mergeGraylistRepo.GetAllSets(ProjId).Result, Has.Count.EqualTo(1));
}

[Test]
public void RemoveFromMergeGraylistErrorTest()
{
_ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result;
var wordIds = new List<string>();
var wordIds1 = new List<string> { "1" };
Assert.That(
Expand All @@ -355,7 +367,6 @@ public void RemoveFromMergeGraylistErrorTest()
[Test]
public void IsInMergeGraylistTest()
{
_ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result;
var wordIds = new List<string> { "1", "2", "3" };
var subWordIds = new List<string> { "3", "2" };

Expand All @@ -367,7 +378,6 @@ public void IsInMergeGraylistTest()
[Test]
public void IsInMergeGraylistErrorTest()
{
_ = _mergeGraylistRepo.DeleteAllSets(ProjId).Result;
var wordIds0 = new List<string>();
var wordIds1 = new List<string> { "1" };
Assert.That(
Expand Down
2 changes: 1 addition & 1 deletion Backend/Interfaces/IMergeService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public interface IMergeService
Task<bool> UndoMerge(string projectId, MergeUndoIds ids);
Task<MergeWordSet> AddToMergeBlacklist(string projectId, string userId, List<string> wordIds);
Task<MergeWordSet> AddToMergeGraylist(string projectId, string userId, List<string> wordIds);
Task<List<string>> RemoveFromMergeGraylist(string projectId, string userId, List<string> wordIds);
Task<bool> RemoveFromMergeGraylist(string projectId, string userId, List<string> wordIds);
Task<bool> IsInMergeBlacklist(string projectId, List<string> wordIds, string? userId = null);
Task<bool> IsInMergeGraylist(string projectId, List<string> wordIds, string? userId = null);
Task<int> UpdateMergeBlacklist(string projectId);
Expand Down
15 changes: 8 additions & 7 deletions Backend/Services/MergeService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ public async Task<MergeWordSet> AddToMergeBlacklist(
public async Task<MergeWordSet> AddToMergeGraylist(
string projectId, string userId, List<string> wordIds)
{
wordIds = wordIds.Distinct().ToList();
if (wordIds.Count < 2)
{
throw new InvalidMergeWordSetException("Cannot graylist a list of fewer than 2 wordIds.");
Expand All @@ -169,27 +170,27 @@ public async Task<MergeWordSet> AddToMergeGraylist(

/// <summary> Remove a List of wordIds from MergeGraylist of specified <see cref="Project"/>. </summary>
/// <exception cref="InvalidMergeWordSetException"> Throws when wordIds has count less than 2. </exception>
/// <returns> List of removed <see cref="MergeWordSet"/> ids. </returns>
public async Task<List<string>> RemoveFromMergeGraylist(
/// <returns> Boolean indicating whether anything was removed. </returns>
public async Task<bool> RemoveFromMergeGraylist(
string projectId, string userId, List<string> wordIds)
{
wordIds = wordIds.Distinct().ToList();
if (wordIds.Count < 2)
{
throw new InvalidMergeWordSetException("Cannot have a graylist entry with fewer than 2 wordIds.");
}

// Remove all graylist entries fully contained in the input List.
// Remove only the set of wordIds, if in graylist (not any subsets)
var graylist = await _mergeGraylistRepo.GetAllSets(projectId, userId);
var removed = new List<string>();
foreach (var entry in graylist)
{
if (entry.WordIds.All(wordIds.Contains))
if (entry.WordIds.All(wordIds.Contains) && wordIds.Count == entry.WordIds.Count)
{
await _mergeGraylistRepo.Delete(projectId, entry.Id);
removed.Add(entry.Id);
return true;
}
}
return removed;
return false;
}

/// <summary> Check if List of wordIds is in MergeBlacklist for specified <see cref="Project"/>. </summary>
Expand Down

0 comments on commit b50bb01

Please sign in to comment.