-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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: KSA QR field rename patch #28832
fix: KSA QR field rename patch #28832
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #28832 +/- ##
===========================================
- Coverage 55.28% 55.25% -0.03%
===========================================
Files 1122 1122
Lines 66828 66828
===========================================
- Hits 36943 36928 -15
- Misses 29885 29900 +15
|
@@ -12,5 +13,20 @@ def execute(): | |||
|
|||
if frappe.db.exists('DocType', 'Sales Invoice'): | |||
frappe.reload_doc('accounts', 'doctype', 'sales_invoice', force=True) | |||
|
|||
# rename_field method assumes that the field already exists or the doc is synced | |||
if not frappe.db.has_column('Sales Invoice', 'ksa_einv_qr'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't https://github.com/frappe/erpnext/blob/develop/erpnext/regional/saudi_arabia/utils.py#L19-L30 be enough? 🤔
It checks if the field is present, if not, creates, before sales invoice is submitted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then there is no point of the patch then, the patch will get executed before the field is created and no data will me moved from the old field in the invoices
@Mergifyio backport version-13-hotfix |
🟠 Waiting for conditions to match
|
@Mergifyio backport version-13-pre-release |
✅ Backports have been created
|
✅ Backports have been created
|
…-28832 fix: KSA QR field rename patch (backport #28832)
…se/pr-28832 fix: KSA QR field rename patch (backport #28832)
Fixes an issue introduced in #28776
The data from old field was not migrated to the new field as it didn't exist in the first place