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: Allow tutors to propose FAQ #9477

Open
wants to merge 36 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
0544837
First draft implementation: Propose FAQ as editor
cremertim Oct 9, 2024
280e3f4
First draft implementation: Added test cases
cremertim Oct 9, 2024
a7dbc99
Merge branch 'develop' into feature/communication/propose-faq
cremertim Oct 9, 2024
2eb7316
fixed tests
cremertim Oct 9, 2024
2be8182
Merge branch 'develop' into feature/communication/propose-faq
cremertim Oct 11, 2024
a1c3562
Merge branch 'refs/heads/develop' into feature/communication/propose-faq
cremertim Oct 12, 2024
0ab7a91
Added proper alert messages
cremertim Oct 12, 2024
47e51cc
merged develop
cremertim Oct 12, 2024
1f6cc87
merge changes
cremertim Oct 9, 2024
8720762
First draft implementation: Added test cases
cremertim Oct 9, 2024
7beeea6
fixed tests
cremertim Oct 9, 2024
ba80f4e
Added proper alert messages
cremertim Oct 12, 2024
059a0b5
Merge remote-tracking branch 'origin/feature/communication/propose-fa…
cremertim Oct 13, 2024
25ac203
Merge branch 'develop' into feature/communication/propose-faq
cremertim Oct 14, 2024
0313480
Included coderabit
cremertim Oct 15, 2024
e10f854
Fixed Server Style
cremertim Oct 15, 2024
e183230
Simplified FaqRessource
cremertim Oct 15, 2024
e7764fa
Fixed css to resemble artemis theme
cremertim Oct 15, 2024
1ef0556
Allowed Tutors to propose Feedback, added checks on server
cremertim Oct 15, 2024
311bdd9
Fixed Tests
cremertim Oct 16, 2024
1e40acc
Removed typos
cremertim Oct 16, 2024
440bb25
Merge branch 'develop' into feature/communication/propose-faq
cremertim Oct 16, 2024
6ae2609
Removed typos
cremertim Oct 16, 2024
6c2be71
Changed to proper article
cremertim Oct 17, 2024
9122137
adjusted on feedback
cremertim Oct 17, 2024
eb21823
Merge branch 'develop' into feature/communication/propose-faq
cremertim Oct 17, 2024
98f9da0
Adressed comments from Florian
cremertim Oct 18, 2024
24d95ae
translation fix
cremertim Oct 18, 2024
08b5a39
Merge branch 'develop' into feature/communication/propose-faq
cremertim Oct 19, 2024
1f7ca0f
allowed tutor to propose FAQ in UI
cremertim Oct 19, 2024
e532c72
Merge branch 'develop' into feature/communication/propose-faq
cremertim Oct 19, 2024
6c84bca
Merge branch 'develop' into feature/communication/propose-faq
cremertim Oct 19, 2024
1d661b1
Merge branch 'develop' into feature/communication/propose-faq
cremertim Oct 20, 2024
39a54ec
fixed with translation
cremertim Oct 20, 2024
a3118f5
Merge remote-tracking branch 'origin/feature/communication/propose-fa…
cremertim Oct 20, 2024
ef1123c
fixed tests
cremertim Oct 20, 2024
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 @@ -12,6 +12,7 @@
import org.springframework.transaction.annotation.Transactional;

import de.tum.cit.aet.artemis.communication.domain.Faq;
import de.tum.cit.aet.artemis.communication.domain.FaqState;
import de.tum.cit.aet.artemis.core.repository.base.ArtemisJpaRepository;

