Skip to content

Conversation

@ArnabChatterjee20k
Copy link
Contributor

@ArnabChatterjee20k ArnabChatterjee20k commented Oct 28, 2025

What does this PR do?

Screencast.from.2025-10-28.17-26-18.webm

Will solve -> appwrite/sdk-for-cli#210 (comment)

(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
    • Added support for geospatial attribute types (point, linestring, and polygon) in database attribute creation and updates. Users can now create and manage geospatial attributes for their database collections.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 28, 2025

Walkthrough

This pull request adds support for three geospatial attribute types (point, linestring, polygon) to the CLI push command. Six new handler methods are added to the databases module and exposed through the import statement. The createAttribute and updateAttribute functions are extended with new cases that route these geospatial types to their respective create/update handlers, passing the appropriate parameters (databaseId, collectionId, key, required, xdefault, and parseOutput: false). Unsupported types continue to throw errors as before.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Single file scope: All changes contained within templates/cli/lib/commands/push.js.twig
  • Repetitive pattern: Six methods follow consistent structure; three type cases follow similar routing logic
  • Multiple integration points: Requires verification across import statement, method declarations, and two separate function handlers (createAttribute and updateAttribute)

Key areas requiring attention:

  • Verify all six new methods (Create/Update for Point, Line, Polygon) are correctly declared and exported
  • Confirm both createAttribute and updateAttribute functions handle all three geospatial types with consistent parameter passing
  • Validate that the parameter structure (databaseId, collectionId, key, required, xdefault, parseOutput: false) is correct for geospatial handlers
  • Ensure the import statement includes all six new methods from the databases module

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "fix - added push command support for cli spatial types" directly and clearly summarizes the main change in the changeset. The raw summary confirms that the modifications add support for geospatial attribute types (point, linestring, and polygon) in the CLI push command file (templates/cli/lib/commands/push.js.twig), including both creation and update paths with six new methods exposed in the public API. The title is concise, specific enough for teammates to understand the primary change when scanning history, and does not include misleading, off-topic, or vague language.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cli-sdk-spatial

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
Contributor

@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: 1

🧹 Nitpick comments (2)
templates/cli/lib/commands/push.js.twig (2)

571-579: Spatial create: confirm array support; fix minor style

  • Do spatial attributes support array? Other types pass array: attribute.array. If supported, add it here; if not, keep as-is. Please confirm against the API.
  • Minor style: add spaces after colons to match surrounding code.

If arrays are supported for point/linestring/polygon, update payloads accordingly (and mirror in updateAttribute).

Style-only diff:

         case 'point':
             return databasesCreatePointAttribute({
                 databaseId,
                 collectionId,
-                key:attribute.key,
-                required:attribute.required,
-                xdefault:attribute.default,
-                parseOutput:false
+                key: attribute.key,
+                required: attribute.required,
+                xdefault: attribute.default,
+                parseOutput: false
             })
         case 'linestring':
             return databasesCreateLineAttribute({
                 databaseId,
                 collectionId,
-                key:attribute.key,
-                required:attribute.required,
-                xdefault:attribute.default,
-                parseOutput:false
+                key: attribute.key,
+                required: attribute.required,
+                xdefault: attribute.default,
+                parseOutput: false
             })
         case 'polygon':
             return databasesCreatePolygonAttribute({
                 databaseId,
                 collectionId,
-                key:attribute.key,
-                required:attribute.required,
-                xdefault:attribute.default,
-                parseOutput:false
+                key: attribute.key,
+                required: attribute.required,
+                xdefault: attribute.default,
+                parseOutput: false
             })

Also applies to: 581-588, 590-597


717-725: Spatial update: mirror create decisions; fix minor style

  • Mirror the createAttribute decision on array support (add array: attribute.array if the API supports it).
  • Normalize spacing for consistency.

Confirm API support for arrays on spatial updates; adjust payloads if applicable.

Style-only diff:

         case 'point':
             return databasesUpdatePointAttribute({
                 databaseId,
                 collectionId,
-                key:attribute.key,
-                required:attribute.required,
-                xdefault:attribute.default,
-                parseOutput:false
+                key: attribute.key,
+                required: attribute.required,
+                xdefault: attribute.default,
+                parseOutput: false
             })
         case 'linestring':
             return databasesUpdateLineAttribute({
                 databaseId,
                 collectionId,
-                key:attribute.key,
-                required:attribute.required,
-                xdefault:attribute.default,
-                parseOutput:false
+                key: attribute.key,
+                required: attribute.required,
+                xdefault: attribute.default,
+                parseOutput: false
             })
         case 'polygon':
             return databasesUpdatePolygonAttribute({
                 databaseId,
                 collectionId,
-                key:attribute.key,
-                required:attribute.required,
-                xdefault:attribute.default,
-                parseOutput:false
+                key: attribute.key,
+                required: attribute.required,
+                xdefault: attribute.default,
+                parseOutput: false
             })

Also applies to: 727-734, 736-743

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b4a2fd9 and 95f91ca.

📒 Files selected for processing (1)
  • templates/cli/lib/commands/push.js.twig (3 hunks)
⏰ 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). (17)
  • GitHub Check: build (8.3, Ruby27)
  • GitHub Check: build (8.3, AppleSwift56)
  • GitHub Check: build (8.3, Python39)
  • GitHub Check: build (8.3, WebNode)
  • GitHub Check: build (8.3, Ruby30)
  • GitHub Check: build (8.3, Swift56)
  • GitHub Check: build (8.3, KotlinJava8)
  • GitHub Check: build (8.3, Node18)
  • GitHub Check: build (8.3, Node16)
  • GitHub Check: build (8.3, DotNet80)
  • GitHub Check: build (8.3, DartStable)
  • GitHub Check: build (8.3, CLINode16)
  • GitHub Check: swift (server)
  • GitHub Check: build (8.3, Android14Java17)
  • GitHub Check: apple (client)
  • GitHub Check: build (8.3, Android5Java17)
  • GitHub Check: android (client)

