Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(india): duplicate qrcode and hide button #31100

Conversation

maharshivpatel
Copy link
Contributor

Problem: allows qrcode generation even if it exists and makes API call even though data will most likely be available in signed_qr_code field.

Solution: I have added logic on the server & client side to prevent the duplicate generation of qrcode and use signed_qr_code if available this will allow generating qrcode even after 2 days. if all fails it will get qrcode using api.

Note: I have added __unsaved check on the client side this is for future proofing.

currently, there is no way to change anything in the invoice once you generate IRN however this is not the correct approach as there are many fields that have nothing to do with Invoice details that we send to the IRP ( government ) and we should be able to change fields that don't affect data we sent to the IRP. I am working on it and will send separate PR or this.

problem: allows qrcode generation even if it exists and makes api call even though data will most likely be available in signed_qr_code field.
solution: i have added logic on server & client side to prevent duplicate generation of qrcode and use signed_qr_code if available this will allow to generate qrcode even after 2 days. if all fails it will get qrcode using irn api.
@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label May 23, 2022
@codecov
Copy link

codecov bot commented May 23, 2022

Codecov Report

Merging #31100 (526a911) into develop (f2b6475) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop   #31100      +/-   ##
===========================================
- Coverage    63.18%   63.09%   -0.09%     
===========================================
  Files          986      986              
  Lines        67345    67368      +23     
===========================================
- Hits         42550    42509      -41     
- Misses       24795    24859      +64     
Impacted Files Coverage Δ
erpnext/regional/india/e_invoice/utils.py 39.89% <0.00%> (-0.37%) ⬇️
...eport/item_variant_details/item_variant_details.py 31.32% <0.00%> (-53.02%) ⬇️
.../report/delayed_item_report/delayed_item_report.py 60.78% <0.00%> (-35.30%) ⬇️
...tch_item_expiry_status/batch_item_expiry_status.py 67.92% <0.00%> (-24.53%) ⬇️
...wise_balance_history/batch_wise_balance_history.py 67.79% <0.00%> (-22.04%) ⬇️
erpnext/stock/reorder_item.py 74.16% <0.00%> (-2.76%) ⬇️
.../report/bom_operations_time/bom_operations_time.py 89.13% <0.00%> (-2.18%) ⬇️
.../fifo_queue_vs_qty_after_transaction_comparison.py 93.10% <0.00%> (-1.73%) ⬇️
erpnext/crm/doctype/prospect/prospect.py 56.71% <0.00%> (-1.50%) ⬇️
.../report/stock_projected_qty/stock_projected_qty.py 88.15% <0.00%> (-1.32%) ⬇️
... and 21 more

indentation and semi colon fix.
indentation fix again
erpnext/regional/india/e_invoice/utils.py Outdated Show resolved Hide resolved
erpnext/regional/india/e_invoice/utils.py Outdated Show resolved Hide resolved
erpnext/regional/india/e_invoice/einvoice.js Outdated Show resolved Hide resolved
erpnext/regional/india/e_invoice/utils.py Outdated Show resolved Hide resolved
maharshivpatel and others added 2 commits May 24, 2022 15:39
removed redundant ( unnecessary ) checks and kwarg

Co-authored-by: Saqib Ansari <nextchamp.saqib@gmail.com>
there aren't enough use cases so i have removed update_url from attach_qrcode_image function and some linter fix.
@nextchamp-saqib
Copy link
Member

currently, there is no way to change anything in the invoice once you generate IRN however this is not the correct approach as there are many fields that have nothing to do with Invoice details that we send to the IRP ( government ) and we should be able to change fields that don't affect data we sent to the IRP. I am working on it and will send a separate PR or this.

I had implemented a validation for the same, it compares the e-invoice JSON before and after a field was modified, if the before & after JSON was the same, then the modification was allowed.

However, this isn't needed anymore right? Once the e-invoice is generated on invoice submission, the invoice need not be edited anymore

Or do you foresee any other scenario where this validation would be needed?

@maharshivpatel
Copy link
Contributor Author

maharshivpatel commented May 25, 2022

Or do you foresee any other scenario where this validation would be needed?

Current if irn will make the "allow on submit" fields not editable so this validation will be needed.

For example, The issue that I am working on right now is that the user doesn't want to create the e-way bill as the invoice value is less than 50000.

if the user adds transport details then an e-way bill will be generated so the temporary fix would be the user would add it after IRN generation but this is not allowed due to the if irn statement.

I am working on a more user-friendly solution for the above issue.

There is e_inovice inside regional that is supposed to be removed in v14 then there is the erpnext_gst_compliance app and maybe there is a private repo that @deepeshgarg007 mentioned here so which one of these will be available in v14 as in erpnext_gst_compliance there aren't any commits after Jan 6, 2022, so it lacks some of the important fixes and is not tested yet in production. we (the public) don't have access to the repo that deepeshgarg007 mentioned

@nextchamp-saqib
Copy link
Member

Current if irn will make the "allow on submit" fields not editable so this validation will be needed.

We can remove the validation altogether. If IRN will be generated on submission, then modifying an invoice value after submission is not possible. So is there a need to keep the validation that some fields of the invoice cannot be modified?

so which one of these will be available in v14

Currently, the changes in the current develop will be available in v14. Both of those apps are not yet in production.

@nextchamp-saqib nextchamp-saqib merged commit 935e5b1 into frappe:develop May 27, 2022
mergify bot pushed a commit that referenced this pull request May 27, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-13-hotfix needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants