chore: add replacewith and deprecated since to account methods #10377
Conversation
… updateMagicURLSession
|
Warning Rate limit exceeded@ChiragAgg5k has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaced boolean deprecation markers with nullable Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/controllers/api/account.php (1)
147-152: Nit: PHP date format 'YYYY' is incorrect; should be 'Y'This is unrelated to the PR scope, but it’s a real user-facing issue:
DateTime::format('YYYY')outputs the literal string “YYYY”. Use'Y'for 4‑digit year.Apply:
$emailVariables = [ 'direction' => $locale->getText('settings.direction'), 'date' => (new \DateTime())->format('F j'), - 'year' => (new \DateTime())->format('YYYY'), + 'year' => (new \DateTime())->format('Y'), 'time' => (new \DateTime())->format('H:i:s'),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (12)
app/config/specs/open-api3-1.8.x-client.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.8.x-console.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.8.x-server.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-client.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-console.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-server.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.8.x-client.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.8.x-console.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.8.x-server.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-client.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-console.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-server.jsonis excluded by!app/config/specs/**
📒 Files selected for processing (1)
app/controllers/api/account.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/controllers/api/account.php (1)
src/Appwrite/SDK/Deprecated.php (1)
Deprecated(5-32)
⏰ 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). (4)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build SDK
- GitHub Check: scan
🔇 Additional comments (1)
app/controllers/api/account.php (1)
2422-2425: Deprecation Version VerifiedI’ve confirmed that both the
updateMagicURLSession(around line 2422) andupdatePhoneSession(around line 2463) endpoints inapp/controllers/api/account.phpcarrysince: '0.16.0', matching each other and indicating they were deprecated in v0.16.0. The later1.8.0deprecations apply only to the MFA-related endpoints, so no version alignment changes are needed here.
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 (3)
src/Appwrite/SDK/Specification/Format/Swagger2.php (1)
6-6: Fix linter failure: remove unused importPipeline reports PSR-12 no_unused_imports for this file. Deprecated is no longer referenced after switching away from instanceof checks. Remove the unused import to make Pint green.
Apply this diff:
use Appwrite\SDK\AuthType; -use Appwrite\SDK\Deprecated; use Appwrite\SDK\Method;src/Appwrite/SDK/Method.php (2)
280-284: Bug: setDeprecated accepts bool but assigns into ?Deprecated-typed propertyThe setter still accepts bool|Deprecated and directly assigns to a property typed ?Deprecated. Passing a boolean will trigger a TypeError at runtime. This is a regression introduced by the new property type and is likely to bite existing call sites.
Apply this diff to make the API coherent with the new contract:
- public function setDeprecated(bool|Deprecated $deprecated): self + public function setDeprecated(?Deprecated $deprecated): self { $this->deprecated = $deprecated; return $this; }If backward compatibility with boolean callers is required, add a temporary shim that rejects booleans explicitly and guides callers to construct Deprecated, e.g.:
public function setDeprecated(?Deprecated $deprecated): self { $this->deprecated = $deprecated; return $this; } /** @deprecated Pass Deprecated|null instead */ public function setDeprecatedFlag(bool $flag): self { if ($flag) { throw new \LogicException('Boolean deprecation flags are no longer supported. Pass an instance of Deprecated with since/replaceWith.'); } $this->deprecated = null; return $this; }
35-60: Remaining Boolean “deprecated” Flags Found – Action RequiredThe grep run identifies legacy boolean flags still in use within
src/Appwrite/Platform/Modules/Functions/Http/Functions/Create.php(lines 101–104):
- Four calls to
->param(…, true, deprecated: true)These must be replaced with proper
Deprecatedinstances (e.g.new Deprecated('sinceVersion', 'replacementMethod')) to align with the updated setter and constructor.• Update these four
->param(…)invocations to pass aDeprecatedobject instead ofdeprecated: true.
• Verify any other service or route metadata for similar boolean deprecation flags and refactor accordingly.
🧹 Nitpick comments (2)
src/Appwrite/SDK/Specification/Format/OpenAPI3.php (1)
170-175: Unify deprecation checks (truthy vs instanceof) for consistencyTop-level still uses instanceof Deprecated while additional methods use a truthy check. Both are correct given getDeprecated(): ?Deprecated, but using the same style improves readability and avoids an unnecessary import.
Apply this diff to align with the rest of the code:
- if ($sdk->getDeprecated() instanceof Deprecated) { + if ($sdk->getDeprecated()) { $temp['x-appwrite']['deprecated'] = [ 'since' => $sdk->getDeprecated()->getSince(), 'replaceWith' => $sdk->getDeprecated()->getReplaceWith(), ]; }Optionally remove the now-unused import:
use Appwrite\SDK\AuthType; -use Appwrite\SDK\Deprecated; use Appwrite\SDK\Method;src/Appwrite/SDK/Method.php (1)
286-291: Typo in setHide signature: should be bool|array, not bool|DeprecatedsetHide’s parameter type doesn’t match the property (array|bool) and return type doc (bool|array). This predates the PR but becomes more confusing next to the deprecation changes.
Apply this diff:
- public function setHide(bool|Deprecated $hide): self + public function setHide(bool|array $hide): self { $this->hide = $hide; return $this; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (12)
app/config/specs/open-api3-1.8.x-client.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.8.x-console.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.8.x-server.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-client.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-console.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-server.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.8.x-client.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.8.x-console.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.8.x-server.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-client.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-console.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-server.jsonis excluded by!app/config/specs/**
📒 Files selected for processing (4)
app/controllers/api/account.php(2 hunks)src/Appwrite/SDK/Method.php(3 hunks)src/Appwrite/SDK/Specification/Format/OpenAPI3.php(1 hunks)src/Appwrite/SDK/Specification/Format/Swagger2.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/controllers/api/account.php
🧰 Additional context used
🧬 Code graph analysis (3)
src/Appwrite/SDK/Specification/Format/OpenAPI3.php (1)
src/Appwrite/SDK/Method.php (1)
getDeprecated(191-194)
src/Appwrite/SDK/Specification/Format/Swagger2.php (1)
src/Appwrite/SDK/Method.php (1)
getDeprecated(191-194)
src/Appwrite/SDK/Method.php (1)
src/Appwrite/SDK/Deprecated.php (1)
Deprecated(5-32)
🪛 GitHub Actions: Linter
src/Appwrite/SDK/Specification/Format/Swagger2.php
[error] 1-1: Pint lint failed: PSR-12 no_unused_imports. Unused import detected in Swagger2.php. Reproduce with 'vendor/bin/pint --test --config pint.json'.
⏰ 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). (18)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (Databases/TablesDB)
- GitHub Check: E2E Service Test (Databases/Legacy)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Tokens)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Site Screenshots)
- GitHub Check: E2E General Test
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build SDK
- GitHub Check: scan
🔇 Additional comments (5)
src/Appwrite/SDK/Specification/Format/Swagger2.php (2)
174-179: Good switch to nullable Deprecated checkUsing a truthy check against getDeprecated() aligns with Method::getDeprecated(): ?Deprecated and keeps the code concise. The subsequent dereference is safe when the condition passes.
238-243: Consistent handling for additional methodsSame rationale here—truthy check is correct and consistent with the new nullable Deprecated type.
src/Appwrite/SDK/Specification/Format/OpenAPI3.php (1)
229-234: Truthy deprecation check for additional methods looks goodMatches the new Method::getDeprecated(): ?Deprecated contract.
src/Appwrite/SDK/Method.php (2)
27-28: Migration to nullable Deprecated on ctor/property is correctSwitching to Deprecated|null on the constructor doc and the promoted property is aligned with the new semantics and with getDeprecated(): ?Deprecated.
Also applies to: 44-45
188-189: isDeprecated()/getDeprecated() updates are soundisDeprecated now checks against null, and getDeprecated returns ?Deprecated—both consistent with the property type.
Also applies to: 191-194
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)
src/Appwrite/SDK/Specification/Format/Swagger2.php (1)
163-163: Fix null-coalescing precedence bug in the “edit” URLWithout parentheses, the
?? ''applies to the concatenation result, not to$sdk->getDescription(). This yields a dangling base URL when description is null.Apply this diff:
- 'edit' => 'https://github.com/appwrite/appwrite/edit/master' . $sdk->getDescription() ?? '', + 'edit' => 'https://github.com/appwrite/appwrite/edit/master' . ($sdk->getDescription() ?? ''),
🧹 Nitpick comments (2)
src/Appwrite/SDK/Specification/Format/Swagger2.php (2)
173-178: Prefer explicit null-check and cache Deprecated to avoid repeated callsGiven
Method::getDeprecated(): ?Deprecated, an explicit null check improves readability and avoids three identical calls in this block.Apply this diff:
- if ($sdk->getDeprecated()) { - $temp['x-appwrite']['deprecated'] = [ - 'since' => $sdk->getDeprecated()->getSince(), - 'replaceWith' => $sdk->getDeprecated()->getReplaceWith(), - ]; - } + $deprecated = $sdk->getDeprecated(); + if (null !== $deprecated) { + $temp['x-appwrite']['deprecated'] = [ + 'since' => $deprecated->getSince(), + 'replaceWith' => $deprecated->getReplaceWith(), + ]; + }
237-242: Mirror the same explicit null-check/caching for additional methodsMinor consistency/readability improvement; also avoids repeated calls in this loop.
Apply this diff:
- if ($methodObj->getDeprecated()) { - $additionalMethod['deprecated'] = [ - 'since' => $methodObj->getDeprecated()->getSince(), - 'replaceWith' => $methodObj->getDeprecated()->getReplaceWith(), - ]; - } + $deprecated = $methodObj->getDeprecated(); + if (null !== $deprecated) { + $additionalMethod['deprecated'] = [ + 'since' => $deprecated->getSince(), + 'replaceWith' => $deprecated->getReplaceWith(), + ]; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/Appwrite/SDK/Specification/Format/Swagger2.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Appwrite/SDK/Specification/Format/Swagger2.php (1)
src/Appwrite/SDK/Method.php (1)
getDeprecated(191-194)
⏰ 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). (16)
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (VCS)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Tokens)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Databases/Legacy)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Databases/TablesDB)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Site Screenshots)
✨ Benchmark results
⚡ Benchmark Comparison
|
… updateMagicURLSession
What does this PR do?
(Provide a description of what this PR does and why it's needed.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist