test: add test cases for Sketch Order Form

[DHINESH00]

PR Type

Tests

---

Description

• Implements comprehensive test cases for Sketch Order Form doctype

• Tests Sales and Purchase order creation workflows

• Tests both New Design and Modified Design order types

• Creates test fixtures for Customer, Supplier, Department, Branch, Sales Person

---

Diagram Walkthrough

flowchart LR
  TestSetup["Test Setup<br/>create_test_data"]
  Factory["Factory Function<br/>make_sketch_order_form"]
  SalesTest["test_sketch_order_created<br/>Sales Order"]
  ModDesignTest["test_sketch_order_created_mod_design<br/>Modified Design"]
  PurchaseTest["test_purchase_order_created<br/>Purchase Order"]
  Assertions["Assertions<br/>Verify Order Creation"]
  
  TestSetup --> Factory
  Factory --> SalesTest
  Factory --> ModDesignTest
  Factory --> PurchaseTest
  SalesTest --> Assertions
  ModDesignTest --> Assertions
  PurchaseTest --> Assertions

Loading

File Walkthrough

Relevant files

Tests

test_sketch_order_form.py Add comprehensive test cases for Sketch Order Form              jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py Replaced empty test class with three functional test methods covering Sales/Purchase order creation Added create_test_data() helper function to set up required test fixtures (Customer, Supplier, Department, Branch, Sales Person) Added make_sketch_order_form() factory function to create test Sketch Order Form documents with configurable parameters Implemented workflow state transitions (Send For Approval, Approve) and database commits in test setup +185/-3

---

---

🛠️ Relevant configurations:

These are the relevant configurations for this tool:

[config]

is_auto_command: True
is_new_pr: True
model_reasoning: vertex_ai/gemini-2.5-pro
model: gpt-5.2-2025-12-11
model_turbo: anthropic/claude-haiku-4-5-20251001
fallback_models: ['anthropic/claude-sonnet-4-5-20250929', 'bedrock/us.anthropic.claude-sonnet-4-5-20250929-v1:0']
second_model_for_exhaustive_mode: o4-mini
git_provider: github
publish_output: True
publish_output_no_suggestions: True
publish_output_progress: True
verbosity_level: 0
publish_logs: False
debug_mode: False
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
use_global_wiki_settings_file: False
disable_auto_feedback: False
ai_timeout: 150
response_language: en-US
clone_repo_instead_of_fetch: True
always_clone: False
add_repo_metadata: True
clone_repo_time_limit: 300
publish_inline_comments_fallback_batch_size: 5
publish_inline_comments_fallback_sleep_time: 2
max_model_tokens: 32000
custom_model_max_tokens: -1
patch_extension_skip_types: ['.md', '.txt']
extra_allowed_extensions: []
allow_dynamic_context: True
allow_forward_dynamic_context: True
max_extra_lines_before_dynamic_context: 12
patch_extra_lines_before: 5
patch_extra_lines_after: 1
ai_handler: litellm
cli_mode: False
TRIAL_GIT_ORG_MAX_INVOKES_PER_MONTH: 75
TRIAL_RATIO_CLOSE_TO_LIMIT: 0.8
invite_only_mode: False
enable_request_access_msg_on_new_pr: False
check_also_invites_field: False
calculate_context: True
disable_checkboxes: False
output_relevant_configurations: True
large_patch_policy: clip
seed: -1
temperature: 0.2
allow_dynamic_context_ab_testing: False
choose_dynamic_context_ab_testing_ratio: 0.5
ignore_pr_title: ['^\\[Auto\\]', '^Auto', '^\\[Bump\\]']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: ['do-not-merge', 'skip-qodo']
ignore_ticket_labels: []
allow_only_specific_folders: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
enable_ai_metadata: True
present_reasoning: True
max_tickets: 10
max_tickets_chars: 8000
prevent_any_approval: False
enable_comment_approval: False
enable_auto_approval: False
auto_approve_for_low_review_effort: -1
auto_approve_for_no_suggestions: False
ensure_ticket_compliance: False
new_diff_format: True
new_diff_format_add_external_references: True
enable_custom_labels: False

[pr_description]

final_update_message: False
publish_labels: False
add_original_user_description: True
generate_ai_title: False
extra_instructions: Highlight any database schema changes (Doctype/child table modifications), new background jobs (frappe.enqueue / scheduler events),
and any changes that could increase DB queries or write amplification.

enable_pr_type: True
enable_help_text: False
enable_help_comment: False
bring_latest_tag: False
enable_pr_diagram: True
publish_description_as_comment: False
publish_description_as_comment_persistent: True
enable_semantic_files_types: True
collapsible_file_list: adaptive
collapsible_file_list_threshold: 8
inline_file_summary: False
use_description_markers: False
include_generated_by_header: True
enable_large_pr_handling: True
max_ai_calls: 4
auto_create_ticket: False

[qodo-free-for-open-source-projects]

PR Reviewer Guide 🔍

(Review updated until commit 76f8094)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🏅 Score: 62

🧪 PR contains tests

✅ No TODO sections

🔒 No security concerns identified

🔀 No multiple PR themes

⚡ Recommended focus areas for review N+1 Query Pattern Lines 14-15 execute frappe.get_value inside setUpClass, which is called once per test class. While not in a loop here, this pattern is repeated in create_test_data (lines 40-99) where multiple frappe.db.exists checks are made sequentially. Consider batching these existence checks with a single frappe.get_all query to reduce database round trips. cls.department = frappe.get_value('Department',{'department_name':'Test_Department'},'name') cls.branch = frappe.get_value('Branch',{'branch_name':'Test Branch'},'name') Heavy Writes Pattern Lines 40-99 use doc.save() for test fixture creation. While acceptable in test setup, this triggers full controller validation chains. If these fixtures are created repeatedly across test runs, consider using frappe.db.set_value or direct SQL inserts for performance, or implement proper test data cleanup/reuse strategy. if not frappe.db.exists('Customer', 'Test_Customer_External'): customer = frappe.get_doc( { 'doctype': 'Customer', 'customer_name': 'Test_Customer_External', 'customer_type': 'Individual', 'custom_sketch_workflow_state': 'External' } ) customer.save() if not frappe.db.exists('Customer', 'Test_Customer_Internal'): customer = frappe.get_doc( { 'doctype': 'Customer', 'customer_name': 'Test_Customer_Internal', 'customer_type': 'Individual', 'custom_sketch_workflow_state': 'Internal' } ) customer.save() if not frappe.db.exists('Supplier', 'Test_Supplier'): supplier = frappe.get_doc( { 'doctype': 'Supplier', 'supplier_name': 'Test_Supplier' } ) supplier.save() if not frappe.db.exists('Department',{'department_name':'Test_Department'}): dep = frappe.get_doc( { 'doctype': 'Department', 'department_name': "Test_Department", "company": 'Gurukrupa Export Private Limited' } ) dep.save() if not frappe.db.exists('Branch',{'branch_name':'Test Branch'}): branch = frappe.get_doc( { 'doctype': 'Branch', 'branch': 'Test Branch', 'branch_name': 'Test Branch', 'company': 'Gurukrupa Export Private Limited' } ) branch.save() if not frappe.db.exists('Sales Person', 'Test_Sales_Person'): salesman = frappe.get_doc( { 'doctype': 'Sales Person', 'sales_person_name': 'Test_Sales_Person' } ) salesman.save() Missing Cleanup Test methods create documents (Sketch Orders, Purchase Orders) but lack teardown logic to clean up test data. This can cause test pollution and database bloat over time. Implement tearDown or tearDownClass methods to delete created test documents, or use transactions with rollback. def test_sketch_order_created(self): sk_ord_frm = make_sketch_order_form(department = self.department, branch = self.branch, order_type = 'Sales', design_type = 'New Design') sketch_order = frappe.get_all("Sketch Order", filters = {'sketch_order_form': sk_ord_frm.name, 'docstatus': 0}) self.assertEqual(len(sketch_order), len(sk_ord_frm.order_details)) def test_sketch_order_created_mod_design(self): sk_ord_frm = make_sketch_order_form(department = self.department, branch = self.branch, order_type = 'Sales', design_type = 'Mod', design_code = 'EA00978-003') sketch_order = frappe.get_all("Sketch Order", filters = {'sketch_order_form': sk_ord_frm.name, 'docstatus': 0}) self.assertEqual(len(sketch_order), len(sk_ord_frm.order_details)) def test_purchase_order_created(self): sk_ord_frm = make_sketch_order_form(department = self.department, branch = self.branch, order_type='Purchase', supplier='Test_Supplier', design_type='New Design') sketch_order = frappe.get_all("Sketch Order", filters={'sketch_order_form': sk_ord_frm.name, 'docstatus': 0}) self.assertEqual(len(sketch_order), len(sk_ord_frm.order_details)) po = frappe.get_all("Purchase Order", filters={'custom_form_id': sk_ord_frm.name, 'docstatus': 0}) self.assertEqual(len(po), 1)

