Communication: Add undo button when deleting posts

[PaRangger]

Checklist

General

• I tested all changes and their related features with all corresponding user types on a test server.
• Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
• I chose a title conforming to the naming conventions for pull requests.

Client

• Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
• I strictly followed the client coding and design guidelines.
• Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
• I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
• I added multiple screenshots/screencasts of my UI changes.
• I translated all newly inserted strings into English and German.

Motivation and Context

Currently, messages are deleted by selecting the trash icon on a post and then confirming the action by clicking the checkmark in the same location. While this confirmation step helps prevent accidental deletions, the double-clicking action commonly used in user interfaces may still lead to unintended deletions. To address this, an additional mechanism should be introduced, allowing users to undo a delete action for a brief period, similar to e.g. the "undo send" functionality in the Mac OS Mail app.

Description

A brief delay has been added before permanently deleting a message, allowing users an opportunity to undo their delete command. This delay is accompanied by user interface elements designed to clearly indicate the status and timing of the deletion process.

Steps for Testing

Prerequisites:

• 1 Instructor/Student/Tutor
• 1 Course with communication enabled

1. Log in to Artemis
2. Navigate to course
3. Navigate to the communication tab and select a channel
4. Write a message or locate an existing message of the current user
5. Delete the message
6. Undo the delete command
7. Delete the message again
8. Check if message is deleted after the delay

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.

Review Progress

Code Review

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Test Coverage

Client

Class/File	Line Coverage	Confirmation (assert/expect)
answer-post.component.ts	100%	✅
post.component.ts	94.82%	✅
answer-post-header.component.ts	96.87%	✅
post-header.component.ts	96.66%	✅
posting-header.directive.ts	100%	✅
posting.directive.ts	82.75%	✅
profile-picture.component.ts	100%	✅

�

Screenshots

Summary by CodeRabbit

Release Notes

• New Features

  • Enhanced handling of deleted posts with new UI elements for undoing deletions.
  • Conditional rendering for editing options based on post deletion status.
  • Added animations for improved visual feedback when posts are deleted or restored.

• Bug Fixes

  • Improved performance in rendering lists of posts by optimizing tracking mechanisms.

• Localization

  • Added new German and English keys for user feedback on post deletion and undo actions.

• Tests

  • Expanded test coverage for deletion functionality and UI responses to deletion state changes.

[github-actions]

⚠️ Unable to deploy to test servers ⚠️

The docker build needs to run through before deploying.

[coderabbitai]

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (7)

src/main/webapp/app/shared/metis/posting-footer/post-footer/post-footer.component.html (1)

Line range hint 1-35: Consider adding ARIA attributes for better accessibility.

To improve accessibility, especially for the new deletion feature, consider adding appropriate ARIA attributes to indicate dynamic content changes and deletion states.

Add aria-live regions to announce deletion states:

 <ng-container class="list-answer-post">
