fix: add missing created_at column to templates table#1600
fix: add missing created_at column to templates table#1600dmfilipenko wants to merge 1 commit intofastrepl:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a new migration that adds a non-null Changes
Sequence Diagram(s)(omitted — changes are schema/struct additions with no new runtime control-flow to diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
crates/db-user/src/lib.rs(2 hunks)crates/db-user/src/templates_migration_2.sql(1 hunks)crates/db-user/src/templates_ops.rs(3 hunks)crates/db-user/src/templates_types.rs(2 hunks)
🔇 Additional comments (6)
crates/db-user/src/lib.rs (1)
145-145: LGTM!The migration is correctly registered by incrementing the array size and appending the new migration file. This follows the append-only policy stated in the comment.
Also applies to: 173-173
crates/db-user/src/templates_types.rs (2)
12-12: LGTM!The
created_atfield is appropriately added to track template creation time.
39-41: LGTM with RFC3339 fallback.The column index 7 is correct for
created_at, and the fallback to RFC3339-formatted current time provides good backward compatibility. However, ensure the migration default format matches this RFC3339 format (see comment ontemplates_migration_2.sql).crates/db-user/src/templates_ops.rs (3)
34-35: LGTM!The
created_atfield is correctly added to the INSERT statement columns, VALUES placeholders, and named parameters. This ensures new templates receive the creation timestamp.Also applies to: 43-44, 60-60
45-51: LGTM: Creation timestamp correctly preserved on update.The ON CONFLICT DO UPDATE clause intentionally omits
created_at, which is correct behavior—creation timestamps should not be modified when updating existing templates.
109-110: LGTM!The test correctly uses RFC3339 format for
created_at, consistent with the format used intemplates_types.rs.
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @dmfilipenko. * #1600 (comment) The following files were modified: * `crates/db-user/src/lib.rs` * `crates/db-user/src/templates_ops.rs` * `crates/db-user/src/templates_types.rs`
## Problem
Templates table schema was missing the `created_at` column that the
frontend TinyBase store expected. This caused templates to fail loading
from SQLite into TinyBase, making user-created templates invisible in
the UI dropdown.
### Schema Mismatch:
- Backend SQL: Missing `created_at` column
- Backend Rust: Missing `created_at` field in Template struct
- Frontend TypeScript: Expected `created_at` field in TinyBase schema
## Solution
1. Created migration `templates_migration_2.sql` with proper ISO 8601 format
- Uses `strftime('%Y-%m-%dT%H:%M:%SZ', 'now')` for correct formatting
- Auto-populates `created_at` via DEFAULT (database-managed)
2. Updated Template struct with `created_at: String` field
- Added `#[serde(default)]` for backward compatibility
- Added `default_created_at()` helper for consistent timestamp generation
3. Updated `from_row()` to read `created_at` from column 7 with fallback
4. Updated `upsert_template()` to let SQLite manage `created_at`
- Removed manual `created_at` parameter (prevents timestamp forgery)
- Database now controls creation timestamps (audit-safe)
5. Updated test to use `default_created_at()` helper
6. Registered new migration in MIGRATIONS array
## Impact
- New templates have `created_at` automatically set by database
- Existing templates get `created_at` via migration
- TinyBase can now successfully load templates from SQLite
- Templates appear correctly in UI dropdown
- Timestamps are immutable and database-controlled (secure)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
dmfilipenko
force-pushed
the
fix/templates-created-at-schema
branch
from
21912cb to
9d2044a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/db-user/src/templates_ops.rs (1)
22-65: Critical:created_atfield is not persisted in the database.The
upsert_templatefunction accepts aTemplatestruct that now includes acreated_atfield (as added in this PR), but the SQL statement does not includecreated_atin either the INSERT columns or the UPDATE clause. This means:
- The
created_atvalue passed to this function is silently ignored- Only the migration's default value will be used for new rows
- Existing templates will never have their
created_atupdatedIf
created_atshould be set explicitly on INSERT (and optionally on UPDATE), add it to the SQL statement:"INSERT INTO templates ( id, user_id, title, description, sections, tags, - context_option + context_option, + created_at ) VALUES ( :id, :user_id, :title, :description, :sections, :tags, - :context_option + :context_option, + :created_at ) ON CONFLICT(id) DO UPDATE SET title = :title, description = :description, sections = :sections, tags = :tags, - context_option = :context_option + context_option = :context_option, + created_at = :created_at RETURNING *", libsql::named_params! { ":id": template.id, ":user_id": template.user_id, ":title": template.title, ":description": template.description, ":sections": serde_json::to_string(&template.sections).unwrap(), ":tags": serde_json::to_string(&template.tags).unwrap(), ":context_option": template.context_option.as_deref().unwrap_or(""), + ":created_at": template.created_at, },Note: If
created_atshould be immutable after creation, remove it from theON CONFLICT DO UPDATE SETclause and only include it in the INSERT.
🧹 Nitpick comments (1)
crates/db-user/src/templates_ops.rs (1)
80-113: Test doesn't verifycreated_atpersistence.While the test correctly provides a
created_atvalue when constructing the Template, it doesn't verify that this value is actually persisted to and retrieved from the database. Due to the SQL bug identified above (missingcreated_atin the INSERT statement), this test would pass even though the explicitcreated_atvalue is being ignored.Consider adding an assertion to verify the
created_atvalue:let templates = db.list_templates(&human.id).await.unwrap(); assert_eq!(templates.len(), 1); +assert_eq!(templates[0].created_at, _template.created_at);This would catch the SQL bug and ensure
created_atis properly round-tripped through the database.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
crates/db-user/src/lib.rs(2 hunks)crates/db-user/src/templates_migration_2.sql(1 hunks)crates/db-user/src/templates_ops.rs(1 hunks)crates/db-user/src/templates_types.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/db-user/src/templates_types.rs
- crates/db-user/src/templates_migration_2.sql
🧰 Additional context used
🧬 Code graph analysis (1)
crates/db-user/src/templates_ops.rs (2)
plugins/db/js/bindings.gen.ts (1)
Template(202-202)crates/db-user/src/templates_types.rs (1)
default_created_at(25-27)
🔇 Additional comments (1)
crates/db-user/src/lib.rs (1)
145-145: Migration registration and implementation verified.The migration file exists and correctly adds the
created_atcolumn with a TEXT type, NOT NULL constraint, and ISO 8601 timestamp default value. The MIGRATIONS array size of 28 is accurate with the new entry appended to the previous 27.
|
|
||
| // Append only. Do not reorder. | ||
| const MIGRATIONS: [&str; 27] = [ | ||
| const MIGRATIONS: [&str; 28] = [ |
There was a problem hiding this comment.
🧩 Analysis chain
Verify TypeScript bindings include the new created_at field.
The relevant code snippets show that the frontend TypeScript type definition for Template (in plugins/db/js/bindings.gen.ts line 201) does not include the created_at field that was added to the backend Rust struct. This inconsistency will cause type errors or runtime issues when the frontend tries to use the created_at field.
Check if the TypeScript bindings need to be regenerated:
🏁 Script executed:
#!/bin/bash
# Description: Check TypeScript Template type definition and look for binding generation commands
# Check current Template type definition
echo "=== Current TypeScript Template type ==="
rg -A2 "export type Template = " plugins/db/js/bindings.gen.ts
# Look for binding generation scripts or commands
echo ""
echo "=== Looking for binding generation commands ==="
rg -n "bindings.gen|specta|ts-rs" --type-add 'toml:*.toml' --type toml
rg -n "bindings.gen|specta|ts-rs" package.json 2>/dev/null || echo "No package.json commands found"
# Check if there's a build script or makefile
fd -t f "Makefile|justfile|build.rs" .Length of output: 10746
Let me verify the backend struct definition and the build process:
Now let me check the plugins/db/build.rs to see how bindings are generated:
Now I need to check the actual Template struct definition and the build.rs file:
---
🏁 Script executed:
```shell
#!/bin/bash
# Look for Template struct definition with specta annotations
echo "=== Searching for Template struct definition ==="
rg -n "struct Template" -A5 --type rs
echo ""
echo "=== Check if Template has specta::Type derive ==="
rg -n "#\[derive.*Type" -B2 --type rs | grep -A2 "Template"
echo ""
echo "=== Check plugins/db/build.rs for binding generation ==="
cat ./plugins/db/build.rs | head -50
Length of output: 1379
---
🏁 Script executed:
```shell
#!/bin/bash
# Corrected approach to find Template struct
echo "=== Find files containing 'struct Template' ==="
fd -e rs -x rg -l "struct Template" {} \;
echo ""
echo "=== Search for Template struct definition ==="
rg -n "struct Template" -A10
echo ""
echo "=== Check build.rs for specta binding generation ==="
cat ./plugins/db/build.rs | grep -A20 "specta\|binding\|export"
echo ""
echo "=== Look for Template in plugins/db/src ==="
find ./plugins/db/src -name "*.rs" -exec grep -l "Template" {} \;
Length of output: 1773
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full struct definition with all derives and attributes
echo "=== Full templates_types.rs file ==="
cat ./crates/db-user/src/templates_types.rs
echo ""
echo "=== Check if templates_types is exported from lib.rs ==="
rg -n "templates_types|pub.*Template" ./crates/db-user/src/lib.rs | head -20
echo ""
echo "=== Check how bindings are generated in plugins/db/build.rs ==="
tail -100 ./plugins/db/build.rs | head -50
Length of output: 2809
---
🏁 Script executed:
```shell
#!/bin/bash
# Find user_common_derives macro definition
echo "=== Search for user_common_derives! macro definition ==="
rg -n "macro_rules.*user_common_derives" -A20
echo ""
echo "=== Search for user_common_derives in use statements ==="
rg -n "user_common_derives" ./crates/db-user/src/lib.rs
echo ""
echo "=== Full plugins/db/build.rs to understand binding export ==="
cat ./plugins/db/build.rs
Length of output: 3069
Regenerate TypeScript bindings to include the new created_at field.
The backend Template struct in crates/db-user/src/templates_types.rs now includes the created_at: String field and is properly annotated with specta::Type via the user_common_derives! macro. However, the TypeScript bindings in plugins/db/js/bindings.gen.ts do not include this field, indicating the bindings are out of sync. Rebuild the project (e.g., cargo build) to regenerate the TypeScript bindings from the updated Rust types.
🤖 Prompt for AI Agents
In crates/db-user/src/lib.rs around line 145, the TypeScript bindings are out of
date and missing the new Template.created_at field; rebuild the Rust crate so
the specta-derived types regenerate the TS output. Run a full cargo build (or
cargo build -p db-user) to run the bindings generation step, then verify
plugins/db/js/bindings.gen.ts includes created_at: string and commit the updated
bindings file.
|
thanks for the contribution. unfortunately, we are doing rewrite of our data layer and created_at is already included there. |
2 participants
Problem
Templates table schema was missing the
created_atcolumn that the frontend TinyBase store expected. This caused templates to fail loading from SQLite into TinyBase, making user-created templates invisible in the UI dropdown.Schema Mismatch:
created_atcolumncreated_atfield in Template structcreated_atfield in TinyBase schemaSolution
templates_migration_2.sqlto addcreated_atcolumn with auto-populated default valuecreated_at: Stringfieldfrom_row()to readcreated_atfrom column 7upsert_template()to insert/updatecreated_atImpact
created_atautomatically setcreated_atvia migration🤖 Generated with Claude Code