Skip to content

Conversation

@karm1000
Copy link
Member

@karm1000 karm1000 commented Jan 7, 2026

Closes: #3865

Summary by CodeRabbit

  • Bug Fixes

    • Improved GSTR-3B report Excel export functionality by fixing filename formatting and streamlining the file delivery process.
  • Refactor

    • Optimized the Excel file export process for better performance and reliability.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 7, 2026

📝 Walkthrough

Walkthrough

Refactor Excel export mechanism in GSTR-3B report to use frappe's provide_binary_file utility instead of manual file handling. The filename construction is adjusted to remove the ".xlsx" extension, delegating extension management to the utility function.

Changes

Cohort / File(s) Summary
GSTR-3B Report Filename
india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py
Modified _get_filename to return "GSTR-3B-{gstin}-{month}-{fiscal_year}" without ".xlsx" extension
Excel Export Utility Refactoring
india_compliance/gst_india/utils/exporter.py
Replaced manual file-type handling and direct response population with provide_binary_file utility call; removed automatic ".xlsx" appending

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

squash

Suggested reviewers

  • ljain112
  • vorasmit

Poem

🐰 A rabbit hops with glee,
No ".xlsx" in sight to see,
Frappe's tools now take the lead,
Export streams with newfound speed!
🎉 Refactored, clean, and free!

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor: use frappe utils to export excel' clearly and concisely describes the main change: refactoring to use Frappe's built-in export utilities instead of custom logic.
Linked Issues check ✅ Passed The PR successfully implements the refactoring requested in issue #3865 by replacing custom Excel export logic with Frappe's provide_binary_file utility, meeting the primary objective.
Out of Scope Changes check ✅ Passed All changes are directly related to the refactoring objective. The filename construction change in gstr_3b_report.py and the exporter utility replacement in exporter.py both support the goal of using Frappe's export utilities.

✏️ Tip: You can configure your own custom Pre-merge Checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26b7886 and 7b8502b.

📒 Files selected for processing (2)
  • india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py
  • india_compliance/gst_india/utils/exporter.py
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3742
File: india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py:822-827
Timestamp: 2025-10-14T09:20:47.761Z
Learning: In the GSTR-3B Excel template (gstr3b_excel_utility_v5.7.xlsx), the SGST (samt) field is a formula-based field that is calculated automatically within the Excel file, so it should not be included in the column mappings (TAX_COLUMNS, ITC_COLUMNS, COLUMN_SETS) when exporting data from Python code in india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py.
📚 Learning: 2025-10-14T09:20:47.761Z
Learnt from: ljain112
Repo: resilient-tech/india-compliance PR: 3742
File: india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py:822-827
Timestamp: 2025-10-14T09:20:47.761Z
Learning: In the GSTR-3B Excel template (gstr3b_excel_utility_v5.7.xlsx), the SGST (samt) field is a formula-based field that is calculated automatically within the Excel file, so it should not be included in the column mappings (TAX_COLUMNS, ITC_COLUMNS, COLUMN_SETS) when exporting data from Python code in india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py.

Applied to files:

  • india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py
📚 Learning: 2025-04-22T12:39:19.768Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3222
File: india_compliance/gst_india/doctype/gst_return_log/generate_gstr_1.py:656-657
Timestamp: 2025-04-22T12:39:19.768Z
Learning: For GSTR-1 filing in India Compliance, when handling quarterly returns, the `month_or_quarter` parameter already contains the last month of the quarter (e.g., "03" for Q1, "06" for Q2, etc.) rather than quarterly identifiers.

Applied to files:

  • india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py
📚 Learning: 2025-06-19T08:21:29.308Z
Learnt from: vorasmit
Repo: resilient-tech/india-compliance PR: 3451
File: india_compliance/gst_india/doctype/gstr_1_beta/gstr_1_export.py:287-288
Timestamp: 2025-06-19T08:21:29.308Z
Learning: In GSTR-1 export code (india_compliance/gst_india/doctype/gstr_1_beta/gstr_1_export.py), date fields like invoice date (DOC_DATE) and shipping bill date (SHIPPING_BILL_DATE) are mandatory by GST regulations and must exist, so transform functions using strftime() don't need None checks.

Applied to files:

  • india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py
📚 Learning: 2025-10-01T10:54:11.096Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3709
File: india_compliance/gst_india/utils/exporter.py:174-174
Timestamp: 2025-10-01T10:54:11.096Z
Learning: In india_compliance/gst_india/doctype/gstr_1/gstr_1_export.py, FIELD_TRANSFORMATIONS dictionary contains lambdas that are only called internally by DataProcessor.apply_transformations() with a single argument. These are separate from header transforms used with ExcelExporter.insert_data() which accept two arguments (value, row).

Applied to files:

  • india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py
  • india_compliance/gst_india/utils/exporter.py
📚 Learning: 2025-04-25T11:12:59.799Z
Learnt from: Ninad1306
Repo: resilient-tech/india-compliance PR: 3344
File: india_compliance/gst_india/report/gst_account_wise_summary/gst_account_wise_summary.py:18-20
Timestamp: 2025-04-25T11:12:59.799Z
Learning: In India Compliance GST reports, date_range is defined as a mandatory filter in the report configuration, making additional defensive coding for its presence unnecessary as the Frappe framework enforces required filters before calling the execute function.

Applied to files:

  • india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py
⏰ 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). (3)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary
🔇 Additional comments (3)
india_compliance/gst_india/doctype/gstr_3b_report/gstr_3b_report.py (1)

890-891: LGTM! Filename format correctly updated.

The removal of the ".xlsx" extension aligns perfectly with the refactored ExcelExporter.export() method, which now handles the extension through the provide_binary_file utility. The filename format remains clear and descriptive.

india_compliance/gst_india/utils/exporter.py (2)

65-72: All callers already pass filenames without extensions—refactoring is complete and correct.

Verification confirms that all six call sites of export() pass filenames without the ".xlsx" extension. Each caller uses a file name generator that returns only the base name (e.g., "GSTR-1-Gov-...", "{company}_{gstin}_report"), and provide_binary_file from Frappe's utility module handles appending the extension based on the type parameter. No caller updates are needed.


9-9: No action required. The import is correct and provide_binary_file is properly used. The function accepts parameters in the expected order (filename, file_extension, filecontent), and the code correctly passes file_name and "xlsx" as the first two arguments.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Use frappe's util to export excel

1 participant