+    <div aria-live="polite" class="visually-hidden">
+        {{ sortedAnswerPosts.length }} answers displayed
+    </div>
     @for (answerPost of sortedAnswerPosts; track postsTrackByFn($index, answerPost); let isLastAnswer = $last) {

src/main/webapp/app/shared/metis/metis.component.scss (1)

111-111: Consider adding easing to the animation.

The linear timing function might feel mechanical. Consider using ease-in-out for a more polished user experience.

-    animation: increaseWidth $delete-delay-duration forwards linear;
+    animation: increaseWidth $delete-delay-duration forwards ease-in-out;

src/main/webapp/app/shared/metis/posting.directive.ts (2)

14-19: Add JSDoc comments for the new properties.

Consider adding documentation to explain the purpose and behavior of these properties, especially timeToDeleteInSeconds and how it differs from deleteTimerInSeconds.

+/** Indicates if the current posting is an answer post */
 isAnswerPost = false;
+/** Indicates if the posting is marked for deletion */
 isDeleted = false;
+/** The total time in seconds before the post is permanently deleted */
 readonly timeToDeleteInSeconds = 6;
+/** Countdown timer showing remaining seconds before deletion */
 deleteTimerInSeconds = 6;

---

63-63: Document the buffer timing.

The comment about the buffer time could be more explicit about its purpose.

-// We add a tiny buffer to make it possible for the user to react a bit longer than the ui displays (+1000)
+// Add 1 second buffer to ensure users have slightly more time than shown in the UI countdown
+// This prevents edge cases where the UI shows time remaining but the deletion occurs

src/main/webapp/app/shared/metis/posting-content/posting-content.component.html (1)

1-8: Add accessibility and loading state improvements

While the implementation follows the new Angular syntax and properly implements the undo functionality, there are a few improvements needed:

1. Add aria-label to the undo button for accessibility
2. Add loading indicator as mentioned in PR feedback
3. Consider adding animation for the countdown timer

Apply these improvements:

 <span class="posting-content-undo-delete d-inline-flex align-items-center">
     <span class="text-secondary" jhiTranslate="artemisApp.metis.post.deletedContent" [translateValues]="{ progress: deleteTimerInSeconds() }"></span>
-    <button class="btn btn-outline-primary btn-sm ms-2 position-relative" (click)="onUndoDeleteEvent.emit()">
+    <button 
+        class="btn btn-outline-primary btn-sm ms-2 position-relative" 
+        (click)="onUndoDeleteEvent.emit()"
+        aria-label="Undo delete post">
         <span class="post-delete-button-label" jhiTranslate="artemisApp.metis.post.undoDelete"></span>
         <span class="post-delete-button-background"></span>
+        <span class="spinner-border spinner-border-sm ms-1" role="status" *ngIf="isLoading"></span>
     </button>
 </span>

src/main/webapp/app/shared/metis/posting-content/posting-content.components.ts (1)

27-29: Properties look good, consider adding documentation.

The new properties effectively support the undo deletion feature with appropriate types and naming. Consider these improvements:

1. Add JSDoc comments to document the purpose and usage of each property
2. Add validation for deleteTimerInSeconds to ensure positive values

Example documentation and validation:

+    /**
+     * Indicates whether the post is in deleted state awaiting potential undo
+     */
     isDeleted = input<boolean>(false);

+    /**
+     * Countdown timer in seconds before permanent deletion
+     * @minimum 0
+     */
     deleteTimerInSeconds = input<number>((value: number) => {
+        if (value < 0) {
+            console.warn('deleteTimerInSeconds should not be negative');
+            return 0;
+        }
+        return value;
     });

+    /**
+     * Event emitted when user clicks the undo button to restore a deleted post
+     */
     onUndoDeleteEvent = output<void>();

src/test/javascript/spec/component/shared/metis/posting-content/posting-content.component.spec.ts (1)

127-132: Enhance test case with additional assertions.

While the test correctly verifies the presence of the undo delete prompt, it could be more comprehensive.

Consider adding these assertions to verify the complete component state:

 it('should display undo delete prompt when isDeleted is set to true', () => {
     fixture.componentRef.setInput('isDeleted', true);
     fixture.detectChanges();
     expect(fixture.nativeElement.querySelector('.posting-content-undo-delete')).not.toBeNull();
+    // Verify the prompt is not shown when isDeleted is false
     fixture.componentRef.setInput('isDeleted', false);
+    fixture.detectChanges();
+    expect(fixture.nativeElement.querySelector('.posting-content-undo-delete')).toBeNull();
 });

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 37f8fce and 82a3563.

📒 Files selected for processing (13)

• src/main/webapp/app/shared/metis/answer-post/answer-post.component.html (3 hunks)
• src/main/webapp/app/shared/metis/answer-post/answer-post.component.scss (1 hunks)
• src/main/webapp/app/shared/metis/metis.component.scss (2 hunks)
• src/main/webapp/app/shared/metis/post/post.component.html (2 hunks)
• src/main/webapp/app/shared/metis/posting-content/posting-content.component.html (1 hunks)
• src/main/webapp/app/shared/metis/posting-content/posting-content.components.ts (2 hunks)
• src/main/webapp/app/shared/metis/posting-footer/post-footer/post-footer.component.html (1 hunks)
• src/main/webapp/app/shared/metis/posting-footer/post-footer/post-footer.component.ts (1 hunks)
• src/main/webapp/app/shared/metis/posting-header/answer-post-header/answer-post-header.component.html (2 hunks)
• src/main/webapp/app/shared/metis/posting.directive.ts (1 hunks)
• src/main/webapp/app/shared/profile-picture/profile-picture.component.html (2 hunks)
• src/main/webapp/app/shared/profile-picture/profile-picture.component.scss (1 hunks)
• src/test/javascript/spec/component/shared/metis/posting-content/posting-content.component.spec.ts (4 hunks)

✅ Files skipped from review due to trivial changes (1)

• src/main/webapp/app/shared/metis/answer-post/answer-post.component.scss

🚧 Files skipped from review as they are similar to previous changes (4)

• src/main/webapp/app/shared/metis/answer-post/answer-post.component.html
• src/main/webapp/app/shared/metis/post/post.component.html
• src/main/webapp/app/shared/metis/posting-header/answer-post-header/answer-post-header.component.html
• src/main/webapp/app/shared/profile-picture/profile-picture.component.html

🧰 Additional context used

📓 Path-based instructions (6)

src/main/webapp/app/shared/metis/posting-content/posting-content.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/shared/metis/posting-content/posting-content.components.ts (1)

Pattern src/main/webapp/**/*.ts: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

src/main/webapp/app/shared/metis/posting-footer/post-footer/post-footer.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/shared/metis/posting-footer/post-footer/post-footer.component.ts (1)

Pattern src/main/webapp/**/*.ts: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

src/main/webapp/app/shared/metis/posting.directive.ts (1)

Pattern src/main/webapp/**/*.ts: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

src/test/javascript/spec/component/shared/metis/posting-content/posting-content.component.spec.ts (1)

Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}

