From 0eab7ed02109d976868a57c893ceac5181183633 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 31 Jan 2022 13:31:31 +0530 Subject: [PATCH 1/6] fix: Call validate_link only if "value" is passed --- frappe/public/js/frappe/form/controls/link.js | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/link.js b/frappe/public/js/frappe/form/controls/link.js index ed355cf8b4..7f671c6e8b 100644 --- a/frappe/public/js/frappe/form/controls/link.js +++ b/frappe/public/js/frappe/form/controls/link.js @@ -458,7 +458,6 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat validate_link_and_fetch(df, options, docname, value) { if (!options) return; - let field_value = ""; const fetch_map = this.fetch_map; const columns_to_fetch = Object.values(fetch_map); @@ -467,16 +466,10 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat return value; } - return frappe.xcall("frappe.client.validate_link", { - doctype: options, - docname: value, - fields: columns_to_fetch, - }).then((response) => { - if (!docname || !columns_to_fetch.length) return response.name; - + function update_dependant_fields(response) { + let field_value = ""; for (const [target_field, source_field] of Object.entries(fetch_map)) { if (value) field_value = response[source_field]; - frappe.model.set_value( df.parent, docname, @@ -485,9 +478,23 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat df.fieldtype, ); } + } - return response.name; - }); + // to avoid unnecessary request + if (value) { + return frappe.xcall("frappe.client.validate_link", { + doctype: options, + docname: value, + fields: columns_to_fetch, + }).then((response) => { + if (!docname || !columns_to_fetch.length) return response.name; + update_dependant_fields(response); + return response.name; + }); + } else { + update_dependant_fields({}); + return value; + } } get fetch_map() { From 5f64dac3c1e35101e6e88926e141f3da548f29a3 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 31 Jan 2022 15:23:08 +0530 Subject: [PATCH 2/6] test: Update UI test for link field --- cypress/integration/control_link.js | 62 ++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/cypress/integration/control_link.js b/cypress/integration/control_link.js index 6d16769b37..13ad52d237 100644 --- a/cypress/integration/control_link.js +++ b/cypress/integration/control_link.js @@ -58,6 +58,23 @@ context('Control Link', () => { cy.get('.frappe-control[data-fieldname=link] input').should('have.value', ''); }); + it("should be possible set empty value explicitly", () => { + get_dialog_with_link().as("dialog"); + + cy.intercept("POST", "/api/method/frappe.client.validate_link").as("validate_link"); + + cy.get(".frappe-control[data-fieldname=link] input") + .type(" ", { delay: 100 }) + .blur(); + cy.wait("@validate_link"); + cy.get(".frappe-control[data-fieldname=link] input").should("have.value", ""); + cy.window() + .its("cur_dialog") + .then((dialog) => { + expect(dialog.get_value("link")).to.equal(''); + }); + }); + it('should route to form on arrow click', () => { get_dialog_with_link().as('dialog'); @@ -78,7 +95,7 @@ context('Control Link', () => { }); }); - it('should fetch valid value', () => { + it('should update dependant fields (via fetch_from)', () => { cy.get('@todos').then(todos => { cy.visit(`/app/todo/${todos[0]}`); cy.intercept('POST', '/api/method/frappe.client.validate_link').as('validate_link'); @@ -89,7 +106,50 @@ context('Control Link', () => { cy.get('.frappe-control[data-fieldname=assigned_by_full_name] .control-value').should( 'contain', 'Administrator' ); + + cy.window() + .its("cur_frm.doc.assigned_by") + .should("eq", "Administrator"); + + // invalid input + cy.get('@input').clear().type('invalid input', {delay: 100}).blur(); + cy.get('.frappe-control[data-fieldname=assigned_by_full_name] .control-value').should( + 'contain', '' + ); + + cy.window() + .its("cur_frm.doc.assigned_by") + .should("eq", null); + + // set valid value again + cy.get('@input').clear().type('Administrator', {delay: 100}).blur(); + cy.wait('@validate_link'); + + cy.window() + .its("cur_frm.doc.assigned_by") + .should("eq", "Administrator"); + + // clear input + cy.get('@input').clear().blur(); + cy.get('.frappe-control[data-fieldname=assigned_by_full_name] .control-value').should( + 'contain', '' + ); + + cy.window() + .its("cur_frm.doc.assigned_by") + .should("eq", ""); }); }); + it("should set default values", () => { + cy.new_form("ToDo"); + cy.window() + .its("cur_frm") + .then(frm => { + frm.set_df_property("assigned_by", "default", "Administrator"); + cy.fill_field("description", "new", "Text Editor"); + cy.findByRole("button", {name: "Save"}).click(); + }); + }); + }); From ebd756c1e54ee2d988d995aa4c7e80373481259b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 1 Feb 2022 13:30:48 +0530 Subject: [PATCH 3/6] test: Update test case for default value in link field --- cypress/integration/control_link.js | 33 ++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/cypress/integration/control_link.js b/cypress/integration/control_link.js index 13ad52d237..bfa70ad338 100644 --- a/cypress/integration/control_link.js +++ b/cypress/integration/control_link.js @@ -142,14 +142,31 @@ context('Control Link', () => { }); it("should set default values", () => { + cy.insert_doc("Property Setter", { + "doctype_or_field": "DocField", + "doc_type": "ToDo", + "field_name": "assigned_by", + "property": "default", + "property_type": "Text", + "value": "Administrator" + }, true); + cy.reload(); cy.new_form("ToDo"); - cy.window() - .its("cur_frm") - .then(frm => { - frm.set_df_property("assigned_by", "default", "Administrator"); - cy.fill_field("description", "new", "Text Editor"); - cy.findByRole("button", {name: "Save"}).click(); - }); + cy.fill_field("description", "new", "Text Editor"); + cy.intercept("POST", "/api/method/frappe.desk.form.save.savedocs").as("save_form"); + cy.findByRole("button", {name: "Save"}).click(); + cy.wait("@save_form"); + cy.get(".frappe-control[data-fieldname=assigned_by_full_name] .control-value").should( + "contain", "Administrator" + ); + // if user clears default value explicitly, system should not reset default again + cy.get_field("assigned_by").clear().blur(); + cy.intercept("POST", "/api/method/frappe.desk.form.save.savedocs").as("save_form"); + cy.findByRole("button", {name: "Save"}).click(); + cy.wait("@save_form"); + cy.get_field("assigned_by").should("have.value", ""); + cy.get(".frappe-control[data-fieldname=assigned_by_full_name] .control-value").should( + "contain", "" + ); }); - }); From 8998d87fc4b96ba00917b5d63dad45320ef3b4ee Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 1 Feb 2022 13:36:17 +0530 Subject: [PATCH 4/6] fix: Update set_nulls code to filters out empty values for link fields - Empty values for link field is invalid and causes issue mentioned here https://github.com/frappe/frappe/pull/15320 --- frappe/public/js/frappe/form/controls/link.js | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/link.js b/frappe/public/js/frappe/form/controls/link.js index 7f671c6e8b..63785e551f 100644 --- a/frappe/public/js/frappe/form/controls/link.js +++ b/frappe/public/js/frappe/form/controls/link.js @@ -374,13 +374,26 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat } set_custom_query(args) { - var set_nulls = function(obj) { - $.each(obj, function(key, value) { - if(value!==undefined) { - obj[key] = value; + const is_valid_value = (value, key) => { + if (value) return true; + // check if empty value is valid + if (this.frm) { + let field = frappe.meta.get_docfield(this.frm.doctype, key); + // empty value link fields is invalid + return !field || !["Link", "Dynamic Link"].includes(field.fieldtype); + } else { + return value !== undefined; + } + } + + const set_nulls = (obj) => { + let new_obj = {} + $.each(obj, (key, value) => { + if (is_valid_value(value, key)) { + new_obj[key] = value; } }); - return obj; + return new_obj; }; if(this.get_query || this.df.get_query) { var get_query = this.get_query || this.df.get_query; From e18f8d6e1075cfc35a2a304a63f2572b54166f8b Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 1 Feb 2022 13:37:46 +0530 Subject: [PATCH 5/6] chore: Remove duplicate command --- cypress/support/commands.js | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 758b3cde2b..4f273af21f 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -110,34 +110,6 @@ Cypress.Commands.add('get_doc', (doctype, name) => { }); }); -Cypress.Commands.add('insert_doc', (doctype, args, ignore_duplicate) => { - return cy - .window() - .its('frappe.csrf_token') - .then(csrf_token => { - return cy - .request({ - method: 'POST', - url: `/api/resource/${doctype}`, - body: args, - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - 'X-Frappe-CSRF-Token': csrf_token - }, - failOnStatusCode: !ignore_duplicate - }) - .then(res => { - let status_codes = [200]; - if (ignore_duplicate) { - status_codes.push(409); - } - expect(res.status).to.be.oneOf(status_codes); - return res.body; - }); - }); -}); - Cypress.Commands.add('remove_doc', (doctype, name) => { return cy .window() From d2436b88c5c94733c36af03158bd93249ace2b50 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Tue, 1 Feb 2022 14:37:39 +0530 Subject: [PATCH 6/6] fix: Update filter whilte removing undefined --- frappe/public/js/frappe/form/controls/link.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/link.js b/frappe/public/js/frappe/form/controls/link.js index 63785e551f..9f02485a9e 100644 --- a/frappe/public/js/frappe/form/controls/link.js +++ b/frappe/public/js/frappe/form/controls/link.js @@ -387,13 +387,12 @@ frappe.ui.form.ControlLink = class ControlLink extends frappe.ui.form.ControlDat } const set_nulls = (obj) => { - let new_obj = {} $.each(obj, (key, value) => { - if (is_valid_value(value, key)) { - new_obj[key] = value; + if (!is_valid_value(value, key)) { + delete obj[key]; } }); - return new_obj; + return obj; }; if(this.get_query || this.df.get_query) { var get_query = this.get_query || this.df.get_query;