Skip to content

Commit

Permalink
Obliviate goal/edit index (#2793)
Browse files Browse the repository at this point in the history
  • Loading branch information
imnasnainaec authored Dec 4, 2023
1 parent 0661f5c commit 7c70504
Show file tree
Hide file tree
Showing 13 changed files with 102 additions and 107 deletions.
24 changes: 13 additions & 11 deletions Backend.Tests/Controllers/UserEditControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -217,61 +217,63 @@ public async Task TestAddStepToGoal()
await _userEditRepo.Create(RandomUserEdit());
}
var origUserEdit = await _userEditRepo.Create(RandomUserEdit());
var firstEditGuid = origUserEdit.Edits.First().Guid;

// Generate correct result for comparison.
var modUserEdit = origUserEdit.Clone();
const string stringStep = "This is another step added.";
const int modGoalIndex = 0;
modUserEdit.Edits[modGoalIndex].StepData.Add(stringStep);

// Create and put wrapper object.
var stepWrapperObj = new UserEditStepWrapper(modGoalIndex, stringStep);
var stepWrapperObj = new UserEditStepWrapper(firstEditGuid, stringStep);
await _userEditController.UpdateUserEditStep(_projId, origUserEdit.Id, stepWrapperObj);

// Step count should have increased by 1.
Assert.That(await _userEditRepo.GetAllUserEdits(_projId), Has.Count.EqualTo(count + 1));

var userEdit = await _userEditRepo.GetUserEdit(_projId, origUserEdit.Id);
Assert.That(userEdit, Is.Not.Null);
Assert.That(userEdit!.Edits[modGoalIndex].StepData, Does.Contain(stringStep));
Assert.That(userEdit!.Edits.First().StepData, Does.Contain(stringStep));

// Now update a step within the goal.
const string modStringStep = "This is a replacement step.";
const int modStepIndex = 1;
modUserEdit.Edits[modGoalIndex].StepData[modStepIndex] = modStringStep;

// Create and put wrapper object.
stepWrapperObj = new UserEditStepWrapper(modGoalIndex, modStringStep, modStepIndex);
stepWrapperObj = new UserEditStepWrapper(firstEditGuid, modStringStep, modStepIndex);
await _userEditController.UpdateUserEditStep(_projId, origUserEdit.Id, stepWrapperObj);

// Step count should not have further increased.
Assert.That(await _userEditRepo.GetAllUserEdits(_projId), Has.Count.EqualTo(count + 1));

userEdit = await _userEditRepo.GetUserEdit(_projId, origUserEdit.Id);
Assert.That(userEdit, Is.Not.Null);
Assert.That(userEdit!.Edits[modGoalIndex].StepData, Does.Contain(modStringStep));
Assert.That(userEdit!.Edits.First().StepData, Does.Contain(modStringStep));
}

[Test]
public async Task TestUpdateUserEditStepNoPermissions()
{
_userEditController.ControllerContext.HttpContext = PermissionServiceMock.UnauthorizedHttpContext();
var userEdit = await _userEditRepo.Create(RandomUserEdit());
var stepWrapper = new UserEditStepWrapper(0, "A new step");
var stepWrapper = new UserEditStepWrapper(userEdit.Edits.First().Guid, "A new step");
var result = await _userEditController.UpdateUserEditStep(_projId, userEdit.Id, stepWrapper);
Assert.That(result, Is.InstanceOf<ForbidResult>());
}

[Test]
public async Task TestUpdateUserEditStepMissingIds()
{
var stepWrapper = new UserEditStepWrapper(0, "A new step");

var userEdit = await _userEditRepo.Create(RandomUserEdit());
var stepWrapper = new UserEditStepWrapper(userEdit.Edits.First().Guid, "step");

var noProjResult = await _userEditController.UpdateUserEditStep(MissingId, userEdit.Id, stepWrapper);
Assert.That(noProjResult, Is.InstanceOf<NotFoundObjectResult>());

var noEditResult = await _userEditController.UpdateUserEditStep(_projId, MissingId, stepWrapper);
var noUserEditResult = await _userEditController.UpdateUserEditStep(_projId, MissingId, stepWrapper);
Assert.That(noUserEditResult, Is.InstanceOf<NotFoundObjectResult>());

var diffGuidWrapper = new UserEditStepWrapper(Guid.NewGuid(), "step");
var noEditResult = await _userEditController.UpdateUserEditStep(_projId, userEdit.Id, diffGuidWrapper);
Assert.That(noEditResult, Is.InstanceOf<NotFoundObjectResult>());
}

Expand Down
20 changes: 12 additions & 8 deletions Backend.Tests/Models/UserEditTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,30 @@ public void TestHashCode()

