test: add test cases for Sketch Order#7
Draft
Kanishkar16Vijay wants to merge 2 commits intodevelop_aerelefrom
Draft
test: add test cases for Sketch Order#7Kanishkar16Vijay wants to merge 2 commits intodevelop_aerelefrom
Kanishkar16Vijay wants to merge 2 commits intodevelop_aerelefrom
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
🛠️ Relevant configurations:
[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_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
|
PR Code Suggestions ✨Explore these optional code suggestions:
🛠️ Relevant configurations:
[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
extra_statistics: {'suggestion_statistics': [{'score': 6, 'label': 'general'}]}
[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
|
||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Type
Tests
Description
Implements comprehensive test cases for Sketch Order doctype
Tests both Purchase and Sales order workflows with state transitions
Validates workflow approvals and item creation from sketch orders
Uses helper function to create test sketch order forms
Diagram Walkthrough
File Walkthrough
test_sketch_order.py
Implement Sketch Order workflow and validation testsjewellery_erpnext/gurukrupa_exports/doctype/sketch_order/test_sketch_order.py
test_sketch_order_purchaseandtest_sketch_order_salessetUpClassto initialize test Department and Branch fixturesapply_workflowfor bothpurchase and sales order scenarios
tearDownmethod with database rollback for test isolation🛠️ Relevant configurations:
These are the relevant configurations for this tool:
[config]
[pr_description]