[4x] Delete redundant universal route check from prevent access MW#1427
[4x] Delete redundant universal route check from prevent access MW#1427
Conversation
The PreventAcessFromUnwantedDomains MW had the routeIsUniversal check either for returning early, or it was a leftover from some older implementation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1427 +/- ##
=========================================
Coverage 86.03% 86.03%
+ Complexity 1156 1155 -1
=========================================
Files 184 184
Lines 3381 3381
=========================================
Hits 2909 2909
Misses 472 472 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe middleware's Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Removes a redundant universal-route short-circuit from PreventAccessFromUnwantedDomains, relying on the existing route-mode checks to ensure universal routes naturally pass through without triggering access prevention.
Changes:
- Removed
tenancy()->routeIsUniversal($route)from the early-return condition in the middleware. - Leaves skipping behavior solely to
shouldBeSkipped($route).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The PreventAcessFromUnwantedDomains MW had the
tenancy()->routeIsUniversal($route)check either for returning early, or it was a leftover from some older implementation, so I removed it.The middleware aborts if the
$this->accessingTenantRouteFromCentralDomain($request, $route) || $this->accessingCentralRouteFromTenantDomain($request, $route)check passes. Meaning, for the middleware to abort, the route has to be either in central or tenant mode. When the route is in universal mode, the middleware will never reachreturn $abortRequest().return $next($request)will always get reached, even when the|| tenancy()->routeIsUniversal($route)check is deleted from the previous condition, so that check was basically useless.Resolves #1418
Summary by CodeRabbit