-
Notifications
You must be signed in to change notification settings - Fork 279
Main to Dev Sync #2514
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Main to Dev Sync #2514
Conversation
Dev to Main Sync
Dev to Main Sync
Dev to Main Sync
Dev to Main Sync
Dev to Main Sync
* chore: remove lastOooUntil migration artifacts and code (#2507) - drop updateLastOooUntil controller now that prod backfill is done - delete runLastOooUntilMigration helpers from userStatus.js - remove POST /users/migration/update-last-ooo-until route - strip integration tests that exercised the migration endpoint * fix(missed-updates): compute gap window using working days (#2511) * fix(missed-updates): compute gap window using working days - import convertTimestampToUTCStartOrEndOfDay so the window uses UTC day bounds - respect caller-provided exclusions instead of hard-coding Sunday into the set - step backwards through calendar days, skipping excluded ones, and bail when none remain - add regression coverage (plus fixture) for Sunday-only gaps and the Sunday-working-day case * fix(missed-updates): align working-day window counting and tests - compute the missed-updates window from consecutive working days without overshooting - adjust the Sunday-gap fixture to actual Saturday/Sunday/Monday dates - ensure regression tests cover both excluded Sunday and Sunday-as-working-day scenarios * fix: failing tests
… issue (#2516) * fix(progress-api): temporarily disable POST route due to security issue * test(progress-api): skip tests for Progress Updates API due to ongoing issues
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughA middleware gate is added to the progress creation endpoint to temporarily block new record submissions due to security concerns. Associated integration tests for progress POST operations are skipped to reflect this temporary disable. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
routes/progresses.ts(1 hunks)test/integration/progressesTasks.test.js(1 hunks)test/integration/progressesUsers.test.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
routes/progresses.ts (3)
middlewares/shortCircuit.ts (1)
disableRoute(4-8)middlewares/validators/progresses.js (1)
validateCreateProgressRecords(4-48)controllers/progresses.js (1)
createProgress(48-78)
🪛 GitHub Check: CodeQL
routes/progresses.ts
[failure] 18-18: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
This route handler performs authorization, but is not rate-limited.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (22.10.0)
🔇 Additional comments (3)
test/integration/progressesTasks.test.js (1)
28-29: LGTM! Test suite appropriately skipped.The POST test suite is correctly disabled using
describe.skipto align with the temporarily disabled route. The ESLint directive properly suppresses themocha/no-skipped-testswarning.test/integration/progressesUsers.test.js (1)
21-22: LGTM! Top-level test suite appropriately skipped.The entire test suite is correctly disabled using
describe.skipto align with the temporarily disabled POST route.routes/progresses.ts (1)
15-18: LGTM! Route correctly disabled with appropriate middleware.The
disableRoutemiddleware is properly positioned after authentication to return a 503 Service Unavailable response for all POST requests. The comment clearly explains the temporary nature of this change.Note: The static analysis tool flags missing rate limiting on this route. While not critical for a disabled route, consider adding rate limiting when re-enabling this endpoint to mitigate abuse and protect against DoS attacks.
Date:
4th December, 2025Developer Name:
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes