Conversation
|
Caution Review failedThe pull request is closed. WalkthroughUpdates include a new .editorconfig, i18n/branding slug changes, Domain symbol refactors and redirect-filter relocation, removal of a database Engine base class and translation scripts, and SSO/magic-link control-flow and API adjustments (early single-blog redirect, query-arg constant rename, removed query-vars method). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant App as WP Ultime App
participant SSO as Magic_Link
participant AdminBar as AdminBarMagicLinks
Note over AdminBar,SSO `#D6EAF8`: Early-redirect + token arg rename change
Browser->>App: Request protected admin page
App->>AdminBar: show_access_denied_with_magic_links()
AdminBar->>App: get_user_sites()
alt user has exactly one blog
AdminBar->>Browser: Redirect to blog admin URL (302)
Browser-->>AdminBar: (exit flow)
else multiple or zero blogs
AdminBar->>SSO: render denial UI (magic link generation)
SSO->>SSO: generate_magic_link(token param = TOKEN_QUERY_ARG)
SSO->>Browser: Provide magic link with TOKEN_QUERY_ARG
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
inc/sso/class-admin-bar-magic-links.php (1)
118-121: Consider more explicit array access.The early redirect for single-blog users is a good UX improvement. However, using
current($blogs)depends on the array's internal pointer. Consider usingreset($blogs)to be explicit, or use array destructuring for clarity.Apply this diff for more explicit array access:
if ( 1 === count($blogs) ) { - wp_safe_redirect(get_admin_url(current($blogs)->userblog_id)); + wp_safe_redirect(get_admin_url(reset($blogs)->userblog_id)); exit; }inc/sso/class-magic-link.php (1)
33-33: LGTM: Constant renamed consistently throughout.The rename from
TOKEN_QUERY_VARtoTOKEN_QUERY_ARGis applied consistently across all usages. The new name is arguably more accurate since it refers to a query string argument.Optionally, update the comment at line 28 for consistency:
/** - * Query var used for magic link tokens. + * Query argument used for magic link tokens. * * @since 2.0.0 * @var string */Also applies to: 76-76, 133-133, 191-191, 241-241
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.editorconfig(1 hunks)inc/admin-pages/class-setup-wizard-admin-page.php(1 hunks)inc/apis/trait-mcp-abilities.php(6 hunks)inc/class-domain-mapping.php(7 hunks)inc/database/engine/class-base.php(0 hunks)inc/sso/class-admin-bar-magic-links.php(1 hunks)inc/sso/class-magic-link.php(5 hunks)readme.txt(1 hunks)scripts/README-TRANSLATION.md(0 hunks)scripts/translate.php(0 hunks)
💤 Files with no reviewable changes (3)
- scripts/README-TRANSLATION.md
- inc/database/engine/class-base.php
- scripts/translate.php
🧰 Additional context used
🧬 Code graph analysis (2)
inc/class-domain-mapping.php (1)
inc/models/class-domain.php (2)
Domain(23-689)get_by_domain(633-688)
inc/sso/class-magic-link.php (1)
inc/functions/helper.php (1)
wu_request(132-137)
🪛 LanguageTool
readme.txt
[grammar] ~234-~234: Use a hyphen to join words.
Context: ...ded magic login links for SSO when third party cookies are disabled. - New: Added...
(QB_NEW_EN_HYPHEN)
⏰ 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). (2)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (7)
inc/admin-pages/class-setup-wizard-admin-page.php (1)
773-773: LGTM: Translation domain updated consistently.The text domain change from 'multisite-ultimate' to 'ultimate-multisite' aligns with the broader package rebranding effort across the PR.
inc/apis/trait-mcp-abilities.php (2)
87-96: LGTM: MCP category registration updated consistently.The category slug has been properly renamed from 'multisite-ultimate' to 'ultimate-multisite' in both the check and registration, ensuring the category is correctly created and referenced.
183-183: LGTM: All MCP ability category references updated consistently.All individual MCP abilities (get-item, get-items, create-item, update-item, delete-item) now correctly reference the renamed 'ultimate-multisite' category.
Also applies to: 234-234, 292-292, 342-342, 377-377
.editorconfig (1)
1-13: LGTM: Standard EditorConfig setup.The configuration follows common conventions with sensible defaults: UTF-8 encoding, LF line endings, tab-based indentation for general files, and space-based indentation for JSON/YAML files.
inc/class-domain-mapping.php (3)
12-12: LGTM: Domain symbol imported and used consistently.Adding the import statement and using the short
Domainreference throughout improves code readability and maintainability.Also applies to: 30-30
56-61: LGTM: Improved redirect host handling.Moving the
allowed_redirect_hostsfilter registration toinit()ensures that network-wide redirects work correctly in all contexts, not just when domain mapping checks are active. This is a good improvement for reliability.
159-159: LGTM: Consistent use of imported Domain symbol.All references to
Domain::INACTIVE_STAGESandDomain::get_by_domain()now use the imported symbol consistently throughout the file.Also applies to: 238-238, 284-284
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Bug Fixes
Chores