Merge pull request #296 from lanedirt/287-saving-existing-credential-…

…with-one-or-more-attachments-fails

Saving existing credential with one or more attachments fails

src/AliasVault.Client/Main/Components/Attachments/AttachmentUploader.razor

@@ -1,17 +1,18 @@
 @using System.IO
+@inject ILogger<AttachmentUploader> Logger
 
 <div class="col-span-6 sm:col-span-3">
     <InputFile OnChange="@HandleFileSelection" class="block w-full text-sm text-gray-900 border border-gray-300 rounded-lg cursor-pointer bg-gray-50 dark:text-gray-400 focus:outline-none dark:bg-gray-700 dark:border-gray-600 dark:placeholder-gray-400" />
     @if (!string.IsNullOrEmpty(StatusMessage))
     {
         <p class="mt-2 text-sm text-gray-500 dark:text-gray-400">@StatusMessage</p>
     }
-    @if (Attachments.Any())
+    @if (Attachments.Exists(x => !x.IsDeleted))
     {
         <div class="mt-4">
             <h4 class="mb-2 text-lg font-semibold dark:text-white">Attachments:</h4>
             <ul class="list-disc list-inside">
-                @foreach (var attachment in Attachments)
+                @foreach (var attachment in Attachments.Where(x => !x.IsDeleted))
                 {
                     <li class="flex items-center justify-between text-sm text-gray-600 dark:text-gray-400">
                         <span>@attachment.Filename</span>
@@ -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).
     /// </summary>
-    private List<Guid> OriginalAttachmentsIds = [];
+    private List<Guid> OriginalAttachmentsIds { get; set; } = [];
 
-    private string StatusMessage = string.Empty;
+    /// <summary>
+    /// Status message to display.
+    /// </summary>
+    private string StatusMessage { get; set; } = string.Empty;
 
     /// <inheritdoc />
     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.");
             }
         }
 

src/AliasVault.Client/Main/Components/Attachments/AttachmentViewer.razor

@@ -2,7 +2,7 @@
 
 <div class="p-4 mb-4 bg-white border border-gray-200 rounded-lg shadow-sm 2xl:col-span-2 dark:border-gray-700 sm:p-6 dark:bg-gray-800">
     <h3 class="mb-4 text-xl font-semibold dark:text-white">Attachments</h3>
-    @if (Attachments.Any())
+    @if (Attachments.Any(x => !x.IsDeleted))
     {
         <div class="overflow-x-auto">
             <table class="w-full text-sm text-left text-gray-500 dark:text-gray-400">
@@ -13,7 +13,7 @@
                     </tr>
                 </thead>
                 <tbody>
-                    @foreach (var attachment in Attachments)
+                    @foreach (var attachment in Attachments.Where(x => !x.IsDeleted))
                     {
                         <tr class="bg-white border-b dark:bg-gray-800 dark:border-gray-700">
                             <td class="px-6 py-4 font-medium text-gray-900 whitespace-nowrap dark:text-white">

src/AliasVault.Client/Main/Components/Forms/CopyPasteFormRow.razor

@@ -2,10 +2,10 @@
 @inject JsInteropService JsInteropService
 @implements IDisposable
 
-<label for="@_inputId" class="block mb-2 text-sm font-medium text-gray-900 dark:text-white">@Label</label>
+<label for="@Id" class="block mb-2 text-sm font-medium text-gray-900 dark:text-white">@Label</label>
 <div class="relative">
-    <input type="text" id="@_inputId" class="outline-0 shadow-sm bg-gray-50 border @(_copied ? "border-green-500 border-2" : "border-gray-300") text-gray-900 sm:text-sm rounded-lg block w-full p-2.5 pr-10 dark:bg-gray-700 dark:border-@(_copied ? "green-500" : "gray-600") dark:placeholder-gray-400 dark:text-white" value="@Value" @onclick="CopyToClipboard" readonly>
-    @if (_copied)
+    <input type="text" id="@Id" class="outline-0 shadow-sm bg-gray-50 border @(Copied ? "border-green-500 border-2" : "border-gray-300") text-gray-900 sm:text-sm rounded-lg block w-full p-2.5 pr-10 dark:bg-gray-700 dark:border-@(Copied ? "green-500" : "gray-600") dark:placeholder-gray-400 dark:text-white" value="@Value" @onclick="CopyToClipboard" readonly>
+    @if (Copied)
     {
         <span class="absolute inset-y-0 right-0 flex items-center pr-3 text-green-500 dark:text-green-400">
             Copied!
@@ -14,6 +14,12 @@
 </div>
 
 @code {
+    /// <summary>
+    /// Id for the input field. Defaults to a random GUID if not provided.
+    /// </summary>
+    [Parameter]
+    public string Id { get; set; } = Guid.NewGuid().ToString();
+
     /// <summary>
     /// The label for the input.
     /// </summary>
@@ -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;
 
     /// <inheritdoc />
     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);
         }

src/AliasVault.Client/Main/Components/Forms/CopyPastePasswordFormRow.razor

@@ -2,12 +2,12 @@
 @inject JsInteropService JsInteropService
 @implements IDisposable
 
-<label for="@_inputId" class="block mb-2 text-sm font-medium text-gray-900 dark:text-white">@Label</label>
+<label for="@Id" class="block mb-2 text-sm font-medium text-gray-900 dark:text-white">@Label</label>
 <div class="relative">
-    <input type="@(_isPasswordVisible ? "text" : "password")" id="@_inputId" class="outline-0 shadow-sm bg-gray-50 border @(_copied ? "border-green-500 border-2" : "border-gray-300") text-gray-900 sm:text-sm rounded-lg block w-full p-2.5 pr-20 dark:bg-gray-700 dark:border-@(_copied ? "green-500" : "gray-600") dark:placeholder-gray-400 dark:text-white" value="@Value" @onclick="CopyToClipboard" readonly>
+    <input type="@(IsPasswordVisible ? "text" : "password")" id="@Id" class="outline-0 shadow-sm bg-gray-50 border @(Copied ? "border-green-500 border-2" : "border-gray-300") text-gray-900 sm:text-sm rounded-lg block w-full p-2.5 pr-20 dark:bg-gray-700 dark:border-@(Copied ? "green-500" : "gray-600") dark:placeholder-gray-400 dark:text-white" value="@Value" @onclick="CopyToClipboard" readonly>
     <button type="button" class="absolute inset-y-1 right-1 flex items-center justify-center w-10 h-8 text-gray-500 bg-gray-200 rounded-md shadow-sm hover:bg-gray-300 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-primary-500 transition-colors duration-200 dark:bg-gray-600 dark:hover:bg-gray-500 dark:text-gray-300" @onclick="TogglePasswordVisibility">
         <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
-            @if (_isPasswordVisible)
+            @if (IsPasswordVisible)
             {
                 <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M15 12a3 3 0 11-6 0 3 3 0 016 0z"></path>
                 <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M2.458 12C3.732 7.943 7.523 5 12 5c4.478 0 8.268 2.943 9.542 7-1.274 4.057-5.064 7-9.542 7-4.477 0-8.268-2.943-9.542-7z"></path>
@@ -18,7 +18,7 @@
             }
         </svg>
     </button>
-    @if (_copied)
+    @if (Copied)
     {
         <span class="absolute inset-y-0 right-10 flex items-center pr-3 text-green-500 dark:text-green-400">
             Copied!
@@ -27,6 +27,12 @@
 </div>
 
 @code {
+    /// <summary>
+    /// Id for the input field. Defaults to a random GUID if not provided.
+    /// </summary>
+    [Parameter]
+    public string Id { get; set; } = Guid.NewGuid().ToString();
+
     /// <summary>
     /// The label for the input.
     /// </summary>
@@ -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; }
 
     /// <inheritdoc />
     protected override void OnInitialized()
@@ -52,19 +58,19 @@
     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);
         }
     }
 
     private void TogglePasswordVisibility()
     {
-        _isPasswordVisible = !_isPasswordVisible;
+        IsPasswordVisible = !IsPasswordVisible;
     }
 
     private void HandleCopy(string copiedElementId)