🔇 Additional comments (13)

src/main/webapp/app/shared/profile-picture/profile-picture.component.scss (2)

7-7: LGTM: Improved contrast with darker background.

The change to --gray-600 provides better contrast, enhancing visibility and accessibility.

---

10-12: LGTM: Well-implemented grayscale effect for deleted posts.

The grayscale implementation effectively indicates deleted post status, following common UI patterns. The class is properly scoped and uses semantic naming.

src/main/webapp/app/shared/metis/posting-footer/post-footer/post-footer.component.html (2)

Line range hint 17-28: Verify integration with the new post deletion feature.

While this template handles the answer posts display correctly, we should verify that the child jhi-answer-post component properly integrates with the new deletion and undo functionality.

Let's check for the deletion-related properties in the answer post component:

#!/bin/bash
# Description: Verify the answer post component handles deletion states
# Look for deletion-related properties and templates
rg -A 5 "isDeleted|undoDelete|deletePost" "src/main/webapp/app/shared/metis/answer-post"

---

15-15: LGTM! Tracking function implementation improves list rendering performance.

The change from tracking by index to using postsTrackByFn is a good practice as it helps Angular's change detection mechanism better handle updates to the list of answer posts.

Let's verify the implementation of the tracking function:

✅ Verification successful

Let me gather more information about the tracking function implementation.

---

Tracking function implementation is correct and optimized

The postsTrackByFn implementation in post-footer.component.ts correctly uses the post's unique ID for tracking, which is the recommended approach for optimal performance in Angular's change detection, especially when handling post deletions and updates.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the postsTrackByFn implementation in the component
ast-grep --pattern 'protected postsTrackByFn(index: number, post: Post): number {
  $$$
}'

Length of output: 2182

---

Script:

#!/bin/bash
# Search for postsTrackByFn implementation in the component file
rg -U "postsTrackByFn.*?\{.*?\}" --multiline --multiline-dotall src/main/webapp/app/shared/metis/posting-footer/post-footer/

Length of output: 2464

src/main/webapp/app/shared/metis/metis.component.scss (2)

104-121: Well-structured implementation with good accessibility consideration!

The styling implementation shows attention to detail:

• Proper z-index layering for background and label
• Accessibility support with prefers-reduced-motion
• Clear semantic class naming

