Development: Faq service DTO migration
#11485
Conversation
github-actions
bot
added
tests
server
WalkthroughAdds CreateFaqDTO and UpdateFaqDTO on backend and frontend; updates REST create/update endpoints to accept DTOs and validate path-vs-body IDs/courseIds; frontend builds/transforms DTOs before service calls; tests updated to use DTO payloads and assertions. DTOs include mapping helpers and category stringification. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Frontend (Components)
participant FS as FaqService (FE)
participant API as FaqResource (BE)
participant Repo as Repository
rect rgb(235,245,255)
note over UI,FS: Create FAQ (DTO-based)
User->>UI: Submit new FAQ
UI->>FS: CreateFaqDTO.toCreateFaqDto(faq)
FS->>FS: convertCreateFaqFromClient(dto)\nstringify categories
FS->>API: POST /courses/{courseId}/faqs\nBody: CreateFaqDTO
API->>API: validate path vs dto.courseId\nmap dto to Faq, load Course
API->>Repo: persist Faq
Repo-->>API: saved Faq
API-->>FS: 201 Created (FaqDTO)
FS-->>UI: return FaqDTO
end
rect rgb(245,235,245)
note over UI,FS: Update FAQ (DTO-based)
User->>UI: Update existing FAQ
UI->>FS: UpdateFaqDTO.toUpdateDto(faq)
FS->>FS: convertUpdateFaqFromClient(dto)\nstringify categories
FS->>API: PUT /courses/{courseId}/faqs/{faqId}\nBody: UpdateFaqDTO
API->>API: validate path vs dto.id and dto.courseId\nmap dto to Faq, load Course
API->>Repo: update Faq
Repo-->>API: updated Faq
API-->>FS: 200 OK (FaqDTO)
FS-->>UI: return FaqDTO
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
WalkthroughIntroduces CreateFaqDTO and UpdateFaqDTO on backend and frontend. Refactors REST endpoints and Angular service/components to use DTOs for create/update. Adds conversion helpers and validation. Updates tests accordingly. Removes prior Faq-based client/server payload handling and related helpers. Response bodies standardized to FaqDTO in integration tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Angular Component
participant Svc as FaqService (client)
participant API as FaqResource (server)
participant Repo as Persistence
rect rgba(230,245,255,0.6)
note over UI,Svc: Create FAQ (DTO-based)
User->>UI: Submit create form
UI->>Svc: create(courseId, CreateFaqDTO)
Svc->>Svc: convertCreateFaqFromClient<br/>(stringify categories)
Svc->>API: POST /api/courses/{courseId}/faqs<br/>body: CreateFaqDTO
API->>API: validate @Valid DTO<br/>toEntity()
API->>Repo: save(Faq)
Repo-->>API: Faq saved
API-->>Svc: 201 Created + FaqDTO
Svc-->>UI: FaqDTO
end
rect rgba(240,255,230,0.6)
note over UI,Svc: Update FAQ (DTO-based)
User->>UI: Change state/title/answer
UI->>UI: UpdateFaqDTO.toUpdateDto(Faq)
UI->>Svc: update(courseId, UpdateFaqDTO)
Svc->>Svc: convertUpdateFaqFromClient<br/>(stringify categories)
Svc->>API: PUT /api/courses/{courseId}/faqs/{id}<br/>body: UpdateFaqDTO
API->>API: validate @Valid, check path/id match<br/>toEntity()
API->>Repo: save(Faq)
Repo-->>API: Faq updated
API-->>Svc: 200 OK + FaqDTO
Svc-->>UI: FaqDTO
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PMD (7.17.0)src/main/java/de/tum/cit/aet/artemis/communication/dto/UpdateFaqDTO.java[0.001s][warning][perf,memops] Cannot use file /tmp/hsperfdata_jailuser/1 because it is locked by another process (errno = 11) 59| ... [truncated 11539 characters] ... prone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185| src/test/java/de/tum/cit/aet/artemis/communication/FaqIntegrationTest.java[0.001s][warning][perf,memops] Cannot use file /tmp/hsperfdata_jailuser/1 because it is locked by another process (errno = 11) 59| ... [truncated 11539 characters] ... prone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185| src/main/java/de/tum/cit/aet/artemis/communication/dto/CreateFaqDTO.java[ERROR] Error at ruleset.xml:58:5 59| ... [truncated 11412 characters] ... prone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185|
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 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
github-actions
bot
added
the
iris
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/webapp/app/communication/faq/faq.component.spec.ts(3 hunks)src/main/webapp/app/communication/faq/faq.service.spec.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/app/communication/faq/faq.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/communication/faq/faq.service.spec.ts
🧬 Code graph analysis (1)
src/main/webapp/app/communication/faq/faq.service.spec.ts (1)
src/main/webapp/app/communication/shared/entities/faq.model.ts (3)
CreateFaqDTO(20-36)UpdateFaqDTO(38-58)Faq(11-18)
🔇 Additional comments (6)
src/main/webapp/app/communication/faq/faq.service.spec.ts (6)
10-10: LGTM!The imports and variable declarations are correct and follow the naming conventions.
Also applies to: 21-22
38-38: LGTM!The initialization of default test data and DTOs using static helper methods is correct.
Also applies to: 41-42
54-54: LGTM!The service method calls correctly use the new DTO parameters, aligning with the DTO migration.
Also applies to: 70-70
238-242: LGTM!The test correctly verifies conversion of stringified categories from the server into FaqCategory objects.
274-303: LGTM!These tests effectively verify immutability during DTO conversion and handle the edge case of undefined categories.
305-323: LGTM!The error handling tests comprehensively verify that the DTO conversion methods throw appropriate errors when required fields are missing.
| expect(() => CreateFaqDTO.toCreateFaqDto(f)).toThrow('The state should be present to create FAQ'); | ||
| }); | ||
|
|
||
| it('should throw if id is missing when converting to upDateFaqDto', () => { |
There was a problem hiding this comment.
Fix typo in test names.
The test names contain a typo: "upDateFaqDto" should be "UpdateFaqDTO" for consistency.
Apply this diff to fix the typo:
- it('should throw if id is missing when converting to upDateFaqDto', () => {
+ it('should throw if id is missing when converting to UpdateFaqDTO', () => {- it('should throw if faqState is missing when converting to upDateFaqDto', () => {
+ it('should throw if faqState is missing when converting to UpdateFaqDTO', () => {Also applies to: 318-318
🤖 Prompt for AI Agents
In src/main/webapp/app/communication/faq/faq.service.spec.ts around lines 311
and 318 the test names contain a typo: "upDateFaqDto" should be "UpdateFaqDTO";
update both it(...) descriptions to use the correct casing and spelling
"UpdateFaqDTO" to keep test names consistent and clear.
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||||||||
1 participant
Checklist
General
Server
Client
Description
Added new DTOs:
CreateFaqDTO for creating FAQs.
UpdateFaqDTO for updating existing FAQs.
Refactored FaqResource:
POST /api/communication/courses/{courseId}/faqs → consumes CreateFaqDTO, returns FaqDTO.
PUT /api/communication/courses/{courseId}/faqs/{faqId} → consumes UpdateFaqDTO, returns FaqDTO.
Steps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Summary by CodeRabbit