---

🛠️ Relevant configurations:

These are the relevant configurations for this tool:

[config]

enable_ai_metadata: False
model_reasoning: vertex_ai/gemini-2.5-pro
model: gpt-5.2-2025-12-11
model_turbo: anthropic/claude-haiku-4-5-20251001
fallback_models: ['anthropic/claude-sonnet-4-5-20250929', 'bedrock/us.anthropic.claude-sonnet-4-5-20250929-v1:0']
second_model_for_exhaustive_mode: o4-mini
git_provider: github
publish_output: True
publish_output_no_suggestions: True
publish_output_progress: True
verbosity_level: 0
publish_logs: False
debug_mode: False
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
use_global_wiki_settings_file: False
disable_auto_feedback: False
ai_timeout: 150
response_language: en-US
clone_repo_instead_of_fetch: True
always_clone: False
add_repo_metadata: True
clone_repo_time_limit: 300
publish_inline_comments_fallback_batch_size: 5
publish_inline_comments_fallback_sleep_time: 2
max_model_tokens: 32000
custom_model_max_tokens: -1
patch_extension_skip_types: ['.md', '.txt']
extra_allowed_extensions: []
allow_dynamic_context: True
allow_forward_dynamic_context: True
max_extra_lines_before_dynamic_context: 12
patch_extra_lines_before: 5
patch_extra_lines_after: 1
ai_handler: litellm
cli_mode: False
TRIAL_GIT_ORG_MAX_INVOKES_PER_MONTH: 75
TRIAL_RATIO_CLOSE_TO_LIMIT: 0.8
invite_only_mode: False
enable_request_access_msg_on_new_pr: False
check_also_invites_field: False
calculate_context: True
disable_checkboxes: False
output_relevant_configurations: True
large_patch_policy: clip
seed: -1
temperature: 0.2
allow_dynamic_context_ab_testing: False
choose_dynamic_context_ab_testing_ratio: 0.5
ignore_pr_title: ['^\\[Auto\\]', '^Auto', '^\\[Bump\\]']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: ['do-not-merge', 'skip-qodo']
ignore_ticket_labels: []
allow_only_specific_folders: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
is_auto_command: False
is_new_pr: False
present_reasoning: True
max_tickets: 10
max_tickets_chars: 8000
prevent_any_approval: False
enable_comment_approval: False
enable_auto_approval: False
auto_approve_for_low_review_effort: -1
auto_approve_for_no_suggestions: False
ensure_ticket_compliance: False
new_diff_format: True
new_diff_format_add_external_references: True
enable_custom_labels: False

[pr_reviewer]

require_score_review: True
require_tests_review: True
require_estimate_effort_to_review: True
require_can_be_split_review: True
require_security_review: True
require_todo_scan: True
require_ticket_analysis_review: True
require_ticket_labels: False
require_no_ticket_labels: False
check_pr_additional_content: False
persistent_comment: True
extra_instructions: You are a Senior Technical Architect specializing in the Frappe Framework, Python, and MariaDB optimization.
Be concise but strict about performance and security. Provide code snippets (or pseudocode) for fixes.

========================
1) 🚨 FRAPPE PERFORMANCE KILLERS (High Priority)
========================
A) N+1 Query Detection (must flag):
- Flag ANY loop that contains `frappe.get_doc`, `frappe.db.get_value`, `frappe.db.get_all/get_list`, or `frappe.db.sql`.
- Explain the query multiplier (e.g., “1 + N queries”) and propose a bulk-fetch refactor:
  - pre-fetch with `frappe.get_all/get_list` (with fields + filters) or one SQL query with IN,
  - build a dict/map keyed by name, then iterate without DB calls.

B) Heavy Writes / Controller chain (must flag):
- If `doc.save()` is used to update just 1–2 fields, aggressively suggest replacing with:
  - `frappe.db.set_value(doctype, name, field, value)`
  - or `frappe.db.set_value(doctype, name, {field1: v1, field2: v2})`
- Explain it avoids triggering heavy validations / hooks / controller logic unnecessarily.
- Exception: if controller validations are required for correctness, require justification.

C) ORM abuse / wrong API choice:
- Warn against `frappe.db.sql()` if the same can be achieved by permission-safe ORM (`frappe.get_list/get_all`)
  unless it is a complex join/aggregation needing SQL.
- Also warn when `frappe.db.*` is used in user-facing endpoints where permission checks matter.

D) Redundant commits (must flag):
- `frappe.db.commit()` inside loops is forbidden unless there is a carefully justified transactional reason.
- Suggest moving commit outside loop, batching, or using background jobs.

