fix: update itc claim period from frontend on change of posting date or company gstin#4238
fix: update itc claim period from frontend on change of posting date or company gstin#4238ljain112 wants to merge 2 commits into
Conversation
|
Warning Rate limit exceeded
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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
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. Comment |
ljain112
force-pushed
the
itc-claim-period-changes
branch
from
068dc3a to
fe10aea
Compare
Confidence Score: 4/5Safe to merge after fixing the missing try/finally guard reset; the feature otherwise works correctly for the happy path. One P1 defect: a transient API error permanently deadlocks the guard flag, silently breaking ITC period auto-update for the rest of the form session. The fix is straightforward (wrap in try/finally). india_compliance/public/js/utils.js — the
|
| Filename | Overview |
|---|---|
| india_compliance/public/js/utils.js | New update_itc_claim_period helper added; guard flag is not reset in a finally block, so an API error permanently disables auto-update for the form session. |
| india_compliance/gst_india/client_scripts/purchase_invoice.js | Adds posting_date and company_gstin handlers to call update_itc_claim_period; existing gst_category handler is preserved. Change looks correct. |
| india_compliance/gst_india/doctype/bill_of_entry/bill_of_entry.js | Adds posting_date/company_gstin handlers and an explicit update_itc_claim_period call at the end of the company handler; await added to set_value calls. The in-flight guard prevents a double API call in the normal case. |
Reviews (2): Last reviewed commit: "fix: correct flag position" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const current_period = frm.doc.itc_claim_period; | ||
| if (current_period && valid_periods?.includes(current_period)) return; | ||
|
|
||
| const period = valid_periods?.[1]; |
There was a problem hiding this comment.
[1] instead of [0]
The function selects valid_periods?.[1] (the second element) as the replacement period. If the list has only one element — a valid edge case when posting date is near the boundary of valid periods — valid_periods[1] will be undefined, and itc_claim_period won't be updated even though valid_periods[0] is a valid choice. Was [0] (most recent / first valid period) intended here?
| if (frm.__updating_itc_claim_period) return; | ||
| frm.__updating_itc_claim_period = true; | ||
|
|
||
| await frappe.after_ajax(); | ||
|
|
||
| const { message: valid_periods } = await frappe.call({ | ||
| method: "india_compliance.gst_india.utils.itc_claim.get_itc_period_options", | ||
| args: { | ||
| company_gstin: frm.doc.company_gstin, | ||
| posting_date: frm.doc.posting_date, | ||
| }, | ||
| }); | ||
|
|
||
| frm.__updating_itc_claim_period = false; |
There was a problem hiding this comment.
If frappe.call() rejects (network failure, server error, etc.), frm.__updating_itc_claim_period is never reset to false. Every subsequent call to update_itc_claim_period will hit the early-return guard and silently do nothing for the lifetime of the form, meaning a transient error permanently breaks ITC period auto-update.
| if (frm.__updating_itc_claim_period) return; | |
| frm.__updating_itc_claim_period = true; | |
| await frappe.after_ajax(); | |
| const { message: valid_periods } = await frappe.call({ | |
| method: "india_compliance.gst_india.utils.itc_claim.get_itc_period_options", | |
| args: { | |
| company_gstin: frm.doc.company_gstin, | |
| posting_date: frm.doc.posting_date, | |
| }, | |
| }); | |
| frm.__updating_itc_claim_period = false; | |
| if (frm.__updating_itc_claim_period) return; | |
| frm.__updating_itc_claim_period = true; | |
| try { | |
| await frappe.after_ajax(); | |
| const { message: valid_periods } = await frappe.call({ | |
| method: "india_compliance.gst_india.utils.itc_claim.get_itc_period_options", | |
| args: { | |
| company_gstin: frm.doc.company_gstin, | |
| posting_date: frm.doc.posting_date, | |
| }, | |
| }); | |
| } finally { | |
| frm.__updating_itc_claim_period = false; | |
| } |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 18 |
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.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
ljain112
force-pushed
the
itc-claim-period-changes
branch
from
7eac000 to
8964e6f
Compare
1 participant
Update itc claim period from the frontend on change of posting date or company GSTIN if invalid.