From 0482530ffd3813b9cfb2b7ecb40d1c5b2ed181ca Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Fri, 3 Feb 2023 20:04:11 +0530 Subject: [PATCH 01/61] fix: do not allow restricted fieldnames for custom fields --- .../custom/doctype/custom_field/custom_field.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index 758d9c1e64..8953153be6 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -18,6 +18,18 @@ class CustomField(Document): self.name = self.dt + "-" + self.fieldname def set_fieldname(self): + restricted = ( + "name", + "parent", + "creation", + "modified", + "modified_by", + "parentfield", + "parenttype", + "file_list", + "flags", + "docstatus", + ) if not self.fieldname: label = self.label if not label: @@ -34,6 +46,9 @@ class CustomField(Document): # fieldnames should be lowercase self.fieldname = self.fieldname.lower() + if self.fieldname in restricted: + self.fieldname = self.fieldname + "1" + def before_insert(self): self.set_fieldname() From d6a41cd2722d1776da774ab2d7521233b5663116 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 9 Feb 2023 15:49:47 +0530 Subject: [PATCH 02/61] feat: Before/After Request Hooks --- frappe/app.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/app.py b/frappe/app.py index 2fe9991c4c..3acae39fb7 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -119,6 +119,9 @@ def init_request(request): if request.method != "OPTIONS": frappe.local.http_request = HTTPRequest() + for before_request_task in frappe.get_hooks("before_request"): + frappe.call(before_request_task) + def setup_read_only_mode(): """During maintenance_mode reads to DB can still be performed to reduce downtime. This @@ -318,7 +321,10 @@ def handle_exception(e): return response -def after_request(rollback): +def after_request(rollback: bool) -> bool: + for after_request_task in frappe.get_hooks("after_request"): + frappe.call(after_request_task) + # if HTTP method would change server state, commit if necessary if ( frappe.db From 34731d1e9e9fa64fbd64f9266b5ac1b8923f7b02 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 11:37:36 +0530 Subject: [PATCH 03/61] feat: Befor/After Job Hooks --- frappe/utils/background_jobs.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index 040a57cc11..3c8a731369 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -168,6 +168,10 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, method_name = cstr(method.__name__) frappe.monitor.start("job", method_name, kwargs) + + for before_job_task in frappe.get_hooks("before_job"): + frappe.call(before_job_task, method=method_name, kwargs=kwargs) + try: method(**kwargs) @@ -202,6 +206,9 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, frappe.db.commit() finally: + for after_job_task in frappe.get_hooks("after_job"): + frappe.call(after_job_task, method=method_name, kwargs=kwargs) + # background job hygiene: release file locks if unreleased # if this breaks something, move it to failed jobs alone - gavin@frappe.io for doc in frappe.local.locked_documents: From 0d547cf71c9057a2d93c5c26902c9403a3b718fb Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 12:55:00 +0530 Subject: [PATCH 04/61] test: Add tests for job sanity + hooks --- frappe/tests/test_background_jobs.py | 83 +++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/frappe/tests/test_background_jobs.py b/frappe/tests/test_background_jobs.py index 1b6680eb9b..72660128cf 100644 --- a/frappe/tests/test_background_jobs.py +++ b/frappe/tests/test_background_jobs.py @@ -1,11 +1,19 @@ import time +from contextlib import contextmanager +from unittest.mock import patch from rq import Queue import frappe from frappe.core.doctype.rq_job.rq_job import remove_failed_jobs from frappe.tests.utils import FrappeTestCase -from frappe.utils.background_jobs import generate_qname, get_redis_conn +from frappe.utils.background_jobs import ( + RQ_JOB_FAILURE_TTL, + RQ_RESULTS_TTL, + execute_job, + generate_qname, + get_redis_conn, +) class TestBackgroundJobs(FrappeTestCase): @@ -44,6 +52,79 @@ class TestBackgroundJobs(FrappeTestCase): # lesser is earlier self.assertTrue(high_priority_job.get_position() < low_priority_job.get_position()) + def test_enqueue_call(self): + with patch.object(Queue, "enqueue_call") as mock_enqueue_call: + frappe.enqueue( + "frappe.handler.ping", + queue="short", + timeout=300, + kwargs={"site": frappe.local.site}, + ) + + mock_enqueue_call.assert_called_once_with( + execute_job, + on_success=None, + on_failure=None, + timeout=300, + kwargs={ + "site": frappe.local.site, + "user": "Administrator", + "method": "frappe.handler.ping", + "event": None, + "job_name": "frappe.handler.ping", + "is_async": True, + "kwargs": {"kwargs": {"site": frappe.local.site}}, + }, + at_front=False, + failure_ttl=RQ_JOB_FAILURE_TTL, + result_ttl=RQ_RESULTS_TTL, + ) + + def test_job_hooks(self): + self.addCleanup(lambda: _test_JOB_HOOK.clear()) + with freeze_local() as locals, frappe.init_site(locals.site), patch( + "frappe.get_hooks", patch_job_hooks + ): + frappe.connect() + self.assertIsNone(_test_JOB_HOOK.get("before_job")) + r = execute_job( + site=frappe.local.site, + user="Administrator", + method="frappe.handler.ping", + event=None, + job_name="frappe.handler.ping", + is_async=True, + kwargs={}, + ) + self.assertEqual(r, "pong") + self.assertLess(_test_JOB_HOOK.get("before_job"), _test_JOB_HOOK.get("after_job")) + def fail_function(): return 1 / 0 + + +_test_JOB_HOOK = {} + + +def before_job(*args, **kwargs): + _test_JOB_HOOK["before_job"] = time.time() + + +def after_job(*args, **kwargs): + _test_JOB_HOOK["after_job"] = time.time() + + +@contextmanager +def freeze_local(): + locals = frappe.local + frappe.local = frappe.Local() + yield locals + frappe.local = locals + + +def patch_job_hooks(event: str): + return { + "before_job": ["frappe.tests.test_background_jobs.before_job"], + "after_job": ["frappe.tests.test_background_jobs.after_job"], + }[event] From 55b31fcdfae7cabfa55b7a55bea29228118295e1 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 13:22:27 +0530 Subject: [PATCH 05/61] test: Added test for before/after request hook --- frappe/tests/test_api.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/frappe/tests/test_api.py b/frappe/tests/test_api.py index 1085f4c39e..c345d8fbcf 100644 --- a/frappe/tests/test_api.py +++ b/frappe/tests/test_api.py @@ -2,6 +2,7 @@ import sys from contextlib import contextmanager from random import choice from threading import Thread +from time import time from unittest.mock import patch import requests @@ -306,3 +307,36 @@ class TestReadOnlyMode(FrappeAPITestCase): response = self.post(self.REQ_PATH, {"description": frappe.mock("paragraph"), "sid": self.sid}) self.assertEqual(response.status_code, 503) self.assertEqual(response.json["exc_type"], "InReadOnlyMode") + + +class TestWSGIApp(FrappeAPITestCase): + def test_request_hooks(self): + self.addCleanup(lambda: _test_REQ_HOOK.clear()) + get_hooks = frappe.get_hooks + + def patch_request_hooks(event: str, *args, **kwargs): + patched_hooks = { + "before_request": ["frappe.tests.test_api.before_request"], + "after_request": ["frappe.tests.test_api.after_request"], + } + if event not in patched_hooks: + return get_hooks(event, *args, **kwargs) + return patched_hooks[event] + + with patch("frappe.get_hooks", patch_request_hooks): + self.assertIsNone(_test_REQ_HOOK.get("before_request")) + self.assertIsNone(_test_REQ_HOOK.get("after_request")) + res = self.get("/api/method/ping") + self.assertEqual(res.json, {"message": "pong"}) + self.assertLess(_test_REQ_HOOK.get("before_request"), _test_REQ_HOOK.get("after_request")) + + +_test_REQ_HOOK = {} + + +def before_request(*args, **kwargs): + _test_REQ_HOOK["before_request"] = time() + + +def after_request(*args, **kwargs): + _test_REQ_HOOK["after_request"] = time() From 83f3cf1991b4c26ba0fbd5e937670e1cfb9e3afb Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 13:23:24 +0530 Subject: [PATCH 06/61] fix(background_jobs): Pass retval in execute_job --- frappe/utils/background_jobs.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index 3c8a731369..2ca34141a9 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -153,6 +153,7 @@ def run_doc_method(doctype, name, doc_method, **kwargs): def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, retry=0): """Executes job in a worker, performs commit/rollback and logs if there is any error""" + retval = None if is_async: frappe.connect(site) if os.environ.get("CI"): @@ -173,7 +174,7 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, frappe.call(before_job_task, method=method_name, kwargs=kwargs) try: - method(**kwargs) + retval = method(**kwargs) except (frappe.db.InternalError, frappe.RetryBackgroundJobError) as e: frappe.db.rollback() @@ -204,6 +205,7 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, else: frappe.db.commit() + return retval finally: for after_job_task in frappe.get_hooks("after_job"): From ea62156a6d028ea764ff8a9aabe7406313e1ab89 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 13:28:50 +0530 Subject: [PATCH 07/61] fix: Add request, job events in hooks boilerplate --- frappe/utils/boilerplate.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py index ee75c672a8..1cd57f4695 100644 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -458,6 +458,15 @@ app_license = "{app_license}" # ignore_links_on_delete = ["Communication", "ToDo"] +# Request Events +# ---------------- +# before_request = ["{app_name}.utils.before_request"] +# after_request = ["{app_name}.utils.after_request"] + +# Job Events +# ---------- +# before_job = ["{app_name}.utils.before_job"] +# after_job = ["{app_name}.utils.after_job"] # User Data Protection # -------------------- From 2ada98fbdb1d93e7747d15977769b5cc8a1f030c Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 13:31:46 +0530 Subject: [PATCH 08/61] fix: Pass result of job to after_job hooks --- frappe/utils/background_jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index 2ca34141a9..a106c3beba 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -209,7 +209,7 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, finally: for after_job_task in frappe.get_hooks("after_job"): - frappe.call(after_job_task, method=method_name, kwargs=kwargs) + frappe.call(after_job_task, method=method_name, kwargs=kwargs, result=retval) # background job hygiene: release file locks if unreleased # if this breaks something, move it to failed jobs alone - gavin@frappe.io From 6d70b5e93440272b241c70b0bd6e68593be5536a Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 15 Feb 2023 14:52:08 +0530 Subject: [PATCH 09/61] fix(app): Move after_request hook inside finally block Also, rename after_request fn to sync_database to better match functionality --- frappe/app.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/frappe/app.py b/frappe/app.py index ff2b95ecb7..0d80d5dc7c 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -74,12 +74,15 @@ def application(request: Request): response = handle_exception(e) else: - rollback = after_request(rollback) + rollback = sync_database(rollback) finally: if request.method in UNSAFE_HTTP_METHODS and frappe.db and rollback: frappe.db.rollback() + for after_request_task in frappe.get_hooks("after_request"): + frappe.call(after_request_task) + frappe.rate_limiter.update() frappe.monitor.stop(response) frappe.recorder.dump() @@ -321,10 +324,7 @@ def handle_exception(e): return response -def after_request(rollback: bool) -> bool: - for after_request_task in frappe.get_hooks("after_request"): - frappe.call(after_request_task) - +def sync_database(rollback: bool) -> bool: # if HTTP method would change server state, commit if necessary if ( frappe.db @@ -338,9 +338,8 @@ def after_request(rollback: bool) -> bool: rollback = False # update session - if getattr(frappe.local, "session_obj", None): - updated_in_db = frappe.local.session_obj.update() - if updated_in_db: + if session := getattr(frappe.local, "session_obj", None): + if session.update(): frappe.db.commit() rollback = False From fe26c542b77697a2a227ec1559f6536a06ba2fc7 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 15 Feb 2023 15:30:02 +0530 Subject: [PATCH 10/61] refactor: Move before/after tasks as hooks Moved before/after tasks in Requests as hooks for: - monitor - rate_limiter - recorder Moved before/after tasks in Jobs as hooks for: - monitor - releasing document locks --- frappe/app.py | 10 ++-------- frappe/hooks.py | 17 +++++++++++++++++ frappe/utils/background_jobs.py | 10 +--------- frappe/utils/file_lock.py | 8 +++++++- 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/frappe/app.py b/frappe/app.py index 0d80d5dc7c..01a5b5f218 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -35,15 +35,13 @@ _sites_path = os.environ.get("SITES_PATH", ".") @Request.application def application(request: Request): response = None + e = None try: rollback = True init_request(request) - frappe.recorder.record() - frappe.monitor.start() - frappe.rate_limiter.apply() frappe.api.validate_auth() if request.method == "OPTIONS": @@ -81,11 +79,7 @@ def application(request: Request): frappe.db.rollback() for after_request_task in frappe.get_hooks("after_request"): - frappe.call(after_request_task) - - frappe.rate_limiter.update() - frappe.monitor.stop(response) - frappe.recorder.dump() + frappe.call(after_request_task, response=response, request=request, exception=e) log_request(request, response) process_response(response) diff --git a/frappe/hooks.py b/frappe/hooks.py index aa6d6f00c7..317439c358 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -393,3 +393,20 @@ ignore_links_on_delete = [ "Integration Request", "Unhandled Email", ] + +# Request Hooks +before_request = [ + "frappe.recorder.record", + "frappe.monitor.start", + "frappe.rate_limiter.apply", +] +after_request = ["frappe.rate_limiter.update", "frappe.monitor.stop", "frappe.recorder.dump"] + +# Background Job Hooks +before_job = [ + "frappe.monitor.start", +] +after_job = [ + "frappe.monitor.stop", + "frappe.utils.file_lock.release_document_locks", +] diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index a106c3beba..fed822700c 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -168,10 +168,8 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, else: method_name = cstr(method.__name__) - frappe.monitor.start("job", method_name, kwargs) - for before_job_task in frappe.get_hooks("before_job"): - frappe.call(before_job_task, method=method_name, kwargs=kwargs) + frappe.call(before_job_task, method=method_name, kwargs=kwargs, transaction_type="job") try: retval = method(**kwargs) @@ -211,12 +209,6 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, for after_job_task in frappe.get_hooks("after_job"): frappe.call(after_job_task, method=method_name, kwargs=kwargs, result=retval) - # background job hygiene: release file locks if unreleased - # if this breaks something, move it to failed jobs alone - gavin@frappe.io - for doc in frappe.local.locked_documents: - doc.unlock() - - frappe.monitor.stop() if is_async: frappe.destroy() diff --git a/frappe/utils/file_lock.py b/frappe/utils/file_lock.py index 5be89c42f6..60d8baec5d 100644 --- a/frappe/utils/file_lock.py +++ b/frappe/utils/file_lock.py @@ -11,7 +11,7 @@ Use `frappe.utils.synchroniztion.filelock` for process synchroniztion. import os from time import time -from frappe import _ +import frappe from frappe.utils import get_site_path, touch_file LOCKS_DIR = "locks" @@ -62,3 +62,9 @@ def get_lock_path(name): name = name.lower() lock_path = get_site_path(LOCKS_DIR, name + ".lock") return lock_path + + +def release_document_locks(): + """Unlocks all documents that were locked by the current context.""" + for doc in frappe.local.locked_documents: + doc.unlock() From 39761d3d7e8a5723c39edf87a241714d3ada3421 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 15 Feb 2023 16:55:28 +0530 Subject: [PATCH 11/61] feat(Calendar): Add a new option `convertToUserTz` to address timezone inconsistencies --- frappe/public/js/frappe/views/calendar/calendar.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/views/calendar/calendar.js b/frappe/public/js/frappe/views/calendar/calendar.js index cba7886a93..4a22457dd7 100644 --- a/frappe/public/js/frappe/views/calendar/calendar.js +++ b/frappe/public/js/frappe/views/calendar/calendar.js @@ -120,6 +120,7 @@ frappe.views.Calendar = class Calendar { start: "start", end: "end", allDay: "all_day", + convertToUserTz: "convert_to_user_tz", }; this.color_map = { danger: "red", @@ -372,10 +373,13 @@ frappe.views.Calendar = class Calendar { }); if (!me.field_map.allDay) d.allDay = 1; + if (!me.field_map.convertToUserTz) d.convertToUserTz = 1; // convert to user tz - d.start = frappe.datetime.convert_to_user_tz(d.start); - d.end = frappe.datetime.convert_to_user_tz(d.end); + if (d.convertToUserTz) { + d.start = frappe.datetime.convert_to_user_tz(d.start); + d.end = frappe.datetime.convert_to_user_tz(d.end); + } // show event on single day if start or end date is invalid if (!frappe.datetime.validate(d.start) && d.end) { From 80a49329831233a4472e0bd8d868b875c3a10d68 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Thu, 16 Feb 2023 18:42:24 +0530 Subject: [PATCH 12/61] fix: ask before changing restricted fieldnames --- .../doctype/customize_form/customize_form.js | 61 ++++++++++++++----- 1 file changed, 46 insertions(+), 15 deletions(-) diff --git a/frappe/custom/doctype/customize_form/customize_form.js b/frappe/custom/doctype/customize_form/customize_form.js index fed8505147..73981eaf28 100644 --- a/frappe/custom/doctype/customize_form/customize_form.js +++ b/frappe/custom/doctype/customize_form/customize_form.js @@ -310,22 +310,53 @@ frappe.ui.form.on("DocType State", { }, }); -frappe.customize_form.set_primary_action = function (frm) { - frm.page.set_primary_action(__("Update"), function () { - if (frm.doc.doc_type) { - return frm.call({ - doc: frm.doc, - freeze: true, - btn: frm.page.btn_primary, - method: "save_customization", - callback: function (r) { - if (!r.exc) { - frappe.customize_form.clear_locals_and_refresh(frm); - frm.script_manager.trigger("doc_type"); - } - }, - }); +frappe.customize_form.validate_fieldnames = async function (frm) { + for (let i = 0; i < frm.doc.fields.length; i++) { + let field = frm.doc.fields[i]; + + let fieldname = field.label && frappe.model.scrub(field.label).toLowerCase(); + if ( + field.label && + !field.fieldname && + in_list(frappe.model.restricted_fields, fieldname) + ) { + let message = __( + "For field {0} in row {1}, fieldname {2} is restricted it will be renamed as {2}1. Do you want to continue?", + [field.label, field.idx, fieldname] + ); + await pause_to_confirm(message); } + } + + function pause_to_confirm(message) { + return new Promise((resolve) => { + frappe.confirm(message, () => resolve()); + }); + } +}; + +frappe.customize_form.save_customization = function (frm) { + if (frm.doc.doc_type) { + return frm.call({ + doc: frm.doc, + freeze: true, + freeze_message: __("Updating Customization..."), + btn: frm.page.btn_primary, + method: "save_customization", + callback: function (r) { + if (!r.exc) { + frappe.customize_form.clear_locals_and_refresh(frm); + frm.script_manager.trigger("doc_type"); + } + }, + }); + } +}; + +frappe.customize_form.set_primary_action = function (frm) { + frm.page.set_primary_action(__("Update"), async () => { + await this.validate_fieldnames(frm); + this.save_customization(frm); }); }; From 908545241bde7fe0561ac8b196f9c1e440e46a31 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Thu, 16 Feb 2023 19:31:07 +0530 Subject: [PATCH 13/61] fix: enable update button if fieldname change is rejected --- frappe/custom/doctype/customize_form/customize_form.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/custom/doctype/customize_form/customize_form.js b/frappe/custom/doctype/customize_form/customize_form.js index a0be0f3d63..d1ee27faba 100644 --- a/frappe/custom/doctype/customize_form/customize_form.js +++ b/frappe/custom/doctype/customize_form/customize_form.js @@ -334,7 +334,13 @@ frappe.customize_form.validate_fieldnames = async function (frm) { function pause_to_confirm(message) { return new Promise((resolve) => { - frappe.confirm(message, () => resolve()); + frappe.confirm( + message, + () => resolve(), + () => { + frm.page.btn_primary.prop("disabled", false); + } + ); }); } }; From 110204e2df335ad7e04884b709a7f290deb683fb Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Thu, 16 Feb 2023 20:05:36 +0530 Subject: [PATCH 14/61] feat: publish button in blog post form --- frappe/website/doctype/blog_post/blog_post.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frappe/website/doctype/blog_post/blog_post.js b/frappe/website/doctype/blog_post/blog_post.js index 0266587f2e..5f7268d074 100644 --- a/frappe/website/doctype/blog_post/blog_post.js +++ b/frappe/website/doctype/blog_post/blog_post.js @@ -7,6 +7,8 @@ frappe.ui.form.on("Blog Post", { frm.set_df_property("hide_cta", "hidden", !value); }); + frm.trigger("add_publish_button"); + generate_google_search_preview(frm); }, title: function (frm) { @@ -30,6 +32,12 @@ frappe.ui.form.on("Blog Post", { }); } }, + add_publish_button(frm) { + frm.add_custom_button(frm.doc.published ? __("Unpublish") : __("Publish"), () => { + frm.set_value("published", !frm.doc.published); + frm.save(); + }); + }, }); function generate_google_search_preview(frm) { From 5d0bd512e10bd3b81c241c0f19d851ec5f62aca1 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 17 Feb 2023 11:24:01 +0530 Subject: [PATCH 15/61] fix(after_hook): Don't pass exception object to hook * it can fetch most relevant details via response object * Exceptions not supported by Frappe's WSGI (unsupported HTTP methods) may not be accessible to the after_request hooks - but the lack of active response may be an indicator / and peeking in the request --- frappe/app.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/app.py b/frappe/app.py index 01a5b5f218..6b16464d2c 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -35,7 +35,6 @@ _sites_path = os.environ.get("SITES_PATH", ".") @Request.application def application(request: Request): response = None - e = None try: rollback = True @@ -79,7 +78,7 @@ def application(request: Request): frappe.db.rollback() for after_request_task in frappe.get_hooks("after_request"): - frappe.call(after_request_task, response=response, request=request, exception=e) + frappe.call(after_request_task, response=response, request=request) log_request(request, response) process_response(response) From 24d76375382db510b756eedc268b1a73f1921aa7 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Fri, 17 Feb 2023 11:31:53 +0530 Subject: [PATCH 16/61] fix: hide published checkbox --- frappe/website/doctype/blog_post/blog_post.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/website/doctype/blog_post/blog_post.json b/frappe/website/doctype/blog_post/blog_post.json index 8b5ab54ba8..d41b344464 100644 --- a/frappe/website/doctype/blog_post/blog_post.json +++ b/frappe/website/doctype/blog_post/blog_post.json @@ -53,6 +53,7 @@ "default": "0", "fieldname": "published", "fieldtype": "Check", + "hidden": 1, "label": "Published" }, { @@ -215,7 +216,7 @@ "is_published_field": "published", "links": [], "make_attachments_public": 1, - "modified": "2022-10-18 10:09:10.550734", + "modified": "2023-02-17 11:31:32.223524", "modified_by": "Administrator", "module": "Website", "name": "Blog Post", From 649c211b9bce0fdae7399dc1c0a59dcf8cb4621e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 20 Feb 2023 10:13:49 +0530 Subject: [PATCH 17/61] fix(UX): show message when form is read only (#20077) [skip ci] --- frappe/public/js/frappe/form/form.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 001279f394..d1fe443f7a 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -406,7 +406,10 @@ frappe.ui.form.Form = class FrappeForm { // read only (workflow) this.read_only = frappe.workflow.is_read_only(this.doctype, this.docname); - if (this.read_only) this.set_read_only(true); + if (this.read_only) { + this.set_read_only(true); + frappe.show_alert(__("This form is not editable due to a Workflow.")); + } // check if doctype is already open if (!this.opendocs[this.docname]) { From 7afc46401b7c23620982745181ab659b636df1a3 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Mon, 20 Feb 2023 11:21:24 +0530 Subject: [PATCH 18/61] chore: changed freeze message --- frappe/custom/doctype/customize_form/customize_form.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/custom/doctype/customize_form/customize_form.js b/frappe/custom/doctype/customize_form/customize_form.js index d1ee27faba..4ab693b415 100644 --- a/frappe/custom/doctype/customize_form/customize_form.js +++ b/frappe/custom/doctype/customize_form/customize_form.js @@ -350,7 +350,7 @@ frappe.customize_form.save_customization = function (frm) { return frm.call({ doc: frm.doc, freeze: true, - freeze_message: __("Updating Customization..."), + freeze_message: __("Saving Customization..."), btn: frm.page.btn_primary, method: "save_customization", callback: function (r) { From 89d63ea82b0fbd2d744aa890d1d49e0ad8804ffa Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 20 Feb 2023 12:18:37 +0530 Subject: [PATCH 19/61] fix: false positive attr check while applying permlevel (#20069) * fix: false positive attr check while applying permlevel * Revert "fix: false positive attr check while applying permlevel" This reverts commit 9114788590ce12be977df847c13b00e3bf72ac2a. * fix: ignore AttributeError while trying to pop low permlevel fields --------- Co-authored-by: Ankush Menat --- frappe/model/document.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index 8a99676b60..7fcb9ac335 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -676,7 +676,11 @@ class Document(BaseDocument): for df in self.meta.fields: if df.permlevel and hasattr(self, df.fieldname) and df.permlevel not in has_access_to: - delattr(self, df.fieldname) + try: + delattr(self, df.fieldname) + except AttributeError: + # hasattr might return True for class attribute which can't be delattr-ed. + continue for table_field in self.meta.get_table_fields(): for df in frappe.get_meta(table_field.options).fields or []: From c94d3ccc16f4b674af2b0aabe446fd6aa1406aca Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 20 Feb 2023 12:51:24 +0530 Subject: [PATCH 20/61] fix: Hide perm level fields for Section, Column and Tab Breaks (#20084) --- frappe/core/doctype/docfield/docfield.json | 3 ++- .../doctype/customize_form_field/customize_form_field.json | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/docfield/docfield.json b/frappe/core/doctype/docfield/docfield.json index 0d8d7ea671..90b1c6cb77 100644 --- a/frappe/core/doctype/docfield/docfield.json +++ b/frappe/core/doctype/docfield/docfield.json @@ -304,6 +304,7 @@ }, { "default": "0", + "depends_on": "eval:!in_list(['Section Break', 'Column Break', 'Tab Break'], doc.fieldtype)", "fieldname": "permlevel", "fieldtype": "Int", "label": "Perm Level", @@ -555,7 +556,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2023-01-11 20:46:43.164926", + "modified": "2023-02-20 12:07:29.552523", "modified_by": "Administrator", "module": "Core", "name": "DocField", diff --git a/frappe/custom/doctype/customize_form_field/customize_form_field.json b/frappe/custom/doctype/customize_form_field/customize_form_field.json index ad0d600a0b..d8da44101b 100644 --- a/frappe/custom/doctype/customize_form_field/customize_form_field.json +++ b/frappe/custom/doctype/customize_form_field/customize_form_field.json @@ -212,6 +212,7 @@ }, { "default": "0", + "depends_on": "eval:!in_list(['Section Break', 'Column Break', 'Tab Break'], doc.fieldtype)", "fieldname": "permlevel", "fieldtype": "Int", "in_list_view": 1, @@ -467,7 +468,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2022-11-30 14:25:50.649449", + "modified": "2023-02-20 12:07:40.242470", "modified_by": "Administrator", "module": "Custom", "name": "Customize Form Field", From b55bbd0a8c3dde53565d6440a0d848f65aba056d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 20 Feb 2023 13:07:32 +0530 Subject: [PATCH 21/61] fix(UX): Sort case-insensitive where it makes sense (#20088) --- frappe/core/page/permission_manager/permission_manager.py | 4 ++-- frappe/desk/doctype/tag/tag.py | 2 +- frappe/desk/search.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/core/page/permission_manager/permission_manager.py b/frappe/core/page/permission_manager/permission_manager.py index 45c1e44fa1..5ed3014778 100644 --- a/frappe/core/page/permission_manager/permission_manager.py +++ b/frappe/core/page/permission_manager/permission_manager.py @@ -62,8 +62,8 @@ def get_roles_and_doctypes(): roles_list = [{"label": _(d.get("name")), "value": d.get("name")} for d in roles] return { - "doctypes": sorted(doctypes_list, key=lambda d: d["label"]), - "roles": sorted(roles_list, key=lambda d: d["label"]), + "doctypes": sorted(doctypes_list, key=lambda d: d["label"].casefold()), + "roles": sorted(roles_list, key=lambda d: d["label"].casefold()), } diff --git a/frappe/desk/doctype/tag/tag.py b/frappe/desk/doctype/tag/tag.py index 84239fae6d..c5fe6407b7 100644 --- a/frappe/desk/doctype/tag/tag.py +++ b/frappe/desk/doctype/tag/tag.py @@ -59,7 +59,7 @@ def get_tags(doctype, txt): tag = frappe.get_list("Tag", filters=[["name", "like", f"%{txt}%"]]) tags = [t.name for t in tag] - return sorted(filter(lambda t: t and txt.lower() in t.lower(), list(set(tags)))) + return sorted(filter(lambda t: t and txt.casefold() in t.casefold(), list(set(tags)))) class DocTags: diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 2af9b575be..ee63f67423 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -282,7 +282,7 @@ def scrub_custom_query(query, key, txt): def relevance_sorter(key, query, as_dict): value = _(key.name if as_dict else key[0]) - return (cstr(value).lower().startswith(query.lower()) is not True, value) + return (cstr(value).casefold().startswith(query.casefold()) is not True, value) def validate_and_sanitize_search_inputs(fn): From 68df7d621f8489ddb4ec6c4a6ebe4cae7dd07389 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 20 Feb 2023 13:13:35 +0530 Subject: [PATCH 22/61] docs: document_naming_settings field label [skip ci] --- .../document_naming_settings.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/document_naming_settings/document_naming_settings.json b/frappe/core/doctype/document_naming_settings/document_naming_settings.json index 4c86b2ec1d..9a12f3f77e 100644 --- a/frappe/core/doctype/document_naming_settings/document_naming_settings.json +++ b/frappe/core/doctype/document_naming_settings/document_naming_settings.json @@ -81,10 +81,10 @@ }, { "depends_on": "transaction_type", - "description": "Generate 3 preview of names generate by any valid series.", + "description": "Get a preview of generated names with a series.", "fieldname": "try_naming_series", "fieldtype": "Data", - "label": "Try a naming Series" + "label": "Try a Naming Series" }, { "fieldname": "transaction_type", @@ -111,7 +111,7 @@ "icon": "fa fa-sort-by-order", "issingle": 1, "links": [], - "modified": "2022-05-30 23:51:36.136535", + "modified": "2023-02-20 13:11:56.662100", "modified_by": "Administrator", "module": "Core", "name": "Document Naming Settings", @@ -130,4 +130,4 @@ "sort_field": "modified", "sort_order": "DESC", "states": [] -} +} \ No newline at end of file From 7f73906528123c0ead88b21c765270f817b25749 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 20 Feb 2023 13:19:20 +0530 Subject: [PATCH 23/61] fix: switching from option store syntax to setup store syntax --- frappe/public/js/form_builder/store.js | 491 +++++++++++++------------ 1 file changed, 256 insertions(+), 235 deletions(-) diff --git a/frappe/public/js/form_builder/store.js b/frappe/public/js/form_builder/store.js index 246956dc94..c337e471d6 100644 --- a/frappe/public/js/form_builder/store.js +++ b/frappe/public/js/form_builder/store.js @@ -1,270 +1,291 @@ import { defineStore } from "pinia"; import { create_layout, scrub_field_names } from "./utils"; -import { nextTick } from "vue"; +import { computed, nextTick, ref } from "vue"; -export const useStore = defineStore("form-builder-store", { - state: () => ({ - doctype: "", - doc: null, - docfields: [], - custom_docfields: [], - layout: {}, - active_tab: "", - selected_field: null, - dirty: false, - read_only: false, - is_customize_form: false, - preview: false, - drag: false, - }), - getters: { - get_animation: () => { - return "cubic-bezier(0.34, 1.56, 0.64, 1)"; - }, - selected: (state) => { - return (name) => state.selected_field?.name == name; - }, - get_docfields: (state) => { - return state.is_customize_form ? state.custom_docfields : state.docfields; - }, - get_df: (state) => { - return (fieldtype, fieldname = "", label = "") => { - let docfield = state.is_customize_form ? "Customize Form Field" : "DocField"; - let df = frappe.model.get_new_doc(docfield); - df.name = frappe.utils.get_random(8); - df.fieldtype = fieldtype; - df.fieldname = fieldname; - df.label = label; - state.is_customize_form && (df.is_custom_field = 1); - return df; - }; - }, - has_standard_field: (state) => { - return (field) => { - if (!state.is_customize_form) return; - if (!field.df.is_custom_field) return true; +export const useStore = defineStore("form-builder-store", () => { + let doctype = ref(""); + let doc = ref(null); + let docfields = ref([]); + let custom_docfields = ref([]); + let layout = ref({}); + let active_tab = ref(""); + let selected_field = ref(null); + let dirty = ref(false); + let read_only = ref(false); + let is_customize_form = ref(false); + let preview = ref(false); + let drag = ref(false); + let get_animation = ref("cubic-bezier(0.34, 1.56, 0.64, 1)"); - let children = { - "Tab Break": "sections", - "Section Break": "columns", - "Column Break": "fields", - }[field.df.fieldtype]; + // Getters + let get_docfields = computed(() => { + return is_customize_form.value ? custom_docfields.value : docfields.value; + }); - if (!children) return false; + let current_tab = computed(() => { + return layout.value.tabs.find((tab) => tab.df.name == active_tab.value); + }); - return field[children].some((child) => { - if (!child.df.is_custom_field) return true; - return state.has_standard_field(child); - }); - }; - }, - current_tab: (state) => { - return state.layout.tabs.find((tab) => tab.df.name == state.active_tab); - }, - }, - actions: { - async fetch() { - await frappe.model.clear_doc("DocType", this.doctype); - await frappe.model.with_doctype(this.doctype); + // Actions + function selected(name) { + return selected_field.value?.name == name; + } - if (this.is_customize_form) { - await frappe.model.with_doc("Customize Form"); - let doc = frappe.get_doc("Customize Form"); - doc.doc_type = this.doctype; - let r = await frappe.call({ method: "fetch_to_customize", doc }); - this.doc = r.docs[0]; + function get_df(fieldtype, fieldname = "", label = "") { + let docfield = is_customize_form.value ? "Customize Form Field" : "DocField"; + let df = frappe.model.get_new_doc(docfield); + df.name = frappe.utils.get_random(8); + df.fieldtype = fieldtype; + df.fieldname = fieldname; + df.label = label; + is_customize_form.value && (df.is_custom_field = 1); + return df; + } + + function has_standard_field(field) { + if (!is_customize_form.value) return; + if (!field.df.is_custom_field) return true; + + let children = { + "Tab Break": "sections", + "Section Break": "columns", + "Column Break": "fields", + }[field.df.fieldtype]; + + if (!children) return false; + + return field[children].some((child) => { + if (!child.df.is_custom_field) return true; + return has_standard_field(child); + }); + } + + async function fetch() { + await frappe.model.clear_doc("DocType", doctype.value); + await frappe.model.with_doctype(doctype.value); + + if (is_customize_form.value) { + await frappe.model.with_doc("Customize Form"); + let doc = frappe.get_doc("Customize Form"); + doc.doc_type = doctype.value; + let r = await frappe.call({ method: "fetch_to_customize", doc }); + doc.value = r.docs[0]; + } else { + doc.value = await frappe.db.get_doc("DocType", doctype.value); + } + + if (!get_docfields.value.length) { + let docfield = is_customize_form.value ? "Customize Form Field" : "DocField"; + await frappe.model.with_doctype(docfield); + let df = frappe.get_meta(docfield).fields; + if (is_customize_form.value) { + custom_docfields.value = df; } else { - this.doc = await frappe.db.get_doc("DocType", this.doctype); + docfields.value = df; + } + } + + layout.value = get_layout(); + active_tab.value = layout.value.tabs[0].df.name; + selected_field.value = null; + + nextTick(() => { + dirty.value = false; + read_only.value = + !is_customize_form.value && !frappe.boot.developer_mode && !doc.value.custom; + preview.value = false; + }); + } + + function reset_changes() { + fetch(); + } + + function validate_fields(fields, is_table) { + fields = scrub_field_names(fields); + + let not_allowed_in_list_view = ["Attach Image", ...frappe.model.no_value_type]; + if (is_table) { + not_allowed_in_list_view = not_allowed_in_list_view.filter((f) => f != "Button"); + } + + function get_field_data(df) { + let fieldname = `${df.label} (${df.fieldname})`; + if (!df.label) { + fieldname = `${df.fieldname}`; + } + let fieldtype = `${df.fieldtype}`; + return [fieldname, fieldtype]; + } + + fields.forEach((df) => { + // check if fieldname already exist + let duplicate = fields.filter((f) => f.fieldname == df.fieldname); + if (duplicate.length > 1) { + frappe.throw(__("Fieldname {0} appears multiple times", get_field_data(df))); } - if (!this.get_docfields.length) { - let docfield = this.is_customize_form ? "Customize Form Field" : "DocField"; - await frappe.model.with_doctype(docfield); - let df = frappe.get_meta(docfield).fields; - if (this.is_customize_form) { - this.custom_docfields = df; - } else { - this.docfields = df; - } + // Link & Table fields should always have options set + if (in_list(["Link", ...frappe.model.table_fields], df.fieldtype) && !df.options) { + frappe.throw( + __("Options is required for field {0} of type {1}", get_field_data(df)) + ); } - this.layout = this.get_layout(); - this.active_tab = this.layout.tabs[0].df.name; - this.selected_field = null; - - nextTick(() => { - this.dirty = false; - this.read_only = - !this.is_customize_form && !frappe.boot.developer_mode && !this.doc.custom; - this.preview = false; - }); - }, - reset_changes() { - this.fetch(); - }, - validate_fields(fields, is_table) { - fields = scrub_field_names(fields); - - let not_allowed_in_list_view = ["Attach Image", ...frappe.model.no_value_type]; - if (is_table) { - not_allowed_in_list_view = not_allowed_in_list_view.filter((f) => f != "Button"); + // Do not allow if field is hidden & required but doesn't have default value + if (df.hidden && df.reqd && !df.default) { + frappe.throw( + __( + "{0} cannot be hidden and mandatory without any default value", + get_field_data(df) + ) + ); } - function get_field_data(df) { - let fieldname = `${df.label} (${df.fieldname})`; - if (!df.label) { - fieldname = `${df.fieldname}`; - } - let fieldtype = `${df.fieldtype}`; - return [fieldname, fieldtype]; + // In List View is not allowed for some fieldtypes + if (df.in_list_view && in_list(not_allowed_in_list_view, df.fieldtype)) { + frappe.throw( + __( + "'In List View' is not allowed for field {0} of type {1}", + get_field_data(df) + ) + ); } - fields.forEach((df) => { - // check if fieldname already exist - let duplicate = fields.filter((f) => f.fieldname == df.fieldname); - if (duplicate.length > 1) { - frappe.throw(__("Fieldname {0} appears multiple times", get_field_data(df))); - } + // In Global Search is not allowed for no_value_type fields + if (df.in_global_search && in_list(frappe.model.no_value_type, df.fieldtype)) { + frappe.throw( + __( + "'In Global Search' is not allowed for field {0} of type {1}", + get_field_data(df) + ) + ); + } + }); + } - // Link & Table fields should always have options set - if (in_list(["Link", ...frappe.model.table_fields], df.fieldtype) && !df.options) { - frappe.throw( - __("Options is required for field {0} of type {1}", get_field_data(df)) - ); - } + async function save_changes() { + if (!dirty.value) { + frappe.show_alert({ message: __("No changes to save"), indicator: "orange" }); + return; + } - // Do not allow if field is hidden & required but doesn't have default value - if (df.hidden && df.reqd && !df.default) { - frappe.throw( - __( - "{0} cannot be hidden and mandatory without any default value", - get_field_data(df) - ) - ); - } + frappe.dom.freeze(__("Saving...")); - // In List View is not allowed for some fieldtypes - if (df.in_list_view && in_list(not_allowed_in_list_view, df.fieldtype)) { - frappe.throw( - __( - "'In List View' is not allowed for field {0} of type {1}", - get_field_data(df) - ) - ); - } + try { + if (is_customize_form.value) { + let doc = frappe.get_doc("Customize Form"); + doc.doc_type = doctype.value; + doc.fields = get_updated_fields(); + validate_fields(doc.fields, doc.istable); + await frappe.call({ method: "save_customization", doc }); + } else { + doc.value.fields = get_updated_fields(); + validate_fields(doc.value.fields, doc.value.istable); + await frappe.call("frappe.client.save", { doc: doc.value }); + frappe.toast("Fields Table Updated"); + } + fetch(); + } catch (e) { + console.error(e); + } finally { + frappe.dom.unfreeze(); + } + } - // In Global Search is not allowed for no_value_type fields - if (df.in_global_search && in_list(frappe.model.no_value_type, df.fieldtype)) { - frappe.throw( - __( - "'In Global Search' is not allowed for field {0} of type {1}", - get_field_data(df) - ) - ); - } - }); - }, - async save_changes() { - if (!this.dirty) { - frappe.show_alert({ message: __("No changes to save"), indicator: "orange" }); - return; + function get_updated_fields() { + let fields = []; + let idx = 0; + + let layout_fields = JSON.parse(JSON.stringify(layout.value.tabs)); + + layout_fields.forEach((tab, i) => { + if ( + (i == 0 && is_df_updated(tab.df, get_df("Tab Break", "", __("Details")))) || + i > 0 + ) { + idx++; + tab.df.idx = idx; + fields.push(tab.df); } - frappe.dom.freeze(__("Saving...")); + tab.sections.forEach((section, j) => { + // data before section is added + let fields_copy = JSON.parse(JSON.stringify(fields)); + let old_idx = idx; + section.has_fields = false; - try { - if (this.is_customize_form) { - let doc = frappe.get_doc("Customize Form"); - doc.doc_type = this.doctype; - doc.fields = this.get_updated_fields(); - this.validate_fields(doc.fields, doc.istable); - await frappe.call({ method: "save_customization", doc }); - } else { - this.doc.fields = this.get_updated_fields(); - this.validate_fields(this.doc.fields, this.doc.istable); - await frappe.call({ - method: "frappe.desk.form.save.savedocs", - args: { doc: this.doc, action: "Save" }, - }); - } - this.fetch(); - } catch (e) { - console.error(e); - } finally { - frappe.dom.unfreeze(); - } - }, - get_updated_fields() { - let fields = []; - let idx = 0; - - let layout_fields = JSON.parse(JSON.stringify(this.layout.tabs)); - - layout_fields.forEach((tab, i) => { - if ( - (i == 0 && - this.is_df_updated(tab.df, this.get_df("Tab Break", "", __("Details")))) || - i > 0 - ) { + // do not consider first section if label is not set + if ((j == 0 && is_df_updated(section.df, get_df("Section Break"))) || j > 0) { idx++; - tab.df.idx = idx; - fields.push(tab.df); + section.df.idx = idx; + fields.push(section.df); } - tab.sections.forEach((section, j) => { - // data before section is added - let fields_copy = JSON.parse(JSON.stringify(fields)); - let old_idx = idx; - section.has_fields = false; - - // do not consider first section if label is not set + section.columns.forEach((column, k) => { + // do not consider first column if label is not set if ( - (j == 0 && this.is_df_updated(section.df, this.get_df("Section Break"))) || - j > 0 + (k == 0 && is_df_updated(column.df, get_df("Column Break"))) || + k > 0 || + column.fields.length == 0 ) { idx++; - section.df.idx = idx; - fields.push(section.df); + column.df.idx = idx; + fields.push(column.df); } - section.columns.forEach((column, k) => { - // do not consider first column if label is not set - if ( - (k == 0 && - this.is_df_updated(column.df, this.get_df("Column Break"))) || - k > 0 || - column.fields.length == 0 - ) { - idx++; - column.df.idx = idx; - fields.push(column.df); - } - - column.fields.forEach((field) => { - idx++; - field.df.idx = idx; - fields.push(field.df); - section.has_fields = true; - }); + column.fields.forEach((field) => { + idx++; + field.df.idx = idx; + fields.push(field.df); + section.has_fields = true; }); - - // restore data back to data before section is added. - if (!section.has_fields) { - fields = fields_copy || []; - idx = old_idx; - } }); - }); - return fields; - }, - is_df_updated(df, new_df) { - delete df.name; - delete new_df.name; - return JSON.stringify(df) != JSON.stringify(new_df); - }, - get_layout() { - return create_layout(this.doc.fields); - }, - }, + // restore data back to data before section is added. + if (!section.has_fields) { + fields = fields_copy || []; + idx = old_idx; + } + }); + }); + + return fields; + } + + function is_df_updated(df, new_df) { + delete df.name; + delete new_df.name; + return JSON.stringify(df) != JSON.stringify(new_df); + } + + function get_layout() { + return create_layout(doc.value.fields); + } + + return { + doctype, + doc, + layout, + active_tab, + selected_field, + dirty, + read_only, + is_customize_form, + preview, + drag, + get_animation, + selected, + get_docfields, + get_df, + has_standard_field, + current_tab, + fetch, + reset_changes, + validate_fields, + save_changes, + get_updated_fields, + is_df_updated, + get_layout, + }; }); From 28bba3c1885ce818d2c39fccd358a36d4f3b346f Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 20 Feb 2023 13:20:41 +0530 Subject: [PATCH 24/61] fix: always use layout from store --- .../public/js/form_builder/components/Tabs.vue | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/frappe/public/js/form_builder/components/Tabs.vue b/frappe/public/js/form_builder/components/Tabs.vue index 625ca38745..3cf40865c4 100644 --- a/frappe/public/js/form_builder/components/Tabs.vue +++ b/frappe/public/js/form_builder/components/Tabs.vue @@ -9,9 +9,8 @@ import { ref, computed, nextTick } from "vue"; let store = useStore(); let dragged = ref(false); -let layout = computed(() => store.layout); -let has_tabs = computed(() => layout.value.tabs.length > 1); -store.active_tab = layout.value.tabs[0].df.name; +let has_tabs = computed(() => store.layout.tabs.length > 1); +store.active_tab = store.layout.tabs[0].df.name; function activate_tab(tab) { store.active_tab = tab.df.name; @@ -35,11 +34,11 @@ function drag_over(tab) { function add_new_tab() { let tab = { - df: store.get_df("Tab Break", "", "Tab " + (layout.value.tabs.length + 1)), + df: store.get_df("Tab Break", "", "Tab " + (store.layout.tabs.length + 1)), sections: [section_boilerplate()], }; - layout.value.tabs.push(tab); + store.layout.tabs.push(tab); activate_tab(tab); } @@ -77,7 +76,7 @@ function remove_tab() { } function delete_tab(with_children) { - let tabs = layout.value.tabs; + let tabs = store.layout.tabs; let index = tabs.indexOf(store.current_tab); if (!with_children) { @@ -109,11 +108,11 @@ function delete_tab(with_children) {