Skip to content

fix: added prop for loginResponse, response CRUD Course curiculum and…#85

Merged
nnh53 merged 3 commits into
mainfrom
fix-responseLogin-add-prop-course
May 25, 2026
Merged

fix: added prop for loginResponse, response CRUD Course curiculum and…#85
nnh53 merged 3 commits into
mainfrom
fix-responseLogin-add-prop-course

Conversation

@nnh53

@nnh53 nnh53 commented May 25, 2026

Copy link
Copy Markdown
Member

… insigint

Summary by CodeRabbit

Release Notes

  • New Features
    • Courses now support curriculum structure and insight descriptions for enhanced content organization and management
    • User accounts now include profile picture display across the application

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@nnh53, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 1 review of capacity. Refill in 37 minutes and 57 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ca81950a-4759-447b-843b-c737cf2c7c4d

📥 Commits

Reviewing files that changed from the base of the PR and between bb6f2a8 and e430e5f.

📒 Files selected for processing (2)
  • backend/Src/PIED_LMS.Application/UserCases/CourseContentHelper.cs
  • backend/Src/PIED_LMS.Contract/Services/Course/Request.cs
📝 Walkthrough

Walkthrough

The PR introduces profile picture URL resolution in the login flow and adds comprehensive curriculum and insight validation across course creation, updates, and retrieval. The backend login handler now resolves user profile picture URLs via a file storage service dependency, returning the URL in LoginResponse with graceful fallback to null on failure. Frontend types and services are updated to propagate the avatar field. Course handlers validate and serialize curriculum/insight using a centralized helper, normalizing behavior across create, update, and query operations.

Changes

Profile Picture URL Resolution in Login

Layer / File(s) Summary
Backend LoginHandler and LoginResponse contract
backend/Src/PIED_LMS.Application/UserCases/Commands/Auth/LoginHandler.cs, backend/Src/PIED_LMS.Contract/Services/Identity/Response.cs
LoginHandler receives IFileStorageService dependency and resolves ProfilePictureUrl via GetFileUrlAsync, catching exceptions and defaulting to null. LoginResponse DTO adds optional ProfilePictureUrl field.
Frontend avatar type definitions and login service integration
frontend/interface/auth/auth.interface.ts, frontend/interface/store/auth.types.ts, frontend/services/auth/auth.service.ts
User and RegisterResponse interfaces gain optional avatar field; useLogin maps avatar from response into auth state.

Course Curriculum and Insight Management

Layer / File(s) Summary
Course content contracts
backend/Src/PIED_LMS.Contract/Services/Course/Command.cs, backend/Src/PIED_LMS.Contract/Services/Course/Request.cs, backend/Src/PIED_LMS.Contract/Services/Course/Response.cs
CreateCourseCommand and UpdateCourseCommand add optional Curriculum and Insight parameters; CreateCourseRequest and UpdateCourseRequest add form-bound curriculum and insight fields; CourseDto adds Curriculum (list of sections) and Insight response fields.
CreateCourseHandler validation
backend/Src/PIED_LMS.Application/UserCases/Commands/Course/CreateCourseHandler.cs
Handler validates curriculum and insight via CourseContentHelper helpers, rejecting with warning logs on failure; validated outputs are assigned to Course entity fields.
UpdateCourseHandler validation and persistence
backend/Src/PIED_LMS.Application/UserCases/Commands/Course/UpdateCourseHandler.cs
Handler validates curriculum and insight with helpers, short-circuits on failure; tracks changes in audit logs; persists validated values to course entity before commit.
Query handler curriculum deserialization
backend/Src/PIED_LMS.Application/UserCases/Queries/Course/GetCourseByIdHandler.cs, backend/Src/PIED_LMS.Application/UserCases/Queries/Course/GetCourseCurriculumHandler.cs, backend/Src/PIED_LMS.Application/UserCases/Queries/Course/GetCoursesHandler.cs
Handlers delegate curriculum deserialization to CourseContentHelper instead of inline JSON handling; responses include deserialized curriculum and non-null insight (defaulted to empty string).
CourseEndpoints command wiring
backend/Src/PIED_LMS.Presentation/APIs/CourseEndpoints.cs
CreateCourse and UpdateCourse endpoints pass curriculum and insight from requests into command constructors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PiedTeam/pied-lms#84: Both PRs extend profile-picture URL resolution via IFileStorageService in authentication handlers, with the referenced PR adding the same pattern to UpdateProfileHandler.
  • PiedTeam/pied-lms#77: Both PRs modify course curriculum handling by replacing inline JSON deserialization with centralized CourseContentHelper in GetCourseCurriculumHandler and related query handlers.

Poem

🎓 Profiles now picture themselves resolved and true,
Curriculum and insight find their proper due,
Helpers centralize the chaos, keeping code quite tight—
From login to course creation, each new feature shines bright. ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title is incomplete and incomprehensible; it cuts off mid-sentence with an ellipsis and contains typos ('curiculum' instead of 'curriculum', 'insigint' instead of 'insight'). Rewrite the title as a complete, coherent sentence that accurately describes the main changes. For example: 'feat: add curriculum and insight fields to course CRUD operations and profile picture URL to login response'.
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-responseLogin-add-prop-course

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/interface/auth/auth.interface.ts (1)

