Skip to content

fix: DSA sheet progress never persists — accept object completedTopics shape - #1729

Open
vedant7007 wants to merge 2 commits into
Canopus-Labs:mainfrom
vedant7007:fix/sheet-progress-object-shape
Open

fix: DSA sheet progress never persists — accept object completedTopics shape#1729
vedant7007 wants to merge 2 commits into
Canopus-Labs:mainfrom
vedant7007:fix/sheet-progress-object-shape

Conversation

@vedant7007

@vedant7007 vedant7007 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Closes #1727

Problem

saveProgress rejected the real payload with 400:

if (completedTopics !== undefined && !Array.isArray(completedTopics)) { /* 400 "must be an array" */ }

The frontend sends completedTopics as an object/map ({ "0-0-0": true }), and so do the Zod validator (z.record(z.string(), z.boolean())), the Mongoose schema ({ type: Object }), resetProgress ({}), and the import util. !Array.isArray({...}) is true, so every save returned 400 and the frontend .catch swallowed it — progress lived only in localStorage and was lost across devices / on cache clear.

Fix

Validate completedTopics as a plain object (reject arrays/null) to match the rest of the system.

Testing

  • node --check on the changed controller.

Contributing as part of Elite Coders Summer of Code (ECSoC 2026).

Summary

  • Fixed saveProgress validation for completedTopics.
  • The controller now accepts non-null plain objects with boolean values, such as { "0-0-0": true }.
  • The controller rejects arrays, null, and non-boolean values.
  • Updated the validation error message.
  • Verified syntax with node --check.

The controller required completedTopics to be an Array, but the frontend,
the Zod validator (z.record), the Mongoose schema ({ type: Object }), the
import util, and resetProgress all use an object/map — so every save hit
!Array.isArray({...}) and returned 400, and progress was silently lost
(kept only in localStorage). Validate it as a plain object instead.

Fixes Canopus-Labs#1727
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 18f6a69d-2097-473c-80ea-38045ca23129

📥 Commits

Reviewing files that changed from the base of the PR and between e2d6a03 and 200d7c2.

📒 Files selected for processing (1)
  • backend/controllers/userSheetProgressController.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/controllers/userSheetProgressController.js

📝 Walkthrough

Walkthrough

The progress controller now validates completedTopics as a non-null object with boolean values. It rejects arrays and invalid value types.

Changes

Progress validation

Layer / File(s) Summary
Completed topics validation
backend/controllers/userSheetProgressController.js
saveProgress accepts boolean-valued object maps for completedTopics, rejects arrays and invalid values, and returns an updated validation error.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested labels: good-backend

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the completedTopics validation fix that restores DSA sheet progress persistence.
Linked Issues check ✅ Passed The change satisfies issue #1727 by accepting boolean-valued object maps and rejecting arrays, null, and invalid values.
Out of Scope Changes check ✅ Passed The pull request only changes completedTopics validation in the relevant controller and contains no unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@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/controllers/userSheetProgressController.js`:
- Around line 63-71: Strengthen the completedTopics validation in the user sheet
progress controller to require a plain object and boolean values for every
property, matching the contract enforced by ValidateUserSheetProgress. Reject
non-plain objects and any entry whose value is not boolean before persisting the
record, while preserving the existing 400 response.
🪄 Autofix

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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f3981c68-c780-418a-ac3b-4a331ec7ad11

📥 Commits

Reviewing files that changed from the base of the PR and between a8a7be0 and e2d6a03.

📒 Files selected for processing (1)
  • backend/controllers/userSheetProgressController.js

Comment thread backend/controllers/userSheetProgressController.js Outdated
@github-actions github-actions Bot added the merge ready PR is mergeable and has no conflicts label Aug 9, 2026
@KaranUnique

Copy link
Copy Markdown
Contributor

@vedant7007 address the coderabbit suggestions

…ntract)

Per review: the object check accepted any non-null object, so { "0-0-0": "true" }
(string value) passed. Also require every value to be a boolean, matching the
Zod z.record(z.string(), z.boolean()) contract and the map/{key:bool} shape.
@vedant7007

Copy link
Copy Markdown
Contributor Author

Good catch @coderabbitai — tightened the check to also require every value to be a boolean (Object.values(completedTopics).some(v => typeof v !== 'boolean')), so { "0-0-0": "true" } is now rejected, matching the z.record(z.string(), z.boolean()) contract in ValidateUserSheetProgress.js. (The route runs validateSaveProgress before the controller, so this is a defensive second layer.)

Contributing as part of Elite Coders Summer of Code (ECSoC 2026).

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@vedant7007 The controller validation now matches the z.record(z.string(), z.boolean()) contract. The defensive check correctly rejects string values such as "true".

🐇 ✓


✏️ Learnings added
Learnt from: vedant7007
URL: https://github.com/Canopus-Labs/PrepPilot/pull/1729

Timestamp: 2026-08-11T14:34:20.016Z
Learning: In `backend/controllers/userSheetProgressController.js`, `completedTopics` must be a non-null plain object whose values are booleans. This matches the `z.record(z.string(), z.boolean())` contract in `ValidateUserSheetProgress.js`. The `validateSaveProgress` route middleware validates this contract first, and the controller repeats it as a defensive validation layer.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge ready PR is mergeable and has no conflicts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] DSA sheet progress never persists — saveProgress rejects the object completedTopics shape with 400

2 participants