fix: doctype rename#3
Conversation
📝 WalkthroughWalkthroughRegisters a pre-model-sync patch to rename the DocType "Razorpay Settings" to "Frappe Razorpay Settings", updates corresponding doctype JSON, backend class, client binding and tests, and includes small formatting tweaks and CI runtime updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant F as Frappe (framework)
participant Patch as Rename Patch
participant Reg as DocType Registry
participant DB as Database
F->>Patch: run pre_model_sync patches
activate Patch
Patch->>Reg: check existence of "Razorpay Settings"
Reg-->>Patch: exists / not exists
alt target "Frappe Razorpay Settings" absent
Patch->>F: call frappe.rename_doc("Razorpay Settings" -> "Frappe Razorpay Settings")
F->>DB: rename metadata & records
DB-->>F: rename complete
F-->>Patch: rename succeeded
else target exists
Patch-->>Patch: skip rename (target already present)
end
Patch->>DB: drop existing sequence if present
DB-->>Patch: dropped / not found
Patch->>DB: query max(name) from "Frappe Razorpay Settings"
DB-->>Patch: max_id
Patch->>DB: create sequence start = max_id + 1
DB-->>Patch: sequence created
deactivate Patch
Patch-->>F: patch finished
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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). (1)
🔇 Additional comments (2)
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: 3
🧹 Nitpick comments (1)
razorpay_frappe/patches/rename_doctypes.py (1)
22-22: Use the doctypes mapping instead of hardcoding the doctype name.For better maintainability and consistency, use the
doctypesmapping defined at the top of the file rather than hardcoding the doctype name.🔎 Proposed refactor
def create_sequences(): - doctype = "Frappe Razorpay Settings" + doctype = list(doctypes.values())[0] # Or iterate if handling multiple doctypes sequence_name = frappe.scrub(doctype + "_id_seq")Alternatively, if you plan to support multiple doctypes in the future:
def create_sequences(): - doctype = "Frappe Razorpay Settings" - sequence_name = frappe.scrub(doctype + "_id_seq") - frappe.db.sql_ddl(f"drop sequence if exists {sequence_name}") - result = frappe.db.get_all(doctype, fields=["max(name) as last"]) - last_name = result[0].last if result else None - start_value = (last_name or 0) + 1 - print(f"Creating sequence for {doctype} starting at {start_value}") - frappe.db.create_sequence(doctype, start_value=start_value, check_not_exists=True) + for doctype in doctypes.values(): + sequence_name = frappe.scrub(doctype + "_id_seq") + frappe.db.sql_ddl(f"drop sequence if exists {sequence_name}") + result = frappe.db.get_all(doctype, fields=["max(name) as last"]) + last_name = result[0].last if result else None + start_value = (last_name or 0) + 1 + print(f"Creating sequence for {doctype} starting at {start_value}") + frappe.db.create_sequence(doctype, start_value=start_value, check_not_exists=True)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
razorpay_frappe/patches.txtrazorpay_frappe/patches/__init__.pyrazorpay_frappe/patches/rename_doctypes.pyrazorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/__init__.pyrazorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/frappe_razorpay_settings.jsrazorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/frappe_razorpay_settings.jsonrazorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/frappe_razorpay_settings.pyrazorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/test_frappe_razorpay_settings.py
🔇 Additional comments (7)
razorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/frappe_razorpay_settings.json (1)
47-47: LGTM: DocType name updated correctly.The name field has been properly updated to "Frappe Razorpay Settings", consistent with the rename operation across the codebase.
razorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/frappe_razorpay_settings.js (1)
4-4: LGTM: Client-side binding updated correctly.The form binding has been properly updated to match the renamed DocType.
razorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/test_frappe_razorpay_settings.py (1)
8-8: LGTM: Test class renamed consistently.The test class has been properly renamed to align with the renamed DocType class.
razorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/frappe_razorpay_settings.py (1)
8-8: LGTM: DocType class renamed correctly.The class has been properly renamed to
FrappeRazorpaySettings, maintaining consistency with the renamed DocType.razorpay_frappe/patches/rename_doctypes.py (3)
1-5: LGTM!The import and doctype mapping are clear and straightforward.
8-10: LGTM!The execution order is correct—rename the DocType first, then manage sequences.
21-28: Remove sequence creation for "Frappe Razorpay Settings" — it is a Single DocType.The DocType is configured as a Single (
issingle: 1), which means it has only one document instance with a fixed name. Single DocTypes do not use auto-incrementing sequences for naming. The sequence creation logic in lines 21-28 is inappropriate and should be removed from the patch.
| @@ -1,4 +1,5 @@ | |||
| [pre_model_sync] | |||
| razorpay_frappe.patches.rename_doctypes | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the patches.txt file and verify its location
find . -name "patches.txt" -type fRepository: BuildWithHussain/razorpay_frappe
Length of output: 105
🏁 Script executed:
# Search for the patch implementation file
find . -name "rename_doctypes.py" -type fRepository: BuildWithHussain/razorpay_frappe
Length of output: 120
🏁 Script executed:
# Search for any remaining references to "Razorpay Settings" (old doctype name)
rg -n "Razorpay Settings" --type=py --type=js --type=json --type=txtRepository: BuildWithHussain/razorpay_frappe
Length of output: 1519
🏁 Script executed:
# Search for RazorpaySettings class references
rg -n '\bRazorpaySettings\b' --type=pyRepository: BuildWithHussain/razorpay_frappe
Length of output: 58
🏁 Script executed:
# Read the patch implementation file
cat -n ./razorpay_frappe/patches/rename_doctypes.pyRepository: BuildWithHussain/razorpay_frappe
Length of output: 1078
🏁 Script executed:
# Check the problematic references in utils.py
sed -n '25,60p' ./razorpay_frappe/utils.pyRepository: BuildWithHussain/razorpay_frappe
Length of output: 1109
🏁 Script executed:
# Check rzp_renderer.py references
sed -n '80,120p' ./razorpay_frappe/rzp_renderer.pyRepository: BuildWithHussain/razorpay_frappe
Length of output: 1128
🏁 Script executed:
# Check test file references
sed -n '50,90p' ./razorpay_frappe/razorpay_integration/doctype/razorpay_order/test_razorpay_order.pyRepository: BuildWithHussain/razorpay_frappe
Length of output: 1298
Update hardcoded doctype references to use the renamed doctype name.
The patch registration and implementation are correct, but the codebase still contains hardcoded string references to the old doctype name "Razorpay Settings". Since frappe.rename_doc() only handles database-level renames, these code references must be manually updated or the application will fail at runtime.
Update the following locations to use "Frappe Razorpay Settings":
utils.pylines 29, 38, 55rzp_renderer.pylines 85, 115test_razorpay_order.pylines 56, 68, 83razorpay.jsonline 54
🤖 Prompt for AI Agents
In razorpay_frappe/patches.txt line 2 the patch registration is present but the
codebase still references the old doctype name; update hardcoded doctype strings
"Razorpay Settings" to the renamed "Frappe Razorpay Settings" in the listed
files: in utils.py update occurrences at lines 29, 38, 55; in rzp_renderer.py
update occurrences at lines 85 and 115; in test_razorpay_order.py update
occurrences at lines 56, 68, 83; and in razorpay.json update the name at line 54
so all runtime lookups and tests use "Frappe Razorpay Settings" instead of the
old name.
| def rename_doctypes(): | ||
| for old in doctypes: | ||
| new = doctypes[old] | ||
| if not frappe.db.exists("DocType", new): | ||
| print(f"Renaming {old} to {new}") | ||
| frappe.rename_doc("DocType", old, new, force=True, ignore_if_exists=True) |
There was a problem hiding this comment.
Check if the old DocType exists before attempting rename.
The current code only checks if the new DocType exists, but doesn't verify that the old DocType exists before attempting to rename it. If the old DocType doesn't exist (e.g., on fresh installations or if the patch has already been applied), frappe.rename_doc will raise an error and block the migration.
🔎 Proposed fix
def rename_doctypes():
for old in doctypes:
new = doctypes[old]
- if not frappe.db.exists("DocType", new):
+ if frappe.db.exists("DocType", old) and not frappe.db.exists("DocType", new):
print(f"Renaming {old} to {new}")
frappe.rename_doc("DocType", old, new, force=True, ignore_if_exists=True)📝 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.
| def rename_doctypes(): | |
| for old in doctypes: | |
| new = doctypes[old] | |
| if not frappe.db.exists("DocType", new): | |
| print(f"Renaming {old} to {new}") | |
| frappe.rename_doc("DocType", old, new, force=True, ignore_if_exists=True) | |
| def rename_doctypes(): | |
| for old in doctypes: | |
| new = doctypes[old] | |
| if frappe.db.exists("DocType", old) and not frappe.db.exists("DocType", new): | |
| print(f"Renaming {old} to {new}") | |
| frappe.rename_doc("DocType", old, new, force=True, ignore_if_exists=True) |
🤖 Prompt for AI Agents
In razorpay_frappe/patches/rename_doctypes.py around lines 13 to 18, the code
checks only that the new DocType does not exist before calling
frappe.rename_doc, but does not verify that the old DocType exists; this can
raise an error if the old doctype is missing. Update the loop to first check
frappe.db.exists("DocType", old) and skip (or log) when the old DocType is
absent, and only call frappe.rename_doc when the old exists and the new does
not, so the migration is safe to re-run or run on fresh installs.
| last_name = frappe.db.get_all(doctype, fields=["max(name) as last"])[0].last | ||
| start_value = (last_name or 0) + 1 |
There was a problem hiding this comment.
Handle empty table case and verify name field type.
Two critical issues:
-
IndexError on empty table: If the DocType table is empty (no records),
get_all()returns an empty list, and accessing[0]will raise anIndexError. -
Type mismatch: If the
namefield is a string (common in Frappe DocTypes using naming series like "SETTINGS-0001"), the expression(last_name or 0) + 1will fail because you cannot add an integer to a string.
🔎 Proposed fix
frappe.db.sql_ddl(f"drop sequence if exists {sequence_name}")
- last_name = frappe.db.get_all(doctype, fields=["max(name) as last"])[0].last
- start_value = (last_name or 0) + 1
+ result = frappe.db.get_all(doctype, fields=["max(name) as last"])
+ last_name = result[0].last if result else None
+ # Handle case where name might be a string or integer
+ if last_name and isinstance(last_name, int):
+ start_value = last_name + 1
+ else:
+ start_value = 1
print(f"Creating sequence for {doctype} starting at {start_value}")
frappe.db.create_sequence(doctype, start_value=start_value, check_not_exists=True)🤖 Prompt for AI Agents
In razorpay_frappe/patches/rename_doctypes.py around lines 25-26, fix the
IndexError and type-mismatch by first checking that frappe.db.get_all(doctype,
...) returned a non-empty list and using a safe default when empty (e.g.,
last_name = None or 0); then handle string names by extracting the numeric
suffix (e.g., with a regex) or attempting int(last_name) in a try/except, and
finally compute start_value = (extracted_numeric or 0) + 1 so adding an integer
always succeeds; ensure you log or fallback to 0 on parse failure.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
razorpay_frappe/patches/rename_doctypes.py (2)
13-20: Critical: Check if old DocType exists before rename.This issue was previously flagged. The code doesn't verify the old DocType exists before attempting to rename it. On fresh installations or if the patch has already run,
frappe.rename_docwill raise an error and block the migration.🔎 Proposed fix
def rename_doctypes(): for old in doctypes: new = doctypes[old] - if not frappe.db.exists("DocType", new): + if frappe.db.exists("DocType", old) and not frappe.db.exists("DocType", new): print(f"Renaming {old} to {new}") - frappe.rename_doc("DocType", old, new, force=True, ignore_if_exists=True) + frappe.rename_doc( + "DocType", old, new, force=True, ignore_if_exists=True + )
23-32: Critical: Handle empty table and type mismatches.This issue was previously flagged. Two problems:
- IndexError: If no records exist,
get_all()[0]will raise anIndexError.- Type mismatch: If the
namefield is a string (common with naming series),(last_name or 0) + 1will fail.🔎 Proposed fix
def create_sequences(): doctype = "Frappe Razorpay Settings" sequence_name = frappe.scrub(doctype + "_id_seq") frappe.db.sql_ddl(f"drop sequence if exists {sequence_name}") - last_name = frappe.db.get_all(doctype, fields=["max(name) as last"])[0].last - start_value = (last_name or 0) + 1 + result = frappe.db.get_all(doctype, fields=["max(name) as last"]) + last_name = result[0].last if result else None + # Handle case where name might be a string or integer + if last_name and isinstance(last_name, int): + start_value = last_name + 1 + else: + start_value = 1 print(f"Creating sequence for {doctype} starting at {start_value}") - frappe.db.create_sequence(doctype, start_value=start_value, check_not_exists=True) + frappe.db.create_sequence( + doctype, start_value=start_value, check_not_exists=True + )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
razorpay_frappe/patches/rename_doctypes.pyrazorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/frappe_razorpay_settings.pyrazorpay_frappe/razorpay_integration/doctype/razorpay_order/razorpay_order.jsrazorpay_frappe/razorpay_integration/doctype/razorpay_order/razorpay_order.pyrazorpay_frappe/razorpay_integration/doctype/razorpay_payment_link/razorpay_payment_link.pyrazorpay_frappe/webhook_processor.py
💤 Files with no reviewable changes (1)
- razorpay_frappe/webhook_processor.py
🧰 Additional context used
🧬 Code graph analysis (2)
razorpay_frappe/razorpay_integration/doctype/razorpay_payment_link/razorpay_payment_link.py (2)
razorpay_frappe/utils.py (3)
convert_from_razorpay_money(48-49)get_in_razorpay_money(44-45)get_razorpay_client(26-41)razorpay_frappe/razorpay_integration/doctype/razorpay_order/razorpay_order.py (1)
sync_status(169-187)
razorpay_frappe/razorpay_integration/doctype/razorpay_order/razorpay_order.py (1)
razorpay_frappe/utils.py (1)
convert_from_razorpay_money(48-49)
🪛 Ruff (0.14.10)
razorpay_frappe/razorpay_integration/doctype/razorpay_payment_link/razorpay_payment_link.py
35-35: Undefined name Created
(F821)
35-35: Syntax error in forward annotation: Unexpected token at the end of an expression
(F722)
35-35: Undefined name Expired
(F821)
35-35: Undefined name Cancelled
(F821)
35-35: Undefined name Paid
(F821)
razorpay_frappe/razorpay_integration/doctype/razorpay_order/razorpay_order.py
44-44: Undefined name Pending
(F821)
44-44: Undefined name Failed
(F821)
44-44: Undefined name Paid
(F821)
44-44: Undefined name Refund
(F821)
44-44: Undefined name Progress
(F821)
44-44: Undefined name Refunded
(F821)
🔇 Additional comments (7)
razorpay_frappe/razorpay_integration/doctype/razorpay_order/razorpay_order.js (1)
26-29: LGTM!The semicolon addition follows JavaScript conventions and has no functional impact.
razorpay_frappe/razorpay_integration/doctype/frappe_razorpay_settings/frappe_razorpay_settings.py (1)
8-8: LGTM!The class rename from
RazorpaySettingstoFrappeRazorpaySettingscorrectly aligns with the doctype rename.razorpay_frappe/razorpay_integration/doctype/razorpay_payment_link/razorpay_payment_link.py (3)
8-12: LGTM!The multiline import formatting improves readability without changing functionality.
34-36: Static analysis false positives can be ignored.The Ruff errors (F821, F722) about undefined names are false positives. These are string literals within a
DF.Literal[]type annotation, not variable references. The syntax is correct.
80-102: LGTM!The multiline formatting of method calls improves readability. No functional changes introduced.
razorpay_frappe/razorpay_integration/doctype/razorpay_order/razorpay_order.py (2)
43-45: Static analysis false positives can be ignored.The Ruff errors (F821) about undefined names are false positives. These are string literals within a
DF.Literal[]type annotation, not variable references. The syntax is correct.
139-141: LGTM!The multiline formatting improves readability without changing functionality.
Also applies to: 176-178
| import frappe | ||
|
|
||
| doctypes = { | ||
| "Razorpay Settings": "Frappe Razorpay Settings", | ||
| } | ||
|
|
||
|
|
||
| def execute(): | ||
| rename_doctypes() | ||
| create_sequences() | ||
|
|
||
|
|
||
| def rename_doctypes(): | ||
| for old in doctypes: | ||
| new = doctypes[old] | ||
| if not frappe.db.exists("DocType", new): | ||
| print(f"Renaming {old} to {new}") | ||
| frappe.rename_doc( | ||
| "DocType", old, new, force=True, ignore_if_exists=True | ||
| ) | ||
|
|
||
|
|
||
| def create_sequences(): | ||
| doctype = "Frappe Razorpay Settings" | ||
| sequence_name = frappe.scrub(doctype + "_id_seq") | ||
| frappe.db.sql_ddl(f"drop sequence if exists {sequence_name}") | ||
| last_name = frappe.db.get_all(doctype, fields=["max(name) as last"])[0].last | ||
| start_value = (last_name or 0) + 1 | ||
| print(f"Creating sequence for {doctype} starting at {start_value}") | ||
| frappe.db.create_sequence( | ||
| doctype, start_value=start_value, check_not_exists=True | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all references to "Razorpay Settings" (excluding patches)
echo "=== All references to 'Razorpay Settings' in codebase ==="
rg -n --type=py 'Razorpay Settings' --glob '!patches' 2>/dev/null || echo "No matches found"
echo -e "\n=== Check utils.py specifically ==="
find . -name "utils.py" -type f 2>/dev/null | head -20Repository: BuildWithHussain/razorpay_frappe
Length of output: 1066
🏁 Script executed:
#!/bin/bash
# Get specific lines from utils.py
echo "=== utils.py references ==="
sed -n '25,60p' razorpay_frappe/utils.py
echo -e "\n=== rzp_renderer.py references ==="
sed -n '80,120p' razorpay_frappe/rzp_renderer.py
echo -e "\n=== test_razorpay_order.py references ==="
sed -n '50,90p' razorpay_frappe/razorpay_integration/doctype/razorpay_order/test_razorpay_order.pyRepository: BuildWithHussain/razorpay_frappe
Length of output: 3492
Update all doctype references to use the new name "Frappe Razorpay Settings".
The patch renames the doctype from "Razorpay Settings" to "Frappe Razorpay Settings", but multiple files still reference the old name and will fail at runtime:
razorpay_frappe/utils.py(lines 29, 38, 55)razorpay_frappe/rzp_renderer.py(lines 85, 115)razorpay_frappe/razorpay_integration/doctype/razorpay_order/test_razorpay_order.py(lines 56, 68, 83)
Update all 8 references to use the new doctype name.
🤖 Prompt for AI Agents
In razorpay_frappe/patches/rename_doctypes.py lines 1-32: update all external
references to the renamed DocType by replacing the old name "Razorpay Settings"
with the new name "Frappe Razorpay Settings" in the other files listed in the
review; specifically change the three occurrences in razorpay_frappe/utils.py
(lines 29, 38, 55), the two occurrences in razorpay_frappe/rzp_renderer.py
(lines 85, 115), and the three occurrences in
razorpay_frappe/razorpay_integration/doctype/razorpay_order/test_razorpay_order.py
(lines 56, 68, 83) so they exactly match the new doctype string, then run tests
to confirm no runtime lookups fail (total 8 replacements).
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/ci.yml
57-57: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (1)
- GitHub Check: Server
🔇 Additional comments (2)
.github/workflows/ci.yml (2)
54-54: LGTM! Python 3.14 is the latest stable release.Python 3.14 was officially released on October 7, 2025, and the current version is Python 3.14.2, making this a valid and up-to-date choice for CI workflows.
59-59: LGTM! Node.js 24 is the latest LTS release.Node.js 24.11.0 "Krypton" entered Long-Term Support (LTS) and will continue receiving maintenance and security updates through April 2028, making it a solid choice for production CI workflows.
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: 18 | ||
| node-version: 24 | ||
| check-latest: true |
There was a problem hiding this comment.
Upgrade actions/setup-node to v4 or later for Node.js 24 support.
The static analysis tool correctly flags that actions/setup-node@v3 is outdated. Node.js 24.12.0 release notes show the project bumping actions/setup-node from 5.0.0 to 6.0.0, indicating that newer action versions provide better support for Node.js 24.
🔎 Proposed fix
- - name: Setup Node
- uses: actions/setup-node@v3
- with:
- node-version: 24
- check-latest: true
+ - name: Setup Node
+ uses: actions/setup-node@v4
+ with:
+ node-version: 24
+ check-latest: true📝 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.
| uses: actions/setup-node@v3 | |
| with: | |
| node-version: 18 | |
| node-version: 24 | |
| check-latest: true | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: 24 | |
| check-latest: true |
🧰 Tools
🪛 actionlint (1.7.9)
57-57: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 57 to 60: the workflow pins
actions/setup-node@v3 which does not fully support Node.js 24; update the action
reference to actions/setup-node@v4 (or a later compatible major, e.g., @v6) to
ensure Node 24 support, keep the existing node-version: 24 and check-latest:
true inputs, and run the workflow locally or trigger CI to verify there are no
breaking input or behavior changes after the upgrade.
|
CI failing, |
|
@vishwajeet-13 can we close this? |
3 participants
rename Razorpay Settings to Frappe Razorpay Settings
Summary by CodeRabbit
Refactor
Chores
Tests
Style
✏️ Tip: You can customize this high-level summary in your review settings.