Skip to content

fix: auto set company gstin in jv#4279

Open
ljain112 wants to merge 4 commits intoresilient-tech:developfrom
ljain112:auto-gstinb-jv
Open

fix: auto set company gstin in jv#4279
ljain112 wants to merge 4 commits intoresilient-tech:developfrom
ljain112:auto-gstinb-jv

Conversation

@ljain112
Copy link
Copy Markdown
Member

@ljain112 ljain112 commented May 1, 2026

closes: #4230

Note: Added Journal Entry test cases

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 1, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 6 complexity

Metric Results
Complexity 6

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 1, 2026

Confidence Score: 4/5

Safe to merge after fixing missing super() calls in the new test class.

The production fix in journal_entry.py is clean and correct. The P1 finding is confined to the new test class missing super().setUpClass() / super().tearDownClass(), which could leave the test harness in a bad state for subsequent test classes but does not affect runtime behaviour.

india_compliance/gst_india/overrides/test_journal_entry.py — missing super() calls in setUp/tearDownClass

Important Files Changed

Filename Overview
india_compliance/gst_india/overrides/journal_entry.py Core fix: refactors validate() to auto-set company_gstin when the company has exactly one GSTIN, and throws only when multiple GSTINs are registered. Logic is clean and well-structured.
india_compliance/gst_india/overrides/test_journal_entry.py New test file; missing super().setUpClass() and super().tearDownClass() calls which are required by IntegrationTestCase to initialise and clean up framework-level state.
india_compliance/gst_india/utils/tests.py Consolidates previously duplicated create_itc_reversal_journal_entry and create_itc_reclaim_journal_entry helpers into the shared test utility module with tax_amount and company parameterisation. Well-implemented refactor.
india_compliance/gst_india/report/gst_purchase_register/test_gst_purchase_register.py Removes local _create_journal_entry helper and delegates to the shared utilities. Tax amounts are now passed explicitly, removing the hard-coded default. Clean refactor.
india_compliance/gst_india/doctype/gstr_3b_report/test_gstr_3b_report.py Switches to shared utility imports and passes explicit tax_amount=9 where previously a default was relied on. Straightforward import update.

Reviews (3): Last reviewed commit: "test: refactor test cases" | Re-trigger Greptile

Comment thread india_compliance/gst_india/utils/tests.py Outdated
Comment thread india_compliance/gst_india/overrides/journal_entry.py
Comment thread india_compliance/gst_india/overrides/test_journal_entry.py
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 1, 2026

Warning

Rate limit exceeded

@ljain112 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 52 minutes and 53 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33ef24fa-d9ac-4c94-9e3b-32d94dd11e15

📥 Commits

Reviewing files that changed from the base of the PR and between 2b1fcfb and 9b84b5a.

📒 Files selected for processing (2)
  • india_compliance/gst_india/overrides/test_journal_entry.py
  • india_compliance/gst_india/utils/tests.py
📝 Walkthrough

Walkthrough

This pull request refactors test utilities and enhances journal entry validation in the GST India module. It extracts duplicate test helper functions into a shared utilities module with four new functions for creating journal entries and ITC-related account rows. The journal entry validation logic is updated to automatically populate the company_gstin field when a company has exactly one linked GSTIN, or raise an error if multiple GSTINs exist. Corresponding tests are added to verify this auto-population behavior and validate error handling scenarios. Existing tests are refactored to use the new shared utilities, reducing code duplication.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No pull request description was provided, making it impossible to assess whether it relates to the changeset. Add a description explaining the purpose of automatically setting company GSTIN and the problem it solves.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: auto set company gstin in jv' accurately describes the main change: automatically setting company GSTIN in journal entries when a single GSTIN is linked.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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


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
Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 53 seconds.

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 917bdbbb-51eb-4966-9c96-d848d5e0510c

📥 Commits

Reviewing files that changed from the base of the PR and between 5db41f6 and 2b1fcfb.

📒 Files selected for processing (5)
  • india_compliance/gst_india/doctype/gstr_3b_report/test_gstr_3b_report.py
  • india_compliance/gst_india/overrides/journal_entry.py
  • india_compliance/gst_india/overrides/test_journal_entry.py
  • india_compliance/gst_india/report/gst_purchase_register/test_gst_purchase_register.py
  • india_compliance/gst_india/utils/tests.py

Comment thread india_compliance/gst_india/overrides/test_journal_entry.py Outdated
Comment thread india_compliance/gst_india/overrides/test_journal_entry.py Outdated
@ljain112
Copy link
Copy Markdown
Member Author

ljain112 commented May 1, 2026

@greptileai

Comment thread india_compliance/gst_india/overrides/test_journal_entry.py
@ljain112
Copy link
Copy Markdown
Member Author

ljain112 commented May 1, 2026

@greptileai

Comment on lines +13 to +14
def setUpClass(cls):
frappe.db.savepoint("before_test_journal_entry")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Missing super().setUpClass() call

TestJournalEntry.setUpClass never calls super().setUpClass(), unlike every other IntegrationTestCase subclass in this codebase (e.g. TestGSTPurchaseRegister calls it on line 41). IntegrationTestCase.setUpClass typically initialises database state and wraps the class in its own savepoint. Skipping it means any per-class initialisation from the framework is not applied, and the class-level savepoint managed by the parent is never created. Similarly, tearDownClass skips super().tearDownClass(), so any parent-class cleanup (e.g. releasing the class savepoint, resetting frappe state) is never run.

Suggested change
def setUpClass(cls):
frappe.db.savepoint("before_test_journal_entry")
@classmethod
def setUpClass(cls):
super().setUpClass()
frappe.db.savepoint("before_test_journal_entry")

Comment on lines +60 to +61
def tearDownClass(cls):
frappe.db.rollback(save_point="before_test_journal_entry")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Missing super().tearDownClass() call

The counterpart issue: tearDownClass doesn't call super().tearDownClass(). If IntegrationTestCase.tearDownClass commits or tears down the framework-level savepoint, skipping it can leave the test harness in an unexpected state for subsequent test classes.

Suggested change
def tearDownClass(cls):
frappe.db.rollback(save_point="before_test_journal_entry")
@classmethod
def tearDownClass(cls):
frappe.db.rollback(save_point="before_test_journal_entry")
super().tearDownClass()

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto set company gstin in backend in jv if only single gstin for company is available

1 participant