Skip to content

Conversation

@abnegate
Copy link
Member

@abnegate abnegate commented Sep 4, 2025

What does this PR do?

(Provide a description of what this PR does.)

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.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

Summary by CodeRabbit

  • New Features

    • Queries: nested-array support plus new builders for created/updated ranges, distance-based and spatial predicates (and their negations).
    • Added support for the HEAD execution method.
  • Breaking Changes

    • Credit card type renamed from "union-china-pay" to "unionpay".
  • Documentation

    • Swift Package Manager example updated to require sdk-for-apple >= 12.0.0.
    • Examples now show populated data and clarified session deprecation messages.
  • Chores

    • SDK identifier/version bumped to 12.0.0.

@coderabbitai
Copy link

coderabbitai bot commented Sep 4, 2025

Walkthrough

README SPM example now requires sdk-for-apple from 12.0.0. Client.default x-sdk-version header changed to 12.0.0. Query adds QueryValue.array with encoding/decoding and nested-array support, plus new builders for created/updated ranges, distance predicates, and spatial predicates. Account deprecation messages for updateMagicURLSession and updatePhoneSession now reference deprecation since 1.6.0 and recommend Account.createSession. CreditCard enum case unionChinaPayunionPay (raw value changed). ExecutionMethod adds HEAD. Several docs examples use non-empty data/prefs payloads.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8ee4f5 and e94f4f7.