Comment on lines +46 to +51
databasesCreatePointAttribute,
databasesUpdatePointAttribute,
databasesCreateLineAttribute,
databasesUpdateLineAttribute,
databasesCreatePolygonAttribute,
databasesUpdatePolygonAttribute,
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify new databases exports exist and match names

Ensure these functions are actually defined and exported from ./databases with the exact identifiers used here (especially “Line” vs “linestring” naming).

Run:


🏁 Script executed:

#!/bin/bash
# Find definitions or exports of the new spatial helpers
rg -nC2 -S --glob '!**/node_modules/**' \
  -e '\b(databases(Create|Update)(Point|Line|Polygon)Attribute)\b' \
  -e 'module\.exports\s*=\s*{[^}]*databases(Create|Update)(Point|Line|Polygon)Attribute' 

Length of output: 306


I'll generate a simplified verification script to search for these functions without the complex regex pattern:


🏁 Script executed:

#!/bin/bash
# Step 1: Find the databases module file
echo "=== Finding databases module ==="
fd -t f 'databases\.(js|ts)' --glob '!**/node_modules/**' | head -20

echo ""
echo "=== Searching for spatial attribute functions ==="
# Search for each function name separately with simpler patterns
rg -n 'databasesCreatePointAttribute|databasesUpdatePointAttribute|databasesCreateLineAttribute|databasesUpdateLineAttribute|databasesCreatePolygonAttribute|databasesUpdatePolygonAttribute' --glob '!**/node_modules/**' -C1

Length of output: 2250



🏁 Script executed:

#!/bin/bash
# Find the import statement to see where these come from
echo "=== Import statement context ==="
rg -n 'databasesCreatePointAttribute|from.*databases' templates/cli/lib/commands/push.js.twig -B5 | head -40

echo ""
echo "=== Looking for databases module in lib/sdks or similar ==="
fd -t f 'databases' --type file | grep -E '\.(js|ts)$' | grep -v node_modules | head -20

echo ""
echo "=== Check for databases exports/definitions ==="
rg -n 'export.*databasesCreatePointAttribute|databasesCreatePointAttribute\s*=|function databasesCreatePointAttribute' --glob '!**/node_modules/**' -A2

Length of output: 622


🏁 Script executed:

#!/bin/bash
# Find the import statement to see where these come from
echo "=== Import statement context ==="
rg -n 'from.*databases|import.*databases' templates/cli/lib/commands/push.js.twig -B2 -A2 | head -50

echo ""
echo "=== Looking for databases module in sdks or sdk/functions ==="
fd 'databases' -type f | grep -E '\.(js|ts)$' | grep -v node_modules

