Hotfix/fix all bugs #218
Hotfix/fix all bugs #218
Conversation
htnghia1423
requested review from
bakaqc,
Copilot,
hardingadonis,
linhlm0210,
punhe and
yuhtnguyen
There was a problem hiding this comment.
Pull Request Overview
This hotfix addresses several payment handling issues and UI updates to improve user flow within courses. The changes include modifications to payment verification logic, middleware route protection adjustments, UI behavior changes in the course card (including login prompts and dynamic button behavior), and minor comment refinements.
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/custom/course/checkPayment.ts | Updates payment check logic with conditional redirection and a refined return value |
| src/services/api/course.ts | Removes redundant comments and clarifies course status checking |
| src/middlewares/clerk-middleware.ts | Removes the '/courses(.*)' route from the protected routes list, affecting course access protection |
| src/components/learners/courses/course/course-detail/CourseDetail.tsx | Deletes an outdated comment about redirecting learners on certain course statuses |
| src/components/learners/courses/course/course-detail/CourseCard.tsx | Introduces login requirement dialog, refines purchase flow logic, and adjusts button styling and text based on purchase status |
| src/app/(learners)/courses/[id]/page.tsx | Replaces a simple loading message with a dedicated Loading component |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (2)
src/middlewares/clerk-middleware.ts:9
- Removal of the '/courses(.*)' protected route may expose course routes to unauthorized access if it was not intentional. Please confirm that this change aligns with the desired security model.
'/courses(.*)',
src/components/learners/courses/course/course-detail/CourseCard.tsx:172
- The 'Add To Cart' button is rendered without an onClick handler, which may lead to a non-functional UI element. Please add the appropriate action or remove the button if it is not needed.
<button className="w-full bg-orange-50 text-orange-500 py-3 rounded-md mt-3 hover:bg-orange-100 transition-colors">
|
There was a problem hiding this comment.
Pull Request Overview
This PR applies several hotfixes focused on bug correction and UI flow improvements. Key changes include updating the payment check logic in custom course services, refining access control in middleware, and enhancing the user experience in course cards and course pages.
- Updated checkPaymentForCourse to handle cases with no payments and conditionally redirect based on an added parameter.
- Removed outdated comments and adjusted protected routes in middleware to fine-tune access control.
- Revised the CourseCard component to differentiate between purchasing and course access, and integrated a login dialog with a SignInButton.
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/custom/course/checkPayment.ts | Modified payment check logic with early return and conditional redirection. |
| src/services/api/course.ts | Removed an obsolete comment regarding course status checks. |
| src/middlewares/clerk-middleware.ts | Removed the '/courses(.*)' route from protected routes. |
| src/components/learners/courses/course/course-detail/CourseDetail.tsx | Removed outdated redirection comment for clarity. |
| src/components/learners/courses/course/course-detail/CourseCard.tsx | Updated purchase logic and UI with conditional redirection and a login dialog. |
| src/app/(learners)/courses/[id]/page.tsx | Replaced the loading text with a Loading component for better UX. |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (2)
src/middlewares/clerk-middleware.ts:9
- The removal of the '/courses(.*)' protected route might unintentionally expose course-related pages. Please verify that this change aligns with the intended access control strategy.
'/courses(.*)',
src/components/learners/courses/course/course-detail/CourseCard.tsx:105
- [nitpick] Consider renaming 'handleAction' to a more descriptive name such as 'handleCourseNavigation' to better communicate its purpose.
const handleAction = () => {
No description provided.