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 4 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 Expand Up @@ -34,6 +33,7 @@
(reactionsUpdated)="onReactionsUpdated($event)"
(mayEditOrDeleteOutput)="onMayEditOrDelete($event)"
(isDeleteEvent)="onDeleteEvent(true)"
[hasChannelModerationRights]="hasChannelModerationRights()"
/>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export class AnswerPostComponent extends PostingDirective<AnswerPost> {
static activeDropdownPost: AnswerPostComponent | null = null;
mayEditOrDelete: boolean = false;
@ViewChild(AnswerPostReactionsBarComponent) private reactionsBarComponent!: AnswerPostReactionsBarComponent;
hasChannelModerationRights = input<boolean>(false);

constructor(
public changeDetector: ChangeDetectorRef,
Expand Down
10 changes: 6 additions & 4 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 All @@ -83,6 +83,7 @@
(mayEditOrDeleteOutput)="onMayEditOrDelete($event)"
(canPinOutput)="onCanPin($event)"
(isDeleteEvent)="onDeleteEvent(true)"
[hasChannelModerationRights]="hasChannelModerationRights()"
/>
}
</div>
Expand All @@ -98,7 +99,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 +111,7 @@
[isCommunicationPage]="isCommunicationPage"
[isThreadSidebar]="isThreadSidebar"
(openPostingCreateEditModal)="openCreateAnswerPostModal()"
(closePostingCreateEditModal)="closeCreateAnswerPostModal()"
(openThread)="openThread.emit()"
(isModalOpen)="displayInlineInput = true"
[isEmojiCount]="true"
Expand All @@ -133,7 +135,7 @@
[lastReadDate]="lastReadDate"
(userReferenceClicked)="onUserReferenceClicked($event)"
(channelReferenceClicked)="onChannelReferenceClicked($event)"
[hasChannelModerationRights]="hasChannelModerationRights"
[hasChannelModerationRights]="hasChannelModerationRights()"
/>

<!-- Right-Click Dropdown -->
Expand All @@ -158,7 +160,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 @@ -25,7 +25,7 @@
(openPostingCreateEditModal)="createAnswerPostModal.open()"
(userReferenceClicked)="userReferenceClicked.emit($event)"
(channelReferenceClicked)="channelReferenceClicked.emit($event)"
[hasChannelModerationRights]="hasChannelModerationRights"
[hasChannelModerationRights]="hasChannelModerationRights()"
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent permission check implementations found

The following files still use property binding instead of method invocation for hasChannelModerationRights:

  • src/main/webapp/app/shared/metis/posting-thread/posting-thread.component.html
  • src/main/webapp/app/overview/course-conversations/layout/conversation-thread-sidebar/conversation-thread-sidebar.component.html

These components should be updated to use method invocation hasChannelModerationRights() for consistent permission evaluation across the application.

🔗 Analysis chain

Verify permission check consistency across components.

The change from property binding to method invocation for hasChannelModerationRights aligns with the PR objective of fixing edit/delete permissions. This ensures that permissions are always freshly evaluated.

Let's verify that this change is consistent across all components:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent usage of hasChannelModerationRights across components
# Expected: All components should use the method invocation style

# Search for any remaining direct property bindings
rg -t html "hasChannelModerationRights\]=\"hasChannelModerationRights\"" src/main/webapp/

# Search for the new method invocation style
rg -t html "hasChannelModerationRights\]=\"hasChannelModerationRights\(\)\"" src/main/webapp/

Length of output: 1144

/>
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
SimpleChanges,
ViewChild,
ViewContainerRef,
input,
} from '@angular/core';
import { PostingFooterDirective } from 'app/shared/metis/posting-footer/posting-footer.directive';
import { Post } from 'app/entities/metis/post.model';
Expand All @@ -36,7 +37,6 @@ export class PostFooterComponent extends PostingFooterDirective<Post> implements
@Input() readOnlyMode = false;
@Input() previewMode = false;
@Input() modalRef?: NgbModalRef;
@Input() hasChannelModerationRights = false;
@Input() showAnswers = false;
@Input() isCommunicationPage = false;
@Input() sortedAnswerPosts: AnswerPost[] = [];
Expand All @@ -48,6 +48,7 @@ export class PostFooterComponent extends PostingFooterDirective<Post> implements
@ViewChild(AnswerPostCreateEditModalComponent) answerPostCreateEditModal?: AnswerPostCreateEditModalComponent;
@ViewChild('createEditAnswerPostContainer', { read: ViewContainerRef }) containerRef!: ViewContainerRef;
@ViewChild('createAnswerPostModal') createAnswerPostModalComponent!: AnswerPostCreateEditModalComponent;
hasChannelModerationRights = input<boolean>(false);

