From 8c923820da107dc93520360a6f1ce787acaff433 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Tue, 21 Dec 2021 22:09:01 +0530 Subject: [PATCH 01/19] fix: Ability to continue partially processed data imports --- .../core/doctype/data_import/data_import.js | 21 +++++++++++++++++-- .../core/doctype/data_import/data_import.py | 4 ++-- .../doctype/data_import/data_import_list.js | 8 +++++++ frappe/core/doctype/data_import/importer.py | 14 ++++++++++--- 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 216db53c72..3838439dcf 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -109,8 +109,18 @@ frappe.ui.form.on('Data Import', { frm.disable_save(); if (frm.doc.status !== 'Success') { if (!frm.is_new() && (frm.has_import_file())) { - let label = - frm.doc.status === 'Pending' ? __('Start Import') : __('Retry'); + let label = '' + if (frm.doc.status === 'Pending') { + let import_log = JSON.parse(frm.doc.import_log || '[]'); + if (import_log) { + label = __('Continue Import') + } else { + label = __('Start Import') + } + } else { + label = __('Retry') + } + frm.page.set_primary_action(label, () => frm.events.start_import(frm)); } else { frm.page.set_primary_action(__('Save'), () => frm.save()); @@ -133,6 +143,13 @@ frappe.ui.form.on('Data Import', { let failed_records = import_log.filter(log => !log.success); if (successful_records.length === 0) return; + if (frm.doc.status == 'Pending' && import_log) { + frm.page.set_indicator(__('Partially Completed'), 'orange'); + + // Do not allow changing file for partially completed imports + frm.set_df_property('import_file', 'read_only', 1); + } + let message; if (failed_records.length === 0) { let message_args = [successful_records.length]; diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index 5935ddc4ba..20d116d0ae 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -66,8 +66,8 @@ class DataImport(Document): if self.name not in enqueued_jobs: enqueue( start_import, - queue="default", - timeout=6000, + queue="long", + timeout=10000, event="data_import", job_name=self.name, data_import=self.name, diff --git a/frappe/core/doctype/data_import/data_import_list.js b/frappe/core/doctype/data_import/data_import_list.js index 0eb05aa354..d1701da15b 100644 --- a/frappe/core/doctype/data_import/data_import_list.js +++ b/frappe/core/doctype/data_import/data_import_list.js @@ -1,6 +1,7 @@ let imports_in_progress = []; frappe.listview_settings['Data Import'] = { + add_fields: ["import_log"], onload(listview) { frappe.realtime.on('data_import_progress', data => { if (!imports_in_progress.includes(data.data_import)) { @@ -19,17 +20,24 @@ frappe.listview_settings['Data Import'] = { 'Pending': 'orange', 'Not Started': 'orange', 'Partial Success': 'orange', + 'Partial Completed': 'orange', 'Success': 'green', 'In Progress': 'orange', 'Error': 'red' }; let status = doc.status; + let import_log = JSON.parse(doc.import_log || '[]'); + if (imports_in_progress.includes(doc.name)) { status = 'In Progress'; } if (status == 'Pending') { status = 'Not Started'; } + if (doc.status == 'Pending' && import_log.length > 0) { + status = 'Partially Completed' + } + return [__(status), colors[status], 'status,=,' + doc.status]; }, formatters: { diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index b9b2050763..2b385dc4b0 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -84,14 +84,16 @@ class Importer: else: import_log = [] - # remove previous failures from import log - import_log = [log for log in import_log if log.get("success")] + # Do not remove rows in case of retry after an error or pending data import + if self.data_import.status == 'Partial Success': + # remove previous failures from import log only in case of retry after partial success + import_log = [log for log in import_log if log.get("success")] # get successfully imported rows imported_rows = [] for log in import_log: log = frappe._dict(log) - if log.success: + if log.success or self.data_import.status == 'Pending': imported_rows += log.row_indexes # start import @@ -149,6 +151,12 @@ class Importer: import_log.append( frappe._dict(success=True, docname=doc.name, row_indexes=row_indexes) ) + + # Update import log after every successful import + # This is done for cases where the background job might get terminated due to timeout + # In such cases the job can be rerun only for pending rows by referring to import logs + self.data_import.db_set("import_log", json.dumps(import_log)) + # commit after every successful import frappe.db.commit() From 866496437a952b9a880ca1fc00fa5650c5bbac2b Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Wed, 22 Dec 2021 11:15:39 +0530 Subject: [PATCH 02/19] fix: Linting issues --- frappe/core/doctype/data_import/data_import.js | 10 +++++----- frappe/core/doctype/data_import/data_import_list.js | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 3838439dcf..15f76a693e 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -109,16 +109,16 @@ frappe.ui.form.on('Data Import', { frm.disable_save(); if (frm.doc.status !== 'Success') { if (!frm.is_new() && (frm.has_import_file())) { - let label = '' + let label = ''; if (frm.doc.status === 'Pending') { let import_log = JSON.parse(frm.doc.import_log || '[]'); - if (import_log) { - label = __('Continue Import') + if (import_log.length > 0) { + label = __('Continue Import'); } else { - label = __('Start Import') + label = __('Start Import'); } } else { - label = __('Retry') + label = __('Retry'); } frm.page.set_primary_action(label, () => frm.events.start_import(frm)); diff --git a/frappe/core/doctype/data_import/data_import_list.js b/frappe/core/doctype/data_import/data_import_list.js index d1701da15b..c9526d8f30 100644 --- a/frappe/core/doctype/data_import/data_import_list.js +++ b/frappe/core/doctype/data_import/data_import_list.js @@ -35,7 +35,7 @@ frappe.listview_settings['Data Import'] = { status = 'Not Started'; } if (doc.status == 'Pending' && import_log.length > 0) { - status = 'Partially Completed' + status = 'Partially Completed'; } return [__(status), colors[status], 'status,=,' + doc.status]; From 9db540a675daba75708d51643978cfdb3334a1fb Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 23 Dec 2021 11:48:03 +0530 Subject: [PATCH 03/19] fix: Add validation for file change --- frappe/core/doctype/data_import/data_import.py | 6 +++++- frappe/core/doctype/data_import/data_import_list.js | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index 20d116d0ae..2545e343c9 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -29,6 +29,10 @@ class DataImport(Document): self.validate_google_sheets_url() def validate_import_file(self): + + if self.status == 'Pending' and self.import_log: + frappe.throw(_("File cannot be changed for partially completed imports. Either continue import or make a fresh import")) + if self.import_file: # validate template self.get_importer() @@ -66,7 +70,7 @@ class DataImport(Document): if self.name not in enqueued_jobs: enqueue( start_import, - queue="long", + queue="default", timeout=10000, event="data_import", job_name=self.name, diff --git a/frappe/core/doctype/data_import/data_import_list.js b/frappe/core/doctype/data_import/data_import_list.js index c9526d8f30..dd40e43973 100644 --- a/frappe/core/doctype/data_import/data_import_list.js +++ b/frappe/core/doctype/data_import/data_import_list.js @@ -20,7 +20,7 @@ frappe.listview_settings['Data Import'] = { 'Pending': 'orange', 'Not Started': 'orange', 'Partial Success': 'orange', - 'Partial Completed': 'orange', + 'Partially Completed': 'orange', 'Success': 'green', 'In Progress': 'orange', 'Error': 'red' From f507011eda3863174ca18f6572685eb80c570996 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 23 Dec 2021 18:56:36 +0530 Subject: [PATCH 04/19] fix: Use long queue for data imports --- frappe/core/doctype/data_import/data_import.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index 2545e343c9..5e43b1e6e5 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -70,7 +70,7 @@ class DataImport(Document): if self.name not in enqueued_jobs: enqueue( start_import, - queue="default", + queue="long", timeout=10000, event="data_import", job_name=self.name, From 1c6d911718ab05967d6a2b0b5ba8727bcfee176b Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 23 Dec 2021 21:19:08 +0530 Subject: [PATCH 05/19] fix: Ability to export import logs --- .../core/doctype/data_import/data_import.js | 18 ++++++++++++++++- .../core/doctype/data_import/data_import.py | 10 +++++++++- frappe/core/doctype/data_import/importer.py | 20 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 15f76a693e..1ee5b1efd2 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -333,6 +333,15 @@ frappe.ui.form.on('Data Import', { ); }, + export_import_log(frm) { + open_url_post( + '/api/method/frappe.core.doctype.data_import.data_import.download_import_log', + { + data_import_name: frm.doc.name + } + ); + }, + show_import_warnings(frm, preview_data) { let columns = preview_data.columns; let warnings = JSON.parse(frm.doc.template_warnings || '[]'); @@ -419,7 +428,9 @@ frappe.ui.form.on('Data Import', { return; } - let rows = logs + // Show import logs only if rows are less than 5000 + if (logs.length < 5000) { + let rows = logs .map(log => { let html = ''; if (log.success) { @@ -495,5 +506,10 @@ frappe.ui.form.on('Data Import', { ${rows} `); + } else if (logs.length > 0) { + frm.add_custom_button(__('Export Import Log'), () => + frm.trigger('export_import_log') + ); + } }, }); diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index 5e43b1e6e5..fb6c5b147a 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -70,7 +70,7 @@ class DataImport(Document): if self.name not in enqueued_jobs: enqueue( start_import, - queue="long", + queue="default", timeout=10000, event="data_import", job_name=self.name, @@ -84,6 +84,9 @@ class DataImport(Document): def export_errored_rows(self): return self.get_importer().export_errored_rows() + def download_import_log(self): + return self.get_importer().export_import_log() + def get_importer(self): return Importer(self.reference_doctype, data_import=self) @@ -149,6 +152,11 @@ def download_errored_template(data_import_name): data_import = frappe.get_doc("Data Import", data_import_name) data_import.export_errored_rows() +@frappe.whitelist() +def download_import_log(data_import_name): + data_import = frappe.get_doc("Data Import", data_import_name) + data_import.download_import_log() + def import_file( doctype, file_path, import_type, submit_after_import=False, console=False diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 2b385dc4b0..706c40c910 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -272,6 +272,26 @@ class Importer: build_csv_response(rows, _(self.doctype)) + def export_import_log(self): + from frappe.utils.csvutils import build_csv_response + + if not self.data_import: + return + import_log = frappe.parse_json(self.data_import.import_log or "[]") + header_row = ["Row Numbers", "Status", "Message", "Exception"] + + rows = [header_row] + + for log in import_log: + row_number = log.get("row_indexes")[0] + status = "Success" if log.get('success') else "Failure" + message = "Successfully Imported {0}".format(log.get('docname')) if log.get('success') else \ + log.get("messages") + exception = frappe.utils.cstr(log.get("exception", '')) + rows += [[row_number, status, message, exception]] + + build_csv_response(rows, self.doctype) + def print_import_log(self, import_log): failed_records = [log for log in import_log if not log.success] successful_records = [log for log in import_log if log.success] From 33feee7ae29aa36c99898c45910351c4bfc12fa4 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 24 Dec 2021 11:38:15 +0530 Subject: [PATCH 06/19] fix: Toggle Import section --- frappe/core/doctype/data_import/data_import.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 1ee5b1efd2..2a8df60b9e 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -507,6 +507,8 @@ frappe.ui.form.on('Data Import', { `); } else if (logs.length > 0) { + frm.toggle_display('import_log_section', false); + frm.add_custom_button(__('Export Import Log'), () => frm.trigger('export_import_log') ); From fc2f7fa028a3feca62d814d2131b2a021a45125d Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 24 Dec 2021 16:27:09 +0530 Subject: [PATCH 07/19] fix: List view status --- frappe/core/doctype/data_import/data_import_list.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import_list.js b/frappe/core/doctype/data_import/data_import_list.js index dd40e43973..e7ddbf32f4 100644 --- a/frappe/core/doctype/data_import/data_import_list.js +++ b/frappe/core/doctype/data_import/data_import_list.js @@ -26,7 +26,6 @@ frappe.listview_settings['Data Import'] = { 'Error': 'red' }; let status = doc.status; - let import_log = JSON.parse(doc.import_log || '[]'); if (imports_in_progress.includes(doc.name)) { status = 'In Progress'; @@ -34,9 +33,6 @@ frappe.listview_settings['Data Import'] = { if (status == 'Pending') { status = 'Not Started'; } - if (doc.status == 'Pending' && import_log.length > 0) { - status = 'Partially Completed'; - } return [__(status), colors[status], 'status,=,' + doc.status]; }, From d99b99cb7bb822f9ec8c8f44e928da1c150e5172 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Sat, 25 Dec 2021 09:16:07 +0530 Subject: [PATCH 08/19] fix: Do not pull import logs in list view --- frappe/core/doctype/data_import/data_import_list.js | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/core/doctype/data_import/data_import_list.js b/frappe/core/doctype/data_import/data_import_list.js index e7ddbf32f4..84b0f0ada1 100644 --- a/frappe/core/doctype/data_import/data_import_list.js +++ b/frappe/core/doctype/data_import/data_import_list.js @@ -1,7 +1,6 @@ let imports_in_progress = []; frappe.listview_settings['Data Import'] = { - add_fields: ["import_log"], onload(listview) { frappe.realtime.on('data_import_progress', data => { if (!imports_in_progress.includes(data.data_import)) { From 764878fe76d0b14d5dd8fe1aee8b76eb32c60881 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 27 Dec 2021 12:20:14 +0530 Subject: [PATCH 09/19] chore: Add separate doctype for Data Import Log --- .../core/doctype/data_import_log/__init__.py | 0 .../data_import_log/data_import_log.js | 8 ++ .../data_import_log/data_import_log.json | 84 +++++++++++++++++++ .../data_import_log/data_import_log.py | 8 ++ .../data_import_log/test_data_import_log.py | 8 ++ 5 files changed, 108 insertions(+) create mode 100644 frappe/core/doctype/data_import_log/__init__.py create mode 100644 frappe/core/doctype/data_import_log/data_import_log.js create mode 100644 frappe/core/doctype/data_import_log/data_import_log.json create mode 100644 frappe/core/doctype/data_import_log/data_import_log.py create mode 100644 frappe/core/doctype/data_import_log/test_data_import_log.py diff --git a/frappe/core/doctype/data_import_log/__init__.py b/frappe/core/doctype/data_import_log/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/core/doctype/data_import_log/data_import_log.js b/frappe/core/doctype/data_import_log/data_import_log.js new file mode 100644 index 0000000000..c376edeec9 --- /dev/null +++ b/frappe/core/doctype/data_import_log/data_import_log.js @@ -0,0 +1,8 @@ +// Copyright (c) 2021, Frappe Technologies and contributors +// For license information, please see license.txt + +frappe.ui.form.on('Data Import Log', { + // refresh: function(frm) { + + // } +}); diff --git a/frappe/core/doctype/data_import_log/data_import_log.json b/frappe/core/doctype/data_import_log/data_import_log.json new file mode 100644 index 0000000000..a39eb9be1d --- /dev/null +++ b/frappe/core/doctype/data_import_log/data_import_log.json @@ -0,0 +1,84 @@ +{ + "actions": [], + "creation": "2021-12-25 16:12:20.205889", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "data_import", + "row_indexes", + "success", + "docname", + "messages", + "exception", + "log_index" + ], + "fields": [ + { + "fieldname": "data_import", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Data Import", + "options": "Data Import" + }, + { + "fieldname": "docname", + "fieldtype": "Data", + "label": "Reference Name" + }, + { + "fieldname": "exception", + "fieldtype": "Text", + "label": "Exception" + }, + { + "fieldname": "row_indexes", + "fieldtype": "Code", + "label": "Row Indexes", + "options": "JSON" + }, + { + "default": "0", + "fieldname": "success", + "fieldtype": "Check", + "in_list_view": 1, + "label": "Success" + }, + { + "fieldname": "log_index", + "fieldtype": "Int", + "in_list_view": 1, + "label": "Log Index" + }, + { + "fieldname": "messages", + "fieldtype": "Code", + "label": "Messages", + "options": "JSON" + } + ], + "in_create": 1, + "index_web_pages_for_search": 1, + "links": [], + "modified": "2021-12-27 11:19:19.646076", + "modified_by": "Administrator", + "module": "Core", + "name": "Data Import Log", + "owner": "Administrator", + "permissions": [ + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "share": 1, + "write": 1 + } + ], + "sort_field": "modified", + "sort_order": "DESC" +} \ No newline at end of file diff --git a/frappe/core/doctype/data_import_log/data_import_log.py b/frappe/core/doctype/data_import_log/data_import_log.py new file mode 100644 index 0000000000..a71aefa8bc --- /dev/null +++ b/frappe/core/doctype/data_import_log/data_import_log.py @@ -0,0 +1,8 @@ +# Copyright (c) 2021, Frappe Technologies and contributors +# For license information, please see license.txt + +# import frappe +from frappe.model.document import Document + +class DataImportLog(Document): + pass diff --git a/frappe/core/doctype/data_import_log/test_data_import_log.py b/frappe/core/doctype/data_import_log/test_data_import_log.py new file mode 100644 index 0000000000..244404936e --- /dev/null +++ b/frappe/core/doctype/data_import_log/test_data_import_log.py @@ -0,0 +1,8 @@ +# Copyright (c) 2021, Frappe Technologies and Contributors +# See license.txt + +# import frappe +import unittest + +class TestDataImportLog(unittest.TestCase): + pass From d79abcf84c2774f44d7a90d8d937862239287a68 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 27 Dec 2021 12:20:40 +0530 Subject: [PATCH 10/19] fix: Changes required for data import log --- .../core/doctype/data_import/data_import.js | 309 ++++++++++-------- .../core/doctype/data_import/data_import.json | 13 +- .../core/doctype/data_import/data_import.py | 33 +- frappe/core/doctype/data_import/importer.py | 77 +++-- .../js/frappe/data_import/import_preview.js | 2 +- 5 files changed, 248 insertions(+), 186 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 2a8df60b9e..e395a199b8 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -44,6 +44,7 @@ frappe.ui.form.on('Data Import', { } frm.dashboard.show_progress(__('Import Progress'), percent, message); frm.page.set_indicator(__('In Progress'), 'orange'); + frm.trigger('update_primary_action'); // hide progress when complete if (data.current === data.total) { @@ -109,19 +110,16 @@ frappe.ui.form.on('Data Import', { frm.disable_save(); if (frm.doc.status !== 'Success') { if (!frm.is_new() && (frm.has_import_file())) { - let label = ''; - if (frm.doc.status === 'Pending') { - let import_log = JSON.parse(frm.doc.import_log || '[]'); - if (import_log.length > 0) { - label = __('Continue Import'); - } else { - label = __('Start Import'); - } - } else { - label = __('Retry'); + let label = + frm.doc.status === 'Pending' ? __('Start Import') : __('Retry'); + + if (frm.doc.status == 'Partially Completed') { + label = __('Continue Import'); } - frm.page.set_primary_action(label, () => frm.events.start_import(frm)); + if (!frm.import_in_progress) { + frm.page.set_primary_action(label, () => frm.events.start_import(frm)); + } } else { frm.page.set_primary_action(__('Save'), () => frm.save()); } @@ -138,47 +136,49 @@ frappe.ui.form.on('Data Import', { }, show_import_status(frm) { - let import_log = JSON.parse(frm.doc.import_log || '[]'); - let successful_records = import_log.filter(log => log.success); - let failed_records = import_log.filter(log => !log.success); - if (successful_records.length === 0) return; - - if (frm.doc.status == 'Pending' && import_log) { - frm.page.set_indicator(__('Partially Completed'), 'orange'); - - // Do not allow changing file for partially completed imports - frm.set_df_property('import_file', 'read_only', 1); - } - - let message; - if (failed_records.length === 0) { - let message_args = [successful_records.length]; - if (frm.doc.import_type === 'Insert New Records') { - message = - successful_records.length > 1 - ? __('Successfully imported {0} records.', message_args) - : __('Successfully imported {0} record.', message_args); - } else { - message = - successful_records.length > 1 - ? __('Successfully updated {0} records.', message_args) - : __('Successfully updated {0} record.', message_args); - } - } else { - let message_args = [successful_records.length, import_log.length]; - if (frm.doc.import_type === 'Insert New Records') { - message = - successful_records.length > 1 - ? __('Successfully imported {0} records out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args) - : __('Successfully imported {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args); - } else { - message = - successful_records.length > 1 - ? __('Successfully updated {0} records out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args) - : __('Successfully updated {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args); + frappe.call({ + 'method': 'frappe.core.doctype.data_import.data_import.get_import_status', + 'args': { + 'data_import_name': frm.doc.name + }, + 'callback': function(r) { + let successful_records = cint(r.message.success); + let failed_records = cint(r.message.failed); + let total_records = cint(r.message.total_records); + + if (!total_records) return + + let message; + if (failed_records === 0) { + let message_args = [successful_records]; + if (me.frm.doc.import_type === 'Insert New Records') { + message = + successful_records > 1 + ? __('Successfully imported {0} records.', message_args) + : __('Successfully imported {0} record.', message_args); + } else { + message = + successful_records > 1 + ? __('Successfully updated {0} records.', message_args) + : __('Successfully updated {0} record.', message_args); + } + } else { + let message_args = [successful_records, total_records]; + if (frm.doc.import_type === 'Insert New Records') { + message = + successful_records > 1 + ? __('Successfully imported {0} records out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args) + : __('Successfully imported {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args); + } else { + message = + successful_records> 1 + ? __('Successfully updated {0} records out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args) + : __('Successfully updated {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args); + } + } + frm.dashboard.set_headline(message); } - } - frm.dashboard.set_headline(message); + }); }, show_report_error_button(frm) { @@ -285,15 +285,14 @@ frappe.ui.form.on('Data Import', { } }) .then(r => { - let preview_data = r.message; - frm.events.show_import_preview(frm, preview_data); + let preview_data = r.message.preview_data; + let import_log = r.message.import_log; + frm.events.show_import_preview(frm, preview_data, import_log); frm.events.show_import_warnings(frm, preview_data); }); }, - show_import_preview(frm, preview_data) { - let import_log = JSON.parse(frm.doc.import_log || '[]'); - + show_import_preview(frm, preview_data, import_log) { if ( frm.import_preview && frm.import_preview.doctype === frm.doc.reference_doctype @@ -417,101 +416,123 @@ frappe.ui.form.on('Data Import', { frm.trigger('show_import_log'); }, - show_import_log(frm) { - let import_log = JSON.parse(frm.doc.import_log || '[]'); - let logs = import_log; - frm.toggle_display('import_log', false); - frm.toggle_display('import_log_section', logs.length > 0); + render_import_log(frm) { + frappe.call({ + 'method': 'frappe.client.get_list', + 'args': { + 'doctype': 'Data Import Log', + 'filters': { + 'data_import': frm.doc.name + }, + 'fields': ['success', 'docname', 'messages', 'exception', 'row_indexes'], + 'page_limit_length': 5000 + }, + callback: function(r) { + let logs = r.message + let rows = logs + .map(log => { + let html = ''; + if (log.success) { + if (frm.doc.import_type === 'Insert New Records') { + html = __('Successfully imported {0}', [ + `${frappe.utils.get_form_link( + frm.doc.reference_doctype, + log.docname, + true + )}` + ]); + } else { + html = __('Successfully updated {0}', [ + `${frappe.utils.get_form_link( + frm.doc.reference_doctype, + log.docname, + true + )}` + ]); + } + } else { + let messages = (log.messages || []) + .map(JSON.parse) + .map(m => { + let title = m.title ? `${m.title}` : ''; + let message = m.message ? `
${m.message}
` : ''; + return title + message; + }) + .join(''); + let id = frappe.dom.get_unique_id(); + html = `${messages} + +
+
+
${log.exception}
+
+
`; + } + let indicator_color = log.success ? 'green' : 'red'; + let title = log.success ? __('Success') : __('Failure'); - if (logs.length === 0) { - frm.get_field('import_log_preview').$wrapper.empty(); - return; - } + if (frm.doc.show_failed_logs && log.success) { + return ''; + } - // Show import logs only if rows are less than 5000 - if (logs.length < 5000) { - let rows = logs - .map(log => { - let html = ''; - if (log.success) { - if (frm.doc.import_type === 'Insert New Records') { - html = __('Successfully imported {0}', [ - `${frappe.utils.get_form_link( - frm.doc.reference_doctype, - log.docname, - true - )}` - ]); - } else { - html = __('Successfully updated {0}', [ - `${frappe.utils.get_form_link( - frm.doc.reference_doctype, - log.docname, - true - )}` - ]); - } - } else { - let messages = log.messages - .map(JSON.parse) - .map(m => { - let title = m.title ? `${m.title}` : ''; - let message = m.message ? `
${m.message}
` : ''; - return title + message; - }) - .join(''); - let id = frappe.dom.get_unique_id(); - html = `${messages} - -
-
-
${log.exception}
-
-
`; - } - let indicator_color = log.success ? 'green' : 'red'; - let title = log.success ? __('Success') : __('Failure'); + return ` + ${JSON.parse(log.row_indexes).join(', ')} + +
${title}
+ + + ${html} + + `; + }) + .join(''); - if (frm.doc.show_failed_logs && log.success) { - return ''; + if (!rows && frm.doc.show_failed_logs) { + rows = ` + ${__('No failed logs')} + `; } - return ` - ${log.row_indexes.join(', ')} - -
${title}
- - - ${html} - - `; - }) - .join(''); + frm.get_field('import_log_preview').$wrapper.html(` + + + + + + + ${rows} +
${__('Row Number')}${__('Status')}${__('Message')}
+ `); + } + }); + }, - if (!rows && frm.doc.show_failed_logs) { - rows = ` - ${__('No failed logs')} - `; + show_import_log(frm) { + if (frm.import_in_progress) { + return; } - frm.get_field('import_log_preview').$wrapper.html(` - - - - - - - ${rows} -
${__('Row Number')}${__('Status')}${__('Message')}
- `); - } else if (logs.length > 0) { - frm.toggle_display('import_log_section', false); - - frm.add_custom_button(__('Export Import Log'), () => - frm.trigger('export_import_log') - ); - } + frappe.call({ + 'method': 'frappe.client.get_count', + 'args': { + 'doctype': 'Data Import Log', + 'filters': { + 'data_import': frm.doc.name + } + }, + 'callback': function(r) { + let count = r.message; + if(count < 5000) { + frm.trigger('render_import_log'); + } else { + frm.toggle_display('import_log_section', false); + frm.add_custom_button(__('Export Import Log'), () => + frm.trigger('export_import_log') + ); + } + } + }); }, }); diff --git a/frappe/core/doctype/data_import/data_import.json b/frappe/core/doctype/data_import/data_import.json index fe6fb90481..997c14cf7f 100644 --- a/frappe/core/doctype/data_import/data_import.json +++ b/frappe/core/doctype/data_import/data_import.json @@ -25,7 +25,6 @@ "section_import_preview", "import_preview", "import_log_section", - "import_log", "show_failed_logs", "import_log_preview" ], @@ -54,7 +53,7 @@ "fieldtype": "Attach", "in_list_view": 1, "label": "Import File", - "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)" + "read_only_depends_on": "eval: ['Success', 'Partial Success', 'Partially Completed'].includes(doc.status)" }, { "fieldname": "import_preview", @@ -78,12 +77,6 @@ "options": "JSON", "read_only": 1 }, - { - "fieldname": "import_log", - "fieldtype": "Code", - "label": "Import Log", - "options": "JSON" - }, { "fieldname": "import_log_section", "fieldtype": "Section Break", @@ -100,7 +93,7 @@ "fieldtype": "Select", "hidden": 1, "label": "Status", - "options": "Pending\nSuccess\nPartial Success\nError", + "options": "Pending\nSuccess\nPartial Success\nError\nPartially Completed", "read_only": 1 }, { @@ -169,7 +162,7 @@ ], "hide_toolbar": 1, "links": [], - "modified": "2021-04-11 01:50:42.074623", + "modified": "2021-12-26 23:51:00.280093", "modified_by": "Administrator", "module": "Core", "name": "Data Import", diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index fb6c5b147a..166a39b39e 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -29,10 +29,6 @@ class DataImport(Document): self.validate_google_sheets_url() def validate_import_file(self): - - if self.status == 'Pending' and self.import_log: - frappe.throw(_("File cannot be changed for partially completed imports. Either continue import or make a fresh import")) - if self.import_file: # validate template self.get_importer() @@ -93,10 +89,20 @@ class DataImport(Document): @frappe.whitelist() def get_preview_from_template(data_import, import_file=None, google_sheets_url=None): - return frappe.get_doc("Data Import", data_import).get_preview_from_template( + preview_data = frappe.get_doc("Data Import", data_import).get_preview_from_template( import_file, google_sheets_url ) + # get first 10 import log if any + import_log = frappe.db.get_all("Data Import Log", fields=["row_indexes"], + filters={"data_import": data_import}, + order_by="log_index", limit=10) + + return { + 'preview_data': preview_data, + 'import_log': import_log + } + @frappe.whitelist() def form_start_import(data_import): @@ -157,6 +163,23 @@ def download_import_log(data_import_name): data_import = frappe.get_doc("Data Import", data_import_name) data_import.download_import_log() +@frappe.whitelist() +def get_import_status(data_import_name): + import_status = {} + + logs = frappe.get_all('Data Import Log', fields=['count(*) as count', 'success'], + filters={'data_import': data_import_name}, + group_by='success') + + for log in logs: + if log.get('success'): + import_status['success'] = log.get('count') + else: + import_status['failed'] = log.get('count') + + import_status['total_records'] = len(logs) + + return import_status def import_file( doctype, file_path, import_type, submit_after_import=False, console=False diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 706c40c910..06f6bcd3f3 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -58,7 +58,6 @@ class Importer: frappe.flags.in_import = True frappe.flags.mute_emails = self.data_import.mute_emails - self.data_import.db_set("status", "Pending") self.data_import.db_set("template_warnings", "") def import_data(self): @@ -79,13 +78,14 @@ class Importer: return # setup import log - if self.data_import.import_log: - import_log = frappe.parse_json(self.data_import.import_log) - else: - import_log = [] + import_log = frappe.db.get_all("Data Import Log", fields=["row_indexes", "success", "log_index"], + filters={"data_import": self.data_import.name}, + order_by="log_index") + + log_index = 0 # Do not remove rows in case of retry after an error or pending data import - if self.data_import.status == 'Partial Success': + if self.data_import.status == "Partial Success": # remove previous failures from import log only in case of retry after partial success import_log = [log for log in import_log if log.get("success")] @@ -93,8 +93,10 @@ class Importer: imported_rows = [] for log in import_log: log = frappe._dict(log) - if log.success or self.data_import.status == 'Pending': - imported_rows += log.row_indexes + if log.success or self.data_import.status == "Partially Completed": + imported_rows += json.loads(log.row_indexes) + + log_index = log.log_index # start import total_payload_count = len(payloads) @@ -148,33 +150,39 @@ class Importer: }, ) - import_log.append( - frappe._dict(success=True, docname=doc.name, row_indexes=row_indexes) - ) + import_log.append(create_import_log(self.data_import.name, log_index, { + 'success': True, + 'docname': doc.name, + 'row_indexes': row_indexes + })) - # Update import log after every successful import - # This is done for cases where the background job might get terminated due to timeout - # In such cases the job can be rerun only for pending rows by referring to import logs - self.data_import.db_set("import_log", json.dumps(import_log)) + log_index += 1 + + if not self.data_import.status == "Partially Completed": + self.data_import.db_set("status", "Partially Completed") # commit after every successful import frappe.db.commit() except Exception: - import_log.append( - frappe._dict( - success=False, - exception=frappe.get_traceback(), - messages=frappe.local.message_log, - row_indexes=row_indexes, - ) - ) frappe.clear_messages() # rollback if exception frappe.db.rollback() + import_log.append(create_import_log(self.data_import.name, log_index, { + 'success': False, + 'exception': frappe.get_traceback(), + 'messages': frappe.local.message_log, + 'row_indexes': row_indexes + })) + + # commit after creating log for failure + frappe.db.commit() + log_index += 1 + # set status failures = [log for log in import_log if not log.get("success")] + print(failures, "$#$#$#") if len(failures) == total_payload_count: status = "Pending" elif len(failures) > 0: @@ -186,7 +194,6 @@ class Importer: self.print_import_log(import_log) else: self.data_import.db_set("status", status) - self.data_import.db_set("import_log", json.dumps(import_log)) self.after_import() @@ -277,13 +284,17 @@ class Importer: if not self.data_import: return - import_log = frappe.parse_json(self.data_import.import_log or "[]") + + import_log = frappe.db.get_all("Data Import Log", fields=["row_indexes", "success", "messages", "exception", "docname"], + filters={"data_import": self.data_import.name}, + order_by="log_index") + header_row = ["Row Numbers", "Status", "Message", "Exception"] rows = [header_row] for log in import_log: - row_number = log.get("row_indexes")[0] + row_number = json.loads(log.get("row_indexes"))[0] status = "Success" if log.get('success') else "Failure" message = "Successfully Imported {0}".format(log.get('docname')) if log.get('success') else \ log.get("messages") @@ -1200,3 +1211,17 @@ def df_as_json(df): def get_select_options(df): return [d for d in (df.options or "").split("\n") if d] + +def create_import_log(data_import, log_index, log_details): + return frappe.get_doc({ + 'doctype': 'Data Import Log', + 'log_index': log_index, + 'success': log_details.get('success'), + 'data_import': data_import, + 'row_indexes': json.dumps(log_details.get('row_indexes')), + 'docname': log_details.get('docname'), + 'message': json.dumps(log_details.get('messages')) if log_details.get('messages') else None, + 'exception': log_details.get('exception') + }).insert(ignore_permissions=True) + + diff --git a/frappe/public/js/frappe/data_import/import_preview.js b/frappe/public/js/frappe/data_import/import_preview.js index 2264042539..b153718c70 100644 --- a/frappe/public/js/frappe/data_import/import_preview.js +++ b/frappe/public/js/frappe/data_import/import_preview.js @@ -331,7 +331,7 @@ frappe.data_import.ImportPreview = class ImportPreview { is_row_imported(row) { let serial_no = row[0].content; return this.import_log.find(log => { - return log.success && log.row_indexes.includes(serial_no); + return log.success && JSON.parse(log.row_indexes || '[]').includes(serial_no); }); } }; From 48f8af5bf8d3c5bf210dcf5d08d389a794762845 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 27 Dec 2021 12:26:05 +0530 Subject: [PATCH 11/19] fix: import log initialization --- frappe/core/doctype/data_import/importer.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 06f6bcd3f3..6d88d25286 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -80,7 +80,7 @@ class Importer: # setup import log import_log = frappe.db.get_all("Data Import Log", fields=["row_indexes", "success", "log_index"], filters={"data_import": self.data_import.name}, - order_by="log_index") + order_by="log_index") or [] log_index = 0 @@ -182,7 +182,6 @@ class Importer: # set status failures = [log for log in import_log if not log.get("success")] - print(failures, "$#$#$#") if len(failures) == total_payload_count: status = "Pending" elif len(failures) > 0: From e925d26c89583ea39a56d6584aff37ee6921795a Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 27 Dec 2021 20:31:33 +0530 Subject: [PATCH 12/19] fix: Error message display and linting issues --- .../core/doctype/data_import/data_import.js | 21 +++++++++++------ frappe/core/doctype/data_import/importer.py | 13 +++++++---- .../core/doctype/data_import/test_importer.py | 23 +++++++++++-------- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index e395a199b8..bf3bbac4aa 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -146,12 +146,12 @@ frappe.ui.form.on('Data Import', { let failed_records = cint(r.message.failed); let total_records = cint(r.message.total_records); - if (!total_records) return + if (!total_records) return; let message; if (failed_records === 0) { let message_args = [successful_records]; - if (me.frm.doc.import_type === 'Insert New Records') { + if (frm.doc.import_type === 'Insert New Records') { message = successful_records > 1 ? __('Successfully imported {0} records.', message_args) @@ -428,7 +428,12 @@ frappe.ui.form.on('Data Import', { 'page_limit_length': 5000 }, callback: function(r) { - let logs = r.message + let logs = r.message; + + if (logs.length === 0) return; + + frm.toggle_display('import_log_section', true); + let rows = logs .map(log => { let html = ''; @@ -451,7 +456,7 @@ frappe.ui.form.on('Data Import', { ]); } } else { - let messages = (log.messages || []) + let messages = (JSON.parse(log.messages || '[]')) .map(JSON.parse) .map(m => { let title = m.title ? `${m.title}` : ''; @@ -505,11 +510,13 @@ frappe.ui.form.on('Data Import', { ${rows} `); - } - }); + } + }); }, show_import_log(frm) { + frm.toggle_display('import_log_section', false); + if (frm.import_in_progress) { return; } @@ -524,7 +531,7 @@ frappe.ui.form.on('Data Import', { }, 'callback': function(r) { let count = r.message; - if(count < 5000) { + if (count < 5000) { frm.trigger('render_import_log'); } else { frm.toggle_display('import_log_section', false); diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 6d88d25286..6915a4e5d1 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -165,14 +165,16 @@ class Importer: frappe.db.commit() except Exception: + messages = frappe.local.message_log frappe.clear_messages() + # rollback if exception frappe.db.rollback() import_log.append(create_import_log(self.data_import.name, log_index, { 'success': False, 'exception': frappe.get_traceback(), - 'messages': frappe.local.message_log, + 'messages': messages, 'row_indexes': row_indexes })) @@ -262,11 +264,14 @@ class Importer: if not self.data_import: return - import_log = frappe.parse_json(self.data_import.import_log or "[]") + import_log = frappe.db.get_all("Data Import Log", fields=["row_indexes", "success"], + filters={"data_import": self.data_import.name}, + order_by="log_index") or [] + failures = [log for log in import_log if not log.get("success")] row_indexes = [] for f in failures: - row_indexes.extend(f.get("row_indexes", [])) + row_indexes.extend(json.loads(f.get("row_indexes", []))) # de duplicate row_indexes = list(set(row_indexes)) @@ -1219,7 +1224,7 @@ def create_import_log(data_import, log_index, log_details): 'data_import': data_import, 'row_indexes': json.dumps(log_details.get('row_indexes')), 'docname': log_details.get('docname'), - 'message': json.dumps(log_details.get('messages')) if log_details.get('messages') else None, + 'messages': json.dumps(log_details.get('messages', '[]')), 'exception': log_details.get('exception') }).insert(ignore_permissions=True) diff --git a/frappe/core/doctype/data_import/test_importer.py b/frappe/core/doctype/data_import/test_importer.py index e1bc0e7ca5..d09b4f69e7 100644 --- a/frappe/core/doctype/data_import/test_importer.py +++ b/frappe/core/doctype/data_import/test_importer.py @@ -3,6 +3,7 @@ # License: MIT. See LICENSE import unittest import frappe +import json from frappe.core.doctype.data_import.importer import Importer from frappe.utils import getdate, format_duration @@ -60,15 +61,19 @@ class TestImporter(unittest.TestCase): frappe.local.message_log = [] data_import.start_import() data_import.reload() - import_log = frappe.parse_json(data_import.import_log) - self.assertEqual(import_log[0]['row_indexes'], [2,3]) - expected_error = "Error: Child 1 of DocType for Import Row #1: Value missing for: Child Title" - self.assertEqual(frappe.parse_json(import_log[0]['messages'][0])['message'], expected_error) - expected_error = "Error: Child 1 of DocType for Import Row #2: Value missing for: Child Title" - self.assertEqual(frappe.parse_json(import_log[0]['messages'][1])['message'], expected_error) - - self.assertEqual(import_log[1]['row_indexes'], [4]) - self.assertEqual(frappe.parse_json(import_log[1]['messages'][0])['message'], "Title is required") + + import_log = frappe.db.get_all("Data Import Log", fields=["row_indexes", "success", "messages", "exception", "docname"], + filters={"data_import": data_import.name}, + order_by="log_index") + + self.assertEqual(frappe.parse_json(import_log[0]['row_indexes']), [2,3]) + expected_error = "Error: Child 1 of DocType for Import Row #1: Value missing for: Child Title" + self.assertEqual(frappe.parse_json(frappe.parse_json(import_log[0]['messages'])[0])['message'], expected_error) + expected_error = "Error: Child 1 of DocType for Import Row #2: Value missing for: Child Title" + self.assertEqual(frappe.parse_json(frappe.parse_json(import_log[0]['messages'])[1])['message'], expected_error) + + self.assertEqual(frappe.parse_json(import_log[1]['row_indexes']), [4]) + self.assertEqual(frappe.parse_json(frappe.parse_json(import_log[1]['messages'])[0])['message'], "Title is required") def test_data_import_update(self): existing_doc = frappe.get_doc( From 4a0343fdf18ba0a1a6447244def3c9566d5759b9 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 3 Jan 2022 13:02:14 +0530 Subject: [PATCH 13/19] fix: Total status count --- frappe/core/doctype/data_import/data_import.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index 166a39b39e..4aacb6925b 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -12,6 +12,7 @@ from frappe.model.document import Document from frappe.modules.import_file import import_file_by_path from frappe.utils.background_jobs import enqueue from frappe.utils.csvutils import validate_google_sheets_url +from frappe.utils import cint class DataImport(Document): @@ -177,7 +178,8 @@ def get_import_status(data_import_name): else: import_status['failed'] = log.get('count') - import_status['total_records'] = len(logs) + import_status['total_records'] = cint(import_status.get('success')) + \ + cint(import_status.get('failed')) return import_status From f9126bccc998926a1e5e5f7e6edd653054281176 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Tue, 25 Jan 2022 11:44:07 +0530 Subject: [PATCH 14/19] fix: Show import logs in data import doctype --- frappe/core/doctype/data_import/data_import.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index bf3bbac4aa..4a1ac7c255 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -425,7 +425,8 @@ frappe.ui.form.on('Data Import', { 'data_import': frm.doc.name }, 'fields': ['success', 'docname', 'messages', 'exception', 'row_indexes'], - 'page_limit_length': 5000 + 'limit_page_length': 5000, + 'order_by': 'log_index' }, callback: function(r) { let logs = r.message; From cafd513c494c69d94266dcb3bdbec0a70e962514 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 31 Jan 2022 11:03:08 +0530 Subject: [PATCH 15/19] fix: Remove partially completed status --- .../core/doctype/data_import/data_import.js | 9 +- .../core/doctype/data_import/data_import.json | 84 ++++++++++++++----- .../doctype/data_import/data_import_list.js | 1 - frappe/core/doctype/data_import/importer.py | 8 +- 4 files changed, 67 insertions(+), 35 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 4a1ac7c255..6ea95fba02 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -112,14 +112,7 @@ frappe.ui.form.on('Data Import', { if (!frm.is_new() && (frm.has_import_file())) { let label = frm.doc.status === 'Pending' ? __('Start Import') : __('Retry'); - - if (frm.doc.status == 'Partially Completed') { - label = __('Continue Import'); - } - - if (!frm.import_in_progress) { - frm.page.set_primary_action(label, () => frm.events.start_import(frm)); - } + frm.page.set_primary_action(label, () => frm.events.start_import(frm)); } else { frm.page.set_primary_action(__('Save'), () => frm.save()); } diff --git a/frappe/core/doctype/data_import/data_import.json b/frappe/core/doctype/data_import/data_import.json index 997c14cf7f..e8b8b7a546 100644 --- a/frappe/core/doctype/data_import/data_import.json +++ b/frappe/core/doctype/data_import/data_import.json @@ -36,7 +36,9 @@ "label": "Document Type", "options": "DocType", "reqd": 1, - "set_only_once": 1 + "set_only_once": 1, + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "import_type", @@ -45,7 +47,9 @@ "label": "Import Type", "options": "\nInsert New Records\nUpdate Existing Records", "reqd": 1, - "set_only_once": 1 + "set_only_once": 1, + "show_days": 1, + "show_seconds": 1 }, { "depends_on": "eval:!doc.__islocal", @@ -53,21 +57,29 @@ "fieldtype": "Attach", "in_list_view": 1, "label": "Import File", - "read_only_depends_on": "eval: ['Success', 'Partial Success', 'Partially Completed'].includes(doc.status)" + "read_only_depends_on": "eval: ['Success', 'Partial Success', 'Partially Completed'].includes(doc.status)", + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "import_preview", "fieldtype": "HTML", - "label": "Import Preview" + "label": "Import Preview", + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "section_import_preview", "fieldtype": "Section Break", - "label": "Preview" + "label": "Preview", + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "column_break_5", - "fieldtype": "Column Break" + "fieldtype": "Column Break", + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "template_options", @@ -75,17 +87,23 @@ "hidden": 1, "label": "Template Options", "options": "JSON", - "read_only": 1 + "read_only": 1, + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "import_log_section", "fieldtype": "Section Break", - "label": "Import Log" + "label": "Import Log", + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "import_log_preview", "fieldtype": "HTML", - "label": "Import Log Preview" + "label": "Import Log Preview", + "show_days": 1, + "show_seconds": 1 }, { "default": "Pending", @@ -93,57 +111,75 @@ "fieldtype": "Select", "hidden": 1, "label": "Status", - "options": "Pending\nSuccess\nPartial Success\nError\nPartially Completed", - "read_only": 1 + "options": "Pending\nSuccess\nPartial Success\nError", + "read_only": 1, + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "template_warnings", "fieldtype": "Code", "hidden": 1, "label": "Template Warnings", - "options": "JSON" + "options": "JSON", + "show_days": 1, + "show_seconds": 1 }, { "default": "0", "fieldname": "submit_after_import", "fieldtype": "Check", "label": "Submit After Import", - "set_only_once": 1 + "set_only_once": 1, + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "import_warnings_section", "fieldtype": "Section Break", - "label": "Import File Errors and Warnings" + "label": "Import File Errors and Warnings", + "show_days": 1, + "show_seconds": 1 }, { "fieldname": "import_warnings", "fieldtype": "HTML", - "label": "Import Warnings" + "label": "Import Warnings", + "show_days": 1, + "show_seconds": 1 }, { "depends_on": "eval:!doc.__islocal", "fieldname": "download_template", "fieldtype": "Button", - "label": "Download Template" + "label": "Download Template", + "show_days": 1, + "show_seconds": 1 }, { "default": "1", "fieldname": "mute_emails", "fieldtype": "Check", "label": "Don't Send Emails", - "set_only_once": 1 + "set_only_once": 1, + "show_days": 1, + "show_seconds": 1 }, { "default": "0", "fieldname": "show_failed_logs", "fieldtype": "Check", - "label": "Show Failed Logs" + "label": "Show Failed Logs", + "show_days": 1, + "show_seconds": 1 }, { "depends_on": "eval:!doc.__islocal && !doc.import_file", "fieldname": "html_5", "fieldtype": "HTML", - "options": "
Or
" + "options": "
Or
", + "show_days": 1, + "show_seconds": 1 }, { "depends_on": "eval:!doc.__islocal && !doc.import_file\n", @@ -151,18 +187,22 @@ "fieldname": "google_sheets_url", "fieldtype": "Data", "label": "Import from Google Sheets", - "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)" + "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)", + "show_days": 1, + "show_seconds": 1 }, { "depends_on": "eval:doc.google_sheets_url && !doc.__unsaved && ['Success', 'Partial Success'].includes(doc.status)", "fieldname": "refresh_google_sheet", "fieldtype": "Button", - "label": "Refresh Google Sheet" + "label": "Refresh Google Sheet", + "show_days": 1, + "show_seconds": 1 } ], "hide_toolbar": 1, "links": [], - "modified": "2021-12-26 23:51:00.280093", + "modified": "2022-01-31 10:50:13.015426", "modified_by": "Administrator", "module": "Core", "name": "Data Import", diff --git a/frappe/core/doctype/data_import/data_import_list.js b/frappe/core/doctype/data_import/data_import_list.js index 84b0f0ada1..6ab750ba25 100644 --- a/frappe/core/doctype/data_import/data_import_list.js +++ b/frappe/core/doctype/data_import/data_import_list.js @@ -19,7 +19,6 @@ frappe.listview_settings['Data Import'] = { 'Pending': 'orange', 'Not Started': 'orange', 'Partial Success': 'orange', - 'Partially Completed': 'orange', 'Success': 'green', 'In Progress': 'orange', 'Error': 'red' diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 6915a4e5d1..8c98099c1a 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -85,7 +85,7 @@ class Importer: log_index = 0 # Do not remove rows in case of retry after an error or pending data import - if self.data_import.status == "Partial Success": + if self.data_import.status == "Partial Success" and len(import_log) > len(payloads): # remove previous failures from import log only in case of retry after partial success import_log = [log for log in import_log if log.get("success")] @@ -93,7 +93,7 @@ class Importer: imported_rows = [] for log in import_log: log = frappe._dict(log) - if log.success or self.data_import.status == "Partially Completed": + if log.success or len(import_log) < len(payloads): imported_rows += json.loads(log.row_indexes) log_index = log.log_index @@ -158,8 +158,8 @@ class Importer: log_index += 1 - if not self.data_import.status == "Partially Completed": - self.data_import.db_set("status", "Partially Completed") + if not self.data_import.status == "Partial Success": + self.data_import.db_set("status", "Partial Success") # commit after every successful import frappe.db.commit() From 2ca5c390d062f42fca95dc61cce154090b749c8b Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Tue, 1 Feb 2022 23:00:41 +0530 Subject: [PATCH 16/19] fix: Use db insert for logs --- .../core/doctype/data_import/data_import.js | 9 +- .../core/doctype/data_import/data_import.json | 420 ++++++++---------- .../core/doctype/data_import/data_import.py | 25 +- frappe/core/doctype/data_import/importer.py | 21 +- .../data_import_log/data_import_log.json | 4 +- 5 files changed, 229 insertions(+), 250 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 6ea95fba02..1460f8698e 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -278,14 +278,15 @@ frappe.ui.form.on('Data Import', { } }) .then(r => { - let preview_data = r.message.preview_data; - let import_log = r.message.import_log; - frm.events.show_import_preview(frm, preview_data, import_log); + let preview_data = r.message; + frm.events.show_import_preview(frm, preview_data); frm.events.show_import_warnings(frm, preview_data); }); }, - show_import_preview(frm, preview_data, import_log) { + show_import_preview(frm, preview_data) { + let import_log = preview_data.import_log; + if ( frm.import_preview && frm.import_preview.doctype === frm.doc.reference_doctype diff --git a/frappe/core/doctype/data_import/data_import.json b/frappe/core/doctype/data_import/data_import.json index e8b8b7a546..9e948dac8c 100644 --- a/frappe/core/doctype/data_import/data_import.json +++ b/frappe/core/doctype/data_import/data_import.json @@ -1,227 +1,197 @@ { - "actions": [], - "autoname": "format:{reference_doctype} Import on {creation}", - "beta": 1, - "creation": "2019-08-04 14:16:08.318714", - "doctype": "DocType", - "editable_grid": 1, - "engine": "InnoDB", - "field_order": [ - "reference_doctype", - "import_type", - "download_template", - "import_file", - "html_5", - "google_sheets_url", - "refresh_google_sheet", - "column_break_5", - "status", - "submit_after_import", - "mute_emails", - "template_options", - "import_warnings_section", - "template_warnings", - "import_warnings", - "section_import_preview", - "import_preview", - "import_log_section", - "show_failed_logs", - "import_log_preview" - ], - "fields": [ - { - "fieldname": "reference_doctype", - "fieldtype": "Link", - "in_list_view": 1, - "label": "Document Type", - "options": "DocType", - "reqd": 1, - "set_only_once": 1, - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "import_type", - "fieldtype": "Select", - "in_list_view": 1, - "label": "Import Type", - "options": "\nInsert New Records\nUpdate Existing Records", - "reqd": 1, - "set_only_once": 1, - "show_days": 1, - "show_seconds": 1 - }, - { - "depends_on": "eval:!doc.__islocal", - "fieldname": "import_file", - "fieldtype": "Attach", - "in_list_view": 1, - "label": "Import File", - "read_only_depends_on": "eval: ['Success', 'Partial Success', 'Partially Completed'].includes(doc.status)", - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "import_preview", - "fieldtype": "HTML", - "label": "Import Preview", - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "section_import_preview", - "fieldtype": "Section Break", - "label": "Preview", - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "column_break_5", - "fieldtype": "Column Break", - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "template_options", - "fieldtype": "Code", - "hidden": 1, - "label": "Template Options", - "options": "JSON", - "read_only": 1, - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "import_log_section", - "fieldtype": "Section Break", - "label": "Import Log", - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "import_log_preview", - "fieldtype": "HTML", - "label": "Import Log Preview", - "show_days": 1, - "show_seconds": 1 - }, - { - "default": "Pending", - "fieldname": "status", - "fieldtype": "Select", - "hidden": 1, - "label": "Status", - "options": "Pending\nSuccess\nPartial Success\nError", - "read_only": 1, - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "template_warnings", - "fieldtype": "Code", - "hidden": 1, - "label": "Template Warnings", - "options": "JSON", - "show_days": 1, - "show_seconds": 1 - }, - { - "default": "0", - "fieldname": "submit_after_import", - "fieldtype": "Check", - "label": "Submit After Import", - "set_only_once": 1, - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "import_warnings_section", - "fieldtype": "Section Break", - "label": "Import File Errors and Warnings", - "show_days": 1, - "show_seconds": 1 - }, - { - "fieldname": "import_warnings", - "fieldtype": "HTML", - "label": "Import Warnings", - "show_days": 1, - "show_seconds": 1 - }, - { - "depends_on": "eval:!doc.__islocal", - "fieldname": "download_template", - "fieldtype": "Button", - "label": "Download Template", - "show_days": 1, - "show_seconds": 1 - }, - { - "default": "1", - "fieldname": "mute_emails", - "fieldtype": "Check", - "label": "Don't Send Emails", - "set_only_once": 1, - "show_days": 1, - "show_seconds": 1 - }, - { - "default": "0", - "fieldname": "show_failed_logs", - "fieldtype": "Check", - "label": "Show Failed Logs", - "show_days": 1, - "show_seconds": 1 - }, - { - "depends_on": "eval:!doc.__islocal && !doc.import_file", - "fieldname": "html_5", - "fieldtype": "HTML", - "options": "
Or
", - "show_days": 1, - "show_seconds": 1 - }, - { - "depends_on": "eval:!doc.__islocal && !doc.import_file\n", - "description": "Must be a publicly accessible Google Sheets URL", - "fieldname": "google_sheets_url", - "fieldtype": "Data", - "label": "Import from Google Sheets", - "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)", - "show_days": 1, - "show_seconds": 1 - }, - { - "depends_on": "eval:doc.google_sheets_url && !doc.__unsaved && ['Success', 'Partial Success'].includes(doc.status)", - "fieldname": "refresh_google_sheet", - "fieldtype": "Button", - "label": "Refresh Google Sheet", - "show_days": 1, - "show_seconds": 1 - } - ], - "hide_toolbar": 1, - "links": [], - "modified": "2022-01-31 10:50:13.015426", - "modified_by": "Administrator", - "module": "Core", - "name": "Data Import", - "owner": "Administrator", - "permissions": [ - { - "create": 1, - "delete": 1, - "email": 1, - "export": 1, - "print": 1, - "read": 1, - "report": 1, - "role": "System Manager", - "share": 1, - "write": 1 - } - ], - "sort_field": "modified", - "sort_order": "DESC", - "track_changes": 1 + "actions": [], + "autoname": "format:{reference_doctype} Import on {creation}", + "beta": 1, + "creation": "2019-08-04 14:16:08.318714", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "reference_doctype", + "import_type", + "download_template", + "import_file", + "payload_count", + "html_5", + "google_sheets_url", + "refresh_google_sheet", + "column_break_5", + "status", + "submit_after_import", + "mute_emails", + "template_options", + "import_warnings_section", + "template_warnings", + "import_warnings", + "section_import_preview", + "import_preview", + "import_log_section", + "show_failed_logs", + "import_log_preview" + ], + "fields": [ + { + "fieldname": "reference_doctype", + "fieldtype": "Link", + "in_list_view": 1, + "label": "Document Type", + "options": "DocType", + "reqd": 1, + "set_only_once": 1 + }, + { + "fieldname": "import_type", + "fieldtype": "Select", + "in_list_view": 1, + "label": "Import Type", + "options": "\nInsert New Records\nUpdate Existing Records", + "reqd": 1, + "set_only_once": 1 + }, + { + "depends_on": "eval:!doc.__islocal", + "fieldname": "import_file", + "fieldtype": "Attach", + "in_list_view": 1, + "label": "Import File", + "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)" + }, + { + "fieldname": "import_preview", + "fieldtype": "HTML", + "label": "Import Preview" + }, + { + "fieldname": "section_import_preview", + "fieldtype": "Section Break", + "label": "Preview" + }, + { + "fieldname": "column_break_5", + "fieldtype": "Column Break" + }, + { + "fieldname": "template_options", + "fieldtype": "Code", + "hidden": 1, + "label": "Template Options", + "options": "JSON", + "read_only": 1 + }, + { + "fieldname": "import_log_section", + "fieldtype": "Section Break", + "label": "Import Log" + }, + { + "fieldname": "import_log_preview", + "fieldtype": "HTML", + "label": "Import Log Preview" + }, + { + "default": "Pending", + "fieldname": "status", + "fieldtype": "Select", + "hidden": 1, + "label": "Status", + "options": "Pending\nSuccess\nPartial Success\nError", + "read_only": 1 + }, + { + "fieldname": "template_warnings", + "fieldtype": "Code", + "hidden": 1, + "label": "Template Warnings", + "options": "JSON" + }, + { + "default": "0", + "fieldname": "submit_after_import", + "fieldtype": "Check", + "label": "Submit After Import", + "set_only_once": 1 + }, + { + "fieldname": "import_warnings_section", + "fieldtype": "Section Break", + "label": "Import File Errors and Warnings" + }, + { + "fieldname": "import_warnings", + "fieldtype": "HTML", + "label": "Import Warnings" + }, + { + "depends_on": "eval:!doc.__islocal", + "fieldname": "download_template", + "fieldtype": "Button", + "label": "Download Template" + }, + { + "default": "1", + "fieldname": "mute_emails", + "fieldtype": "Check", + "label": "Don't Send Emails", + "set_only_once": 1 + }, + { + "default": "0", + "fieldname": "show_failed_logs", + "fieldtype": "Check", + "label": "Show Failed Logs" + }, + { + "depends_on": "eval:!doc.__islocal && !doc.import_file", + "fieldname": "html_5", + "fieldtype": "HTML", + "options": "
Or
" + }, + { + "depends_on": "eval:!doc.__islocal && !doc.import_file\n", + "description": "Must be a publicly accessible Google Sheets URL", + "fieldname": "google_sheets_url", + "fieldtype": "Data", + "label": "Import from Google Sheets", + "read_only_depends_on": "eval: ['Success', 'Partial Success'].includes(doc.status)" + }, + { + "depends_on": "eval:doc.google_sheets_url && !doc.__unsaved && ['Success', 'Partial Success'].includes(doc.status)", + "fieldname": "refresh_google_sheet", + "fieldtype": "Button", + "label": "Refresh Google Sheet" + }, + { + "fieldname": "payload_count", + "fieldtype": "Int", + "hidden": 1, + "label": "Payload Count", + "read_only": 1 + } + ], + "hide_toolbar": 1, + "links": [], + "modified": "2022-02-01 20:08:37.624914", + "modified_by": "Administrator", + "module": "Core", + "name": "Data Import", + "naming_rule": "Expression", + "owner": "Administrator", + "permissions": [ + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "System Manager", + "share": 1, + "write": 1 + } + ], + "sort_field": "modified", + "sort_order": "DESC", + "states": [], + "track_changes": 1 } \ No newline at end of file diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index 4aacb6925b..0c8ba072a1 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -28,6 +28,7 @@ class DataImport(Document): self.validate_import_file() self.validate_google_sheets_url() + self.set_payload_count() def validate_import_file(self): if self.import_file: @@ -39,6 +40,12 @@ class DataImport(Document): return validate_google_sheets_url(self.google_sheets_url) + def set_payload_count(self): + if self.import_file: + i = self.get_importer() + payloads = i.import_file.get_payloads_for_import() + self.payload_count = len(payloads) + @frappe.whitelist() def get_preview_from_template(self, import_file=None, google_sheets_url=None): if import_file: @@ -90,21 +97,10 @@ class DataImport(Document): @frappe.whitelist() def get_preview_from_template(data_import, import_file=None, google_sheets_url=None): - preview_data = frappe.get_doc("Data Import", data_import).get_preview_from_template( + return frappe.get_doc("Data Import", data_import).get_preview_from_template( import_file, google_sheets_url ) - # get first 10 import log if any - import_log = frappe.db.get_all("Data Import Log", fields=["row_indexes"], - filters={"data_import": data_import}, - order_by="log_index", limit=10) - - return { - 'preview_data': preview_data, - 'import_log': import_log - } - - @frappe.whitelist() def form_start_import(data_import): return frappe.get_doc("Data Import", data_import).start_import() @@ -172,14 +168,15 @@ def get_import_status(data_import_name): filters={'data_import': data_import_name}, group_by='success') + total_payload_count = frappe.db.get_value('Data Import', data_import_name, 'payload_count') + for log in logs: if log.get('success'): import_status['success'] = log.get('count') else: import_status['failed'] = log.get('count') - import_status['total_records'] = cint(import_status.get('success')) + \ - cint(import_status.get('failed')) + import_status['total_records'] = total_payload_count return import_status diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 8c98099c1a..d13f4c1389 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -47,7 +47,13 @@ class Importer: ) def get_data_for_import_preview(self): - return self.import_file.get_data_for_import_preview() + out = self.import_file.get_data_for_import_preview() + + out.import_log = frappe.db.get_all("Data Import Log", fields=["row_indexes", "success"], + filters={"data_import": self.data_import.name}, + order_by="log_index", limit=10) + + return out def before_import(self): # set user lang for translations @@ -85,7 +91,7 @@ class Importer: log_index = 0 # Do not remove rows in case of retry after an error or pending data import - if self.data_import.status == "Partial Success" and len(import_log) > len(payloads): + if self.data_import.status == "Partial Success" and len(import_log) >= self.data_import.payload_count: # remove previous failures from import log only in case of retry after partial success import_log = [log for log in import_log if log.get("success")] @@ -93,7 +99,7 @@ class Importer: imported_rows = [] for log in import_log: log = frappe._dict(log) - if log.success or len(import_log) < len(payloads): + if log.success or len(import_log) < self.data_import.payload_count: imported_rows += json.loads(log.row_indexes) log_index = log.log_index @@ -182,6 +188,11 @@ class Importer: frappe.db.commit() log_index += 1 + # Logs are db inserted directly so will have to be fetched again + import_log = frappe.db.get_all("Data Import Log", fields=["row_indexes", "success", "log_index"], + filters={"data_import": self.data_import.name}, + order_by="log_index") or [] + # set status failures = [log for log in import_log if not log.get("success")] if len(failures) == total_payload_count: @@ -1217,7 +1228,7 @@ def get_select_options(df): return [d for d in (df.options or "").split("\n") if d] def create_import_log(data_import, log_index, log_details): - return frappe.get_doc({ + frappe.get_doc({ 'doctype': 'Data Import Log', 'log_index': log_index, 'success': log_details.get('success'), @@ -1226,6 +1237,6 @@ def create_import_log(data_import, log_index, log_details): 'docname': log_details.get('docname'), 'messages': json.dumps(log_details.get('messages', '[]')), 'exception': log_details.get('exception') - }).insert(ignore_permissions=True) + }).db_insert() diff --git a/frappe/core/doctype/data_import_log/data_import_log.json b/frappe/core/doctype/data_import_log/data_import_log.json index a39eb9be1d..8747728945 100644 --- a/frappe/core/doctype/data_import_log/data_import_log.json +++ b/frappe/core/doctype/data_import_log/data_import_log.json @@ -3,7 +3,7 @@ "creation": "2021-12-25 16:12:20.205889", "doctype": "DocType", "editable_grid": 1, - "engine": "InnoDB", + "engine": "MyISAM", "field_order": [ "data_import", "row_indexes", @@ -60,7 +60,7 @@ "in_create": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2021-12-27 11:19:19.646076", + "modified": "2021-12-28 11:19:19.646076", "modified_by": "Administrator", "module": "Core", "name": "Data Import Log", From 93c476ef30224529f8b3b4dc853fb3ba848aa061 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Wed, 2 Feb 2022 23:14:41 +0530 Subject: [PATCH 17/19] fix: Test case --- frappe/core/doctype/data_import/test_importer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/data_import/test_importer.py b/frappe/core/doctype/data_import/test_importer.py index d09b4f69e7..d024f950c8 100644 --- a/frappe/core/doctype/data_import/test_importer.py +++ b/frappe/core/doctype/data_import/test_importer.py @@ -67,9 +67,9 @@ class TestImporter(unittest.TestCase): order_by="log_index") self.assertEqual(frappe.parse_json(import_log[0]['row_indexes']), [2,3]) - expected_error = "Error: Child 1 of DocType for Import Row #1: Value missing for: Child Title" + expected_error = "Error: Child 1 of DocType for Import Row #1: Value missing for: Child Title" self.assertEqual(frappe.parse_json(frappe.parse_json(import_log[0]['messages'])[0])['message'], expected_error) - expected_error = "Error: Child 1 of DocType for Import Row #2: Value missing for: Child Title" + expected_error = "Error: Child 1 of DocType for Import Row #2: Value missing for: Child Title" self.assertEqual(frappe.parse_json(frappe.parse_json(import_log[0]['messages'])[1])['message'], expected_error) self.assertEqual(frappe.parse_json(import_log[1]['row_indexes']), [4]) From 977c9d1622b7d9f3383c21817c60c87cf8d4375e Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Wed, 2 Feb 2022 23:28:05 +0530 Subject: [PATCH 18/19] chore: Remove unused imports --- frappe/core/doctype/data_import/data_import.py | 1 - frappe/core/doctype/data_import/test_importer.py | 1 - 2 files changed, 2 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index 0c8ba072a1..5972e79b4d 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -12,7 +12,6 @@ from frappe.model.document import Document from frappe.modules.import_file import import_file_by_path from frappe.utils.background_jobs import enqueue from frappe.utils.csvutils import validate_google_sheets_url -from frappe.utils import cint class DataImport(Document): diff --git a/frappe/core/doctype/data_import/test_importer.py b/frappe/core/doctype/data_import/test_importer.py index d024f950c8..3f78594dd2 100644 --- a/frappe/core/doctype/data_import/test_importer.py +++ b/frappe/core/doctype/data_import/test_importer.py @@ -3,7 +3,6 @@ # License: MIT. See LICENSE import unittest import frappe -import json from frappe.core.doctype.data_import.importer import Importer from frappe.utils import getdate, format_duration From f23eb73b2a9e2c6122dcf1c0abc5328bd7870a00 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 3 Feb 2022 16:11:57 +0530 Subject: [PATCH 19/19] fix: Code cleanup --- frappe/core/doctype/data_import/data_import.js | 7 +++++-- frappe/core/doctype/data_import/importer.py | 10 ++++------ .../core/doctype/data_import_log/data_import_log.json | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.js b/frappe/core/doctype/data_import/data_import.js index 1460f8698e..dfc560a98a 100644 --- a/frappe/core/doctype/data_import/data_import.js +++ b/frappe/core/doctype/data_import/data_import.js @@ -81,7 +81,10 @@ frappe.ui.form.on('Data Import', { frm.trigger('show_import_log'); frm.trigger('show_import_warnings'); frm.trigger('toggle_submit_after_import'); - frm.trigger('show_import_status'); + + if (frm.doc.status != 'Pending') + frm.trigger('show_import_status'); + frm.trigger('show_report_error_button'); if (frm.doc.status === 'Partial Success') { @@ -164,7 +167,7 @@ frappe.ui.form.on('Data Import', { : __('Successfully imported {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args); } else { message = - successful_records> 1 + successful_records > 1 ? __('Successfully updated {0} records out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args) : __('Successfully updated {0} record out of {1}. Click on Export Errored Rows, fix the errors and import again.', message_args); } diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index d13f4c1389..107c05a66a 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -156,11 +156,11 @@ class Importer: }, ) - import_log.append(create_import_log(self.data_import.name, log_index, { + create_import_log(self.data_import.name, log_index, { 'success': True, 'docname': doc.name, 'row_indexes': row_indexes - })) + }) log_index += 1 @@ -177,15 +177,13 @@ class Importer: # rollback if exception frappe.db.rollback() - import_log.append(create_import_log(self.data_import.name, log_index, { + create_import_log(self.data_import.name, log_index, { 'success': False, 'exception': frappe.get_traceback(), 'messages': messages, 'row_indexes': row_indexes - })) + }) - # commit after creating log for failure - frappe.db.commit() log_index += 1 # Logs are db inserted directly so will have to be fetched again diff --git a/frappe/core/doctype/data_import_log/data_import_log.json b/frappe/core/doctype/data_import_log/data_import_log.json index 8747728945..b1d991f099 100644 --- a/frappe/core/doctype/data_import_log/data_import_log.json +++ b/frappe/core/doctype/data_import_log/data_import_log.json @@ -60,7 +60,7 @@ "in_create": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2021-12-28 11:19:19.646076", + "modified": "2021-12-29 11:19:19.646076", "modified_by": "Administrator", "module": "Core", "name": "Data Import Log",