From dcf04f040d7bd99599486824f2ba4f55faad739b Mon Sep 17 00:00:00 2001 From: Leendert de Borst Date: Sun, 13 Oct 2024 18:29:36 +0200 Subject: [PATCH] Add attachment delete test and fix bug with IsDeleted flag (#287) --- .../Attachments/AttachmentUploader.razor | 14 +-- .../Attachments/AttachmentViewer.razor | 4 +- .../Components/Forms/CopyPasteFormRow.razor | 19 +++-- .../Forms/CopyPastePasswordFormRow.razor | 26 +++--- .../Main/Pages/Credentials/AddEdit.razor | 85 ++++++++++--------- .../Main/Pages/Credentials/View.razor | 6 +- .../Services/CredentialService.cs | 22 ++--- .../Tests/Client/Shard5/AttachmentTests.cs | 64 +++++++++++++- 8 files changed, 161 insertions(+), 79 deletions(-) diff --git a/src/AliasVault.Client/Main/Components/Attachments/AttachmentUploader.razor b/src/AliasVault.Client/Main/Components/Attachments/AttachmentUploader.razor index 2fa24f1a1..733790c07 100644 --- a/src/AliasVault.Client/Main/Components/Attachments/AttachmentUploader.razor +++ b/src/AliasVault.Client/Main/Components/Attachments/AttachmentUploader.razor @@ -1,4 +1,5 @@ @using System.IO +@inject ILogger Logger
@@ -6,12 +7,12 @@ {

@StatusMessage

} - @if (Attachments.Any()) + @if (Attachments.Exists(x => !x.IsDeleted)) {

Attachments:

    - @foreach (var attachment in Attachments) + @foreach (var attachment in Attachments.Where(x => !x.IsDeleted)) {
  • @attachment.Filename @@ -42,9 +43,12 @@ /// Original attachments that were passed in. This is used to determine if a deleted attachment was part of the original set and /// can be hard deleted (did not exist in the original set) or should be soft deleted (was part of the original set). /// - private List OriginalAttachmentsIds = []; + private List OriginalAttachmentsIds { get; set; } = []; - private string StatusMessage = string.Empty; + /// + /// Status message to display. + /// + private string StatusMessage { get; set; } = string.Empty; /// protected override void OnInitialized() @@ -81,7 +85,7 @@ catch (Exception ex) { StatusMessage = $"Error uploading file: {ex.Message}"; - Console.Error.WriteLine("Error uploading file: {0}", ex.Message); + Logger.LogError(ex, "Error uploading file."); } } diff --git a/src/AliasVault.Client/Main/Components/Attachments/AttachmentViewer.razor b/src/AliasVault.Client/Main/Components/Attachments/AttachmentViewer.razor index af0f63698..e93751003 100644 --- a/src/AliasVault.Client/Main/Components/Attachments/AttachmentViewer.razor +++ b/src/AliasVault.Client/Main/Components/Attachments/AttachmentViewer.razor @@ -2,7 +2,7 @@

    Attachments

    - @if (Attachments.Any()) + @if (Attachments.Any(x => !x.IsDeleted)) {
    @@ -13,7 +13,7 @@ - @foreach (var attachment in Attachments) + @foreach (var attachment in Attachments.Where(x => !x.IsDeleted)) {
    diff --git a/src/AliasVault.Client/Main/Components/Forms/CopyPasteFormRow.razor b/src/AliasVault.Client/Main/Components/Forms/CopyPasteFormRow.razor index df1e5b499..45bd2763a 100644 --- a/src/AliasVault.Client/Main/Components/Forms/CopyPasteFormRow.razor +++ b/src/AliasVault.Client/Main/Components/Forms/CopyPasteFormRow.razor @@ -2,10 +2,10 @@ @inject JsInteropService JsInteropService @implements IDisposable - +
    - - @if (_copied) + + @if (Copied) { Copied! @@ -14,6 +14,12 @@
    @code { + /// + /// Id for the input field. Defaults to a random GUID if not provided. + /// + [Parameter] + public string Id { get; set; } = Guid.NewGuid().ToString(); + /// /// The label for the input. /// @@ -26,8 +32,7 @@ [Parameter] public string Value { get; set; } = string.Empty; - private bool _copied => ClipboardCopyService.GetCopiedId() == _inputId; - private readonly string _inputId = Guid.NewGuid().ToString(); + private bool Copied => ClipboardCopyService.GetCopiedId() == Id; /// protected override void OnInitialized() @@ -38,11 +43,11 @@ private async Task CopyToClipboard() { await JsInteropService.CopyToClipboard(Value); - ClipboardCopyService.SetCopied(_inputId); + ClipboardCopyService.SetCopied(Id); // After 2 seconds, reset the copied state if it's still the same element await Task.Delay(2000); - if (ClipboardCopyService.GetCopiedId() == _inputId) + if (ClipboardCopyService.GetCopiedId() == Id) { ClipboardCopyService.SetCopied(string.Empty); } diff --git a/src/AliasVault.Client/Main/Components/Forms/CopyPastePasswordFormRow.razor b/src/AliasVault.Client/Main/Components/Forms/CopyPastePasswordFormRow.razor index 1fef43d2c..a93f9dcd8 100644 --- a/src/AliasVault.Client/Main/Components/Forms/CopyPastePasswordFormRow.razor +++ b/src/AliasVault.Client/Main/Components/Forms/CopyPastePasswordFormRow.razor @@ -2,12 +2,12 @@ @inject JsInteropService JsInteropService @implements IDisposable - +
    - + - @if (_copied) + @if (Copied) { Copied! @@ -27,6 +27,12 @@
    @code { + /// + /// Id for the input field. Defaults to a random GUID if not provided. + /// + [Parameter] + public string Id { get; set; } = Guid.NewGuid().ToString(); + /// /// The label for the input. /// @@ -39,9 +45,9 @@ [Parameter] public string Value { get; set; } = string.Empty; - private bool _copied => ClipboardCopyService.GetCopiedId() == _inputId; - private readonly string _inputId = Guid.NewGuid().ToString(); - private bool _isPasswordVisible = false; + private bool Copied => ClipboardCopyService.GetCopiedId() == Id; + + private bool IsPasswordVisible { get; set; } /// protected override void OnInitialized() @@ -52,11 +58,11 @@ private async Task CopyToClipboard() { await JsInteropService.CopyToClipboard(Value); - ClipboardCopyService.SetCopied(_inputId); + ClipboardCopyService.SetCopied(Id); // After 2 seconds, reset the copied state if it's still the same element await Task.Delay(2000); - if (ClipboardCopyService.GetCopiedId() == _inputId) + if (ClipboardCopyService.GetCopiedId() == Id) { ClipboardCopyService.SetCopied(string.Empty); } @@ -64,7 +70,7 @@ private void TogglePasswordVisibility() { - _isPasswordVisible = !_isPasswordVisible; + IsPasswordVisible = !IsPasswordVisible; } private void HandleCopy(string copiedElementId) diff --git a/src/AliasVault.Client/Main/Pages/Credentials/AddEdit.razor b/src/AliasVault.Client/Main/Pages/Credentials/AddEdit.razor index 4ca752453..0e41ceb30 100644 --- a/src/AliasVault.Client/Main/Pages/Credentials/AddEdit.razor +++ b/src/AliasVault.Client/Main/Pages/Credentials/AddEdit.razor @@ -305,46 +305,6 @@ else Obj.Password.Value = CredentialService.GenerateRandomPassword(); } - private async Task SaveAlias() - { - GlobalLoadingSpinner.Show(); - StateHasChanged(); - - if (EditMode) - { - if (Id is not null) - { - Id = await CredentialService.UpdateEntryAsync(CredentialEditToCredential(Obj)); - } - } - else - { - Id = await CredentialService.InsertEntryAsync(CredentialEditToCredential(Obj)); - } - - GlobalLoadingSpinner.Hide(); - StateHasChanged(); - - if (Id is null || Id == Guid.Empty) - { - // Error saving. - GlobalNotificationService.AddErrorMessage("Error saving credentials. Please try again.", true); - return; - } - - // No error, add success message. - if (EditMode) - { - GlobalNotificationService.AddSuccessMessage("Credential updated successfully."); - } - else - { - GlobalNotificationService.AddSuccessMessage("Credential created successfully."); - } - - NavigationManager.NavigateTo("/credentials/" + Id); - } - /// /// Helper method to convert a Credential object to a CredentialEdit object. /// @@ -390,7 +350,7 @@ else /// private Credential CredentialEditToCredential(CredentialEdit alias) { - var credential = new Credential() + var credential = new Credential { Id = alias.Id, Notes = alias.Notes, @@ -441,4 +401,47 @@ else await SaveAlias(); } + + /// + /// Save the alias to the database. + /// + private async Task SaveAlias() + { + GlobalLoadingSpinner.Show(); + StateHasChanged(); + + if (EditMode) + { + if (Id is not null) + { + Id = await CredentialService.UpdateEntryAsync(CredentialEditToCredential(Obj)); + } + } + else + { + Id = await CredentialService.InsertEntryAsync(CredentialEditToCredential(Obj)); + } + + GlobalLoadingSpinner.Hide(); + StateHasChanged(); + + if (Id is null || Id == Guid.Empty) + { + // Error saving. + GlobalNotificationService.AddErrorMessage("Error saving credentials. Please try again.", true); + return; + } + + // No error, add success message. + if (EditMode) + { + GlobalNotificationService.AddSuccessMessage("Credential updated successfully."); + } + else + { + GlobalNotificationService.AddSuccessMessage("Credential created successfully."); + } + + NavigationManager.NavigateTo("/credentials/" + Id); + } } diff --git a/src/AliasVault.Client/Main/Pages/Credentials/View.razor b/src/AliasVault.Client/Main/Pages/Credentials/View.razor index f2aa12008..95f6de64a 100644 --- a/src/AliasVault.Client/Main/Pages/Credentials/View.razor +++ b/src/AliasVault.Client/Main/Pages/Credentials/View.razor @@ -59,13 +59,13 @@ else
    - +
    - +
    - +
    diff --git a/src/AliasVault.Client/Services/CredentialService.cs b/src/AliasVault.Client/Services/CredentialService.cs index 23adaac54..7340341fc 100644 --- a/src/AliasVault.Client/Services/CredentialService.cs +++ b/src/AliasVault.Client/Services/CredentialService.cs @@ -228,25 +228,27 @@ public sealed class CredentialService(HttpClient httpClient, DbService dbService login.Service.UpdatedAt = DateTime.UtcNow; // Remove attachments that are no longer in the list - var existingAttachments = login.Attachments.ToList(); - foreach (var existingAttachment in existingAttachments) + var attachmentsToRemove = login.Attachments.Where(existingAttachment => + !loginObject.Attachments.Any(a => a.Id == existingAttachment.Id)).ToList(); + foreach (var attachmentToRemove in attachmentsToRemove) { - if (!loginObject.Attachments.Any(a => a.Id != Guid.Empty && a.Id == existingAttachment.Id)) - { - context.Entry(existingAttachment).State = EntityState.Deleted; - } + login.Attachments.Remove(attachmentToRemove); + context.Entry(attachmentToRemove).State = EntityState.Deleted; } - // Add new attachments + // Update existing attachments and add new ones foreach (var attachment in loginObject.Attachments) { - if (!login.Attachments.Any(a => attachment.Id != Guid.Empty && a.Id == attachment.Id)) + var existingAttachment = login.Attachments.FirstOrDefault(a => a.Id == attachment.Id); + if (existingAttachment != null) { - login.Attachments.Add(attachment); + // Update existing attachment + context.Entry(existingAttachment).CurrentValues.SetValues(attachment); } else { - context.Entry(attachment).State = EntityState.Modified; + // Add new attachment + login.Attachments.Add(attachment); } } diff --git a/src/Tests/AliasVault.E2ETests/Tests/Client/Shard5/AttachmentTests.cs b/src/Tests/AliasVault.E2ETests/Tests/Client/Shard5/AttachmentTests.cs index 243088a55..821557a37 100644 --- a/src/Tests/AliasVault.E2ETests/Tests/Client/Shard5/AttachmentTests.cs +++ b/src/Tests/AliasVault.E2ETests/Tests/Client/Shard5/AttachmentTests.cs @@ -118,8 +118,11 @@ public class AttachmentTests : ClientPlaywrightTest }); // Check that the updated username and attachment name still appear on the alias page. + var usernameElement = await Page.QuerySelectorAsync("#username"); + Assert.That(usernameElement, Is.Not.Null, "Username element not found."); + Assert.That(await usernameElement.InputValueAsync(), Is.EqualTo(updatedUsername), "Updated username does not appear on alias page."); + pageContent = await Page.TextContentAsync("body"); - Assert.That(pageContent, Does.Contain(updatedUsername), "Updated username does not appear on alias page."); Assert.That(pageContent, Does.Contain("TestAttachment.txt"), "Attachment name does not appear on alias page after update."); // Download the attachment @@ -140,4 +143,63 @@ public class AttachmentTests : ClientPlaywrightTest // Clean up: delete the downloaded file File.Delete(downloadedFilePath); } + + /// + /// Test that uploading and deleting an attachment works correctly. + /// + /// Async task. + [Test] + [Order(3)] + public async Task UploadAndDeleteAttachment() + { + // Create a new alias with service name = "Test Service for Deletion". + var serviceName = "Test Service for Deletion"; + await CreateCredentialEntry( + new Dictionary + { + { "service-name", serviceName }, + }, + async () => + { + // Upload file. + var fileInput = Page.Locator("input[type='file']"); + var fileContent = await ResourceReaderUtility.ReadEmbeddedResourceBytesAsync("AliasVault.E2ETests.TestData.TestAttachment.txt"); + + // Create a temporary file with the content and original file name + var originalFileName = "TestAttachment.txt"; + var tempFilePath = Path.Combine(Path.GetTempPath(), originalFileName); + await File.WriteAllBytesAsync(tempFilePath, fileContent); + + // Set the file input using the temporary file + await fileInput.SetInputFilesAsync(tempFilePath); + + // Delete the temporary file + File.Delete(tempFilePath); + }); + + // Check that the attachment name appears on the alias page. + var pageContent = await Page.TextContentAsync("body"); + Assert.That(pageContent, Does.Contain("TestAttachment.txt"), "Uploaded attachment name does not appear on alias page."); + + // Click the edit button + await Page.ClickAsync("text=Edit"); + await WaitForUrlAsync("credentials/**/edit", "Edit the existing credentials"); + + // Find and click the delete button for the attachment + var deleteButton = Page.Locator("button:has-text('Delete')").First; + await deleteButton.ClickAsync(); + + // Check that the attachment name no longer appears on the edit page + pageContent = await Page.TextContentAsync("body"); + Assert.That(pageContent, Does.Not.Contain("TestAttachment.txt"), "Deleted attachment name still appears on edit page."); + + // Save the credential + var saveButton = Page.Locator("text=Save Credentials").First; + await saveButton.ClickAsync(); + await WaitForUrlAsync("credentials/**", "Credential updated successfully"); + + // Check that the attachment name does not appear on the view page + pageContent = await Page.TextContentAsync("body"); + Assert.That(pageContent, Does.Not.Contain("TestAttachment.txt"), "Deleted attachment name appears on view page after saving."); + } }