pre_meeting_memo_html db update #946
Conversation
📝 WalkthroughWalkthroughA new optional field, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JS_Bindings
participant Rust_Backend
participant Database
Client->>JS_Bindings: Create/Update Session (with optional pre_meeting_memo_html)
JS_Bindings->>Rust_Backend: upsert_session(Session)
Rust_Backend->>Database: INSERT/UPDATE sessions (including pre_meeting_memo_html)
Database-->>Rust_Backend: Confirmation/Result
Rust_Backend-->>JS_Bindings: Session (with pre_meeting_memo_html)
JS_Bindings-->>Client: Session (with pre_meeting_memo_html)
sequenceDiagram
participant Rust_Backend
participant Database
Rust_Backend->>Database: ALTER TABLE sessions ADD COLUMN pre_meeting_memo_html TEXT
Database-->>Rust_Backend: Migration Complete
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
🔭 Outside diff range comments (1)
crates/db-user/src/tags_ops.rs (1)
80-85:list_session_tagsreturns wrong result set – missing join with bridge table
tagsdoes not containsession_id; the relationship is modelled viatags_sessions.
Current query silently returns an empty set for every call.- let mut rows = conn - .query( - "SELECT * FROM tags WHERE session_id = ?", - vec![session_id.into()], - ) + let mut rows = conn + .query( + "SELECT t.* \ + FROM tags_sessions ts \ + JOIN tags t ON t.id = ts.tag_id \ + WHERE ts.session_id = ?", + vec![session_id.into()], + )
🧹 Nitpick comments (4)
crates/db-user/src/lib.rs (1)
131-151: Avoid manual array-length bookkeeping forMIGRATIONSEvery new migration forces an update of the array length literal (
18). Switching to a slice eliminates this maintenance step and prevents mismatches.-// Append only. Do not reorder. -const MIGRATIONS: [&str; 18] = [ +// Append only. Do not reorder. +const MIGRATIONS: &[&str] = &[ include_str!("./calendars_migration.sql"), @@ include_str!("./sessions_migration_3.sql"), include_str!("./sessions_migration_4.sql"), ];No other code changes are needed – the slice still supports
.to_vec().crates/db-user/src/sessions_ops.rs (2)
19-20: Simplify NULL / empty-string predicate
(pre_meeting_memo_html IS NULL OR pre_meeting_memo_html = '')is correct but verbose.
UsingCOALESCEkeeps the pattern consistent with other predicates and is slightly clearer:- (pre_meeting_memo_html IS NULL OR pre_meeting_memo_html = '') AND + COALESCE(pre_meeting_memo_html, '') = '' AND
203-205: Manual column duplication makes the UPSERT brittleEvery time a field is added/removed you now have to touch three separate lists (INSERT columns, VALUES, UPDATE set, param map).
This has already bitten us withrecord_end/pre_meeting_memo_html. A miss will silently insertNULLor fail at runtime.Consider:
- Building the SQL with a helper that joins a single slice of field names.
- Or switching to a query-builder/ORM (e.g.
sea-query) that derives the lists for you.Keeps migrations and code in sync and avoids subtle production inconsistencies.
(Not blocking for this PR, but worth a quick backlog ticket.)Also applies to: 217-218, 232-233, 247-248
crates/db-user/src/init.rs (1)
474-475: Keepnew_default_sessionin sync withSessionstructYou added
pre_meeting_memo_html: None, which is exactly what we need.
As the struct evolves this helper must always be updated; consider derivingDefaultonSessionand usingSession::default()to prevent drift.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.cargo/config.toml(0 hunks)crates/db-user/src/extensions_types.rs(0 hunks)crates/db-user/src/init.rs(2 hunks)crates/db-user/src/lib.rs(2 hunks)crates/db-user/src/sessions_migration_4.sql(1 hunks)crates/db-user/src/sessions_ops.rs(6 hunks)crates/db-user/src/sessions_types.rs(2 hunks)crates/db-user/src/tags_ops.rs(1 hunks)plugins/db/js/bindings.gen.ts(1 hunks)
💤 Files with no reviewable changes (2)
- .cargo/config.toml
- crates/db-user/src/extensions_types.rs
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,ts,tsx,rs}`: 1. No error handling. 2. No unused imports, variables, or functions. 3. For comments, keep it minimal. It should be about "Why", not "What".
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
crates/db-user/src/tags_ops.rscrates/db-user/src/lib.rsplugins/db/js/bindings.gen.tscrates/db-user/src/sessions_ops.rscrates/db-user/src/sessions_types.rscrates/db-user/src/init.rs
🧬 Code Graph Analysis (2)
plugins/db/js/bindings.gen.ts (1)
plugins/listener/js/bindings.gen.ts (1)
Word(77-77)
crates/db-user/src/init.rs (3)
plugins/db/js/bindings.gen.ts (1)
Session(159-159)crates/db-user/src/sessions_ops.rs (1)
onboarding_session_id(7-9)plugins/db/src/commands/sessions.rs (1)
onboarding_session_id(4-17)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ci
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (5)
crates/db-user/src/sessions_migration_4.sql (1)
1-4: Migration looks good – column appended safelyAdding the nullable
TEXTcolumn at the tail of the table preserves the positional order of existing columns, so no index offsets are introduced.
No further action needed.crates/db-user/src/tags_ops.rs (1)
126-126: Test data kept in sync with new struct fieldSetting
pre_meeting_memo_html: Nonekeeps the test compilation-safe after the struct change. 👍plugins/db/js/bindings.gen.ts (1)
159-160: Generated bindings correctly propagated – no actionThe new optional field is present and typed as
string | null. Looks consistent with the Rust definition.crates/db-user/src/sessions_ops.rs (1)
401-404: 👍 Assertion for the new field looks goodThe additional test ensures round-tripping of
pre_meeting_memo_html; coverage for the new column is now in place.crates/db-user/src/init.rs (1)
67-68: Nice reduction of boilerplate withnew_default_sessionReplacing the hand-written field list with struct-update syntax keeps the onboarding logic concise and future-proof.
Also applies to: 75-76
No description provided.