📒 Files selected for processing (1)
  • Sources/Appwrite/Query.swift (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Sources/Appwrite/Query.swift
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/Appwrite/Services/Account.swift (1)

52-60: Fix typos in user-facing doc comments

Minor spelling fixes; these ship to developers in symbol docs.

-/// [/account/verfication](https://appwrite.io/docs/references/cloud/client-web/account#createVerification)
+/// [/account/verification](https://appwrite.io/docs/references/cloud/client-web/account#createVerification)

-/// List the factors available on the account to be used as a MFA challange.
+/// List the factors available on the account to be used as an MFA challenge.

-/// generate and show then immediately after user successfully adds their
-/// authehticator.
+/// generate and show them immediately after the user successfully adds their
+/// authenticator.

-/// An OTP challenge is required to regenreate recovery codes.
+/// An OTP challenge is required to regenerate recovery codes.

Also applies to: 102-110, 793-803, 943-951, 976-984

🧹 Nitpick comments (7)
README.md (1)

22-22: Prefer HTTPS package URL for SPM/Xcode UX.

Xcode’s “Add Package” flow expects an HTTPS URL. Suggest switching from SSH to HTTPS in both places to reduce friction.

- In the dialog that appears, enter the Appwrite Swift SDK [package URL]([email protected]:appwrite/sdk-for-apple.git) in the search field. Once found, select `sdk-for-apple`.
+ In the dialog that appears, enter the Appwrite Swift SDK [package URL](https://github.com/appwrite/sdk-for-apple.git) in the search field. Once found, select `sdk-for-apple`.
-        .package(url: "[email protected]:appwrite/sdk-for-apple.git", from: "12.0.0"),
+        .package(url: "https://github.com/appwrite/sdk-for-apple.git", from: "12.0.0"),

Also applies to: 34-35

Sources/Appwrite/Services/Account.swift (2)

1511-1578: Add renamed: to deprecation for fix-it guidance

Xcode can auto-rename usages when you include renamed:.

-@available(*, deprecated, message: "This API has been deprecated since 1.6.0. Please use `Account.createSession` instead.")
+@available(*, deprecated, renamed: "createSession", message: "Deprecated since 1.6.0. Use `Account.createSession`.")
 open func updateMagicURLSession(

1651-1678: Add renamed: to deprecation for fix-it guidance

Mirror the same improvement here.

-@available(*, deprecated, message: "This API has been deprecated since 1.6.0. Please use `Account.createSession` instead.")
+@available(*, deprecated, renamed: "createSession", message: "Deprecated since 1.6.0. Use `Account.createSession`.")
 open func updatePhoneSession(
Sources/Appwrite/Query.swift (2)

72-96: Handle Float and avoid silent element drops in nested conversion

Currently, Float values are dropped and unknown items are silently skipped. Add Float support and fail conversion when any element is unsupported to prevent partial data loss.

Run tests that exercise nested coordinates containing Float values and mixed numeric types.

 private static func convertToQueryValueArray(_ values: Any?) -> [QueryValue]? {
     switch values {
     case let valueArray as [QueryValue]:
         return valueArray
     case let stringArray as [String]:
         return stringArray.map { .string($0) }
     case let intArray as [Int]:
         return intArray.map { .int($0) }
     case let doubleArray as [Double]:
         return doubleArray.map { .double($0) }
+    case let floatArray as [Float]:
+        return floatArray.map { .double(Double($0)) }
     case let boolArray as [Bool]:
         return boolArray.map { .bool($0) }
     case let queryArray as [Query]:
         return queryArray.map { .query($0) }
     case let stringValue as String:
         return [.string(stringValue)]
     case let intValue as Int:
         return [.int(intValue)]
     case let doubleValue as Double:
         return [.double(doubleValue)]
+    case let floatValue as Float:
+        return [.double(Double(floatValue))]
     case let boolValue as Bool:
         return [.bool(boolValue)]
     case let queryValue as Query:
         return [.query(queryValue)]
     case let anyArray as [Any]:
-        // Handle nested arrays
-        let nestedValues = anyArray.compactMap { item -> QueryValue? in
-            if let stringValue = item as? String {
-                return .string(stringValue)
-            } else if let intValue = item as? Int {
-                return .int(intValue)
-            } else if let doubleValue = item as? Double {
-                return .double(doubleValue)
-            } else if let boolValue = item as? Bool {
-                return .bool(boolValue)
-            } else if let queryValue = item as? Query {
-                return .query(queryValue)
-            } else if let nestedArray = item as? [Any] {
-                // Convert nested array to QueryValue.array
-                if let converted = convertToQueryValueArray(nestedArray) {
-                    return .array(converted)
-                }
-                return nil
-            }
-            return nil
-        }
-        return nestedValues.isEmpty ? nil : nestedValues
+        // Handle nested arrays; fail the whole array if any element is unsupported
+        var nestedValues: [QueryValue] = []
+        for item in anyArray {
+            if let stringValue = item as? String {
+                nestedValues.append(.string(stringValue))
+            } else if let intValue = item as? Int {
+                nestedValues.append(.int(intValue))
+            } else if let doubleValue = item as? Double {
+                nestedValues.append(.double(doubleValue))
+            } else if let floatValue = item as? Float {
+                nestedValues.append(.double(Double(floatValue)))
+            } else if let boolValue = item as? Bool {
+                nestedValues.append(.bool(boolValue))
+            } else if let queryValue = item as? Query {
+                nestedValues.append(.query(queryValue))
+            } else if let nestedArray = item as? [Any], let converted = convertToQueryValueArray(nestedArray) {
+                nestedValues.append(.array(converted))
+            } else {
+                return nil
+            }
+        }
+        return nestedValues
     default:
         return nil
     }
 }

Also applies to: 96-119


447-541: Consider minimal docs for new spatial/distance builders

A short doc comment on expected values shape (e.g., GeoJSON-like coordinates) and meters semantics will reduce misuse.

Sources/Appwrite/Client.swift (1)

26-26: Version bump verified across codebase
No stale x-sdk-version entries found and README.md’s SPM snippet is updated to 12.0.0. Consider refactoring x-sdk-version to use a single static sdkVersion constant in Client.swift for future bumps.

Sources/AppwriteEnums/ExecutionMethod.swift (1)

10-10: Naming nit: consider lowerCamelCase in a future major

Swift case names are typically lowerCamelCase (e.g., head). Keeping hEAD is fine for consistency today; consider a rename with deprecations in the next major.

Optionally add a short doc comment now:

-    case hEAD = "HEAD"
+    /// HTTP HEAD
+    case hEAD = "HEAD"
📜 Review details

Configuration used: CodeRabbit UI

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0dafc0a and a8ee4f5.

📒 Files selected for processing (9)
  • README.md (1 hunks)
  • Sources/Appwrite/Client.swift (1 hunks)
  • Sources/Appwrite/Query.swift (7 hunks)
  • Sources/Appwrite/Services/Account.swift (2 hunks)
  • Sources/AppwriteEnums/CreditCard.swift (1 hunks)
  • Sources/AppwriteEnums/ExecutionMethod.swift (1 hunks)
  • docs/examples/account/update-prefs.md (1 hunks)
  • docs/examples/databases/create-document.md (1 hunks)
  • docs/examples/tablesdb/create-row.md (1 hunks)
🔇 Additional comments (9)
README.md (1)

34-35: Confirm version bump and tag
README updated to 12.0.0; no lingering 11.0.0 references or x-sdk-version headers detected in Sources. Please manually verify the upstream 12.0.0 tag exists.

docs/examples/account/update-prefs.md (1)

10-14: LGTM — clearer prefs payload.

Concrete keys/values make the example more useful. No issues from my side.

Sources/AppwriteEnums/CreditCard.swift (1)

16-16: Add legacy decoding for “union-china-pay”
Implement Codable with a custom init(from:) that maps both "unionpay" and "union-china-pay" to .unionPay, and default encoding of rawValue. Optionally add a deprecated unionChinaPay alias for migration. A search (rg -n 'unionChinaPay|union-china-pay') returned no hits—manually verify no remaining legacy references.

Sources/Appwrite/Query.swift (4)

9-10: Enum case for nested arrays looks good

New QueryValue.array([QueryValue]) enables nested arrays without breaking ABI.


14-31: Decoder path LGTM

Order of decode attempts is fine; arrays fall through to [QueryValue] safely.


47-49: Encoder path LGTM

Nested arrays are encoded correctly via single value container.


389-395: Range helpers are consistent and clear

createdBetween/updatedBetween align with existing before/after APIs.

Also applies to: 410-415

Sources/AppwriteEnums/ExecutionMethod.swift (2)

10-10: HEAD support added correctly

The new case maps to "HEAD" via rawValue and will serialize as expected. No behavioral concerns here.


10-10: Remove unused ExecutionMethod enum or integrate it for HEAD support
ExecutionMethod isn’t referenced in any switch or whitelist – HTTP calls pass raw strings. Either eliminate the dead enum or refactor Client.call to use ExecutionMethod (including .hEAD) if you intend to support HEAD.

Likely an incorrect or invalid review comment.

@abnegate abnegate merged commit c6f0f39 into main Sep 8, 2025
1 check passed
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.

3 participants