E) Unbounded reads:
- Flag `get_all/get_list` without strong filters or without pagination on potentially large tables.
- Recommend adding filters, indexed ordering, `limit/page_length`, or background processing.

F) Hot hooks:
- Flag heavy work (bulk queries, file IO, network calls) inside validate/on_update/after_insert hooks.
- Recommend `frappe.enqueue` and async jobs.

========================
2) 🛡️ SECURITY & SQL SAFETY (must be strict)
========================
A) SQL injection:
- Strictly forbid f-strings / concatenation inside `frappe.db.sql()`.
- REQUIRE parameter passing: `frappe.db.sql("... %s", (val,))`.

B) Whitelisting:
- If functions are intended to be called from client (JS / REST), ensure `@frappe.whitelist()` exists.
- Confirm sensitive functions are NOT accidentally exposed.

C) Permission checks:
- For new `get_list/get_all`/SQL calls, ensure strict permissions are not bypassed.
- If `ignore_permissions=True` or `frappe.db.*` is used, REQUIRE explicit justification + risk assessment.

D) Unsafe patterns:
- Flag `eval/exec`, insecure deserialization, and unsafe file path usage.

========================
3) 🐍 PYTHONIC & LOGIC OPTIMIZATIONS
========================
A) Mutable defaults (must flag):
- `def f(x=[])` / `def f(x={})` etc. Require `None` + initialization.

B) Data structure choices:
- If code does `if item in big_list`, suggest `set(big_list)` for O(1) lookups.

C) Inefficient loops:
- Reduce nested loops; pull invariants out; memoize expensive computations.

D) List building:
- Suggest list comprehensions ONLY when readability remains good.
- Avoid creating large intermediate lists when generators/chunking is better.

E) String + datetime churn:
- Flag repeated parsing/formatting/conversions inside loops.

========================
4) 📉 MARIADB / DATABASE OPTIMIZATION CHECKS
========================
A) Wildcards / over-fetching:
- Warn against `SELECT *`. Require selecting specific columns.

B) Indexing:
- If a field is used in WHERE / JOIN / ORDER BY patterns frequently, ask if it needs an index.
- If appropriate, suggest composite index order aligned with query filters (e.g., company + docstatus + posting_date).

C) Sorting + limiting:
- Flag ORDER BY on large sets without indexed order; recommend index or reduce rows first.

D) Aggregations:
- If Python loops compute SUM/COUNT over many rows, suggest SQL aggregation where safe.

========================
5) OUTPUT REQUIREMENTS
========================
- Provide concrete refactor steps and code snippets.
- Quantify impact when possible: “reduces from N queries to 1”.
- If table sizes/indexes are unknown, recommend checking EXPLAIN and adding indexes if confirmed.

final_update_message: False
enable_review_labels_security: True
enable_review_labels_effort: True
enable_help_text: False
num_max_findings: 8

[qodo-free-for-open-source-projects]

PR Code Suggestions ✨

Latest suggestions up to df4962a