createdAnswerPost: AnswerPost;
isAtLeastTutorInCourse = false;
Expand Down Expand Up @@ -173,6 +174,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 @@ -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,4 +1,4 @@
import { Component, EventEmitter, Input, OnChanges, OnInit, Output } from '@angular/core';
import { Component, EventEmitter, Input, OnChanges, OnInit, Output, input } from '@angular/core';
import { Reaction } from 'app/entities/metis/reaction.model';
import { PostingsReactionsBarDirective } from 'app/shared/metis/posting-reactions-bar/posting-reactions-bar.directive';
import { AnswerPost } from 'app/entities/metis/answer-post.model';
Expand Down Expand Up @@ -27,6 +27,7 @@ export class AnswerPostReactionsBarComponent extends PostingsReactionsBarDirecti
readonly faPencilAlt = faPencilAlt;
@Input() isEmojiCount: boolean = false;
@Output() postingUpdated = new EventEmitter<void>();
hasChannelModerationRights = input<boolean>(false);

constructor(metisService: MetisService) {
super(metisService);
Expand Down Expand Up @@ -70,8 +71,7 @@ export class AnswerPostReactionsBarComponent extends PostingsReactionsBarDirecti
this.isAnswerOfAnnouncement = getAsChannelDTO(this.posting.post?.conversation)?.isAnnouncementChannel ?? false;
const isCourseWideChannel = getAsChannelDTO(this.posting.post?.conversation)?.isCourseWide ?? false;
const isAtLeastInstructorInCourse = this.metisService.metisUserIsAtLeastInstructorInCourse();
const mayEditOrDeleteOtherUsersAnswer =
(isCourseWideChannel && isAtLeastInstructorInCourse) || (getAsChannelDTO(this.metisService.getCurrentConversation())?.hasChannelModerationRights ?? false);
const mayEditOrDeleteOtherUsersAnswer = (isCourseWideChannel && isAtLeastInstructorInCourse) || (this.hasChannelModerationRights() ?? false);
this.mayEditOrDelete = !this.isReadOnlyMode && (this.isAuthorOfPosting || mayEditOrDeleteOtherUsersAnswer);
this.mayEditOrDeleteOutput.emit(this.mayEditOrDelete);
}
Expand Down
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
@@ -1,4 +1,4 @@
import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, ViewChild } from '@angular/core';
import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, ViewChild, input } from '@angular/core';
import { Reaction } from 'app/entities/metis/reaction.model';
import { Post } from 'app/entities/metis/post.model';
import { PostingsReactionsBarDirective } from 'app/shared/metis/posting-reactions-bar/posting-reactions-bar.directive';
Expand Down 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 @@ -51,6 +52,7 @@ export class PostReactionsBarComponent extends PostingsReactionsBarDirective<Pos
@Input() isEmojiCount = false;
@Input() hoverBar: boolean = true;
@ViewChild('createEditModal') createEditModal!: PostCreateEditModalComponent;
hasChannelModerationRights = input<boolean>(false);

constructor(
metisService: MetisService,
Expand All @@ -63,6 +65,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 Expand Up @@ -188,8 +200,7 @@ export class PostReactionsBarComponent extends PostingsReactionsBarDirective<Pos
setMayEditOrDelete(): void {
this.isAtLeastInstructorInCourse = this.metisService.metisUserIsAtLeastInstructorInCourse();
const isCourseWideChannel = getAsChannelDTO(this.posting.conversation)?.isCourseWide ?? false;
const mayEditOrDeleteOtherUsersAnswer =
(isCourseWideChannel && this.isAtLeastInstructorInCourse) || (getAsChannelDTO(this.metisService.getCurrentConversation())?.hasChannelModerationRights ?? false);
const mayEditOrDeleteOtherUsersAnswer = (isCourseWideChannel && this.isAtLeastInstructorInCourse) || (this.hasChannelModerationRights() ?? false);
this.mayEditOrDelete = !this.readOnlyMode && !this.previewMode && (this.isAuthorOfPosting || mayEditOrDeleteOtherUsersAnswer);
this.mayEditOrDeleteOutput.emit(this.mayEditOrDelete);
}
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
4 changes: 2 additions & 2 deletions src/main/webapp/app/shared/metis/posting.directive.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Posting } from 'app/entities/metis/posting.model';
import { ChangeDetectorRef, Directive, Input, OnDestroy, OnInit, inject } from '@angular/core';
import { ChangeDetectorRef, Directive, Input, OnDestroy, OnInit, inject, input } from '@angular/core';
import { MetisService } from 'app/shared/metis/metis.service';
import { DisplayPriority } from 'app/shared/metis/metis.util';

Expand All @@ -9,7 +9,7 @@ export abstract class PostingDirective<T extends Posting> implements OnInit, OnD
@Input() isCommunicationPage: boolean;
@Input() showChannelReference?: boolean;

@Input() hasChannelModerationRights = false;
hasChannelModerationRights = input<boolean>(false);
@Input() isThreadSidebar: boolean;
abstract get reactionsBar(): any;
showDropdown = false;
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
Loading
Loading