Skip to content

fix: fix ITC claim period filtering GST Purchase Register and reports#4280

Open
ljain112 wants to merge 1 commit intoresilient-tech:developfrom
ljain112:fix-report-filters
Open

fix: fix ITC claim period filtering GST Purchase Register and reports#4280
ljain112 wants to merge 1 commit intoresilient-tech:developfrom
ljain112:fix-report-filters

Conversation

@ljain112
Copy link
Copy Markdown
Member

@ljain112 ljain112 commented May 1, 2026

Issue: Data was not getting filtered between dates.

@codacy-production
Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 4 complexity

Metric Results
Complexity 4

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: 5/5

Safe to merge — the fix is targeted, correctly handles edge cases, and is well covered by new unit and integration tests.

No P0 or P1 findings. The core logic change is straightforward: a single == filter becomes an isin over the correct period set. All callers of apply_period_filter that omit return_period benefit from the fix. The new get_periods_between_dates is equivalent in behaviour to the removed rrule loop for first-of-month inputs standard in this codebase.

No files require special attention.

Important Files Changed

Filename Overview
india_compliance/gst_india/utils/itc_claim.py Fixes apply_period_filter to use isin over all periods in the date range (instead of only to_date's period) when return_period is not provided and filter_by="ITC Claim Period".
india_compliance/gst_india/utils/init.py Adds get_periods_between_dates utility to generate inclusive MMYYYY month period list between two dates, replacing the removed dateutil.rrule dependency.
india_compliance/gst_india/doctype/purchase_reconciliation_tool/init.py Delegates _get_periods to the new shared get_periods_between_dates, removing the local dateutil.rrule usage.
india_compliance/gst_india/utils/test_itc_claim.py Adds unit tests for get_periods_between_dates (range, reversed, same-month) and for apply_period_filter with both explicit and range-based period modes.
india_compliance/gst_india/report/gst_purchase_register/test_gst_purchase_register.py Adds an integration test verifying that "ITC Claim Period" date-range filtering in the GST Purchase Register includes the correct invoices across multiple months.

Reviews (1): Last reviewed commit: "fix: fix ITC claim period filtering GST ..." | Re-trigger Greptile

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 1, 2026

📝 Walkthrough

Walkthrough

This pull request introduces a new utility function get_periods_between_dates that generates month periods in MMYYYY format between two dates. The BaseUtil._get_periods method is refactored to use this new helper instead of dateutil.rrule. The apply_period_filter function is updated to apply multi-period filtering for "ITC Claim Period" logic, covering all periods between from_date and to_date unless an explicit return_period is provided. Integration and unit tests are added to validate the new period calculation logic and the ITC Claim Period filtering behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the fix for ITC claim period filtering in GST Purchase Register and reports, which is the main change across multiple files in the changeset.
Description check ✅ Passed The description directly relates to the changeset by identifying the core issue (data not being filtered between dates) that the PR fixes.
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 60 minutes.

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: 1

🧹 Nitpick comments (1)
india_compliance/gst_india/utils/test_itc_claim.py (1)

97-107: ⚡ Quick win

Add one cross-month partial-boundary test case.

Current coverage misses a high-signal case where from_date/to_date are in different months but not at month boundaries (e.g., 15th → 1st). Adding this helps lock down date-range semantics.

✅ Suggested test addition
+    def test_get_periods_between_date_partial_month_boundaries(self):
+        periods = get_periods_between_dates("2024-04-15", "2024-06-01")
+        self.assertEqual(periods, ["042024", "052024", "062024"])

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ac0f8b8a-646e-43c0-8eb1-d850a3e7aede

📥 Commits

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

📒 Files selected for processing (5)
  • india_compliance/gst_india/doctype/purchase_reconciliation_tool/__init__.py
  • india_compliance/gst_india/report/gst_purchase_register/test_gst_purchase_register.py
  • india_compliance/gst_india/utils/__init__.py
  • india_compliance/gst_india/utils/itc_claim.py
  • india_compliance/gst_india/utils/test_itc_claim.py

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.

1 participant