Skip to content

Fix txn API scope backwards compat#10640

Merged
abnegate merged 1 commit into1.8.xfrom
feat-txn
Oct 13, 2025
Merged

Fix txn API scope backwards compat#10640
abnegate merged 1 commit into1.8.xfrom
feat-txn

Conversation

@abnegate
Copy link
Member

What does this PR do?

Allow using API keys with only documents scopes for TablesDB transactions

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

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 13, 2025

📝 Walkthrough

Walkthrough

The PR updates authorization scope definitions for TablesDB transaction endpoints. In six files (Create, Delete, Get, Update, XList, and Operations/Create), scope labels are changed from single strings to arrays. Read endpoints now require ['documents.read', 'rows.read'] instead of 'rows.read'. Write endpoints now require ['documents.write', 'rows.write'] instead of 'rows.write'. No other logic, control flow, method signatures, or HTTP behaviors are altered.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • Meldiron
  • adityaoberai
  • ItzNotABug

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 title succinctly summarizes the primary change by indicating a fix to the transaction API scope for backward compatibility, which matches the adjustments made to support documents scopes in the TablesDB transactions. It is concise and clearly relates to the main intent of the changeset.
Description Check ✅ Passed The description clearly states that the PR allows API keys with only documents scopes for TablesDB transactions, directly matching the scope changes in the code and providing relevant context for the change.
✨ 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 feat-txn

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between de7dd60 and 935011b.

📒 Files selected for processing (6)
  • src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Create.php (1 hunks)
  • src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Delete.php (1 hunks)
  • src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Get.php (1 hunks)
  • src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Operations/Create.php (1 hunks)
  • src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Update.php (1 hunks)
  • src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/XList.php (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-19T09:20:03.312Z
Learnt from: ItzNotABug
PR: appwrite/appwrite#9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php:57-59
Timestamp: 2025-06-19T09:20:03.312Z
Learning: In table-related endpoints (such as `src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php`), parameter descriptions should use "table" and "row" terminology instead of "collection" and "document" for clarity and consistency.

Applied to files:

  • src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Operations/Create.php
  • src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Update.php
⏰ 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). (3)
  • GitHub Check: Benchmark
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan
🔇 Additional comments (6)
src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Update.php (1)

34-34: Scope expansion enables backwards compatibility.

The change from 'rows.write' to ['documents.write', 'rows.write'] allows API keys with either scope to update transactions, aligning with the PR objective.

src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Get.php (1)

33-33: Scope expansion enables backwards compatibility.

The change from 'rows.read' to ['documents.read', 'rows.read'] allows API keys with either scope to retrieve transactions, consistent with the broader scope expansion across all transaction endpoints.

src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Create.php (1)

33-33: Scope expansion enables backwards compatibility.

The change from 'rows.write' to ['documents.write', 'rows.write'] allows API keys with either scope to create transactions, supporting the backwards compatibility objective.

src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Operations/Create.php (1)

35-35: Scope expansion enables backwards compatibility.

The change from 'rows.write' to ['documents.write', 'rows.write'] allows API keys with either scope to create transaction operations, consistent with the scope expansion pattern across all transaction endpoints.

src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/Delete.php (1)

33-33: Scope expansion enables backwards compatibility.

The change from 'rows.write' to ['documents.write', 'rows.write'] allows API keys with either scope to delete transactions, completing the consistent scope expansion across all transaction endpoints.

Consider adding integration tests to verify API keys with documents.* scopes can successfully access all transaction endpoints:

# Test cases to add:
# 1. Create API key with only 'documents.read' scope
# 2. Verify GET /v1/tablesdb/transactions works
# 3. Verify GET /v1/tablesdb/transactions/:id works
# 4. Create API key with only 'documents.write' scope  
# 5. Verify POST /v1/tablesdb/transactions works
# 6. Verify PATCH /v1/tablesdb/transactions/:id works
# 7. Verify DELETE /v1/tablesdb/transactions/:id works
# 8. Verify POST /v1/tablesdb/transactions/:id/operations works

Also, ensure API documentation is updated to reflect the expanded scope requirements.

src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Transactions/XList.php (1)

33-33: Approve scope expansion on list transactions endpoint. Array-based scopes are already supported across similar list operations, enabling documents.read keys to access this endpoint without breaking existing rows.read access.


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.

@github-actions
Copy link

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
binutils 2.44-r2 CVE-2025-5244 HIGH
binutils 2.44-r2 CVE-2025-5245 HIGH
libxml2 2.13.8-r0 CVE-2025-49794 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49796 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49795 HIGH
libxml2 2.13.8-r0 CVE-2025-6021 HIGH
golang.org/x/crypto v0.31.0 CVE-2025-22869 HIGH
golang.org/x/oauth2 v0.24.0 CVE-2025-22868 HIGH
stdlib 1.22.10 CVE-2025-47907 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@github-actions
Copy link

✨ Benchmark results

  • Requests per second: 1,169
  • Requests with 200 status code: 210,506
  • P99 latency: 0.166273972

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,169 974
200 210,506 175,290
P99 0.166273972 0.199163846

@abnegate abnegate merged commit e303501 into 1.8.x Oct 13, 2025
41 checks passed
@abnegate abnegate deleted the feat-txn branch October 13, 2025 03:30
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.

2 participants