26-33: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align login response field name with the backend contract.

Line 32 uses avatar, but the backend response field is ProfilePictureUrl (JSON profilePictureUrl). This mismatch will drop the profile picture on login.

Proposed fix
// frontend/interface/auth/auth.interface.ts
 export interface LoginResponse {
   accessToken: string;
   refreshToken?: string;
   email: string;
   firstName: string;
   lastName: string;
-  avatar?: string | null;
+  profilePictureUrl?: string | null;
 }

// frontend/services/auth/auth.service.ts
       const user: User = {
         uuid,
         email,
         fullName: `${firstName} ${lastName}`.trim() || null,
         role,
-        avatar: data.avatar ?? null,
+        avatar: data.profilePictureUrl ?? null,
       };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/interface/auth/auth.interface.ts` around lines 26 - 33, The
LoginResponse interface currently defines an avatar field that doesn't match the
backend JSON; replace avatar with profilePictureUrl (matching backend's
ProfilePictureUrl / profilePictureUrl) and keep the same optional string | null
type to preserve nullable behavior; update the LoginResponse interface
declaration and any code referencing LoginResponse.avatar to use
LoginResponse.profilePictureUrl (or map the backend field into that property
where responses are parsed).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@backend/Src/PIED_LMS.Application/UserCases/Commands/Course/CreateCourseHandler.cs`:
- Line 4: CreateCourseHandler references CourseContentHelper (used around where
CreateCourseHandler constructs course content) but the type is missing causing
CS0103; restore or add the CourseContentHelper class in the appropriate
namespace and then import that namespace with a using, or fully qualify the type
where used in CreateCourseHandler; ensure the class name and public API
(methods/properties) match the usages in CreateCourseHandler so the references
compile.

In `@backend/Src/PIED_LMS.Contract/Services/Course/Request.cs`:
- Around line 21-22: The request contract declares "insight" as nullable but
handlers expect it non-null; update the request model to make insight required
by changing the parameter/property from [FromForm(Name = "insight")] string?
Insight to a non-nullable string and/or add a validation attribute (e.g.,
[Required]) on the Insight member in the Request class/record, and ensure
create/update handlers that reference Insight (the create/update handler
methods) keep their current validation logic unchanged so the contract and
runtime behavior are aligned.

---

Outside diff comments:
In `@frontend/interface/auth/auth.interface.ts`:
- Around line 26-33: The LoginResponse interface currently defines an avatar
field that doesn't match the backend JSON; replace avatar with profilePictureUrl
(matching backend's ProfilePictureUrl / profilePictureUrl) and keep the same
optional string | null type to preserve nullable behavior; update the
LoginResponse interface declaration and any code referencing
LoginResponse.avatar to use LoginResponse.profilePictureUrl (or map the backend
field into that property where responses are parsed).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 59716f36-b36f-4fbe-886c-e06c2f385255

📥 Commits

Reviewing files that changed from the base of the PR and between 8c04ed1 and bb6f2a8.

📒 Files selected for processing (14)
  • backend/Src/PIED_LMS.Application/UserCases/Commands/Auth/LoginHandler.cs
  • backend/Src/PIED_LMS.Application/UserCases/Commands/Course/CreateCourseHandler.cs
  • backend/Src/PIED_LMS.Application/UserCases/Commands/Course/UpdateCourseHandler.cs
  • backend/Src/PIED_LMS.Application/UserCases/Queries/Course/GetCourseByIdHandler.cs
  • backend/Src/PIED_LMS.Application/UserCases/Queries/Course/GetCourseCurriculumHandler.cs
  • backend/Src/PIED_LMS.Application/UserCases/Queries/Course/GetCoursesHandler.cs
  • backend/Src/PIED_LMS.Contract/Services/Course/Command.cs
  • backend/Src/PIED_LMS.Contract/Services/Course/Request.cs
  • backend/Src/PIED_LMS.Contract/Services/Course/Response.cs
  • backend/Src/PIED_LMS.Contract/Services/Identity/Response.cs
  • backend/Src/PIED_LMS.Presentation/APIs/CourseEndpoints.cs
  • frontend/interface/auth/auth.interface.ts
  • frontend/interface/store/auth.types.ts
  • frontend/services/auth/auth.service.ts

Comment thread backend/Src/PIED_LMS.Contract/Services/Course/Request.cs Outdated
Comment thread backend/Src/PIED_LMS.Application/UserCases/Commands/Auth/LoginHandler.cs Dismissed
@nnh53 nnh53 merged commit bfc1a6f into main May 25, 2026
7 checks passed
@nnh53 nnh53 deleted the fix-responseLogin-add-prop-course branch May 25, 2026 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants