Skip to content

Conversation

@giangnht19
Copy link

@giangnht19 giangnht19 commented Aug 17, 2025

Description

This PR introduces two main enhancements to the codebase:
1. Mark Unit as Complete (related to this PR: #386)

  • Added functionality for users to mark a unit as complete.
  • Remove ability to move unit from study period container when marked as complete.

2. Skill Summary Component

  • Added a new component that displays a summary of skills (Description).
  • The summary updates automatically based on the completion status of the chosen unit.

Fixes # (issue)
Refactor Codebase: #385
Fetch this PR: thoth-tech/doubtfire-api#71
Migrate and use this command (rake db:test_data:create_frontend_data) to populate the database
If required units are not populated, use the elective units instead (make sure you are using admin account)

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

image image

Tests Performed
1. Unit Completion Toggle

  • Unit status updated (unit displayed as complete).
  • Completed unit cannot be remove from study periods
  • Skill summary component updated accordingly.

2. Skill Summary Component

  • Verified that only completed units contribute to the skill summary.
  • Checked that summary updates dynamically when toggling completion state.

Testing Checklist:

  • Tested in latest Chrome
  • Tested in latest Safari
  • Tested in latest Firefox

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review from @macite and @jakerenzella on the Pull Request

@giangnht19 giangnht19 closed this Aug 17, 2025
@giangnht19 giangnht19 reopened this Aug 17, 2025
Copy link

@SahiruWithanage SahiruWithanage left a comment

Choose a reason for hiding this comment

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

Great job on this PR, the components look really solid. I pulled the branch down and got it running locally. The frontend side, which is this PR (#396) works nicely on top of the stable 9.x backend, but when I switched the backend to PR #71 as suggested, the project failed to run properly, so something seems off there and might need a closer look to confirm if it’s linked to this PR. With the frontend though, everything behaved as expected: I tested with elective units, added them into the trimester, marked them complete (which correctly locks them in place) and the Skills Summary dialog shows completed vs in-progress units as intended. The only UX suggestion I’d make is to surface the Skills Summary in a more direct way. Right now it’s tucked behind the three-dot menu on each unit even though it shows the same info regardless, so having a single dedicated entry point in the UI could make it easier to discover and use. Overall, Great job!

@EkamBhullar
Copy link

Everything on the frontend side worked smoothly for me as well. The flow for electives and the Skills Summary behaved exactly as expected. Only note is the backend PR (#71) didn’t run cleanly for me either, so that might need another look. Otherwise, great job!

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