Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;

import java.util.List;
import java.util.Set;

import org.springframework.context.annotation.Lazy;
Expand All @@ -24,7 +25,7 @@
@Repository
public interface FaqRepository extends ArtemisJpaRepository<Faq, Long> {

Set<Faq> findAllByCourseId(Long courseId);
List<Faq> findAllByCourseIdOrderByCreatedDateDesc(Long courseId);

@Query("""
SELECT DISTINCT faq.categories
Expand All @@ -40,7 +41,7 @@ public interface FaqRepository extends ArtemisJpaRepository<Faq, Long> {
""")
Set<String> findAllCategoriesByCourseIdAndState(@Param("courseId") Long courseId, @Param("faqState") FaqState faqState);

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

@Transactional // ok because of delete
@Modifying
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void ingestFaqsIntoPyris(Long courseId, Optional<Long> faqId) {
throw new IllegalArgumentException("Faq is not in the state accepted, you cannot ingest this faq");
}
pyrisFaqApi.get().addFaq(faq);
}, () -> faqRepository.findAllByCourseIdAndFaqState(courseId, FaqState.ACCEPTED).forEach(faq -> pyrisFaqApi.get().addFaq(faq)));
}, () -> faqRepository.findAllByCourseIdAndFaqStateOrderByCreatedDateDesc(courseId, FaqState.ACCEPTED).forEach(faq -> pyrisFaqApi.get().addFaq(faq)));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@

import java.net.URI;
import java.net.URISyntaxException;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import jakarta.validation.Valid;

