Skip to content

Commit

Permalink
Communication: Only show accepted categories of accepted FAQs (#9591)
Browse files Browse the repository at this point in the history
  • Loading branch information
@cremertim
cremertim authored Nov 2, 2024
1 parent 7c7b7d4 commit a4a56db
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 25 deletions.
8 changes: 7 additions & 1 deletion src/main/java/de/tum/cit/aet/artemis/communication/repository/FaqRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,16 @@ public interface FaqRepository extends ArtemisJpaRepository<Faq, Long> {
""")
Set<String> findAllCategoriesByCourseId(@Param("courseId") Long courseId);

@Query("""
SELECT DISTINCT faq.categories
FROM Faq faq
WHERE faq.course.id = :courseId AND faq.faqState = :faqState
""")
Set<String> findAllCategoriesByCourseIdAndState(@Param("courseId") Long courseId, @Param("faqState") FaqState faqState);

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

@Transactional
@Modifying
void deleteAllByCourseId(Long courseId);

}
58 changes: 40 additions & 18 deletions src/main/java/de/tum/cit/aet/artemis/communication/web/FaqResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public ResponseEntity<FaqDTO> createFaq(@RequestBody Faq faq, @PathVariable Long
if (faq.getId() != null) {
throw new BadRequestAlertException("A new faq cannot already have an ID", ENTITY_NAME, "idExists");
}
checkPriviledgeForAcceptedElseThrow(faq, courseId);
checkIsInstructorForAcceptedFaq(faq.getFaqState(), 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");
}
Expand All @@ -105,9 +105,9 @@ public ResponseEntity<FaqDTO> updateFaq(@RequestBody Faq faq, @PathVariable Long
if (faqId == null || !faqId.equals(faq.getId())) {
throw new BadRequestAlertException("Id of FAQ and path must match", ENTITY_NAME, "idNull");
}
checkPriviledgeForAcceptedElseThrow(faq, courseId);
checkIsInstructorForAcceptedFaq(faq.getFaqState(), courseId);
Faq existingFaq = faqRepository.findByIdElseThrow(faqId);
checkPriviledgeForAcceptedElseThrow(existingFaq, courseId);
checkIsInstructorForAcceptedFaq(faq.getFaqState(), courseId);
if (!Objects.equals(existingFaq.getCourse().getId(), courseId)) {
throw new BadRequestAlertException("Course ID of the FAQ provided courseID must match", ENTITY_NAME, "idNull");
}
Expand All @@ -116,19 +116,6 @@ 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 @@ -144,6 +131,7 @@ public ResponseEntity<FaqDTO> getFaq(@PathVariable Long faqId, @PathVariable Lon
if (faq.getCourse() == null || !faq.getCourse().getId().equals(courseId)) {
throw new BadRequestAlertException("Course ID in path and FAQ do not match", ENTITY_NAME, "courseIdMismatch");
}
checkShouldAccessNotAccepted(faq.getFaqState(), courseId);
FaqDTO dto = new FaqDTO(faq);
return ResponseEntity.ok(dto);
}
Expand Down Expand Up @@ -176,9 +164,10 @@ public ResponseEntity<Void> deleteFaq(@PathVariable Long faqId, @PathVariable Lo
*/
@GetMapping("courses/{courseId}/faqs")
@EnforceAtLeastStudentInCourse
public ResponseEntity<Set<FaqDTO>> getFaqForCourse(@PathVariable Long courseId) {
public ResponseEntity<Set<FaqDTO>> getFaqsForCourse(@PathVariable Long courseId) {
log.debug("REST request to get all Faqs for the course with id : {}", courseId);
Set<Faq> faqs = faqRepository.findAllByCourseId(courseId);
Set<Faq> faqs = authCheckService.isAtLeastTeachingAssistantInCourse(courseId) ? faqRepository.findAllByCourseId(courseId)
: faqRepository.findAllByCourseIdAndFaqState(courseId, FaqState.ACCEPTED);
Set<FaqDTO> faqDTOS = faqs.stream().map(FaqDTO::new).collect(Collectors.toSet());
return ResponseEntity.ok().body(faqDTOS);
}
Expand All @@ -194,6 +183,7 @@ public ResponseEntity<Set<FaqDTO>> getFaqForCourse(@PathVariable Long courseId)
@EnforceAtLeastStudentInCourse
public ResponseEntity<Set<FaqDTO>> getAllFaqsForCourseByStatus(@PathVariable Long courseId, @PathVariable FaqState faqState) {
log.debug("REST request to get all Faqs for the course with id : " + courseId + "and status " + faqState, courseId);
checkShouldAccessNotAccepted(faqState, courseId);
Set<Faq> faqs = faqRepository.findAllByCourseIdAndFaqState(courseId, faqState);
Set<FaqDTO> faqDTOS = faqs.stream().map(FaqDTO::new).collect(Collectors.toSet());
return ResponseEntity.ok().body(faqDTOS);
Expand All @@ -210,7 +200,39 @@ public ResponseEntity<Set<FaqDTO>> getAllFaqsForCourseByStatus(@PathVariable Lon
public ResponseEntity<Set<String>> getFaqCategoriesForCourse(@PathVariable Long courseId) {
log.debug("REST request to get all Faq Categories for the course with id : {}", courseId);
Set<String> faqs = faqRepository.findAllCategoriesByCourseId(courseId);
return ResponseEntity.ok().body(faqs);
}

@GetMapping("courses/{courseId}/faq-categories/{faqState}")
@EnforceAtLeastStudentInCourse
public ResponseEntity<Set<String>> getFaqCategoriesForCourseByState(@PathVariable Long courseId, @PathVariable FaqState faqState) {
log.debug("REST request to get all Faq Categories for the course with id : {} and the state {}", courseId, faqState);
checkShouldAccessNotAccepted(faqState, courseId);
Set<String> faqs = faqRepository.findAllCategoriesByCourseIdAndState(courseId, faqState);
return ResponseEntity.ok().body(faqs);
}

/**
* @param courseId the id of the course the faq belongs to
* @param role the required role of the user
* @throws AccessForbiddenException if the user does not have at least role
*
*/
private void checkRoleForCourse(Long courseId, Role role) {
Course course = courseRepository.findByIdElseThrow(courseId);
authCheckService.checkHasAtLeastRoleInCourseElseThrow(role, course, null);
}

private void checkShouldAccessNotAccepted(FaqState faqState, Long courseId) {
if (faqState != FaqState.ACCEPTED) {
checkRoleForCourse(courseId, Role.TEACHING_ASSISTANT);
}
}

private void checkIsInstructorForAcceptedFaq(FaqState faqState, Long courseId) {
if (faqState == FaqState.ACCEPTED) {
checkRoleForCourse(courseId, Role.INSTRUCTOR);
}
}

}
6 changes: 6 additions & 0 deletions src/main/webapp/app/faq/faq.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,10 @@ export class FaqService {
const faqText = `${faq.questionTitle ?? ''} ${faq.questionAnswer ?? ''}`.toLowerCase();
return tokens.every((token) => faqText.includes(token));
}

findAllCategoriesByCourseIdAndCategory(courseId: number, faqState: FaqState) {
return this.http.get<string[]>(`${this.resourceUrl}/${courseId}/faq-categories/${faqState}`, {
observe: 'response',
});
}
}
16 changes: 15 additions & 1 deletion src/main/webapp/app/faq/faq.utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,26 @@ import { AlertService } from 'app/core/util/alert.service';
import { Observable, catchError, map, of } from 'rxjs';
import { FaqService } from 'app/faq/faq.service';
import { FaqCategory } from 'app/entities/faq-category.model';
import { FaqState } from 'app/entities/faq.model';

export function loadCourseFaqCategories(courseId: number | undefined, alertService: AlertService, faqService: FaqService): Observable<FaqCategory[]> {
export function loadCourseFaqCategories(courseId: number | undefined, alertService: AlertService, faqService: FaqService, faqState?: FaqState): Observable<FaqCategory[]> {
if (courseId === undefined) {
return of([]);
}

if (faqState) {
return faqService.findAllCategoriesByCourseIdAndCategory(courseId, faqState).pipe(
map((categoryRes: HttpResponse<string[]>) => {
const existingCategories = faqService.convertFaqCategoriesAsStringFromServer(categoryRes.body || []);
return existingCategories;
}),
catchError((error: HttpErrorResponse) => {
onError(alertService, error);
return of([]);
}),
);
}

return faqService.findAllCategoriesByCourseId(courseId).pipe(
map((categoryRes: HttpResponse<string[]>) => {
const existingCategories = faqService.convertFaqCategoriesAsStringFromServer(categoryRes.body || []);
Expand Down
5 changes: 3 additions & 2 deletions src/main/webapp/app/overview/course-faq/course-faq.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export class CourseFaqComponent implements OnInit, OnDestroy {

courseId: number;
faqs: Faq[];
faqState = FaqState.ACCEPTED;

filteredFaqs: Faq[];
existingCategories: FaqCategory[];
Expand Down Expand Up @@ -64,15 +65,15 @@ export class CourseFaqComponent implements OnInit, OnDestroy {
}

private loadCourseExerciseCategories(courseId: number) {
loadCourseFaqCategories(courseId, this.alertService, this.faqService).subscribe((existingCategories) => {
loadCourseFaqCategories(courseId, this.alertService, this.faqService, this.faqState).subscribe((existingCategories) => {
this.existingCategories = existingCategories;
this.hasCategories = existingCategories.length > 0;
});
}

private loadFaqs() {
this.faqService
.findAllByCourseIdAndState(this.courseId, FaqState.ACCEPTED)
.findAllByCourseIdAndState(this.courseId, this.faqState)
.pipe(map((res: HttpResponse<Faq[]>) => res.body))
.subscribe({
next: (res: Faq[]) => {
Expand Down
31 changes: 29 additions & 2 deletions src/test/java/de/tum/cit/aet/artemis/communication/FaqIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,26 @@ void deleteFaq_IdsDoNotMatch_shouldNotDeleteFAQ() throws Exception {

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void getFaq_shouldGetFaqByCourseId() throws Exception {
Set<Faq> faqs = faqRepository.findAllByCourseIdAndFaqState(this.course1.getId(), FaqState.PROPOSED);
void getFaq_InstructorsShouldGetAllFaqByCourseId() throws Exception {
Set<Faq> faqs = faqRepository.findAllByCourseId(this.course1.getId());
Set<FaqDTO> returnedFaqs = request.get("/api/courses/" + course1.getId() + "/faqs", HttpStatus.OK, Set.class);
assertThat(returnedFaqs).hasSize(faqs.size());
}

@Test
@WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
void getFaq_StudentsShouldOnlyGetAcceptedFaqByCourseId() throws Exception {
Set<Faq> faqs = faqRepository.findAllByCourseIdAndFaqState(this.course1.getId(), FaqState.ACCEPTED);
Set<FaqDTO> returnedFaqs = request.get("/api/courses/" + course1.getId() + "/faqs", HttpStatus.OK, Set.class);
assertThat(returnedFaqs).hasSize(faqs.size());
assertThat(returnedFaqs).noneMatch(faq -> faq.faqState() == FaqState.PROPOSED);
assertThat(returnedFaqs).noneMatch(faq -> faq.faqState() == FaqState.REJECTED);
}

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void getFaq_ShouldGetFaqByCourseId() throws Exception {
Set<Faq> faqs = faqRepository.findAllByCourseId(this.course1.getId());
Set<FaqDTO> returnedFaqs = request.get("/api/courses/" + course1.getId() + "/faqs", HttpStatus.OK, Set.class);
assertThat(returnedFaqs).hasSize(faqs.size());
}
Expand All @@ -194,4 +212,13 @@ void getFaq_shouldGetFaqByCourseIdAndState() throws Exception {
assertThat(returnedFaqs).hasSize(faqs.size());
}

@Test
@WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
void getFaqs_StudentShouldOnlyGetAcceptedFaqByCourse() throws Exception {
Set<Faq> faqs = faqRepository.findAllByCourseIdAndFaqState(course1.getId(), FaqState.ACCEPTED);
Set<FaqDTO> returnedFaqs = request.get("/api/courses/" + course1.getId() + "/faqs", HttpStatus.OK, Set.class);
assertThat(returnedFaqs).hasSize(faqs.size());
assertThat(returnedFaqs.size()).isEqualTo(faqs.size());
}

}
2 changes: 1 addition & 1 deletion src/test/javascript/spec/component/overview/course-faq/course-faq.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ describe('CourseFaqs', () => {
it('should catch error if no categories are found', () => {
alertServiceStub = jest.spyOn(alertService, 'error');
const error = { status: 404 };
jest.spyOn(faqService, 'findAllCategoriesByCourseId').mockReturnValue(throwError(() => new HttpErrorResponse(error)));
jest.spyOn(faqService, 'findAllCategoriesByCourseIdAndCategory').mockReturnValue(throwError(() => new HttpErrorResponse(error)));
courseFaqComponentFixture.detectChanges();
expect(alertServiceStub).toHaveBeenCalledOnce();
});
Expand Down
21 changes: 21 additions & 0 deletions src/test/javascript/spec/service/faq.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,27 @@ describe('Faq Service', () => {
expect(expectedResult.body).toEqual(expected);
});

it('should find all categories by courseId and faqState', () => {
const category = {
color: '#6ae8ac',
category: 'category1',
} as FaqCategory;
const returnedFromService = { categories: [JSON.stringify(category)] };
const expected = { ...returnedFromService };
const courseId = 1;
const faqState = FaqState.ACCEPTED;
service
.findAllCategoriesByCourseIdAndCategory(courseId, faqState)
.pipe(take(1))
.subscribe((resp) => (expectedResult = resp));
const req = httpMock.expectOne({
url: `api/courses/${courseId}/faq-categories/${faqState}`,
method: 'GET',
});
req.flush(returnedFromService);
expect(expectedResult.body).toEqual(expected);
});

it('should set add active filter correctly', () => {
let activeFilters = new Set<string>();
activeFilters = service.toggleFilter('category1', activeFilters);
Expand Down

0 comments on commit a4a56db

Please sign in to comment.