/**
Expand All @@ -30,6 +31,8 @@ public interface FaqRepository extends ArtemisJpaRepository<Faq, Long> {
""")
Set<String> findAllCategoriesByCourseId(@Param("courseId") Long courseId);

Set<Faq> findAllByCourseIdAndFaqState(Long courseId, FaqState faqState);

@Transactional
@Modifying
void deleteAllByCourseId(Long courseId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,17 @@
import org.springframework.web.bind.annotation.RestController;

import de.tum.cit.aet.artemis.communication.domain.Faq;
import de.tum.cit.aet.artemis.communication.domain.FaqState;
import de.tum.cit.aet.artemis.communication.dto.FaqDTO;
import de.tum.cit.aet.artemis.communication.repository.FaqRepository;
import de.tum.cit.aet.artemis.core.domain.Course;
import de.tum.cit.aet.artemis.core.exception.AccessForbiddenException;
import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException;
import de.tum.cit.aet.artemis.core.repository.CourseRepository;
import de.tum.cit.aet.artemis.core.security.Role;
import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastInstructor;
import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastStudent;
import de.tum.cit.aet.artemis.core.security.annotations.enforceRoleInCourse.EnforceAtLeastInstructorInCourse;
import de.tum.cit.aet.artemis.core.security.annotations.enforceRoleInCourse.EnforceAtLeastStudentInCourse;
import de.tum.cit.aet.artemis.core.security.annotations.enforceRoleInCourse.EnforceAtLeastTutorInCourse;
import de.tum.cit.aet.artemis.core.service.AuthorizationCheckService;
import de.tum.cit.aet.artemis.core.util.HeaderUtil;

Expand All @@ -56,10 +59,9 @@ public class FaqResource {
private final FaqRepository faqRepository;

public FaqResource(CourseRepository courseRepository, AuthorizationCheckService authCheckService, FaqRepository faqRepository) {

this.faqRepository = faqRepository;
this.courseRepository = courseRepository;
this.authCheckService = authCheckService;
this.faqRepository = faqRepository;
}

/**
Expand All @@ -72,18 +74,16 @@ public FaqResource(CourseRepository courseRepository, AuthorizationCheckService
* @throws URISyntaxException if the Location URI syntax is incorrect
*/
@PostMapping("courses/{courseId}/faqs")
@EnforceAtLeastInstructor
@EnforceAtLeastTutorInCourse
florian-glombik marked this conversation as resolved.
Show resolved Hide resolved
public ResponseEntity<FaqDTO> createFaq(@RequestBody Faq faq, @PathVariable Long courseId) throws URISyntaxException {
cremertim marked this conversation as resolved.
Show resolved Hide resolved
log.debug("REST request to save Faq : {}", faq);
if (faq.getId() != null) {
throw new BadRequestAlertException("A new faq cannot already have an ID", ENTITY_NAME, "idExists");
}

checkPriviledgeForAcceptedElseThrow(faq, courseId);
if (faq.getCourse() == null || !faq.getCourse().getId().equals(courseId)) {
throw new BadRequestAlertException("Course ID in path and FAQ do not match", ENTITY_NAME, "courseIdMismatch");
}
authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, faq.getCourse(), null);

Faq savedFaq = faqRepository.save(faq);
FaqDTO dto = new FaqDTO(savedFaq);
return ResponseEntity.created(new URI("/api/courses/" + courseId + "/faqs/" + savedFaq.getId())).body(dto);
Expand All @@ -99,14 +99,15 @@ public ResponseEntity<FaqDTO> createFaq(@RequestBody Faq faq, @PathVariable Long
* if the faq is not valid or if the faq course id does not match with the path variable
*/
@PutMapping("courses/{courseId}/faqs/{faqId}")
@EnforceAtLeastInstructor
@EnforceAtLeastTutorInCourse
public ResponseEntity<FaqDTO> updateFaq(@RequestBody Faq faq, @PathVariable Long faqId, @PathVariable Long courseId) {
log.debug("REST request to update Faq : {}", faq);
if (faqId == null || !faqId.equals(faq.getId())) {
throw new BadRequestAlertException("Id of FAQ and path must match", ENTITY_NAME, "idNull");
}
authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, faq.getCourse(), null);
checkPriviledgeForAcceptedElseThrow(faq, courseId);
Faq existingFaq = faqRepository.findByIdElseThrow(faqId);
checkPriviledgeForAcceptedElseThrow(existingFaq, courseId);
florian-glombik marked this conversation as resolved.
Show resolved Hide resolved
if (!Objects.equals(existingFaq.getCourse().getId(), courseId)) {
throw new BadRequestAlertException("Course ID of the FAQ provided courseID must match", ENTITY_NAME, "idNull");
}
Expand All @@ -115,6 +116,19 @@ public ResponseEntity<FaqDTO> updateFaq(@RequestBody Faq faq, @PathVariable Long
return ResponseEntity.ok().body(dto);
}

/**
* @param faq the faq to be checked *
* @param courseId the id of the course the faq belongs to
* @throws AccessForbiddenException if the user is not an instructor
*
*/
private void checkPriviledgeForAcceptedElseThrow(Faq faq, Long courseId) {
if (faq.getFaqState() == FaqState.ACCEPTED) {
Course course = courseRepository.findByIdElseThrow(courseId);
authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, null);
}
}