public class UserEditStepWrapperTests
{
private const int GoalIndex = 1;
private readonly Guid EditGuid = Guid.NewGuid();
private const string StepString = "step";
private const int StepIndex = 1;

[Test]
public void TestEquals()
{
var wrapper = new UserEditStepWrapper(GoalIndex, StepString, StepIndex);
Assert.That(wrapper.Equals(new UserEditStepWrapper(GoalIndex, StepString, StepIndex)), Is.True);
Assert.That(wrapper.Equals(new UserEditStepWrapper(99, StepString, StepIndex)), Is.False);
Assert.That(wrapper.Equals(new UserEditStepWrapper(GoalIndex, "Different step", StepIndex)), Is.False);
Assert.That(wrapper.Equals(new UserEditStepWrapper(GoalIndex, StepString, 99)), Is.False);
var wrapper = new UserEditStepWrapper(EditGuid, StepString, StepIndex);
Assert.That(wrapper.Equals(new UserEditStepWrapper(EditGuid, StepString, StepIndex)), Is.True);
Assert.That(wrapper.Equals(new UserEditStepWrapper(Guid.NewGuid(), StepString, StepIndex)), Is.False);
Assert.That(wrapper.Equals(new UserEditStepWrapper(EditGuid, "Different step", StepIndex)), Is.False);
Assert.That(wrapper.Equals(new UserEditStepWrapper(EditGuid, StepString, 99)), Is.False);
Assert.That(wrapper.Equals(null), Is.False);
}

[Test]
public void TestHashCode()
{
Assert.That(new UserEditStepWrapper(GoalIndex, StepString, StepIndex).GetHashCode(),
Is.Not.EqualTo(new UserEditStepWrapper(99, StepString, StepIndex).GetHashCode()));
var code = new UserEditStepWrapper(EditGuid, StepString, StepIndex).GetHashCode();
Assert.That(code,
Is.Not.EqualTo(new UserEditStepWrapper(Guid.NewGuid(), StepString, StepIndex).GetHashCode()));
Assert.That(code,
Is.Not.EqualTo(new UserEditStepWrapper(EditGuid, "Different step", StepIndex).GetHashCode()));
Assert.That(code, Is.Not.EqualTo(new UserEditStepWrapper(EditGuid, StepString, 99).GetHashCode()));
}
}

Expand Down
39 changes: 16 additions & 23 deletions Backend/Controllers/UserEditController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ public async Task<IActionResult> CreateUserEdit(string projectId)
}

/// <summary> Adds/updates a goal to/in a specified <see cref="UserEdit"/> </summary>
/// <returns> Index of added/updated edit </returns>
/// <returns> Guid of added/updated edit </returns>
[HttpPost("{userEditId}", Name = "UpdateUserEditGoal")]
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(int))]
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(string))]
public async Task<IActionResult> UpdateUserEditGoal(
string projectId, string userEditId, [FromBody, BindRequired] Edit newEdit)
{
Expand All @@ -142,35 +142,27 @@ public async Task<IActionResult> UpdateUserEditGoal(
return NotFound($"projectId: {projectId}");
}

// Ensure userEdit exists
var toBeMod = await _userEditRepo.GetUserEdit(projectId, userEditId);
if (toBeMod is null)
{
return NotFound($"userEditId: {userEditId}");
}

var (isSuccess, editIndex) = await _userEditService.AddGoalToUserEdit(projectId, userEditId, newEdit);
var (isSuccess, editGuid) = await _userEditService.AddGoalToUserEdit(projectId, userEditId, newEdit);

if (editIndex == -1)
if (editGuid is null)
{
return BadRequest("UserEdit found but update failed.");
return NotFound($"userEditId: {userEditId}");
}

// If the replacement was successful
if (isSuccess)
{
return Ok(editIndex);
return Ok(editGuid);
}

return StatusCode(StatusCodes.Status304NotModified, editIndex);
return StatusCode(StatusCodes.Status304NotModified, editGuid);
}