Category	Suggestion	Impact
Incremental [*]	Eliminate duplicate test data Refactor the duplicated order_details dictionary into a reusable variable to improve code maintainability and avoid redundancy. jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py [130-164] -sketch_order_form.append('order_details', { +order_detail = { 'design_type': args.design_type, 'metal_type': 'Gold', 'tag__design_id': args.design_code, - ... -}) + 'budget': 50000, + 'metal_target': 1.1, + 'diamond_target': 1.25, + 'product_size': '10', + 'sizer_type': 'Rod', + 'gemstone_type': 'Ruby', + 'stone_changeable': 'No', + 'length': 10, + 'width': 10, + 'height': 10, + 'diamond_part_length': 10, + 'gemstone_size': '1.60*1.00 MM' +} +sketch_order_form.append('order_details', order_detail.copy()) +sketch_order_form.append('order_details', order_detail.copy()) -sketch_order_form.append('order_details', { - 'design_type': args.design_type, - 'metal_type': 'Gold', - 'tag__design_id': args.design_code, - ... -}) - [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies duplicated code and proposes extracting it into a variable, which significantly improves maintainability and reduces the risk of future inconsistencies.	Medium
	Batch existence checks for performance Batch the multiple frappe.db.exists() calls into fewer database queries to improve performance by reducing DB roundtrips. jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py [41-110] -if not frappe.db.exists('Customer', 'Test_Customer_External'): +existing_records = { + 'Customer': frappe.db.get_all('Customer', filters={'name': ['in', ['Test_Customer_External', 'Test_Customer_Internal']]}, pluck='name'), + 'Supplier': frappe.db.get_all('Supplier', filters={'name': 'Test_Supplier'}, pluck='name'), + 'Department': frappe.db.get_all('Department', filters={'department_name': 'Test_Department'}, pluck='name'), + 'Branch': frappe.db.get_all('Branch', filters={'branch_name': 'Test Branch'}, pluck='name'), + 'Sales Person': frappe.db.get_all('Sales Person', filters={'name': 'Test_Sales_Person'}, pluck='name') +} + +if 'Test_Customer_External' not in existing_records['Customer']: customer = frappe.get_doc(...) ... - customer.save() -if not frappe.db.exists('Customer', 'Test_Customer_Internal'): - customer = frappe.get_doc(...) - ... - customer.save() - -if not frappe.db.exists('Supplier', 'Test_Supplier'): - supplier = frappe.get_doc(...) - supplier.save() - -if not frappe.db.exists('Department',{'department_name':'Test_Department'}): - dep = frappe.get_doc(...) - dep.save() - -if not frappe.db.exists('Branch',{'branch_name':'Test Branch'}): - branch = frappe.get_doc(...) - branch.save() - -if not frappe.db.exists('Sales Person', 'Test_Sales_Person'): - salesman = frappe.get_doc(...) - salesman.save() - [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies an opportunity to optimize database interactions by batching existence checks, which is a good practice for test data setup performance.	Low
	Use insert for new documents For new documents, use the insert() method instead of save() for better performance and code clarity. jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py [50-55] customer.append('diamond_grades', { 'diamond_quality': 'EF-VVS', 'diamond_grade_1': '6B', 'diamond_grade_2': '4' }) -customer.save() +customer.insert() Apply / Chat Suggestion importance[1-10]: 5 __ Why: Using insert() for new documents is more explicit and slightly more performant than save(), correctly reflecting the code's intent within the if not frappe.db.exists(...) block.	Low
	Use standard DB API Replace the frappe.get_value alias with the more explicit frappe.db.get_value for improved code clarity. jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py [15-16] -cls.department = frappe.get_value('Department',{'department_name':'Test_Department'},'name') -cls.branch = frappe.get_value('Branch',{'branch_name':'Test Branch'},'name') +cls.department = frappe.db.get_value('Department', {'department_name': 'Test_Department'}, 'name') +cls.branch = frappe.db.get_value('Branch', {'branch_name': 'Test Branch'}, 'name') Apply / Chat Suggestion importance[1-10]: 3 __ Why: The suggestion correctly points out that using frappe.db.get_value is more explicit than the alias frappe.get_value, which improves code clarity, but it has no functional impact as they are equivalent.	Low
Security	Ensure workflow permissions are validated Ensure workflow transitions have the correct permissions by using frappe.set_user() to execute the workflow actions as a user with the necessary roles, such as 'Administrator'. jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py [196-198] sketch_order_form.save() +frappe.set_user("Administrator") # Or a user with appropriate workflow permissions apply_workflow(sketch_order_form, 'Send For Approval') apply_workflow(sketch_order_form, 'Approve') Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out a potential permissions issue when applying workflows in tests and suggests a valid solution using frappe.set_user(), which improves test reliability.	Medium
General	Use count instead of get_all Replace frappe.get_all() with the more efficient frappe.db.count() when only the number of records is needed for an assertion. jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py [21-22] -sketch_order = frappe.get_all("Sketch Order", filters = {'sketch_order_form': sk_ord_frm.name, 'docstatus': 0}) -self.assertEqual(len(sketch_order), len(sk_ord_frm.order_details)) +sketch_order_count = frappe.db.count("Sketch Order", filters = {'sketch_order_form': sk_ord_frm.name, 'docstatus': 0}) +self.assertEqual(sketch_order_count, len(sk_ord_frm.order_details)) Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies an inefficient data retrieval pattern and proposes using frappe.db.count(), which is a more performant and standard way to count records.	Low
Update

• Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

---

🛠️ Relevant configurations:

These are the relevant configurations for this tool:

[config]

enable_ai_metadata: False
model_reasoning: vertex_ai/gemini-2.5-pro
model: gpt-5.2-2025-12-11
model_turbo: anthropic/claude-haiku-4-5-20251001
fallback_models: ['anthropic/claude-sonnet-4-5-20250929', 'bedrock/us.anthropic.claude-sonnet-4-5-20250929-v1:0']
second_model_for_exhaustive_mode: o4-mini
git_provider: github
publish_output: True
publish_output_no_suggestions: True
publish_output_progress: True
verbosity_level: 0
publish_logs: False
debug_mode: False
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
use_global_wiki_settings_file: False
disable_auto_feedback: False
ai_timeout: 150
response_language: en-US
clone_repo_instead_of_fetch: True
always_clone: False
add_repo_metadata: True
clone_repo_time_limit: 300
publish_inline_comments_fallback_batch_size: 5
publish_inline_comments_fallback_sleep_time: 2
max_model_tokens: 32000
custom_model_max_tokens: -1
patch_extension_skip_types: ['.md', '.txt']
extra_allowed_extensions: []
allow_dynamic_context: True
allow_forward_dynamic_context: True
max_extra_lines_before_dynamic_context: 12
patch_extra_lines_before: 5
patch_extra_lines_after: 1
ai_handler: litellm
cli_mode: False
TRIAL_GIT_ORG_MAX_INVOKES_PER_MONTH: 75
TRIAL_RATIO_CLOSE_TO_LIMIT: 0.8
invite_only_mode: False
enable_request_access_msg_on_new_pr: False
check_also_invites_field: False
calculate_context: True
disable_checkboxes: False
output_relevant_configurations: True
large_patch_policy: clip
seed: -1
temperature: 0.2
allow_dynamic_context_ab_testing: False
choose_dynamic_context_ab_testing_ratio: 0.5
ignore_pr_title: ['^\\[Auto\\]', '^Auto', '^\\[Bump\\]']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: ['do-not-merge', 'skip-qodo']
ignore_ticket_labels: []
allow_only_specific_folders: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
is_auto_command: False
is_new_pr: False
present_reasoning: True
max_tickets: 10
max_tickets_chars: 8000
prevent_any_approval: False
enable_comment_approval: False
enable_auto_approval: False
auto_approve_for_low_review_effort: -1
auto_approve_for_no_suggestions: False
ensure_ticket_compliance: False
new_diff_format: True
new_diff_format_add_external_references: True
enable_custom_labels: False
user_interaction_statistics: {'action_type': 'more_suggestions'}
disable_head_sha_cleanup: False
extra_statistics: {'suggestion_statistics': [{'score': 3, 'label': "incremental <sup><a href='https://qodo-merge-docs.qodo.ai/core-abilities/incremental_update/'>[*]</a></sup>"}, {'score': 5, 'label': "incremental <sup><a href='https://qodo-merge-docs.qodo.ai/core-abilities/incremental_update/'>[*]</a></sup>"}, {'score': 6, 'label': "incremental <sup><a href='https://qodo-merge-docs.qodo.ai/core-abilities/incremental_update/'>[*]</a></sup>"}, {'score': 7, 'label': "incremental <sup><a href='https://qodo-merge-docs.qodo.ai/core-abilities/incremental_update/'>[*]</a></sup>"}, {'score': 6, 'label': 'general'}, {'score': 7, 'label': 'security'}]}

[pr_code_suggestions]

suggestions_depth: exhaustive
commitable_code_suggestions: False
decouple_hunks: False
dual_publishing_score_threshold: -1
focus_only_on_problems: True
allow_thumbs_up_down: False
enable_suggestion_type_reuse: False
enable_more_suggestions_checkbox: True
high_level_suggestions_enabled: True
extra_instructions: Focus suggestions on PERFORMANCE + SECURITY:
- Reduce DB roundtrips (bulk fetch, IN queries, joins/aggregates).
- Prefer set_value over save for small field updates unless validations are required.
- Ensure parameterized SQL only.
- Ensure endpoints are whitelisted intentionally and respect permissions.
- Prefer chunking/generators for big datasets.
Return copy-paste ready snippets when feasible.

enable_help_text: False
show_extra_context: False
persistent_comment: True
max_history_len: 5
apply_suggestions_checkbox: True
enable_chat_in_code_suggestions: True
apply_limit_scope: True
suggestions_score_threshold: 0
new_score_mechanism: True
new_score_mechanism_th_high: 9
new_score_mechanism_th_medium: 7
discard_unappliable_suggestions: False
num_code_suggestions_per_chunk: 6
num_best_practice_suggestions: 2
max_number_of_calls: 3
final_clip_factor: 0.8
demand_code_suggestions_self_review: True
code_suggestions_self_review_text: **Author self-review**: I have reviewed the PR code suggestions, and addressed the relevant ones.
approve_pr_on_self_review: False
fold_suggestions_on_self_review: True
publish_post_process_suggestion_impact: True
wiki_page_accepted_suggestions: True
enable_local_self_reflect_in_large_prs: False
simplify_response: True

---

Previous suggestions

✅ Suggestions up to commit 3d3b8bd

Category	Suggestion	Impact
High-level	✅ Avoid explicit commits in test cases Suggestion Impact: The commit directly implements the suggestion by removing the frappe.db.commit() call at line 184 in the make_sketch_order_form function. The explicit commit that was breaking test isolation has been removed as recommended. code diff: - frappe.db.commit() Remove the explicit frappe.db.commit() call from the make_sketch_order_form test helper. This is necessary to preserve the transactional isolation of FrappeTestCase and prevent tests from interfering with each other. Examples: jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py [189] frappe.db.commit() Solution Walkthrough: Before: def make_sketch_order_form(**args): # ... document creation and setup ... sketch_order_form.insert() apply_workflow(sketch_order_form, 'Send For Approval') apply_workflow(sketch_order_form, 'Approve') frappe.db.commit() return sketch_order_form After: def make_sketch_order_form(**args): # ... document creation and setup ... sketch_order_form.insert() apply_workflow(sketch_order_form, 'Send For Approval') apply_workflow(sketch_order_form, 'Approve') # The explicit commit is removed to maintain test isolation. return sketch_order_form Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that using frappe.db.commit() within a FrappeTestCase breaks test isolation, which is a critical flaw that can lead to flaky and unreliable tests.	High
General	✅ Use frappe.db.exists for existence checks Suggestion Impact: The suggestion was directly implemented in the commit. Lines 71 and 84 of the diff show that frappe.get_value was replaced with frappe.db.exists for both Department and Branch existence checks, exactly as suggested. code diff: + if not frappe.db.exists('Department',{'department_name':'Test_Department'}): dep = frappe.get_doc( { 'doctype': 'Department', @@ -72,9 +87,9 @@ "company": 'Gurukrupa Export Private Limited' } ) - dep.insert() - - if not frappe.get_value('Branch',{'branch_name':'Test Branch'},'name'): + dep.save() + + if not frappe.db.exists('Branch',{'branch_name':'Test Branch'}): Replace frappe.get_value with the more performant frappe.db.exists for checking the existence of 'Department' and 'Branch' records in the create_test_data function. jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py [67-86] - if not frappe.get_value('Department',{'department_name':'Test_Department'},'name'): + if not frappe.db.exists('Department', {'department_name': 'Test_Department'}): dep = frappe.get_doc( { 'doctype': 'Department', 'department_name': "Test_Department", "company": 'Gurukrupa Export Private Limited' } ) dep.insert() - if not frappe.get_value('Branch',{'branch_name':'Test Branch'},'name'): + if not frappe.db.exists('Branch', {'branch_name': 'Test Branch'}): branch = frappe.get_doc( { 'doctype': 'Branch', 'branch': 'Test Branch', 'branch_name': 'Test Branch', 'company': 'Gurukrupa Export Private Limited' } ) branch.insert() [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that using frappe.db.exists is more performant for existence checks than frappe.get_value, improving the efficiency of the test data setup function.	Low
	✅ Avoid unnecessary database queries in tests Suggestion Impact: The suggestion was implemented but with a different approach. Instead of hardcoding the values directly in make_sketch_order_form, the commit moved the database queries to setUpClass (lines 11-12) where they are executed once per test class, and then passed as parameters to make_sketch_order_form (lines 112-113). This achieves the same goal of avoiding repeated database queries during test execution while maintaining flexibility. code diff: + @classmethod + def setUpClass(cls): + super().setUpClass() create_test_data() + cls.department = frappe.get_value('Department',{'department_name':'Test_Department'},'name') + cls.branch = frappe.get_value('Branch',{'branch_name':'Test Branch'},'name') def test_sketch_order_created(self): - sk_ord_frm = make_sketch_order_form(order_type = 'Sales', design_type = 'New Design') + sk_ord_frm = make_sketch_order_form(department = self.department, branch = self.branch, order_type = 'Sales', design_type = 'New Design') sketch_order = frappe.get_all("Sketch Order", filters = {'sketch_order_form': sk_ord_frm.name, 'docstatus': 0}) self.assertEqual(len(sketch_order), len(sk_ord_frm.order_details)) def test_sketch_order_created_mod_design(self): - sk_ord_frm = make_sketch_order_form(order_type = 'Sales', design_type = 'Mod', design_code = 'EA00978-003') + sk_ord_frm = make_sketch_order_form(department = self.department, branch = self.branch, order_type = 'Sales', design_type = 'Mod', design_code = 'EA00978-003') sketch_order = frappe.get_all("Sketch Order", filters = {'sketch_order_form': sk_ord_frm.name, 'docstatus': 0}) self.assertEqual(len(sketch_order), len(sk_ord_frm.order_details)) def test_purchase_order_created(self): - sk_ord_frm = make_sketch_order_form(order_type='Purchase', supplier='Test_Supplier', design_type='New Design') + sk_ord_frm = make_sketch_order_form(department = self.department, branch = self.branch, order_type='Purchase', design_type='New Design') sketch_order = frappe.get_all("Sketch Order", filters={'sketch_order_form': sk_ord_frm.name, 'docstatus': 0}) self.assertEqual(len(sketch_order), len(sk_ord_frm.order_details)) @@ -42,7 +47,12 @@ 'custom_sketch_workflow_state': 'External' } ) - customer.insert() + customer.append('diamond_grades', { + 'diamond_quality': 'EF-VVS', + 'diamond_grade_1': '6B', + 'diamond_grade_2': '4' + }) + customer.save() if not frappe.db.exists('Customer', 'Test_Customer_Internal'): customer = frappe.get_doc( @@ -53,7 +63,12 @@ 'custom_sketch_workflow_state': 'Internal' } ) - customer.insert() + customer.append('diamond_grades', { + 'diamond_quality': 'EF-VVS', + 'diamond_grade_1': '6B', + 'diamond_grade_2': '4' + }) + customer.save() if not frappe.db.exists('Supplier', 'Test_Supplier'): supplier = frappe.get_doc( @@ -62,9 +77,9 @@ 'supplier_name': 'Test_Supplier' } ) - supplier.insert() - - if not frappe.get_value('Department',{'department_name':'Test_Department'},'name'): + supplier.save() + + if not frappe.db.exists('Department',{'department_name':'Test_Department'}): dep = frappe.get_doc( { 'doctype': 'Department', @@ -72,9 +87,9 @@ "company": 'Gurukrupa Export Private Limited' } ) - dep.insert() - - if not frappe.get_value('Branch',{'branch_name':'Test Branch'},'name'): + dep.save() + + if not frappe.db.exists('Branch',{'branch_name':'Test Branch'}): branch = frappe.get_doc( { 'doctype': 'Branch', @@ -83,7 +98,7 @@ 'company': 'Gurukrupa Export Private Limited' } ) - branch.insert() + branch.save() if not frappe.db.exists('Sales Person', 'Test_Sales_Person'): salesman = frappe.get_doc( @@ -92,7 +107,7 @@ 'sales_person_name': 'Test_Sales_Person' } ) - salesman.insert() + salesman.save() def make_sketch_order_form(**args): @@ -100,8 +115,8 @@ sketch_order_form = frappe.new_doc('Sketch Order Form') sketch_order_form.company = 'Gurukrupa Export Private Limited' sketch_order_form.customer_code = 'Test_Customer_External' - sketch_order_form.department = frappe.get_value('Department',{'department_name':'Test_Department'},'name') - sketch_order_form.branch = frappe.get_value('Branch',{'branch_name':'Test Branch'},'name') + sketch_order_form.department = args.department + sketch_order_form.branch = args.branch In make_sketch_order_form, replace frappe.get_value calls for 'Department' and 'Branch' with their hardcoded names to avoid unnecessary database queries and improve test performance. jewellery_erpnext/gurukrupa_exports/doctype/sketch_order_form/test_sketch_order_form.py [98-105] def make_sketch_order_form(**args): args = frappe._dict(args) sketch_order_form = frappe.new_doc('Sketch Order Form') sketch_order_form.company = 'Gurukrupa Export Private Limited' sketch_order_form.customer_code = 'Test_Customer_External' - sketch_order_form.department = frappe.get_value('Department',{'department_name':'Test_Department'},'name') - sketch_order_form.branch = frappe.get_value('Branch',{'branch_name':'Test Branch'},'name') + sketch_order_form.department = 'Test_Department' + sketch_order_form.branch = 'Test Branch' sketch_order_form.salesman_name = 'Test_Sales_Person' ... Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that database lookups for 'Department' and 'Branch' are unnecessary since their names are known from the test setup. Hardcoding these values improves test performance by reducing database queries.	Low

[Kanishkar16Vijay]

/review

[Kanishkar16Vijay]

/review

[qodo-free-for-open-source-projects]

Persistent suggestions updated to latest commit df4962a