Expand Down Expand Up @@ -186,11 +186,11 @@ public ResponseEntity<Void> deleteFaq(@PathVariable Long faqId, @PathVariable Lo
*/
@GetMapping("courses/{courseId}/faqs")
@EnforceAtLeastStudentInCourse
public ResponseEntity<Set<FaqDTO>> getFaqsForCourse(@PathVariable Long courseId) {
public ResponseEntity<List<FaqDTO>> getFaqsForCourse(@PathVariable Long courseId) {
log.debug("REST request to get all Faqs for the course with id : {}", 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());
List<Faq> faqs = authCheckService.isAtLeastTeachingAssistantInCourse(courseId) ? faqRepository.findAllByCourseIdOrderByCreatedDateDesc(courseId)
: faqRepository.findAllByCourseIdAndFaqStateOrderByCreatedDateDesc(courseId, FaqState.ACCEPTED);
List<FaqDTO> faqDTOS = faqs.stream().map(FaqDTO::new).toList();
return ResponseEntity.ok().body(faqDTOS);
}

Expand All @@ -203,11 +203,11 @@ public ResponseEntity<Set<FaqDTO>> getFaqsForCourse(@PathVariable Long courseId)
*/
@GetMapping("courses/{courseId}/faq-state/{faqState}")
@EnforceAtLeastStudentInCourse
public ResponseEntity<Set<FaqDTO>> getAllFaqsForCourseByStatus(@PathVariable Long courseId, @PathVariable FaqState faqState) {
public ResponseEntity<List<FaqDTO>> getAllFaqsForCourseByStatus(@PathVariable Long courseId, @PathVariable FaqState faqState) {
log.debug("REST request to get all Faqs for the course with id {} and status {}", courseId, faqState);
checkShouldAccessNotAccepted(faqState, courseId);
Set<Faq> faqs = faqRepository.findAllByCourseIdAndFaqState(courseId, faqState);
Set<FaqDTO> faqDTOS = faqs.stream().map(FaqDTO::new).collect(Collectors.toSet());
List<Faq> faqs = faqRepository.findAllByCourseIdAndFaqStateOrderByCreatedDateDesc(courseId, faqState);
List<FaqDTO> faqDTOS = faqs.stream().map(FaqDTO::new).toList();
return ResponseEntity.ok().body(faqDTOS);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
import static de.tum.cit.aet.artemis.core.config.Constants.SHORT_NAME_PATTERN;

import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Matcher;

Expand Down Expand Up @@ -232,8 +234,9 @@ public class Course extends DomainObject {
private TutorialGroupsConfiguration tutorialGroupsConfiguration;

@OneToMany(mappedBy = "course", cascade = CascadeType.REMOVE, orphanRemoval = true, fetch = FetchType.LAZY)
@OrderBy("createdDate DESC")
@JsonIgnoreProperties(value = "course", allowSetters = true)
private Set<Faq> faqs = new HashSet<>();
private List<Faq> faqs = new ArrayList<>();

// NOTE: Helpers variable names must be different from Getter name, so that Jackson ignores the @Transient annotation, but Hibernate still respects it
@Transient
Expand Down Expand Up @@ -1003,11 +1006,11 @@ public void setCourseInformationSharingMessagingCodeOfConduct(String courseInfor
this.courseInformationSharingMessagingCodeOfConduct = courseInformationSharingMessagingCodeOfConduct;
}

public Set<Faq> getFaqs() {
public List<Faq> getFaqs() {
return faqs;
}

public void setFaqs(Set<Faq> faqs) {
public void setFaqs(List<Faq> faqs) {
this.faqs = faqs;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public Course findOneWithExercisesAndLecturesAndExamsAndCompetenciesAndTutorialG
course.setNumberOfTutorialGroups(0L);
}
if (course.isFaqEnabled()) {
course.setFaqs(faqRepository.findAllByCourseIdAndFaqState(courseId, FaqState.ACCEPTED));
course.setFaqs(faqRepository.findAllByCourseIdAndFaqStateOrderByCreatedDateDesc(courseId, FaqState.ACCEPTED));
}
if (authCheckService.isOnlyStudentInCourse(course, user) && examRepositoryApi.isPresent()) {
var examRepoApi = examRepositoryApi.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { CourseFaqAccordionComponent } from 'app/communication/course-faq/course
import { Faq, FaqState } from 'app/communication/shared/entities/faq.model';
import { FaqCategory } from 'app/communication/shared/entities/faq-category.model';
import { SearchFilterComponent } from 'app/shared/search-filter/search-filter.component';
import { SortService } from 'app/shared/service/sort.service';
import { ElementRef, signal } from '@angular/core';
import { CustomExerciseCategoryBadgeComponent } from 'app/exercise/exercise-categories/custom-exercise-category-badge/custom-exercise-category-badge.component';

Expand All @@ -36,7 +35,6 @@ describe('CourseFaqs', () => {
let faqService: FaqService;
let alertServiceStub: jest.SpyInstance;
let alertService: AlertService;
let sortService: SortService;

let faq1: Faq;
let faq2: Faq;
Expand Down Expand Up @@ -100,7 +98,6 @@ describe('CourseFaqs', () => {

faqService = TestBed.inject(FaqService);
alertService = TestBed.inject(AlertService);
sortService = TestBed.inject(SortService);
});
});

Expand Down Expand Up @@ -154,10 +151,28 @@ describe('CourseFaqs', () => {
expect(alertServiceStub).toHaveBeenCalledOnce();
});

it('should call sortService when sortRows is called', () => {
jest.spyOn(sortService, 'sortByProperty').mockReturnValue([]);
courseFaqComponent.sortFaqs();
expect(sortService.sortByProperty).toHaveBeenCalledOnce();
it('should display FAQs in the order received from backend (server-side sorting)', () => {
// Mock service to return FAQs in descending order by creation date (newest first)
const newestFaq = createFaq(3, 'category3', '#0ab84f');
const middleFaq = createFaq(2, 'category2', '#0ab84f');
const oldestFaq = createFaq(1, 'category1', '#94a11c');

jest.spyOn(faqService, 'findAllByCourseIdAndState').mockReturnValue(
of(
new HttpResponse({
body: [newestFaq, middleFaq, oldestFaq], // Backend returns sorted
status: 200,
}),
),
);

courseFaqComponentFixture.detectChanges();

// Verify component displays FAQs in the exact order received from backend
expect(courseFaqComponent.faqs).toHaveLength(3);
expect(courseFaqComponent.faqs[0].id).toBe(3); // Newest first
expect(courseFaqComponent.faqs[1].id).toBe(2);
expect(courseFaqComponent.faqs[2].id).toBe(1); // Oldest last
});

it('should scroll and focus on the faq element with given id', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { FaqCategory } from 'app/communication/shared/entities/faq-category.mode
import { loadCourseFaqCategories } from 'app/communication/faq/faq.utils';
import { onError } from 'app/shared/util/global.utils';
import { SearchFilterComponent } from 'app/shared/search-filter/search-filter.component';
import { SortService } from 'app/shared/service/sort.service';
import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
import { TranslateDirective } from 'app/shared/language/translate.directive';
import { FontAwesomeModule } from '@fortawesome/angular-fontawesome';
Expand Down Expand Up @@ -56,7 +55,6 @@ export class CourseFaqComponent implements OnInit, OnDestroy {

private faqService = inject(FaqService);
private alertService = inject(AlertService);
private sortService = inject(SortService);
private renderer = inject(Renderer2);

constructor() {
Expand Down Expand Up @@ -98,7 +96,6 @@ export class CourseFaqComponent implements OnInit, OnDestroy {
next: (res: Faq[]) => {
this.faqs = res;
this.applyFilters();
this.sortFaqs();
},
error: (res: HttpErrorResponse) => onError(this.alertService, res),
});
Expand Down Expand Up @@ -135,10 +132,6 @@ export class CourseFaqComponent implements OnInit, OnDestroy {
this.applySearch(searchTerm);
}

sortFaqs() {
this.sortService.sortByProperty(this.filteredFaqs, 'id', true);
}

scrollToFaq(faqId: number): void {
const faqElement = this.faqElements().find((faq) => faq.nativeElement.id === 'faq-' + String(faqId));
if (faqElement) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export class Faq implements BaseEntity {
public faqState?: FaqState;
public course?: Course;
public categories?: FaqCategory[];
public createdDate?: Date;
}

export class CreateFaqDTO {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,16 +195,16 @@ void deleteFaq_IdsDoNotMatch_shouldNotDeleteFAQ() throws Exception {
@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void getFaq_InstructorsShouldGetAllFaqByCourseId() throws Exception {
Set<Faq> faqs = faqRepository.findAllByCourseId(this.course1.getId());
Set<FaqDTO> returnedFaqs = request.get("/api/communication/courses/" + course1.getId() + "/faqs", HttpStatus.OK, Set.class);
List<Faq> faqs = faqRepository.findAllByCourseIdOrderByCreatedDateDesc(this.course1.getId());
List<FaqDTO> returnedFaqs = request.getList("/api/communication/courses/" + course1.getId() + "/faqs", HttpStatus.OK, FaqDTO.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/communication/courses/" + course1.getId() + "/faqs", HttpStatus.OK, Set.class);
List<Faq> faqs = faqRepository.findAllByCourseIdAndFaqStateOrderByCreatedDateDesc(this.course1.getId(), FaqState.ACCEPTED);
List<FaqDTO> returnedFaqs = request.getList("/api/communication/courses/" + course1.getId() + "/faqs", HttpStatus.OK, FaqDTO.class);
assertThat(returnedFaqs).hasSize(faqs.size());
assertThat(returnedFaqs).noneMatch(faq -> faq.faqState() == FaqState.PROPOSED);
assertThat(returnedFaqs).noneMatch(faq -> faq.faqState() == FaqState.REJECTED);
Expand All @@ -213,24 +213,24 @@ void getFaq_StudentsShouldOnlyGetAcceptedFaqByCourseId() throws Exception {
@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/communication/courses/" + course1.getId() + "/faqs", HttpStatus.OK, Set.class);
List<Faq> faqs = faqRepository.findAllByCourseIdOrderByCreatedDateDesc(this.course1.getId());
List<FaqDTO> returnedFaqs = request.getList("/api/communication/courses/" + course1.getId() + "/faqs", HttpStatus.OK, FaqDTO.class);
assertThat(returnedFaqs).hasSize(faqs.size());
}

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void getFaq_shouldGetFaqByCourseIdAndState() throws Exception {
Set<Faq> faqs = faqRepository.findAllByCourseIdAndFaqState(this.course1.getId(), FaqState.PROPOSED);
Set<FaqDTO> returnedFaqs = request.get("/api/communication/courses/" + course1.getId() + "/faq-state/" + "PROPOSED", HttpStatus.OK, Set.class);
List<Faq> faqs = faqRepository.findAllByCourseIdAndFaqStateOrderByCreatedDateDesc(this.course1.getId(), FaqState.PROPOSED);
List<FaqDTO> returnedFaqs = request.getList("/api/communication/courses/" + course1.getId() + "/faq-state/" + "PROPOSED", HttpStatus.OK, FaqDTO.class);
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/communication/courses/" + course1.getId() + "/faqs", HttpStatus.OK, Set.class);
List<Faq> faqs = faqRepository.findAllByCourseIdAndFaqStateOrderByCreatedDateDesc(course1.getId(), FaqState.ACCEPTED);
List<FaqDTO> returnedFaqs = request.getList("/api/communication/courses/" + course1.getId() + "/faqs", HttpStatus.OK, FaqDTO.class);
assertThat(returnedFaqs).hasSize(faqs.size());
assertThat(returnedFaqs.size()).isEqualTo(faqs.size());
}
Expand All @@ -244,6 +244,31 @@ void testEnableFaq() throws Exception {
assertThat(updatedCourse.isFaqEnabled()).isTrue();
}

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void getFaqs_shouldReturnFaqsSortedByCreationDateDescending() throws Exception {
Faq faq1 = FaqFactory.generateFaq(course1, FaqState.ACCEPTED, "SortTest1", "answer1");
faqRepository.save(faq1);
Thread.sleep(10);

Faq faq2 = FaqFactory.generateFaq(course1, FaqState.ACCEPTED, "SortTest2", "answer2");
faqRepository.save(faq2);
Thread.sleep(10);

Faq faq3 = FaqFactory.generateFaq(course1, FaqState.ACCEPTED, "SortTest3", "answer3");
faqRepository.save(faq3);

List<FaqDTO> returnedFaqs = request.getList("/api/communication/courses/" + course1.getId() + "/faqs", HttpStatus.OK, FaqDTO.class);

List<FaqDTO> createdFaqs = returnedFaqs.stream().filter(faq -> faq.questionTitle().startsWith("SortTest")).toList();

// Verify they are sorted by creation date descending (newest first)
assertThat(createdFaqs).hasSize(3);
assertThat(createdFaqs.get(0).questionTitle()).isEqualTo("SortTest3"); // Newest
assertThat(createdFaqs.get(1).questionTitle()).isEqualTo("SortTest2");
assertThat(createdFaqs.get(2).questionTitle()).isEqualTo("SortTest1"); // Oldest
}

private void disableFaq(Course course) {
course = courseRepository.findByIdElseThrow(course.getId());
course.setFaqEnabled(false);
Expand Down
Loading