/**
* GET /courses/:courseId/faqs/:faqId : get the faq with the id faqId.
*
Expand All @@ -123,14 +137,13 @@ public ResponseEntity<FaqDTO> updateFaq(@RequestBody Faq faq, @PathVariable Long
* @return the ResponseEntity with status 200 (OK) and with body the faq, or with status 404 (Not Found)
*/
@GetMapping("courses/{courseId}/faqs/{faqId}")
@EnforceAtLeastStudent
@EnforceAtLeastStudentInCourse
public ResponseEntity<FaqDTO> getFaq(@PathVariable Long faqId, @PathVariable Long courseId) {
log.debug("REST request to get faq {}", faqId);
Faq faq = faqRepository.findByIdElseThrow(faqId);
if (faq.getCourse() == null || !faq.getCourse().getId().equals(courseId)) {
throw new BadRequestAlertException("Course ID in path and FAQ do not match", ENTITY_NAME, "courseIdMismatch");
}
authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.STUDENT, faq.getCourse(), null);
FaqDTO dto = new FaqDTO(faq);
return ResponseEntity.ok(dto);
}
Expand All @@ -143,12 +156,11 @@ public ResponseEntity<FaqDTO> getFaq(@PathVariable Long faqId, @PathVariable Lon
* @return the ResponseEntity with status 200 (OK)
*/
@DeleteMapping("courses/{courseId}/faqs/{faqId}")
@EnforceAtLeastInstructor
@EnforceAtLeastInstructorInCourse
public ResponseEntity<Void> deleteFaq(@PathVariable Long faqId, @PathVariable Long courseId) {

log.debug("REST request to delete faq {}", faqId);
Faq existingFaq = faqRepository.findByIdElseThrow(faqId);
authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, existingFaq.getCourse(), null);
if (!Objects.equals(existingFaq.getCourse().getId(), courseId)) {
throw new BadRequestAlertException("Course ID of the FAQ provided courseID must match", ENTITY_NAME, "idNull");
}
Expand All @@ -163,30 +175,40 @@ public ResponseEntity<Void> deleteFaq(@PathVariable Long faqId, @PathVariable Lo
* @return the ResponseEntity with status 200 (OK) and the list of faqs in body
*/
@GetMapping("courses/{courseId}/faqs")
@EnforceAtLeastStudent
@EnforceAtLeastStudentInCourse
public ResponseEntity<Set<FaqDTO>> getFaqForCourse(@PathVariable Long courseId) {
log.debug("REST request to get all Faqs for the course with id : {}", courseId);

Course course = courseRepository.findByIdElseThrow(courseId);
authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.STUDENT, course, null);
Set<Faq> faqs = faqRepository.findAllByCourseId(courseId);
Set<FaqDTO> faqDTOS = faqs.stream().map(FaqDTO::new).collect(Collectors.toSet());
return ResponseEntity.ok().body(faqDTOS);
}

/**
* GET /courses/:courseId/faq-status/:faqState : get all the faqs of a course in the specified status
*
* @param courseId the courseId of the course for which all faqs should be returned
* @param faqState the state of all returned FAQs
* @return the ResponseEntity with status 200 (OK) and the list of faqs in body
*/
cremertim marked this conversation as resolved.
Show resolved Hide resolved
@GetMapping("courses/{courseId}/faq-state/{faqState}")
@EnforceAtLeastStudentInCourse
public ResponseEntity<Set<FaqDTO>> getAllFaqsForCourseByStatus(@PathVariable Long courseId, @PathVariable FaqState faqState) {
cremertim marked this conversation as resolved.
Show resolved Hide resolved
log.debug("REST request to get all Faqs for the course with id : " + courseId + "and status " + faqState, courseId);
cremertim marked this conversation as resolved.
Show resolved Hide resolved
Set<Faq> faqs = faqRepository.findAllByCourseIdAndFaqState(courseId, faqState);
Set<FaqDTO> faqDTOS = faqs.stream().map(FaqDTO::new).collect(Collectors.toSet());
return ResponseEntity.ok().body(faqDTOS);
}

/**
* GET /courses/:courseId/faq-categories : get all the faq categories of a course
*
* @param courseId the courseId of the course for which all faq categories should be returned
* @return the ResponseEntity with status 200 (OK) and the list of faqs in body
*/
@GetMapping("courses/{courseId}/faq-categories")
@EnforceAtLeastStudent
@EnforceAtLeastStudentInCourse
public ResponseEntity<Set<String>> getFaqCategoriesForCourse(@PathVariable Long courseId) {
log.debug("REST request to get all Faq Categories for the course with id : {}", courseId);

Course course = courseRepository.findByIdElseThrow(courseId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this unintentionally there or is there a specific reason this has been removed now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it to stay consistent with all other implementations. You couldleave it in, but it does not grant much benefit besides ensuring that the request comes from a student of the course. however, it is not so, there would be no benefit for the caller besides seeing categories

authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.STUDENT, course, null);
Set<String> faqs = faqRepository.findAllCategoriesByCourseId(courseId);

return ResponseEntity.ok().body(faqs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
<span class="tab-link-text" jhiTranslate="entity.action.scores"></span>
</a>
}
@if (course.isAtLeastInstructor && course.faqEnabled) {
@if (course.isAtLeastTutor && course.faqEnabled) {
<a class="tab-link" [routerLink]="['/course-management', course.id, 'faqs']" routerLinkActive="active">
<fa-icon [icon]="faQuestion" />
<span class="tab-link-text" jhiTranslate="entity.action.faq"></span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ export const courseManagementState: Routes = [
course: CourseManagementResolve,
},
data: {
authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
authorities: [Authority.TA, Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
pageTitle: 'artemisApp.faq.home.title',
},
canActivate: [UserRouteAccessService],
Expand All @@ -363,7 +363,7 @@ export const courseManagementState: Routes = [
path: 'new',
component: FaqUpdateComponent,
data: {
authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
authorities: [Authority.TA, Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
pageTitle: 'global.generic.create',
},
canActivate: [UserRouteAccessService],
Expand All @@ -378,7 +378,7 @@ export const courseManagementState: Routes = [
path: 'edit',
component: FaqUpdateComponent,
data: {
authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
authorities: [Authority.TA, Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
pageTitle: 'global.generic.edit',
},
canActivate: [UserRouteAccessService],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ <h4 class="text-center no-exercises mt-3 fw-medium" jhiTranslate="artemisApp.cou
</a>
}

@if (course.isAtLeastInstructor && course.faqEnabled) {
@if (course.isAtLeastTutor && course.faqEnabled) {
<a
[routerLink]="['/course-management', course.id, 'faqs']"
class="btn btn-info me-1 mb-1"
Expand Down
6 changes: 3 additions & 3 deletions src/main/webapp/app/entities/faq.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { Course } from 'app/entities/course.model';
import { FaqCategory } from './faq-category.model';

export enum FaqState {
ACCEPTED,
REJECTED,
PROPOSED,
ACCEPTED = 'ACCEPTED',
REJECTED = 'REJECTED',
PROPOSED = 'PROPOSED',
cremertim marked this conversation as resolved.
Show resolved Hide resolved
}

export class Faq implements BaseEntity {
Expand Down
29 changes: 22 additions & 7 deletions src/main/webapp/app/faq/faq-update.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { ArtemisMarkdownEditorModule } from 'app/shared/markdown-editor/markdown
import { ArtemisCategorySelectorModule } from 'app/shared/category-selector/category-selector.module';
import { ArtemisSharedModule } from 'app/shared/shared.module';
import { ArtemisSharedComponentModule } from 'app/shared/components/shared-component.module';
import { AccountService } from 'app/core/auth/account.service';

@Component({
selector: 'jhi-faq-update',
Expand All @@ -31,19 +32,21 @@ export class FaqUpdateComponent implements OnInit {
existingCategories: FaqCategory[];
faqCategories: FaqCategory[];
courseId: number;
isAtLeastInstructor: boolean = false;
cremertim marked this conversation as resolved.
Show resolved Hide resolved
cremertim marked this conversation as resolved.
Show resolved Hide resolved
domainActionsDescription = [new FormulaAction()];

// Icons
faQuestionCircle = faQuestionCircle;
faSave = faSave;
faBan = faBan;
readonly faQuestionCircle = faQuestionCircle;
readonly faSave = faSave;
readonly faBan = faBan;

private alertService = inject(AlertService);
private faqService = inject(FaqService);
private activatedRoute = inject(ActivatedRoute);
private navigationUtilService = inject(ArtemisNavigationUtilService);
private router = inject(Router);
private translateService = inject(TranslateService);
private accountService = inject(AccountService);

ngOnInit() {
this.isSaving = false;
Expand All @@ -56,6 +59,7 @@ export class FaqUpdateComponent implements OnInit {
if (course) {
this.faq.course = course;
this.loadCourseFaqCategories(course.id);
this.isAtLeastInstructor = this.accountService.isAtLeastInstructorInCourse(course);
}
this.faqCategories = faq?.categories ? faq.categories : [];
});
Expand All @@ -77,10 +81,10 @@ export class FaqUpdateComponent implements OnInit {
*/
save() {
this.isSaving = true;
this.faq.faqState = this.isAtLeastInstructor ? FaqState.ACCEPTED : FaqState.PROPOSED;
if (this.faq.id !== undefined) {
this.subscribeToSaveResponse(this.faqService.update(this.courseId, this.faq));
} else {
this.faq.faqState = FaqState.ACCEPTED;
this.subscribeToSaveResponse(this.faqService.create(this.courseId, this.faq));
}
}
Expand All @@ -107,17 +111,28 @@ export class FaqUpdateComponent implements OnInit {
if (faqBody) {
this.faq = faqBody;
}
this.alertService.success(this.translateService.instant('artemisApp.faq.created', { id: faq.id }));
this.showSuccessAlert(faq, false);
this.router.navigate(['course-management', this.courseId, 'faqs']);
},
});
} else {
this.isSaving = false;
this.alertService.success(this.translateService.instant('artemisApp.faq.updated', { id: faq.id }));
this.showSuccessAlert(faq, true);
this.router.navigate(['course-management', this.courseId, 'faqs']);
}
}

private showSuccessAlert(faq: Faq, newlyCreated: boolean): void {
let messageKey: string;

if (this.isAtLeastInstructor) {
messageKey = newlyCreated ? 'artemisApp.faq.updated' : 'artemisApp.faq.created';
} else {
console.log(faq.id);
messageKey = newlyCreated ? 'artemisApp.faq.proposedChange' : 'artemisApp.faq.proposed';
}
this.alertService.success(messageKey, { title: faq.questionTitle });
}

/**
* Action on unsuccessful faq creation or edit
* @param errorRes the errorRes handed to the alert service
Expand Down
Loading
Loading