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: Add course-wide channel option #10052

Open
wants to merge 2 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 @@ -232,6 +232,7 @@ public ResponseEntity<ChannelDTO> createChannel(@PathVariable Long courseId, @Re
channelToCreate.setIsAnnouncementChannel(channelDTO.getIsAnnouncementChannel());
channelToCreate.setIsArchived(false);
channelToCreate.setDescription(channelDTO.getDescription());
channelToCreate.setIsCourseWide(channelDTO.getIsCourseWide());

if (channelToCreate.getName() != null && channelToCreate.getName().trim().startsWith("$")) {
throw new BadRequestAlertException("User generated channels cannot start with $", "channel", "channelNameInvalid");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ export class CourseConversationsComponent implements OnInit, OnDestroy {
this.channelActions$
.pipe(
debounceTime(500),
distinctUntilChanged((prev, curr) => prev.action === curr.action && prev.channel.id === curr.channel.id),
distinctUntilChanged((prev, curr) => prev.action === curr.action && prev.channel.id === curr.channel.id && prev.channel.name === curr.channel.name),
takeUntil(this.ngUnsubscribe),
)
.subscribe((channelAction) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,31 @@
></small>
</div>
</div>
<!-- Course-Wide Channel -->
Copy link
Member

Choose a reason for hiding this comment

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

this option should only be shown for moderators of the course (tutors, instructors, admins). All others should not even see the option

<div>
<div class="form-group">
<label class="d-block" jhiTranslate="artemisApp.dialogs.createChannel.channelForm.isCourseWideChannelInput.label"></label>
<div class="btn-group" role="group">
<input formControlName="isCourseWideChannel" type="radio" class="btn-check" id="isCourseWideChannel" autocomplete="off" checked [value]="true" />
<label
class="btn btn-outline-secondary"
for="isCourseWideChannel"
jhiTranslate="artemisApp.dialogs.createChannel.channelForm.isCourseWideChannelInput.true"
></label>
<input formControlName="isCourseWideChannel" type="radio" class="btn-check" id="isNotCourseWideChannel" autocomplete="off" [value]="false" />
<label
class="btn btn-outline-secondary"
for="isNotCourseWideChannel"
jhiTranslate="artemisApp.dialogs.createChannel.channelForm.isCourseWideChannelInput.false"
></label>
</div>
<small
id="isCourseWideChannelHelp"
class="form-text text-body-secondary d-block"
jhiTranslate="artemisApp.dialogs.createChannel.channelForm.isCourseWideChannelInput.explanation"
></small>
</div>
</div>
<!--Announcement Channel -->
<div>
<div class="form-group">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, EventEmitter, OnChanges, OnDestroy, OnInit, Output } from '@angular/core';
import { Component, EventEmitter, OnChanges, OnDestroy, OnInit, Output, output } from '@angular/core';
import { FormBuilder, FormGroup, Validators } from '@angular/forms';
import { Subject, takeUntil } from 'rxjs';

Expand All @@ -7,6 +7,7 @@ export interface ChannelFormData {
description?: string;
isPublic?: boolean;
isAnnouncementChannel?: boolean;
isCourseWideChannel?: boolean;
}

export type ChannelType = 'PUBLIC' | 'PRIVATE';
Expand All @@ -25,10 +26,12 @@ export class ChannelFormComponent implements OnInit, OnChanges, OnDestroy {
description: undefined,
isPublic: undefined,
isAnnouncementChannel: undefined,
isCourseWideChannel: undefined,
};
@Output() formSubmitted: EventEmitter<ChannelFormData> = new EventEmitter<ChannelFormData>();
@Output() channelTypeChanged: EventEmitter<ChannelType> = new EventEmitter<ChannelType>();
@Output() isAnnouncementChannelChanged: EventEmitter<boolean> = new EventEmitter<boolean>();
isCourseWideChannelChanged = output<boolean>();

form: FormGroup;

Expand All @@ -50,6 +53,10 @@ export class ChannelFormComponent implements OnInit, OnChanges, OnDestroy {
return this.form.get('isAnnouncementChannel');
}

get isisCourseWideChannelControl() {
return this.form.get('isCourseWideChannel');
}

get isSubmitPossible() {
return !this.form.invalid;
}
Expand Down Expand Up @@ -81,6 +88,7 @@ export class ChannelFormComponent implements OnInit, OnChanges, OnDestroy {
description: [undefined, [Validators.maxLength(250)]],
isPublic: [true, [Validators.required]],
isAnnouncementChannel: [false, [Validators.required]],
isCourseWideChannel: [false, [Validators.required]],
});

if (this.isPublicControl) {
Expand All @@ -94,5 +102,11 @@ export class ChannelFormComponent implements OnInit, OnChanges, OnDestroy {
this.isAnnouncementChannelChanged.emit(value);
});
}

if (this.isisCourseWideChannelControl) {
this.isisCourseWideChannelControl.valueChanges.pipe(takeUntil(this.ngUnsubscribe)).subscribe((value) => {
this.isCourseWideChannelChanged.emit(value);
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<h4 class="modal-title">
<span
>{{ (isPublicChannel ? 'artemisApp.dialogs.createChannel.titlePublicChannel' : 'artemisApp.dialogs.createChannel.titlePrivateChannel') | artemisTranslate }}
{{ isCourseWideChannel ? ('artemisApp.dialogs.createChannel.titleCourseWideChannel' | artemisTranslate) : '' }}
{{
(isAnnouncementChannel ? 'artemisApp.dialogs.createChannel.titleAnnouncementChannel' : 'artemisApp.dialogs.createChannel.titleRegularChannel')
| artemisTranslate
Expand All @@ -17,6 +18,7 @@ <h4 class="modal-title">
<jhi-channel-form
(channelTypeChanged)="onChannelTypeChanged($event)"
(isAnnouncementChannelChanged)="onIsAnnouncementChannelChanged($event)"
(isCourseWideChannelChanged)="onIsCourseWideChannelChanged($event)"
(formSubmitted)="onFormSubmitted($event)"
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export class ChannelsCreateDialogComponent extends AbstractDialogComponent {
channelToCreate: ChannelDTO = new ChannelDTO();
isPublicChannel = true;
isAnnouncementChannel = false;
isCourseWideChannel = false;

onChannelTypeChanged($event: ChannelType) {
this.isPublicChannel = $event === 'PUBLIC';
}
Expand All @@ -27,16 +29,21 @@ export class ChannelsCreateDialogComponent extends AbstractDialogComponent {
this.isAnnouncementChannel = $event;
}

onIsCourseWideChannelChanged($event: boolean) {
this.isCourseWideChannel = $event;
}

onFormSubmitted($event: ChannelFormData) {
this.createChannel($event);
}

createChannel(formData: ChannelFormData) {
const { name, description, isPublic, isAnnouncementChannel } = formData;
const { name, description, isPublic, isAnnouncementChannel, isCourseWideChannel } = formData;
this.channelToCreate.name = name ? name.trim() : undefined;
this.channelToCreate.description = description ? description.trim() : undefined;
this.channelToCreate.isPublic = isPublic;
this.channelToCreate.isAnnouncementChannel = isAnnouncementChannel;
this.channelToCreate.isCourseWide = isCourseWideChannel;
this.close(this.channelToCreate);
}
}
7 changes: 7 additions & 0 deletions src/main/webapp/i18n/de/conversation.json
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@
"createChannel": {
"titlePublicChannel": "Erstelle einen öffentlichen",
"titlePrivateChannel": "Erstelle einen privaten",
"titleCourseWideChannel": "kursweit",
"titleAnnouncementChannel": "Ankündigungskanal",
"titleRegularChannel": "Kanal",
"description": "Ein Kanal ist eine Möglichkeit, Menschen für ein Projekt, ein Thema oder nur zum Spaß zusammenzubringen. Du kannst so viele Kanäle erstellen, wie du möchtest. Du wirst der / die erste Kanalmoderator:in werden. Du wirst den Kanal nicht verlassen können.",
Expand Down Expand Up @@ -236,6 +237,12 @@
"true": "Ankündigungskanal",
"false": "Uneingeschränkter Kanal"
},
"isCourseWideChannelInput": {
"label": "Kanalbereich",
"explanation": "In einem kursweiten Kanal werden alle Benutzer des Kurses automatisch hinzugefügt. In einem zielgerichteten Kanal können Sie die hinzuzufügenden Benutzer manuell auswählen.",
asliayk marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

we use "du" instead of "Sie"

"true": "Kursweiter Kanal",
"false": "Zielgerichteter Kanal"
},
"createButton": "Kanal erstellen"
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/main/webapp/i18n/en/conversation.json
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@
"createChannel": {
"titlePublicChannel": "Create a public",
"titlePrivateChannel": "Create a private",
"titleCourseWideChannel": "course-wide",
"titleAnnouncementChannel": "announcement channel",
"titleRegularChannel": "channel",
"description": "A channel is a way to group people together around a project, a topic, or just for fun. You can create as many channels as you want. You will become the first channel moderator. You will not be able to leave the channel.",
Expand Down Expand Up @@ -236,6 +237,12 @@
"true": "Announcement Channel",
"false": "Unrestricted Channel"
},
"isCourseWideChannelInput": {
"label": "Channel Scope",
"explanation": "In a course-wide channel, all users in the course are automatically added. In a targeted channel, you can manually select the users to be added after creation.",
"true": "Course-wide Channel",
"false": "Targeted Channel"
Copy link
Member

Choose a reason for hiding this comment

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

I don't really like targeted channel. Can you propose a different name?

},
"createButton": "Create Channel"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ ChannelDTO createChannel(boolean isPublicChannel, String name) throws Exception
channelDTO.setIsPublic(isPublicChannel);
channelDTO.setIsAnnouncementChannel(false);
channelDTO.setDescription("general channel");
channelDTO.setIsCourseWide(false);

var chat = request.postWithResponseBody("/api/courses/" + exampleCourseId + "/channels", channelDTO, ChannelDTO.class, HttpStatus.CREATED);
resetWebsocketMock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ private void isAllowedToCreateChannelTest(boolean isPublicChannel, String loginN
channelDTO.setIsPublic(isPublicChannel);
channelDTO.setIsAnnouncementChannel(false);
channelDTO.setDescription("general channel");
channelDTO.setIsCourseWide(false);
asliayk marked this conversation as resolved.
Show resolved Hide resolved

// when
var chat = request.postWithResponseBody("/api/courses/" + exampleCourseId + "/channels", channelDTO, ChannelDTO.class, HttpStatus.CREATED);
Expand Down Expand Up @@ -169,6 +170,7 @@ void createTest_messagingDeactivated(CourseInformationSharingConfiguration cours
channelDTO.setIsAnnouncementChannel(false);
channelDTO.setName(TEST_PREFIX);
channelDTO.setDescription("general channel");
channelDTO.setIsCourseWide(false);

expectCreateForbidden(channelDTO);

Expand All @@ -187,6 +189,7 @@ void update_messagingFeatureDeactivated_shouldReturnForbidden() throws Exception
channelDTO.setIsAnnouncementChannel(false);
channelDTO.setName(TEST_PREFIX);
channelDTO.setDescription("general channel");
channelDTO.setIsCourseWide(false);

expectUpdateForbidden(1L, channelDTO);

Expand Down Expand Up @@ -232,6 +235,7 @@ void createChannel_asNonCourseInstructorOrTutorOrEditor_shouldReturnForbidden(bo
channelDTO.setIsPublic(isPublicChannel);
channelDTO.setIsAnnouncementChannel(false);
channelDTO.setDescription("general channel");
channelDTO.setIsCourseWide(false);

// then
expectCreateForbidden(channelDTO);
Expand Down Expand Up @@ -930,6 +934,7 @@ void createFeedbackChannel_asStudent_shouldReturnForbidden() {
channelDTO.setDescription("Discussion channel for feedback");
channelDTO.setIsPublic(true);
channelDTO.setIsAnnouncementChannel(false);
channelDTO.setIsCourseWide(false);

FeedbackChannelRequestDTO feedbackChannelRequest = new FeedbackChannelRequestDTO(channelDTO, "Sample feedback text");

Expand All @@ -954,6 +959,7 @@ void createFeedbackChannel_asInstructor_shouldCreateChannel() {
channelDTO.setDescription("Discussion channel for feedback");
channelDTO.setIsPublic(true);
channelDTO.setIsAnnouncementChannel(false);
channelDTO.setIsCourseWide(false);

FeedbackChannelRequestDTO feedbackChannelRequest = new FeedbackChannelRequestDTO(channelDTO, "Sample feedback text");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('ChannelFormComponent', () => {
const validDescription = 'This is a general channel';
const validIsPublic = true;
const validIsAnnouncementChannel = false;
const validIsCourseWideChannel = false;
asliayk marked this conversation as resolved.
Show resolved Hide resolved

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
Expand Down Expand Up @@ -73,6 +74,7 @@ describe('ChannelFormComponent', () => {
description: undefined,
isPublic: validIsPublic,
isAnnouncementChannel: validIsAnnouncementChannel,
isCourseWideChannel: validIsCourseWideChannel,
};

clickSubmitButton(true, expectChannelData);
Expand All @@ -95,6 +97,7 @@ describe('ChannelFormComponent', () => {
description: validDescription,
isPublic: validIsPublic,
isAnnouncementChannel: validIsAnnouncementChannel,
isCourseWideChannel: validIsCourseWideChannel,
};

clickSubmitButton(true, expectChannelData);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { ChannelsCreateDialogComponent } from 'app/overview/course-conversations
import { ArtemisTranslatePipe } from 'app/shared/pipes/artemis-translate.pipe';
import { MockPipe, MockProvider } from 'ng-mocks';
import { Course } from 'app/entities/course.model';
import { Component, EventEmitter, Output } from '@angular/core';
import { Component, output } from '@angular/core';
import { ChannelFormData, ChannelType } from 'app/overview/course-conversations/dialogs/channels-create-dialog/channel-form/channel-form.component';
import { By } from '@angular/platform-browser';
import { Channel } from 'app/entities/metis/conversation/channel.model';
Expand All @@ -15,9 +15,10 @@ import { initializeDialog } from '../dialog-test-helpers';
template: '',
})
class ChannelFormStubComponent {
@Output() formSubmitted: EventEmitter<ChannelFormData> = new EventEmitter<ChannelFormData>();
@Output() channelTypeChanged: EventEmitter<ChannelType> = new EventEmitter<ChannelType>();
@Output() isAnnouncementChannelChanged: EventEmitter<boolean> = new EventEmitter<boolean>();
formSubmitted = output<ChannelFormData>();
channelTypeChanged = output<ChannelType>();
isAnnouncementChannelChanged = output<boolean>();
isCourseWideChannelChanged = output<boolean>();
}

describe('ChannelsCreateDialogComponent', () => {
Expand Down Expand Up @@ -47,6 +48,13 @@ describe('ChannelsCreateDialogComponent', () => {
expect(component).toBeTruthy();
});

it('should initialize the dialog correctly', () => {
const initializeSpy = jest.spyOn(component, 'initialize');
component.initialize();
expect(initializeSpy).toHaveBeenCalledOnce();
expect(component.course).toBe(course);
});

it('clicking close button in modal header should dismiss the modal', () => {
const closeButton = fixture.debugElement.nativeElement.querySelector('.modal-header button');
const activeModal = TestBed.inject(NgbActiveModal);
Expand All @@ -69,6 +77,13 @@ describe('ChannelsCreateDialogComponent', () => {
expect(component.isAnnouncementChannel).toBeTrue();
});

it('should change channel scope type when channel scope type is changed in channel form', () => {
expect(component.isCourseWideChannel).toBeFalse();
const form: ChannelFormStubComponent = fixture.debugElement.query(By.directive(ChannelFormStubComponent)).componentInstance;
form.isCourseWideChannelChanged.emit(true);
expect(component.isCourseWideChannel).toBeTrue();
});

it('should close modal with the channel to create when form is submitted', () => {
const activeModal = TestBed.inject(NgbActiveModal);
const closeSpy = jest.spyOn(activeModal, 'close');
Expand All @@ -89,4 +104,46 @@ describe('ChannelsCreateDialogComponent', () => {
expect(closeSpy).toHaveBeenCalledOnce();
expect(closeSpy).toHaveBeenCalledWith(expectedChannel);
});

it('should call createChannel with correct data', () => {
const createChannelSpy = jest.spyOn(component, 'createChannel');

const formData: ChannelFormData = {
name: 'testChannel',
description: 'Test description',
isPublic: false,
isAnnouncementChannel: true,
isCourseWideChannel: false,
};

const form: ChannelFormStubComponent = fixture.debugElement.query(By.directive(ChannelFormStubComponent)).componentInstance;
form.formSubmitted.emit(formData);

expect(createChannelSpy).toHaveBeenCalledOnce();
expect(createChannelSpy).toHaveBeenCalledWith(formData);
});

it('should close modal when createChannel is called', () => {
const activeModal = TestBed.inject(NgbActiveModal);
const closeSpy = jest.spyOn(activeModal, 'close');

const formData: ChannelFormData = {
name: 'testChannel',
description: 'Test description',
isPublic: true,
isAnnouncementChannel: false,
isCourseWideChannel: true,
};

component.createChannel(formData);

expect(closeSpy).toHaveBeenCalledOnce();
expect(closeSpy).toHaveBeenCalledWith(
expect.objectContaining({
name: formData.name,
description: formData.description,
isPublic: formData.isPublic,
}),
);
});
});
13 changes: 13 additions & 0 deletions src/test/playwright/e2e/course/CourseMessages.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,19 @@ test.describe('Course messages', { tag: '@fast' }, () => {
await expect(courseMessages.getName()).toContainText(name);
});

test('Instructor should be able to create a public course-wide unrestricted channel', async ({ login, courseMessages }) => {
await login(instructor, `/courses/${course.id}/communication`);
const name = 'public-cw-unrstct-ch';
await courseMessages.createChannelButton();
await courseMessages.setName(name);
await courseMessages.setDescription('A public unrestricted channel');
await courseMessages.setPublic();
await courseMessages.setUnrestrictedChannel();
await courseMessages.setCourseWideChannel();
await courseMessages.createChannel(false, true);
await expect(courseMessages.getName()).toContainText(name);
});

test('Instructor should be able to create a private unrestricted channel', async ({ login, courseMessages }) => {
await login(instructor, `/courses/${course.id}/communication`);
const name = 'private-unrstct-ch';
Expand Down
Loading
Loading