feat: Deprecation DEPPS11: Replace PublicAPIRouter with PagesRouter
#9974
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughRemoves PublicAPIRouter and six static public_html pages, always mounts PagesRouter at root, changes Config.pagesEndpoint lookup, updates many tests to assert 200/content instead of 302 redirects (and to extract tokens from form inputs), updates DEPPS11 to removed, and adjusts Dockerfile public directory copy. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ 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). (15)
🔇 Additional comments (1)
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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
mtrezza
changed the title
PublicAPIRouter with PagesRouter
mtrezza
added
the
state:breaking
|
BREAKING CHANGE: This release replaces |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/PasswordPolicy.spec.js (1)
49-50: Test expectations correctly updated for content responses.The tests now expect HTTP 200 content responses instead of HTTP 302 redirects, reflecting the fundamental behavior change from the redirect-based flow to a content-based flow. Token extraction has been updated to parse HTML form fields instead of URL parameters.
Consider extracting the token regex pattern into a test helper constant to avoid repetition:
const TOKEN_REGEX = /name="token"[^>]*value="([^"]+)"/; // Then use it in tests: const match = response.text.match(TOKEN_REGEX);This would make the pattern easier to maintain if the HTML structure changes in the future.
Also applies to: 107-108, 621-622, 643-644, 711-712, 733-734, 895-896, 917-918, 984-985, 1044-1045, 1066-1067, 1308-1309, 1330-1331, 1461-1462, 1488-1489
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
DEPRECATIONS.md(1 hunks)public_html/invalid_link.html(0 hunks)public_html/invalid_verification_link.html(0 hunks)public_html/link_send_fail.html(0 hunks)public_html/link_send_success.html(0 hunks)public_html/password_reset_success.html(0 hunks)public_html/verify_email_success.html(0 hunks)spec/PasswordPolicy.spec.js(15 hunks)spec/PublicAPI.spec.js(0 hunks)src/Config.js(1 hunks)src/ParseServer.ts(1 hunks)src/Routers/PublicAPIRouter.js(0 hunks)
💤 Files with no reviewable changes (8)
- public_html/invalid_verification_link.html
- public_html/verify_email_success.html
- public_html/invalid_link.html
- src/Routers/PublicAPIRouter.js
- spec/PublicAPI.spec.js
- public_html/link_send_fail.html
- public_html/link_send_success.html
- public_html/password_reset_success.html
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR #9938 and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:08.440Z
Learning: For Parse Server 9 release preparation, the parse/push-adapter dependency should be upgraded to version >= 8.0.0, not 7.x, as version 8.x is required despite dropping Node 18 support (which aligns with Parse Server 9's removal of EOL Node versions).
📚 Learning: 2025-12-02T08:00:20.138Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR #9938 and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
Applied to files:
DEPRECATIONS.md
📚 Learning: 2025-12-02T08:00:08.440Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:08.440Z
Learning: For Parse Server 9 release preparation, the parse/push-adapter dependency should be upgraded to version >= 8.0.0, not 7.x, as version 8.x is required despite dropping Node 18 support (which aligns with Parse Server 9's removal of EOL Node versions).
Applied to files:
DEPRECATIONS.md
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
spec/PasswordPolicy.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Applied to files:
spec/PasswordPolicy.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Applied to files:
spec/PasswordPolicy.spec.js
🧬 Code graph analysis (1)
src/ParseServer.ts (1)
src/Routers/PagesRouter.js (2)
PagesRouter(64-734)pages(12-42)
⏰ 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). (15)
- GitHub Check: Redis Cache
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: Node 20
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: Node 22
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: Code Analysis (javascript)
- GitHub Check: Benchmarks
🔇 Additional comments (3)
DEPRECATIONS.md (1)
17-17: LGTM! Deprecation status correctly updated.The status change from "deprecated" to "removed" accurately reflects the implementation of this breaking change.
src/Config.js (1)
822-826: LGTM! Endpoint resolution simplified correctly.The removal of the
enableRouterdependency is appropriate sincePagesRouteris now the sole router. The logic correctly returns the configured endpoint or defaults to 'apps'.src/ParseServer.ts (1)
329-333: Routing consolidation to PagesRouter is correct.This change implements the intentional deprecation of
PublicAPIRouterin favor ofPagesRouter(PR #9526). ThePagesRouterprovides comprehensive coverage of email verification, password reset flows, custom routes, and localization—validated through extensive test coverage.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/ValidationAndPasswordsReset.spec.js (1)
1135-1205: Fix incorrect assertion:database.findreturns an array
Parse.Server.database.find(...)returns an array (as used earlier viaobj[0]), but the final assertion readsexpect(obj._perishable_token)..., which will beundefined/ throw depending on runner behavior.- expect(obj._perishable_token).not.toBe(token); + expect(obj[0]._perishable_token).not.toBe(token);
🧹 Nitpick comments (3)
spec/EmailVerificationToken.spec.js (1)
39-45: Good shift to 200 + content assertions; consider reducing brittleness of filename-fragment checksThe move from redirect assertions to
status === 200andresponse.textchecks matches the PagesRouter direction and should be more robust than URL-based redirects. The couple of assertions that check for HTML filename fragments (e.g.email_verification_send_success.html) are comparatively brittle—if possible, prefer asserting on a stable user-facing message or a semantic marker in the HTML instead of a filename.Also applies to: 76-83, 109-115, 141-148, 174-182, 261-278, 482-499, 525-541, 569-586, 622-628, 939-960, 1124-1145
spec/RegexVulnerabilities.spec.js (1)
95-104: Optionally assert status/body for the verify_email regex case (to avoid masking 5xx)Right now it only asserts that
emailVerifiedstaysfalse; adding a minimalstatus === 200+ “Invalid verification link!” (or similar) check would make regressions noisier if the endpoint starts erroring.spec/ValidationAndPasswordsReset.spec.js (1)
806-842: Token extraction via hidden input looks good; consider migrating remainingdone/setTimeouttests to async/awaitThe regex extraction from
name="token"is a reasonable replacement for URL token parsing. Separately, this file still heavily usesdonecallbacks andsetTimeout, which tends to be flaky; migrating these tests to promise-basedasync/awaitwould align with repo testing learnings. Based on learnings, prefer promise-based patterns overdone().Also applies to: 866-939, 941-996, 998-1056
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
spec/EmailVerificationToken.spec.js(15 hunks)spec/RegexVulnerabilities.spec.js(2 hunks)spec/ValidationAndPasswordsReset.spec.js(14 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR #9938 and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Applied to files:
spec/ValidationAndPasswordsReset.spec.jsspec/EmailVerificationToken.spec.js
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
spec/ValidationAndPasswordsReset.spec.jsspec/EmailVerificationToken.spec.js
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Applied to files:
spec/ValidationAndPasswordsReset.spec.js
⏰ 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). (14)
- GitHub Check: Node 20
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Benchmarks
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Node 22
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: Redis Cache
🔇 Additional comments (2)
spec/RegexVulnerabilities.spec.js (1)
146-167: LGTM: validates non-redirect behavior for invalid/valid reset tokensAsserting
200plus body text is a solid way to ensure the PagesRouter flow remains safe and isn’t relying on redirects.Also applies to: 194-211
spec/ValidationAndPasswordsReset.spec.js (1)
742-765: Re-check the expected behavior forresend_verification_emailfailure:303vs200This test expects
response.status === 303and asserts onresponse.textcontainingemail_verification_send_fail.html. If the new PagesRouter returns a200with rendered content (as other updated tests do), this should likely be200(and/or assert on a message) rather than303.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9974 +/- ##
==========================================
- Coverage 92.59% 92.56% -0.04%
==========================================
Files 191 190 -1
Lines 15544 15418 -126
Branches 177 176 -1
==========================================
- Hits 14393 14271 -122
+ Misses 1139 1135 -4
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Summary by CodeRabbit
Deprecations
Removals
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.