/// <summary> Adds/updates a step to/in specified goal </summary>
/// <returns> Index of added/modified step in specified goal </returns>
[HttpPut("{userEditId}", Name = "UpdateUserEditStep")]
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(int))]
public async Task<IActionResult> UpdateUserEditStep(string projectId, string userEditId,
[FromBody, BindRequired] UserEditStepWrapper stepEdit)
[FromBody, BindRequired] UserEditStepWrapper stepWrapper)
{
if (!await _permissionService.HasProjectPermission(
HttpContext, Permission.MergeAndReviewEntries, projectId))
Expand Down Expand Up @@ -198,13 +190,14 @@ public async Task<IActionResult> UpdateUserEditStep(string projectId, string use
return NotFound(projectId);
}

// Ensure indices exist.
if (stepEdit.GoalIndex < 0 || stepEdit.GoalIndex >= document.Edits.Count)
// Ensure Edit exist.
var edit = document.Edits.FindLast(e => e.Guid == stepWrapper.EditGuid);
if (edit is null)
{
return BadRequest("Goal index out of range.");
return NotFound(stepWrapper.EditGuid);
}
var maxStepIndex = document.Edits[stepEdit.GoalIndex].StepData.Count;
var stepIndex = stepEdit.StepIndex ?? maxStepIndex;
var maxStepIndex = edit.StepData.Count;
var stepIndex = stepWrapper.StepIndex ?? maxStepIndex;
if (stepIndex < 0 || stepIndex > maxStepIndex)
{
return BadRequest("Step index out of range.");
Expand All @@ -214,12 +207,12 @@ public async Task<IActionResult> UpdateUserEditStep(string projectId, string use
if (stepIndex == maxStepIndex)
{
await _userEditService.AddStepToGoal(
projectId, userEditId, stepEdit.GoalIndex, stepEdit.StepString);
projectId, userEditId, stepWrapper.EditGuid, stepWrapper.StepString);
}
else
{
await _userEditService.UpdateStepInGoal(
projectId, userEditId, stepEdit.GoalIndex, stepEdit.StepString, stepIndex);
projectId, userEditId, stepWrapper.EditGuid, stepWrapper.StepString, stepIndex);
}

return Ok(stepIndex);
Expand Down
6 changes: 3 additions & 3 deletions Backend/Interfaces/IUserEditService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ namespace BackendFramework.Interfaces
{
public interface IUserEditService
{
Task<Tuple<bool, int>> AddGoalToUserEdit(string projectId, string userEditId, Edit edit);
Task<bool> AddStepToGoal(string projectId, string userEditId, int goalIndex, string stepString);
Task<Tuple<bool, Guid?>> AddGoalToUserEdit(string projectId, string userEditId, Edit edit);
Task<bool> AddStepToGoal(string projectId, string userEditId, Guid editGuid, string stepString);
Task<bool> UpdateStepInGoal(
string projectId, string userEditId, int goalIndex, string stepString, int stepIndex);
string projectId, string userEditId, Guid editGuid, string stepString, int stepIndex);
}
}
13 changes: 5 additions & 8 deletions Backend/Models/UserEdit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,18 @@ public override int GetHashCode()
public class UserEditStepWrapper
{
[Required]
[BsonElement("goalIndex")]
public int GoalIndex { get; set; }
public Guid EditGuid { get; set; }

[Required]
[BsonElement("stepString")]
public string StepString { get; set; }

/* A null StepIndex implies index equal to the length of the step list--
* i.e. the step is to be added to the end of the list. */
[BsonElement("stepIndex")]
public int? StepIndex { get; set; }

public UserEditStepWrapper(int goalIndex, string stepString, int? stepIndex = null)
public UserEditStepWrapper(Guid editGuid, string stepString, int? stepIndex = null)
{
GoalIndex = goalIndex;
EditGuid = editGuid;
StepString = stepString;
StepIndex = stepIndex;
}
Expand All @@ -100,14 +97,14 @@ public override bool Equals(object? obj)
return false;
}

return other.GoalIndex == GoalIndex &&
return other.EditGuid == EditGuid &&
other.StepString.Equals(StepString, StringComparison.Ordinal) &&
other.StepIndex == StepIndex;
}

public override int GetHashCode()
{
return HashCode.Combine(GoalIndex, StepString, StepIndex);
return HashCode.Combine(EditGuid, StepString, StepIndex);
}
}

Expand Down
72 changes: 37 additions & 35 deletions Backend/Services/UserEditService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,72 +21,74 @@ public UserEditService(IUserEditRepository userEditRepo)
/// </summary>
/// <returns>
/// Tuple of
/// bool: true if edit replaced, false if nothing modified
/// int: index at which the edit was placed or -1 on failure
/// bool: true if Edit added/updated, false if nothing modified
/// Guid?: guid of added/updated Edit, or null if UserEdit not found
/// </returns>
public async Task<Tuple<bool, int>> AddGoalToUserEdit(string projectId, string userEditId, Edit edit)
public async Task<Tuple<bool, Guid?>> AddGoalToUserEdit(string projectId, string userEditId, Edit edit)
{
// Get userEdit to change
var oldUserEdit = await _userEditRepo.GetUserEdit(projectId, userEditId);
const int invalidEditIndex = -1;
var failureResult = new Tuple<bool, int>(false, invalidEditIndex);
if (oldUserEdit is null)
var userEdit = await _userEditRepo.GetUserEdit(projectId, userEditId);
if (userEdit is null)
{
return failureResult;
return new Tuple<bool, Guid?>(false, null);
}

var newUserEdit = oldUserEdit.Clone();

// Update existing Edit if guid exists, otherwise add new one at end of List.
var indexOfNewestEdit = newUserEdit.Edits.FindIndex(e => e.Guid == edit.Guid);
if (indexOfNewestEdit > invalidEditIndex)
var editIndex = userEdit.Edits.FindLastIndex(e => e.Guid == edit.Guid);
if (editIndex > -1)
{
newUserEdit.Edits[indexOfNewestEdit] = edit;
userEdit.Edits[editIndex] = edit;
}
else
{
newUserEdit.Edits.Add(edit);
indexOfNewestEdit = newUserEdit.Edits.Count - 1;
userEdit.Edits.Add(edit);
}

// Replace the old UserEdit object with the new one that contains the new/updated edit
var editReplaced = await _userEditRepo.Replace(projectId, userEditId, newUserEdit);
var editReplaced = await _userEditRepo.Replace(projectId, userEditId, userEdit);

return new Tuple<bool, int>(editReplaced, indexOfNewestEdit);
return new Tuple<bool, Guid?>(editReplaced, edit.Guid);
}

/// <summary> Adds a string representation of a step to a specified <see cref="Edit"/> </summary>
/// <returns> A bool: success of operation </returns>
public async Task<bool> AddStepToGoal(string projectId, string userEditId, int goalIndex, string stepString)
public async Task<bool> AddStepToGoal(string projectId, string userEditId, Guid editGuid, string stepString)
{
var oldUserEdit = await _userEditRepo.GetUserEdit(projectId, userEditId);
if (oldUserEdit is null || goalIndex >= oldUserEdit.Edits.Count)
var userEdit = await _userEditRepo.GetUserEdit(projectId, userEditId);
if (userEdit is null)
{
return false;
}

var newUserEdit = oldUserEdit.Clone();
newUserEdit.Edits[goalIndex].StepData.Add(stepString);
var updateResult = await _userEditRepo.Replace(projectId, userEditId, newUserEdit);
return updateResult;
var edit = userEdit.Edits.FindLast(e => e.Guid == editGuid);
if (edit is null)
{
return false;
}
edit.StepData.Add(stepString);
return await _userEditRepo.Replace(projectId, userEditId, userEdit);
}

/// <summary> Updates a specified step to in a specified <see cref="Edit"/> </summary>
/// <summary> Updates a specified step in a specified <see cref="Edit"/> </summary>
/// <returns> A bool: success of operation </returns>
public async Task<bool> UpdateStepInGoal(
string projectId, string userEditId, int goalIndex, string stepString, int stepIndex)
string projectId, string userEditId, Guid editGuid, string stepString, int stepIndex)
{
var oldUserEdit = await _userEditRepo.GetUserEdit(projectId, userEditId);
if (oldUserEdit is null || goalIndex >= oldUserEdit.Edits.Count
|| stepIndex >= oldUserEdit.Edits[goalIndex].StepData.Count)
if (stepIndex < 0)
{
return false;
}

var newUserEdit = oldUserEdit.Clone();
newUserEdit.Edits[goalIndex].StepData[stepIndex] = stepString;
var updateResult = await _userEditRepo.Replace(projectId, userEditId, newUserEdit);
return updateResult;
var userEdit = await _userEditRepo.GetUserEdit(projectId, userEditId);
if (userEdit is null)
{
return false;
}
var edit = userEdit.Edits.FindLast(e => e.Guid == editGuid);
if (edit is null || stepIndex >= edit.StepData.Count)
{
return false;
}
edit.StepData[stepIndex] = stepString;
return await _userEditRepo.Replace(projectId, userEditId, userEdit);
}
}
}
4 changes: 2 additions & 2 deletions src/api/api/user-edit-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ export const UserEditApiFp = function (configuration?: Configuration) {
edit: Edit,
options?: any
): Promise<
(axios?: AxiosInstance, basePath?: string) => AxiosPromise<number>
(axios?: AxiosInstance, basePath?: string) => AxiosPromise<string>
> {
const localVarAxiosArgs =
await localVarAxiosParamCreator.updateUserEditGoal(
Expand Down Expand Up @@ -613,7 +613,7 @@ export const UserEditApiFactory = function (
userEditId: string,
edit: Edit,
options?: any
): AxiosPromise<number> {
): AxiosPromise<string> {
return localVarFp
.updateUserEditGoal(projectId, userEditId, edit, options)
.then((request) => request(axios, basePath));
Expand Down
4 changes: 2 additions & 2 deletions src/api/models/user-edit-step-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@
export interface UserEditStepWrapper {
/**
*
* @type {number}
* @type {string}
* @memberof UserEditStepWrapper
*/
goalIndex: number;
editGuid: string;
/**
*
* @type {string}
Expand Down
Loading

0 comments on commit 7c70504

Please sign in to comment.