From fdafce9b63e79731a0b80944151c417b015c4b72 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 20 Dec 2022 14:44:05 +0100
Subject: [PATCH 01/20] fix: only System Manager can work with public Notes
---
frappe/desk/doctype/note/note.js | 33 +++++++++------------
frappe/desk/doctype/note/note.json | 41 ++++++++++++++++++++++----
frappe/desk/doctype/note/note.py | 47 +++++++++---------------------
frappe/hooks.py | 2 --
4 files changed, 63 insertions(+), 60 deletions(-)
diff --git a/frappe/desk/doctype/note/note.js b/frappe/desk/doctype/note/note.js
index dd556a3969..b8f3b02262 100644
--- a/frappe/desk/doctype/note/note.js
+++ b/frappe/desk/doctype/note/note.js
@@ -1,41 +1,36 @@
frappe.ui.form.on("Note", {
refresh: function (frm) {
- if (frm.doc.__islocal) {
- frm.events.set_editable(frm, true);
- } else {
- if (!frm.doc.content) {
- frm.doc.content = "";
- }
+ if (!frm.is_new()) {
+ frm.is_note_editable = false;
+ frm.events.set_editable(frm, frm.is_note_editable);
// toggle edit
frm.add_custom_button("Edit", function () {
- frm.events.set_editable(frm, !frm.is_note_editable);
+ frm.is_note_editable = !frm.is_note_editable;
+ frm.events.set_editable(frm, frm.is_note_editable);
});
- frm.events.set_editable(frm, false);
}
},
set_editable: function (frm, editable) {
- // hide all fields other than content
-
- // no permission
- if (editable && !frm.perm[0].write) return;
+ // toggle "read_only" for content and "hidden" of all other fields
// content read_only
frm.set_df_property("content", "read_only", editable ? 0 : 1);
// hide all other fields
- $.each(frm.fields_dict, function (fieldname) {
- if (fieldname !== "content") {
- frm.set_df_property(fieldname, "hidden", editable ? 0 : 1);
+ for (const field of frm.meta.fields) {
+ if (field.fieldname !== "content") {
+ frm.set_df_property(
+ field.fieldname,
+ "hidden",
+ editable && !field.hidden && frm.get_perm(field.permlevel, "write") ? 0 : 1
+ );
}
- });
+ }
// no label, description for content either
frm.get_field("content").toggle_label(editable);
frm.get_field("content").toggle_description(editable);
-
- // set flag for toggle
- frm.is_note_editable = editable;
},
});
diff --git a/frappe/desk/doctype/note/note.json b/frappe/desk/doctype/note/note.json
index 69a9518ac4..e4a19a4750 100644
--- a/frappe/desk/doctype/note/note.json
+++ b/frappe/desk/doctype/note/note.json
@@ -1,6 +1,7 @@
{
"actions": [],
"allow_rename": 1,
+ "autoname": "field:title",
"creation": "2013-05-24 13:41:00",
"doctype": "DocType",
"document_type": "Document",
@@ -24,7 +25,8 @@
"label": "Title",
"no_copy": 1,
"print_hide": 1,
- "reqd": 1
+ "reqd": 1,
+ "unique": 1
},
{
"bold": 1,
@@ -32,6 +34,7 @@
"fieldname": "public",
"fieldtype": "Check",
"label": "Public",
+ "permlevel": 1,
"print_hide": 1
},
{
@@ -40,7 +43,8 @@
"depends_on": "public",
"fieldname": "notify_on_login",
"fieldtype": "Check",
- "label": "Notify users with a popup when they log in"
+ "label": "Notify users with a popup when they log in",
+ "permlevel": 1
},
{
"bold": 1,
@@ -49,13 +53,15 @@
"description": "If enabled, users will be notified every time they login. If not enabled, users will only be notified once.",
"fieldname": "notify_on_every_login",
"fieldtype": "Check",
- "label": "Notify Users On Every Login"
+ "label": "Notify Users On Every Login",
+ "permlevel": 1
},
{
"depends_on": "eval:doc.notify_on_login && doc.public",
"fieldname": "expire_notification_on",
"fieldtype": "Date",
"label": "Expire Notification On",
+ "permlevel": 1,
"search_index": 1
},
{
@@ -68,9 +74,11 @@
},
{
"collapsible": 1,
+ "depends_on": "notify_on_login",
"fieldname": "seen_by_section",
"fieldtype": "Section Break",
- "label": "Seen By"
+ "label": "Seen By",
+ "permlevel": 1
},
{
"fieldname": "seen_by",
@@ -82,18 +90,40 @@
"icon": "fa fa-file-text",
"idx": 1,
"links": [],
- "modified": "2021-09-18 10:57:51.352643",
+ "modified": "2022-12-20 14:35:23.257985",
"modified_by": "Administrator",
"module": "Desk",
"name": "Note",
+ "naming_rule": "By fieldname",
"owner": "Administrator",
"permissions": [
{
"create": 1,
"delete": 1,
"email": 1,
+ "export": 1,
"print": 1,
"read": 1,
+ "report": 1,
+ "role": "System Manager",
+ "share": 1,
+ "write": 1
+ },
+ {
+ "permlevel": 1,
+ "read": 1,
+ "role": "System Manager",
+ "write": 1
+ },
+ {
+ "create": 1,
+ "delete": 1,
+ "email": 1,
+ "export": 1,
+ "if_owner": 1,
+ "print": 1,
+ "read": 1,
+ "report": 1,
"role": "All",
"share": 1,
"write": 1
@@ -102,5 +132,6 @@
"quick_entry": 1,
"sort_field": "modified",
"sort_order": "ASC",
+ "states": [],
"track_changes": 1
}
\ No newline at end of file
diff --git a/frappe/desk/doctype/note/note.py b/frappe/desk/doctype/note/note.py
index c0a37d5f44..85ff95d52e 100644
--- a/frappe/desk/doctype/note/note.py
+++ b/frappe/desk/doctype/note/note.py
@@ -1,53 +1,32 @@
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
# License: MIT. See LICENSE
-import re
-
import frappe
from frappe.model.document import Document
-NAME_PATTERN = re.compile("[%'\"#*?`]")
-
class Note(Document):
- def autoname(self):
- # replace forbidden characters
- self.name = NAME_PATTERN.sub("", self.title.strip())
-
def validate(self):
if self.notify_on_login and not self.expire_notification_on:
-
# expire this notification in a week (default)
self.expire_notification_on = frappe.utils.add_days(self.creation, 7)
+ if not self.content:
+ self.content = ""
+
def before_print(self, settings=None):
self.print_heading = self.name
self.sub_heading = ""
+ def mark_seen_by(self, user: str) -> None:
+ if user in [d.user for d in self.seen_by]:
+ return
+
+ self.append("seen_by", {"user": user})
+
@frappe.whitelist()
-def mark_as_seen(note):
- note = frappe.get_doc("Note", note)
- if frappe.session.user not in [d.user for d in note.seen_by]:
- note.append("seen_by", {"user": frappe.session.user})
- note.save(ignore_version=True)
-
-
-def get_permission_query_conditions(user):
- if not user:
- user = frappe.session.user
-
- if user == "Administrator":
- return ""
-
- return f"""(`tabNote`.public=1 or `tabNote`.owner={frappe.db.escape(user)})"""
-
-
-def has_permission(doc, ptype, user):
- if doc.public == 1 or user == "Administrator":
- return True
-
- if user == doc.owner:
- return True
-
- return False
+def mark_as_seen(note: str):
+ note: Note = frappe.get_doc("Note", note)
+ note.mark_seen_by(frappe.session.user)
+ note.save(ignore_permissions=True, ignore_version=True)
diff --git a/frappe/hooks.py b/frappe/hooks.py
index beff7d2de2..03eed0da49 100644
--- a/frappe/hooks.py
+++ b/frappe/hooks.py
@@ -95,7 +95,6 @@ permission_query_conditions = {
"Dashboard Chart": "frappe.desk.doctype.dashboard_chart.dashboard_chart.get_permission_query_conditions",
"Number Card": "frappe.desk.doctype.number_card.number_card.get_permission_query_conditions",
"Notification Settings": "frappe.desk.doctype.notification_settings.notification_settings.get_permission_query_conditions",
- "Note": "frappe.desk.doctype.note.note.get_permission_query_conditions",
"Kanban Board": "frappe.desk.doctype.kanban_board.kanban_board.get_permission_query_conditions",
"Contact": "frappe.contacts.address_and_contact.get_permission_query_conditions_for_contact",
"Address": "frappe.contacts.address_and_contact.get_permission_query_conditions_for_address",
@@ -108,7 +107,6 @@ has_permission = {
"Event": "frappe.desk.doctype.event.event.has_permission",
"ToDo": "frappe.desk.doctype.todo.todo.has_permission",
"User": "frappe.core.doctype.user.user.has_permission",
- "Note": "frappe.desk.doctype.note.note.has_permission",
"Dashboard Chart": "frappe.desk.doctype.dashboard_chart.dashboard_chart.has_permission",
"Number Card": "frappe.desk.doctype.number_card.number_card.has_permission",
"Kanban Board": "frappe.desk.doctype.kanban_board.kanban_board.has_permission",
From ebce8ee002712cb502b5c0d78e7d25ecb4f38541 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 20 Dec 2022 14:54:43 +0100
Subject: [PATCH 02/20] fix: button label
---
frappe/desk/doctype/note/note.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/desk/doctype/note/note.js b/frappe/desk/doctype/note/note.js
index b8f3b02262..fe6e234187 100644
--- a/frappe/desk/doctype/note/note.js
+++ b/frappe/desk/doctype/note/note.js
@@ -5,7 +5,7 @@ frappe.ui.form.on("Note", {
frm.events.set_editable(frm, frm.is_note_editable);
// toggle edit
- frm.add_custom_button("Edit", function () {
+ frm.add_custom_button(__("Editing mode"), function () {
frm.is_note_editable = !frm.is_note_editable;
frm.events.set_editable(frm, frm.is_note_editable);
});
From 5ab75908ee74f6fcc3b4659362cb2ac8c4a29b42 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 20 Dec 2022 15:04:13 +0100
Subject: [PATCH 03/20] fix: permlevel 1 on seen_by
---
frappe/desk/doctype/note/note.json | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/frappe/desk/doctype/note/note.json b/frappe/desk/doctype/note/note.json
index e4a19a4750..eaad75d79d 100644
--- a/frappe/desk/doctype/note/note.json
+++ b/frappe/desk/doctype/note/note.json
@@ -84,13 +84,14 @@
"fieldname": "seen_by",
"fieldtype": "Table",
"label": "Seen By Table",
- "options": "Note Seen By"
+ "options": "Note Seen By",
+ "permlevel": 1
}
],
"icon": "fa fa-file-text",
"idx": 1,
"links": [],
- "modified": "2022-12-20 14:35:23.257985",
+ "modified": "2022-12-20 15:03:43.581298",
"modified_by": "Administrator",
"module": "Desk",
"name": "Note",
From 3afdb39372dfbb96feace426938a07897844c2fe Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 20 Dec 2022 15:30:15 +0100
Subject: [PATCH 04/20] fix: test update with content instead of title
---
frappe/tests/test_frappe_client.py | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/frappe/tests/test_frappe_client.py b/frappe/tests/test_frappe_client.py
index e80e43f49c..925d795ffe 100644
--- a/frappe/tests/test_frappe_client.py
+++ b/frappe/tests/test_frappe_client.py
@@ -105,16 +105,16 @@ class TestFrappeClient(FrappeTestCase):
frappe.db.set_value("Website Settings", None, "title_prefix", "")
def test_update_doc(self):
- server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
- frappe.db.delete("Note", {"title": ("in", ("Sing", "sing"))})
+ server = FrappeClient(get_url(), "Administrator", "3zF2-89X4-AYSm-JA9M", verify=False)
+ frappe.db.delete("Note", {"title": "Sing"})
frappe.db.commit()
- server.insert({"doctype": "Note", "public": True, "title": "Sing"})
+ server.insert({"doctype": "Note", "title": "Sing"})
doc = server.get_doc("Note", "Sing")
- changed_title = "sing"
- doc["title"] = changed_title
+ new_content = "Hello, World!
"
+ doc["content"] = new_content
doc = server.update(doc)
- self.assertTrue(doc["title"] == changed_title)
+ self.assertTrue(doc["content"] == new_content)
def test_update_child_doc(self):
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
From d0f75363a5d54b2f55993a9acb950ab871b37848 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 20 Dec 2022 16:30:11 +0100
Subject: [PATCH 05/20] refactor: make test for assignment independent of Note
---
.../assignment_rule/test_assignment_rule.py | 159 +++++++++++-------
frappe/tests/test_assign.py | 27 ++-
2 files changed, 120 insertions(+), 66 deletions(-)
diff --git a/frappe/automation/doctype/assignment_rule/test_assignment_rule.py b/frappe/automation/doctype/assignment_rule/test_assignment_rule.py
index 8f1773608f..45a1b85291 100644
--- a/frappe/automation/doctype/assignment_rule/test_assignment_rule.py
+++ b/frappe/automation/doctype/assignment_rule/test_assignment_rule.py
@@ -4,7 +4,8 @@
import frappe
from frappe.test_runner import make_test_records
from frappe.tests.utils import FrappeTestCase
-from frappe.utils import random_string
+
+TEST_DOCTYPE = "Assignment Test"
class TestAutoAssign(FrappeTestCase):
@@ -12,6 +13,7 @@ class TestAutoAssign(FrappeTestCase):
def setUpClass(cls):
super().setUpClass()
frappe.db.delete("Assignment Rule")
+ create_test_doctype(TEST_DOCTYPE)
@classmethod
def tearDownClass(cls):
@@ -33,45 +35,49 @@ class TestAutoAssign(FrappeTestCase):
clear_assignments()
def test_round_robin(self):
- note = make_note(dict(public=1))
-
# check if auto assigned to first user
+ record = _make_test_record(public=1)
self.assertEqual(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "allocated_to"
+ "ToDo",
+ dict(reference_type=TEST_DOCTYPE, reference_name=record.name, status="Open"),
+ "allocated_to",
),
"test@example.com",
)
- note = make_note(dict(public=1))
-
# check if auto assigned to second user
+ record = _make_test_record(public=1)
self.assertEqual(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "allocated_to"
+ "ToDo",
+ dict(reference_type=TEST_DOCTYPE, reference_name=record.name, status="Open"),
+ "allocated_to",
),
"test1@example.com",
)
clear_assignments()
- note = make_note(dict(public=1))
-
# check if auto assigned to third user, even if
# previous assignments where closed
+ record = _make_test_record(public=1)
self.assertEqual(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "allocated_to"
+ "ToDo",
+ dict(reference_type=TEST_DOCTYPE, reference_name=record.name, status="Open"),
+ "allocated_to",
),
"test2@example.com",
)
# check loop back to first user
- note = make_note(dict(public=1))
-
+ record = _make_test_record(public=1)
self.assertEqual(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "allocated_to"
+ "ToDo",
+ dict(reference_type=TEST_DOCTYPE, reference_name=record.name, status="Open"),
+ "allocated_to",
),
"test@example.com",
)
@@ -81,29 +87,29 @@ class TestAutoAssign(FrappeTestCase):
self.assignment_rule.save()
for _ in range(30):
- note = make_note(dict(public=1))
+ _make_test_record(public=1)
# check if each user has 10 assignments (?)
for user in ("test@example.com", "test1@example.com", "test2@example.com"):
self.assertEqual(
- len(frappe.get_all("ToDo", dict(allocated_to=user, reference_type="Note"))), 10
+ len(frappe.get_all("ToDo", dict(allocated_to=user, reference_type=TEST_DOCTYPE))), 10
)
# clear 5 assignments for first user
# can't do a limit in "delete" since postgres does not support it
for d in frappe.get_all(
- "ToDo", dict(reference_type="Note", allocated_to="test@example.com"), limit=5
+ "ToDo", dict(reference_type=TEST_DOCTYPE, allocated_to="test@example.com"), limit=5
):
frappe.db.delete("ToDo", {"name": d.name})
# add 5 more assignments
for i in range(5):
- make_note(dict(public=1))
+ _make_test_record(public=1)
# check if each user still has 10 assignments
for user in ("test@example.com", "test1@example.com", "test2@example.com"):
self.assertEqual(
- len(frappe.get_all("ToDo", dict(allocated_to=user, reference_type="Note"))), 10
+ len(frappe.get_all("ToDo", dict(allocated_to=user, reference_type=TEST_DOCTYPE))), 10
)
def test_based_on_field(self):
@@ -112,21 +118,21 @@ class TestAutoAssign(FrappeTestCase):
self.assignment_rule.save()
frappe.set_user("test1@example.com")
- note = make_note(dict(public=1))
+ note = _make_test_record(public=1)
# check if auto assigned to doc owner, test1@example.com
self.assertEqual(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "owner"
+ "ToDo", dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"), "owner"
),
"test1@example.com",
)
frappe.set_user("test2@example.com")
- note = make_note(dict(public=1))
+ note = _make_test_record(public=1)
# check if auto assigned to doc owner, test2@example.com
self.assertEqual(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "owner"
+ "ToDo", dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"), "owner"
),
"test2@example.com",
)
@@ -135,21 +141,23 @@ class TestAutoAssign(FrappeTestCase):
def test_assign_condition(self):
# check condition
- note = make_note(dict(public=0))
+ note = _make_test_record(public=0)
self.assertEqual(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "allocated_to"
+ "ToDo",
+ dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"),
+ "allocated_to",
),
None,
)
def test_clear_assignment(self):
- note = make_note(dict(public=1))
+ note = _make_test_record(public=1)
# check if auto assigned to first user
todo = frappe.get_list(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), limit=1
+ "ToDo", dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"), limit=1
)[0]
todo = frappe.get_doc("ToDo", todo["name"])
@@ -165,11 +173,11 @@ class TestAutoAssign(FrappeTestCase):
self.assertEqual(todo.status, "Cancelled")
def test_close_assignment(self):
- note = make_note(dict(public=1, content="valid"))
+ note = _make_test_record(public=1, content="valid")
# check if auto assigned
todo = frappe.get_list(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), limit=1
+ "ToDo", dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"), limit=1
)[0]
todo = frappe.get_doc("ToDo", todo["name"])
@@ -186,12 +194,14 @@ class TestAutoAssign(FrappeTestCase):
self.assertEqual(todo.allocated_to, "test@example.com")
def check_multiple_rules(self):
- note = make_note(dict(public=1, notify_on_login=1))
+ note = _make_test_record(public=1, notify_on_login=1)
# check if auto assigned to test3 (2nd rule is applied, as it has higher priority)
self.assertEqual(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "allocated_to"
+ "ToDo",
+ dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"),
+ "allocated_to",
),
"test@example.com",
)
@@ -206,21 +216,25 @@ class TestAutoAssign(FrappeTestCase):
get_assignment_rule([days_1, days_2], ["public == 1", "public == 1"])
frappe.flags.assignment_day = "Monday"
- note = make_note(dict(public=1))
+ note = _make_test_record(public=1)
self.assertIn(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "allocated_to"
+ "ToDo",
+ dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"),
+ "allocated_to",
),
["test@example.com", "test1@example.com", "test2@example.com"],
)
frappe.flags.assignment_day = "Friday"
- note = make_note(dict(public=1))
+ note = _make_test_record(public=1)
self.assertIn(
frappe.db.get_value(
- "ToDo", dict(reference_type="Note", reference_name=note.name, status="Open"), "allocated_to"
+ "ToDo",
+ dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"),
+ "allocated_to",
),
["test3@example.com"],
)
@@ -228,17 +242,11 @@ class TestAutoAssign(FrappeTestCase):
def test_assignment_rule_condition(self):
frappe.db.delete("Assignment Rule")
- # Add expiry_date custom field
- from frappe.custom.doctype.custom_field.custom_field import create_custom_field
-
- df = dict(fieldname="expiry_date", label="Expiry Date", fieldtype="Date")
- create_custom_field("Note", df)
-
assignment_rule = frappe.get_doc(
dict(
name="Assignment with Due Date",
doctype="Assignment Rule",
- document_type="Note",
+ document_type=TEST_DOCTYPE,
assign_condition="public == 0",
due_date_based_on="expiry_date",
assignment_days=self.days,
@@ -249,11 +257,11 @@ class TestAutoAssign(FrappeTestCase):
).insert()
expiry_date = frappe.utils.add_days(frappe.utils.nowdate(), 2)
- note1 = make_note({"expiry_date": expiry_date})
- note2 = make_note({"expiry_date": expiry_date})
+ note1 = _make_test_record(expiry_date=expiry_date)
+ note2 = _make_test_record(expiry_date=expiry_date)
note1_todo = frappe.get_all(
- "ToDo", filters=dict(reference_type="Note", reference_name=note1.name, status="Open")
+ "ToDo", filters=dict(reference_type=TEST_DOCTYPE, reference_name=note1.name, status="Open")
)[0]
note1_todo_doc = frappe.get_doc("ToDo", note1_todo.name)
@@ -268,7 +276,7 @@ class TestAutoAssign(FrappeTestCase):
# saving one note's expiry should not update other note todo's due date
note2_todo = frappe.get_all(
"ToDo",
- filters=dict(reference_type="Note", reference_name=note2.name, status="Open"),
+ filters=dict(reference_type=TEST_DOCTYPE, reference_name=note2.name, status="Open"),
fields=["name", "date"],
)[0]
self.assertNotEqual(frappe.utils.get_date_str(note2_todo.date), note1.expiry_date)
@@ -278,21 +286,21 @@ class TestAutoAssign(FrappeTestCase):
def clear_assignments():
- frappe.db.delete("ToDo", {"reference_type": "Note"})
+ frappe.db.delete("ToDo", {"reference_type": TEST_DOCTYPE})
def get_assignment_rule(days, assign=None):
- frappe.delete_doc_if_exists("Assignment Rule", "For Note 1")
+ frappe.delete_doc_if_exists("Assignment Rule", f"For {TEST_DOCTYPE} 1")
if not assign:
assign = ["public == 1", "notify_on_login == 1"]
assignment_rule = frappe.get_doc(
dict(
- name="For Note 1",
+ name=f"For {TEST_DOCTYPE} 1",
doctype="Assignment Rule",
priority=0,
- document_type="Note",
+ document_type=TEST_DOCTYPE,
assign_condition=assign[0],
unassign_condition="public == 0 or notify_on_login == 1",
close_condition='"Closed" in content',
@@ -306,15 +314,15 @@ def get_assignment_rule(days, assign=None):
)
).insert()
- frappe.delete_doc_if_exists("Assignment Rule", "For Note 2")
+ frappe.delete_doc_if_exists("Assignment Rule", f"For {TEST_DOCTYPE} 2")
# 2nd rule
frappe.get_doc(
dict(
- name="For Note 2",
+ name=f"For {TEST_DOCTYPE} 2",
doctype="Assignment Rule",
priority=1,
- document_type="Note",
+ document_type=TEST_DOCTYPE,
assign_condition=assign[1],
unassign_condition="notify_on_login == 0",
rule="Round Robin",
@@ -326,12 +334,47 @@ def get_assignment_rule(days, assign=None):
return assignment_rule
-def make_note(values=None):
- note = frappe.get_doc(dict(doctype="Note", title=random_string(10), content=random_string(20)))
+def _make_test_record(**kwargs):
+ doc = frappe.new_doc(TEST_DOCTYPE)
- if values:
- note.update(values)
+ if kwargs:
+ doc.update(kwargs)
- note.insert()
+ return doc.insert()
- return note
+
+def create_test_doctype(doctype):
+ """Create custom doctype."""
+ if frappe.db.exists("DocType", doctype):
+ return
+
+ frappe.get_doc(
+ {
+ "doctype": "DocType",
+ "name": doctype,
+ "module": "Custom",
+ "custom": 1,
+ "fields": [
+ {
+ "fieldname": "expiry_date",
+ "label": "Expiry Date",
+ "fieldtype": "Date",
+ },
+ {
+ "fieldname": "notify_on_login",
+ "label": "Notify on Login",
+ "fieldtype": "Check",
+ },
+ {
+ "fieldname": "public",
+ "label": "Public",
+ "fieldtype": "Check",
+ },
+ {
+ "fieldname": "content",
+ "label": "Content",
+ "fieldtype": "Text",
+ },
+ ],
+ }
+ ).insert()
diff --git a/frappe/tests/test_assign.py b/frappe/tests/test_assign.py
index f3b579c028..4243bd080a 100644
--- a/frappe/tests/test_assign.py
+++ b/frappe/tests/test_assign.py
@@ -2,13 +2,22 @@
# License: MIT. See LICENSE
import frappe
import frappe.desk.form.assign_to
-from frappe.automation.doctype.assignment_rule.test_assignment_rule import make_note
+from frappe.automation.doctype.assignment_rule.test_assignment_rule import (
+ TEST_DOCTYPE,
+ _make_test_record,
+ create_test_doctype,
+)
from frappe.desk.form.load import get_assignments
from frappe.desk.listview import get_group_by_count
from frappe.tests.utils import FrappeTestCase
class TestAssign(FrappeTestCase):
+ @classmethod
+ def setUpClass(cls):
+ super().setUpClass()
+ create_test_doctype(TEST_DOCTYPE)
+
def test_assign(self):
todo = frappe.get_doc({"doctype": "ToDo", "description": "test"}).insert()
if not frappe.db.exists("User", "test@example.com"):
@@ -18,7 +27,7 @@ class TestAssign(FrappeTestCase):
self.assertTrue("test@example.com" in [d.owner for d in added])
- removed = frappe.desk.form.assign_to.remove(todo.doctype, todo.name, "test@example.com")
+ frappe.desk.form.assign_to.remove(todo.doctype, todo.name, "test@example.com")
# assignment is cleared
assignments = frappe.desk.form.assign_to.get(dict(doctype=todo.doctype, name=todo.name))
@@ -47,25 +56,27 @@ class TestAssign(FrappeTestCase):
}
).insert()
- note = make_note()
+ note = _make_test_record()
assign(note, "test_assign1@example.com")
- note = make_note(dict(public=1))
+ note = _make_test_record(public=1)
assign(note, "test_assign2@example.com")
- note = make_note(dict(public=1))
+ note = _make_test_record(public=1)
assign(note, "test_assign2@example.com")
- note = make_note()
+ note = _make_test_record()
assign(note, "test_assign2@example.com")
- data = {d.name: d.count for d in get_group_by_count("Note", "[]", "assigned_to")}
+ data = {d.name: d.count for d in get_group_by_count(TEST_DOCTYPE, "[]", "assigned_to")}
self.assertTrue("test_assign1@example.com" in data)
self.assertEqual(data["test_assign1@example.com"], 1)
self.assertEqual(data["test_assign2@example.com"], 3)
- data = {d.name: d.count for d in get_group_by_count("Note", '[{"public": 1}]', "assigned_to")}
+ data = {
+ d.name: d.count for d in get_group_by_count(TEST_DOCTYPE, '[{"public": 1}]', "assigned_to")
+ }
self.assertFalse("test_assign1@example.com" in data)
self.assertEqual(data["test_assign2@example.com"], 2)
From 636b85174e6e98f4ee89bebefe9695f3036784a1 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 17 Jan 2023 19:49:43 +0100
Subject: [PATCH 06/20] fix: test assignment rule
---
.../assignment_rule/test_assignment_rule.py | 51 ++++++++++---------
1 file changed, 27 insertions(+), 24 deletions(-)
diff --git a/frappe/automation/doctype/assignment_rule/test_assignment_rule.py b/frappe/automation/doctype/assignment_rule/test_assignment_rule.py
index 45a1b85291..2460c40e8a 100644
--- a/frappe/automation/doctype/assignment_rule/test_assignment_rule.py
+++ b/frappe/automation/doctype/assignment_rule/test_assignment_rule.py
@@ -20,6 +20,7 @@ class TestAutoAssign(FrappeTestCase):
frappe.db.rollback()
def setUp(self):
+ frappe.set_user("Administrator")
make_test_records("User")
days = [
dict(day="Sunday"),
@@ -117,27 +118,16 @@ class TestAutoAssign(FrappeTestCase):
self.assignment_rule.field = "owner"
self.assignment_rule.save()
- frappe.set_user("test1@example.com")
- note = _make_test_record(public=1)
- # check if auto assigned to doc owner, test1@example.com
- self.assertEqual(
- frappe.db.get_value(
- "ToDo", dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"), "owner"
- ),
- "test1@example.com",
- )
-
- frappe.set_user("test2@example.com")
- note = _make_test_record(public=1)
- # check if auto assigned to doc owner, test2@example.com
- self.assertEqual(
- frappe.db.get_value(
- "ToDo", dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"), "owner"
- ),
- "test2@example.com",
- )
-
- frappe.set_user("Administrator")
+ for test_user in ("test1@example.com", "test2@example.com"):
+ frappe.set_user(test_user)
+ note = _make_test_record(public=1)
+ # check if auto assigned to doc owner, test1@example.com
+ self.assertEqual(
+ frappe.db.get_value(
+ "ToDo", dict(reference_type=TEST_DOCTYPE, reference_name=note.name, status="Open"), "owner"
+ ),
+ test_user,
+ )
def test_assign_condition(self):
# check condition
@@ -343,10 +333,9 @@ def _make_test_record(**kwargs):
return doc.insert()
-def create_test_doctype(doctype):
+def create_test_doctype(doctype: str):
"""Create custom doctype."""
- if frappe.db.exists("DocType", doctype):
- return
+ frappe.db.delete("DocType", doctype)
frappe.get_doc(
{
@@ -376,5 +365,19 @@ def create_test_doctype(doctype):
"fieldtype": "Text",
},
],
+ "permissions": [
+ {
+ "create": 1,
+ "delete": 1,
+ "email": 1,
+ "export": 1,
+ "print": 1,
+ "read": 1,
+ "report": 1,
+ "role": "All",
+ "share": 1,
+ "write": 1,
+ },
+ ],
}
).insert()
From ecb54ff066d4ea3945b49ac792a702955837e7c1 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 17 Jan 2023 20:07:37 +0100
Subject: [PATCH 07/20] feat: hash naming for note
---
frappe/desk/doctype/note/note.json | 14 +++++---------
frappe/desk/doctype/note/note_list.js | 6 ++----
2 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/frappe/desk/doctype/note/note.json b/frappe/desk/doctype/note/note.json
index eaad75d79d..471e457a5d 100644
--- a/frappe/desk/doctype/note/note.json
+++ b/frappe/desk/doctype/note/note.json
@@ -1,7 +1,6 @@
{
"actions": [],
- "allow_rename": 1,
- "autoname": "field:title",
+ "autoname": "hash",
"creation": "2013-05-24 13:41:00",
"doctype": "DocType",
"document_type": "Document",
@@ -20,13 +19,9 @@
{
"fieldname": "title",
"fieldtype": "Data",
- "in_global_search": 1,
"in_list_view": 1,
"label": "Title",
- "no_copy": 1,
- "print_hide": 1,
- "reqd": 1,
- "unique": 1
+ "reqd": 1
},
{
"bold": 1,
@@ -91,11 +86,11 @@
"icon": "fa fa-file-text",
"idx": 1,
"links": [],
- "modified": "2022-12-20 15:03:43.581298",
+ "modified": "2023-01-17 20:04:01.789170",
"modified_by": "Administrator",
"module": "Desk",
"name": "Note",
- "naming_rule": "By fieldname",
+ "naming_rule": "Random",
"owner": "Administrator",
"permissions": [
{
@@ -134,5 +129,6 @@
"sort_field": "modified",
"sort_order": "ASC",
"states": [],
+ "title_field": "title",
"track_changes": 1
}
\ No newline at end of file
diff --git a/frappe/desk/doctype/note/note_list.js b/frappe/desk/doctype/note/note_list.js
index 1e0ed40880..a8e948bc94 100644
--- a/frappe/desk/doctype/note/note_list.js
+++ b/frappe/desk/doctype/note/note_list.js
@@ -1,8 +1,6 @@
frappe.listview_settings["Note"] = {
- onload: function (me) {
- me.page.set_title(__("Notes"));
- },
- add_fields: ["title", "public"],
+ hide_name_column: true,
+ add_fields: ["public"],
get_indicator: function (doc) {
if (doc.public) {
return [__("Public"), "green", "public,=,Yes"];
From 88937883659a215c815cc6a7c1ee011a031c59f4 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 17 Jan 2023 20:14:50 +0100
Subject: [PATCH 08/20] fix: undo temp constant in test
---
frappe/tests/test_frappe_client.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/tests/test_frappe_client.py b/frappe/tests/test_frappe_client.py
index 925d795ffe..e3a95af006 100644
--- a/frappe/tests/test_frappe_client.py
+++ b/frappe/tests/test_frappe_client.py
@@ -105,7 +105,7 @@ class TestFrappeClient(FrappeTestCase):
frappe.db.set_value("Website Settings", None, "title_prefix", "")
def test_update_doc(self):
- server = FrappeClient(get_url(), "Administrator", "3zF2-89X4-AYSm-JA9M", verify=False)
+ server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
frappe.db.delete("Note", {"title": "Sing"})
frappe.db.commit()
From da925ce681d95d76bc1c67db6944737cdf6661f9 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 17 Jan 2023 21:00:05 +0100
Subject: [PATCH 09/20] fix: test cases for frappe client
---
frappe/tests/test_frappe_client.py | 102 +++++++++++++----------------
1 file changed, 44 insertions(+), 58 deletions(-)
diff --git a/frappe/tests/test_frappe_client.py b/frappe/tests/test_frappe_client.py
index e3a95af006..4d7897b344 100644
--- a/frappe/tests/test_frappe_client.py
+++ b/frappe/tests/test_frappe_client.py
@@ -17,31 +17,26 @@ class TestFrappeClient(FrappeTestCase):
def test_insert_many(self):
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
- frappe.db.delete("Note", {"title": ("in", ("Sing", "a", "song", "of", "sixpence"))})
- frappe.db.commit()
-
server.insert_many(
[
- {"doctype": "Note", "public": True, "title": "Sing"},
- {"doctype": "Note", "public": True, "title": "a"},
- {"doctype": "Note", "public": True, "title": "song"},
- {"doctype": "Note", "public": True, "title": "of"},
- {"doctype": "Note", "public": True, "title": "sixpence"},
+ {"doctype": "Note", "title": "Sing"},
+ {"doctype": "Note", "title": "a"},
+ {"doctype": "Note", "title": "song"},
+ {"doctype": "Note", "title": "of"},
+ {"doctype": "Note", "title": "sixpence"},
]
)
+ records = frappe.get_all("Note", pluck="title")
- self.assertTrue(frappe.db.get_value("Note", {"title": "Sing"}))
- self.assertTrue(frappe.db.get_value("Note", {"title": "a"}))
- self.assertTrue(frappe.db.get_value("Note", {"title": "song"}))
- self.assertTrue(frappe.db.get_value("Note", {"title": "of"}))
- self.assertTrue(frappe.db.get_value("Note", {"title": "sixpence"}))
+ self.assertIn("Sing", records)
+ self.assertIn("a", records)
+ self.assertIn("song", records)
+ self.assertIn("of", records)
+ self.assertIn("sixpence", records)
def test_create_doc(self):
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
- frappe.db.delete("Note", {"title": "test_create"})
- frappe.db.commit()
-
- server.insert({"doctype": "Note", "public": True, "title": "test_create"})
+ server.insert({"doctype": "Note", "title": "test_create"})
self.assertTrue(frappe.db.get_value("Note", {"title": "test_create"}))
@@ -52,37 +47,38 @@ class TestFrappeClient(FrappeTestCase):
self.assertTrue(len(doc_list))
def test_get_doc(self):
+ USER = "Administrator"
+ TITLE = "get_this"
+ DOCTYPE = "Note"
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
- frappe.db.delete("Note", {"title": "get_this"})
- frappe.db.commit()
- server.insert_many(
- [
- {"doctype": "Note", "public": True, "title": "get_this"},
- ]
- )
- doc = server.get_doc("Note", "get_this")
- self.assertTrue(doc)
+ NAME = server.insert({"doctype": DOCTYPE, "title": TITLE}).get("name")
+ doc = server.get_doc(DOCTYPE, NAME)
- def test_get_value(self):
+ self.assertEqual(doc.get("doctype"), DOCTYPE)
+ self.assertEqual(doc.get("name"), NAME)
+ self.assertEqual(doc.get("title"), TITLE)
+ self.assertEqual(doc.get("owner"), USER)
+
+ def test_get_value_by_filters(self):
+ CONTENT = "test get value"
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
- frappe.db.delete("Note", {"title": "get_value"})
- frappe.db.commit()
+ server.insert({"doctype": "Note", "title": "get_value", "content": CONTENT}).get("name")
- test_content = "test get value"
-
- server.insert_many(
- [
- {"doctype": "Note", "public": True, "title": "get_value", "content": test_content},
- ]
- )
self.assertEqual(
- server.get_value("Note", "content", {"title": "get_value"}).get("content"), test_content
+ server.get_value("Note", "content", {"title": "get_value"}).get("content"), CONTENT
)
- name = server.get_value("Note", "name", {"title": "get_value"}).get("name")
- # test by name
- self.assertEqual(server.get_value("Note", "content", name).get("content"), test_content)
+ def test_get_value_by_name(self):
+ server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
+ CONTENT = "test get value"
+ NAME = server.insert({"doctype": "Note", "title": "get_value", "content": CONTENT}).get("name")
+
+ self.assertEqual(server.get_value("Note", "content", NAME).get("content"), CONTENT)
+
+ def test_get_value_with_malicious_query(self):
+ server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
+ server.insert({"doctype": "Note", "title": "get_value"})
self.assertRaises(
FrappeException,
@@ -106,15 +102,13 @@ class TestFrappeClient(FrappeTestCase):
def test_update_doc(self):
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
- frappe.db.delete("Note", {"title": "Sing"})
- frappe.db.commit()
+ resp = server.insert({"doctype": "Note", "title": "Sing"})
+ doc = server.get_doc("Note", resp.get("name"))
- server.insert({"doctype": "Note", "title": "Sing"})
- doc = server.get_doc("Note", "Sing")
- new_content = "Hello, World!
"
- doc["content"] = new_content
+ CONTENT = "Hello, World!
"
+ doc["content"] = CONTENT
doc = server.update(doc)
- self.assertTrue(doc["content"] == new_content)
+ self.assertTrue(doc["content"] == CONTENT)
def test_update_child_doc(self):
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
@@ -160,17 +154,9 @@ class TestFrappeClient(FrappeTestCase):
def test_delete_doc(self):
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
- frappe.db.delete("Note", {"title": "delete"})
- frappe.db.commit()
-
- server.insert_many(
- [
- {"doctype": "Note", "public": True, "title": "delete"},
- ]
- )
- server.delete("Note", "delete")
-
- self.assertFalse(frappe.db.get_value("Note", {"title": "delete"}))
+ NAME_TO_DELETE = server.insert({"doctype": "Note", "title": "Sing"}).get("name")
+ server.delete("Note", NAME_TO_DELETE)
+ self.assertFalse(frappe.db.get_value("Note", NAME_TO_DELETE))
def test_auth_via_api_key_secret(self):
# generate API key and API secret for administrator
From 913417e1dccb7fb698fd04f4c7433169cfa89b4c Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Tue, 17 Jan 2023 23:51:58 +0100
Subject: [PATCH 10/20] fix: naming test cases
... that used to rely on title based naming of Note
---
frappe/tests/test_naming.py | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py
index 9f5b46a4d1..da750554d7 100644
--- a/frappe/tests/test_naming.py
+++ b/frappe/tests/test_naming.py
@@ -31,16 +31,13 @@ class TestNaming(FrappeTestCase):
if Bottle-1 exists
Bottle -> Bottle-2
"""
+ TITLE = "Bottle"
+ DOCTYPE = "Note"
- note = frappe.new_doc("Note")
- note.title = "Test"
- note.insert()
+ note = frappe.get_doc({"doctype": DOCTYPE, "title": TITLE}).insert()
- title2 = append_number_if_name_exists("Note", "Test")
- self.assertEqual(title2, "Test-1")
-
- title2 = append_number_if_name_exists("Note", "Test", "title", "_")
- self.assertEqual(title2, "Test_1")
+ self.assertEqual(append_number_if_name_exists(DOCTYPE, note.name), f"{note.name}-1")
+ self.assertEqual(append_number_if_name_exists(DOCTYPE, TITLE, "title", "_"), f"{TITLE}_1")
def test_field_autoname_name_sync(self):
@@ -276,8 +273,8 @@ class TestNaming(FrappeTestCase):
# set by passing set_name as ToDo
self.assertRaises(frappe.NameError, make_invalid_todo)
- # set new name - Note
- note = frappe.get_doc({"doctype": "Note", "title": "Note"})
+ # name (via title field) cannot be the same as the doctype
+ note = frappe.get_doc({"doctype": "Currency", "currency_name": "Currency"})
self.assertRaises(frappe.NameError, note.insert)
# case 2: set name with "New ---"
From ce545c807d2c1ffab8d299073866c49375c5ab2a Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Wed, 18 Jan 2023 13:24:13 +0100
Subject: [PATCH 11/20] fix: test frappe client
---
frappe/tests/test_frappe_client.py | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/frappe/tests/test_frappe_client.py b/frappe/tests/test_frappe_client.py
index 4d7897b344..40ba854b82 100644
--- a/frappe/tests/test_frappe_client.py
+++ b/frappe/tests/test_frappe_client.py
@@ -8,6 +8,7 @@ import requests
import frappe
from frappe.core.doctype.user.user import generate_keys
from frappe.frappeclient import FrappeClient, FrappeException
+from frappe.model import default_fields
from frappe.tests.utils import FrappeTestCase
from frappe.utils.data import get_url
@@ -26,7 +27,8 @@ class TestFrappeClient(FrappeTestCase):
{"doctype": "Note", "title": "sixpence"},
]
)
- records = frappe.get_all("Note", pluck="title")
+ records = server.get_list("Note", fields=["title"])
+ records = [r.get("title") for r in records]
self.assertIn("Sing", records)
self.assertIn("a", records)
@@ -36,9 +38,13 @@ class TestFrappeClient(FrappeTestCase):
def test_create_doc(self):
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
- server.insert({"doctype": "Note", "title": "test_create"})
+ response = server.insert({"doctype": "Note", "title": "test_create"})
- self.assertTrue(frappe.db.get_value("Note", {"title": "test_create"}))
+ for field in default_fields:
+ self.assertIn(field, response)
+
+ self.assertEqual(response.get("doctype"), "Note")
+ self.assertEqual(response.get("title"), "test_create")
def test_list_docs(self):
server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False)
@@ -55,6 +61,9 @@ class TestFrappeClient(FrappeTestCase):
NAME = server.insert({"doctype": DOCTYPE, "title": TITLE}).get("name")
doc = server.get_doc(DOCTYPE, NAME)
+ for field in default_fields:
+ self.assertIn(field, doc)
+
self.assertEqual(doc.get("doctype"), DOCTYPE)
self.assertEqual(doc.get("name"), NAME)
self.assertEqual(doc.get("title"), TITLE)
From 2e816ed68cd163f0058af8795fdde966293ae554 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Wed, 18 Jan 2023 13:49:57 +0100
Subject: [PATCH 12/20] fix: test_client_insert_many
---
frappe/tests/test_client.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/frappe/tests/test_client.py b/frappe/tests/test_client.py
index 485c023bc4..febf3412ad 100644
--- a/frappe/tests/test_client.py
+++ b/frappe/tests/test_client.py
@@ -237,8 +237,8 @@ class TestClient(FrappeTestCase):
docs = insert_many(doc_list)
self.assertEqual(len(docs), 7)
- self.assertEqual(docs[3], "not-a-random-title")
- self.assertEqual(docs[6], "another-note-title")
+ self.assertEqual(frappe.db.get_value("Note", docs[3], "title"), "not-a-random-title")
+ self.assertEqual(frappe.db.get_value("Note", docs[6], "title"), "another-note-title")
self.assertIn(note1.name, docs)
# cleanup
From f3c30065a2e4b8e8edb6e77bcf249b55f9ba2cf4 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Wed, 8 Mar 2023 17:17:14 +0100
Subject: [PATCH 13/20] test: web form with new Note behaviour
---
cypress/integration/web_form.js | 33 ++++++++++++++++++++-------------
frappe/tests/ui_test_helpers.py | 4 +++-
2 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/cypress/integration/web_form.js b/cypress/integration/web_form.js
index 53f72ab013..8dd16389e7 100644
--- a/cypress/integration/web_form.js
+++ b/cypress/integration/web_form.js
@@ -6,7 +6,7 @@ context("Web Form", () => {
.window()
.its("frappe")
.then((frappe) => {
- return frappe.xcall("frappe.tests.ui_test_helpers.clear_notes");
+ return frappe.xcall("frappe.tests.ui_test_helpers.prepare_webform_test");
});
});
@@ -99,7 +99,7 @@ context("Web Form", () => {
cy.visit("/note");
cy.url().should("include", "/note/list");
- cy.get(".web-list-table thead th").contains("Name");
+ cy.get(".web-list-table thead th").contains("Sr.");
cy.get(".web-list-table thead th").contains("Title");
cy.visit("/app/web-form/note");
@@ -133,13 +133,17 @@ context("Web Form", () => {
cy.visit("/note");
cy.url().should("include", "/note/list");
+ cy.get(".web-list-table thead th").contains("Sr.");
cy.get(".web-list-table thead th").contains("Title");
cy.get(".web-list-table thead th").contains("Public");
cy.get(".web-list-table thead th").contains("Content");
});
it("Breadcrumbs", () => {
- cy.visit("/note/Note 1");
+ cy.visit("/note");
+ cy.url().should("include", "/note/list");
+ cy.get(".web-list-table tbody tr:last").click();
+
cy.get(".breadcrumb-container .breadcrumb .breadcrumb-item:first a")
.should("contain.text", "Note")
.click();
@@ -154,7 +158,9 @@ context("Web Form", () => {
cy.get(".form-tabs .nav-item .nav-link").contains("Customization").click();
cy.save();
- cy.visit("/note/Note 1");
+ cy.visit("/note");
+ cy.url().should("include", "/note/list");
+ cy.get(".web-list-table tbody tr:last").click();
cy.get(".breadcrumb-container .breadcrumb .breadcrumb-item:first a").should(
"contain.text",
"Notes"
@@ -167,7 +173,7 @@ context("Web Form", () => {
cy.url().should("include", "/note/list");
// Read Only Field
- cy.get('.web-list-table tbody tr[id="Note 1"]').click();
+ cy.get(".web-list-table tbody tr:last").click();
cy.get('.frappe-control[data-fieldname="title"] .control-input').should(
"have.css",
"display",
@@ -183,11 +189,12 @@ context("Web Form", () => {
cy.save();
- cy.visit("/note/Note 1");
- cy.url().should("include", "/note/Note%201");
+ cy.visit("/note");
+ cy.url().should("include", "/note/list");
+ cy.get(".web-list-table tbody tr:last").click();
cy.get(".web-form-actions a").contains("Edit Response").click();
- cy.url().should("include", "/note/Note%201/edit");
+ cy.url().should("include", "/edit");
// Editable Field
cy.get_field("title").should("have.value", "Note 1");
@@ -227,16 +234,16 @@ context("Web Form", () => {
cy.visit("/note");
cy.url().should("include", "/note/list");
- cy.get('.web-list-table tbody tr[id="Note 1"] .list-col-checkbox input').click();
- cy.get('.web-list-table tbody tr[id="Note 2"] .list-col-checkbox input').click();
+ cy.get(".web-list-table tbody tr:nth-child(1) .list-col-checkbox input").click();
+ cy.get(".web-list-table tbody tr:nth-child(2) .list-col-checkbox input").click();
cy.get(".web-list-actions button:visible").contains("Delete").click({ force: true });
cy.get(".web-list-actions button").contains("Delete").should("not.be.visible");
cy.visit("/note");
- cy.get('.web-list-table tbody tr[id="Note 1"]').should("not.exist");
- cy.get('.web-list-table tbody tr[id="Note 2"]').should("not.exist");
- cy.get('.web-list-table tbody tr[id="Guest Note 1"]').should("exist");
+ cy.get(".web-list-table tbody tr:nth-child(1)").should("exist");
+ cy.get(".web-list-table tbody tr:nth-child(2)").should("not.exist");
+ cy.get(".web-list-table tbody tr:nth-child(3)").should("not.exist");
});
it("Navigate and Submit a WebForm", () => {
diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py
index 2846b33eb9..26d6b2d5da 100644
--- a/frappe/tests/ui_test_helpers.py
+++ b/frappe/tests/ui_test_helpers.py
@@ -77,10 +77,12 @@ def create_todo_records():
@whitelist_for_tests
-def clear_notes():
+def prepare_webform_test():
for note in frappe.get_all("Note", pluck="name"):
frappe.delete_doc("Note", note, force=True)
+ frappe.delete_doc_if_exists("Web Form", "note")
+
@whitelist_for_tests
def create_communication_record():
From cff29e23b0aee1198120f1b415af26db53f8af17 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Wed, 8 Mar 2023 18:21:25 +0100
Subject: [PATCH 14/20] test: route to form
Test routing to User form instead of Note
---
cypress/integration/view_routing.js | 15 +++++++--------
frappe/tests/ui_test_helpers.py | 6 ------
2 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/cypress/integration/view_routing.js b/cypress/integration/view_routing.js
index 9267974154..72fb6836ec 100644
--- a/cypress/integration/view_routing.js
+++ b/cypress/integration/view_routing.js
@@ -215,14 +215,13 @@ context("View", () => {
});
it("Route to Form", () => {
- cy.call("frappe.tests.ui_test_helpers.create_note").then(() => {
- cy.visit("/app/note/Routing Test");
- cy.window()
- .its("cur_frm")
- .then((frm) => {
- expect(frm.doc.title).to.equal("Routing Test");
- });
- });
+ const test_user = cy.config("testUser");
+ cy.visit(`/app/user/${test_user}`);
+ cy.window()
+ .its("cur_frm")
+ .then((frm) => {
+ expect(frm.doc.name).to.equal(test_user);
+ });
});
it("Route to Settings Workspace", () => {
diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py
index 26d6b2d5da..b153181b27 100644
--- a/frappe/tests/ui_test_helpers.py
+++ b/frappe/tests/ui_test_helpers.py
@@ -535,12 +535,6 @@ def setup_default_view(view, force_reroute=None):
).insert()
-@whitelist_for_tests
-def create_note():
- if not frappe.db.exists("Note", "Routing Test"):
- frappe.get_doc({"doctype": "Note", "title": "Routing Test"}).insert()
-
-
@whitelist_for_tests
def create_kanban():
if not frappe.db.exists("Custom Field", "Note-kanban"):
From 9579ab7be92cb44ca9e1c7bfc4e124ee3751c5b0 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Mon, 24 Apr 2023 16:12:28 +0200
Subject: [PATCH 15/20] fix: show edit button only if write permitted
---
frappe/desk/doctype/note/note.js | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/frappe/desk/doctype/note/note.js b/frappe/desk/doctype/note/note.js
index fe6e234187..e55a1ec724 100644
--- a/frappe/desk/doctype/note/note.js
+++ b/frappe/desk/doctype/note/note.js
@@ -4,11 +4,12 @@ frappe.ui.form.on("Note", {
frm.is_note_editable = false;
frm.events.set_editable(frm, frm.is_note_editable);
- // toggle edit
- frm.add_custom_button(__("Editing mode"), function () {
- frm.is_note_editable = !frm.is_note_editable;
- frm.events.set_editable(frm, frm.is_note_editable);
- });
+ if (frm.has_perm("write")) {
+ frm.add_custom_button(__("Editing mode"), function () {
+ frm.is_note_editable = !frm.is_note_editable;
+ frm.events.set_editable(frm, frm.is_note_editable);
+ });
+ }
}
},
set_editable: function (frm, editable) {
From 20a08757e5109d91cd1f6713c738b1b935043839 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Mon, 24 Apr 2023 16:18:28 +0200
Subject: [PATCH 16/20] fix: role permissions for Note
---
frappe/desk/doctype/note/note.json | 24 +++++++++++++++----
.../doctype/note_seen_by/note_seen_by.json | 5 ++--
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/frappe/desk/doctype/note/note.json b/frappe/desk/doctype/note/note.json
index 471e457a5d..b297e2ada6 100644
--- a/frappe/desk/doctype/note/note.json
+++ b/frappe/desk/doctype/note/note.json
@@ -86,7 +86,7 @@
"icon": "fa fa-file-text",
"idx": 1,
"links": [],
- "modified": "2023-01-17 20:04:01.789170",
+ "modified": "2023-04-24 16:07:03.281184",
"modified_by": "Administrator",
"module": "Desk",
"name": "Note",
@@ -112,17 +112,31 @@
"write": 1
},
{
- "create": 1,
- "delete": 1,
- "email": 1,
+ "permlevel": 2,
+ "read": 1,
+ "role": "System Manager",
+ "write": 1
+ },
+ {
"export": 1,
- "if_owner": 1,
"print": 1,
"read": 1,
"report": 1,
+ "role": "All"
+ },
+ {
+ "create": 1,
+ "delete": 1,
+ "email": 1,
+ "if_owner": 1,
"role": "All",
"share": 1,
"write": 1
+ },
+ {
+ "permlevel": 1,
+ "read": 1,
+ "role": "All"
}
],
"quick_entry": 1,
diff --git a/frappe/desk/doctype/note_seen_by/note_seen_by.json b/frappe/desk/doctype/note_seen_by/note_seen_by.json
index f54559d2f5..905a043284 100644
--- a/frappe/desk/doctype/note_seen_by/note_seen_by.json
+++ b/frappe/desk/doctype/note_seen_by/note_seen_by.json
@@ -13,12 +13,13 @@
"fieldtype": "Link",
"in_list_view": 1,
"label": "User",
- "options": "User"
+ "options": "User",
+ "permlevel": 2
}
],
"istable": 1,
"links": [],
- "modified": "2022-08-03 12:20:51.030908",
+ "modified": "2023-04-24 16:14:53.684098",
"modified_by": "Administrator",
"module": "Desk",
"name": "Note Seen By",
From bc287ab857215b94fc6e4bcdbec20429a6c7424d Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Mon, 24 Apr 2023 16:18:38 +0200
Subject: [PATCH 17/20] feat: permission query for Note
---
frappe/desk/doctype/note/note.py | 7 +++++++
frappe/hooks.py | 1 +
2 files changed, 8 insertions(+)
diff --git a/frappe/desk/doctype/note/note.py b/frappe/desk/doctype/note/note.py
index 85ff95d52e..bbe5c1c0fc 100644
--- a/frappe/desk/doctype/note/note.py
+++ b/frappe/desk/doctype/note/note.py
@@ -30,3 +30,10 @@ def mark_as_seen(note: str):
note: Note = frappe.get_doc("Note", note)
note.mark_seen_by(frappe.session.user)
note.save(ignore_permissions=True, ignore_version=True)
+
+
+def get_permission_query_conditions(user):
+ if not user:
+ user = frappe.session.user
+
+ return f"(`tabNote`.owner = '{user}' or `tabNote`.public = 1)"
diff --git a/frappe/hooks.py b/frappe/hooks.py
index a5edcc49e1..03c4c701cb 100644
--- a/frappe/hooks.py
+++ b/frappe/hooks.py
@@ -100,6 +100,7 @@ permission_query_conditions = {
"Dashboard Chart": "frappe.desk.doctype.dashboard_chart.dashboard_chart.get_permission_query_conditions",
"Number Card": "frappe.desk.doctype.number_card.number_card.get_permission_query_conditions",
"Notification Settings": "frappe.desk.doctype.notification_settings.notification_settings.get_permission_query_conditions",
+ "Note": "frappe.desk.doctype.note.note.get_permission_query_conditions",
"Kanban Board": "frappe.desk.doctype.kanban_board.kanban_board.get_permission_query_conditions",
"Contact": "frappe.contacts.address_and_contact.get_permission_query_conditions_for_contact",
"Address": "frappe.contacts.address_and_contact.get_permission_query_conditions_for_address",
From 3a35a22d5a9cbee7e1d827cf41512f535e59670a Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Mon, 24 Apr 2023 16:33:08 +0200
Subject: [PATCH 18/20] fix: escape user in db query
---
frappe/desk/doctype/note/note.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/frappe/desk/doctype/note/note.py b/frappe/desk/doctype/note/note.py
index bbe5c1c0fc..6aba6391cb 100644
--- a/frappe/desk/doctype/note/note.py
+++ b/frappe/desk/doctype/note/note.py
@@ -36,4 +36,4 @@ def get_permission_query_conditions(user):
if not user:
user = frappe.session.user
- return f"(`tabNote`.owner = '{user}' or `tabNote`.public = 1)"
+ return f"(`tabNote`.owner = {frappe.db.escape(user)} or `tabNote`.public = 1)"
From df22b98f0c7b89f172f902007b3fc7555c51c2ab Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Mon, 24 Apr 2023 16:33:34 +0200
Subject: [PATCH 19/20] feat: toggle between read and edit mode
---
frappe/desk/doctype/note/note.js | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/frappe/desk/doctype/note/note.js b/frappe/desk/doctype/note/note.js
index e55a1ec724..567e7534ba 100644
--- a/frappe/desk/doctype/note/note.js
+++ b/frappe/desk/doctype/note/note.js
@@ -2,21 +2,23 @@ frappe.ui.form.on("Note", {
refresh: function (frm) {
if (!frm.is_new()) {
frm.is_note_editable = false;
- frm.events.set_editable(frm, frm.is_note_editable);
-
- if (frm.has_perm("write")) {
- frm.add_custom_button(__("Editing mode"), function () {
- frm.is_note_editable = !frm.is_note_editable;
- frm.events.set_editable(frm, frm.is_note_editable);
- });
- }
+ frm.events.set_editable(frm);
}
},
- set_editable: function (frm, editable) {
+ set_editable: function (frm) {
+ if (frm.has_perm("write")) {
+ const read_label = __("Read mode");
+ const edit_label = __("Edit mode");
+ frm.remove_custom_button(frm.is_note_editable ? edit_label : read_label);
+ frm.add_custom_button(frm.is_note_editable ? read_label : edit_label, function () {
+ frm.is_note_editable = !frm.is_note_editable;
+ frm.events.set_editable(frm);
+ });
+ }
// toggle "read_only" for content and "hidden" of all other fields
// content read_only
- frm.set_df_property("content", "read_only", editable ? 0 : 1);
+ frm.set_df_property("content", "read_only", frm.is_note_editable ? 0 : 1);
// hide all other fields
for (const field of frm.meta.fields) {
@@ -24,14 +26,16 @@ frappe.ui.form.on("Note", {
frm.set_df_property(
field.fieldname,
"hidden",
- editable && !field.hidden && frm.get_perm(field.permlevel, "write") ? 0 : 1
+ frm.is_note_editable && !field.hidden && frm.get_perm(field.permlevel, "write")
+ ? 0
+ : 1
);
}
}
// no label, description for content either
- frm.get_field("content").toggle_label(editable);
- frm.get_field("content").toggle_description(editable);
+ frm.get_field("content").toggle_label(frm.is_note_editable);
+ frm.get_field("content").toggle_description(frm.is_note_editable);
},
});
From e133124e71b6204f3ec840a2ec761ed1b6133314 Mon Sep 17 00:00:00 2001
From: barredterra <14891507+barredterra@users.noreply.github.com>
Date: Mon, 24 Apr 2023 18:47:31 +0200
Subject: [PATCH 20/20] test: web form
---
cypress/integration/web_form.js | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/cypress/integration/web_form.js b/cypress/integration/web_form.js
index 8dd16389e7..fcb24219b9 100644
--- a/cypress/integration/web_form.js
+++ b/cypress/integration/web_form.js
@@ -241,9 +241,7 @@ context("Web Form", () => {
cy.get(".web-list-actions button").contains("Delete").should("not.be.visible");
cy.visit("/note");
- cy.get(".web-list-table tbody tr:nth-child(1)").should("exist");
- cy.get(".web-list-table tbody tr:nth-child(2)").should("not.exist");
- cy.get(".web-list-table tbody tr:nth-child(3)").should("not.exist");
+ cy.get(".web-list-table tbody tr:nth-child(1)").should("not.exist");
});
it("Navigate and Submit a WebForm", () => {