From 4a04e9f9dcf385dd417a37657bbbc012d684080a Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 26 Jul 2022 09:56:19 +0200 Subject: [PATCH 01/34] fix: appropriate password hint --- frappe/core/doctype/user/user.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 232e915435..e7ea8b203a 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -1045,10 +1045,9 @@ def notify_admin_access_to_system_manager(login_manager=None): def handle_password_test_fail(result): suggestions = result["feedback"]["suggestions"][0] if result["feedback"]["suggestions"] else "" warning = result["feedback"]["warning"] if "warning" in result["feedback"] else "" - suggestions += ( - "
" + _("Hint: Include symbols, numbers and capital letters in the password") + "
" - ) - frappe.throw(" ".join([_("Invalid Password:"), warning, suggestions])) + suggestions += f"
{_('Your password is too short or not complex enough.')}
" + + frappe.throw(" ".join([warning, suggestions]), title=_("Invalid Password")) def update_gravatar(name): From 244ffb4e233b7de3fc5f3cc7fdb25a9f848f9316 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 26 Jul 2022 19:18:08 +0200 Subject: [PATCH 02/34] refactor: handle_password_test_fail --- frappe/core/doctype/user/user.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index e7ea8b203a..276c05cbaf 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -540,7 +540,7 @@ class User(Document): feedback = result.get("feedback", None) if feedback and not feedback.get("password_policy_validation_passed", False): - handle_password_test_fail(result) + handle_password_test_fail(result["feedback"]) def suggest_username(self): def _check_suggestion(suggestion): @@ -686,7 +686,7 @@ def update_password(new_password, logout_all_sessions=0, key=None, old_password= feedback = result.get("feedback", None) if feedback and not feedback.get("password_policy_validation_passed", False): - handle_password_test_fail(result) + handle_password_test_fail(result["feedback"]) res = _get_user_for_update_password(key, old_password) if res.get("message"): @@ -1042,12 +1042,15 @@ def notify_admin_access_to_system_manager(login_manager=None): ) -def handle_password_test_fail(result): - suggestions = result["feedback"]["suggestions"][0] if result["feedback"]["suggestions"] else "" - warning = result["feedback"]["warning"] if "warning" in result["feedback"] else "" - suggestions += f"
{_('Your password is too short or not complex enough.')}
" +def handle_password_test_fail(feedback: dict): + # Backward compatibility + if "feedback" in feedback: + feedback = feedback["feedback"] - frappe.throw(" ".join([warning, suggestions]), title=_("Invalid Password")) + suggestions = feedback.get("suggestions", []) + warning = feedback.get("warning", "") + + frappe.throw(msg=" ".join([warning] + suggestions), title=_("Invalid Password")) def update_gravatar(name): From 3e351fe4fa30c13c407145aee744b0c5771968f9 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 26 Jul 2022 19:24:52 +0200 Subject: [PATCH 03/34] refactor: pass feedback --- frappe/core/doctype/user/user.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 276c05cbaf..a982403935 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -540,7 +540,7 @@ class User(Document): feedback = result.get("feedback", None) if feedback and not feedback.get("password_policy_validation_passed", False): - handle_password_test_fail(result["feedback"]) + handle_password_test_fail(feedback) def suggest_username(self): def _check_suggestion(suggestion): @@ -686,7 +686,7 @@ def update_password(new_password, logout_all_sessions=0, key=None, old_password= feedback = result.get("feedback", None) if feedback and not feedback.get("password_policy_validation_passed", False): - handle_password_test_fail(result["feedback"]) + handle_password_test_fail(feedback) res = _get_user_for_update_password(key, old_password) if res.get("message"): From a65f8637cf47eabfc1f2979d9e5835af8a660176 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 26 Jul 2022 19:29:59 +0200 Subject: [PATCH 04/34] test: password strength test and error --- frappe/core/doctype/user/test_user.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 7582954175..2367f101c9 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -8,6 +8,7 @@ from unittest.mock import patch import frappe import frappe.exceptions from frappe.core.doctype.user.user import ( + handle_password_test_fail, reset_password, sign_up, test_password_strength, @@ -206,6 +207,15 @@ class TestUser(unittest.TestCase): user.save() frappe.flags.in_test = True + def test_password_validation(self): + result = test_password_strength("P@ssw0rd") + feedback = result["feedback"] + self.assertEqual(feedback["password_policy_validation_passed"], False) + self.assertRaises(frappe.exceptions.ValidationError, handle_password_test_fail, feedback) + + # test backwards compatibility + self.assertRaises(frappe.exceptions.ValidationError, handle_password_test_fail, result) + def test_comment_mentions(self): comment = """ From baa31d71722345e5e4c5c620e8c84d0d2136271b Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Wed, 15 Jun 2022 11:35:24 +0530 Subject: [PATCH 05/34] feat: Form Editing --- frappe/public/js/frappe/form/column.js | 17 +++++++++++++++-- frappe/public/js/frappe/form/form.js | 5 +++++ frappe/public/js/frappe/form/form_editor.js | 15 +++++++++++++++ frappe/public/js/frappe/form/layout.js | 11 ++++------- frappe/public/js/frappe/form/section.js | 7 +++++++ frappe/public/js/frappe/form/tab.js | 6 ++++++ 6 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 frappe/public/js/frappe/form/form_editor.js diff --git a/frappe/public/js/frappe/form/column.js b/frappe/public/js/frappe/form/column.js index 939ce6e2d7..279b88974d 100644 --- a/frappe/public/js/frappe/form/column.js +++ b/frappe/public/js/frappe/form/column.js @@ -4,6 +4,8 @@ export default class Column { this.df = df; this.section = section; + this.section.columns.push(this); + this.fields_list = []; this.make(); this.resize_all_columns(); } @@ -15,8 +17,9 @@ export default class Column { `) - .appendTo(this.section.body) - .find("form") + .appendTo(this.section.body); + + this.form = this.wrapper.find("form") .on("submit", function () { return false; }); @@ -41,7 +44,17 @@ export default class Column { .addClass("col-sm-" + colspan); } + add_field(fieldobj) { + this.fields_list.push(fieldobj); + } + refresh() { this.section.refresh(); } + + make_sortable() { + this.sortable = new Sortable(this.form.get(0), { + group: this.section.layout.frm.doctype + }); + } } diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 77eacf6964..324c37175d 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -13,6 +13,7 @@ import "./script_helpers"; import "./sidebar/form_sidebar"; import "./footer/footer"; import "./form_tour"; +import './form_editor'; import { UndoManager } from "./undo_manager"; frappe.ui.form.Controller = class FormController { @@ -267,6 +268,10 @@ frappe.ui.form.Form = class FrappeForm { this.states = new frappe.ui.form.States({ frm: this, }); + + this.form_editor = new frappe.ui.form.FormEditor({ + frm: this + }); } watch_model_updates() { diff --git a/frappe/public/js/frappe/form/form_editor.js b/frappe/public/js/frappe/form/form_editor.js new file mode 100644 index 0000000000..a3a3da1fca --- /dev/null +++ b/frappe/public/js/frappe/form/form_editor.js @@ -0,0 +1,15 @@ +frappe.ui.form.FormEditor = class FormEditor { + constructor({ frm }) { + this.frm = frm; + } + + setup() { + // setup sortable in all column + for(let section of this.frm.layout.sections) { + for (let column of section.columns) { + column.make_sortable(); + } + } + + } +} diff --git a/frappe/public/js/frappe/form/layout.js b/frappe/public/js/frappe/form/layout.js index d396a4587a..62a42ee531 100644 --- a/frappe/public/js/frappe/form/layout.js +++ b/frappe/public/js/frappe/form/layout.js @@ -211,14 +211,11 @@ frappe.ui.form.Layout = class Layout { fieldobj.perm = this.frm.perm; } - this.section.fields_list.push(fieldobj); - this.section.fields_dict[df.fieldname] = fieldobj; - fieldobj.section = this.section; + this.section.add_field(fieldobj); + this.column.add_field(fieldobj); if (this.current_tab) { - fieldobj.tab = this.current_tab; - this.current_tab.fields_list.push(fieldobj); - this.current_tab.fields_dict[df.fieldname] = fieldobj; + this.current_tab.add_field(fieldobj); } } @@ -226,7 +223,7 @@ frappe.ui.form.Layout = class Layout { const fieldobj = frappe.ui.form.make_control({ df: df, doctype: this.doctype, - parent: this.column.wrapper.get(0), + parent: this.column.form.get(0), frm: this.frm, render_input: render, doc: this.doc, diff --git a/frappe/public/js/frappe/form/section.js b/frappe/public/js/frappe/form/section.js index d2b5cb4ec1..8e7bfd2f80 100644 --- a/frappe/public/js/frappe/form/section.js +++ b/frappe/public/js/frappe/form/section.js @@ -4,6 +4,7 @@ export default class Section { this.card_layout = card_layout; this.parent = parent; this.df = df || {}; + this.columns = []; this.fields_list = []; this.fields_dict = {}; @@ -82,6 +83,12 @@ export default class Section { } } + add_field(fieldobj) { + this.fields_list.push(fieldobj); + this.fields_dict[fieldobj.fieldname] = fieldobj; + fieldobj.section = this.section; + } + refresh(hide) { if (!this.df) return; // hide if explicitly hidden diff --git a/frappe/public/js/frappe/form/tab.js b/frappe/public/js/frappe/form/tab.js index e5fab2d982..58d29a89d4 100644 --- a/frappe/public/js/frappe/form/tab.js +++ b/frappe/public/js/frappe/form/tab.js @@ -79,6 +79,12 @@ export default class Tab { this.parent.hide(); } + add_field(fieldobj) { + fieldobj.tab = this; + this.fields_list.push(fieldobj); + this.fields_dict[fieldobj.df.fieldname] = fieldobj; + } + set_active() { this.parent.find(".nav-link").tab("show"); this.wrapper.addClass("active"); From aa6bb5130dc944941c2c304669f4b0471bac0d70 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Tue, 5 Jul 2022 14:40:43 +0530 Subject: [PATCH 06/34] feat(minor): save new order based on re-arranged fields --- frappe/core/doctype/doctype/doctype.py | 30 ++++++++++ frappe/public/js/frappe/form/column.js | 7 +-- frappe/public/js/frappe/form/form.js | 7 ++- frappe/public/js/frappe/form/form_editor.js | 63 +++++++++++++++++++++ frappe/public/js/frappe/form/layout.js | 29 +++++++++- frappe/public/js/frappe/form/section.js | 13 ++++- frappe/public/js/frappe/form/tab.js | 62 +++++++++++--------- 7 files changed, 173 insertions(+), 38 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 9a8a976b9f..af7124b347 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1740,3 +1740,33 @@ def get_field(doc, fieldname): for field in doc.fields: if field.fieldname == fieldname: return field + + +@frappe.whitelist() +def set_field_order(doctype, field_order): + """Update field order in doctype""" + + frappe.only_for("System Manager") + + meta = frappe.get_meta(doctype) + field_order = json.loads(field_order) + + json_file_path = get_file_path(meta.module, "DocType", meta.name) + with open(json_file_path, "r") as json_file: + doc_obj = json.loads(json_file.read()) + + doc_obj["field_order"] = field_order + with open(json_file_path, "w") as json_file: + json_file.write(frappe.as_json(doc_obj)) + + idx = 1 + for fieldname in field_order: + docfield = frappe.qb.DocType("DocField") + frappe.qb.update(docfield).set(docfield.idx, idx).where( + (docfield.fieldname == fieldname) & (docfield.parent == doctype) + ).run() + idx += 1 + + frappe.db.commit() + + frappe.clear_cache(doctype=doctype) diff --git a/frappe/public/js/frappe/form/column.js b/frappe/public/js/frappe/form/column.js index 279b88974d..c2b214092e 100644 --- a/frappe/public/js/frappe/form/column.js +++ b/frappe/public/js/frappe/form/column.js @@ -5,14 +5,13 @@ export default class Column { this.df = df; this.section = section; this.section.columns.push(this); - this.fields_list = []; this.make(); this.resize_all_columns(); } make() { this.wrapper = $(` -
+
@@ -44,9 +43,7 @@ export default class Column { .addClass("col-sm-" + colspan); } - add_field(fieldobj) { - this.fields_list.push(fieldobj); - } + add_field(fieldobj) { } refresh() { this.section.refresh(); diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 324c37175d..6d394fefa6 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -261,9 +261,14 @@ frappe.ui.form.Form = class FrappeForm { this.dashboard = new frappe.ui.form.Dashboard(dashboard_parent, this); this.tour = new frappe.ui.form.FormTour({ - frm: this, + frm: this }); + this.form_editor = new frappe.ui.form.FormEditor({ + frm: this + }); + this.form_editor.setup(); + // workflow state this.states = new frappe.ui.form.States({ frm: this, diff --git a/frappe/public/js/frappe/form/form_editor.js b/frappe/public/js/frappe/form/form_editor.js index a3a3da1fca..796ad59605 100644 --- a/frappe/public/js/frappe/form/form_editor.js +++ b/frappe/public/js/frappe/form/form_editor.js @@ -4,12 +4,75 @@ frappe.ui.form.FormEditor = class FormEditor { } setup() { + this.setup_sortable(); + this.setup_switch_tabs_on_hover(); + } + + setup_sortable() { // setup sortable in all column for(let section of this.frm.layout.sections) { for (let column of section.columns) { column.make_sortable(); } } + // sortable for moving tabs + if (this.frm.layout.tab_link_container) { + this.tab_sortable = new Sortable(this.frm.layout.tab_link_container.get(0)); + } + } + setup_switch_tabs_on_hover() { + for(let tab of this.frm.layout.tabs) { + tab.setup_switch_on_hover(); + } + } + + update_field_order() { + this.field_order = []; + if (this.frm.layout.is_tabbed_layout()) { + for (let tab of this.frm.layout.tab_link_container.find('.nav-link')) { + this.add_field_to_field_order(tab); + const tab_id = tab.getAttribute('href').slice(1); + + this.add_sections(document.getElementById(tab_id)); + } + } else { + this.add_sections(this.frm.layout.page); + } + + frappe.call('frappe.core.doctype.doctype.doctype.set_field_order', { + doctype: this.frm.doctype, field_order: this.field_order}) + .then(() => frappe.toast('Field order updated')); + } + + add_sections(container) { + for(let section of $(container).find('.form-section')) { + this.add_field_to_field_order(section); + for (let column of $(section).find('.form-column')) { + this.add_field_to_field_order(column); + + for (let control of $(column).find('.frappe-control')) { + this.add_field_to_field_order(control); + } + } + } + } + + rebuid_fields_list() { + // rebuild the .fields_list and .fields_dict property of sections and columns + // refresh is based on the these properties + + for(let section of this.frm.layout.sections) { + section.rebuild_fields_list_from_dom(); + } + } + + add_field_to_field_order(element) { + const fieldname = element.getAttribute('data-fieldname'); + const fieldobj = this.frm.fields_dict[fieldname]; + const is_custom_field = fieldobj ? (fieldobj.df && fieldobj.df.is_custom_field) : false; + if (fieldname && !is_custom_field && fieldname.substr(0, 2) !== '__') { + this.field_order.push(fieldname); + } } } diff --git a/frappe/public/js/frappe/form/layout.js b/frappe/public/js/frappe/form/layout.js index 62a42ee531..dda1b43958 100644 --- a/frappe/public/js/frappe/form/layout.js +++ b/frappe/public/js/frappe/form/layout.js @@ -8,9 +8,15 @@ frappe.ui.form.Layout = class Layout { this.pages = []; this.tabs = []; this.sections = []; +<<<<<<< HEAD this.page_breaks = []; +======= + this.sections_dict = {}; +>>>>>>> 69ca4ff5f7 (feat(minor): save new order based on re-arranged fields) this.fields_list = []; this.fields_dict = {}; + this.section_count = 0; + this.column_count = 0; $.extend(this, opts); } @@ -41,10 +47,15 @@ frappe.ui.form.Layout = class Layout {
`).appendTo(this.page); +<<<<<<< HEAD this.tabs_list = this.page.find(".form-tabs"); this.tabs_content = $(`
`).appendTo( this.page ); +======= + this.tab_link_container = this.page.find('.form-tabs'); + this.tabs_content = $(`
`).appendTo(this.page); +>>>>>>> f7a89297c9 (feat(minor): save new order based on re-arranged fields) this.setup_events(); } @@ -273,13 +284,18 @@ frappe.ui.form.Layout = class Layout { this.fold_btn.trigger("click"); } - make_section(df) { + make_section(df = {}) { + this.section_count++; + if (!df.fieldname) df.fieldname = `__section_${this.section_count}`; + this.section = new Section( this.current_tab ? this.current_tab.wrapper : this.page, df, this.card_layout, this ); + this.sections.push(this.section); + this.sections_dict[df.fieldname] = this.section; // append to layout fields if (df) { @@ -290,7 +306,10 @@ frappe.ui.form.Layout = class Layout { this.column = null; } - make_column(df) { + make_column(df = {}) { + this.column_count ++; + if (!df.fieldname) df.fieldname = `__column_${this.section_count}`; + this.column = new Column(this.section, df); if (df && df.fieldname) { this.fields_list.push(this.column); @@ -299,7 +318,7 @@ frappe.ui.form.Layout = class Layout { make_tab(df) { this.section = null; - let tab = new Tab(this, df, this.frm, this.tabs_list, this.tabs_content); + let tab = new Tab(this, df, this.frm, this.tab_link_container, this.tabs_content); this.current_tab = tab; this.make_section({ fieldtype: "Section Break" }); this.tabs.push(tab); @@ -453,7 +472,11 @@ frappe.ui.form.Layout = class Layout { } setup_events() { +<<<<<<< HEAD this.tabs_list.off("click").on("click", ".nav-link", (e) => { +======= + this.tab_link_container.off('click').on('click', '.nav-link', (e) => { +>>>>>>> f7a89297c9 (feat(minor): save new order based on re-arranged fields) e.preventDefault(); e.stopImmediatePropagation(); $(e.currentTarget).tab("show"); diff --git a/frappe/public/js/frappe/form/section.js b/frappe/public/js/frappe/form/section.js index 8e7bfd2f80..a84a57ba59 100644 --- a/frappe/public/js/frappe/form/section.js +++ b/frappe/public/js/frappe/form/section.js @@ -29,9 +29,12 @@ export default class Section { let make_card = this.card_layout; this.wrapper = $(`
+======= + ${ make_card ? "card-section" : "" }" data-fieldname="${this.df.fieldname}"> +>>>>>>> f7a89297c9 (feat(minor): save new order based on re-arranged fields) `).appendTo(this.parent); - this.layout && this.layout.sections.push(this); if (this.df) { if (this.df.label) { @@ -156,4 +159,12 @@ export default class Section { this.wrapper.toggleClass("hide-control", !show); // this.on_section_toggle && this.on_section_toggle(show); } + + rebuild_fields_list_from_dom() { + this.fields_list = []; + this.fields_dict = {}; + for (let ele of $(this.wrapper).find('.frappe-control')) { + let f = this.layout.frm.fields_dict[ele.getAttribute('data-fieldname')]; + } + } } diff --git a/frappe/public/js/frappe/form/tab.js b/frappe/public/js/frappe/form/tab.js index 58d29a89d4..e8ee35c65b 100644 --- a/frappe/public/js/frappe/form/tab.js +++ b/frappe/public/js/frappe/form/tab.js @@ -1,32 +1,31 @@ export default class Tab { - constructor(parent, df, frm, tabs_list, tabs_content) { - this.parent = parent; + constructor(layout, df, frm, tab_link_container, tabs_content) { + this.layout = layout; this.df = df || {}; this.frm = frm; this.doctype = this.frm.doctype; this.label = this.df && this.df.label; - this.tabs_list = tabs_list; + this.tab_link_container = tab_link_container; this.tabs_content = tabs_content; - this.fields_list = []; - this.fields_dict = {}; this.make(); this.setup_listeners(); this.refresh(); } make() { - const id = `${frappe.scrub(this.doctype, "-")}-${this.df.fieldname}`; - this.parent = $(` + const id = `${frappe.scrub(this.doctype, '-')}-${this.df.fieldname}`; + this.tab_link = $(` - `).appendTo(this.tabs_list); + `).appendTo(this.tab_link_container); this.wrapper = $(`
`).appendTo(this.tabs_content); @@ -38,11 +37,6 @@ export default class Tab { // hide if explicitly hidden let hide = this.df.hidden || this.df.hidden_due_to_dependency; - // hide if dashboard and not saved - if (!hide && this.df.show_dashboard && this.frm.is_new() && !this.fields_list.length) { - hide = true; - } - // hide if no read permission if (!hide && this.frm && !this.frm.get_perm(this.df.permlevel || 0, "read")) { hide = true; @@ -60,35 +54,38 @@ export default class Tab { } } + // hide if dashboard and not saved + if (!hide && this.df.show_dashboard && this.frm.is_new()) { + hide = true; + } + this.toggle(!hide); } toggle(show) { - this.parent.toggleClass("hide", !show); - this.wrapper.toggleClass("hide", !show); - this.parent.toggleClass("show", show); - this.wrapper.toggleClass("show", show); + this.tab_link.toggleClass('hide', !show); + this.wrapper.toggleClass('hide', !show); + this.tab_link.toggleClass('show', show); + this.wrapper.toggleClass('show', show); this.hidden = !show; } show() { - this.parent.show(); + this.tab_link.show(); } hide() { - this.parent.hide(); + this.tab_link.hide(); } add_field(fieldobj) { fieldobj.tab = this; - this.fields_list.push(fieldobj); - this.fields_dict[fieldobj.df.fieldname] = fieldobj; } set_active() { - this.parent.find(".nav-link").tab("show"); - this.wrapper.addClass("active"); - this.frm?.set_active_tab?.(this); + this.tab_link.find('.nav-link').tab('show'); + this.wrapper.addClass('show'); + this.frm.active_tab = this; } is_active() { @@ -96,12 +93,21 @@ export default class Tab { } is_hidden() { - return this.wrapper.hasClass("hide"); + this.wrapper.hasClass('hide') + && this.tab_link.hasClass('hide'); } - setup_listeners() { - this.parent.find(".nav-link").on("shown.bs.tab", () => { - this?.frm.set_active_tab?.(this); + setup_switch_on_hover() { + this.tab_link.on('dragenter', () => { + this.action = setTimeout(() => { + this.set_active(); + }, 2000); }); + this.tab_link.on('dragout', () => { + if (this.action) { + clearTimeout(this.action); + this.action = null; + } + }) } } From 09e001b57da8c9ab853766150213864800f3e0e2 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Wed, 10 Aug 2022 14:50:49 +0530 Subject: [PATCH 07/34] fix(minor): merge conflicts --- frappe/public/js/frappe/form/form.js | 2 +- frappe/public/js/frappe/form/form_editor.js | 2 +- frappe/public/js/frappe/form/layout.js | 18 ++----------- frappe/public/js/frappe/form/section.js | 4 --- frappe/public/js/frappe/form/tab.js | 28 +++++++++++++-------- 5 files changed, 21 insertions(+), 33 deletions(-) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 6d394fefa6..ae62a3659d 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -267,7 +267,7 @@ frappe.ui.form.Form = class FrappeForm { this.form_editor = new frappe.ui.form.FormEditor({ frm: this }); - this.form_editor.setup(); + //this.form_editor.setup(); // workflow state this.states = new frappe.ui.form.States({ diff --git a/frappe/public/js/frappe/form/form_editor.js b/frappe/public/js/frappe/form/form_editor.js index 796ad59605..5e3570b068 100644 --- a/frappe/public/js/frappe/form/form_editor.js +++ b/frappe/public/js/frappe/form/form_editor.js @@ -27,7 +27,7 @@ frappe.ui.form.FormEditor = class FormEditor { } } - update_field_order() { + save() { this.field_order = []; if (this.frm.layout.is_tabbed_layout()) { for (let tab of this.frm.layout.tab_link_container.find('.nav-link')) { diff --git a/frappe/public/js/frappe/form/layout.js b/frappe/public/js/frappe/form/layout.js index dda1b43958..be26749d4e 100644 --- a/frappe/public/js/frappe/form/layout.js +++ b/frappe/public/js/frappe/form/layout.js @@ -8,11 +8,8 @@ frappe.ui.form.Layout = class Layout { this.pages = []; this.tabs = []; this.sections = []; -<<<<<<< HEAD this.page_breaks = []; -======= this.sections_dict = {}; ->>>>>>> 69ca4ff5f7 (feat(minor): save new order based on re-arranged fields) this.fields_list = []; this.fields_dict = {}; this.section_count = 0; @@ -47,15 +44,8 @@ frappe.ui.form.Layout = class Layout {
`).appendTo(this.page); -<<<<<<< HEAD - this.tabs_list = this.page.find(".form-tabs"); - this.tabs_content = $(`
`).appendTo( - this.page - ); -======= - this.tab_link_container = this.page.find('.form-tabs'); + this.tab_link_container = this.page.find(".form-tabs"); this.tabs_content = $(`
`).appendTo(this.page); ->>>>>>> f7a89297c9 (feat(minor): save new order based on re-arranged fields) this.setup_events(); } @@ -472,11 +462,7 @@ frappe.ui.form.Layout = class Layout { } setup_events() { -<<<<<<< HEAD - this.tabs_list.off("click").on("click", ".nav-link", (e) => { -======= - this.tab_link_container.off('click').on('click', '.nav-link', (e) => { ->>>>>>> f7a89297c9 (feat(minor): save new order based on re-arranged fields) + this.tab_link_container.off("click").on("click", ".nav-link", (e) => { e.preventDefault(); e.stopImmediatePropagation(); $(e.currentTarget).tab("show"); diff --git a/frappe/public/js/frappe/form/section.js b/frappe/public/js/frappe/form/section.js index a84a57ba59..9f028d2ee4 100644 --- a/frappe/public/js/frappe/form/section.js +++ b/frappe/public/js/frappe/form/section.js @@ -29,11 +29,7 @@ export default class Section { let make_card = this.card_layout; this.wrapper = $(`
-======= ${ make_card ? "card-section" : "" }" data-fieldname="${this.df.fieldname}"> ->>>>>>> f7a89297c9 (feat(minor): save new order based on re-arranged fields) `).appendTo(this.parent); if (this.df) { diff --git a/frappe/public/js/frappe/form/tab.js b/frappe/public/js/frappe/form/tab.js index e8ee35c65b..af6148b966 100644 --- a/frappe/public/js/frappe/form/tab.js +++ b/frappe/public/js/frappe/form/tab.js @@ -13,7 +13,7 @@ export default class Tab { } make() { - const id = `${frappe.scrub(this.doctype, '-')}-${this.df.fieldname}`; + const id = `${frappe.scrub(this.doctype, "-")}-${this.df.fieldname}`; this.tab_link = $(`
- `) - .appendTo(this.section.body); + `).appendTo(this.section.body); - this.form = this.wrapper.find("form") - .on("submit", function () { - return false; - }); + this.form = this.wrapper.find("form").on("submit", function () { + return false; + }); if (this.df.label) { $(` @@ -43,7 +41,7 @@ export default class Column { .addClass("col-sm-" + colspan); } - add_field() { } + add_field() {} refresh() { this.section.refresh(); @@ -51,7 +49,7 @@ export default class Column { make_sortable() { this.sortable = new Sortable(this.form.get(0), { - group: this.section.layout.frm.doctype + group: this.section.layout.frm.doctype, }); } } diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index ae62a3659d..10dd98954f 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -13,7 +13,7 @@ import "./script_helpers"; import "./sidebar/form_sidebar"; import "./footer/footer"; import "./form_tour"; -import './form_editor'; +import "./form_editor"; import { UndoManager } from "./undo_manager"; frappe.ui.form.Controller = class FormController { @@ -261,11 +261,11 @@ frappe.ui.form.Form = class FrappeForm { this.dashboard = new frappe.ui.form.Dashboard(dashboard_parent, this); this.tour = new frappe.ui.form.FormTour({ - frm: this + frm: this, }); this.form_editor = new frappe.ui.form.FormEditor({ - frm: this + frm: this, }); //this.form_editor.setup(); @@ -275,7 +275,7 @@ frappe.ui.form.Form = class FrappeForm { }); this.form_editor = new frappe.ui.form.FormEditor({ - frm: this + frm: this, }); } diff --git a/frappe/public/js/frappe/form/form_editor.js b/frappe/public/js/frappe/form/form_editor.js index 9f78ae94e9..ac9bc79f85 100644 --- a/frappe/public/js/frappe/form/form_editor.js +++ b/frappe/public/js/frappe/form/form_editor.js @@ -30,9 +30,9 @@ frappe.ui.form.FormEditor = class FormEditor { save() { this.field_order = []; if (this.frm.layout.is_tabbed_layout()) { - for (let tab of this.frm.layout.tab_link_container.find('.nav-link')) { + for (let tab of this.frm.layout.tab_link_container.find(".nav-link")) { this.add_field_to_field_order(tab); - const tab_id = tab.getAttribute('href').slice(1); + const tab_id = tab.getAttribute("href").slice(1); this.add_sections(document.getElementById(tab_id)); } @@ -40,18 +40,21 @@ frappe.ui.form.FormEditor = class FormEditor { this.add_sections(this.frm.layout.page); } - frappe.call('frappe.core.doctype.doctype.doctype.set_field_order', { - doctype: this.frm.doctype, field_order: this.field_order}) - .then(() => frappe.toast('Field order updated')); + frappe + .call("frappe.core.doctype.doctype.doctype.set_field_order", { + doctype: this.frm.doctype, + field_order: this.field_order, + }) + .then(() => frappe.toast("Field order updated")); } add_sections(container) { - for (let section of $(container).find('.form-section')) { + for (let section of $(container).find(".form-section")) { this.add_field_to_field_order(section); - for (let column of $(section).find('.form-column')) { + for (let column of $(section).find(".form-column")) { this.add_field_to_field_order(column); - for (let control of $(column).find('.frappe-control')) { + for (let control of $(column).find(".frappe-control")) { this.add_field_to_field_order(control); } } @@ -68,10 +71,10 @@ frappe.ui.form.FormEditor = class FormEditor { } add_field_to_field_order(element) { - const fieldname = element.getAttribute('data-fieldname'); + const fieldname = element.getAttribute("data-fieldname"); const fieldobj = this.frm.fields_dict[fieldname]; - const is_custom_field = fieldobj ? (fieldobj.df && fieldobj.df.is_custom_field) : false; - if (fieldname && !is_custom_field && fieldname.substr(0, 2) !== '__') { + const is_custom_field = fieldobj ? fieldobj.df && fieldobj.df.is_custom_field : false; + if (fieldname && !is_custom_field && fieldname.substr(0, 2) !== "__") { this.field_order.push(fieldname); } } diff --git a/frappe/public/js/frappe/form/layout.js b/frappe/public/js/frappe/form/layout.js index 9ff6e61fb0..678958d7b5 100644 --- a/frappe/public/js/frappe/form/layout.js +++ b/frappe/public/js/frappe/form/layout.js @@ -45,7 +45,9 @@ frappe.ui.form.Layout = class Layout {
`).appendTo(this.page); this.tab_link_container = this.page.find(".form-tabs"); - this.tabs_content = $(`
`).appendTo(this.page); + this.tabs_content = $(`
`).appendTo( + this.page + ); this.setup_events(); } diff --git a/frappe/public/js/frappe/form/section.js b/frappe/public/js/frappe/form/section.js index 663a82dcc1..af40b0aa48 100644 --- a/frappe/public/js/frappe/form/section.js +++ b/frappe/public/js/frappe/form/section.js @@ -29,7 +29,7 @@ export default class Section { let make_card = this.card_layout; this.wrapper = $(`
+ ${make_card ? "card-section" : ""}" data-fieldname="${this.df.fieldname}"> `).appendTo(this.parent); if (this.df) { diff --git a/frappe/public/js/frappe/form/tab.js b/frappe/public/js/frappe/form/tab.js index 171b57b7b5..0a0eb8c80e 100644 --- a/frappe/public/js/frappe/form/tab.js +++ b/frappe/public/js/frappe/form/tab.js @@ -93,8 +93,7 @@ export default class Tab { } is_hidden() { - return this.wrapper.hasClass("hide") - && this.tab_link.hasClass("hide"); + return this.wrapper.hasClass("hide") && this.tab_link.hasClass("hide"); } setup_listeners() { From ec9239525b24d2de601186ec8bc76cccbdc152cf Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Thu, 11 Aug 2022 18:10:30 +0530 Subject: [PATCH 11/34] fix(tests): fix tests related to tab + User cleanup --- cypress/integration/control_data.js | 2 +- cypress/integration/dashboard_links.js | 3 +++ cypress/support/commands.js | 4 ++++ frappe/public/js/frappe/form/form.js | 6 ++++-- frappe/public/js/frappe/form/layout.js | 11 ++++++++++- 5 files changed, 22 insertions(+), 4 deletions(-) diff --git a/cypress/integration/control_data.js b/cypress/integration/control_data.js index ee6dfbca95..4c7ee589ab 100644 --- a/cypress/integration/control_data.js +++ b/cypress/integration/control_data.js @@ -41,7 +41,7 @@ context("Data Control", () => { it("check custom formatters", () => { cy.visit(`/app/doctype/User`); cy.get( - '[data-fieldname="fields"] .grid-row[data-idx="2"] [data-fieldname="fieldtype"] .static-area' + '[data-fieldname="fields"] .grid-row[data-idx="3"] [data-fieldname="fieldtype"] .static-area' ).should("have.text", "Section Break"); }); diff --git a/cypress/integration/dashboard_links.js b/cypress/integration/dashboard_links.js index 31572b7976..40e9b84363 100644 --- a/cypress/integration/dashboard_links.js +++ b/cypress/integration/dashboard_links.js @@ -31,6 +31,7 @@ context("Dashboard links", () => { cy.get(".list-row-col > .level-item > .ellipsis").eq(0).click({ force: true }); //To check if initially the dashboard contains only the "Contact" link and there is no counter + cy.select_form_tab("Connections"); cy.get('[data-doctype="Contact"]').should("contain", "Contact"); //Adding a new contact @@ -44,6 +45,7 @@ context("Dashboard links", () => { cy.get(".list-row-col > .level-item > .ellipsis").eq(0).click({ force: true }); //To check if the counter for contact doc is "1" after adding the contact + cy.select_form_tab("Connections"); cy.get('[data-doctype="Contact"] > .count').should("contain", "1"); cy.get('[data-doctype="Contact"]').contains("Contact").click(); @@ -64,6 +66,7 @@ context("Dashboard links", () => { it("Report link in dashboard", () => { cy.visit("/app/user"); cy.visit("/app/user/Administrator"); + cy.select_form_tab("Connections"); cy.get('[data-doctype="Contact"]').should("contain", "Contact"); cy.findByText("Connections"); cy.window() diff --git a/cypress/support/commands.js b/cypress/support/commands.js index cbb88cb8cb..ba8eb93127 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -240,6 +240,10 @@ Cypress.Commands.add("new_form", (doctype) => { cy.get("body").should("have.attr", "data-ajax-state", "complete"); }); +Cypress.Commands.add("select_form_tab", (label) => { + cy.get(".form-tabs-list [data-toggle='tab']").contains(label).click(); +}); + Cypress.Commands.add("go_to_list", (doctype) => { let dt_in_route = doctype.toLowerCase().replace(/ /g, "-"); cy.visit(`/app/${dt_in_route}`); diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 10dd98954f..feadc08ead 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -1910,7 +1910,7 @@ frappe.ui.form.Form = class FrappeForm { } // uncollapse section - if (field.section.is_collapsed()) { + if (field.section?.is_collapsed()) { field.section.collapse(false); } @@ -1919,7 +1919,9 @@ frappe.ui.form.Form = class FrappeForm { // focus if text field if (focus) { - $el.find("input, select, textarea").focus(); + setTimeout(() => { + $el.find("input, select, textarea").focus(); + }, 500); } // highlight control inside field diff --git a/frappe/public/js/frappe/form/layout.js b/frappe/public/js/frappe/form/layout.js index 678958d7b5..44085375c3 100644 --- a/frappe/public/js/frappe/form/layout.js +++ b/frappe/public/js/frappe/form/layout.js @@ -378,11 +378,20 @@ frappe.ui.form.Layout = class Layout { const visible_tabs = this.tabs.filter((tab) => !tab.hidden); if (visible_tabs && visible_tabs.length == 1) { - visible_tabs[0].parent.toggleClass("hide show"); + visible_tabs[0].tab_link.toggleClass("hide show"); } this.set_tab_as_active(); } + select_tab(label_or_fieldname) { + for (let tab of this.tabs) { + if (tab.label.toLowerCase() === label_or_fieldname.toLowerCase() || tab.df.fieldname?.toLowerCase() === label_or_fieldname.toLowerCase()) { + tab.set_active(); + return; + } + } + } + set_tab_as_active() { let frm_active_tab = this?.frm.get_active_tab?.(); if (frm_active_tab) { From 217dd72edb3d5d9aa51dc1553a31b8e941df9c0f Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Thu, 11 Aug 2022 18:12:22 +0530 Subject: [PATCH 12/34] fix(chore): remote un-necessary commit --- frappe/core/doctype/doctype/doctype.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 47b60dcec9..64270851b3 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1761,7 +1761,4 @@ def set_field_order(doctype, field_order): # save to update frappe.get_doc("DocType", doctype).save() - - frappe.db.commit() - frappe.clear_cache(doctype=doctype) From 30f6ad622518a82757c8cf418f5b740764365bce Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Thu, 11 Aug 2022 18:19:39 +0530 Subject: [PATCH 13/34] fix(chore): lint --- frappe/core/doctype/doctype/doctype.py | 1 - 1 file changed, 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 64270851b3..e90a62f0a2 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1748,7 +1748,6 @@ def set_field_order(doctype, field_order): frappe.only_for("System Manager") - meta = frappe.get_meta(doctype) field_order = json.loads(field_order) idx = 1 From e97f709e5be6a8e57f5da5a423f801af40535bae Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Thu, 11 Aug 2022 22:38:03 +0530 Subject: [PATCH 14/34] fix(test): dashboard_link.js --- cypress/integration/dashboard_links.js | 8 ++++---- cypress/support/commands.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cypress/integration/dashboard_links.js b/cypress/integration/dashboard_links.js index 40e9b84363..995f8d0d9f 100644 --- a/cypress/integration/dashboard_links.js +++ b/cypress/integration/dashboard_links.js @@ -67,8 +67,7 @@ context("Dashboard links", () => { cy.visit("/app/user"); cy.visit("/app/user/Administrator"); cy.select_form_tab("Connections"); - cy.get('[data-doctype="Contact"]').should("contain", "Contact"); - cy.findByText("Connections"); + cy.get('.document-link[data-doctype="Contact"]').contains("Contact"); cy.window() .its("cur_frm") .then((cur_frm) => { @@ -79,8 +78,9 @@ context("Dashboard links", () => { }, ]; cur_frm.dashboard.render_report_links(); - cy.get('[data-report="Website Analytics"]').contains("Website Analytics").click(); - cy.findByText("Website Analytics"); + cy.get('.document-link[data-report="Website Analytics"]') + .contains("Website Analytics") + .click(); }); }); diff --git a/cypress/support/commands.js b/cypress/support/commands.js index ba8eb93127..44b6c6b4fb 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -241,7 +241,7 @@ Cypress.Commands.add("new_form", (doctype) => { }); Cypress.Commands.add("select_form_tab", (label) => { - cy.get(".form-tabs-list [data-toggle='tab']").contains(label).click(); + cy.get(".form-tabs-list [data-toggle='tab']").contains(label).click().wait(500); }); Cypress.Commands.add("go_to_list", (doctype) => { From 393f54a092791b8300c34c2a49179782db6851a7 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Fri, 12 Aug 2022 11:28:54 +0530 Subject: [PATCH 15/34] chore: lint --- frappe/public/js/frappe/form/layout.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/layout.js b/frappe/public/js/frappe/form/layout.js index 44085375c3..770df2a5a9 100644 --- a/frappe/public/js/frappe/form/layout.js +++ b/frappe/public/js/frappe/form/layout.js @@ -385,7 +385,10 @@ frappe.ui.form.Layout = class Layout { select_tab(label_or_fieldname) { for (let tab of this.tabs) { - if (tab.label.toLowerCase() === label_or_fieldname.toLowerCase() || tab.df.fieldname?.toLowerCase() === label_or_fieldname.toLowerCase()) { + if ( + tab.label.toLowerCase() === label_or_fieldname.toLowerCase() || + tab.df.fieldname?.toLowerCase() === label_or_fieldname.toLowerCase() + ) { tab.set_active(); return; } From 953b6bde2cbe0dcffa1377748b4eb7d10b61448c Mon Sep 17 00:00:00 2001 From: gavin Date: Thu, 19 May 2022 18:13:13 +0530 Subject: [PATCH 16/34] ci: Use separate script outside frappe This is an attempt to calculate python coverage more accurately Co-Authored-By: Ankush Menat --- .github/helper/ci.py | 102 ++++++++++++++++++++ .github/workflows/server-mariadb-tests.yml | 3 +- .github/workflows/server-postgres-tests.yml | 3 +- 3 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 .github/helper/ci.py diff --git a/.github/helper/ci.py b/.github/helper/ci.py new file mode 100644 index 0000000000..3449c464cd --- /dev/null +++ b/.github/helper/ci.py @@ -0,0 +1,102 @@ +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors +# MIT License. See LICENSE +import os +from pathlib import Path + +STANDARD_INCLUSIONS = ["*.py"] + +STANDARD_EXCLUSIONS = [ + "*.js", + "*.xml", + "*.pyc", + "*.css", + "*.less", + "*.scss", + "*.vue", + "*.html", + "*/test_*", + "*/node_modules/*", + "*/doctype/*/*_dashboard.py", + "*/patches/*", +] + +# tested via commands' test suite +TESTED_VIA_CLI = [ + "*/frappe/installer.py", + "*/frappe/build.py", + "*/frappe/database/__init__.py", + "*/frappe/database/db_manager.py", + "*/frappe/database/**/setup_db.py", +] + +FRAPPE_EXCLUSIONS = [ + "*/tests/*", + "*/commands/*", + "*/frappe/change_log/*", + "*/frappe/exceptions*", + "*/frappe/coverage.py", + "*frappe/setup.py", + "*/doctype/*/*_dashboard.py", + "*/patches/*", +] + TESTED_VIA_CLI + + +def get_bench_path(): + return Path(__file__).resolve().parents[4] + + +class CodeCoverage: + def __init__(self, with_coverage, app): + self.with_coverage = with_coverage + self.app = app or "frappe" + + def __enter__(self): + if self.with_coverage: + import os + + from coverage import Coverage + + # Generate coverage report only for app that is being tested + source_path = os.path.join(get_bench_path(), "apps", self.app) + print(f"Source path: {source_path}") + omit = STANDARD_EXCLUSIONS[:] + + if self.app == "frappe": + omit.extend(FRAPPE_EXCLUSIONS) + + self.coverage = Coverage(source=[source_path], omit=omit, include=STANDARD_INCLUSIONS) + self.coverage.start() + + def __exit__(self, exc_type, exc_value, traceback): + if self.with_coverage: + self.coverage.stop() + self.coverage.save() + self.coverage.xml_report() + + +if __name__ == "__main__": + app = "frappe" + site = os.environ.get("SITE") or "test_site" + use_orchestrator = bool(os.environ.get("ORCHESTRATOR_URL")) + build_number = 1 + total_builds = 1 + + try: + build_number = int(os.environ.get("BUILD_NUMBER")) + except Exception: + pass + + try: + total_builds = int(os.environ.get("TOTAL_BUILDS")) + except Exception: + pass + + with CodeCoverage(with_coverage=True, app=app): + if use_orchestrator: + from frappe.parallel_test_runner import ParallelTestWithOrchestrator + + ParallelTestWithOrchestrator(app, site=site) + else: + from frappe.parallel_test_runner import ParallelTestRunner + + ParallelTestRunner(app, site=site, build_number=build_number, total_builds=total_builds) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index f5b87178ee..64e6cdc381 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -122,8 +122,9 @@ jobs: - name: Run Tests if: ${{ steps.check-build.outputs.build == 'strawberry' }} - run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator --with-coverage + run: cd ~/frappe-bench/sites && ../env/bin/python3 ../apps/frappe/.github/helper/ci.py env: + SITE: test_site CI_BUILD_ID: ${{ github.run_id }} ORCHESTRATOR_URL: http://test-orchestrator.frappe.io diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index d8f9a4bf99..bec635a4e7 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -125,8 +125,9 @@ jobs: - name: Run Tests if: ${{ steps.check-build.outputs.build == 'strawberry' }} - run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator --with-coverage + run: cd ~/frappe-bench/sites && ../env/bin/python3 ../apps/frappe/.github/helper/ci.py env: + SITE: test_site CI_BUILD_ID: ${{ github.run_id }} ORCHESTRATOR_URL: http://test-orchestrator.frappe.io From 5c3d86209d135ca59fbcf3035bedcaff4be83e5d Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sun, 14 Aug 2022 18:55:30 +0200 Subject: [PATCH 17/34] feat(Language): show title in link field --- frappe/core/doctype/language/language.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/language/language.json b/frappe/core/doctype/language/language.json index 9ab8f55f6b..7e9bbb1038 100644 --- a/frappe/core/doctype/language/language.json +++ b/frappe/core/doctype/language/language.json @@ -51,7 +51,7 @@ "icon": "fa fa-globe", "in_create": 1, "links": [], - "modified": "2021-10-18 14:02:06.818219", + "modified": "2022-08-14 18:54:03.490836", "modified_by": "Administrator", "module": "Core", "name": "Language", @@ -76,8 +76,10 @@ } ], "search_fields": "language_name", + "show_title_field_in_link": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "title_field": "language_name", "track_changes": 1 } \ No newline at end of file From c8eab1ef7514d2f42d652162444853f7c7d08bcb Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 15 Aug 2022 12:51:11 +0200 Subject: [PATCH 18/34] test: don't check for specific exception title This would be testing other libraries. Not our job. --- frappe/core/doctype/user/test_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index 2367f101c9..df3993e31c 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -201,7 +201,7 @@ class TestUser(unittest.TestCase): user = frappe.get_doc("User", "test@example.com") frappe.flags.in_test = False user.new_password = "password" - self.assertRaisesRegex(frappe.exceptions.ValidationError, "Invalid Password", user.save) + self.assertRaises(frappe.exceptions.ValidationError, user.save) user.reload() user.new_password = "Eastern_43A1W" user.save() From 2cf45366ecbd929058ecb953af76c930b89cba16 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 15 Aug 2022 13:52:32 +0200 Subject: [PATCH 19/34] test: merge password validation test cases --- frappe/core/doctype/user/test_user.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/frappe/core/doctype/user/test_user.py b/frappe/core/doctype/user/test_user.py index df3993e31c..d4536c6355 100644 --- a/frappe/core/doctype/user/test_user.py +++ b/frappe/core/doctype/user/test_user.py @@ -192,6 +192,12 @@ class TestUser(unittest.TestCase): # Score 1; should now fail result = test_password_strength("bee2ve") self.assertEqual(result["feedback"]["password_policy_validation_passed"], False) + self.assertRaises( + frappe.exceptions.ValidationError, handle_password_test_fail, result["feedback"] + ) + self.assertRaises( + frappe.exceptions.ValidationError, handle_password_test_fail, result + ) # test backwards compatibility # Score 4; should pass result = test_password_strength("Eastern_43A1W") @@ -207,15 +213,6 @@ class TestUser(unittest.TestCase): user.save() frappe.flags.in_test = True - def test_password_validation(self): - result = test_password_strength("P@ssw0rd") - feedback = result["feedback"] - self.assertEqual(feedback["password_policy_validation_passed"], False) - self.assertRaises(frappe.exceptions.ValidationError, handle_password_test_fail, feedback) - - # test backwards compatibility - self.assertRaises(frappe.exceptions.ValidationError, handle_password_test_fail, result) - def test_comment_mentions(self): comment = """ From 26bf65b87ca78742d9eda3804872320bd311fa27 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 13:55:13 +0530 Subject: [PATCH 20/34] test: phone number validation --- frappe/tests/test_utils.py | 15 +++++++++++++++ frappe/utils/__init__.py | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 42094e145f..cfb9003a5e 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -32,6 +32,7 @@ from frappe.utils import ( random_string, scrub_urls, validate_email_address, + validate_phone_number_with_country_code, validate_url, ) from frappe.utils.data import ( @@ -322,11 +323,25 @@ class TestValidationUtils(unittest.TestCase): self.assertFalse(validate_email_address("someone")) self.assertFalse(validate_email_address("someone@----.com")) + self.assertFalse(validate_email_address("test@example.com test2@example.com,undisclosed-recipient")) + # Invalid with throw self.assertRaises( frappe.InvalidEmailAddressError, validate_email_address, "someone.com", throw=True ) + def test_valid_phone(self): + valid_phones = ["+91 1234567890", ""] + + for phone in valid_phones: + validate_phone_number_with_country_code(phone, "field") + self.assertRaises( + frappe.InvalidPhoneNumberError, + validate_phone_number_with_country_code, + "+420 1234567890", + "field", + ) + class TestImage(unittest.TestCase): def test_strip_exif_data(self): diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 9f25b33266..bc6c027f35 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -91,7 +91,7 @@ def extract_email_id(email): return email_id -def validate_phone_number_with_country_code(phone_number, fieldname): +def validate_phone_number_with_country_code(phone_number: str, fieldname: str) -> None: from phonenumbers import NumberParseException, is_valid_number, parse from frappe import _ From 9bf9256049f01118d0104745bf20fc4f2b2829e4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 14:00:48 +0530 Subject: [PATCH 21/34] test: validate name --- frappe/tests/test_utils.py | 10 ++++++++++ frappe/utils/__init__.py | 2 ++ 2 files changed, 12 insertions(+) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index cfb9003a5e..7f2f9f5779 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -32,6 +32,7 @@ from frappe.utils import ( random_string, scrub_urls, validate_email_address, + validate_name, validate_phone_number_with_country_code, validate_url, ) @@ -342,6 +343,15 @@ class TestValidationUtils(unittest.TestCase): "field", ) + def test_validate_name(self): + valid_names = ["", "abc", "asd a13", "asd-asd"] + for name in valid_names: + validate_name(name, True) + + invalid_names = ["asd$wat", "asasd/ads"] + for name in invalid_names: + self.assertRaises(frappe.InvalidNameError, validate_name, name, True) + class TestImage(unittest.TestCase): def test_strip_exif_data(self): diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index bc6c027f35..3390c36b50 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -138,6 +138,8 @@ def validate_name(name, throw=False): """Returns True if the name is valid valid names may have unicode and ascii characters, dash, quotes, numbers anything else is considered invalid + + Note: "Name" here is name of a person, not the primary key in Frappe doctypes. """ if not name: return False From 9b5565437cdf23a9f0f7ea5a6d095add8d666db3 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 14:02:35 +0530 Subject: [PATCH 22/34] refactor: extract_email_id condition The condition made no sense and could never be True. --- frappe/utils/__init__.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 3390c36b50..f8fcf6ae7a 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -85,10 +85,7 @@ def get_formatted_email(user, mail=None): def extract_email_id(email): """fetch only the email part of the Email Address""" - email_id = parse_addr(email)[1] - if email_id and isinstance(email_id, str) and not isinstance(email_id, str): - email_id = email_id.decode("utf-8", "ignore") - return email_id + return cstr(parse_addr(email)[1]) def validate_phone_number_with_country_code(phone_number: str, fieldname: str) -> None: From ad4cb710f20a56a34714223e26557fb01f7c454c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 14:11:16 +0530 Subject: [PATCH 23/34] refactor: duplication in gravatar code --- frappe/tests/test_utils.py | 3 ++- frappe/utils/__init__.py | 12 +++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 7f2f9f5779..e3c2914c09 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -19,6 +19,7 @@ from PIL import Image import frappe from frappe.installer import parse_app_name from frappe.model.document import Document +from frappe.tests.utils import FrappeTestCase from frappe.utils import ( ceil, evaluate_filters, @@ -749,7 +750,7 @@ class TestLazyLoader(unittest.TestCase): self.assertEqual(["Module `frappe.tests.data.load_sleep` loaded"], output) -class TestIdenticon(unittest.TestCase): +class TestIdenticon(FrappeTestCase): def test_get_gravatar(self): # developers@frappe.io has a gravatar linked so str URL will be returned frappe.flags.in_test = False diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index f8fcf6ae7a..bc54728579 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -14,6 +14,7 @@ from email.header import decode_header, make_header from email.utils import formataddr, parseaddr from gzip import GzipFile from urllib.parse import quote, urlparse +from typing import Literal from redis.exceptions import ConnectionError from traceback_with_variables import iter_exc_lines @@ -260,9 +261,7 @@ def has_gravatar(email): # since querying gravatar for every item will be slow return "" - hexdigest = hashlib.md5(frappe.as_unicode(email).encode("utf-8")).hexdigest() - - gravatar_url = f"https://secure.gravatar.com/avatar/{hexdigest}?d=404&s=200" + gravatar_url = get_gravatar_url(email, "404") try: res = requests.get(gravatar_url) if res.status_code == 200: @@ -273,10 +272,9 @@ def has_gravatar(email): return "" -def get_gravatar_url(email): - return "https://secure.gravatar.com/avatar/{hash}?d=mm&s=200".format( - hash=hashlib.md5(email.encode("utf-8")).hexdigest() - ) +def get_gravatar_url(email, default: Literal["mm", "404"]="mm"): + hexdigest = hashlib.md5(frappe.as_unicode(email).encode("utf-8")).hexdigest() + return f"https://secure.gravatar.com/avatar/{hexdigest}?d={default}&s=200" def get_gravatar(email): From 3fa4ec1bd6c4682691f011839b80583b59151635 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 14:32:39 +0530 Subject: [PATCH 24/34] refactor: container utils and misc tests --- frappe/tests/test_utils.py | 69 +++++++++++++++++++++++++++++++++++++- frappe/utils/__init__.py | 65 ++++++++++++++++++----------------- 2 files changed, 103 insertions(+), 31 deletions(-) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index e3c2914c09..94c858f06d 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -22,15 +22,22 @@ from frappe.model.document import Document from frappe.tests.utils import FrappeTestCase from frappe.utils import ( ceil, + dict_to_str, evaluate_filters, + execute_in_shell, floor, format_timedelta, get_bench_path, + get_file_timestamp, get_gravatar, + get_site_info, + get_sites, get_url, money_in_words, parse_timedelta, random_string, + remove_blanks, + safe_json_loads, scrub_urls, validate_email_address, validate_name, @@ -39,13 +46,18 @@ from frappe.utils import ( ) from frappe.utils.data import ( add_to_date, + add_years, cast, + cstr, + duration_to_seconds, get_first_day_of_week, get_time, get_timedelta, + get_timespan_date_range, getdate, now_datetime, nowtime, + to_timedelta, validate_python_code, ) from frappe.utils.dateutils import get_dates_from_timegrain @@ -325,7 +337,9 @@ class TestValidationUtils(unittest.TestCase): self.assertFalse(validate_email_address("someone")) self.assertFalse(validate_email_address("someone@----.com")) - self.assertFalse(validate_email_address("test@example.com test2@example.com,undisclosed-recipient")) + self.assertFalse( + validate_email_address("test@example.com test2@example.com,undisclosed-recipient") + ) # Invalid with throw self.assertRaises( @@ -502,6 +516,24 @@ class TestDateUtils(unittest.TestCase): self.assertIsInstance(get_timedelta(str(timedelta_input)), timedelta) self.assertIsInstance(get_timedelta(str(time_input)), timedelta) + def test_to_timedelta(self): + self.assertEqual(to_timedelta("00:00:01"), timedelta(seconds=1)) + self.assertEqual(to_timedelta("10:00:01"), timedelta(seconds=1, hours=10)) + self.assertEqual(to_timedelta(time(hour=2)), timedelta(hours=2)) + + def test_add_date_utils(self): + self.assertEqual(add_years(datetime(2020, 1, 1), 1), datetime(2021, 1, 1)) + + def test_duration_to_sec(self): + self.assertEqual(duration_to_seconds("3h 34m 45s"), 12885) + self.assertEqual(duration_to_seconds("1h"), 3600) + self.assertEqual(duration_to_seconds("110m"), 110 * 60) + self.assertEqual(duration_to_seconds("110m"), 110 * 60) + + + def test_get_timespan_date_range(self): + get_timespan_date_range() + def test_date_from_timegrain(self): start_date = getdate("2021-01-01") @@ -773,3 +805,38 @@ class TestIdenticon(FrappeTestCase): identicon_bs64 = identicon.base64() self.assertIsInstance(identicon_bs64, str) self.assertTrue(identicon_bs64.startswith("data:image/png;base64,")) + + +class TestContainerUtils(FrappeTestCase): + def test_dict_to_str(self): + self.assertEqual(dict_to_str({"a": "b"}), "a=b") + + def test_remove_blanks(self): + a = {"asd": "", "b": None, "c": "d"} + remove_blanks(a) + self.assertEqual(len(a), 1) + self.assertEqual(a["c"], "d") + + +class TestMiscUtils(FrappeTestCase): + def test_get_file_timestamp(self): + self.assertIsInstance(get_file_timestamp(__file__), str) + + def test_execute_in_shell(self): + err, out = execute_in_shell("ls") + self.assertIn("apps", cstr(out)) + + def test_get_all_sites(self): + self.assertIn(frappe.local.site, get_sites()) + + def test_get_site_info(self): + info = get_site_info() + + installed_apps = [app["app_name"] for app in info["installed_apps"]] + self.assertIn("frappe", installed_apps) + self.assertGreaterEqual(len(info["users"]), 1) + + def test_safe_json_load(self): + self.assertEqual(safe_json_loads("{}"), {}) + self.assertEqual(safe_json_loads("{ /}"), "{ /}") + self.assertEqual(safe_json_loads("12"), 12) # this is a quirk diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index bc54728579..e2ec445be4 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -9,12 +9,19 @@ import os import re import sys import traceback -from collections.abc import Generator, Iterable, MutableMapping, MutableSequence, Sequence +from collections.abc import ( + Container, + Generator, + Iterable, + MutableMapping, + MutableSequence, + Sequence, +) from email.header import decode_header, make_header from email.utils import formataddr, parseaddr from gzip import GzipFile +from typing import Any, Literal from urllib.parse import quote, urlparse -from typing import Literal from redis.exceptions import ConnectionError from traceback_with_variables import iter_exc_lines @@ -218,7 +225,11 @@ def split_emails(txt): return email_list -def validate_url(txt, throw=False, valid_schemes=None): +def validate_url( + txt: str, + throw: bool = False, + valid_schemes: str | Container[str] | None = None, +) -> bool: """ Checks whether `txt` has a valid URL string @@ -244,7 +255,7 @@ def validate_url(txt, throw=False, valid_schemes=None): return is_valid -def random_string(length): +def random_string(length: int) -> str: """generate a random string""" import string from random import choice @@ -252,7 +263,7 @@ def random_string(length): return "".join(choice(string.ascii_letters + string.digits) for i in range(length)) -def has_gravatar(email): +def has_gravatar(email: str) -> str: """Returns gravatar url if user has set an avatar at gravatar.com""" import requests @@ -272,12 +283,12 @@ def has_gravatar(email): return "" -def get_gravatar_url(email, default: Literal["mm", "404"]="mm"): +def get_gravatar_url(email: str, default: Literal["mm", "404"] = "mm") -> str: hexdigest = hashlib.md5(frappe.as_unicode(email).encode("utf-8")).hexdigest() return f"https://secure.gravatar.com/avatar/{hexdigest}?d={default}&s=200" -def get_gravatar(email): +def get_gravatar(email: str) -> str: from frappe.utils.identicon import Identicon return has_gravatar(email) or Identicon(email).base64() @@ -307,7 +318,7 @@ def log(event, details): frappe.logger(event).info(details) -def dict_to_str(args, sep="&"): +def dict_to_str(args: dict[str, Any], sep: str = "&") -> str: """ Converts a dictionary to URL """ @@ -343,18 +354,13 @@ def set_default(key, val): return frappe.db.set_default(key, val) -def remove_blanks(d): +def remove_blanks(d: dict) -> dict: """ - Returns d with empty ('' or None) values stripped + Returns d with empty ('' or None) values stripped. Mutates inplace. """ - empty_keys = [] - for key in d: - if d[key] == "" or d[key] is None: - # del d[key] raises runtime exception, using a workaround - empty_keys.append(key) - for key in empty_keys: - del d[key] - + for k, v in tuple(d.items()): + if v == "" or v == None: + del d[k] return d @@ -414,21 +420,20 @@ def execute_in_shell(cmd, verbose=0, low_priority=False): import tempfile from subprocess import Popen - with tempfile.TemporaryFile() as stdout: - with tempfile.TemporaryFile() as stderr: - kwargs = {"shell": True, "stdout": stdout, "stderr": stderr} + with (tempfile.TemporaryFile() as stdout, tempfile.TemporaryFile() as stderr): + kwargs = {"shell": True, "stdout": stdout, "stderr": stderr} - if low_priority: - kwargs["preexec_fn"] = lambda: os.nice(10) + if low_priority: + kwargs["preexec_fn"] = lambda: os.nice(10) - p = Popen(cmd, **kwargs) - p.wait() + p = Popen(cmd, **kwargs) + p.wait() - stdout.seek(0) - out = stdout.read() + stdout.seek(0) + out = stdout.read() - stderr.seek(0) - err = stderr.read() + stderr.seek(0) + err = stderr.read() if verbose: if err: @@ -560,7 +565,7 @@ def update_progress_bar(txt, i, l, absolute=False): sys.stdout.flush() return - if not getattr(frappe.local, "request", None) or is_cli(): + if not getattr(frappe.local, "request", None) or is_cli(): # pragma: no cover lt = len(txt) try: col = 40 if os.get_terminal_size().columns > 80 else 20 From a88819230a8967cf50fde1d831f39771e44369d0 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 19:13:52 +0530 Subject: [PATCH 25/34] refactor: convert get_timespan_date_range to use match --- frappe/utils/data.py | 121 ++++++++++++++++++++++++------------------- 1 file changed, 69 insertions(+), 52 deletions(-) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index bdef60b930..dcbbce1f74 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -724,60 +724,77 @@ def get_weekday(datetime: datetime.datetime | None = None) -> str: return weekdays[datetime.weekday()] -def get_timespan_date_range(timespan: str) -> tuple[datetime.datetime, datetime.datetime]: +def get_timespan_date_range(timespan: str) -> tuple[datetime.datetime, datetime.datetime] | None: today = nowdate() - date_range_map = { - "last week": lambda: ( - get_first_day_of_week(add_to_date(today, days=-7)), - get_last_day_of_week(add_to_date(today, days=-7)), - ), - "last month": lambda: ( - get_first_day(add_to_date(today, months=-1)), - get_last_day(add_to_date(today, months=-1)), - ), - "last quarter": lambda: ( - get_quarter_start(add_to_date(today, months=-3)), - get_quarter_ending(add_to_date(today, months=-3)), - ), - "last 6 months": lambda: ( - get_quarter_start(add_to_date(today, months=-6)), - get_quarter_ending(add_to_date(today, months=-3)), - ), - "last year": lambda: ( - get_year_start(add_to_date(today, years=-1)), - get_year_ending(add_to_date(today, years=-1)), - ), - "yesterday": lambda: (add_to_date(today, days=-1),) * 2, - "today": lambda: (today, today), - "tomorrow": lambda: (add_to_date(today, days=1),) * 2, - "this week": lambda: (get_first_day_of_week(today), get_last_day_of_week(today)), - "this month": lambda: (get_first_day(today), get_last_day(today)), - "this quarter": lambda: (get_quarter_start(today), get_quarter_ending(today)), - "this year": lambda: (get_year_start(today), get_year_ending(today)), - "next week": lambda: ( - get_first_day_of_week(add_to_date(today, days=7)), - get_last_day_of_week(add_to_date(today, days=7)), - ), - "next month": lambda: ( - get_first_day(add_to_date(today, months=1)), - get_last_day(add_to_date(today, months=1)), - ), - "next quarter": lambda: ( - get_quarter_start(add_to_date(today, months=3)), - get_quarter_ending(add_to_date(today, months=3)), - ), - "next 6 months": lambda: ( - get_quarter_start(add_to_date(today, months=3)), - get_quarter_ending(add_to_date(today, months=6)), - ), - "next year": lambda: ( - get_year_start(add_to_date(today, years=1)), - get_year_ending(add_to_date(today, years=1)), - ), - } - if timespan in date_range_map: - return date_range_map[timespan]() + match timespan: + case "last week": + return ( + get_first_day_of_week(add_to_date(today, days=-7)), + get_last_day_of_week(add_to_date(today, days=-7)), + ) + case "last month": + return ( + get_first_day(add_to_date(today, months=-1)), + get_last_day(add_to_date(today, months=-1)), + ) + case "last quarter": + return ( + get_quarter_start(add_to_date(today, months=-3)), + get_quarter_ending(add_to_date(today, months=-3)), + ) + case "last 6 months": + return ( + get_quarter_start(add_to_date(today, months=-6)), + get_quarter_ending(add_to_date(today, months=-3)), + ) + case "last year": + return ( + get_year_start(add_to_date(today, years=-1)), + get_year_ending(add_to_date(today, years=-1)), + ) + + case "yesterday": + return (add_to_date(today, days=-1),) * 2 + case "today": + return (today, today) + case "tomorrow": + return (add_to_date(today, days=1),) * 2 + case "this week": + return (get_first_day_of_week(today), get_last_day_of_week(today)) + case "this month": + return (get_first_day(today), get_last_day(today)) + case "this quarter": + return (get_quarter_start(today), get_quarter_ending(today)) + case "this year": + return (get_year_start(today), get_year_ending(today)) + case "next week": + return ( + get_first_day_of_week(add_to_date(today, days=7)), + get_last_day_of_week(add_to_date(today, days=7)), + ) + case "next month": + return ( + get_first_day(add_to_date(today, months=1)), + get_last_day(add_to_date(today, months=1)), + ) + case "next quarter": + return ( + get_quarter_start(add_to_date(today, months=3)), + get_quarter_ending(add_to_date(today, months=3)), + ) + case "next 6 months": + return ( + get_quarter_start(add_to_date(today, months=3)), + get_quarter_ending(add_to_date(today, months=6)), + ) + case "next year": + return ( + get_year_start(add_to_date(today, years=1)), + get_year_ending(add_to_date(today, years=1)), + ) + case _: + return def global_date_format(date, format="long"): From 9de31d03c127fc2ba7a2e4401e4ad2ead88146d9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 19:18:47 +0530 Subject: [PATCH 26/34] refactor!: timespan utils consistent output BREAKING CHANGE: - `get_year_ending` returns datetime.date instead of str - `get_timespan_date_range` will always return datetime.date ranges --- frappe/tests/test_utils.py | 35 +++++++++++++++++++++++++++++++++-- frappe/utils/data.py | 10 ++++------ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 94c858f06d..4ecea82a31 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -54,6 +54,7 @@ from frappe.utils.data import ( get_time, get_timedelta, get_timespan_date_range, + get_year_ending, getdate, now_datetime, nowtime, @@ -530,9 +531,39 @@ class TestDateUtils(unittest.TestCase): self.assertEqual(duration_to_seconds("110m"), 110 * 60) self.assertEqual(duration_to_seconds("110m"), 110 * 60) - def test_get_timespan_date_range(self): - get_timespan_date_range() + + supported_timespans = [ + "last week", + "last month", + "last quarter", + "last 6 months", + "last year", + "yesterday", + "today", + "tomorrow", + "this week", + "this month", + "this quarter", + "this year", + "next week", + "next month", + "next quarter", + "next 6 months", + "next year", + ] + + for ts in supported_timespans: + res = get_timespan_date_range(ts) + self.assertEqual(len(res), 2) + + # Manual type checking eh? + self.assertIsInstance(res[0], date) + self.assertIsInstance(res[1], date) + + def test_timesmap_utils(self): + self.assertEqual(get_year_ending(date(2021, 1, 1)), date(2021, 12, 31)) + self.assertEqual(get_year_ending(date(2021, 1, 31)), date(2021, 12, 31)) def test_date_from_timegrain(self): start_date = getdate("2021-01-01") diff --git a/frappe/utils/data.py b/frappe/utils/data.py index dcbbce1f74..1e0927dbfa 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -483,13 +483,11 @@ def get_quarter_ending(date): return date -def get_year_ending(date): +def get_year_ending(date) -> datetime.date: """returns year ending of the given date""" date = getdate(date) - # first day of next year (note year starts from 1) - date = add_to_date(f"{date.year}-01-01", months=12) - # last day of this month - return add_to_date(date, days=-1) + next_year_start = datetime.date(date.year + 1, 1, 1) + return add_to_date(next_year_start, days=-1) def get_time(time_str: str) -> datetime.time: @@ -725,7 +723,7 @@ def get_weekday(datetime: datetime.datetime | None = None) -> str: def get_timespan_date_range(timespan: str) -> tuple[datetime.datetime, datetime.datetime] | None: - today = nowdate() + today = getdate() match timespan: case "last week": From 53af10a0649e7b963de6571051520dbcd13576ee Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 19:45:26 +0530 Subject: [PATCH 27/34] refactor!: python pretty_date consistent with JS Python pretty date was ceiling month and week instead of flooring them, this is incorrect AND inconsistent with popular JS library pretty date that we use on client side --- frappe/tests/test_utils.py | 27 +++++++++++++++++++++++++++ frappe/utils/data.py | 6 +++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 4ecea82a31..9dd8661fc4 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -50,6 +50,7 @@ from frappe.utils.data import ( cast, cstr, duration_to_seconds, + get_datetime, get_first_day_of_week, get_time, get_timedelta, @@ -58,6 +59,7 @@ from frappe.utils.data import ( getdate, now_datetime, nowtime, + pretty_date, to_timedelta, validate_python_code, ) @@ -565,6 +567,31 @@ class TestDateUtils(unittest.TestCase): self.assertEqual(get_year_ending(date(2021, 1, 1)), date(2021, 12, 31)) self.assertEqual(get_year_ending(date(2021, 1, 31)), date(2021, 12, 31)) + def test_pretty_date(self): + from frappe import _ + + # differnt cases + now = get_datetime() + + test_cases = { + now: _("just now"), + add_to_date(now, minutes=-1): _("1 minute ago"), + add_to_date(now, minutes=-3): _("3 minutes ago"), + add_to_date(now, hours=-1): _("1 hour ago"), + add_to_date(now, hours=-2): _("2 hours ago"), + add_to_date(now, days=-1): _("Yesterday"), + add_to_date(now, days=-5): _("5 days ago"), + add_to_date(now, days=-8): _("1 week ago"), + add_to_date(now, days=-14): _("2 weeks ago"), + add_to_date(now, days=-32): _("1 month ago"), + add_to_date(now, days=-32 * 2): _("2 months ago"), + add_to_date(now, years=-1, days=-5): _("1 year ago"), + add_to_date(now, years=-2, days=-10): _("2 years ago"), + } + + for dt, exp_message in test_cases.items(): + self.assertEqual(pretty_date(dt), exp_message) + def test_date_from_timegrain(self): start_date = getdate("2021-01-01") diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 1e0927dbfa..a856e44c31 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -1475,15 +1475,15 @@ def pretty_date(iso_datetime: datetime.datetime | str) -> str: elif dt_diff_days < 12: return _("1 week ago") elif dt_diff_days < 31.0: - return _("{0} weeks ago").format(cint(math.ceil(dt_diff_days / 7.0))) + return _("{0} weeks ago").format(dt_diff_days // 7) elif dt_diff_days < 46: return _("1 month ago") elif dt_diff_days < 365.0: - return _("{0} months ago").format(cint(math.ceil(dt_diff_days / 30.0))) + return _("{0} months ago").format(dt_diff_days // 30) elif dt_diff_days < 550.0: return _("1 year ago") else: - return f"{cint(math.floor(dt_diff_days / 365.0))} years ago" + return _("{0} years ago").format(dt_diff_days // 365) def comma_or(some_list, add_quotes=True): From 9f31723555876dc0f04f7f9f407b03a5903a50dd Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 19:50:54 +0530 Subject: [PATCH 28/34] refactor: directly map function over lambda call --- frappe/utils/data.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/frappe/utils/data.py b/frappe/utils/data.py index a856e44c31..6d4a96ce5f 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -1673,14 +1673,14 @@ operator_map = { "in": lambda a, b: operator.contains(b, a), "not in": lambda a, b: not operator.contains(b, a), # comparison operators - "=": lambda a, b: operator.eq(a, b), - "!=": lambda a, b: operator.ne(a, b), - ">": lambda a, b: operator.gt(a, b), - "<": lambda a, b: operator.lt(a, b), - ">=": lambda a, b: operator.ge(a, b), - "<=": lambda a, b: operator.le(a, b), - "not None": lambda a, b: a and True or False, - "None": lambda a, b: (not a) and True or False, + "=": operator.eq, + "!=": operator.ne, + ">": operator.gt, + "<": operator.lt, + ">=": operator.ge, + "<=": operator.le, + "not None": lambda a, b: a is not None, + "None": lambda a, b: a is None, } @@ -1702,13 +1702,12 @@ def evaluate_filters(doc, filters: dict | list | tuple): def compare(val1: Any, condition: str, val2: Any, fieldtype: str | None = None): - ret = False if fieldtype: val2 = cast(fieldtype, val2) if condition in operator_map: - ret = operator_map[condition](val1, val2) + return operator_map[condition](val1, val2) - return ret + return False def get_filter(doctype: str, f: dict | list | tuple, filters_config=None) -> "frappe._dict": From 4391f8d0f176f8a792719973e34a92da25600b39 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 14 Aug 2022 20:04:15 +0530 Subject: [PATCH 29/34] ci: dont check python syntax for patch test patch test runs across multiple versions, doesn't make sense here. --- .github/workflows/patch-mariadb-tests.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/patch-mariadb-tests.yml b/.github/workflows/patch-mariadb-tests.yml index 61c01870b3..ffc64a946f 100644 --- a/.github/workflows/patch-mariadb-tests.yml +++ b/.github/workflows/patch-mariadb-tests.yml @@ -30,9 +30,8 @@ jobs: - name: Clone uses: actions/checkout@v3 - - name: Check for valid Python & Merge Conflicts + - name: Check for Merge Conflicts run: | - python -m compileall -f "${GITHUB_WORKSPACE}" if grep -lr --exclude-dir=node_modules "^<<<<<<< " "${GITHUB_WORKSPACE}" then echo "Found merge conflicts" exit 1 From b7a7f97a7a3a6f20cba3d5e494f7d65fa6821f39 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 16 Aug 2022 10:22:01 +0530 Subject: [PATCH 30/34] refactor: convert get email count query to ORM - also fix capitalization of column names on postgres --- frappe/email/queue.py | 30 ++++++++++-------------------- frappe/utils/__init__.py | 2 +- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/frappe/email/queue.py b/frappe/email/queue.py index bc02c6be32..b593dd9a21 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -3,9 +3,8 @@ import frappe from frappe import _, msgprint -from frappe.query_builder import DocType, Interval -from frappe.query_builder.functions import Now from frappe.utils import cint, get_url, now_datetime +from frappe.utils.data import getdate from frappe.utils.verified_command import get_signed_params, verify_request @@ -16,26 +15,17 @@ def get_emails_sent_this_month(email_account=None): if email_account=None, email account filter is not applied while counting """ - q = """ - SELECT - COUNT(*) - FROM - `tabEmail Queue` - WHERE - `status`='Sent' - AND - EXTRACT(YEAR_MONTH FROM `creation`) = EXTRACT(YEAR_MONTH FROM NOW()) - """ + today = getdate() + month_start = today.replace(day=1) - q_args = {} - if email_account is not None: - if email_account: - q += " AND email_account = %(email_account)s" - q_args["email_account"] = email_account - else: - q += " AND (email_account is null OR email_account='')" + filters = { + "status": "Sent", + "creation": [">=", str(month_start)], + } + if email_account: + filters["email_account"] = email_account - return frappe.db.sql(q, q_args)[0][0] + return frappe.db.count("Email Queue", filters=filters) def get_emails_sent_today(email_account=None): diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index e2ec445be4..f84ad5a0da 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -748,7 +748,7 @@ def get_site_info(): kwargs = { "fields": ["user", "creation", "full_name"], - "filters": {"Operation": "Login", "Status": "Success"}, + "filters": {"operation": "Login", "status": "Success"}, "limit": "10", } From c9da8d87e5f0b9f5d3a07ec0862c9f0bf0e8d024 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 16 Aug 2022 11:07:49 +0530 Subject: [PATCH 31/34] ci: dont submit coverage if tests din't run --- .github/workflows/server-mariadb-tests.yml | 5 +++++ .github/workflows/server-postgres-tests.yml | 5 +++++ .github/workflows/ui-tests.yml | 7 ++++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml index 64e6cdc381..2a0915f387 100644 --- a/.github/workflows/server-mariadb-tests.yml +++ b/.github/workflows/server-mariadb-tests.yml @@ -20,6 +20,9 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 + outputs: + build: ${{ steps.check-build.outputs.build }} + strategy: fail-fast: false matrix: @@ -144,9 +147,11 @@ jobs: uses: actions/checkout@v3 - name: Download artifacts + if: ${{ needs.test.outputs.build == 'strawberry' }} uses: actions/download-artifact@v3 - name: Upload coverage data + if: ${{ needs.test.outputs.build == 'strawberry' }} uses: codecov/codecov-action@v3 with: name: MariaDB diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml index bec635a4e7..1537430384 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-postgres-tests.yml @@ -19,6 +19,9 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 + outputs: + build: ${{ steps.check-build.outputs.build }} + strategy: fail-fast: false matrix: @@ -147,9 +150,11 @@ jobs: uses: actions/checkout@v3 - name: Download artifacts + if: ${{ needs.test.outputs.build == 'strawberry' }} uses: actions/download-artifact@v3 - name: Upload coverage data + if: ${{ needs.test.outputs.build == 'strawberry' }} uses: codecov/codecov-action@v3 with: name: Postgres diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 4d5ec5c1db..b1d56a963c 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -17,6 +17,8 @@ jobs: test: runs-on: ubuntu-latest timeout-minutes: 60 + outputs: + build: ${{ steps.check-build.outputs.build }} strategy: fail-fast: false @@ -184,18 +186,21 @@ jobs: uses: actions/checkout@v2 - name: Download artifacts + if: ${{ needs.test.outputs.build == 'strawberry' }} uses: actions/download-artifact@v3 - name: Upload python coverage data + if: ${{ needs.test.outputs.build == 'strawberry' }} uses: codecov/codecov-action@v3 with: - name: MariaDB + name: UIBackend fail_ci_if_error: true verbose: true files: ./coverage-py-1/coverage.xml,./coverage-py-2/coverage.xml,./coverage-py-3/coverage.xml flags: server-ui - name: Upload JS coverage data + if: ${{ needs.test.outputs.build == 'strawberry' }} uses: codecov/codecov-action@v3 with: name: Cypress From d61705f528da80c5ea657d38eb08ea57dd631a2d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 16 Aug 2022 10:05:06 +0530 Subject: [PATCH 32/34] perf: use cached docs for system settings --- frappe/__init__.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 1d3cf7f62e..a796db9a83 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -2283,14 +2283,22 @@ def safe_eval(code, eval_globals=None, eval_locals=None): def get_website_settings(key): if not hasattr(local, "website_settings"): - local.website_settings = db.get_singles_dict("Website Settings", cast=True) + try: + local.website_settings = get_cached_doc("Website Settings") + except DoesNotExistError: + clear_last_message() + return return local.website_settings.get(key) def get_system_settings(key): if not hasattr(local, "system_settings"): - local.system_settings = db.get_singles_dict("System Settings", cast=True) + try: + local.system_settings = get_cached_doc("System Settings") + except DoesNotExistError: # possible during new install + clear_last_message() + return return local.system_settings.get(key) From 29b991749fc0390f09974229b54ab603c1c957b0 Mon Sep 17 00:00:00 2001 From: gavin Date: Tue, 16 Aug 2022 12:28:37 +0530 Subject: [PATCH 33/34] docs: Update README shields (#17841) * docs: Update README shields * docs: Set shields colour to success instead of green --- README.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 4942d87e18..01f4199fdd 100644 --- a/README.md +++ b/README.md @@ -14,25 +14,28 @@
+ + + + + + + + + - - - - - -
-Full-stack web application framework that uses Python and MariaDB on the server side and a tightly integrated client side library. Built for [ERPNext](https://erpnext.com) +Full-stack web application framework that uses Python and MariaDB on the server side and a tightly integrated client side library. Built for [ERPNext](https://erpnext.com).
From 3ec4a618e3a9c56b56c26f11cd76e157dddd3179 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 16 Aug 2022 14:10:12 +0530 Subject: [PATCH 34/34] test: FrappeClient tests not skipped (#17843) --- .github/helper/ci.py | 1 + frappe/test_runner.py | 1 - frappe/tests/test_frappe_client.py | 18 +++--------------- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/.github/helper/ci.py b/.github/helper/ci.py index 3449c464cd..2eadd468c1 100644 --- a/.github/helper/ci.py +++ b/.github/helper/ci.py @@ -18,6 +18,7 @@ STANDARD_EXCLUSIONS = [ "*/node_modules/*", "*/doctype/*/*_dashboard.py", "*/patches/*", + ".github/*", ] # tested via commands' test suite diff --git a/frappe/test_runner.py b/frappe/test_runner.py index e8c1656ca8..1e3573336a 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -143,7 +143,6 @@ def set_test_email_config(): "mail_server": "smtp.example.com", "mail_login": "test@example.com", "mail_password": "test", - "admin_password": "admin", } ) diff --git a/frappe/tests/test_frappe_client.py b/frappe/tests/test_frappe_client.py index facb0b3b72..e80e43f49c 100644 --- a/frappe/tests/test_frappe_client.py +++ b/frappe/tests/test_frappe_client.py @@ -2,31 +2,19 @@ # License: MIT. See LICENSE import base64 -import unittest import requests import frappe from frappe.core.doctype.user.user import generate_keys -from frappe.frappeclient import AuthError, FrappeClient, FrappeException +from frappe.frappeclient import FrappeClient, FrappeException +from frappe.tests.utils import FrappeTestCase from frappe.utils.data import get_url -class TestFrappeClient(unittest.TestCase): +class TestFrappeClient(FrappeTestCase): PASSWORD = frappe.conf.admin_password or "admin" - @classmethod - def setUpClass(cls) -> None: - site_url = get_url() - try: - FrappeClient(site_url, "Administrator", cls.PASSWORD, verify=False) - except AuthError: - raise unittest.SkipTest( - f"AuthError raised for {site_url} [usr=Administrator, pwd={cls.PASSWORD}]" - ) - - return super().setUpClass() - def test_insert_many(self): server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) frappe.db.delete("Note", {"title": ("in", ("Sing", "a", "song", "of", "sixpence"))})