echo ""
echo "=== Check for database spatial attribute function definitions ==="
rg -n 'databasesCreatePointAttribute|databasesCreateLineAttribute|databasesCreatePolygonAttribute' --glob '!**/node_modules/**' -A3

Length of output: 1915


🏁 Script executed:

#!/bin/bash
# Look at the beginning of push.js.twig to find import statements
echo "=== First 60 lines of push.js.twig ==="
head -60 templates/cli/lib/commands/push.js.twig

echo ""
echo "=== Search for where databases imports come from ==="
rg -n '^\s*import|^\s*const.*=.*require|from\s' templates/cli/lib/commands/push.js.twig | head -20

echo ""
echo "=== Look for any files containing these function names as exports ==="
rg -n 'export.*Point|export.*Line|export.*Polygon' --glob '!**/node_modules/**'

Length of output: 4870


🏁 Script executed:

#!/bin/bash
# Find databases.js in the commands directory
echo "=== Looking for databases file in commands directory ==="
ls -la templates/cli/lib/commands/ | grep databases

echo ""
echo "=== Check content of databases.js/twig ==="
if [ -f "templates/cli/lib/commands/databases.js" ]; then
  head -100 templates/cli/lib/commands/databases.js
elif [ -f "templates/cli/lib/commands/databases.js.twig" ]; then
  head -100 templates/cli/lib/commands/databases.js.twig
fi

echo ""
echo "=== Search for the 6 spatial functions in databases file ==="
rg -n 'databasesCreatePointAttribute|databasesUpdatePointAttribute|databasesCreateLineAttribute|databasesUpdateLineAttribute|databasesCreatePolygonAttribute|databasesUpdatePolygonAttribute' templates/cli/lib/commands/databases* -A1

Length of output: 240


🏁 Script executed:

#!/bin/bash
# Broad search for databases files
echo "=== Search for all databases-related files ==="
find . -name '*databases*' -type f | grep -v node_modules | head -20

echo ""
echo "=== Search entire codebase for databasesCreatePointAttribute definition ==="
rg -n 'const databasesCreatePointAttribute|function databasesCreatePointAttribute|databasesCreatePointAttribute\s*=' --glob '!**/node_modules/**'

echo ""
echo "=== Search for module.exports in any databases file ==="
rg -n 'module\.exports|exports\.' --glob '!**/node_modules/**' -g '*databases*'

Length of output: 329


🏁 Script executed:

#!/bin/bash
# List full directory to see what files exist
echo "=== Full contents of templates/cli/lib/commands/ ==="
find templates/cli/lib/commands/ -type f | sort

echo ""
echo "=== Check if there's a databases.js.twig template ==="
find . -name 'databases.js.twig' 2>/dev/null

echo ""
echo "=== Search for any function named databasesCreatePoint anywhere ==="
rg 'databasesCreatePoint' --glob '!**/node_modules/**'

echo ""
echo "=== Check templates directory structure ==="
ls -la templates/cli/lib/

Length of output: 1786


Spatial attribute functions are imported but not exported from databases module

The six spatial functions (databasesCreatePointAttribute, databasesUpdatePointAttribute, databasesCreateLineAttribute, databasesUpdateLineAttribute, databasesCreatePolygonAttribute, databasesUpdatePolygonAttribute) are imported in push.js.twig but do not exist anywhere in the codebase. There is no databases.js.twig template file, and no module exports these functions. The import statement require("./databases") references a module that either doesn't exist or is incomplete. These functions must be defined and exported from a databases module before they can be used in push.js.twig.

🤖 Prompt for AI Agents
In templates/cli/lib/commands/push.js.twig around lines 46 to 51, six spatial
functions are imported from "./databases" but that module does not export them
(and the databases template is missing); implement and export
databasesCreatePointAttribute, databasesUpdatePointAttribute,
databasesCreateLineAttribute, databasesUpdateLineAttribute,
databasesCreatePolygonAttribute, and databasesUpdatePolygonAttribute from the
databases module (create templates/cli/lib/commands/databases.js.twig if
missing) with the expected signatures used by push.js.twig and update the
require path if necessary so push.js.twig imports the module that actually
exports these functions.

@ChiragAgg5k ChiragAgg5k merged commit ae9f475 into master Oct 28, 2025
52 checks passed
@ChiragAgg5k ChiragAgg5k deleted the cli-sdk-spatial branch October 28, 2025 15:38
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