From 3e715985c6921ce4e24288f22aa6d7628bb1cd8b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 9 Jan 2023 16:53:23 +0530 Subject: [PATCH 01/16] fix: ignore unhandled emails while deleting email account (#19534) [skip ci] --- frappe/hooks.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/hooks.py b/frappe/hooks.py index 9ad95e796b..28b2c0dea1 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -392,4 +392,5 @@ ignore_links_on_delete = [ "Email Queue", "Document Share Key", "Integration Request", + "Unhandled Email", ] From f5771baf6637ac461c73e5f4c6a7d4fb5bba24ee Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 9 Jan 2023 17:09:59 +0530 Subject: [PATCH 02/16] ci: use GITHUB_TOKEN for roulette api calls (#19537) This is safe cuz we set read only permission on this token in workflows. NOTE: Changing permission type is dangerous here! --- .github/helper/roulette.py | 7 ++++++- .github/workflows/patch-mariadb-tests.yml | 2 ++ .github/workflows/server-tests.yml | 2 ++ .github/workflows/ui-tests.yml | 2 ++ 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/helper/roulette.py b/.github/helper/roulette.py index f2a8bcb700..e3b212fa89 100644 --- a/.github/helper/roulette.py +++ b/.github/helper/roulette.py @@ -23,10 +23,15 @@ def fetch_pr_data(pr_number, repo, endpoint=""): def req(url): "Simple resilient request call to handle rate limits." + headers = None + token = os.environ.get("GITHUB_TOKEN") + if token: + headers = {"authorization": f"Bearer {token}"} + retries = 0 while True: try: - req = urllib.request.Request(url) + req = urllib.request.Request(url, headers=headers) return urllib.request.urlopen(req) except HTTPError as exc: if exc.code == 403 and retries < 5: diff --git a/.github/workflows/patch-mariadb-tests.yml b/.github/workflows/patch-mariadb-tests.yml index 57f421cc1b..4b487d2aea 100644 --- a/.github/workflows/patch-mariadb-tests.yml +++ b/.github/workflows/patch-mariadb-tests.yml @@ -9,6 +9,7 @@ concurrency: cancel-in-progress: true permissions: + # Do not change this as GITHUB_TOKEN is being used by roulette contents: read jobs: @@ -31,6 +32,7 @@ jobs: TYPE: "server" PR_NUMBER: ${{ github.event.number }} REPO_NAME: ${{ github.repository }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} test: name: Patch diff --git a/.github/workflows/server-tests.yml b/.github/workflows/server-tests.yml index cc310fd8d7..3b76da1973 100644 --- a/.github/workflows/server-tests.yml +++ b/.github/workflows/server-tests.yml @@ -12,6 +12,7 @@ concurrency: permissions: + # Do not change this as GITHUB_TOKEN is being used by roulette contents: read jobs: @@ -34,6 +35,7 @@ jobs: TYPE: "server" PR_NUMBER: ${{ github.event.number }} REPO_NAME: ${{ github.repository }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} test: name: Unit Tests diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index f022f10ea4..1b88bc73ce 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -11,6 +11,7 @@ concurrency: cancel-in-progress: true permissions: + # Do not change this as GITHUB_TOKEN is being used by roulette contents: read jobs: @@ -33,6 +34,7 @@ jobs: TYPE: "ui" PR_NUMBER: ${{ github.event.number }} REPO_NAME: ${{ github.repository }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} test: runs-on: ubuntu-latest From 97a57f7105a5f22840725ba52cdce5b0f66cecc6 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 9 Jan 2023 17:51:25 +0530 Subject: [PATCH 03/16] feat: hide/unhide workspace from sidebar --- frappe/desk/desktop.py | 14 +++- frappe/desk/doctype/workspace/workspace.json | 9 ++- frappe/desk/doctype/workspace/workspace.py | 26 +++++++ frappe/public/icons/timeless/icons.svg | 10 +++ .../js/frappe/views/workspace/workspace.js | 72 +++++++++++++++++-- frappe/public/scss/desk/desktop.scss | 72 ++++++++++++++----- 6 files changed, 178 insertions(+), 25 deletions(-) diff --git a/frappe/desk/desktop.py b/frappe/desk/desktop.py index dfe7850349..f2243c2e56 100644 --- a/frappe/desk/desktop.py +++ b/frappe/desk/desktop.py @@ -379,7 +379,17 @@ def get_workspace_sidebar_items(): # pages sorted based on sequence id order_by = "sequence_id asc" - fields = ["name", "title", "for_user", "parent_page", "content", "public", "module", "icon"] + fields = [ + "name", + "title", + "for_user", + "parent_page", + "content", + "public", + "module", + "icon", + "is_hidden", + ] all_pages = frappe.get_all( "Workspace", fields=fields, filters=filters, order_by=order_by, ignore_permissions=True ) @@ -391,7 +401,7 @@ def get_workspace_sidebar_items(): try: workspace = Workspace(page, True) if has_access or workspace.is_permitted(): - if page.public: + if page.public and (has_access or not page.is_hidden): pages.append(page) elif page.for_user == frappe.session.user: private_pages.append(page) diff --git a/frappe/desk/doctype/workspace/workspace.json b/frappe/desk/doctype/workspace/workspace.json index 58a1f718fd..edd5c32e99 100644 --- a/frappe/desk/doctype/workspace/workspace.json +++ b/frappe/desk/doctype/workspace/workspace.json @@ -19,6 +19,7 @@ "restrict_to_domain", "hide_custom", "public", + "is_hidden", "content", "tab_break_2", "charts", @@ -174,11 +175,17 @@ "fieldtype": "Table", "label": "Quick Lists", "options": "Workspace Quick List" + }, + { + "default": "0", + "fieldname": "is_hidden", + "fieldtype": "Check", + "label": "Is Hidden" } ], "in_create": 1, "links": [], - "modified": "2023-01-07 17:02:48.278025", + "modified": "2023-01-07 19:37:39.512482", "modified_by": "Administrator", "module": "Desk", "name": "Workspace", diff --git a/frappe/desk/doctype/workspace/workspace.py b/frappe/desk/doctype/workspace/workspace.py index c4110f52a7..7649a2204d 100644 --- a/frappe/desk/doctype/workspace/workspace.py +++ b/frappe/desk/doctype/workspace/workspace.py @@ -247,6 +247,32 @@ def update_page(name, title, icon, parent, public): return {"name": title, "public": public, "label": new_name} +def hide_unhide_page(page_name: str, is_hidden: int): + page = frappe.get_doc("Workspace", page_name) + + if page.get("public") and not is_workspace_manager(): + frappe.throw( + _("Need Workspace Manager role to hide/unhide public workspaces"), frappe.PermissionError + ) + + if not page.get("public") and page.get("for_user") != frappe.session.user: + frappe.throw(_("Cannot update private workspace of other users"), frappe.PermissionError) + + page.is_hidden = is_hidden + page.save(ignore_permissions=True) + return True + + +@frappe.whitelist() +def hide_page(page_name: str): + return hide_unhide_page(page_name, 1) + + +@frappe.whitelist() +def unhide_page(page_name: str): + return hide_unhide_page(page_name, 0) + + @frappe.whitelist() def duplicate_page(page_name, new_page): if not loads(new_page): diff --git a/frappe/public/icons/timeless/icons.svg b/frappe/public/icons/timeless/icons.svg index 69e9e920e6..fed77e7df1 100644 --- a/frappe/public/icons/timeless/icons.svg +++ b/frappe/public/icons/timeless/icons.svg @@ -48,6 +48,16 @@ + + + + + + + + + + diff --git a/frappe/public/js/frappe/views/workspace/workspace.js b/frappe/public/js/frappe/views/workspace/workspace.js index cbcea38f38..6ff7e8ead0 100644 --- a/frappe/public/js/frappe/views/workspace/workspace.js +++ b/frappe/public/js/frappe/views/workspace/workspace.js @@ -78,9 +78,13 @@ frappe.views.Workspace = class Workspace { sidebar_item_container(item) { return $(` -
+
{ if (!this.editor || !this.editor.readOnly) return; this.is_read_only = false; + this.toggle_hidden_workspaces(true); await this.editor.readOnly.toggle(); this.editor.isReady.then(() => { this.initialize_editorjs_undo(); @@ -441,6 +446,7 @@ frappe.views.Workspace = class Workspace { this.page.set_secondary_action(__("Discard"), async () => { this.discard = true; this.clear_page_actions(); + this.toggle_hidden_workspaces(false); await this.editor.readOnly.toggle(); this.is_read_only = true; this.sidebar_pages = this.cached_pages; @@ -455,6 +461,10 @@ frappe.views.Workspace = class Workspace { } } + toggle_hidden_workspaces(show) { + $(".desk-sidebar").toggleClass("show-hidden-workspaces", show); + } + show_sidebar_actions() { this.sidebar.find(".standard-sidebar-section").addClass("show-control"); this.make_sidebar_sortable(); @@ -481,6 +491,15 @@ frappe.views.Workspace = class Workspace { null, sidebar_control ); + } else if (item.is_hidden) { + frappe.utils.add_custom_button( + frappe.utils.icon("unhide", "sm"), + (e) => this.unhide_workspace(item, e), + "unhide-workspace-btn", + __("Unhide Workspace"), + null, + sidebar_control + ); } else { frappe.utils.add_custom_button( frappe.utils.icon("drag", "xs"), @@ -720,6 +739,12 @@ frappe.views.Workspace = class Workspace { icon: frappe.utils.icon("duplicate", "sm"), action: () => this.duplicate_page(item), }, + { + label: __("Hide"), + title: __("Hide Workspace"), + icon: frappe.utils.icon("hide", "sm"), + action: (e) => this.hide_workspace(item, e), + }, ]; if (this.is_item_deletable(item)) { @@ -748,7 +773,7 @@ frappe.views.Workspace = class Workspace { html.click((event) => { event.stopPropagation(); - action && action(); + action && action(event); }); return html; @@ -910,6 +935,45 @@ frappe.views.Workspace = class Workspace { d.show(); } + hide_unhide_workspace(page, event, hide) { + page.is_hidden = hide; + + let sidebar_control = event.target.closest(".sidebar-item-control"); + let sidebar_item_container = sidebar_control.closest(".sidebar-item-container"); + $(sidebar_item_container).attr("item-is-hidden", hide); + + $(sidebar_control).empty(); + this.add_sidebar_actions(page, $(sidebar_control)); + + let cached_page = this.cached_pages.pages.findIndex((p) => p.name === page.name); + if (cached_page !== -1) { + this.cached_pages.pages[cached_page].is_hidden = hide; + } + + let method = hide ? "hide_page" : "unhide_page"; + frappe.call({ + method: "frappe.desk.doctype.workspace.workspace." + method, + args: { + page_name: page.name, + }, + callback: (r) => { + if (!r.message) return; + + let message = hide ? "{0} is hidden successfully" : "{0} is unhidden successfully"; + message = __(message, [page.title.bold()]); + frappe.show_alert({ message: message, indicator: "green" }); + }, + }); + } + + hide_workspace(page, event) { + this.hide_unhide_workspace(page, event, 1); + } + + unhide_workspace(page, event) { + this.hide_unhide_workspace(page, event, 0); + } + make_sidebar_sortable() { let me = this; $(".nested-container").each(function () { diff --git a/frappe/public/scss/desk/desktop.scss b/frappe/public/scss/desk/desktop.scss index b39514b67b..f9ce4f31dd 100644 --- a/frappe/public/scss/desk/desktop.scss +++ b/frappe/public/scss/desk/desktop.scss @@ -1017,6 +1017,16 @@ body { .sidebar-item-container { position: relative; + + &[item-is-hidden="1"] { + display: none; + opacity: 0.4; + + &:hover { + opacity: 1; + } + } + .sidebar-item-container{ margin-left: 10px; @@ -1026,31 +1036,57 @@ body { } } - .standard-sidebar-section.show-control { - .desk-sidebar-item.standard-sidebar-item { + .desk-sidebar { + &.show-hidden-workspaces { - &:hover, &.selected { - .drag-handle { - display: inline-block; - } - - .setting-btn, .duplicate-page { - display: inline-block; - margin-right: 8px; - } - - .drop-icon { - padding: 10px 8px 10px 2px; - margin-left: -8px; - } + .unhide-page-btn { + display: none; } - .block-click { - pointer-events:none; + .standard-sidebar-section { + display: block; + + .sidebar-item-container[item-is-hidden="1"] { + display: block; + } + } + } + + .standard-sidebar-section { + display: none; + + &:has(> [item-is-hidden="0"]) { + display: block; + } + } + + .standard-sidebar-section.show-control { + .desk-sidebar-item.standard-sidebar-item { + + &:hover, &.selected { + .drag-handle { + display: inline-block; + } + + .setting-btn, .duplicate-page, .unhide-workspace-btn { + display: inline-block; + margin-right: 8px; + } + + .drop-icon { + padding: 10px 8px 10px 2px; + margin-left: -8px; + } + } + + .block-click { + pointer-events:none; + } } } } + .codex-editor__loader { display: none !important; } From 29ff3be1cca76d546027b5848e26aa5aebc35290 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 9 Jan 2023 18:31:04 +0530 Subject: [PATCH 04/16] style: hide drop-icon if child workspaces are hidden --- frappe/public/scss/desk/desktop.scss | 29 +++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/frappe/public/scss/desk/desktop.scss b/frappe/public/scss/desk/desktop.scss index f9ce4f31dd..e61fb31d7f 100644 --- a/frappe/public/scss/desk/desktop.scss +++ b/frappe/public/scss/desk/desktop.scss @@ -1027,7 +1027,19 @@ body { } } - .sidebar-item-container{ + &[item-is-hidden="0"] { + .drop-icon { + display: none; + } + + &:has(.sidebar-item-container[item-is-hidden="0"]) { + .drop-icon { + display: inline-block; + } + } + } + + .sidebar-item-container { margin-left: 10px; .standard-sidebar-item { @@ -1039,15 +1051,22 @@ body { .desk-sidebar { &.show-hidden-workspaces { - .unhide-page-btn { + .unhide-workspace-btn { display: none; } .standard-sidebar-section { display: block; - .sidebar-item-container[item-is-hidden="1"] { - display: block; + .sidebar-item-container { + &[item-is-hidden="1"] { + display: block; + } + &[item-is-hidden="0"] { + .drop-icon { + display: inline-block; + } + } } } } @@ -1055,7 +1074,7 @@ body { .standard-sidebar-section { display: none; - &:has(> [item-is-hidden="0"]) { + &:has([item-is-hidden="0"]) { display: block; } } From fabe3906aa9e3f734210b15c98e9756ba548022c Mon Sep 17 00:00:00 2001 From: Ritwik Puri Date: Mon, 9 Jan 2023 18:32:09 +0530 Subject: [PATCH 05/16] chore: remove deprecated check_admin_or_system_manager function (#19540) --- frappe/permissions.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/frappe/permissions.py b/frappe/permissions.py index af4d4178a3..ef33c03875 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -27,22 +27,6 @@ rights = ( ) -def check_admin_or_system_manager(user=None): - from frappe.utils.commands import warn - - warn( - "The function check_admin_or_system_manager will be deprecated in version 15." - 'Please use frappe.only_for("System Manager") instead.', - category=PendingDeprecationWarning, - ) - - if not user: - user = frappe.session.user - - if ("System Manager" not in frappe.get_roles(user)) and (user != "Administrator"): - frappe.throw(_("Not permitted"), frappe.PermissionError) - - def print_has_permission_check_logs(func): def inner(*args, **kwargs): frappe.flags["has_permission_check_logs"] = [] From 9ca33d294605c820d3ee00d8d553ef4434f619cd Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 9 Jan 2023 18:46:35 +0530 Subject: [PATCH 06/16] chore: type check --- frappe/desk/doctype/workspace/workspace.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/desk/doctype/workspace/workspace.py b/frappe/desk/doctype/workspace/workspace.py index 7649a2204d..f7d9e8ac3e 100644 --- a/frappe/desk/doctype/workspace/workspace.py +++ b/frappe/desk/doctype/workspace/workspace.py @@ -247,7 +247,7 @@ def update_page(name, title, icon, parent, public): return {"name": title, "public": public, "label": new_name} -def hide_unhide_page(page_name: str, is_hidden: int): +def hide_unhide_page(page_name: str, is_hidden: bool): page = frappe.get_doc("Workspace", page_name) if page.get("public") and not is_workspace_manager(): @@ -258,7 +258,7 @@ def hide_unhide_page(page_name: str, is_hidden: int): if not page.get("public") and page.get("for_user") != frappe.session.user: frappe.throw(_("Cannot update private workspace of other users"), frappe.PermissionError) - page.is_hidden = is_hidden + page.is_hidden = int(is_hidden) page.save(ignore_permissions=True) return True From d6c6df74dbae6468b13d5fe73976a71c74538472 Mon Sep 17 00:00:00 2001 From: Daizy Modi Date: Mon, 9 Jan 2023 19:03:45 +0530 Subject: [PATCH 07/16] fix: use `doc.has_permission` instead of `frappe.has_permission` (#19541) --- frappe/desk/reportview.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 593e6bf0c2..8f929311e0 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -294,7 +294,7 @@ def save_report(name, doctype, report_settings): if report.report_type != "Report Builder": frappe.throw(_("Only reports of type Report Builder can be edited")) - if report.owner != frappe.session.user and not frappe.has_permission("Report", "write"): + if report.owner != frappe.session.user and not report.has_permission("write"): frappe.throw(_("Insufficient Permissions for editing Report"), frappe.PermissionError) else: report = frappe.new_doc("Report") @@ -323,7 +323,7 @@ def delete_report(name): if report.report_type != "Report Builder": frappe.throw(_("Only reports of type Report Builder can be deleted")) - if report.owner != frappe.session.user and not frappe.has_permission("Report", "delete"): + if report.owner != frappe.session.user and not report.has_permission("delete"): frappe.throw(_("Insufficient Permissions for deleting Report"), frappe.PermissionError) report.delete(ignore_permissions=True) From a6b56c36a9abbf99815ea530d0663fc5870e4439 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 9 Jan 2023 19:30:13 +0530 Subject: [PATCH 08/16] fix: render drop-icon if hide/unhide parent workspace --- frappe/public/js/frappe/views/workspace/workspace.js | 2 ++ frappe/public/scss/desk/desktop.scss | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/workspace/workspace.js b/frappe/public/js/frappe/views/workspace/workspace.js index 6ff7e8ead0..ee5d2c92a0 100644 --- a/frappe/public/js/frappe/views/workspace/workspace.js +++ b/frappe/public/js/frappe/views/workspace/workspace.js @@ -945,6 +945,8 @@ frappe.views.Workspace = class Workspace { $(sidebar_control).empty(); this.add_sidebar_actions(page, $(sidebar_control)); + this.add_drop_icon(page, $(sidebar_control), $(sidebar_item_container)); + let cached_page = this.cached_pages.pages.findIndex((p) => p.name === page.name); if (cached_page !== -1) { this.cached_pages.pages[cached_page].is_hidden = hide; diff --git a/frappe/public/scss/desk/desktop.scss b/frappe/public/scss/desk/desktop.scss index e61fb31d7f..ee2e103ea7 100644 --- a/frappe/public/scss/desk/desktop.scss +++ b/frappe/public/scss/desk/desktop.scss @@ -1074,7 +1074,7 @@ body { .standard-sidebar-section { display: none; - &:has([item-is-hidden="0"]) { + &:has(> [item-is-hidden="0"]) { display: block; } } From f19e7281ae90e12c36a35199cd3cf50e487ba515 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 9 Jan 2023 20:41:18 +0530 Subject: [PATCH 09/16] test: added UI test for hide/unhide workspace --- cypress/integration/workspace.js | 42 ++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/cypress/integration/workspace.js b/cypress/integration/workspace.js index e7d97c705b..d52417b234 100644 --- a/cypress/integration/workspace.js +++ b/cypress/integration/workspace.js @@ -190,6 +190,48 @@ context("Workspace 2.0", () => { cy.get('.standard-actions .btn-primary[data-label="Save"]').click(); }); + it("Hide/Unhide Workspaces", () => { + // hide + cy.intercept({ + method: "POST", + url: "api/method/frappe.desk.doctype.workspace.workspace.hide_page", + }).as("hide_page"); + + cy.get(".codex-editor__redactor .ce-block"); + cy.get(".standard-actions .btn-secondary[data-label=Edit]").click(); + + cy.get('.sidebar-item-container[item-name="Duplicate Page"]') + .find(".sidebar-item-control .setting-btn") + .click(); + cy.get('.sidebar-item-container[item-name="Duplicate Page"]') + .find('.dropdown-item[title="Hide Workspace"]') + .click({ force: true }); + cy.wait(300); + cy.get('.standard-actions .btn-secondary[data-label="Discard"]').click(); + cy.get('.sidebar-item-container[item-name="Duplicate Page"]').should("not.be.visible"); + + cy.wait("@hide_page"); + + // unhide + cy.intercept({ + method: "POST", + url: "api/method/frappe.desk.doctype.workspace.workspace.unhide_page", + }).as("unhide_page"); + + cy.get(".codex-editor__redactor .ce-block"); + cy.get(".standard-actions .btn-secondary[data-label=Edit]").click(); + + cy.get('.sidebar-item-container[item-name="Duplicate Page"]') + .find('[title="Unhide Workspace"]') + .click({ force: true }); + cy.wait(300); + + cy.get('.standard-actions .btn-secondary[data-label="Discard"]').click(); + cy.get('.sidebar-item-container[item-name="Duplicate Page"]').should("be.visible"); + + cy.wait("@unhide_page"); + }); + it("Delete Duplicate Page", () => { cy.intercept({ method: "POST", From 278a56417b4a792bd448c19ce8a0dadc00985688 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 10 Jan 2023 00:02:44 +0530 Subject: [PATCH 10/16] fix: logging type annotations closes https://github.com/frappe/frappe/issues/19521 --- frappe/utils/logger.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/utils/logger.py b/frappe/utils/logger.py index 461204371e..b642821c9d 100755 --- a/frappe/utils/logger.py +++ b/frappe/utils/logger.py @@ -2,6 +2,7 @@ import logging import os from logging.handlers import RotatingFileHandler +from typing import Literal # imports - module imports import frappe @@ -95,7 +96,7 @@ class SiteContextFilter(logging.Filter): return True -def set_log_level(level: int) -> None: +def set_log_level(level: Literal["ERROR", "WARNING", "WARN", "INFO", "DEBUG"]) -> None: """Use this method to set log level to something other than the default DEBUG""" frappe.log_level = getattr(logging, (level or "").upper(), None) or default_log_level frappe.loggers = {} From 1eaca4e1d568524b85f81ffbc5b19bac7213e477 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 10 Jan 2023 11:45:31 +0530 Subject: [PATCH 11/16] fix: RSS feed escaping (#19546) if title contains reserved chars then malformed XML is generated. Try title with ampersand in it. [skip ci] --- frappe/tests/ui_test_helpers.py | 2 +- frappe/www/rss.py | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py index 16a5688cec..2846b33eb9 100644 --- a/frappe/tests/ui_test_helpers.py +++ b/frappe/tests/ui_test_helpers.py @@ -495,7 +495,7 @@ def setup_image_doctype(): @whitelist_for_tests def setup_inbox(): - frappe.db.sql("DELETE FROM `tabUser Email`") + frappe.db.delete("User Email") user = frappe.get_doc("User", frappe.session.user) user.append("user_emails", {"email_account": "Email Linking"}) diff --git a/frappe/www/rss.py b/frappe/www/rss.py index dbd2aac9e9..672e33e1de 100644 --- a/frappe/www/rss.py +++ b/frappe/www/rss.py @@ -15,18 +15,19 @@ def get_context(context): host = get_request_site_address() - blog_list = frappe.db.sql( - """\ - select route as name, published_on, modified, title, content from `tabBlog Post` - where ifnull(published,0)=1 - order by published_on desc limit 20""", - as_dict=1, + blog_list = frappe.get_all( + "Blog Post", + fields=["name", "published_on", "modified", "title", "content"], + filters={"published": 1}, + order_by="published_on desc", + limit=20, ) for blog in blog_list: blog_page = cstr(quote(blog.name.encode("utf-8"))) blog.link = urljoin(host, blog_page) blog.content = escape_html(blog.content or "") + blog.title = escape_html(blog.title or "") if blog_list: modified = max(blog["modified"] for blog in blog_list) From 41f8fb1a6839556adbbdfd97a3ac90777785fae6 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 10 Jan 2023 12:30:01 +0530 Subject: [PATCH 12/16] fix: Log settings - Handle validation failures (#19549) There's only one sane way to get past validations - remove the rows, so just do it instead of asking users to do it. [skip ci] --- .../core/doctype/log_settings/log_settings.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py index e1e1d5d176..eaa55f4bca 100644 --- a/frappe/core/doctype/log_settings/log_settings.py +++ b/frappe/core/doctype/log_settings/log_settings.py @@ -42,27 +42,26 @@ def _supports_log_clearing(doctype: str) -> bool: class LogSettings(Document): def validate(self): - self.validate_supported_doctypes() - self.validate_duplicates() + self._remove_unsupported_doctypes() + self._deduplicate_entries() self.add_default_logtypes() - def validate_supported_doctypes(self): - for entry in self.logs_to_clear: + def _remove_unsupported_doctypes(self): + for entry in list(self.logs_to_clear): if _supports_log_clearing(entry.ref_doctype): continue msg = _("{} does not support automated log clearing.").format(frappe.bold(entry.ref_doctype)) if frappe.conf.developer_mode: msg += "
" + _("Implement `clear_old_logs` method to enable auto error clearing.") - frappe.throw(msg, title=_("DocType not supported by Log Settings.")) + frappe.msgprint(msg, title=_("DocType not supported by Log Settings.")) + self.remove(entry) - def validate_duplicates(self): + def _deduplicate_entries(self): seen = set() - for entry in self.logs_to_clear: + for entry in list(self.logs_to_clear): if entry.ref_doctype in seen: - frappe.throw( - _("{} appears more than once in configured log doctypes.").format(entry.ref_doctype) - ) + self.remove(entry) seen.add(entry.ref_doctype) def add_default_logtypes(self): From 49fd3a5372823041dae1eb642aadfaf943155be3 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 10 Jan 2023 13:17:32 +0530 Subject: [PATCH 13/16] ci: update codecov.yml flags --- codecov.yml | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/codecov.yml b/codecov.yml index b16c49c8d6..3fca9d9384 100644 --- a/codecov.yml +++ b/codecov.yml @@ -9,7 +9,7 @@ coverage: target: auto threshold: 0.5% flags: - - server-mariadb + - server patch: default: target: 85% @@ -17,7 +17,7 @@ coverage: only_pulls: true if_ci_failed: ignore flags: - - server-mariadb + - server comment: layout: "diff, flags" @@ -25,11 +25,7 @@ comment: show_critical_paths: true flags: - server-mariadb: - paths: - - "**/*.py" - carryforward: true - server-postgres: + server: paths: - "**/*.py" carryforward: true From 0d62cbb470bc3fefe26dc54d9dc6bfd893f4fa31 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Tue, 10 Jan 2023 16:49:44 +0530 Subject: [PATCH 14/16] Update SECURITY.md --- SECURITY.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 1126c8b4da..cbe6016713 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,7 +1,7 @@ # Security Policy -The Frappe team and community take security issues in the Frappe Framework seriously. To report a security issue, fill out the form at [https://erpnext.com/security/report](https://erpnext.com/security/report). +The Frappe team and community take security issues in the Frappe Framework seriously. To report a security issue, please go through the information mentioned [here](https://frappe.io/security). You can help us make Frappe and consequently all Frappe dependent apps like [ERPNext](https://erpnext.com) more secure by following the [Reporting guidelines](https://erpnext.com/security). -We appreciate your efforts to responsibly disclose your findings. We'll endeavor to respond quickly, and will keep you updated throughout the process. \ No newline at end of file +We appreciate your efforts to responsibly disclose your findings. We'll endeavor to respond quickly, and will keep you updated throughout the process. From 894f0affcc9933c03c98863875ccd795c20f5513 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 10 Jan 2023 16:55:29 +0530 Subject: [PATCH 15/16] fix(DX): Print alter query if it fails during migration (#19552) --- frappe/database/mariadb/schema.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index 1173fa3591..13525d2328 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -1,3 +1,5 @@ +from pymysql.constants.ER import DUP_ENTRY + import frappe from frappe import _ from frappe.database.schema import DBTable @@ -115,17 +117,15 @@ class MariaDBTable(DBTable): frappe.db.sql(query) except Exception as e: - # sanitize - if e.args[0] == 1060: - frappe.throw(str(e)) - elif e.args[0] == 1062: + if query := locals().get("query"): # this weirdness is to avoid potentially unbounded vars + print(f"Failed to alter schema using query: {query}") + + if e.args[0] == DUP_ENTRY: fieldname = str(e).split("'")[-2] frappe.throw( _("{0} field cannot be set as unique in {1}, as there are non-unique existing values").format( fieldname, self.table_name ) ) - elif e.args[0] == 1067: - frappe.throw(str(e.args[1])) - else: - raise e + + raise From ed30a6d59f722e39ad3c4e1d3d77ed04f6b72cfd Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Tue, 10 Jan 2023 13:08:11 +0100 Subject: [PATCH 16/16] refactor(minor): use DocStatus (#19545) --- frappe/desk/form/save.py | 11 +++++++++-- frappe/model/delete_doc.py | 23 ++++++++++------------- frappe/www/printview.py | 4 ++-- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/frappe/desk/form/save.py b/frappe/desk/form/save.py index f43031c899..9ee2541a90 100644 --- a/frappe/desk/form/save.py +++ b/frappe/desk/form/save.py @@ -6,6 +6,7 @@ import json import frappe from frappe.core.doctype.submission_queue.submission_queue import queue_submission from frappe.desk.form.load import run_onload +from frappe.model.docstatus import DocStatus from frappe.monitor import add_data_to_monitor from frappe.utils.scheduler import is_scheduler_inactive @@ -17,8 +18,14 @@ def savedocs(doc, action): set_local_name(doc) # action - doc.docstatus = {"Save": 0, "Submit": 1, "Update": 1, "Cancel": 2}[action] - if doc.docstatus == 1: + doc.docstatus = { + "Save": DocStatus.draft(), + "Submit": DocStatus.submitted(), + "Update": DocStatus.submitted(), + "Cancel": DocStatus.cancelled(), + }[action] + + if doc.docstatus.is_submitted(): if action == "Submit" and doc.meta.queue_in_background and not is_scheduler_inactive(): queue_submission(doc, action) return diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index 1836896a9e..48eaa63460 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -9,6 +9,7 @@ import frappe.defaults import frappe.model.meta from frappe import _, get_module_path from frappe.desk.doctype.tag.tag import delete_tags_for_document +from frappe.model.docstatus import DocStatus from frappe.model.dynamic_links import get_dynamic_link_map from frappe.model.naming import revert_series_if_last from frappe.model.utils import is_virtual_doctype @@ -265,7 +266,7 @@ def check_if_doc_is_linked(doc, method="Delete"): # don't check for communication and todo! continue - if method != "Delete" and (method != "Cancel" or item.docstatus != 1): + if method != "Delete" and (method != "Cancel" or not DocStatus(item.docstatus).is_submitted()): # don't raise exception if not # linked to a non-cancelled doc when deleting or to a submitted doc when cancelling continue @@ -302,13 +303,12 @@ def check_if_doc_is_dynamically_linked(doc, method="Delete"): refdoc.get(df.options) == doc.doctype and refdoc.get(df.fieldname) == doc.name and ( - (method == "Delete" and refdoc.docstatus < 2) - or (method == "Cancel" and refdoc.docstatus == 1) + # linked to an non-cancelled doc when deleting + (method == "Delete" and not DocStatus(refdoc.docstatus).is_cancelled()) + # linked to a submitted doc when cancelling + or (method == "Cancel" and DocStatus(refdoc.docstatus).is_submitted()) ) ): - # raise exception only if - # linked to an non-cancelled doc when deleting - # or linked to a submitted doc when cancelling raise_link_exists_exception(doc, df.parent, df.parent) else: # dynamic link in table @@ -321,14 +321,11 @@ def check_if_doc_is_dynamically_linked(doc, method="Delete"): (doc.doctype, doc.name), as_dict=True, ): - - if (method == "Delete" and refdoc.docstatus < 2) or ( - method == "Cancel" and refdoc.docstatus == 1 + # linked to an non-cancelled doc when deleting + # or linked to a submitted doc when cancelling + if (method == "Delete" and not DocStatus(refdoc.docstatus).is_cancelled()) or ( + method == "Cancel" and DocStatus(refdoc.docstatus).is_submitted() ): - # raise exception only if - # linked to an non-cancelled doc when deleting - # or linked to a submitted doc when cancelling - reference_doctype = refdoc.parenttype if meta.istable else df.parent reference_docname = refdoc.parent if meta.istable else refdoc.name at_position = f"at Row: {refdoc.idx}" if meta.istable else "" diff --git a/frappe/www/printview.py b/frappe/www/printview.py index 56b5b14ece..f67aa9eee1 100644 --- a/frappe/www/printview.py +++ b/frappe/www/printview.py @@ -118,10 +118,10 @@ def get_rendered_template( validate_print_permission(doc) if doc.meta.is_submittable: - if doc.docstatus == 0 and not cint(print_settings.allow_print_for_draft): + if doc.docstatus.is_draft() and not cint(print_settings.allow_print_for_draft): frappe.throw(_("Not allowed to print draft documents"), frappe.PermissionError) - if doc.docstatus == 2 and not cint(print_settings.allow_print_for_cancelled): + if doc.docstatus.is_cancelled() and not cint(print_settings.allow_print_for_cancelled): frappe.throw(_("Not allowed to print cancelled documents"), frappe.PermissionError) doc.run_method("before_print", print_settings)