Skip to content

Conversation

@MayankBansal12
Copy link
Member

@MayankBansal12 MayankBansal12 commented Jan 1, 2026

Date: 01-01-2026

Developer Name: @MayankBansal12


Issue Ticket Number:-

Description:

  • Removes dev flag from notifications page
  • removes tests for notifications

Is Under Feature Flag

  • Yes
  • No

Database changes

  • Yes
  • No

Breaking changes (If your feature is breaking/missing something please mention pending tickets)

  • Yes
  • No

Is Development Tested?

  • Yes
  • No

Tested in staging?

  • Yes
  • No

Add relevant Screenshot below ( e.g test coverage etc. )

demo notifications
demo-notificatoins.mp4

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 1, 2026

Deploying www-rds with  Cloudflare Pages  Cloudflare Pages

Latest commit: ebf679f
Status:🚫  Build failed.

View logs

@coderabbitai
Copy link

coderabbitai bot commented Jan 1, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The NotificationsRoute class has been simplified by removing its router service injection, development mode queryParam configuration, and beforeModel guard that previously redirected non-dev users to /page-not-found. The corresponding test validating this redirect behavior is also removed.

Changes

Cohort / File(s) Summary
Route simplification
app/routes/notifications.js
Removed router service, dev queryParam config, and beforeModel guard that handled access control logic
Test cleanup
tests/unit/routes/notifications-test.js
Removed unit test "it should redirect to '/page-not-found' page when dev flag is not present"

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 The guards have left, the path now free,
No dev checks block where users be,
Notifications route, stripped and bare,
Simpler flows float through the air!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title clearly and concisely describes the main change: removing the dev flag functionality from the notifications page, which aligns with the code changes shown in the summary.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description clearly relates to the changeset, which removes the dev flag and associated tests from the notifications page.

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.

❤️ Share

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

@MayankBansal12 MayankBansal12 self-assigned this Jan 1, 2026
@iamitprakash iamitprakash merged commit 24914e2 into develop Jan 6, 2026
3 of 4 checks passed
@iamitprakash iamitprakash deleted the remove/notification-flag branch January 6, 2026 17:46
@MayankBansal12 MayankBansal12 mentioned this pull request Jan 7, 2026
10 tasks
iamitprakash added a commit that referenced this pull request Jan 16, 2026
* chore: remove dev flag from identity page (#1119)

* chore: remove dev flag from discord (#1118)

* chore: remove dev flag from notifications (#1117)

* chore: remove dev flag from profile page (#1114)

* chore: remove dev flag from profile page

* fix: add test description

* fix: naming for profile route test

* chore: remove dev flag from new signup page (#1115)

* chore: remove feature flag from mobile page (#1019)

* chore: remove feature flag from mobile page

* fix: naming for mobile route test

* chore: replace my site url with updated links (#1129)
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.

4 participants