Date: Sat, 3 Jun 2023 16:52:51 +0530
Subject: [PATCH 06/44] fix: handle multiple webform for same doctype
---
frappe/website/doctype/web_form/web_form.json | 31 +++++++++++++++----
frappe/website/doctype/web_form/web_form.py | 14 ++++++---
2 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/frappe/website/doctype/web_form/web_form.json b/frappe/website/doctype/web_form/web_form.json
index 96749e460d..e0883ba439 100644
--- a/frappe/website/doctype/web_form/web_form.json
+++ b/frappe/website/doctype/web_form/web_form.json
@@ -31,6 +31,10 @@
"allow_incomplete",
"section_break_2",
"max_attachment_size",
+ "section_break_xzqr",
+ "condition",
+ "column_break_tjgl",
+ "condition_description",
"section_break_3",
"list_setting_message",
"show_list",
@@ -279,10 +283,6 @@
"fieldtype": "Tab Break",
"label": "Form"
},
- {
- "fieldname": "column_break_1",
- "fieldtype": "Column Break"
- },
{
"fieldname": "section_break_1",
"fieldtype": "Section Break"
@@ -297,7 +297,6 @@
"fieldtype": "Column Break"
},
{
- "collapsible": 1,
"fieldname": "section_break_2",
"fieldtype": "Section Break"
},
@@ -374,13 +373,33 @@
"fieldname": "anonymous",
"fieldtype": "Check",
"label": "Anonymous"
+ },
+ {
+ "fieldname": "condition",
+ "fieldtype": "Code",
+ "label": "Condition",
+ "max_height": "150px"
+ },
+ {
+ "fieldname": "section_break_xzqr",
+ "fieldtype": "Section Break"
+ },
+ {
+ "fieldname": "column_break_tjgl",
+ "fieldtype": "Column Break"
+ },
+ {
+ "fieldname": "condition_description",
+ "fieldtype": "HTML",
+ "label": "Condition Description",
+ "options": "Multiple webforms can be created for a single doctype. Write a condition specific to this webform to display correct record after submission.
For Example:
\nIf you create a separate webform every year to capture feedback from employees add a \n field named year in doctype and add a condition doc.year==\"2023\"
\n"
}
],
"has_web_view": 1,
"icon": "icon-edit",
"is_published_field": "published",
"links": [],
- "modified": "2023-04-20 17:24:42.657731",
+ "modified": "2023-06-03 19:18:56.760479",
"modified_by": "Administrator",
"module": "Website",
"name": "Web Form",
diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py
index 3e2705bdbe..81c6001558 100644
--- a/frappe/website/doctype/web_form/web_form.py
+++ b/frappe/website/doctype/web_form/web_form.py
@@ -153,10 +153,16 @@ def get_context(context):
and not frappe.form_dict.name
and not frappe.form_dict.is_list
):
- name = frappe.db.get_value(self.doc_type, {"owner": frappe.session.user}, "name")
- if name:
- context.in_view_mode = True
- frappe.redirect(f"/{self.route}/{name}")
+ names = frappe.db.get_values(self.doc_type, {"owner": frappe.session.user}, pluck="name")
+ for name in names:
+ if self.condition:
+ doc = frappe.get_doc(self.doc_type, name)
+ if frappe.safe_eval(self.condition, None, {"doc": doc.as_dict()}):
+ context.in_view_mode = True
+ frappe.redirect(f"/{self.route}/{name}")
+ else:
+ context.in_view_mode = True
+ frappe.redirect(f"/{self.route}/{name}")
# Show new form when
# - User is Guest
From ba2251219141cc57b714bd3d6aece682d419de1c Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Sat, 3 Jun 2023 14:16:43 +0200
Subject: [PATCH 07/44] refactor: call control's `on_section_collapse()`
This way, the logic can stay in the control itself. Each control can
decide what it needs to do on section collapse/expand.
---
frappe/public/js/frappe/form/controls/signature.js | 3 +++
frappe/public/js/frappe/form/section.js | 7 +------
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/frappe/public/js/frappe/form/controls/signature.js b/frappe/public/js/frappe/form/controls/signature.js
index 0cbc1f3c26..6ab96012f1 100644
--- a/frappe/public/js/frappe/form/controls/signature.js
+++ b/frappe/public/js/frappe/form/controls/signature.js
@@ -133,4 +133,7 @@ frappe.ui.form.ControlSignature = class ControlSignature extends frappe.ui.form.
this.set_my_value(base64_img);
this.set_image(this.get_value());
}
+ on_section_collapse() {
+ this.refresh();
+ }
};
diff --git a/frappe/public/js/frappe/form/section.js b/frappe/public/js/frappe/form/section.js
index a692cbac0d..b4908e749a 100644
--- a/frappe/public/js/frappe/form/section.js
+++ b/frappe/public/js/frappe/form/section.js
@@ -110,12 +110,7 @@ export default class Section {
this.set_icon(hide);
- // refresh signature fields
- this.fields_list.forEach((f) => {
- if (f.df.fieldtype == "Signature") {
- f.refresh();
- }
- });
+ this.fields_list.forEach((f) => f.on_section_collapse && f.on_section_collapse(hide));
// save state for next reload ('' is falsy)
if (this.df.css_class)
From 79aaf072bde8646d11cc7a5e80d73a0ffa74f2a6 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Sat, 3 Jun 2023 14:17:59 +0200
Subject: [PATCH 08/44] fix: fit and recenter map when section is expanded
---
.../js/frappe/form/controls/geolocation.js | 25 +++++++++++++------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js
index 37d799e96d..29d36539ec 100644
--- a/frappe/public/js/frappe/form/controls/geolocation.js
+++ b/frappe/public/js/frappe/form/controls/geolocation.js
@@ -73,15 +73,8 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f
})
);
this.add_non_group_layers(data_layers, this.editableLayers);
- try {
- this.map.fitBounds(this.editableLayers.getBounds(), {
- padding: [50, 50],
- });
- } catch (err) {
- // suppress error if layer has a point.
- }
this.editableLayers.addTo(this.map);
- this.map.invalidateSize();
+ this.fit_and_recenter_map();
}
bind_leaflet_map() {
@@ -205,4 +198,20 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f
this.editableLayers.removeLayer(l);
});
}
+
+ fit_and_recenter_map() {
+ // Spread map across the wrapper, recenter and zoom w.r.t bounds
+ try {
+ this.map.invalidateSize();
+ this.map.fitBounds(this.editableLayers.getBounds(), {
+ padding: [50, 50],
+ });
+ } catch (err) {
+ // suppress error if layer has a point.
+ }
+ }
+
+ on_section_collapse(hide) {
+ !hide && this.fit_and_recenter_map();
+ }
};
From f9251f0f5b2a051def991bd1e7828ba83e425a11 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Sat, 3 Jun 2023 14:38:14 +0200
Subject: [PATCH 09/44] refactor: move pointToLayer into a separate method
For better customizability
---
.../js/frappe/form/controls/geolocation.js | 30 ++++++++++++-------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js
index 29d36539ec..ada729fd66 100644
--- a/frappe/public/js/frappe/form/controls/geolocation.js
+++ b/frappe/public/js/frappe/form/controls/geolocation.js
@@ -60,23 +60,31 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f
this.clear_editable_layers();
const data_layers = new L.FeatureGroup().addLayer(
- L.geoJson(JSON.parse(value), {
- pointToLayer: function (geoJsonPoint, latlng) {
- if (geoJsonPoint.properties.point_type == "circle") {
- return L.circle(latlng, { radius: geoJsonPoint.properties.radius });
- } else if (geoJsonPoint.properties.point_type == "circlemarker") {
- return L.circleMarker(latlng, { radius: geoJsonPoint.properties.radius });
- } else {
- return L.marker(latlng);
- }
- },
- })
+ L.geoJson(JSON.parse(value), { pointToLayer: this.point_to_layer })
);
this.add_non_group_layers(data_layers, this.editableLayers);
this.editableLayers.addTo(this.map);
this.fit_and_recenter_map();
}
+ /**
+ * Defines custom rules for how geoJSON data is rendered on the map.
+ *
+ * @param {Object} geoJsonPoint - The geoJSON object to be rendered on the map.
+ * @param {Object} latlng - The latitude and longitude where the geoJSON data should be rendered on the map.
+ * @returns {Object} - Returns the Leaflet layer object to be rendered on the map.
+ */
+ point_to_layer(geoJsonPoint, latlng) {
+ // Custom rules for how geojson data is rendered on the map
+ if (geoJsonPoint.properties.point_type == "circle") {
+ return L.circle(latlng, { radius: geoJsonPoint.properties.radius });
+ } else if (geoJsonPoint.properties.point_type == "circlemarker") {
+ return L.circleMarker(latlng, { radius: geoJsonPoint.properties.radius });
+ } else {
+ return L.marker(latlng);
+ }
+ }
+
bind_leaflet_map() {
const circleToGeoJSON = L.Circle.prototype.toGeoJSON;
L.Circle.include({
From 235e4855c2ed45953c0c1bdb3115901ee7d4618c Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 19:27:06 +0530
Subject: [PATCH 10/44] feat: DBHooks to run things before/after
commit/rollback
This is a common pattern which is implemented in inconsistent and undocumented ways using these:
- `frappe.local.rollback_observers`
- `frappe.flags.enqueue_after_commit`
- `frappe.local.realtime_log`
- `frappe.local.before_commit`
- `flush_local_link_count`
Instead new simple api:
- Simple function call `frappe.db.after_commit.run(function)`
- If you need args just pass partial function `frappe.db.after_commit.run(lambda: frappe.clear_cache(doctype, name)`
---
frappe/database/database.py | 91 ++++++++++++++++++++++++++++++-------
1 file changed, 75 insertions(+), 16 deletions(-)
diff --git a/frappe/database/database.py b/frappe/database/database.py
index 728d1e9584..8109a83d43 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -8,9 +8,10 @@ import random
import re
import string
import traceback
+from collections import deque
from contextlib import contextmanager, suppress
from time import time
-from typing import Any, Iterable, Sequence
+from typing import Any, Callable, Iterable, Sequence
from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder
from pypika.terms import Criterion, NullValue
@@ -107,6 +108,12 @@ class Database:
self.value_cache = {}
self.logger = frappe.logger("database")
self.logger.setLevel("WARNING")
+
+ self.before_commit = DBHooks()
+ self.after_commit = DBHooks()
+ self.before_rollback = DBHooks()
+ self.after_rollback = DBHooks()
+
# self.db_type: str
# self.last_query (lazy) attribute of last sql query executed
@@ -973,14 +980,45 @@ class Database:
for method in frappe.local.before_commit:
frappe.call(method[0], *(method[1] or []), **(method[2] or {}))
+ # Invalidated by a commit.
+ self.before_rollback.reset()
+ self.after_rollback.reset()
+
+ self.before_commit.run()
+
self.sql("commit")
self.begin() # explicitly start a new transaction
+ self.after_commit.run()
+
frappe.local.rollback_observers = []
self.flush_realtime_log()
enqueue_jobs_after_commit()
flush_local_link_count()
+ def rollback(self, *, save_point=None):
+ """`ROLLBACK` current transaction. Optionally rollback to a known save_point."""
+ if save_point:
+ self.sql(f"rollback to savepoint {save_point}")
+ else:
+ self.before_commit.reset()
+ self.after_commit.reset()
+
+ self.before_rollback.run()
+
+ self.sql("rollback")
+ self.begin()
+
+ self.after_rollback.run()
+
+ for obj in dict.fromkeys(frappe.local.rollback_observers):
+ if hasattr(obj, "on_rollback"):
+ obj.on_rollback()
+ frappe.local.rollback_observers = []
+
+ frappe.local.realtime_log = []
+ frappe.flags.enqueue_after_commit = []
+
def add_before_commit(self, method, args=None, kwargs=None):
frappe.local.before_commit.append([method, args, kwargs])
@@ -1004,21 +1042,6 @@ class Database:
def release_savepoint(self, save_point):
self.sql(f"release savepoint {save_point}")
- def rollback(self, *, save_point=None):
- """`ROLLBACK` current transaction. Optionally rollback to a known save_point."""
- if save_point:
- self.sql(f"rollback to savepoint {save_point}")
- else:
- self.sql("rollback")
- self.begin()
- for obj in dict.fromkeys(frappe.local.rollback_observers):
- if hasattr(obj, "on_rollback"):
- obj.on_rollback()
- frappe.local.rollback_observers = []
-
- frappe.local.realtime_log = []
- frappe.flags.enqueue_after_commit = []
-
def field_exists(self, dt, fn):
"""Return true of field exists."""
return self.exists("DocField", {"fieldname": fn, "parent": dt})
@@ -1304,6 +1327,42 @@ class Database:
raise NotImplementedError
+class DBHooks:
+ """Hooks for database events.
+
+ Primarily used for doing things before/after commit/rollback.
+
+ hook_manager = DBHooks()
+
+ # Put a function call in queue
+ hook_manager.add(func)
+
+ # Run all pending functions in queue
+ hook_manager.run()
+
+ # Reset quue
+ hook_manager.reset()
+ """
+
+ __slots__ = ("_functions",)
+
+ def __init__(self) -> None:
+ self._functions = deque()
+
+ def add(self, func: Callable) -> None:
+ """Add a function to queue, functions are executed in order of addition."""
+ self._functions.append(func)
+
+ def run(self):
+ """Run all functions in queue"""
+ while self._functions:
+ _func = self._functions.popleft()
+ _func()
+
+ def reset(self):
+ self._functions = deque()
+
+
def enqueue_jobs_after_commit():
from frappe.utils.background_jobs import (
RQ_JOB_FAILURE_TTL,
From 7e9ef00bea42fb7b296f4a6ca371d8650511f026 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 19:49:17 +0530
Subject: [PATCH 11/44] refactor!: Remove `frappe.db.add_before_commit`
Not used anywhere, use `frappe.db.before_commit.add()` instead.
---
frappe/__init__.py | 1 -
frappe/database/database.py | 6 ------
frappe/tests/utils.py | 1 -
3 files changed, 8 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index e5a0b9c4aa..68a87e5226 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -209,7 +209,6 @@ def init(site: str, sites_path: str = ".", new_site: bool = False, force=False)
)
local.rollback_observers = []
local.locked_documents = []
- local.before_commit = []
local.test_objects = {}
local.site = site
diff --git a/frappe/database/database.py b/frappe/database/database.py
index 8109a83d43..ab752e0a6b 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -977,9 +977,6 @@ class Database:
def commit(self):
"""Commit current transaction. Calls SQL `COMMIT`."""
- for method in frappe.local.before_commit:
- frappe.call(method[0], *(method[1] or []), **(method[2] or {}))
-
# Invalidated by a commit.
self.before_rollback.reset()
self.after_rollback.reset()
@@ -1019,9 +1016,6 @@ class Database:
frappe.local.realtime_log = []
frappe.flags.enqueue_after_commit = []
- def add_before_commit(self, method, args=None, kwargs=None):
- frappe.local.before_commit.append([method, args, kwargs])
-
@staticmethod
def flush_realtime_log():
for args in frappe.local.realtime_log:
diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py
index 2cdcfb5643..e1517e38a0 100644
--- a/frappe/tests/utils.py
+++ b/frappe/tests/utils.py
@@ -101,7 +101,6 @@ def _commit_watcher():
def _rollback_db():
- frappe.local.before_commit = []
frappe.local.rollback_observers = []
frappe.db.value_cache = {}
frappe.db.rollback()
From b3d370a0b18613c3d9cfa8ab36d11e5a137018ab Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 20:14:38 +0530
Subject: [PATCH 12/44] refactor!: remove `rollback_observers`
use `frappe.db.after_rollback.add` instead
---
frappe/__init__.py | 1 -
frappe/commands/utils.py | 2 +-
frappe/core/doctype/file/file.py | 18 ++++++++++++------
frappe/core/doctype/file/test_file.py | 12 +++++++++++-
frappe/database/database.py | 7 -------
frappe/tests/utils.py | 1 -
6 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index 68a87e5226..f87807a7cb 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -207,7 +207,6 @@ def init(site: str, sites_path: str = ".", new_site: bool = False, force=False)
"read_only": False,
}
)
- local.rollback_observers = []
local.locked_documents = []
local.test_objects = {}
diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py
index 03374986d4..e44009a886 100644
--- a/frappe/commands/utils.py
+++ b/frappe/commands/utils.py
@@ -623,7 +623,7 @@ frappe.db.connect()
def _console_cleanup():
- # Execute rollback_observers on console close
+ # Execute after_rollback on console close
frappe.db.rollback()
frappe.destroy()
diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py
index 3728bd0af0..c4cefc7271 100755
--- a/frappe/core/doctype/file/file.py
+++ b/frappe/core/doctype/file/file.py
@@ -69,7 +69,7 @@ class File(Document):
else:
self.save_file(content=self.get_content())
self.flags.new_file = True
- frappe.local.rollback_observers.append(self)
+ frappe.db.after_rollback.add(self.on_rollback)
def after_insert(self):
if not self.is_folder:
@@ -121,10 +121,16 @@ class File(Document):
self.add_comment_in_reference_doc("Attachment Removed", _("Removed {0}").format(self.file_name))
def on_rollback(self):
+ rollback_flags = ("new_file", "original_content", "original_path")
+
+ def pop_rollback_flags():
+ for flag in rollback_flags:
+ self.flags.pop(flag, None)
+
# following condition is only executed when an insert has been rolledback
if self.flags.new_file:
self._delete_file_on_disk()
- self.flags.pop("new_file")
+ pop_rollback_flags()
return
# if original_content flag is set, this rollback should revert the file to its original state
@@ -139,14 +145,14 @@ class File(Document):
with open(file_path, mode) as f:
f.write(self.flags.original_content)
os.fsync(f.fileno())
- self.flags.pop("original_content")
+ pop_rollback_flags()
# used in case file path (File.file_url) has been changed
if self.flags.original_path:
target = self.flags.original_path["old"]
source = self.flags.original_path["new"]
shutil.move(source, target)
- self.flags.pop("original_path")
+ pop_rollback_flags()
def get_name_based_on_parent_folder(self) -> str | None:
if self.folder:
@@ -218,7 +224,7 @@ class File(Document):
# Uses os.rename which is an atomic operation
shutil.move(source, target)
self.flags.original_path = {"old": source, "new": target}
- frappe.local.rollback_observers.append(self)
+ frappe.db.after_rollback.add(self.on_rollback)
self.file_url = updated_file_url
update_existing_file_docs(self)
@@ -520,7 +526,7 @@ class File(Document):
f.write(self._content)
os.fsync(f.fileno())
- frappe.local.rollback_observers.append(self)
+ frappe.db.after_rollback.add(self.on_rollback)
return file_path
diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py
index 51e065f710..bbe8bb6d1a 100644
--- a/frappe/core/doctype/file/test_file.py
+++ b/frappe/core/doctype/file/test_file.py
@@ -17,7 +17,7 @@ from frappe.core.api.file import (
move_file,
unzip_file,
)
-from frappe.core.doctype.file.utils import get_extension
+from frappe.core.doctype.file.utils import delete_file, get_extension
from frappe.exceptions import ValidationError
from frappe.tests.utils import FrappeTestCase
from frappe.utils import get_files_path
@@ -77,6 +77,16 @@ class TestSimpleFile(FrappeTestCase):
self.assertEqual(content, self.test_content)
+class TestFSRollbacks(FrappeTestCase):
+ def test_rollback_from_file_system(self):
+ file_name = content = frappe.generate_hash()
+ file = frappe.new_doc("File", file_name=file_name, content=content).insert()
+ self.assertTrue(file.exists_on_disk())
+
+ frappe.db.rollback()
+ self.assertFalse(file.exists_on_disk())
+
+
class TestBase64File(FrappeTestCase):
def setUp(self):
self.attached_to_doctype, self.attached_to_docname = make_test_doc()
diff --git a/frappe/database/database.py b/frappe/database/database.py
index ab752e0a6b..cd00e28554 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -125,7 +125,6 @@ class Database:
self.cur_db_name = self.user
self._conn = self.get_connection()
self._cursor = self._conn.cursor()
- frappe.local.rollback_observers = []
try:
if execution_timeout := get_query_execution_timeout():
@@ -988,7 +987,6 @@ class Database:
self.after_commit.run()
- frappe.local.rollback_observers = []
self.flush_realtime_log()
enqueue_jobs_after_commit()
flush_local_link_count()
@@ -1008,11 +1006,6 @@ class Database:
self.after_rollback.run()
- for obj in dict.fromkeys(frappe.local.rollback_observers):
- if hasattr(obj, "on_rollback"):
- obj.on_rollback()
- frappe.local.rollback_observers = []
-
frappe.local.realtime_log = []
frappe.flags.enqueue_after_commit = []
diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py
index e1517e38a0..df02ee2789 100644
--- a/frappe/tests/utils.py
+++ b/frappe/tests/utils.py
@@ -101,7 +101,6 @@ def _commit_watcher():
def _rollback_db():
- frappe.local.rollback_observers = []
frappe.db.value_cache = {}
frappe.db.rollback()
From 65196510028ba8b19b5595f6ac142c88370de11a Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 20:24:53 +0530
Subject: [PATCH 13/44] refactor: change implementation of enqueue_after_commit
if enqueue_after_commit then pass partial function after commit instead
of storing it in flags. SLIGHTLY less efficient, but uses consistent API.
---
frappe/database/database.py | 24 --------------------
frappe/utils/background_jobs.py | 40 ++++++++++++++-------------------
2 files changed, 17 insertions(+), 47 deletions(-)
diff --git a/frappe/database/database.py b/frappe/database/database.py
index cd00e28554..44876602b7 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -988,7 +988,6 @@ class Database:
self.after_commit.run()
self.flush_realtime_log()
- enqueue_jobs_after_commit()
flush_local_link_count()
def rollback(self, *, save_point=None):
@@ -1007,7 +1006,6 @@ class Database:
self.after_rollback.run()
frappe.local.realtime_log = []
- frappe.flags.enqueue_after_commit = []
@staticmethod
def flush_realtime_log():
@@ -1350,28 +1348,6 @@ class DBHooks:
self._functions = deque()
-def enqueue_jobs_after_commit():
- from frappe.utils.background_jobs import (
- RQ_JOB_FAILURE_TTL,
- RQ_RESULTS_TTL,
- execute_job,
- get_queue,
- )
-
- if frappe.flags.enqueue_after_commit and len(frappe.flags.enqueue_after_commit) > 0:
- for job in frappe.flags.enqueue_after_commit:
- q = get_queue(job.get("queue"), is_async=job.get("is_async"))
- q.enqueue_call(
- execute_job,
- timeout=job.get("timeout"),
- kwargs=job.get("queue_args"),
- failure_ttl=frappe.conf.get("rq_job_failure_ttl") or RQ_JOB_FAILURE_TTL,
- result_ttl=frappe.conf.get("rq_results_ttl") or RQ_RESULTS_TTL,
- job_id=job.get("job_id"),
- )
- frappe.flags.enqueue_after_commit = []
-
-
@contextmanager
def savepoint(catch: type | tuple[type, ...] = Exception):
"""Wrapper for wrapping blocks of DB operations in a savepoint.
diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py
index 0fbc9e15ec..93c957639b 100755
--- a/frappe/utils/background_jobs.py
+++ b/frappe/utils/background_jobs.py
@@ -113,6 +113,7 @@ def enqueue(
if not timeout:
timeout = get_queues_timeout().get(queue) or 300
+
queue_args = {
"site": frappe.local.site,
"user": frappe.session.user,
@@ -122,32 +123,25 @@ def enqueue(
"is_async": is_async,
"kwargs": kwargs,
}
- if enqueue_after_commit:
- if not frappe.flags.enqueue_after_commit:
- frappe.flags.enqueue_after_commit = []
- frappe.flags.enqueue_after_commit.append(
- {
- "queue": queue,
- "is_async": is_async,
- "timeout": timeout,
- "queue_args": queue_args,
- "job_id": job_id,
- }
+ def enqueue_call():
+ return q.enqueue_call(
+ execute_job,
+ on_success=on_success,
+ on_failure=on_failure,
+ timeout=timeout,
+ kwargs=queue_args,
+ at_front=at_front,
+ failure_ttl=frappe.conf.get("rq_job_failure_ttl") or RQ_JOB_FAILURE_TTL,
+ result_ttl=frappe.conf.get("rq_results_ttl") or RQ_RESULTS_TTL,
+ job_id=job_id,
)
- return frappe.flags.enqueue_after_commit
- return q.enqueue_call(
- execute_job,
- on_success=on_success,
- on_failure=on_failure,
- timeout=timeout,
- kwargs=queue_args,
- at_front=at_front,
- failure_ttl=frappe.conf.get("rq_job_failure_ttl") or RQ_JOB_FAILURE_TTL,
- result_ttl=frappe.conf.get("rq_results_ttl") or RQ_RESULTS_TTL,
- job_id=job_id,
- )
+ if enqueue_after_commit:
+ frappe.db.after_commit.add(enqueue_call)
+ return
+
+ return enqueue_call()
def enqueue_doc(
From ccc107b41fbd0edac3318d77d45976a474aeb207 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 21:30:56 +0530
Subject: [PATCH 14/44] test: use db.before_commit
---
frappe/database/database.py | 1 -
frappe/tests/utils.py | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/frappe/database/database.py b/frappe/database/database.py
index 44876602b7..816d91062c 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -976,7 +976,6 @@ class Database:
def commit(self):
"""Commit current transaction. Calls SQL `COMMIT`."""
- # Invalidated by a commit.
self.before_rollback.reset()
self.after_rollback.reset()
diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py
index df02ee2789..33b2a6c478 100644
--- a/frappe/tests/utils.py
+++ b/frappe/tests/utils.py
@@ -32,7 +32,7 @@ class FrappeTestCase(unittest.TestCase):
# flush changes done so far to avoid flake
frappe.db.commit()
if cls.SHOW_TRANSACTION_COMMIT_WARNINGS:
- frappe.db.add_before_commit(_commit_watcher)
+ frappe.db.before_commit.add(_commit_watcher)
# enqueue teardown actions (executed in LIFO order)
cls.addClassCleanup(_restore_thread_locals, copy.deepcopy(frappe.local.flags))
From 3f1c66de1048058da7b71d4e1ce0585a22ffe5f8 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 20:43:03 +0530
Subject: [PATCH 15/44] refactor: move flush_local_link_count to hook
---
frappe/database/database.py | 1 -
frappe/model/utils/link_count.py | 15 ++++++++-------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/frappe/database/database.py b/frappe/database/database.py
index 816d91062c..e11e2fccbd 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -987,7 +987,6 @@ class Database:
self.after_commit.run()
self.flush_realtime_log()
- flush_local_link_count()
def rollback(self, *, save_point=None):
"""`ROLLBACK` current transaction. Optionally rollback to a known save_point."""
diff --git a/frappe/model/utils/link_count.py b/frappe/model/utils/link_count.py
index 9a7694b9f8..3218d8482a 100644
--- a/frappe/model/utils/link_count.py
+++ b/frappe/model/utils/link_count.py
@@ -14,6 +14,8 @@ def notify_link_count(doctype, name):
else:
frappe.local.link_count[(doctype, name)] = 1
+ frappe.db.after_commit.add(flush_local_link_count)
+
def flush_local_link_count():
"""flush from local before ending request"""
@@ -31,6 +33,7 @@ def flush_local_link_count():
link_count[key] = frappe.local.link_count[key]
frappe.cache().set_value("_link_count", link_count)
+ frappe.local.link_count = {}
def update_link_count():
@@ -38,14 +41,12 @@ def update_link_count():
link_count = frappe.cache().get_value("_link_count")
if link_count:
- for key, count in link_count.items():
- if key[0] not in ignore_doctypes:
+ for (doctype, name), count in link_count.items():
+ if doctype not in ignore_doctypes:
try:
- frappe.db.sql(
- f"update `tab{key[0]}` set idx = idx + {count} where name=%s",
- key[1],
- auto_commit=1,
- )
+ table = frappe.qb.DocType(doctype)
+ frappe.qb.update(table).set(table.idx, table.idx + count).where(table.name == name).run()
+ frappe.db.commit()
except Exception as e:
if not frappe.db.is_table_missing(e): # table not found, single
raise e
From 680cf73cba177fad075eac0d7b57dc588ebdd309 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 22:11:21 +0530
Subject: [PATCH 16/44] fix: `link_count`
This didn't work correctly, if link_count is present in cache it would
just read and dump it back in.
This has practically never worked correctly.
---
frappe/__init__.py | 1 -
frappe/model/utils/link_count.py | 31 +++++++++++++++----------------
2 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index f87807a7cb..563404c570 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -231,7 +231,6 @@ def init(site: str, sites_path: str = ".", new_site: bool = False, force=False)
local.role_permissions = {}
local.valid_columns = {}
local.new_doc_templates = {}
- local.link_count = {}
local.jenv = None
local.jloader = None
diff --git a/frappe/model/utils/link_count.py b/frappe/model/utils/link_count.py
index 3218d8482a..49ed0d5a6c 100644
--- a/frappe/model/utils/link_count.py
+++ b/frappe/model/utils/link_count.py
@@ -1,6 +1,8 @@
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
# License: MIT. See LICENSE
+from collections import defaultdict
+
import frappe
ignore_doctypes = ("DocType", "Print Format", "Role", "Module Def", "Communication", "ToDo")
@@ -8,32 +10,29 @@ ignore_doctypes = ("DocType", "Print Format", "Role", "Module Def", "Communicati
def notify_link_count(doctype, name):
"""updates link count for given document"""
- if hasattr(frappe.local, "link_count"):
- if (doctype, name) in frappe.local.link_count:
- frappe.local.link_count[(doctype, name)] += 1
- else:
- frappe.local.link_count[(doctype, name)] = 1
+ if not hasattr(frappe.local, "_link_count"):
+ frappe.local._link_count = defaultdict(int)
+ frappe.db.after_commit.add(flush_local_link_count)
- frappe.db.after_commit.add(flush_local_link_count)
+ frappe.local._link_count[(doctype, name)] += 1
def flush_local_link_count():
"""flush from local before ending request"""
- if not getattr(frappe.local, "link_count", None):
+ new_links = getattr(frappe.local, "_link_count", None)
+ if not new_links:
return
- link_count = frappe.cache().get_value("_link_count")
- if not link_count:
- link_count = {}
+ link_count = frappe.cache().get_value("_link_count") or {}
- for key, value in frappe.local.link_count.items():
- if key in link_count:
- link_count[key] += frappe.local.link_count[key]
- else:
- link_count[key] = frappe.local.link_count[key]
+ for key, value in new_links.items():
+ if key in link_count:
+ link_count[key] += value
+ else:
+ link_count[key] = value
frappe.cache().set_value("_link_count", link_count)
- frappe.local.link_count = {}
+ new_links.clear()
def update_link_count():
From 6ce7444669c7462e26875bb6772838dab0df3cb5 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 22:29:05 +0530
Subject: [PATCH 17/44] refactor: generic callback manager
---
frappe/database/database.py | 48 +++++--------------------------------
frappe/utils/__init__.py | 42 +++++++++++++++++++++++++++++++-
2 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/frappe/database/database.py b/frappe/database/database.py
index e11e2fccbd..bd99f790f4 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -8,10 +8,9 @@ import random
import re
import string
import traceback
-from collections import deque
from contextlib import contextmanager, suppress
from time import time
-from typing import Any, Callable, Iterable, Sequence
+from typing import Any, Iterable, Sequence
from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder
from pypika.terms import Criterion, NullValue
@@ -32,6 +31,7 @@ from frappe.database.utils import (
from frappe.exceptions import DoesNotExistError, ImplicitCommitError
from frappe.model.utils.link_count import flush_local_link_count
from frappe.query_builder.functions import Count
+from frappe.utils import CallbackManager
from frappe.utils import cast as cast_fieldtype
from frappe.utils import cint, get_datetime, get_table_name, getdate, now, sbool
from frappe.utils.deprecations import deprecated, deprecation_warning
@@ -109,10 +109,10 @@ class Database:
self.logger = frappe.logger("database")
self.logger.setLevel("WARNING")
- self.before_commit = DBHooks()
- self.after_commit = DBHooks()
- self.before_rollback = DBHooks()
- self.after_rollback = DBHooks()
+ self.before_commit = CallbackManager()
+ self.after_commit = CallbackManager()
+ self.before_rollback = CallbackManager()
+ self.after_rollback = CallbackManager()
# self.db_type: str
# self.last_query (lazy) attribute of last sql query executed
@@ -1310,42 +1310,6 @@ class Database:
raise NotImplementedError
-class DBHooks:
- """Hooks for database events.
-
- Primarily used for doing things before/after commit/rollback.
-
- hook_manager = DBHooks()
-
- # Put a function call in queue
- hook_manager.add(func)
-
- # Run all pending functions in queue
- hook_manager.run()
-
- # Reset quue
- hook_manager.reset()
- """
-
- __slots__ = ("_functions",)
-
- def __init__(self) -> None:
- self._functions = deque()
-
- def add(self, func: Callable) -> None:
- """Add a function to queue, functions are executed in order of addition."""
- self._functions.append(func)
-
- def run(self):
- """Run all functions in queue"""
- while self._functions:
- _func = self._functions.popleft()
- _func()
-
- def reset(self):
- self._functions = deque()
-
-
@contextmanager
def savepoint(catch: type | tuple[type, ...] = Exception):
"""Wrapper for wrapping blocks of DB operations in a savepoint.
diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py
index ef32ff5653..b7dc565555 100644
--- a/frappe/utils/__init__.py
+++ b/frappe/utils/__init__.py
@@ -9,6 +9,7 @@ import os
import re
import sys
import traceback
+from collections import deque
from collections.abc import (
Container,
Generator,
@@ -20,7 +21,7 @@ from collections.abc import (
from email.header import decode_header, make_header
from email.utils import formataddr, parseaddr
from gzip import GzipFile
-from typing import Any, Literal
+from typing import Any, Callable, Literal
from urllib.parse import quote, urlparse
from redis.exceptions import ConnectionError
@@ -1092,3 +1093,42 @@ def is_git_url(url: str) -> bool:
# modified to allow without the tailing .git from https://github.com/jonschlinkert/is-git-url.git
pattern = r"(?:git|ssh|https?|\w*@[-\w.]+):(\/\/)?(.*?)(\.git)?(\/?|\#[-\d\w._]+?)$"
return bool(re.match(pattern, url))
+
+
+class CallbackManager:
+ """Manage callbacks.
+
+ ```
+ # Capture callacks
+ callbacks = CallbackManager()
+
+ # Put a function call in queue
+ callbacks.add(func)
+
+ # Run all pending functions in queue
+ callbacks.run()
+
+ # Reset queue
+ callbacks.reset()
+ ```
+
+ Example usage: frappe.db.after_commit
+ """
+
+ __slots__ = ("_functions",)
+
+ def __init__(self) -> None:
+ self._functions = deque()
+
+ def add(self, func: Callable) -> None:
+ """Add a function to queue, functions are executed in order of addition."""
+ self._functions.append(func)
+
+ def run(self):
+ """Run all functions in queue"""
+ while self._functions:
+ _func = self._functions.popleft()
+ _func()
+
+ def reset(self):
+ self._functions.clear()
From 54ae0c4a21976798b4effab6ebc9c5838d7ee381 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 22:40:12 +0530
Subject: [PATCH 18/44] refactor: move flush_realtime_log to realtime.py
This doesn't have anything to do with databases
---
frappe/__init__.py | 1 -
frappe/database/database.py | 11 -----------
frappe/realtime.py | 20 ++++++++++++++++++--
frappe/tests/utils.py | 1 -
4 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index 563404c570..b13c9230b3 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -190,7 +190,6 @@ def init(site: str, sites_path: str = ".", new_site: bool = False, force=False)
local.error_log = []
local.message_log = []
local.debug_log = []
- local.realtime_log = []
local.flags = _dict(
{
"currently_saving": [],
diff --git a/frappe/database/database.py b/frappe/database/database.py
index bd99f790f4..7209ec39da 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -986,8 +986,6 @@ class Database:
self.after_commit.run()
- self.flush_realtime_log()
-
def rollback(self, *, save_point=None):
"""`ROLLBACK` current transaction. Optionally rollback to a known save_point."""
if save_point:
@@ -1003,15 +1001,6 @@ class Database:
self.after_rollback.run()
- frappe.local.realtime_log = []
-
- @staticmethod
- def flush_realtime_log():
- for args in frappe.local.realtime_log:
- frappe.realtime.emit_via_redis(*args)
-
- frappe.local.realtime_log = []
-
def savepoint(self, save_point):
"""Savepoints work as a nested transaction.
diff --git a/frappe/realtime.py b/frappe/realtime.py
index e6980ef917..fdb86546f3 100644
--- a/frappe/realtime.py
+++ b/frappe/realtime.py
@@ -73,13 +73,29 @@ def publish_realtime(
room = get_site_room()
if after_commit:
+ if not hasattr(frappe.local, "_realtime_log"):
+ frappe.local._realtime_log = []
+ frappe.db.after_commit.add(flush_realtime_log)
+ frappe.db.after_rollback.add(clear_realtime_log)
+
params = [event, message, room]
- if params not in frappe.local.realtime_log:
- frappe.local.realtime_log.append(params)
+ if params not in frappe.local._realtime_log:
+ frappe.local._realtime_log.append(params)
else:
emit_via_redis(event, message, room)
+def flush_realtime_log():
+ for args in frappe.local._realtime_log:
+ frappe.realtime.emit_via_redis(*args)
+
+ frappe.local._realtime_log = []
+
+
+def clear_realtime_log():
+ frappe.local._realtime_log = []
+
+
def emit_via_redis(event, message, room):
"""Publish real-time updates via redis
diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py
index 33b2a6c478..07003d3b8c 100644
--- a/frappe/tests/utils.py
+++ b/frappe/tests/utils.py
@@ -110,7 +110,6 @@ def _restore_thread_locals(flags):
frappe.local.error_log = []
frappe.local.message_log = []
frappe.local.debug_log = []
- frappe.local.realtime_log = []
frappe.local.conf = frappe._dict(frappe.get_site_config())
frappe.local.cache = {}
frappe.local.lang = "en"
From 0b9dee47913d314b3e48bc306011b278a62f2f27 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 22:52:03 +0530
Subject: [PATCH 19/44] test: db callbacks
---
frappe/tests/test_db.py | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py
index ed01af655c..e27d1db0ba 100644
--- a/frappe/tests/test_db.py
+++ b/frappe/tests/test_db.py
@@ -593,6 +593,37 @@ class TestDB(FrappeTestCase):
modify_values((23, 23.0, 23.00004345, "wow", [1, 2, 3, "abc"])),
)
+ def test_callbacks(self):
+
+ order_of_execution = []
+
+ def f(val):
+ nonlocal order_of_execution
+ order_of_execution.append(val)
+
+ frappe.db.before_commit.add(lambda: f(0))
+ frappe.db.before_commit.add(lambda: f(1))
+
+ frappe.db.after_commit.add(lambda: f(2))
+ frappe.db.after_commit.add(lambda: f(3))
+
+ frappe.db.before_rollback.add(lambda: f("IGNORED"))
+ frappe.db.before_rollback.add(lambda: f("IGNORED"))
+
+ frappe.db.commit()
+
+ frappe.db.after_commit.add(lambda: f("IGNORED"))
+ frappe.db.after_commit.add(lambda: f("IGNORED"))
+
+ frappe.db.before_rollback.add(lambda: f(4))
+ frappe.db.before_rollback.add(lambda: f(5))
+ frappe.db.after_rollback.add(lambda: f(6))
+ frappe.db.after_rollback.add(lambda: f(7))
+
+ frappe.db.rollback()
+
+ self.assertEqual(order_of_execution, list(range(0, 8)))
+
@run_only_if(db_type_is.MARIADB)
class TestDDLCommandsMaria(FrappeTestCase):
From 07e1d34568a1ba73b110fd791b7d3a314b57e642 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 12:02:39 +0530
Subject: [PATCH 20/44] refactor: RQ enqueue after commit and tests
---
frappe/core/doctype/rq_job/test_rq_job.py | 22 +++++++++++-
frappe/database/database.py | 1 -
frappe/utils/background_jobs.py | 42 +++++++++++------------
3 files changed, 42 insertions(+), 23 deletions(-)
diff --git a/frappe/core/doctype/rq_job/test_rq_job.py b/frappe/core/doctype/rq_job/test_rq_job.py
index 265583fe83..09a90f7445 100644
--- a/frappe/core/doctype/rq_job/test_rq_job.py
+++ b/frappe/core/doctype/rq_job/test_rq_job.py
@@ -11,7 +11,7 @@ import frappe
from frappe.core.doctype.rq_job.rq_job import RQJob, remove_failed_jobs, stop_job
from frappe.tests.utils import FrappeTestCase, timeout
from frappe.utils import cstr, execute_in_shell
-from frappe.utils.background_jobs import is_job_enqueued
+from frappe.utils.background_jobs import get_job_status, is_job_enqueued
class TestRQJob(FrappeTestCase):
@@ -104,6 +104,26 @@ class TestRQJob(FrappeTestCase):
self.check_status(job, "finished")
self.assertFalse(is_job_enqueued(job_id))
+ @timeout(20)
+ def test_enqueue_after_commit(self):
+ job_id = frappe.generate_hash()
+
+ frappe.enqueue(self.BG_JOB, enqueue_after_commit=True, job_id=job_id)
+ self.assertIsNone(get_job_status(job_id))
+
+ frappe.db.commit()
+ self.assertIsNotNone(get_job_status(job_id))
+
+ job_id = frappe.generate_hash()
+ frappe.enqueue(self.BG_JOB, enqueue_after_commit=True, job_id=job_id)
+ self.assertIsNone(get_job_status(job_id))
+
+ frappe.db.rollback()
+ self.assertIsNone(get_job_status(job_id))
+
+ frappe.db.commit()
+ self.assertIsNone(get_job_status(job_id))
+
def test_func(fail=False, sleep=0):
if fail:
diff --git a/frappe/database/database.py b/frappe/database/database.py
index 7209ec39da..fc4f7b3b1b 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -29,7 +29,6 @@ from frappe.database.utils import (
is_query_type,
)
from frappe.exceptions import DoesNotExistError, ImplicitCommitError
-from frappe.model.utils.link_count import flush_local_link_count
from frappe.query_builder.functions import Count
from frappe.utils import CallbackManager
from frappe.utils import cast as cast_fieldtype
diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py
index 93c957639b..6a203f8dc7 100755
--- a/frappe/utils/background_jobs.py
+++ b/frappe/utils/background_jobs.py
@@ -3,13 +3,14 @@ import socket
import time
from collections import defaultdict
from functools import lru_cache
-from typing import TYPE_CHECKING, Any, Literal, NoReturn, Union
+from typing import Any, Callable, Literal, NoReturn
from uuid import uuid4
import redis
from redis.exceptions import BusyLoadingError, ConnectionError
from rq import Connection, Queue, Worker
from rq.exceptions import NoSuchJobError
+from rq.job import Job, JobStatus
from rq.logutils import setup_loghandlers
from rq.worker import RandomWorker, RoundRobinWorker
from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_fixed
@@ -22,10 +23,6 @@ from frappe.utils.commands import log
from frappe.utils.deprecations import deprecation_warning
from frappe.utils.redis_queue import RedisQueue
-if TYPE_CHECKING:
- from rq.job import Job
-
-
# TTL to keep RQ job logs in redis for.
RQ_JOB_FAILURE_TTL = 7 * 24 * 60 * 60 # 7 days instead of 1 year (default)
RQ_RESULTS_TTL = 10 * 60
@@ -54,21 +51,21 @@ redis_connection = None
def enqueue(
- method,
- queue="default",
- timeout=None,
- on_success=None,
- on_failure=None,
+ method: str | Callable,
+ queue: str = "default",
+ timeout: int | None = None,
event=None,
- is_async=True,
- job_name=None,
- now=False,
- enqueue_after_commit=False,
+ is_async: bool = True,
+ job_name: str | None = None,
+ now: bool = False,
+ enqueue_after_commit: bool = False,
*,
- at_front=False,
- job_id=None,
+ on_success: Callable = None,
+ on_failure: Callable = None,
+ at_front: bool = False,
+ job_id: str = None,
**kwargs,
-) -> Union["Job", Any]:
+) -> Job | Any:
"""
Enqueue method to be executed using a background worker
@@ -431,12 +428,15 @@ def create_job_id(job_id: str) -> str:
return f"{frappe.local.site}::{job_id}"
-def is_job_enqueued(job_id: str) -> str:
- from rq.job import Job
+def is_job_enqueued(job_id: str) -> bool:
+ return get_job_status(job_id) in (JobStatus.QUEUED, JobStatus.STARTED)
+
+def get_job_status(job_id: str) -> JobStatus | None:
+ """Get RQ job status, returns None if job is not found."""
try:
job = Job.fetch(create_job_id(job_id), connection=get_redis_conn())
except NoSuchJobError:
- return False
+ return None
- return job.get_status() in ("queued", "started")
+ return job.get_status()
From 6717b07ab9e4bc3720211f1dc6b527e6908fedb7 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 22:07:54 +0530
Subject: [PATCH 21/44] perf(customize_form): rebuild global search in bg
---
frappe/custom/doctype/customize_form/customize_form.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py
index 9e6b8990d5..9aa61869d3 100644
--- a/frappe/custom/doctype/customize_form/customize_form.py
+++ b/frappe/custom/doctype/customize_form/customize_form.py
@@ -192,7 +192,9 @@ class CustomizeForm(Document):
if self.flags.rebuild_doctype_for_global_search:
frappe.enqueue(
- "frappe.utils.global_search.rebuild_for_doctype", now=True, doctype=self.doc_type
+ "frappe.utils.global_search.rebuild_for_doctype",
+ doctype=self.doc_type,
+ enqueue_after_commit=True,
)
def set_property_setters(self):
From 339cbf208cf30e0fc544a6fa5796805be8ebc465 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 19:43:15 +0530
Subject: [PATCH 22/44] fix: Cache clearing implementation
---
frappe/model/document.py | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/frappe/model/document.py b/frappe/model/document.py
index 75c3a005c9..b8053d8076 100644
--- a/frappe/model/document.py
+++ b/frappe/model/document.py
@@ -1122,8 +1122,15 @@ class Document(BaseDocument):
frappe.flags.currently_saving.remove((self.doctype, self.name))
def clear_cache(self):
+ # TODO: Remove this call after verifying,
+ # after commit call alone should be enough.
frappe.clear_document_cache(self.doctype, self.name)
+ # There's a possibility that another worker might read data after clearing cache and before
+ # changes are commited to DB, in which case stale doc can be read and stored in DB. So
+ # clear cache after commiting.
+ frappe.db.after_commit.add(lambda: frappe.clear_document_cache(self.doctype, self.name))
+
def reset_seen(self):
"""Clear _seen property and set current user as seen"""
if getattr(self.meta, "track_seen", False):
From be1da0dd00bd3fb81ed65327f26390c0458699aa Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 18:53:24 +0530
Subject: [PATCH 23/44] chore: remove duplicate cache clearing
---
frappe/__init__.py | 1 -
frappe/desk/doctype/list_view_settings/list_view_settings.py | 3 +--
frappe/model/document.py | 1 -
3 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index b13c9230b3..507d174a8b 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -1116,7 +1116,6 @@ def get_document_cache_key(doctype: str, name: str):
def clear_document_cache(doctype, name):
- cache().hdel("last_modified", doctype)
cache().hdel("document_cache", get_document_cache_key(doctype, name))
if doctype == "System Settings" and hasattr(local, "system_settings"):
diff --git a/frappe/desk/doctype/list_view_settings/list_view_settings.py b/frappe/desk/doctype/list_view_settings/list_view_settings.py
index 36ebce34d5..e3b6a60a42 100644
--- a/frappe/desk/doctype/list_view_settings/list_view_settings.py
+++ b/frappe/desk/doctype/list_view_settings/list_view_settings.py
@@ -6,8 +6,7 @@ from frappe.model.document import Document
class ListViewSettings(Document):
- def on_update(self):
- frappe.clear_document_cache(self.doctype, self.name)
+ pass
@frappe.whitelist()
diff --git a/frappe/model/document.py b/frappe/model/document.py
index b8053d8076..92379f407c 100644
--- a/frappe/model/document.py
+++ b/frappe/model/document.py
@@ -1207,7 +1207,6 @@ class Document(BaseDocument):
if notify:
self.notify_update()
- self.clear_cache()
if commit:
frappe.db.commit()
From 106ff1f1ee519701627540f54861a456ef0bc300 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 19:07:41 +0530
Subject: [PATCH 24/44] fix: move cache clearing away from document
Passing lambda function from inside document object would keep reference
to document alive. This means increasing memeory usage in bulk
processing.
Refer https://github.com/frappe/frappe/pull/17061 for example
This also extends it to db.set_value
---
frappe/__init__.py | 7 ++++++-
frappe/model/document.py | 7 -------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index 507d174a8b..80e1cb0235 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -1116,7 +1116,12 @@ def get_document_cache_key(doctype: str, name: str):
def clear_document_cache(doctype, name):
- cache().hdel("document_cache", get_document_cache_key(doctype, name))
+ def clear_in_redis():
+ cache().hdel("document_cache", get_document_cache_key(doctype, name))
+
+ clear_in_redis()
+ if hasattr(db, "after_commit"):
+ db.after_commit.add(clear_in_redis)
if doctype == "System Settings" and hasattr(local, "system_settings"):
delattr(local, "system_settings")
diff --git a/frappe/model/document.py b/frappe/model/document.py
index 92379f407c..f944b28a49 100644
--- a/frappe/model/document.py
+++ b/frappe/model/document.py
@@ -1122,15 +1122,8 @@ class Document(BaseDocument):
frappe.flags.currently_saving.remove((self.doctype, self.name))
def clear_cache(self):
- # TODO: Remove this call after verifying,
- # after commit call alone should be enough.
frappe.clear_document_cache(self.doctype, self.name)
- # There's a possibility that another worker might read data after clearing cache and before
- # changes are commited to DB, in which case stale doc can be read and stored in DB. So
- # clear cache after commiting.
- frappe.db.after_commit.add(lambda: frappe.clear_document_cache(self.doctype, self.name))
-
def reset_seen(self):
"""Clear _seen property and set current user as seen"""
if getattr(self.meta, "track_seen", False):
From 356a2587e2a68a53a0864991b8684a0b1d5a8110 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 19:43:24 +0530
Subject: [PATCH 25/44] refactor: Use plain keys instead of hashes for caching
- Hashes are supposed to be used for representing complex object, not
multiple documents of same DocType.
- Redis's auto cache clearing wont clear individual key from hashes even if they
are rarely used.
- Keys can have expiry.
---
frappe/__init__.py | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index 80e1cb0235..c512de0de6 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -1075,7 +1075,7 @@ def set_value(doctype, docname, fieldname, value=None):
def get_cached_doc(*args, **kwargs) -> "Document":
- if (key := can_cache_doc(args)) and (doc := cache().hget("document_cache", key)):
+ if (key := can_cache_doc(args)) and (doc := cache().get_value(key)):
return doc
# Not found in cache, fetch from DB
@@ -1091,7 +1091,7 @@ def get_cached_doc(*args, **kwargs) -> "Document":
def _set_document_in_cache(key: str, doc: "Document") -> None:
- cache().hset("document_cache", key, doc)
+ cache().set_value(key, doc)
def can_cache_doc(args) -> str | None:
@@ -1112,12 +1112,12 @@ def can_cache_doc(args) -> str | None:
def get_document_cache_key(doctype: str, name: str):
- return f"{doctype}::{name}"
+ return f"document_cache::{doctype}::{name}"
def clear_document_cache(doctype, name):
def clear_in_redis():
- cache().hdel("document_cache", get_document_cache_key(doctype, name))
+ cache().delete_value(get_document_cache_key(doctype, name))
clear_in_redis()
if hasattr(db, "after_commit"):
@@ -1206,7 +1206,7 @@ def get_doc(*args, **kwargs):
doc = frappe.model.document.get_doc(*args, **kwargs)
# Replace cache if stale one exists
- if (key := can_cache_doc(args)) and cache().hexists("document_cache", key):
+ if (key := can_cache_doc(args)) and cache().exists(key):
_set_document_in_cache(key, doc)
return doc
From 98b4693dcf869e32632e41e29ed2051e9cec65df Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 19:48:02 +0530
Subject: [PATCH 26/44] perf: finer cache eviction on db.set_value
Instead of nuking everything, just clear matching prefix
---
frappe/__init__.py | 7 +++++--
frappe/cache_manager.py | 3 ++-
frappe/database/database.py | 6 ++----
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index c512de0de6..b31377a563 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -1115,9 +1115,12 @@ def get_document_cache_key(doctype: str, name: str):
return f"document_cache::{doctype}::{name}"
-def clear_document_cache(doctype, name):
+def clear_document_cache(doctype: str, name: str | None = None) -> None:
def clear_in_redis():
- cache().delete_value(get_document_cache_key(doctype, name))
+ if name is not None:
+ cache().delete_value(get_document_cache_key(doctype, name))
+ else:
+ cache().delete_keys(get_document_cache_key(doctype, ""))
clear_in_redis()
if hasattr(db, "after_commit"):
diff --git a/frappe/cache_manager.py b/frappe/cache_manager.py
index 12e829ff09..eee703bcf9 100644
--- a/frappe/cache_manager.py
+++ b/frappe/cache_manager.py
@@ -125,8 +125,9 @@ def clear_doctype_cache(doctype=None):
clear_controller_cache(doctype)
cache = frappe.cache()
- for key in ("is_table", "doctype_modules", "document_cache"):
+ for key in ("is_table", "doctype_modules"):
cache.delete_value(key)
+ cache.delete_keys("document_cache")
def clear_single(dt):
for name in doctype_cache_keys:
diff --git a/frappe/database/database.py b/frappe/database/database.py
index fc4f7b3b1b..2bac5a1ffc 100644
--- a/frappe/database/database.py
+++ b/frappe/database/database.py
@@ -920,10 +920,8 @@ class Database:
if isinstance(dn, str):
frappe.clear_document_cache(dt, dn)
else:
- # TODO: Fix this; doesn't work rn - gavin@frappe.io
- # frappe.cache().hdel_keys(dt, "document_cache")
- # Workaround: clear all document caches
- frappe.cache().delete_value("document_cache")
+ # No way to guess which documents are modified, clear all of them
+ frappe.clear_document_cache(dt)
for column, value in to_update.items():
query = query.set(column, value)
From 4193a251a5fe8def2665dc73047a04bc1f25a3aa Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 20:20:38 +0530
Subject: [PATCH 27/44] fix: Invalidate cache on rollback too
Steps:
- Document modified
- Document refetched from cache
- Transaction rolled back
- Cache now contains unmodified changes.
---
frappe/__init__.py | 1 +
frappe/tests/test_caching.py | 47 ++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index b31377a563..8a2a4513d4 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -1125,6 +1125,7 @@ def clear_document_cache(doctype: str, name: str | None = None) -> None:
clear_in_redis()
if hasattr(db, "after_commit"):
db.after_commit.add(clear_in_redis)
+ db.after_rollback.add(clear_in_redis)
if doctype == "System Settings" and hasattr(local, "system_settings"):
delattr(local, "system_settings")
diff --git a/frappe/tests/test_caching.py b/frappe/tests/test_caching.py
index 37f1583097..b9f7c66887 100644
--- a/frappe/tests/test_caching.py
+++ b/frappe/tests/test_caching.py
@@ -163,3 +163,50 @@ class TestRedisCache(FrappeAPITestCase):
calculate_area(radius=10)
# kwargs should hit cache too
self.assertEqual(function_call_count, 4)
+
+
+class TestDocumentCache(FrappeAPITestCase):
+ TEST_DOCTYPE = "User"
+ TEST_DOCNAME = "Administrator"
+ TEST_FIELD = "middle_name"
+
+ def setUp(self) -> None:
+ self.test_value = frappe.generate_hash()
+
+ def test_caching(self):
+ doc = frappe.get_cached_doc(self.TEST_DOCTYPE, self.TEST_DOCNAME)
+
+ with self.assertQueryCount(0):
+ doc = frappe.get_cached_doc(self.TEST_DOCTYPE, self.TEST_DOCNAME)
+
+ doc.db_set(self.TEST_FIELD, self.test_value)
+ new_doc = frappe.get_cached_doc(self.TEST_DOCTYPE, self.TEST_DOCNAME)
+
+ self.assertIsNot(doc, new_doc) # Shouldn't be same object from frappe.local
+ self.assertEqual(new_doc.get(self.TEST_FIELD), self.test_value) # Cache invalidated and fetched
+ frappe.db.rollback()
+
+ doc_after_rollback = frappe.get_cached_doc(self.TEST_DOCTYPE, self.TEST_DOCNAME)
+ self.assertIsNot(new_doc, doc_after_rollback)
+ # Cache invalidated after rollback
+ self.assertNotEqual(doc_after_rollback.get(self.TEST_FIELD), self.test_value)
+
+ with self.assertQueryCount(0):
+ frappe.get_cached_doc(self.TEST_DOCTYPE, self.TEST_DOCNAME)
+
+ def test_cache_invalidation_set_value(self):
+ doc = frappe.get_cached_doc(self.TEST_DOCTYPE, self.TEST_DOCNAME)
+
+ frappe.db.set_value(
+ self.TEST_DOCTYPE,
+ {"name": ("like", "%Admin%")},
+ self.TEST_FIELD,
+ self.test_value,
+ )
+
+ new_doc = frappe.get_cached_doc(self.TEST_DOCTYPE, self.TEST_DOCNAME)
+ self.assertIsNot(doc, new_doc)
+ self.assertEqual(new_doc.get(self.TEST_FIELD), self.test_value)
+
+ with self.assertQueryCount(0):
+ frappe.get_cached_doc(self.TEST_DOCTYPE, self.TEST_DOCNAME)
From 7d50ef19d3a366c41cafb752e0d1ff12d69e182a Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 21:25:04 +0530
Subject: [PATCH 28/44] perf: Delete multiple keys in O(1) redis calls
Currently we call redis for each key, redis already supports deleting
multiple keys in one go.
---
frappe/tests/test_caching.py | 15 +++++++++++++++
frappe/utils/redis_wrapper.py | 20 +++++++++++---------
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/frappe/tests/test_caching.py b/frappe/tests/test_caching.py
index b9f7c66887..c71f116649 100644
--- a/frappe/tests/test_caching.py
+++ b/frappe/tests/test_caching.py
@@ -210,3 +210,18 @@ class TestDocumentCache(FrappeAPITestCase):
with self.assertQueryCount(0):
frappe.get_cached_doc(self.TEST_DOCTYPE, self.TEST_DOCNAME)
+
+
+class TestRedisWrapper(FrappeAPITestCase):
+ def test_delete_keys(self):
+
+ c = frappe.cache()
+
+ prefix = "test_del_"
+
+ for i in range(5):
+ c.set_value(f"{prefix}{i}", 1)
+
+ self.assertEqual(len(c.get_keys(prefix)), 5)
+ c.delete_keys(prefix)
+ self.assertEqual(len(c.get_keys(prefix)), 0)
diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py
index 3b335b2c1d..eb3ea3707e 100644
--- a/frappe/utils/redis_wrapper.py
+++ b/frappe/utils/redis_wrapper.py
@@ -127,20 +127,22 @@ class RedisWrapper(redis.Redis):
def delete_value(self, keys, user=None, make_keys=True, shared=False):
"""Delete value, list of values."""
+ if not keys:
+ return
+
if not isinstance(keys, (list, tuple)):
keys = (keys,)
+ if make_keys:
+ keys = [self.make_key(k, shared=shared, user=user) for k in keys]
+
for key in keys:
- if make_keys:
- key = self.make_key(key, shared=shared)
+ frappe.local.cache.pop(key, None)
- if key in frappe.local.cache:
- del frappe.local.cache[key]
-
- try:
- self.delete(key)
- except redis.exceptions.ConnectionError:
- pass
+ try:
+ self.delete(*keys)
+ except redis.exceptions.ConnectionError:
+ pass
def lpush(self, key, value):
super().lpush(self.make_key(key), value)
From 84ef2cc89ce141b12c14a77b5dd3926faee114b6 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 21:54:05 +0530
Subject: [PATCH 29/44] fix: reliable cache clearing for doctype
reclear cache after commit to prevent stale caching.
---
frappe/cache_manager.py | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/frappe/cache_manager.py b/frappe/cache_manager.py
index eee703bcf9..9c1754148a 100644
--- a/frappe/cache_manager.py
+++ b/frappe/cache_manager.py
@@ -123,13 +123,21 @@ def clear_defaults_cache(user=None):
def clear_doctype_cache(doctype=None):
clear_controller_cache(doctype)
+
+ _clear_doctype_cache_form_redis()
+ if hasattr(frappe.db, "after_commit"):
+ frappe.db.after_commit.add(_clear_doctype_cache_form_redis)
+ frappe.db.after_rollback.add(_clear_doctype_cache_form_redis)
+
+
+def _clear_doctype_cache_form_redis(doctype: str | None = None):
cache = frappe.cache()
for key in ("is_table", "doctype_modules"):
cache.delete_value(key)
- cache.delete_keys("document_cache")
def clear_single(dt):
+ frappe.clear_document_cache(dt)
for name in doctype_cache_keys:
cache.hdel(name, dt)
@@ -156,6 +164,7 @@ def clear_doctype_cache(doctype=None):
# clear all
for name in doctype_cache_keys:
cache.delete_value(name)
+ cache.delete_keys("document_cache::")
def clear_controller_cache(doctype=None):
From 0d056a3a2bc4f7e1ce9545eeb3ec00fa942ef042 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 22:23:11 +0530
Subject: [PATCH 30/44] test: fix broken tests
Fixture test:
This is broken cause it's trying to find doctype after it has been
deleted (wut?)
It was working so far because cache wasn't cleared correctly so you'd
still find it from cache.
db.set_value test:
converted to use last query instead of patching SQL
---
frappe/tests/test_db.py | 25 ++++++++++++-------------
frappe/tests/test_fixture_import.py | 8 +++++---
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py
index e27d1db0ba..067d6c49b0 100644
--- a/frappe/tests/test_db.py
+++ b/frappe/tests/test_db.py
@@ -796,21 +796,20 @@ class TestDBSetValue(FrappeTestCase):
def test_set_value(self):
self.todo1.reload()
- with patch.object(Database, "sql") as sql_called:
- frappe.db.set_value(
- self.todo1.doctype,
- self.todo1.name,
- "description",
- f"{self.todo1.description}-edit by `test_for_update`",
- )
- first_query = sql_called.call_args_list[0].args[0]
+ frappe.db.set_value(
+ self.todo1.doctype,
+ self.todo1.name,
+ "description",
+ f"{self.todo1.description}-edit by `test_for_update`",
+ )
+ query = frappe.db.last_query
- if frappe.conf.db_type == "postgres":
- from frappe.database.postgres.database import modify_query
+ if frappe.conf.db_type == "postgres":
+ from frappe.database.postgres.database import modify_query
- self.assertTrue(modify_query("UPDATE `tabToDo` SET") in first_query)
- if frappe.conf.db_type == "mariadb":
- self.assertTrue("UPDATE `tabToDo` SET" in first_query)
+ self.assertTrue(modify_query("UPDATE `tabToDo` SET") in query)
+ if frappe.conf.db_type == "mariadb":
+ self.assertTrue("UPDATE `tabToDo` SET" in query)
def test_cleared_cache(self):
self.todo2.reload()
diff --git a/frappe/tests/test_fixture_import.py b/frappe/tests/test_fixture_import.py
index b9bd4550b2..8e4fa16763 100644
--- a/frappe/tests/test_fixture_import.py
+++ b/frappe/tests/test_fixture_import.py
@@ -69,10 +69,12 @@ class TestFixtureImport(FrappeTestCase):
import_doc(path_to_exported_fixtures)
- delete_doc("DocType", "temp_singles", delete_permanently=True)
- os.remove(path_to_exported_fixtures)
-
data = frappe.db.get_single_value("temp_singles", "member_name")
truncate_query.run()
self.assertEqual(data, dummy_name_list[0])
+
+ delete_doc("DocType", "temp_singles", delete_permanently=True)
+ os.remove(path_to_exported_fixtures)
+
+ frappe.db.commit()
From 26722b1a1cd63a456e23406c6abe3acfee12b406 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sat, 3 Jun 2023 22:31:49 +0530
Subject: [PATCH 31/44] fix: ignore ConnectionError in
`frappe.cache().exists()`
---
frappe/tests/test_db.py | 2 +-
frappe/utils/redis_wrapper.py | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py
index 067d6c49b0..afc24ecf68 100644
--- a/frappe/tests/test_db.py
+++ b/frappe/tests/test_db.py
@@ -802,7 +802,7 @@ class TestDBSetValue(FrappeTestCase):
"description",
f"{self.todo1.description}-edit by `test_for_update`",
)
- query = frappe.db.last_query
+ query = str(frappe.db.last_query)
if frappe.conf.db_type == "postgres":
from frappe.database.postgres.database import modify_query
diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py
index eb3ea3707e..70280e4bf7 100644
--- a/frappe/utils/redis_wrapper.py
+++ b/frappe/utils/redis_wrapper.py
@@ -199,7 +199,11 @@ class RedisWrapper(redis.Redis):
def exists(self, *names: str, user=None, shared=None) -> int:
names = [self.make_key(n, user=user, shared=shared) for n in names]
- return super().exists(*names)
+
+ try:
+ return super().exists(*names)
+ except redis.exceptions.ConnectionError:
+ return False
def hgetall(self, name):
value = super().hgetall(self.make_key(name))
From 65a2cdcffc2c4e71d8e0700c741f3da61a97c101 Mon Sep 17 00:00:00 2001
From: Suraj Shetty
Date: Sun, 4 Jun 2023 08:59:08 +0530
Subject: [PATCH 32/44] fix(safe_eval): Normalize code passed before validating
the code
---
frappe/__init__.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index e5a0b9c4aa..68797d2f06 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -16,6 +16,7 @@ import inspect
import json
import os
import re
+import unicodedata
import warnings
from typing import TYPE_CHECKING, Any, Callable, Literal, Optional, TypeAlias, overload
@@ -2271,6 +2272,7 @@ def bold(text):
def safe_eval(code, eval_globals=None, eval_locals=None):
"""A safer `eval`"""
whitelisted_globals = {"int": int, "float": float, "long": int, "round": round}
+ code = unicodedata.normalize("NFKC", code)
UNSAFE_ATTRIBUTES = {
# Generator Attributes
From e30b823eeb2ff0819460eed9fe81e08159a0a310 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Fri, 2 Jun 2023 17:00:40 +0530
Subject: [PATCH 33/44] fix!: Webhook naming should be prompt
---
.../test_document_naming_settings.py | 31 +++++++++++++++----
.../doctype/webhook/test_webhook.py | 7 ++++-
.../integrations/doctype/webhook/webhook.json | 15 +++------
3 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py b/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py
index 98ce9e738b..bcd3197112 100644
--- a/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py
+++ b/frappe/core/doctype/document_naming_settings/test_document_naming_settings.py
@@ -2,6 +2,7 @@
# See license.txt
import frappe
+from frappe.core.doctype.doctype.test_doctype import new_doctype
from frappe.core.doctype.document_naming_settings.document_naming_settings import (
DocumentNamingSettings,
)
@@ -11,6 +12,25 @@ from frappe.utils import cint
class TestNamingSeries(FrappeTestCase):
+ @classmethod
+ def setUpClass(cls):
+ super().setUpClass()
+ cls.ns_doctype = (
+ new_doctype(
+ fields=[
+ {
+ "label": "Series",
+ "fieldname": "naming_series",
+ "fieldtype": "Select",
+ "options": f"\n{frappe.generate_hash()}-.###",
+ }
+ ],
+ autoname="naming_series:",
+ )
+ .insert()
+ .name
+ )
+
def setUp(self):
self.dns: DocumentNamingSettings = frappe.get_doc("Document Naming Settings")
@@ -23,7 +43,7 @@ class TestNamingSeries(FrappeTestCase):
return VALID_SERIES + exisiting_series
def test_naming_preview(self):
- self.dns.transaction_type = "Webhook"
+ self.dns.transaction_type = self.ns_doctype
self.dns.try_naming_series = "AXBZ.####"
serieses = self.dns.preview_series().split("\n")
@@ -35,23 +55,22 @@ class TestNamingSeries(FrappeTestCase):
def test_get_transactions(self):
naming_info = self.dns.get_transactions_and_prefixes()
- self.assertIn("Webhook", naming_info["transactions"])
+ self.assertIn(self.ns_doctype, naming_info["transactions"])
- existing_naming_series = frappe.get_meta("Webhook").get_field("naming_series").options
+ existing_naming_series = frappe.get_meta(self.ns_doctype).get_field("naming_series").options
for series in existing_naming_series.split("\n"):
self.assertIn(NamingSeries(series).get_prefix(), naming_info["prefixes"])
def test_default_naming_series(self):
- self.assertIn("HOOK", get_default_naming_series("Webhook"))
self.assertIsNone(get_default_naming_series("DocType"))
def test_updates_naming_options(self):
- self.dns.transaction_type = "Webhook"
+ self.dns.transaction_type = self.ns_doctype
test_series = "KOOHBEW.###"
self.dns.naming_series_options = self.dns.get_options() + "\n" + test_series
self.dns.update_series()
- self.assertIn(test_series, frappe.get_meta("Webhook").get_naming_series_options())
+ self.assertIn(test_series, frappe.get_meta(self.ns_doctype).get_naming_series_options())
def test_update_series_counter(self):
for series in self.get_valid_serieses():
diff --git a/frappe/integrations/doctype/webhook/test_webhook.py b/frappe/integrations/doctype/webhook/test_webhook.py
index 7235447662..2edf2fcf5c 100644
--- a/frappe/integrations/doctype/webhook/test_webhook.py
+++ b/frappe/integrations/doctype/webhook/test_webhook.py
@@ -14,7 +14,10 @@ from frappe.tests.utils import FrappeTestCase
@contextmanager
def get_test_webhook(config):
- wh = frappe.get_doc(config).insert()
+ wh = frappe.get_doc(config)
+ if not wh.name:
+ wh.name = frappe.generate_hash()
+ wh.insert()
wh.reload()
try:
yield wh
@@ -37,6 +40,7 @@ class TestWebhook(FrappeTestCase):
def create_sample_webhooks(cls):
samples_webhooks_data = [
{
+ "name": frappe.generate_hash(),
"webhook_doctype": "User",
"webhook_docevent": "after_insert",
"request_url": "https://httpbin.org/post",
@@ -44,6 +48,7 @@ class TestWebhook(FrappeTestCase):
"enabled": True,
},
{
+ "name": frappe.generate_hash(),
"webhook_doctype": "User",
"webhook_docevent": "after_insert",
"request_url": "https://httpbin.org/post",
diff --git a/frappe/integrations/doctype/webhook/webhook.json b/frappe/integrations/doctype/webhook/webhook.json
index cfb2a2e01c..404e0be944 100644
--- a/frappe/integrations/doctype/webhook/webhook.json
+++ b/frappe/integrations/doctype/webhook/webhook.json
@@ -1,13 +1,12 @@
{
"actions": [],
- "autoname": "naming_series:",
+ "autoname": "prompt",
"creation": "2017-09-08 16:16:13.060641",
"doctype": "DocType",
"editable_grid": 1,
"engine": "InnoDB",
"field_order": [
"sb_doc_events",
- "naming_series",
"webhook_doctype",
"cb_doc_events",
"webhook_docevent",
@@ -46,6 +45,7 @@
{
"fieldname": "webhook_doctype",
"fieldtype": "Link",
+ "in_list_view": 1,
"label": "DocType",
"options": "DocType",
"reqd": 1,
@@ -136,12 +136,6 @@
"label": "JSON Request Body",
"options": "JSON"
},
- {
- "fieldname": "naming_series",
- "fieldtype": "Select",
- "label": "Naming Series",
- "options": "\nHOOK-.####"
- },
{
"fieldname": "sb_security",
"fieldtype": "Section Break",
@@ -218,11 +212,11 @@
"link_fieldname": "webhook"
}
],
- "modified": "2023-05-22 16:30:10.740512",
+ "modified": "2023-06-02 17:25:12.598232",
"modified_by": "Administrator",
"module": "Integrations",
"name": "Webhook",
- "naming_rule": "By \"Naming Series\" field",
+ "naming_rule": "Set by user",
"owner": "Administrator",
"permissions": [
{
@@ -241,6 +235,5 @@
"sort_field": "modified",
"sort_order": "DESC",
"states": [],
- "title_field": "webhook_doctype",
"track_changes": 1
}
\ No newline at end of file
From 7c7c11b4541a27a265a6895a0ed6fbde4630a6bd Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sun, 4 Jun 2023 12:31:07 +0530
Subject: [PATCH 34/44] perf: Cache web view routes
Each call to evaluate if route is web view makes N queries where N = #
of web view doctypes. This entire computation can be definitely cached
for short duration.
- Added cache bursting in WebsiteGenerator doctype updates.
- Added 60 minutes TTL in case cache invalidation wasn't done reliably.
---
frappe/__init__.py | 3 ++
.../website/page_renderers/document_page.py | 46 ++++++++++++-------
frappe/website/utils.py | 4 ++
3 files changed, 36 insertions(+), 17 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index e081a65e7b..9c40d56f07 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -876,6 +876,7 @@ def clear_cache(user: str | None = None, doctype: str | None = None):
:param doctype: If doctype is given, only DocType cache is cleared."""
import frappe.cache_manager
import frappe.utils.caching
+ from frappe.website.page_renderers.document_page import clear_routing_cache
if doctype:
frappe.cache_manager.clear_doctype_cache(doctype)
@@ -904,6 +905,8 @@ def clear_cache(user: str | None = None, doctype: str | None = None):
if hasattr(local, "website_settings"):
del local.website_settings
+ clear_routing_cache()
+
def only_has_select_perm(doctype, user=None, ignore_permissions=False):
if ignore_permissions:
diff --git a/frappe/website/page_renderers/document_page.py b/frappe/website/page_renderers/document_page.py
index abfd72ac6f..1d8ce82317 100644
--- a/frappe/website/page_renderers/document_page.py
+++ b/frappe/website/page_renderers/document_page.py
@@ -1,5 +1,6 @@
import frappe
from frappe.model.document import get_controller
+from frappe.utils.caching import redis_cache
from frappe.website.page_renderers.base_template_page import BaseTemplatePage
from frappe.website.router import (
get_doctypes_with_web_view,
@@ -22,22 +23,9 @@ class DocumentPage(BaseTemplatePage):
return False
def search_in_doctypes_with_web_view(self):
- for doctype in get_doctypes_with_web_view():
- filters = dict(route=self.path)
- meta = frappe.get_meta(doctype)
- condition_field = self.get_condition_field(meta)
-
- if condition_field:
- filters[condition_field] = 1
-
- try:
- self.docname = frappe.db.get_value(doctype, filters, "name")
- if self.docname:
- self.doctype = doctype
- return True
- except Exception as e:
- if not frappe.db.is_missing_column(e):
- raise e
+ if document := _find_matching_document_webview(self.path):
+ self.doctype, self.docname = document
+ return True
def search_web_page_dynamic_routes(self):
d = get_page_info_from_web_page_with_dynamic_routes(self.path)
@@ -83,7 +71,8 @@ class DocumentPage(BaseTemplatePage):
if prop not in self.context:
self.context[prop] = getattr(self.doc, prop, False)
- def get_condition_field(self, meta):
+ @staticmethod
+ def get_condition_field(meta):
condition_field = None
if meta.is_published_field:
condition_field = meta.is_published_field
@@ -92,3 +81,26 @@ class DocumentPage(BaseTemplatePage):
condition_field = controller.website.condition_field
return condition_field
+
+
+@redis_cache(ttl=60 * 60)
+def _find_matching_document_webview(route: str) -> tuple[str, str] | None:
+ for doctype in get_doctypes_with_web_view():
+ filters = dict(route=route)
+ meta = frappe.get_meta(doctype)
+ condition_field = DocumentPage.get_condition_field(meta)
+
+ if condition_field:
+ filters[condition_field] = 1
+
+ try:
+ docname = frappe.db.get_value(doctype, filters, "name")
+ if docname:
+ return (doctype, docname)
+ except Exception as e:
+ if not frappe.db.is_missing_column(e):
+ raise e
+
+
+def clear_routing_cache():
+ _find_matching_document_webview.clear_cache()
diff --git a/frappe/website/utils.py b/frappe/website/utils.py
index 71af463c96..66b4198256 100644
--- a/frappe/website/utils.py
+++ b/frappe/website/utils.py
@@ -360,9 +360,13 @@ def get_html_content_based_on_type(doc, fieldname, content_type):
def clear_cache(path=None):
"""Clear website caches
:param path: (optional) for the given path"""
+ from frappe.website.page_renderers.document_page import clear_routing_cache
+
for key in ("website_generator_routes", "website_pages", "website_full_index", "sitemap_routes"):
frappe.cache().delete_value(key)
+ clear_routing_cache()
+
frappe.cache().delete_value("website_404")
if path:
frappe.cache().hdel("website_redirects", path)
From c94c0591c338f41df97a7020e754777ea464b948 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sun, 4 Jun 2023 12:47:09 +0530
Subject: [PATCH 35/44] perf: cache dynamic web pages
---
frappe/website/doctype/web_page/web_page.py | 16 +++++++++++-----
frappe/website/page_renderers/document_page.py | 3 +++
frappe/website/router.py | 7 +++----
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/frappe/website/doctype/web_page/web_page.py b/frappe/website/doctype/web_page/web_page.py
index 9a16654085..2f89252626 100644
--- a/frappe/website/doctype/web_page/web_page.py
+++ b/frappe/website/doctype/web_page/web_page.py
@@ -8,6 +8,7 @@ from jinja2.exceptions import TemplateSyntaxError
import frappe
from frappe import _
from frappe.utils import get_datetime, now, quoted, strip_html
+from frappe.utils.caching import redis_cache
from frappe.utils.jinja import render_template
from frappe.utils.safe_exec import safe_exec
from frappe.website.doctype.website_slideshow.website_slideshow import get_slideshow
@@ -30,11 +31,9 @@ class WebPage(WebsiteGenerator):
if not self.dynamic_route:
self.route = quoted(self.route)
- def on_update(self):
- super().on_update()
-
- def on_trash(self):
- super().on_trash()
+ def clear_cache(self):
+ super().clear_cache()
+ get_dynamic_web_pages.clear_cache()
def get_context(self, context):
context.main_section = get_html_content_based_on_type(self, "main_section", self.content_type)
@@ -247,3 +246,10 @@ def extract_script_and_style_tags(html):
style.extract()
return str(soup), scripts, styles
+
+
+@redis_cache(ttl=60 * 60)
+def get_dynamic_web_pages() -> dict[str, str]:
+ return frappe.get_all(
+ "Web Page", fields=["name", "route", "modified"], filters=dict(published=1, dynamic_route=1)
+ )
diff --git a/frappe/website/page_renderers/document_page.py b/frappe/website/page_renderers/document_page.py
index 1d8ce82317..6c760fa0b0 100644
--- a/frappe/website/page_renderers/document_page.py
+++ b/frappe/website/page_renderers/document_page.py
@@ -103,4 +103,7 @@ def _find_matching_document_webview(route: str) -> tuple[str, str] | None:
def clear_routing_cache():
+ from frappe.website.doctype.web_page.web_page import get_dynamic_web_pages
+
_find_matching_document_webview.clear_cache()
+ get_dynamic_web_pages.clear_cache()
diff --git a/frappe/website/router.py b/frappe/website/router.py
index 655fcc1357..4493d437b8 100644
--- a/frappe/website/router.py
+++ b/frappe/website/router.py
@@ -16,12 +16,11 @@ def get_page_info_from_web_page_with_dynamic_routes(path):
"""
Query Web Page with dynamic_route = 1 and evaluate if any of the routes match
"""
+ from frappe.website.doctype.web_page.web_page import get_dynamic_web_pages
+
rules, page_info = [], {}
- # build rules from all web page with `dynamic_route = 1`
- for d in frappe.get_all(
- "Web Page", fields=["name", "route", "modified"], filters=dict(published=1, dynamic_route=1)
- ):
+ for d in get_dynamic_web_pages():
rules.append(Rule("/" + d.route, endpoint=d.name))
d.doctype = "Web Page"
page_info[d.name] = d
From 392a506a76c0e8c8cf743b937fc3b402b4920dfc Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sun, 4 Jun 2023 12:57:56 +0530
Subject: [PATCH 36/44] perf: Cache published web forms
---
frappe/__init__.py | 2 +-
frappe/website/doctype/web_form/web_form.py | 9 ++++++---
frappe/website/doctype/web_page/web_page.py | 4 ----
frappe/website/page_renderers/document_page.py | 7 -------
frappe/website/router.py | 15 +++++++++++++--
frappe/website/utils.py | 2 +-
6 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/frappe/__init__.py b/frappe/__init__.py
index 9c40d56f07..ff2a663d50 100644
--- a/frappe/__init__.py
+++ b/frappe/__init__.py
@@ -876,7 +876,7 @@ def clear_cache(user: str | None = None, doctype: str | None = None):
:param doctype: If doctype is given, only DocType cache is cleared."""
import frappe.cache_manager
import frappe.utils.caching
- from frappe.website.page_renderers.document_page import clear_routing_cache
+ from frappe.website.router import clear_routing_cache
if doctype:
frappe.cache_manager.clear_doctype_cache(doctype)
diff --git a/frappe/website/doctype/web_form/web_form.py b/frappe/website/doctype/web_form/web_form.py
index 81c6001558..fd9949c45f 100644
--- a/frappe/website/doctype/web_form/web_form.py
+++ b/frappe/website/doctype/web_form/web_form.py
@@ -12,6 +12,7 @@ from frappe.desk.form.meta import get_code_files_via_hooks
from frappe.modules.utils import export_module_json, get_doc_module
from frappe.rate_limiter import rate_limit
from frappe.utils import cstr, dict_with_keys, strip_html
+from frappe.utils.caching import redis_cache
from frappe.website.utils import get_boot_data, get_comment_list, get_sidebar_items
from frappe.website.website_generator import WebsiteGenerator
@@ -19,9 +20,6 @@ from frappe.website.website_generator import WebsiteGenerator
class WebForm(WebsiteGenerator):
website = frappe._dict(no_cache=1)
- def onload(self):
- super().onload()
-
def validate(self):
super().validate()
@@ -639,3 +637,8 @@ def get_link_options(web_form_name, doctype, allow_read_on_all_link_options=Fals
raise frappe.PermissionError(
_("You don't have permission to access the {0} DocType.").format(doctype)
)
+
+
+@redis_cache(ttl=60 * 60)
+def get_published_web_forms() -> dict[str, str]:
+ return frappe.get_all("Web Form", ["name", "route", "modified"], {"published": 1})
diff --git a/frappe/website/doctype/web_page/web_page.py b/frappe/website/doctype/web_page/web_page.py
index 2f89252626..02e419001c 100644
--- a/frappe/website/doctype/web_page/web_page.py
+++ b/frappe/website/doctype/web_page/web_page.py
@@ -31,10 +31,6 @@ class WebPage(WebsiteGenerator):
if not self.dynamic_route:
self.route = quoted(self.route)
- def clear_cache(self):
- super().clear_cache()
- get_dynamic_web_pages.clear_cache()
-
def get_context(self, context):
context.main_section = get_html_content_based_on_type(self, "main_section", self.content_type)
context.source_content_type = self.content_type
diff --git a/frappe/website/page_renderers/document_page.py b/frappe/website/page_renderers/document_page.py
index 6c760fa0b0..54ee58ddb9 100644
--- a/frappe/website/page_renderers/document_page.py
+++ b/frappe/website/page_renderers/document_page.py
@@ -100,10 +100,3 @@ def _find_matching_document_webview(route: str) -> tuple[str, str] | None:
except Exception as e:
if not frappe.db.is_missing_column(e):
raise e
-
-
-def clear_routing_cache():
- from frappe.website.doctype.web_page.web_page import get_dynamic_web_pages
-
- _find_matching_document_webview.clear_cache()
- get_dynamic_web_pages.clear_cache()
diff --git a/frappe/website/router.py b/frappe/website/router.py
index 4493d437b8..98be1138e4 100644
--- a/frappe/website/router.py
+++ b/frappe/website/router.py
@@ -32,9 +32,10 @@ def get_page_info_from_web_page_with_dynamic_routes(path):
def get_page_info_from_web_form(path):
"""Query published web forms and evaluate if the route matches"""
+ from frappe.website.doctype.web_form.web_form import get_published_web_forms
+
rules, page_info = [], {}
- web_forms = frappe.get_all("Web Form", ["name", "route", "modified"], {"published": 1})
- for d in web_forms:
+ for d in get_published_web_forms():
rules.append(Rule(f"/{d.route}", endpoint=d.name))
rules.append(Rule(f"/{d.route}/list", endpoint=d.name))
rules.append(Rule(f"/{d.route}/new", endpoint=d.name))
@@ -314,3 +315,13 @@ def get_doctypes_with_web_view():
def get_start_folders():
return frappe.local.flags.web_pages_folders or ("www", "templates/pages")
+
+
+def clear_routing_cache():
+ from frappe.website.doctype.web_form.web_form import get_published_web_forms
+ from frappe.website.doctype.web_page.web_page import get_dynamic_web_pages
+ from frappe.website.page_renderers.document_page import _find_matching_document_webview
+
+ _find_matching_document_webview.clear_cache()
+ get_dynamic_web_pages.clear_cache()
+ get_published_web_forms.clear_cache()
diff --git a/frappe/website/utils.py b/frappe/website/utils.py
index 66b4198256..ff8c69639e 100644
--- a/frappe/website/utils.py
+++ b/frappe/website/utils.py
@@ -360,7 +360,7 @@ def get_html_content_based_on_type(doc, fieldname, content_type):
def clear_cache(path=None):
"""Clear website caches
:param path: (optional) for the given path"""
- from frappe.website.page_renderers.document_page import clear_routing_cache
+ from frappe.website.router import clear_routing_cache
for key in ("website_generator_routes", "website_pages", "website_full_index", "sitemap_routes"):
frappe.cache().delete_value(key)
From a7413fe4a7d7ef19a6d78dbf9d21eb464c7dbfa2 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Sun, 4 Jun 2023 13:02:16 +0530
Subject: [PATCH 37/44] chore: separate out func call key for @redis_cache
---
frappe/utils/caching.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/utils/caching.py b/frappe/utils/caching.py
index 007582f25f..370227ea72 100644
--- a/frappe/utils/caching.py
+++ b/frappe/utils/caching.py
@@ -150,7 +150,7 @@ def redis_cache(ttl: int | None = 3600, user: str | bool | None = None) -> Calla
@wraps(func)
def redis_cache_wrapper(*args, **kwargs):
- func_call_key = func_key + str(__generate_request_cache_key(args, kwargs))
+ func_call_key = func_key + "::" + str(__generate_request_cache_key(args, kwargs))
if frappe.cache().exists(func_call_key):
return frappe.cache().get_value(func_call_key, user=user)
else:
From 54fabab0105c79e3084629d34d5d40d6908226d6 Mon Sep 17 00:00:00 2001
From: gn306029
Date: Mon, 5 Jun 2023 15:04:17 +0800
Subject: [PATCH 38/44] fix: currency formatter got incorrect format when use
precision 0 (#21239)
* fix: currency formatter got incorrect format when use precision 0
* Revert "fix: currency formatter got incorrect format when use precision 0"
This reverts commit 1919cf4763b16e0cca2c2596223443d901e00e27.
* fix: allow 0 as default precision
---------
Co-authored-by: Ankush Menat
---
frappe/public/js/frappe/form/formatters.js | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/frappe/public/js/frappe/form/formatters.js b/frappe/public/js/frappe/form/formatters.js
index 9739eed8bb..637fd7063d 100644
--- a/frappe/public/js/frappe/form/formatters.js
+++ b/frappe/public/js/frappe/form/formatters.js
@@ -103,8 +103,9 @@ frappe.form.formatters = {
},
Currency: function (value, docfield, options, doc) {
var currency = frappe.meta.get_field_currency(docfield, doc);
- var precision =
- docfield.precision || cint(frappe.boot.sysdefaults.currency_precision) || 2;
+ var precision = cint(
+ docfield.precision ?? frappe.boot.sysdefaults.currency_precision ?? 2
+ );
// If you change anything below, it's going to hurt a company in UAE, a bit.
if (precision > 2) {
From 64613fec4835eca8a7afb9af5719971b826bf9e3 Mon Sep 17 00:00:00 2001
From: Shariq Ansari
Date: Mon, 5 Jun 2023 15:11:46 +0530
Subject: [PATCH 39/44] fix: changed section with card url field to small text
---
.../section_with_cards.json | 20 +++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/frappe/website/web_template/section_with_cards/section_with_cards.json b/frappe/website/web_template/section_with_cards/section_with_cards.json
index c891119f97..5501147d89 100644
--- a/frappe/website/web_template/section_with_cards/section_with_cards.json
+++ b/frappe/website/web_template/section_with_cards/section_with_cards.json
@@ -49,7 +49,7 @@
},
{
"fieldname": "card_1_url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
},
@@ -79,7 +79,7 @@
},
{
"fieldname": "card_2_url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
},
@@ -109,7 +109,7 @@
},
{
"fieldname": "card_3_url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
},
@@ -139,7 +139,7 @@
},
{
"fieldname": "card_4_url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
},
@@ -169,7 +169,7 @@
},
{
"fieldname": "card_5_url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
},
@@ -199,7 +199,7 @@
},
{
"fieldname": "card_6_url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
},
@@ -229,7 +229,7 @@
},
{
"fieldname": "card_7_url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
},
@@ -259,7 +259,7 @@
},
{
"fieldname": "card_8_url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
},
@@ -289,13 +289,13 @@
},
{
"fieldname": "card_9_url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
}
],
"idx": 0,
- "modified": "2021-05-03 13:26:34.470232",
+ "modified": "2023-06-05 13:26:34.470232",
"modified_by": "Administrator",
"module": "Website",
"name": "Section with Cards",
From 2358bd8fc1c9bf8ba2d89da56b259e7122974907 Mon Sep 17 00:00:00 2001
From: Shariq Ansari
Date: Mon, 5 Jun 2023 15:17:00 +0530
Subject: [PATCH 40/44] fix: changed for section with features and testimonial
web temlates
---
.../section_with_features/section_with_features.json | 4 ++--
.../section_with_testimonials/section_with_testimonials.json | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/frappe/website/web_template/section_with_features/section_with_features.json b/frappe/website/web_template/section_with_features/section_with_features.json
index a5734aa293..2683e92aae 100644
--- a/frappe/website/web_template/section_with_features/section_with_features.json
+++ b/frappe/website/web_template/section_with_features/section_with_features.json
@@ -43,7 +43,7 @@
},
{
"fieldname": "url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
},
@@ -55,7 +55,7 @@
}
],
"idx": 2,
- "modified": "2020-10-26 17:43:08.219285",
+ "modified": "2023-06-05 13:26:34.470232",
"modified_by": "Administrator",
"module": "Website",
"name": "Section with Features",
diff --git a/frappe/website/web_template/section_with_testimonials/section_with_testimonials.json b/frappe/website/web_template/section_with_testimonials/section_with_testimonials.json
index c1ba071be2..dd1d3bd0bd 100644
--- a/frappe/website/web_template/section_with_testimonials/section_with_testimonials.json
+++ b/frappe/website/web_template/section_with_testimonials/section_with_testimonials.json
@@ -56,13 +56,13 @@
},
{
"fieldname": "url",
- "fieldtype": "Data",
+ "fieldtype": "Small Text",
"label": "URL",
"reqd": 0
}
],
"idx": 0,
- "modified": "2022-03-21 15:39:39.044104",
+ "modified": "2023-06-05 13:26:34.470232",
"modified_by": "Administrator",
"module": "Website",
"name": "Section with Testimonials",
From ba9e2aab175bd185bddf54d7ac8859a228c46140 Mon Sep 17 00:00:00 2001
From: Shariq Ansari
Date: Mon, 5 Jun 2023 22:11:42 +0530
Subject: [PATCH 41/44] fix: avoid loading workspace content if already loaded
---
frappe/public/js/frappe/views/workspace/workspace.js | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/frappe/public/js/frappe/views/workspace/workspace.js b/frappe/public/js/frappe/views/workspace/workspace.js
index b5fb0e2e54..894844497b 100644
--- a/frappe/public/js/frappe/views/workspace/workspace.js
+++ b/frappe/public/js/frappe/views/workspace/workspace.js
@@ -22,6 +22,7 @@ frappe.views.Workspace = class Workspace {
this.page = wrapper.page;
this.blocks = frappe.workspace_block.blocks;
this.is_read_only = true;
+ this.is_page_loaded = false;
this.pages = {};
this.sorted_public_items = [];
this.sorted_private_items = [];
@@ -248,10 +249,14 @@ frappe.views.Workspace = class Workspace {
this.update_selected_sidebar(page, true); //add selected on new page
if (!frappe.router.current_route[0]) {
+ this.is_page_loaded = true;
frappe.set_route(frappe.router.slug(page.public ? page.name : "private/" + page.name));
}
- this.show_page(page);
+ if (!this.is_page_loaded) {
+ this.show_page(page);
+ this.is_page_loaded = false;
+ }
}
update_selected_sidebar(page, add) {
From 90fd74859200d6151ca7bbbbc9378aa23efc8044 Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Mon, 5 Jun 2023 22:27:01 +0530
Subject: [PATCH 42/44] fix(UX): Notify if newly created user has no roles
(#21251)
- System user with no role is useless
- By default adding a new system user doesn't give them ANY role so
they can't really access desk even if they have system user role.
---
frappe/core/doctype/user/user.json | 3 ++-
frappe/core/doctype/user/user.py | 16 ++++++++++++++++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/frappe/core/doctype/user/user.json b/frappe/core/doctype/user/user.json
index 654f20936e..0396776183 100644
--- a/frappe/core/doctype/user/user.json
+++ b/frappe/core/doctype/user/user.json
@@ -212,6 +212,7 @@
"read_only": 1
},
{
+ "allow_in_quick_entry": 1,
"fieldname": "role_profile_name",
"fieldtype": "Link",
"label": "Role Profile",
@@ -761,7 +762,7 @@
"link_fieldname": "user"
}
],
- "modified": "2023-05-24 15:20:06.434506",
+ "modified": "2023-06-05 17:26:04.127555",
"modified_by": "Administrator",
"module": "Core",
"name": "User",
diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py
index 94ea8b16a0..81d9715c32 100644
--- a/frappe/core/doctype/user/user.py
+++ b/frappe/core/doctype/user/user.py
@@ -75,6 +75,7 @@ class User(Document):
self.validate_email_type(self.email)
self.validate_email_type(self.name)
self.add_system_manager_role()
+ self.check_roles_added()
self.set_system_user()
self.set_full_name()
self.check_enable_disable()
@@ -673,6 +674,21 @@ class User(Document):
if not self.time_zone:
self.time_zone = get_system_timezone()
+ def check_roles_added(self):
+ if self.user_type != "System User" or self.roles or not self.is_new():
+ return
+
+ frappe.msgprint(
+ _("Newly created user {0} has no roles enabled.").format(frappe.bold(self.name)),
+ title=_("No Roles Specified"),
+ indicator="orange",
+ primary_action={
+ "label": _("Add Roles"),
+ "client_action": "frappe.set_route",
+ "args": ["Form", self.doctype, self.name],
+ },
+ )
+
@frappe.whitelist()
def get_timezones():
From 4f459f7146ee2dec1c3ba113554543fa251004b1 Mon Sep 17 00:00:00 2001
From: Shariq Ansari
Date: Tue, 6 Jun 2023 08:07:01 +0530
Subject: [PATCH 43/44] fix(unrelated): make create doctype using form builder
default button
---
frappe/core/doctype/doctype/doctype_list.js | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/frappe/core/doctype/doctype/doctype_list.js b/frappe/core/doctype/doctype/doctype_list.js
index c66edf1e21..f4811fa01d 100644
--- a/frappe/core/doctype/doctype/doctype_list.js
+++ b/frappe/core/doctype/doctype/doctype_list.js
@@ -6,16 +6,16 @@ frappe.listview_settings["DocType"] = {
setup_select_primary_button: function (me) {
let actions = [
- {
- label: __("Add DocType"),
- description: __("Create a new DocType"),
- action: () => frappe.new_doc("DocType"),
- },
{
label: __("Add DocType (Form Builder)"),
description: __("Use the form builder to create a new DocType"),
action: () => frappe.set_route("form-builder", "new-doctype"),
},
+ {
+ label: __("Add DocType"),
+ description: __("Create a new DocType"),
+ action: () => frappe.new_doc("DocType"),
+ },
];
frappe.utils.add_select_group_button(
From 34ee6a16776c060ea7add27bd3b0a5dc2b8c7e1f Mon Sep 17 00:00:00 2001
From: Ankush Menat
Date: Tue, 6 Jun 2023 16:11:02 +0530
Subject: [PATCH 44/44] fix: declare the function (#21261)
[skip ci]
---
frappe/desk/doctype/form_tour/form_tour.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/desk/doctype/form_tour/form_tour.js b/frappe/desk/doctype/form_tour/form_tour.js
index 8a65cc1619..390f519367 100644
--- a/frappe/desk/doctype/form_tour/form_tour.js
+++ b/frappe/desk/doctype/form_tour/form_tour.js
@@ -95,7 +95,7 @@ frappe.ui.form.on("Form Tour", {
},
});
-add_custom_button = (frm) => {
+let add_custom_button = (frm) => {
if (frm.doc.ui_tour) {
frm.add_custom_button(__("Reset"), function () {
frappe.confirm(