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: Only show accepted categories of accepted FAQs #9591

Merged
merged 12 commits into from
Nov 2, 2024
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);

}
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);
checkIsInstructor(faq.getFaqState(), courseId);
cremertim marked this conversation as resolved.
Show resolved Hide resolved
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);
checkIsInstructor(faq.getFaqState(), courseId);
Faq existingFaq = faqRepository.findByIdElseThrow(faqId);
checkPriviledgeForAcceptedElseThrow(existingFaq, courseId);
checkIsInstructor(faq.getFaqState(), courseId);
cremertim 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 @@ -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);
cremertim marked this conversation as resolved.
Show resolved Hide resolved
cremertim marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}
cremertim marked this conversation as resolved.
Show resolved Hide resolved

private void checkIsInstructor(FaqState faqState, Long courseId) {
cremertim marked this conversation as resolved.
Show resolved Hide resolved
if (faqState == FaqState.ACCEPTED) {
checkRoleForCourse(courseId, Role.INSTRUCTOR);
}
}
cremertim marked this conversation as resolved.
Show resolved Hide resolved

}
6 changes: 6 additions & 0 deletions src/main/webapp/app/faq/faq.service.ts
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
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([]);
}),
);
}
cremertim marked this conversation as resolved.
Show resolved Hide resolved

return faqService.findAllCategoriesByCourseId(courseId).pipe(
map((categoryRes: HttpResponse<string[]>) => {
const existingCategories = faqService.convertFaqCategoriesAsStringFromServer(categoryRes.body || []);
Expand Down
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;
});
}
cremertim marked this conversation as resolved.
Show resolved Hide resolved

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
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());
}
cremertim marked this conversation as resolved.
Show resolved Hide resolved

@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);
cremertim marked this conversation as resolved.
Show resolved Hide resolved
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());
}
cremertim marked this conversation as resolved.
Show resolved Hide resolved

}
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();
});
cremertim marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
21 changes: 21 additions & 0 deletions src/test/javascript/spec/service/faq.service.spec.ts
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)
cremertim marked this conversation as resolved.
Show resolved Hide resolved
.pipe(take(1))
.subscribe((resp) => (expectedResult = resp));
cremertim marked this conversation as resolved.
Show resolved Hide resolved
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
Loading