chore: essential doctypes #2
Conversation
WalkthroughReplaces the legacy "Chat Log" DocType with a new "NextGenie Chat Log" DocType, removes the old DocType JSON, renames the server Document class and its test class, and updates a commented client-script target name. Changes
Sequence Diagram(s)(omitted — changes are schema/class renames and do not alter runtime control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.py (1)
4-9: Add server-side validation and derive is_botUI depends_on doesn’t enforce data integrity. Validate that
useris set whensender == "User"and auto-deriveis_botfromsender.Apply this diff:
-# import frappe +import frappe +from frappe import _ @@ -class NextGenieChatLog(Document): - pass +class NextGenieChatLog(Document): + def validate(self): + if self.sender == "User" and not self.user: + frappe.throw(_("User is required when Sender is 'User'")) + # derive is_bot from sender to keep data consistent + self.is_bot = 1 if self.sender == "Genie" else 0
🧹 Nitpick comments (5)
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json (3)
18-26: Index frequent filters (optional)If volume grows, add DB indexes for
sender(anduserbelow) to speed list filters.Apply this diff (supported on recent Frappe versions):
{ "default": "User", "fieldname": "sender", "fieldtype": "Select", "in_list_view": 1, "in_standard_filter": 1, + "index": 1, "label": "Sender", "options": "User\nGenie", "read_only": 1 },And for
user:{ "depends_on": "eval: doc.sender == \"User\"", "fieldname": "user", "fieldtype": "Link", "in_list_view": 1, "in_standard_filter": 1, + "index": 1, "label": "User", "options": "User" },
7-15: Avoid duplicating built-in timestamp
chat_creationduplicates standardcreation. Prefer the built-in to reduce drift.Apply this diff to remove the field and order entry:
"field_order": [ "sender", - "user", - "column_break_yhtw", - "chat_creation", + "user", + "column_break_yhtw", "is_bot", "section_break_wkcf", "message" ], @@ - { - "fieldname": "chat_creation", - "fieldtype": "Datetime", - "label": "Chat Creation" - },If you need a custom timestamp (e.g., external event time), keep it but rename to
event_tsand document semantics.Also applies to: 31-35
1-8: Prefer hashed autoname for logsPrevents enumeration and leaks via sequential names.
Apply this diff:
{ "actions": [], "allow_rename": 1, "creation": "2025-08-27 19:48:34.122664", "doctype": "DocType", "engine": "InnoDB", + "autoname": "hash",(Combine with the earlier change to disallow rename.)
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.js (1)
4-8: Consider removing the unused client scriptIf no client logic is planned, drop this file to avoid dead scaffolding.
nextgenie/nextgenie/doctype/nextgenie_chat_log/test_nextgenie_chat_log.py (1)
8-9: Add minimal tests to prevent regressionsStub is fine to land, but add a couple of creation/validation tests.
Apply this diff:
class TestNextGenieChatLog(FrappeTestCase): - pass + def test_user_required_when_sender_user(self): + import frappe + doc = frappe.get_doc({ + "doctype": "NextGenie Chat Log", + "sender": "User", + "message": "Hello" + }) + with self.assertRaises(Exception): + doc.insert() + + def test_genie_sets_is_bot(self): + import frappe + doc = frappe.get_doc({ + "doctype": "NextGenie Chat Log", + "sender": "Genie", + "message": "Hi, I'm Genie!" + }) + doc.insert(ignore_permissions=True) + try: + self.assertEqual(doc.is_bot, 1) + finally: + doc.delete()(Assumes the validation added in the DocType class.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
nextgenie/nextgenie/doctype/chat_log/chat_log.json(0 hunks)nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.js(1 hunks)nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json(1 hunks)nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.py(1 hunks)nextgenie/nextgenie/doctype/nextgenie_chat_log/test_nextgenie_chat_log.py(1 hunks)
💤 Files with no reviewable changes (1)
- nextgenie/nextgenie/doctype/chat_log/chat_log.json
🔇 Additional comments (3)
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.py (1)
8-9: Rename aligns with DocType — LGTMClass name matches "NextGenie Chat Log". No functional changes; safe.
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json (1)
69-84: Resolve read-only vs. create/write permissionsDocType is
read_only: 1whilein_create: 1and full perms are granted. Clarify intent: is this log append-only (via backend) or editable in Desk?If append-only, consider:
-"read_only": 1, -"in_create": 1, +"read_only": 1, +"in_create": 0,If Desk creation/editing is intended, set:
-"read_only": 1, +"read_only": 0,nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.js (1)
4-8: Binding rename matches DocType — LGTMStill commented; no runtime effect.
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json
Outdated
Show resolved
Hide resolved
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json (2)
1-86: Migrate data from old “Chat Log” to the new DocTypeAvoid data loss after the rename/removal by adding an idempotent migration patch that copies rows from
tabChat LogtotabNextGenie Chat Logwith field mapping.I can draft
nextgenie/patches/migrate_chat_logs.pyand register it inhooks.py. Want me to open a patch PR?
1-5: Add autoname and explicitly disallow renames for immutable logsAvoid manual naming and prevent post-facto renames to preserve audit trails.
"doctype": "DocType", "engine": "InnoDB", + "autoname": "hash", + "allow_rename": 0,
🧹 Nitpick comments (2)
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json (2)
17-25: Make sender required and index it; keep UI read-only if set by backendEnsures every row has a sender and speeds common filters.
{ "default": "User", "fieldname": "sender", "fieldtype": "Select", "in_list_view": 1, "in_standard_filter": 1, "label": "Sender", "options": "User\nGenie", - "read_only": 1 + "read_only": 1, + "reqd": 1, + "index": 1 },If logs can be authored manually, remove
"read_only": 1so “Genie” can be selected.
31-34: Lock chat timestamp and index itLeverage server-set creation time; avoid edits.
{ "fieldname": "chat_creation", "fieldtype": "Datetime", - "label": "Chat Creation" + "label": "Chat Creation", + "default": "Now", + "set_only_once": 1, + "read_only": 1, + "index": 1 },If this duplicates the built-in
creationfield intentionally (e.g., imported chats), keep as above; otherwise consider removing and usingcreation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json(1 hunks)
🔇 Additional comments (2)
nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json (2)
64-66: Module naming is consistent; no changes required. Module field uses “Nextgenie” (matching the app foldernextgenie), and display titles use “NextGenie” branding—intentional and correct.
45-49: Remove redundantis_botfield
Search withrg -nPI '\bis_bot\b' --glob '!**/dist/**' --glob '!**/node_modules/**'returns only its declaration in this JSON, with no other references. Remove the field to prevent future divergence.- { - "default": "0", - "fieldname": "is_bot", - "fieldtype": "Check", - "label": "Is Bot?" - },Likely an incorrect or invalid review comment.
| "depends_on": "eval: doc.sender == \"User\"", | ||
| "fieldname": "user", | ||
| "fieldtype": "Link", | ||
| "in_list_view": 1, | ||
| "in_standard_filter": 1, | ||
| "label": "User", | ||
| "options": "User" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make user mandatory when sender is “User” and add an index
Prevents orphaned rows and speeds lookups.
{
"depends_on": "eval: doc.sender == \"User\"",
"fieldname": "user",
"fieldtype": "Link",
"in_list_view": 1,
"in_standard_filter": 1,
"label": "User",
- "options": "User"
+ "options": "User",
+ "mandatory_depends_on": "eval: doc.sender == \"User\"",
+ "index": 1
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "depends_on": "eval: doc.sender == \"User\"", | |
| "fieldname": "user", | |
| "fieldtype": "Link", | |
| "in_list_view": 1, | |
| "in_standard_filter": 1, | |
| "label": "User", | |
| "options": "User" | |
| }, | |
| { | |
| "depends_on": "eval: doc.sender == \"User\"", | |
| "fieldname": "user", | |
| "fieldtype": "Link", | |
| "in_list_view": 1, | |
| "in_standard_filter": 1, | |
| "label": "User", | |
| "options": "User", | |
| "mandatory_depends_on": "eval: doc.sender == \"User\"", | |
| "index": 1 | |
| }, |
🤖 Prompt for AI Agents
In nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json around
lines 36 to 43, the "user" Link field needs to be required when sender == "User"
and indexed; change its requirement to an eval expression such as setting
"reqd": "eval: doc.sender == 'User'" (so the field is mandatory only for
user-sent messages) and add "index": 1 to the field definition to create a DB
index for faster lookups.
| "permissions": [ | ||
| { | ||
| "create": 1, | ||
| "delete": 1, | ||
| "email": 1, | ||
| "export": 1, | ||
| "print": 1, | ||
| "read": 1, | ||
| "report": 1, | ||
| "role": "System Manager", | ||
| "share": 1, | ||
| "write": 1 | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Tighten permissions and creation flow for audit integrity
If this is a system-generated log, UI edits/deletes should be disallowed and quick-create disabled.
"permissions": [
{
- "create": 1,
- "delete": 1,
+ "create": 0,
+ "delete": 0,
"email": 1,
"export": 1,
"print": 1,
"read": 1,
"report": 1,
"role": "System Manager",
"share": 1,
- "write": 1
+ "write": 0
}
],
- "read_only": 1,
+ "read_only": 1,
@@
- "in_create": 1,
+ "in_create": 0,If you intend to allow UI creation/editing, drop the doctype-level "read_only": 1 instead and keep granular field protections. Please confirm intent.
Also applies to: 81-81, 60-60
🏁 Script executed:
#!/bin/bash
# Locate the JSON definition for the nextgenie_chat_log doctype
FILE=$(fd nextgenie_chat_log.json -t f)
echo "Found file: $FILE"
# Show the header and metadata up through the permissions block
sed -n '1,200p' "$FILE"
# Show lines 60–100 to capture the in_create and read_only settings
sed -n '60,100p' "$FILE"Length of output: 2530
🏁 Script executed:
#!/bin/bash
# Check for quick_entry in the nextgenie_chat_log doctype
grep -n '"quick_entry"' nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json || echo "no quick_entry found"Length of output: 147
Tighten NextGenie Chat Log permissions
Update nextgenie_chat_log.json to lock down UI creation/editing:
--- nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json
@@ "permissions": [
- "create": 1,
- "delete": 1,
+ "create": 0,
+ "delete": 0,
@@
- "write": 1
+ "write": 0
@@
- "in_create": 1,
+ "in_create": 0,The existing "read_only": 1 already prevents UI edits; if you need UI creation/editing instead, remove it. quick_entry is unset (defaults to disabled). Confirm your intended usage.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "permissions": [ | |
| { | |
| "create": 1, | |
| "delete": 1, | |
| "email": 1, | |
| "export": 1, | |
| "print": 1, | |
| "read": 1, | |
| "report": 1, | |
| "role": "System Manager", | |
| "share": 1, | |
| "write": 1 | |
| } | |
| "permissions": [ | |
| { | |
| - "create": 1, | |
| "create": 0, | |
| "delete": 0, | |
| "email": 1, | |
| "export": 1, | |
| "print": 1, | |
| "read": 1, | |
| "report": 1, | |
| "role": "System Manager", | |
| "share": 1, | |
| "write": 0 | |
| } | |
| ], | |
| "read_only": 1, | |
| "in_create": 0, |
🤖 Prompt for AI Agents
In nextgenie/nextgenie/doctype/nextgenie_chat_log/nextgenie_chat_log.json around
lines 67–79, the permissions block needs to explicitly reflect intended UI
creation/edit behavior: if you want to lock down UI creation/editing, add
"read_only": 1 to this permission object; if instead you intend to allow UI
creation/editing remove any "read_only" key. Leave "quick_entry" unset (it
defaults to disabled) or set it explicitly to 0 if you want to be explicit.
Ensure the JSON object remains valid after the change.
Summary by CodeRabbit
New Features
Refactor
Tests