src/AliasVault.Client/Main/Components/Widgets/CreateNewIdentityWidget.razor

@@ -159,7 +159,7 @@
         }
 
         // No error, add success message.
-        GlobalNotificationService.AddSuccessMessage("Credentials created successfully.");
+        GlobalNotificationService.AddSuccessMessage("Credential created successfully.");
 
         NavigationManager.NavigateTo("/credentials/" + id);
 

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("Credentials updated successfully.");
-        }
-        else
-        {
-            GlobalNotificationService.AddSuccessMessage("Credentials created successfully.");
-        }
-
-        NavigationManager.NavigateTo("/credentials/" + Id);
-    }
-
     /// <summary>
     /// Helper method to convert a Credential object to a CredentialEdit object.
     /// </summary>
@@ -390,7 +350,7 @@ else
     /// </summary>
     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();
     }
+
+    /// <summary>
+    /// Save the alias to the database.
+    /// </summary>
+    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);
+    }
 }

src/AliasVault.Client/Main/Pages/Credentials/View.razor

@@ -59,13 +59,13 @@ else
                 <form action="#">
                     <div class="grid gap-6">
                         <div class="col-span-6 sm:col-span-3">
-                            <CopyPasteFormRow Label="Email" Value="@Alias.Alias.Email"></CopyPasteFormRow>
+                            <CopyPasteFormRow Id="email" Label="Email" Value="@Alias.Alias.Email"></CopyPasteFormRow>
                         </div>
                         <div class="col-span-6 sm:col-span-3">
-                            <CopyPasteFormRow Label="Username" Value="@(Alias.Username)"></CopyPasteFormRow>
+                            <CopyPasteFormRow Id="username" Label="Username" Value="@(Alias.Username)"></CopyPasteFormRow>
                         </div>
                         <div class="col-span-6 sm:col-span-3">
-                            <CopyPastePasswordFormRow Label="Password" Value="@(Alias.Passwords.FirstOrDefault()?.Value ?? string.Empty)"></CopyPastePasswordFormRow>
+                            <CopyPastePasswordFormRow Id="password" Label="Password" Value="@(Alias.Passwords.FirstOrDefault()?.Value ?? string.Empty)"></CopyPastePasswordFormRow>
                         </div>
                     </div>
                 </form>

src/AliasVault.Client/Services/CredentialService.cs

@@ -228,25 +228,27 @@ public async Task<Guid> UpdateEntryAsync(Credential loginObject)
         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);
             }
         }
 

src/Tests/AliasVault.E2ETests/AliasVault.E2ETests.csproj

@@ -23,6 +23,7 @@
     <ItemGroup>
       <AdditionalFiles Include="..\..\stylecop.json" Link="stylecop.json" />
       <EmbeddedResource Include="TestData\AliasClientDb_encrypted_base64_1.0.0.txt" />
+      <EmbeddedResource Include="TestData\TestAttachment.txt" />
     </ItemGroup>
 
     <ItemGroup>