Skip to content

fix: missing nullable in datetime#10819

Merged
loks0n merged 1 commit into1.8.xfrom
fix-missing-nullable
Nov 14, 2025
Merged

fix: missing nullable in datetime#10819
loks0n merged 1 commit into1.8.xfrom
fix-missing-nullable

Conversation

@ChiragAgg5k
Copy link
Copy Markdown
Member

@ChiragAgg5k ChiragAgg5k commented Nov 14, 2025

No description provided.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Nov 14, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

Two datetime creation handlers in the Databases module have been updated to accept nullable defaults. Both Datetime/Create.php files—one for Collections/Attributes and one for TablesDB/Tables—now import the Nullable validator and wrap their existing DatetimeValidator with it. This changes the validation logic to permit null values as valid defaults for datetime fields, where previously only datetime values were accepted.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Consistent, homogeneous changes applied identically across two parallel files
  • Single import addition and one-line validator wrapping in each file
  • Straightforward pattern with minimal logic density
  • Primary consideration: verify that allowing null defaults for datetime fields is intentional and does not introduce unexpected behavioral changes
  • Check that both attribute and column handlers require parallel updates or if discrepancies are warranted

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
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.
Description check ❓ Inconclusive No description was provided by the author, making it impossible to assess whether it relates to the changeset. Add a pull request description explaining why the nullable wrapper is needed and what issue this fixes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding missing nullable validation to datetime fields in two files.
✨ 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 fix-missing-nullable
📜 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 a1dff37 and cc7b5b7.

📒 Files selected for processing (2)
  • src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Attributes/Datetime/Create.php (2 hunks)
  • src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Tables/Columns/Datetime/Create.php (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings 
Learnt from: ItzNotABug
Repo: appwrite/appwrite PR: 9693
File: src/Appwrite/Platform/Modules/Databases/Http/Indexes/Get.php:54-54
Timestamp: 2025-05-05T03:49:58.171Z
Learning: In Appwrite's API handlers, path parameters can use `null` as the default value rather than empty strings, as this pattern is consistent with existing code in the codebase.
🧬 Code graph analysis (2)
src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Attributes/Datetime/Create.php (1)
src/Appwrite/GraphQL/Types/Mapper.php (1)
  • param (255-363)
src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Tables/Columns/Datetime/Create.php (1)
src/Appwrite/GraphQL/Types/Mapper.php (1)
  • param (255-363)
⏰ 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: Benchmark
  • GitHub Check: scan
🔇 Additional comments (4)
src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Attributes/Datetime/Create.php (2)

20-20: LGTM: Import added for Nullable validator.

The import is necessary and correctly placed for wrapping the DatetimeValidator.

67-67: Code change is consistent with all other attribute types.

Verification confirms that all 13 attribute types (Boolean, Datetime, Email, Enum, Float, IP, Integer, Line, Point, Polygon, String, URL, Relationship) consistently use new Nullable(...) to wrap their default parameter validators, with param('default', null, ...) for all cases. The Datetime/Create.php change at line 67 follows this established pattern without deviation.

src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Tables/Columns/Datetime/Create.php (2)

16-16: LGTM: Import added for Nullable validator.

The import is necessary because this class overrides the constructor completely, so it cannot rely on the parent class's import.

59-59: Fix correctly mirrors the attribute change for columns.

The validator change is consistent with the Collections/Attributes/Datetime/Create.php change, ensuring null default values are permitted for datetime columns as well.

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
Copy Markdown

github-actions bot commented Nov 14, 2025

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
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
github.com/containerd/containerd/v2 v2.0.2 CVE-2024-25621 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
stdlib 1.22.10 CVE-2025-58183 HIGH
stdlib 1.22.10 CVE-2025-58186 HIGH
stdlib 1.22.10 CVE-2025-58187 HIGH
stdlib 1.22.10 CVE-2025-58188 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 14, 2025

✨ Benchmark results

  • Requests per second: 1,172
  • Requests with 200 status code: 211,082
  • P99 latency: 0.165987724

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,172 1,286
200 211,082 231,574
P99 0.165987724 0.157087283

@loks0n loks0n merged commit 99d16df into 1.8.x Nov 14, 2025
41 of 42 checks passed
@stnguyen90 stnguyen90 mentioned this pull request Dec 5, 2025
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@loks0n loks0n loks0n approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ChiragAgg5k @loks0n