Skip to content

Feat/test ci cd#92

Closed
EricN2907 wants to merge 2 commits into
mainfrom
feat/test-ci-cd
Closed

Feat/test ci cd#92
EricN2907 wants to merge 2 commits into
mainfrom
feat/test-ci-cd

Conversation

@EricN2907

@EricN2907 EricN2907 commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Updates
    • Modified course creation and update endpoints to accept curriculum data in text format, replacing the previous structured format for improved flexibility in curriculum input.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Request contracts now accept curriculum as a JSON string instead of a strongly-typed list. Endpoint handlers deserialize that string into List<CurriculumSectionDto> and forward the parsed result to course commands.

Changes

Curriculum JSON Binding and Deserialization

Layer / File(s) Summary
Request contract types
backend/Src/PIED_LMS.Contract/Services/Course/Request.cs
CreateCourseRequest and UpdateCourseRequest change Curriculum from List<CurriculumSectionDto>? to string?, shifting the contract to accept curriculum as raw JSON text bound from form data.
Endpoint deserialization and command wiring
backend/Src/PIED_LMS.Presentation/APIs/CourseEndpoints.cs
CreateCourse and UpdateCourse each introduce conditional JSON deserialization logic that parses request.Curriculum (when non-blank) into List<CurriculumSectionDto> with case-insensitive matching, then pass the parsed curriculumList to the respective course commands.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • PiedTeam/pied-lms#89: Identical curriculum string-to-list binding changes across both Request contracts and CourseEndpoints deserialization.
  • PiedTeam/pied-lms#87: Refactors curriculum handling away from helper-based string processing toward strongly typed CurriculumSection lists, directly overlapping with the main PR's deserialization approach.
  • PiedTeam/pied-lms#77: Adds curriculum deserialization on retrieval that mirrors the main PR's new deserialization on submission, establishing symmetric JSON-to-typed-list conversion across both directions.

Poem

📋 A string arrives in form, so raw and plain,
The endpoint calls upon the JSON chain—
Deserialize, convert, and shape with care,
Strong types emerge, curriculum data fair.
From binding to command, the flow runs bright! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Feat/test ci cd' is vague and does not clearly describe the actual changes. The PR modifies course curriculum API contracts and request handling, but the title suggests CI/CD testing infrastructure changes. Revise the title to accurately reflect the core change, such as 'Change course curriculum API contract from List to string' or 'Update curriculum request handling to use JSON deserialization'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/test-ci-cd

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: 1

🤖 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.Presentation/APIs/CourseEndpoints.cs`:
- Around line 81-83: Wrap the inline deserialization of request.Curriculum in
both CreateCourse and UpdateCourse with a try/catch that specifically catches
System.Text.Json.JsonException; if caught, return a client error ServiceResponse
(e.g., a 400 BadRequest/validation response) instead of allowing the exception
to propagate to mediator.Send/global handler, so malformed JSON yields a
deterministic validation error. Ensure you reference the same JsonSerializer
call that deserializes into List<CurriculumSectionDto> and return the
ServiceResponse from the endpoint method when catching the exception.
🪄 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: ca285a25-db05-4ce5-9ded-358cf6ff693d

📥 Commits

Reviewing files that changed from the base of the PR and between d4f545c and 9610f6e.

⛔ Files ignored due to path filters (3)
  • backend/Src/PIED_LMS.Persistence/Migrations/20260602155608_UpdateCourseModel.Designer.cs is excluded by !**/*.Designer.cs, !**/Migrations/** and included by backend/Src/**
  • backend/Src/PIED_LMS.Persistence/Migrations/20260602155608_UpdateCourseModel.cs is excluded by !**/Migrations/** and included by backend/Src/**
  • backend/Src/PIED_LMS.Persistence/Migrations/PiedLmsDbContextModelSnapshot.cs is excluded by !**/Migrations/** and included by backend/Src/**
📒 Files selected for processing (2)
  • backend/Src/PIED_LMS.Contract/Services/Course/Request.cs
  • backend/Src/PIED_LMS.Presentation/APIs/CourseEndpoints.cs

Comment on lines +81 to +83
var curriculumList = string.IsNullOrWhiteSpace(request.Curriculum)
? null
: System.Text.Json.JsonSerializer.Deserialize<List<CurriculumSectionDto>>(request.Curriculum, new System.Text.Json.JsonSerializerOptions { PropertyNameCaseInsensitive = true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify whether the web pipeline explicitly maps JsonException to HTTP 400.
fd -e cs . backend/Src | xargs rg -n -C3 'JsonException|UseExceptionHandler|IExceptionHandler|ProblemDetails|StatusCodes\.Status400BadRequest'

Repository: PiedTeam/pied-lms

Length of output: 13015


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the exact code around the cited deserialization lines in CourseEndpoints.cs
sed -n '60,140p' backend/Src/PIED_LMS.Presentation/APIs/CourseEndpoints.cs

# Inspect GlobalExceptionHandler.cs for JsonException mapping (or lack thereof)
sed -n '1,220p' backend/Src/PIED_LMS.API/Middlewares/GlobalExceptionHandler.cs

Repository: PiedTeam/pied-lms

Length of output: 5476


Catch System.Text.Json.JsonException for request.Curriculum in both CreateCourse and UpdateCourse.

Both endpoints deserialize request.Curriculum inline (no try/catch), so malformed payloads throw before mediator.Send; GlobalExceptionHandler does not handle JsonException and falls back to 500, not a deterministic client validation response. Catch JsonException in CourseEndpoints.cs (lines 81-83 and 117-119) and return a client error ServiceResponse (e.g., 400 BadRequest / validation) instead of letting it escape to the global handler.

🤖 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 `@backend/Src/PIED_LMS.Presentation/APIs/CourseEndpoints.cs` around lines 81 -
83, Wrap the inline deserialization of request.Curriculum in both CreateCourse
and UpdateCourse with a try/catch that specifically catches
System.Text.Json.JsonException; if caught, return a client error ServiceResponse
(e.g., a 400 BadRequest/validation response) instead of allowing the exception
to propagate to mediator.Send/global handler, so malformed JSON yields a
deterministic validation error. Ensure you reference the same JsonSerializer
call that deserializes into List<CurriculumSectionDto> and return the
ServiceResponse from the endpoint method when catching the exception.

@EricN2907 EricN2907 closed this Jun 22, 2026
@EricN2907 EricN2907 deleted the feat/test-ci-cd branch June 22, 2026 05:46
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.

1 participant