---

123-130: Verify animation visibility in threaded messages.

The animation implementation is correct, but per PR comments, the loading indicator doesn't appear in threaded messages. This needs investigation.

src/main/webapp/app/shared/metis/posting.directive.ts (1)

30-38: LGTM! Proper cleanup implementation.

The ngOnDestroy implementation correctly cleans up timers to prevent memory leaks.

src/main/webapp/app/shared/metis/posting-content/posting-content.component.html (1)

Line range hint 9-48: LGTM! Verify content parts rendering

The implementation follows the new Angular syntax guidelines and maintains a clear separation of concerns between preview and non-preview modes.

Let's verify the content parts rendering implementation:

✅ Verification successful

Content parts implementation is well-structured and properly tested

The verification shows that the content parts implementation is robust and well-implemented:

• The PostingContentPart interface is properly defined in metis.util.ts
• Content parts are managed through a signal in the parent component (posting-content.components.ts)
• The implementation includes proper computation logic via computePostingContentParts
• The component is thoroughly tested with multiple test cases covering different scenarios
• The child component PostingContentPartComponent correctly handles the rendering of individual parts

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the content parts implementation across components

# Check for the PostingContentPart interface/type definition
rg -t typescript "interface PostingContentPart|type PostingContentPart" 

# Check for content parts processing logic
ast-grep --pattern 'postingContentParts() {
  $$$
}'

Length of output: 163

---

Script:

#!/bin/bash
# Let's try different approaches to find the content parts implementation

# Search for PostingContentPart in all files
rg "PostingContentPart"

# Look for the posting-content-part component
fd "posting-content-part.component"

# Search for postingContentParts method/property
rg "postingContentParts"

# Look for content parsing or processing logic
rg "parseContent|processContent" -A 5

Length of output: 16402

src/main/webapp/app/shared/metis/posting-footer/post-footer/post-footer.component.ts (1)

91-94: Verify trackBy function usage for smooth animations.

Since this PR implements post deletion with undo functionality, ensure the tracking function properly handles post state transitions for smooth animations.

Let's verify the trackBy implementation in the template:

src/main/webapp/app/shared/metis/posting-content/posting-content.components.ts (2)

1-1: LGTM! Modern Angular patterns used.

The addition of signal-based input and output imports from '@angular/core' follows Angular's modern best practices for component communication.

---

27-29: Verify thread-specific behavior of deletion UI.

Based on PR feedback, there's an issue with the loading indicator not appearing for deletions within threads. Let's verify the integration:

src/test/javascript/spec/component/shared/metis/posting-content/posting-content.component.spec.ts (2)

3-3: LGTM! Import statements are correctly organized.

The addition of MockDirective and TranslateDirective imports aligns with the new test requirements.

Also applies to: 16-16

---

27-33: LGTM! Test module declarations are properly configured.

The TranslateDirective is correctly mocked using MockDirective in the test module setup, following Angular testing best practices.

[PaRangger]

Tested on TS2. Unfortunately I had multiple problems.. Fist of all, great feature though!

• deleting replies does not have the same animation, the progressing bar, as deleting normal messages (not sure if intended?)
• deleting a message and resizing the window leads to hiding the following message
• deleting a message and changing to another thread either doesn't delete the message or deletes the wrong message

Thank you for your feedback and the video, that helped a lot with troubleshooting! I found the issue and tried to resolve all the problems you stated in this commit.

[PaRangger]

I tested on TS2, the base-case works. But one issue I encountered is deleting messages in threads with multiple messages:

...

Thank you for your feedback! I found the issue that was causing the behaviour in the answer threads and tried to fix it in this commit.

[Malekos74]

Server tested on: Server 01

Works as expected. I think the error that you talked about earlier in the conversation is now fixed. Or at least, I couldn't reproduce it.

[sawys777]

Tested on TS2, everything worked as expected from the description. However, I have also tried switching to another channel while waiting for the timer to countdown, when I am back to the previous channel, the message did not get deleted and remain there. Not sure if this is the intended behaviour.