Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Communication: Fix reply button message editing issue in exercise view #9815

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
[isReadOnlyMode]="isReadOnlyMode"
[isCommunicationPage]="isCommunicationPage"
[lastReadDate]="lastReadDate"
[hasChannelModerationRights]="hasChannelModerationRights"
[isDeleted]="isDeleted"
/>
}
Expand Down
7 changes: 4 additions & 3 deletions src/main/webapp/app/shared/metis/post/post.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
[posting]="posting"
[isDeleted]="isDeleted"
[isCommunicationPage]="isCommunicationPage"
[hasChannelModerationRights]="hasChannelModerationRights"
(isModalOpen)="displayInlineInput = true"
[lastReadDate]="lastReadDate"
/>
Expand Down Expand Up @@ -68,6 +67,7 @@
></jhi-posting-content>
<div class="hover-actions post-content-margin" [ngClass]="{ 'mb-2': previewMode }">
@if (!previewMode) {
<!-- Post reactions (that appear when post is hovered) -->
<jhi-post-reactions-bar
[lastReadDate]="lastReadDate"
[readOnlyMode]="readOnlyMode"
Expand Down Expand Up @@ -98,7 +98,7 @@
}
@if (!isDeleted) {
<div class="post-content-margin justify-content-between" [ngClass]="{ 'mb-2': previewMode }">
<!-- Post reactions -->
<!-- Post reactions (to show replies & reactions under the post) -->
@if (!previewMode) {
<jhi-post-reactions-bar
[lastReadDate]="lastReadDate"
Expand All @@ -110,6 +110,7 @@
[isCommunicationPage]="isCommunicationPage"
[isThreadSidebar]="isThreadSidebar"
(openPostingCreateEditModal)="openCreateAnswerPostModal()"
(closePostingCreateEditModal)="closeCreateAnswerPostModal()"
(openThread)="openThread.emit()"
(isModalOpen)="displayInlineInput = true"
[isEmojiCount]="true"
Expand Down Expand Up @@ -158,7 +159,7 @@
<span jhiTranslate="artemisApp.metis.post.deleteMessage"></span>
</button>
}
<button class="dropdown-item d-flex" (click)="openThread.emit()">
<button class="dropdown-item d-flex" (click)="isCommunicationPage ? openThread.emit() : reactionsBarComponent?.openAnswerView()">
<fa-icon [icon]="faComments" class="item-icon"></fa-icon>
<span jhiTranslate="artemisApp.metis.post.replyMessage"></span>
</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
top: -1.8rem;
right: 1%;
display: flex;
max-height: 2.2rem;
gap: 10px;
visibility: hidden;
transition:
Expand Down
9 changes: 8 additions & 1 deletion src/main/webapp/app/shared/metis/post/post.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export class PostComponent extends PostingDirective<Post> implements OnInit, OnC

isConsecutive = input<boolean>(false);
dropdownPosition = { x: 0, y: 0 };
@ViewChild(PostReactionsBarComponent) private reactionsBarComponent!: PostReactionsBarComponent;
@ViewChild(PostReactionsBarComponent) protected reactionsBarComponent!: PostReactionsBarComponent;

constructor(
public metisService: MetisService,
Expand Down Expand Up @@ -218,6 +218,13 @@ export class PostComponent extends PostingDirective<Post> implements OnInit, OnC
this.postFooterComponent.openCreateAnswerPostModal();
}

/**
* Close create answer modal
*/
closeCreateAnswerPostModal() {
this.postFooterComponent.closeCreateAnswerPostModal();
}

/**
* sorts answerPosts by two criteria
* 1. criterion: resolvesPost -> true comes first
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,13 @@ export class PostFooterComponent extends PostingFooterDirective<Post> implements
this.createAnswerPostModalComponent.open();
}

/**
* Close create answer modal
*/
closeCreateAnswerPostModal() {
this.createAnswerPostModalComponent.close();
}
Comment on lines +176 to +181
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for modal component.

While the method implementation is straightforward, it should handle cases where createAnswerPostModalComponent might be undefined.

Consider this safer implementation:

 closeCreateAnswerPostModal() {
-    this.createAnswerPostModalComponent.close();
+    if (this.createAnswerPostModalComponent) {
+        this.createAnswerPostModalComponent.close();
+    } else {
+        console.warn('Modal component is not initialized');
+    }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Close create answer modal
*/
closeCreateAnswerPostModal() {
this.createAnswerPostModalComponent.close();
}
/**
* Close create answer modal
*/
closeCreateAnswerPostModal() {
if (this.createAnswerPostModalComponent) {
this.createAnswerPostModalComponent.close();
} else {
console.warn('Modal component is not initialized');
}
}


protected postsTrackByFn(index: number, post: Post): number {
return post.id!;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Component, EventEmitter, Input, OnChanges, OnInit, Output } from '@angu
import { AnswerPost } from 'app/entities/metis/answer-post.model';
import { PostingHeaderDirective } from 'app/shared/metis/posting-header/posting-header.directive';
import { MetisService } from 'app/shared/metis/metis.service';
import { faCheck, faCog, faPencilAlt } from '@fortawesome/free-solid-svg-icons';
import { faCheck, faPencilAlt } from '@fortawesome/free-solid-svg-icons';
import dayjs from 'dayjs/esm';
import { AccountService } from 'app/core/auth/account.service';

Expand All @@ -18,13 +18,9 @@ export class AnswerPostHeaderComponent extends PostingHeaderDirective<AnswerPost
@Input() lastReadDate?: dayjs.Dayjs;
@Output() openPostingCreateEditModal = new EventEmitter<void>();

isAuthorOfOriginalPost: boolean;
isAnswerOfAnnouncement: boolean;

// Icons
faCheck = faCheck;
faPencilAlt = faPencilAlt;
faCog = faCog;
readonly faCheck = faCheck;
readonly faPencilAlt = faPencilAlt;

constructor(
protected metisService: MetisService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Post } from 'app/entities/metis/post.model';
import { PostingHeaderDirective } from 'app/shared/metis/posting-header/posting-header.directive';
import { MetisService } from 'app/shared/metis/metis.service';
import { PostCreateEditModalComponent } from 'app/shared/metis/posting-create-edit-modal/post-create-edit-modal/post-create-edit-modal.component';
import { faCheckSquare, faCog, faPencilAlt } from '@fortawesome/free-solid-svg-icons';
import { faCheckSquare, faPencilAlt } from '@fortawesome/free-solid-svg-icons';
import dayjs from 'dayjs/esm';
import { CachingStrategy } from 'app/shared/image/secured-image.component';
import { AccountService } from 'app/core/auth/account.service';
Expand All @@ -22,9 +22,8 @@ export class PostHeaderComponent extends PostingHeaderDirective<Post> implements
isAtLeastInstructorInCourse: boolean;

// Icons
faPencilAlt = faPencilAlt;
faCheckSquare = faCheckSquare;
faCog = faCog;
readonly faPencilAlt = faPencilAlt;
readonly faCheckSquare = faCheckSquare;

constructor(
protected metisService: MetisService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,4 @@
}
}
</div>
<!-- add new answer to expanded discussion -->
@if (isLastAnswer && !isThreadSidebar) {
<div>
<button class="reaction-button clickable reply-btn" (click)="openPostingCreateEditModal.emit()">
<jhi-emoji [emoji]="speechBalloonId" />
<span jhiTranslate="artemisApp.metis.reply" class="emoji-count"></span>
</button>
</div>
}
</div>
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
<div class="d-flex flex-wrap gap-2 fs-xx-small align-items-center">
<div class="d-flex flex-wrap gap-2 fs-xx-small align-items-center" style="width: max-content">
@if (hoverBar && sortedAnswerPosts.length === 0) {
<div>
<button class="reaction-button clickable reply-btn" (click)="isCommunicationPage ? openThread.emit() : openAnswerView()">
<fa-icon class="fa-xs align-self-center" [icon]="faArrowRight" />
<span jhiTranslate="artemisApp.conversationsLayout.threadSideBar.reply" class="emoji-count px-2"></span>
</button>
</div>
}
@if (!isCommunicationPage) {
@if (sortedAnswerPosts.length) {
<!-- collapse answers -->
@if (showAnswers) {
<div>
<button class="reaction-button clickable collapse-answers-btn" [class.reaction-button--reacted]="showAnswers" (click)="showAnswersChange.emit(false)">
<button class="reaction-button clickable collapse-answers-btn" [class.reaction-button--reacted]="showAnswers" (click)="closeAnswerView()">
<jhi-emoji [emoji]="closeCrossId" />
<span jhiTranslate="artemisApp.metis.collapseAnswers" class="emoji-count"></span>
</button>
</div>
} @else {
<!-- expand answers -->
<div>
<button class="reaction-button clickable expand-answers-btn" (click)="showAnswersChange.emit(true)">
<jhi-emoji [emoji]="speechBalloonId" />
<button class="reaction-button clickable expand-answers-btn" (click)="openAnswerView()">
<fa-icon class="fa-xs align-self-center" [icon]="faArrowRight" />
<span class="emoji-count">{{
sortedAnswerPosts.length === 1
? ('artemisApp.metis.showSingleAnswer' | artemisTranslate)
Expand All @@ -22,26 +30,9 @@
</button>
</div>
}
} @else {
<!-- start discussion -->
<div>
<button class="reaction-button clickable reply-btn" (click)="openPostingCreateEditModal.emit()" [disabled]="readOnlyMode">
<jhi-emoji [emoji]="speechBalloonId" />
<span jhiTranslate="artemisApp.metis.reply" class="emoji-count"></span>
</button>
</div>
}
} @else {
@if (!isThreadSidebar) {
<!-- start discussion -->
@if (hoverBar && sortedAnswerPosts.length === 0) {
<div>
<button class="reaction-button clickable reply-btn" (click)="openThread.emit()">
<fa-icon class="fa-xs align-self-center" [icon]="faArrowRight" />
<span jhiTranslate="artemisApp.conversationsLayout.threadSideBar.reply" class="emoji-count px-2"></span>
</button>
</div>
}
<!-- expand answers -->
@if (!showAnswers && sortedAnswerPosts.length) {
<div>
Expand Down Expand Up @@ -142,7 +133,7 @@
}
</div>
<div>
@if (getShowNewMessageIcon()) {
@if (isEmojiCount && getShowNewMessageIcon()) {
<div jhiTranslate="global.generic.new" class="badge bg-secondary hideAfter5Seconds"></div>
}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export class PostReactionsBarComponent extends PostingsReactionsBarDirective<Pos

@Output() showAnswersChange = new EventEmitter<boolean>();
@Output() openPostingCreateEditModal = new EventEmitter<void>();
@Output() closePostingCreateEditModal = new EventEmitter<void>();
@Output() openThread = new EventEmitter<void>();
@Input() previewMode: boolean;
isAtLeastInstructorInCourse: boolean;
Expand All @@ -63,6 +64,16 @@ export class PostReactionsBarComponent extends PostingsReactionsBarDirective<Pos
return Object.values(this.reactionMetaDataMap).some((reaction) => reaction.count >= 1);
}

openAnswerView() {
this.showAnswersChange.emit(true);
this.openPostingCreateEditModal.emit();
}

closeAnswerView() {
this.showAnswersChange.emit(false);
this.closePostingCreateEditModal.emit();
}

/**
* on initialization: call resetTooltipsAndPriority
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
border-radius: 1rem;
align-items: center;
height: 1.2rem;
display: inline-flex;

&.open-selector {
box-shadow: inset 0 0 0 1px var(--metis-gray);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,15 @@ describe('PostComponent', () => {
expect(postFooterOpenCreateAnswerPostModal).toHaveBeenCalledOnce();
});

it('should close create answer post modal', () => {
component.posting = metisPostExerciseUser1;
component.ngOnInit();
fixture.detectChanges();
const postFooterOpenCreateAnswerPostModal = jest.spyOn(component.postFooterComponent, 'closeCreateAnswerPostModal');
component.closeCreateAnswerPostModal();
expect(postFooterOpenCreateAnswerPostModal).toHaveBeenCalledOnce();
});

it('should create or navigate to oneToOneChat when not on messaging page', () => {
const navigateSpy = jest.spyOn(router, 'navigate');
const oneToOneChatService = TestBed.inject(OneToOneChatService);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,13 @@ describe('PostFooterComponent', () => {
component.openCreateAnswerPostModal();
expect(createAnswerPostModalOpen).toHaveBeenCalledOnce();
});

it('should close create answer post modal', () => {
component.posting = metisPostExerciseUser1;
component.ngOnInit();
fixture.detectChanges();
const createAnswerPostModalClose = jest.spyOn(component.createAnswerPostModalComponent, 'close');
component.closeCreateAnswerPostModal();
expect(createAnswerPostModalClose).toHaveBeenCalledOnce();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -193,15 +193,6 @@ describe('AnswerPostReactionsBarComponent', () => {
expect(answerNowButton).toBeNull();
});

it('answer now button should be visible if answer is the last one', () => {
component.posting = post;
component.isLastAnswer = true;
component.ngOnInit();
fixture.detectChanges();
const answerNowButton = fixture.debugElement.query(By.css('.reply-btn')).nativeElement;
expect(answerNowButton.innerHTML).toContain('reply');
});

it('should invoke metis service when toggle resolve is clicked', () => {
metisServiceUserPostingAuthorMock.mockReturnValue(true);
fixture.detectChanges();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { MetisService } from 'app/shared/metis/metis.service';
import { DebugElement } from '@angular/core';
import { Post } from 'app/entities/metis/post.model';
import { MockComponent, MockDirective, MockModule, MockPipe, MockProvider } from 'ng-mocks';
import { getElement, getElements } from '../../../../../helpers/utils/general.utils';
import { getElement } from '../../../../../helpers/utils/general.utils';
import { PostReactionsBarComponent } from 'app/shared/metis/posting-reactions-bar/post-reactions-bar/post-reactions-bar.component';
import { OverlayModule } from '@angular/cdk/overlay';
import { Reaction } from 'app/entities/metis/reaction.model';
Expand Down Expand Up @@ -239,9 +239,8 @@ describe('PostReactionsBarComponent', () => {
component.posting = post;

component.ngOnInit();
component.isEmojiCount = true;
fixture.detectChanges();
const reactions = getElements(debugElement, 'jhi-emoji');
expect(reactions).toHaveLength(2);
expect(component.reactionMetaDataMap).toEqual({
smile: {
count: 1,
Expand Down Expand Up @@ -311,14 +310,6 @@ describe('PostReactionsBarComponent', () => {
expect(component.pinTooltip).toBe('artemisApp.metis.pinnedPostTooltip');
});

it('start discussion button should be visible if post does not yet have any answers', () => {
component.posting = post;
component.sortedAnswerPosts = [];
fixture.detectChanges();
const startDiscussion = fixture.debugElement.query(By.css('.reply-btn')).nativeElement;
expect(startDiscussion.innerHTML).toContain('reply');
});

it('should display button to show single answer', () => {
component.posting = post;
component.sortedAnswerPosts = [metisPostExerciseUser1];
Expand All @@ -343,4 +334,24 @@ describe('PostReactionsBarComponent', () => {
const answerNowButton = fixture.debugElement.query(By.css('.collapse-answers-btn')).nativeElement;
expect(answerNowButton.innerHTML).toContain('collapseAnswers');
});

it('should emit showAnswersChange and openPostingCreateEditModal when openAnswerView is called', () => {
const showAnswersChangeSpy = jest.spyOn(component.showAnswersChange, 'emit');
const openPostingCreateEditModalSpy = jest.spyOn(component.openPostingCreateEditModal, 'emit');

component.openAnswerView();

expect(showAnswersChangeSpy).toHaveBeenCalledWith(true);
expect(openPostingCreateEditModalSpy).toHaveBeenCalled();
});

it('should emit showAnswersChange and closePostingCreateEditModal when closeAnswerView is called', () => {
const showAnswersChangeSpy = jest.spyOn(component.showAnswersChange, 'emit');
const closePostingCreateEditModalSpy = jest.spyOn(component.closePostingCreateEditModal, 'emit');

component.closeAnswerView();

expect(showAnswersChangeSpy).toHaveBeenCalledWith(false);
expect(closePostingCreateEditModalSpy).toHaveBeenCalled();
});
});
Loading