From 9e5ca78d788be3d54e3797e1668c5fb29ec5e56f Mon Sep 17 00:00:00 2001 From: "Parth J. Kharwar" Date: Fri, 26 Feb 2021 13:03:42 +0530 Subject: [PATCH 01/14] feat: enables, disables and deletes notification settings when a user is modified [CU-j8wutt] (#9) --- frappe/core/doctype/user/user.py | 11 ++++++++++- .../notification_settings/notification_settings.py | 3 +++ .../doctype/energy_point_log/energy_point_log.py | 5 ++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 3f19a6ef7b..1e3a25678f 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -8,7 +8,7 @@ from frappe.utils import cint, flt, has_gravatar, escape_html, format_datetime, from frappe import throw, msgprint, _ from frappe.utils.password import update_password as _update_password, check_password from frappe.desk.notifications import clear_notifications -from frappe.desk.doctype.notification_settings.notification_settings import create_notification_settings +from frappe.desk.doctype.notification_settings.notification_settings import create_notification_settings, enable_disable_notifications from frappe.utils.user import get_system_managers from bs4 import BeautifulSoup import frappe.permissions @@ -141,11 +141,17 @@ class User(Document): if not cint(self.enabled): self.a_system_manager_should_exist() + # disable notifications if the user has been disabled + enable_disable_notifications(self.name, enabled=False) # clear sessions if disabled if not cint(self.enabled) and getattr(frappe.local, "login_manager", None): frappe.local.login_manager.logout(user=self.name) + # enable notifications if the user has been enabled + if cint(self.enabled): + enable_disable_notifications(self.name, enabled=True) + def add_system_manager_role(self): # if adding system manager, do nothing if not cint(self.enabled) or ("System Manager" in [user_role.role for user_role in @@ -358,6 +364,9 @@ class User(Document): set `user`=null where `user`=%s""", (self.name)) + # delete notification settings + frappe.delete_doc("Notification Settings", self.name, ignore_permissions=True) + def before_rename(self, old_name, new_name, merge=False): self.check_demo() diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index 34726bdf8a..4b32252ac8 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -43,6 +43,9 @@ def create_notification_settings(user): _doc.name = user _doc.insert(ignore_permissions=True) +def enable_disable_notifications(user, enabled): + frappe.set_value("Notification Settings", user, 'enabled', enabled) + @frappe.whitelist() def get_subscribed_documents(): diff --git a/frappe/social/doctype/energy_point_log/energy_point_log.py b/frappe/social/doctype/energy_point_log/energy_point_log.py index 21a0e24598..e9425cec86 100644 --- a/frappe/social/doctype/energy_point_log/energy_point_log.py +++ b/frappe/social/doctype/energy_point_log/energy_point_log.py @@ -319,7 +319,10 @@ def send_summary(timespan): from_date = getdate(from_date) to_date = getdate() - all_users = [user.email for user in get_enabled_system_users()] + + # select only those users that have energy point email notifications enabled + all_users = [user.email for user in get_enabled_system_users() if + is_email_notifications_enabled_for_type(user.name, 'Energy Point')] frappe.sendmail( subject = '{} energy points summary'.format(timespan), From a8d11f23d81ef406ebc20fa6d95de3206f137d24 Mon Sep 17 00:00:00 2001 From: Parth Kharwar Date: Fri, 26 Feb 2021 16:40:09 +0530 Subject: [PATCH 02/14] fix: adds check before enabling notification settings --- .../doctype/notification_settings/notification_settings.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index 4b32252ac8..3ee68c440c 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -44,7 +44,8 @@ def create_notification_settings(user): _doc.insert(ignore_permissions=True) def enable_disable_notifications(user, enabled): - frappe.set_value("Notification Settings", user, 'enabled', enabled) + if frappe.db.exists("Notification Settings", user): + frappe.set_value("Notification Settings", user, 'enabled', enabled) @frappe.whitelist() @@ -78,4 +79,4 @@ def get_permission_query_conditions(user): @frappe.whitelist() def set_seen_value(value, user): - frappe.db.set_value('Notification Settings', user, 'seen', value, update_modified=False) \ No newline at end of file + frappe.db.set_value('Notification Settings', user, 'seen', value, update_modified=False) From c098ed069ae2abce3e5b641786b7af51a0d449f4 Mon Sep 17 00:00:00 2001 From: Valmik Date: Thu, 4 Mar 2021 11:43:32 +0000 Subject: [PATCH 03/14] fix: use db.set_value instead of set_value to avoid permission issues --- .../desk/doctype/notification_settings/notification_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index 3ee68c440c..0831f4683d 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -45,7 +45,7 @@ def create_notification_settings(user): def enable_disable_notifications(user, enabled): if frappe.db.exists("Notification Settings", user): - frappe.set_value("Notification Settings", user, 'enabled', enabled) + frappe.db.set_value("Notification Settings", user, 'enabled', enabled) @frappe.whitelist() From 0f42096e87fde34497c7802910a709e255c2a5aa Mon Sep 17 00:00:00 2001 From: Parth Kharwar Date: Fri, 5 Mar 2021 13:13:03 +0530 Subject: [PATCH 04/14] refactor: updates notification toggle function name and if logic --- frappe/core/doctype/user/user.py | 11 +++++------ .../notification_settings/notification_settings.py | 5 +++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 1e3a25678f..23ebe7d6b1 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -8,7 +8,7 @@ from frappe.utils import cint, flt, has_gravatar, escape_html, format_datetime, from frappe import throw, msgprint, _ from frappe.utils.password import update_password as _update_password, check_password from frappe.desk.notifications import clear_notifications -from frappe.desk.doctype.notification_settings.notification_settings import create_notification_settings, enable_disable_notifications +from frappe.desk.doctype.notification_settings.notification_settings import create_notification_settings, toggle_notifications from frappe.utils.user import get_system_managers from bs4 import BeautifulSoup import frappe.permissions @@ -142,16 +142,15 @@ class User(Document): if not cint(self.enabled): self.a_system_manager_should_exist() # disable notifications if the user has been disabled - enable_disable_notifications(self.name, enabled=False) + toggle_notifications(self.name, enable=False) + else: + # enable notifications if the user has been enabled + toggle_notifications(self.name, enable=True) # clear sessions if disabled if not cint(self.enabled) and getattr(frappe.local, "login_manager", None): frappe.local.login_manager.logout(user=self.name) - # enable notifications if the user has been enabled - if cint(self.enabled): - enable_disable_notifications(self.name, enabled=True) - def add_system_manager_role(self): # if adding system manager, do nothing if not cint(self.enabled) or ("System Manager" in [user_role.role for user_role in diff --git a/frappe/desk/doctype/notification_settings/notification_settings.py b/frappe/desk/doctype/notification_settings/notification_settings.py index 0831f4683d..4ab40bffe9 100644 --- a/frappe/desk/doctype/notification_settings/notification_settings.py +++ b/frappe/desk/doctype/notification_settings/notification_settings.py @@ -43,9 +43,10 @@ def create_notification_settings(user): _doc.name = user _doc.insert(ignore_permissions=True) -def enable_disable_notifications(user, enabled): + +def toggle_notifications(user, enable=False): if frappe.db.exists("Notification Settings", user): - frappe.db.set_value("Notification Settings", user, 'enabled', enabled) + frappe.db.set_value("Notification Settings", user, 'enabled', enable) @frappe.whitelist() From 7effd9db7a4bc808a7341d13ac2ad69e46d85034 Mon Sep 17 00:00:00 2001 From: Parth Kharwar Date: Fri, 5 Mar 2021 14:35:34 +0530 Subject: [PATCH 05/14] refactor: moves notification toggle to individual code block --- frappe/core/doctype/user/user.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 23ebe7d6b1..0e7b9d43a7 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -141,16 +141,14 @@ class User(Document): if not cint(self.enabled): self.a_system_manager_should_exist() - # disable notifications if the user has been disabled - toggle_notifications(self.name, enable=False) - else: - # enable notifications if the user has been enabled - toggle_notifications(self.name, enable=True) # clear sessions if disabled if not cint(self.enabled) and getattr(frappe.local, "login_manager", None): frappe.local.login_manager.logout(user=self.name) + # toggle notifications based on the user's status + toggle_notifications(self.name, enable=cint(self.enabled)) + def add_system_manager_role(self): # if adding system manager, do nothing if not cint(self.enabled) or ("System Manager" in [user_role.role for user_role in From a1d77752989297020376f39d82a8a985498e543e Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 5 Mar 2021 17:26:53 +0530 Subject: [PATCH 06/14] fix: 404 not loading on formview due to undefined parameter --- frappe/public/js/frappe/views/formview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/formview.js b/frappe/public/js/frappe/views/formview.js index 5d2d4ec303..6708a6ffb1 100644 --- a/frappe/public/js/frappe/views/formview.js +++ b/frappe/public/js/frappe/views/formview.js @@ -85,7 +85,7 @@ frappe.views.FormFactory = class FormFactory extends frappe.views.Factory { if (name && name.substr(0, 3) === 'new') { this.render_new_doc(doctype, name, doctype_layout); } else { - frappe.show_not_found(route); + frappe.show_not_found(); } return; } From 268cbbf8d95ea7003267ad60ca2300dd92c93055 Mon Sep 17 00:00:00 2001 From: walstanb Date: Fri, 5 Mar 2021 18:14:26 +0530 Subject: [PATCH 07/14] fix: doc not found after renaming doc --- frappe/public/js/frappe/model/model.js | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/public/js/frappe/model/model.js b/frappe/public/js/frappe/model/model.js index 9ec7b0e931..7167fc3982 100644 --- a/frappe/public/js/frappe/model/model.js +++ b/frappe/public/js/frappe/model/model.js @@ -621,6 +621,7 @@ $.extend(frappe.model, { r.message || args.new_name]); if(locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; + this.frm.reload_doc(); d.hide(); if(callback) callback(r.message); From 0e0413d0963897ef2ef1580c2ae945b4bcdf9125 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 7 Mar 2021 10:43:40 +0530 Subject: [PATCH 08/14] fix: Remove redundant css_variables import --- frappe/public/scss/desk/index.scss | 1 - frappe/public/scss/website/index.scss | 1 - frappe/public/scss/website/variables.scss | 1 + 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/public/scss/desk/index.scss b/frappe/public/scss/desk/index.scss index 1643034322..5f5fef251b 100644 --- a/frappe/public/scss/desk/index.scss +++ b/frappe/public/scss/desk/index.scss @@ -1,5 +1,4 @@ @import "variables"; -@import "css_variables"; @import "../common/mixins.scss"; @import "../common/global.scss"; @import "../common/icons.scss"; diff --git a/frappe/public/scss/website/index.scss b/frappe/public/scss/website/index.scss index 88f3b1db73..94b1249de1 100644 --- a/frappe/public/scss/website/index.scss +++ b/frappe/public/scss/website/index.scss @@ -1,6 +1,5 @@ @import '~quill/dist/quill.core'; @import 'variables'; -@import 'css_variables'; @import '~bootstrap/scss/bootstrap'; @import "../common/mixins"; @import "../common/global"; diff --git a/frappe/public/scss/website/variables.scss b/frappe/public/scss/website/variables.scss index fa68b57ad6..293d02b06d 100644 --- a/frappe/public/scss/website/variables.scss +++ b/frappe/public/scss/website/variables.scss @@ -131,5 +131,6 @@ $spacers: ( @import "~bootstrap/scss/functions"; @import "~bootstrap/scss/variables"; @import "~bootstrap/scss/mixins"; +@import 'css_variables'; $code-color: $purple; From caaed1a4e5ce0c0b079a31bd7b757076c7ad4358 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 7 Mar 2021 10:46:41 +0530 Subject: [PATCH 09/14] fix: Move data-theme attribute to root (html) - So that the HTML background matches with the theme background color --- frappe/public/js/frappe/ui/theme_switcher.js | 33 ++---- frappe/www/app.html | 114 ++++++++++--------- 2 files changed, 69 insertions(+), 78 deletions(-) diff --git a/frappe/public/js/frappe/ui/theme_switcher.js b/frappe/public/js/frappe/ui/theme_switcher.js index 31baf697f0..317198bca5 100644 --- a/frappe/public/js/frappe/ui/theme_switcher.js +++ b/frappe/public/js/frappe/ui/theme_switcher.js @@ -14,7 +14,7 @@ frappe.ui.ThemeSwitcher = class ThemeSwitcher { } refresh() { - this.current_theme = document.body.dataset.theme; + this.current_theme = document.documentElement.getAttribute("data-theme") || "light"; this.fetch_themes().then(() => { this.render(); }); @@ -45,7 +45,6 @@ frappe.ui.ThemeSwitcher = class ThemeSwitcher { }); } - get_preview_html(theme) { const preview = $(`
@@ -69,15 +68,9 @@ frappe.ui.ThemeSwitcher = class ThemeSwitcher {
`); - // preview.on('mouseover', () => { - // this.toggle_theme(theme.name, true) - // }) - - // preview.on('mouseleave', () => { - // this.toggle_theme(this.current_theme, true) - // }) - preview.on('click', () => { + if (this.current_theme === theme.name) return; + this.themes.forEach((th) => { th.$html.removeClass("selected"); }); @@ -89,19 +82,15 @@ frappe.ui.ThemeSwitcher = class ThemeSwitcher { return preview; } - toggle_theme(theme, preview=false) { - if (!preview) { - document.body.dataset.theme = theme.toLowerCase(); - frappe.show_alert("Theme Changed", 3); + toggle_theme(theme) { + this.current_theme = theme.toLowerCase(); + document.documentElement.setAttribute("data-theme", this.current_theme); + frappe.show_alert("Theme Changed", 3); - frappe.call('frappe.core.doctype.user.user.switch_theme', { - theme: toTitle(theme) - }); - } else { - document.body.dataset.theme = theme.toLowerCase(); - } + frappe.xcall("frappe.core.doctype.user.user.switch_theme", { + theme: toTitle(theme) + }); } - show() { this.dialog.show(); } @@ -109,4 +98,4 @@ frappe.ui.ThemeSwitcher = class ThemeSwitcher { hide() { this.dialog.hide(); } -}; \ No newline at end of file +}; diff --git a/frappe/www/app.html b/frappe/www/app.html index 715ddaf409..8da4d11c00 100644 --- a/frappe/www/app.html +++ b/frappe/www/app.html @@ -1,65 +1,67 @@ - - - - - - - - - - - - - - - Frappe - - - {% for include in include_css -%} - - {%- endfor -%} - - - {% include "public/icons/timeless/symbol-defs.svg" %} -
- -
-
-
-
-
-
+ + + + + + + + + + + + + + + + Frappe + + + {% for include in include_css -%} + + {%- endfor -%} + + + {% include "public/icons/timeless/symbol-defs.svg" %} +
+ +
+
+
+
+
+
- + - + - {% for include in include_js %} - - {% endfor %} - {% include "templates/includes/app_analytics/google_analytics.html" %} - {% include "templates/includes/app_analytics/mixpanel_analytics.html" %} + {% for include in include_js %} + + {% endfor %} + {% include "templates/includes/app_analytics/google_analytics.html" %} + {% include "templates/includes/app_analytics/mixpanel_analytics.html" %} - {% for sound in (sounds or []) %} - - {% endfor %} - + {% for sound in (sounds or []) %} + + {% endfor %} + + From 738e6f50ab954bffaa83d805a466074e7a1cb0cb Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 7 Mar 2021 10:48:40 +0530 Subject: [PATCH 10/14] fix: Remove unnecessary background transitions - to avoid inconsitent transition while switching the theme --- frappe/public/scss/desk/global.scss | 13 ------------- frappe/public/scss/desk/variables.scss | 4 ++++ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/frappe/public/scss/desk/global.scss b/frappe/public/scss/desk/global.scss index f4d6384352..b145879e1f 100644 --- a/frappe/public/scss/desk/global.scss +++ b/frappe/public/scss/desk/global.scss @@ -3,19 +3,6 @@ html { background-color: var(--bg-color); } -// transition -* { - transition: background-color 0.5s, background 0.5s; -} - -a, -.badge { - transition: 0.2s; -} - -.btn { - transition: background-color 0.2s, background 0.2s; -} body { diff --git a/frappe/public/scss/desk/variables.scss b/frappe/public/scss/desk/variables.scss index 57a117cd31..2855277ccd 100644 --- a/frappe/public/scss/desk/variables.scss +++ b/frappe/public/scss/desk/variables.scss @@ -95,6 +95,10 @@ $btn-active-box-shadow: var(--shadow-inset); $mark-bg: #FDF9AF; $mark-padding: 0; +// transitions +$btn-transition: none; +$input-transition: none; + // popover $enable-shadows: true; $popover-border-radius: var(--border-radius); From 87826f024179d3989b48ecf15998526f00a3690b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 7 Mar 2021 12:52:39 +0530 Subject: [PATCH 11/14] fix: Line hieght of checkbox label --- frappe/public/scss/common/global.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/public/scss/common/global.scss b/frappe/public/scss/common/global.scss index 91cc31c50d..20778176d4 100644 --- a/frappe/public/scss/common/global.scss +++ b/frappe/public/scss/common/global.scss @@ -8,7 +8,6 @@ --checkbox-right-margin: 8px; .label-area { - line-height: 1; font-size: var(--text-sm); } From 8962ba064756a49c939d2ff0bf629d579cd98025 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Sun, 7 Mar 2021 19:14:31 +0530 Subject: [PATCH 12/14] fix: Temporarily use firefox for testing - https://travis-ci.com/github/frappe/frappe/jobs/488819006 testing is failing due to some incompatibility --- .travis.yml | 2 +- frappe/commands/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 53ad56a948..ffada0286f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,7 @@ addons: - test_site_producer mariadb: 10.3 postgresql: 9.5 - chrome: stable + firefox: latest services: - xvfb diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index e9fa7217a8..13c6ca812f 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -578,7 +578,7 @@ def run_ui_tests(context, app, headless=False): frappe.commands.popen("yarn add cypress@^6 cypress-file-upload@^5 --no-lockfile") # run for headless mode - run_or_open = 'run --browser chrome --record --key 4a48f41c-11b3-425b-aa88-c58048fa69eb' if headless else 'open' + run_or_open = 'run --browser firefox --record --key 4a48f41c-11b3-425b-aa88-c58048fa69eb' if headless else 'open' command = '{site_env} {password_env} {cypress} {run_or_open}' formatted_command = command.format(site_env=site_env, password_env=password_env, cypress=cypress_path, run_or_open=run_or_open) From d28f5a54a5dd203dfa6d163654dadd850cd05077 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Fri, 5 Mar 2021 12:37:03 +0530 Subject: [PATCH 13/14] fix: dashboard edit and show buttons are not working --- frappe/public/js/frappe/views/pageview.js | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/public/js/frappe/views/pageview.js b/frappe/public/js/frappe/views/pageview.js index 6fd91d73c4..705d13b7f0 100644 --- a/frappe/public/js/frappe/views/pageview.js +++ b/frappe/public/js/frappe/views/pageview.js @@ -73,7 +73,6 @@ frappe.views.Page = class Page { return; } this.wrapper = frappe.container.add_page(this.name); - this.wrapper.label = this.pagedoc.title || this.pagedoc.name; this.wrapper.page_name = this.pagedoc.name; // set content, script and style From f17cf17b57d8e825d0871d720f22222fd41c4678 Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Mon, 8 Mar 2021 14:24:54 +0530 Subject: [PATCH 14/14] fix: remove data import mandatory check (#12363) * fix: ignore data import mandatory fields error * test: find the source of garbage * test: fix extra messages * test: fix: message log * test: remove debugging lines --- frappe/core/doctype/data_import/importer.py | 87 ------------------- .../core/doctype/data_import/test_importer.py | 22 ++--- 2 files changed, 12 insertions(+), 97 deletions(-) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index dde3dfaee9..388d9389f2 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -472,32 +472,6 @@ class ImportFile: doc = parent_doc - if self.import_type == INSERT: - # check if there is atleast one row for mandatory table fields - meta = frappe.get_meta(self.doctype) - mandatory_table_fields = [ - df - for df in meta.fields - if df.fieldtype in table_fieldtypes - and df.reqd - and len(doc.get(df.fieldname, [])) == 0 - ] - if len(mandatory_table_fields) == 1: - self.warnings.append( - { - "row": first_row.row_number, - "message": _("There should be atleast one row for {0} table").format( - frappe.bold(mandatory_table_fields[0].label) - ), - } - ) - elif mandatory_table_fields: - fields_string = ", ".join([df.label for df in mandatory_table_fields]) - message = _("There should be atleast one row for the following tables: {0}").format( - fields_string - ) - self.warnings.append({"row": first_row.row_number, "message": message}) - return doc, rows, data[len(rows) :] def get_warnings(self): @@ -626,7 +600,6 @@ class Row: new_doc.update(doc) doc = new_doc - self.check_mandatory_fields(doctype, doc, table_df) return doc def validate_value(self, value, col): @@ -727,66 +700,6 @@ class Row: pass return value - def check_mandatory_fields(self, doctype, doc, table_df=None): - """If import type is Insert: - Check for mandatory fields (except table fields) in doc - if import type is Update: - Check for name field or autoname field in doc - """ - meta = frappe.get_meta(doctype) - if self.import_type == UPDATE: - if meta.istable: - # when updating records with table rows, - # there are two scenarios: - # 1. if row 'name' is provided in the template - # the table row will be updated - # 2. if row 'name' is not provided - # then a new row will be added - # so we dont need to check for mandatory - return - - # for update, only ID (name) field is mandatory - id_field = get_id_field(doctype) - if doc.get(id_field.fieldname) in INVALID_VALUES: - self.warnings.append( - { - "row": self.row_number, - "message": _("{0} is a mandatory field").format(id_field.label), - } - ) - return - - fields = [ - df - for df in meta.fields - if df.fieldtype not in table_fieldtypes - and df.reqd - and doc.get(df.fieldname) in INVALID_VALUES - ] - - if not fields: - return - - def get_field_label(df): - return "{0}{1}".format(df.label, " ({})".format(table_df.label) if table_df else "") - - if len(fields) == 1: - field_label = get_field_label(fields[0]) - self.warnings.append( - { - "row": self.row_number, - "message": _("{0} is a mandatory field").format(frappe.bold(field_label)), - } - ) - else: - fields_string = ", ".join([frappe.bold(get_field_label(df)) for df in fields]) - self.warnings.append( - { - "row": self.row_number, - "message": _("{0} are mandatory fields").format(fields_string), - } - ) - def get_values(self, indexes): return [self.data[i] for i in indexes] diff --git a/frappe/core/doctype/data_import/test_importer.py b/frappe/core/doctype/data_import/test_importer.py index b083b9eaaa..f76d4504a4 100644 --- a/frappe/core/doctype/data_import/test_importer.py +++ b/frappe/core/doctype/data_import/test_importer.py @@ -13,7 +13,7 @@ doctype_name = 'DocType for Import' class TestImporter(unittest.TestCase): @classmethod def setUpClass(cls): - create_doctype_if_not_exists(doctype_name) + create_doctype_if_not_exists(doctype_name,) def test_data_import_from_file(self): import_file = get_import_file('sample_import_file') @@ -59,18 +59,18 @@ class TestImporter(unittest.TestCase): def test_data_import_without_mandatory_values(self): import_file = get_import_file('sample_import_file_without_mandatory') data_import = self.get_importer(doctype_name, import_file) + frappe.local.message_log = [] data_import.start_import() data_import.reload() - warnings = frappe.parse_json(data_import.template_warnings) + 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(warnings[0]['row'], 2) - self.assertEqual(warnings[0]['message'], "Child Title (Table Field 1) is a mandatory field") - - self.assertEqual(warnings[1]['row'], 3) - self.assertEqual(warnings[1]['message'], "Child Title (Table Field 1 Again) is a mandatory field") - - self.assertEqual(warnings[2]['row'], 4) - self.assertEqual(warnings[2]['message'], "Title is a mandatory field") + self.assertEqual(import_log[1]['row_indexes'], [4]) + self.assertEqual(frappe.parse_json(import_log[1]['messages'][0])['message'], "Title is required") def test_data_import_update(self): existing_doc = frappe.get_doc( @@ -104,6 +104,8 @@ class TestImporter(unittest.TestCase): data_import.reference_doctype = doctype data_import.import_file = import_file.file_url data_import.insert() + # Commit so that the first import failure does not rollback the Data Import insert. + frappe.db.commit() return data_import