General: Sort the FAQ posts by creation date
#11643
Conversation
github-actions
bot
added
server
WalkthroughThe changes systematically convert FAQ collections from unordered Sets to ordered Lists throughout the application stack, introduce a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🧰 Additional context used📓 Path-based instructions (1)src/main/java/**/*.java⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (14)📚 Learning: 2025-08-08T08:50:28.791ZApplied to files:
📚 Learning: 2025-09-20T16:43:32.823ZApplied to files:
📚 Learning: 2025-09-20T16:43:32.823ZApplied to files:
📚 Learning: 2025-09-20T16:47:54.380ZApplied to files:
📚 Learning: 2024-06-10T19:44:09.116ZApplied to files:
📚 Learning: 2025-09-05T09:12:34.098ZApplied to files:
📚 Learning: 2025-08-08T08:56:49.931ZApplied to files:
📚 Learning: 2025-09-05T15:13:32.171ZApplied to files:
📚 Learning: 2025-05-24T16:06:41.454ZApplied to files:
📚 Learning: 2025-09-18T20:55:38.115ZApplied to files:
📚 Learning: 2024-10-08T15:35:42.972ZApplied to files:
📚 Learning: 2025-09-20T16:47:54.380ZApplied to files:
📚 Learning: 2024-06-10T19:44:09.116ZApplied to files:
📚 Learning: 2025-09-20T16:45:43.715ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
🔇 Additional comments (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts (1)
23-30: AddcreatedDateto test FAQ objects for realistic test data.The
createFaqhelper should include thecreatedDateproperty to reflect the updated FAQ model. While the current unit test mocks the sort service, having test FAQs withcreatedDatevalues will make the tests more representative of production behavior and enable future integration tests to verify actual date-based sorting.Apply this diff to add
createdDateto the helper:function createFaq(id: number, category: string, color: string): Faq { const faq = new Faq(); faq.id = id; faq.questionTitle = 'questionTitle ' + id; faq.questionAnswer = 'questionAnswer ' + id; faq.categories = [new FaqCategory(category, color)]; + faq.createdDate = new Date(2025, 0, id); // Different dates for sorting tests return faq; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
🧠 Learnings (7)
📚 Learning: 2024-10-10T11:42:23.069Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9443
File: src/test/javascript/spec/component/hestia/git-diff-report/git-diff-modal.component.spec.ts:55-60
Timestamp: 2024-10-10T11:42:23.069Z
Learning: In `git-diff-report-modal.component.spec.ts`, using `fakeAsync` and `tick` does not work for handling asynchronous operations in the tests; alternative methods are needed.
Applied to files:
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
📚 Learning: 2025-08-21T17:30:20.530Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/drag-and-drop-question/drag-and-drop-question.component.spec.ts:34-34
Timestamp: 2025-08-21T17:30:20.530Z
Learning: FitTextDirective in src/main/webapp/app/quiz/shared/fit-text/fit-text.directive.ts is a standalone directive marked with standalone: true, so it should be imported in TestBed imports array, not declarations array.
Applied to files:
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
📚 Learning: 2024-10-13T12:03:02.430Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9463
File: src/test/javascript/spec/component/shared/monaco-editor/monaco-editor.service.spec.ts:50-55
Timestamp: 2024-10-13T12:03:02.430Z
Learning: In `src/test/javascript/spec/component/shared/monaco-editor/monaco-editor.service.spec.ts`, when a function is called multiple times in a test, use `toHaveBeenCalledTimes` and `toHaveBeenNthCalledWith` assertions instead of `toHaveBeenCalledExactlyOnceWith`.
Applied to files:
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
📚 Learning: 2025-09-01T10:20:40.706Z
Learnt from: Michael-Breu-UIbk
Repo: ls1intum/Artemis PR: 10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:132-149
Timestamp: 2025-09-01T10:20:40.706Z
Learning: In the Artemis codebase, Angular component test files for ProgrammingExerciseDetailComponent follow a pattern where the component is imported but not explicitly declared in TestBed.configureTestingModule(), yet TestBed.createComponent() still works successfully. This pattern is consistently used across test files like programming-exercise-detail.component.spec.ts and programming-exercise-detail.component.with-sharing.spec.ts.
Applied to files:
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
📚 Learning: 2024-10-20T22:00:52.335Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9505
File: src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts:179-181
Timestamp: 2024-10-20T22:00:52.335Z
Learning: In `src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts`, `ResizeObserver` is mocked within the `beforeEach` block.
Applied to files:
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
📚 Learning: 2025-09-01T13:47:02.624Z
Learnt from: Michael-Breu-UIbk
Repo: ls1intum/Artemis PR: 10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:167-169
Timestamp: 2025-09-01T13:47:02.624Z
Learning: In Jest tests, prefer using jest.spyOn() over direct method assignment for mocking service methods, as spies are automatically restored by Jest's cleanup mechanisms (jest.restoreAllMocks()), while direct assignment bypasses this system and can lead to test pollution where mocked methods affect subsequent tests.
Applied to files:
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
📚 Learning: 2024-07-09T19:09:34.276Z
Learnt from: florian-glombik
Repo: ls1intum/Artemis PR: 8858
File: src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts:91-96
Timestamp: 2024-07-09T19:09:34.276Z
Learning: For the PR ls1intum/Artemis#8858, avoid suggesting to change `expect(component.expandAll).toHaveBeenCalledOnce()` to `expect(component.expandAll).toHaveBeenCalledTimes(1)`.
Applied to files:
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: client-tests-selected
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
| it('should call sortService when sortFaqs is called', () => { | ||
| courseFaqComponent.filteredFaqs = [faq1, faq2, faq3]; | ||
| const sortByFunctionSpy = jest.spyOn(sortService, 'sortByFunction').mockReturnValue([faq3, faq2, faq1]); | ||
| courseFaqComponent.sortFaqs(); | ||
| expect(sortService.sortByProperty).toHaveBeenCalledOnce(); | ||
| expect(sortByFunctionSpy).toHaveBeenCalledOnce(); | ||
| expect(sortByFunctionSpy).toHaveBeenCalledWith([faq1, faq2, faq3], expect.any(Function), false); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Verify that filteredFaqs is updated with the sorted result.
The test verifies that sortByFunction is called but doesn't check whether filteredFaqs is updated with the sorted result. Add an assertion to ensure the component properly updates filteredFaqs after sorting.
Apply this diff to add the missing assertion:
it('should call sortService when sortFaqs is called', () => {
courseFaqComponent.filteredFaqs = [faq1, faq2, faq3];
const sortByFunctionSpy = jest.spyOn(sortService, 'sortByFunction').mockReturnValue([faq3, faq2, faq1]);
courseFaqComponent.sortFaqs();
expect(sortByFunctionSpy).toHaveBeenCalledOnce();
expect(sortByFunctionSpy).toHaveBeenCalledWith([faq1, faq2, faq3], expect.any(Function), false);
+ expect(courseFaqComponent.filteredFaqs).toEqual([faq3, faq2, faq1]);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should call sortService when sortFaqs is called', () => { | |
| courseFaqComponent.filteredFaqs = [faq1, faq2, faq3]; | |
| const sortByFunctionSpy = jest.spyOn(sortService, 'sortByFunction').mockReturnValue([faq3, faq2, faq1]); | |
| courseFaqComponent.sortFaqs(); | |
| expect(sortService.sortByProperty).toHaveBeenCalledOnce(); | |
| expect(sortByFunctionSpy).toHaveBeenCalledOnce(); | |
| expect(sortByFunctionSpy).toHaveBeenCalledWith([faq1, faq2, faq3], expect.any(Function), false); | |
| }); | |
| it('should call sortService when sortFaqs is called', () => { | |
| courseFaqComponent.filteredFaqs = [faq1, faq2, faq3]; | |
| const sortByFunctionSpy = jest.spyOn(sortService, 'sortByFunction').mockReturnValue([faq3, faq2, faq1]); | |
| courseFaqComponent.sortFaqs(); | |
| expect(sortByFunctionSpy).toHaveBeenCalledOnce(); | |
| expect(sortByFunctionSpy).toHaveBeenCalledWith([faq1, faq2, faq3], expect.any(Function), false); | |
| expect(courseFaqComponent.filteredFaqs).toEqual([faq3, faq2, faq1]); | |
| }); |
🤖 Prompt for AI Agents
In src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
around lines 157 to 163, the test checks that sortService.sortByFunction is
called but does not assert that courseFaqComponent.filteredFaqs was updated with
the returned sorted array; update the test to assert that after
courseFaqComponent.sortFaqs() completes, courseFaqComponent.filteredFaqs equals
the mocked sorted result (e.g., [faq3, faq2, faq1]) so the component's state is
validated in addition to the service call.
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
Anishyou
left a comment
Anishyou
left a comment
There was a problem hiding this comment.
code lgtm, I added a comment. I think test can be better but it is optional. Let me know
src/main/webapp/app/communication/course-faq/course-faq.component.spec.ts
Show resolved
Hide resolved
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
FullbusterSteve
left a comment
FullbusterSteve
left a comment
There was a problem hiding this comment.
I tested this PR on TS1.
It works as expected.
atharvamp
left a comment
atharvamp
left a comment
There was a problem hiding this comment.
Tested on ts1. Creation of FAQs maintains order across instructors and students. Searching through FAQs maintains order of creation in the search results and seems to use both FAQ title and FAQ description for searching.
| sortFaqs() { | ||
| this.sortService.sortByProperty(this.filteredFaqs, 'id', true); | ||
| // Sort by createdDate descending (newest first) | ||
| this.sortService.sortByFunction(this.filteredFaqs, (faq) => (faq.createdDate ? dayjs(faq.createdDate) : undefined), false); |
There was a problem hiding this comment.
Just curious, is there a reason you're filtering on the client side and not on the GET faq endpoint on the server? Like this we'd need to implement sorting on both mobile apps too.
There was a problem hiding this comment.
You're right! I initially thought I could simply modify the existing client-side sorting logic (which was sorting by ID) to sort by creation date instead. I've now moved the sorting mechanism to the server side as you suggested. I implemented it at the repository layer. This way, the sorting happens at the database level, which is more efficient in terms of CPU and memory usage.
anian03
left a comment
anian03
left a comment
There was a problem hiding this comment.
Nice implementation in general, but please consider if it makes sense to move the sorting logic to the server. We could already sort the results in the database query, and it would eliminate the need to implement this logic in both mobile app clients too
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
There was a problem hiding this comment.
I tested the PR related to the FAQ. I followed every step as mentioned.
Result:
I created some FAQ, and they were listed as expected like in order. I controlled from a student account and confirmed. Additionally, by using the search bar, I could manage to filter based on typing; however, the filtering feature has not been active yet.
Test Account:
Role: Instructor | Student
Account: artemis_test_user_17 | artemis_test_user_5
ScreenShots:
…reation-date-newest-first
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
…reation-date-newest-first
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
…reation-date-newest-first
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
HawKhiem
dismissed stale reviews from kaleliguray, atharvamp, and FullbusterSteve
via
475a742
github-actions
bot
added
tests
core
…reation-date-newest-first
HawKhiem
requested review from
Anishyou,
FullbusterSteve,
anian03,
atharvamp and
kaleliguray
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
…reation-date-newest-first
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
Checklist
General
Server
Client
Motivation and Context
Students were unable to easily discover newly added FAQ posts in the course FAQ view because posts were displayed in random order (sorted by ID). This made it difficult for students to find recent announcements or answers to new questions. This PR closes issue #9883
Description
This PR implements chronological sorting for FAQ posts in the course FAQ view, displaying the newest posts at the top of the list.
Changes:
createdDatefield to FaqDTO to include creation timestamps in API responsescreatedDateproperty to the Faq modelcreatedDatein descending order (newest first) using dayjs for proper date comparisonNote: This change only affects the student-facing course FAQ view (
jhi-course-faq). The instructor FAQ management page remains unchanged.Steps for Testing
Prerequisites:
Review Progress
Performance Review
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.