diff --git a/.github/helper/install.sh b/.github/helper/install.sh index 5a4d341a9b..5cdcbebe1a 100644 --- a/.github/helper/install.sh +++ b/.github/helper/install.sh @@ -54,8 +54,6 @@ fi echo "Starting Bench..." -export FRAPPE_TUNE_GC=True - bench start &> ~/frappe-bench/bench_start.log & if [ "$TYPE" == "server" ] diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index c563f9e43f..481041ed68 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -25,7 +25,7 @@ jobs: fetch-depth: 200 - uses: actions/setup-node@v3 with: - node-version: 16 + node-version: 18 check-latest: true - name: Check commit titles diff --git a/.github/workflows/on_release.yml b/.github/workflows/on_release.yml index 851b5b1d6a..c17a7c6639 100644 --- a/.github/workflows/on_release.yml +++ b/.github/workflows/on_release.yml @@ -22,7 +22,7 @@ jobs: - uses: actions/setup-node@v3 with: - node-version: 16 + node-version: 18 - uses: actions/setup-python@v4 with: diff --git a/.github/workflows/patch-mariadb-tests.yml b/.github/workflows/patch-mariadb-tests.yml index 3311f5235e..87cd530538 100644 --- a/.github/workflows/patch-mariadb-tests.yml +++ b/.github/workflows/patch-mariadb-tests.yml @@ -71,7 +71,7 @@ jobs: - name: Setup Node uses: actions/setup-node@v3 with: - node-version: 16 + node-version: 18 check-latest: true - name: Add to Hosts diff --git a/.github/workflows/publish-assets-develop.yml b/.github/workflows/publish-assets-develop.yml index 4feaebe15d..f42c3bc55c 100644 --- a/.github/workflows/publish-assets-develop.yml +++ b/.github/workflows/publish-assets-develop.yml @@ -16,7 +16,7 @@ jobs: path: 'frappe' - uses: actions/setup-node@v3 with: - node-version: 16 + node-version: 18 - uses: actions/setup-python@v4 with: python-version: '3.11' diff --git a/.github/workflows/server-tests.yml b/.github/workflows/server-tests.yml index 8ae0be0197..f5eac8e380 100644 --- a/.github/workflows/server-tests.yml +++ b/.github/workflows/server-tests.yml @@ -90,7 +90,7 @@ jobs: - uses: actions/setup-node@v3 with: - node-version: 16 + node-version: 18 check-latest: true - name: Add to Hosts diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml index 1b88bc73ce..bea00748e9 100644 --- a/.github/workflows/ui-tests.yml +++ b/.github/workflows/ui-tests.yml @@ -78,7 +78,7 @@ jobs: - uses: actions/setup-node@v3 with: - node-version: 16 + node-version: 18 check-latest: true - name: Add to Hosts diff --git a/cypress/integration/form.js b/cypress/integration/form.js index 8186647a14..bbd89ddb7a 100644 --- a/cypress/integration/form.js +++ b/cypress/integration/form.js @@ -59,11 +59,13 @@ context("Form", () => { .blur(); cy.click_listview_row_item_with_text("Test Form Contact 3"); + cy.scrollTo(0); cy.get("#page-Contact .page-head").findByTitle("Test Form Contact 3").should("exist"); cy.get(".prev-doc").should("be.visible").click(); cy.get(".msgprint-dialog .modal-body").contains("No further records").should("be.visible"); cy.hide_dialog(); + cy.scrollTo(0); cy.get("#page-Contact .page-head").findByTitle("Test Form Contact 3").should("exist"); cy.get(".next-doc").should("be.visible").click(); cy.get(".msgprint-dialog .modal-body").contains("No further records").should("be.visible"); @@ -109,22 +111,6 @@ context("Form", () => { cy.get("@email_input2").should("not.have.class", "invalid"); }); - it("Shows version conflict warning", { scrollBehavior: false }, () => { - cy.visit("/app/todo"); - - cy.insert_doc("ToDo", { description: "old" }).then((doc) => { - cy.visit(`/app/todo/${doc.name}`); - // make form dirty - cy.fill_field("status", "Cancelled", "Select"); - - // update doc using api - simulating parallel change by another user - cy.update_doc("ToDo", doc.name, { status: "Closed" }).then(() => { - cy.findByRole("button", { name: "Refresh" }).click(); - cy.get_field("status", "Select").should("have.value", "Closed"); - }); - }); - }); - it("Jump to field in collapsed section", { scrollBehavior: false }, () => { cy.new_form("User"); diff --git a/cypress/integration/list_view.js b/cypress/integration/list_view.js index 3fa0758f0c..b07f18edc2 100644 --- a/cypress/integration/list_view.js +++ b/cypress/integration/list_view.js @@ -13,15 +13,8 @@ context("List View", () => { it("Keep checkbox checked after Refresh", { scrollBehavior: false }, () => { cy.go_to_list("ToDo"); cy.clear_filters(); - cy.get(".list-row-container .list-row-checkbox").click({ - multiple: true, - force: true, - }); - cy.get(".actions-btn-group button").contains("Actions").should("be.visible"); - cy.intercept("/api/method/frappe.desk.reportview.get").as("list-refresh"); - cy.wait(3000); // wait before you hit another refresh - cy.get('button[data-original-title="Refresh"]').click(); - cy.wait("@list-refresh"); + cy.get(".list-header-subject > .list-subject > .list-check-all").click(); + cy.get("button[data-original-title='Refresh']").click(); cy.get(".list-row-container .list-row-checkbox:checked").should("be.visible"); }); @@ -39,11 +32,8 @@ context("List View", () => { ]; cy.go_to_list("ToDo"); cy.clear_filters(); - cy.get('.list-row-container:contains("Pending") .list-row-checkbox').click({ - multiple: true, - force: true, - }); - cy.get(".actions-btn-group button").contains("Actions").should("be.visible").click(); + cy.get(".list-header-subject > .list-subject > .list-check-all").click(); + cy.findByRole("button", { name: "Actions" }).click(); cy.get(".dropdown-menu li:visible .dropdown-item") .should("have.length", 9) .each((el, index) => { @@ -56,8 +46,7 @@ context("List View", () => { }).as("bulk-approval"); cy.wrap(elements).contains("Approve").click(); cy.wait("@bulk-approval"); - cy.wait(300); - cy.get_open_dialog().find(".btn-modal-close").click(); + cy.hide_dialog(); cy.reload(); cy.clear_filters(); cy.get(".list-row-container:visible").should("contain", "Approved"); diff --git a/cypress/integration/socket_updates.js b/cypress/integration/socket_updates.js new file mode 100644 index 0000000000..d0642e1b03 --- /dev/null +++ b/cypress/integration/socket_updates.js @@ -0,0 +1,80 @@ +context("Realtime updates", () => { + before(() => { + cy.login(); + }); + + beforeEach(() => { + cy.visit("/app/todo"); + // required because immediately after load socket is still connecting. + // Not a huge deal breaker in prod. + cy.wait(500); + cy.clear_filters(); + }); + + it("Shows version conflict warning", { scrollBehavior: false }, () => { + cy.insert_doc("ToDo", { description: "old" }).then((doc) => { + cy.visit(`/app/todo/${doc.name}`); + // make form dirty + cy.fill_field("status", "Cancelled", "Select"); + + // update doc using api - simulating parallel change by another user + cy.update_doc("ToDo", doc.name, { status: "Closed" }).then(() => { + cy.findByRole("button", { name: "Refresh" }).click(); + cy.get_field("status", "Select").should("have.value", "Closed"); + }); + }); + }); + + it("List view updates in realtime on insert", { scrollBehavior: false }, () => { + const original = "Added for realtime update"; + const updated = "Updated for realtime update"; + cy.insert_doc("ToDo", { description: original }).then((doc) => { + cy.contains(original).should("be.visible"); + + // update doc using api - simulating parallel change by another user + cy.update_doc("ToDo", doc.name, { description: updated }).then(() => { + cy.contains(updated).should("be.visible"); + }); + }); + }); + + it("Recieves msgprint from server", { scrollBehavior: false }, () => { + // required because immediately after load socket is still connecting. + // Not a deal breaker in prod + const msg = "msgprint sent via realtime"; + publish_realtime({ event: "msgprint", message: msg }).then(() => { + cy.contains(msg).should("be.visible"); + }); + }); + + it("Recieves custom messages from server", { scrollBehavior: false }, () => { + const event = "cypress_event"; + let handler = { + handle() { + console.log("clear"); + }, + }; + cy.spy(handler, "handle").as("callback"); + + cy.window() + .its("frappe") + .then((frappe) => { + frappe.realtime.on(event, () => handler.handle()); + }); + + publish_realtime({ event }).then(() => { + cy.get("@callback").should("be.called"); + }); + }); + + it("Progress bar", { scrollBehavior: false }, () => { + const title = "RealTime Progress"; + cy.call("frappe.tests.ui_test_helpers.publish_progress", { title }).then(() => { + cy.contains(title).should("be.visible"); + }); + }); +}); + +function publish_realtime(args) { + return cy.call("frappe.tests.ui_test_helpers.publish_realtime", args); +} diff --git a/esbuild/esbuild.js b/esbuild/esbuild.js index e1594aa651..1476db3c20 100644 --- a/esbuild/esbuild.js +++ b/esbuild/esbuild.js @@ -60,6 +60,11 @@ const argv = yargs type: "boolean", description: "Run build command for apps", }) + .option("save-metafiles", { + type: "boolean", + description: + "Saves esbuild metafiles for built assets. Useful for analyzing bundle size. More info: https://esbuild.github.io/api/#metafile", + }) .example("node esbuild --apps frappe,erpnext", "Run build only for frappe and erpnext") .example( "node esbuild --files frappe/website.bundle.js,frappe/desk.bundle.js", @@ -401,6 +406,13 @@ async function write_assets_json(metafile) { await fs.promises.writeFile(assets_json_path, JSON.stringify(new_assets_json, null, 4)); await update_assets_json_in_cache(); + if (argv["save-metafiles"]) { + // use current timestamp in readable formate as a suffix for filename + let current_timestamp = new Date().getTime(); + const metafile_name = `meta-${current_timestamp}.json`; + await fs.promises.writeFile(`${metafile_name}`, JSON.stringify(metafile)); + log(`Saved metafile as ${metafile_name}`); + } return { new_assets_json, prev_assets_json, diff --git a/frappe/__init__.py b/frappe/__init__.py index 998d881a13..88b995d17b 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -58,7 +58,7 @@ re._MAXCACHE = ( 50 # reduced from default 512 given we are already maintaining this on parent worker ) -_tune_gc = bool(os.environ.get("FRAPPE_TUNE_GC", False)) +_tune_gc = bool(sbool(os.environ.get("FRAPPE_TUNE_GC", True))) if _dev_server: warnings.simplefilter("always", DeprecationWarning) @@ -2213,41 +2213,6 @@ def logger( ) -def log_error(title=None, message=None, reference_doctype=None, reference_name=None): - """Log error to Error Log""" - # Parameter ALERT: - # the title and message may be swapped - # the better API for this is log_error(title, message), and used in many cases this way - # this hack tries to be smart about whats a title (single line ;-)) and fixes it - - traceback = None - if message: - if "\n" in title: # traceback sent as title - traceback, title = title, message - else: - traceback = message - - title = title or "Error" - traceback = as_unicode(traceback or get_traceback(with_context=True)) - - if not db: - print(f"Failed to log error in db: {title}") - return - - error_log = get_doc( - doctype="Error Log", - error=traceback, - method=title, - reference_doctype=reference_doctype, - reference_name=reference_name, - ) - - if flags.read_only: - error_log.deferred_insert() - else: - return error_log.insert(ignore_permissions=True) - - def get_desk_link(doctype, name): html = ( '{doctype_local} {name}' @@ -2439,6 +2404,8 @@ def validate_and_sanitize_search_inputs(fn): return wrapper +from frappe.utils.error import log_error # noqa: backward compatibility + if _tune_gc: # generational GC gets triggered after certain allocs (g0) which is 700 by default. # This number is quite small for frappe where a single query can potentially create 700+ diff --git a/frappe/app.py b/frappe/app.py index 5113c858a5..1cbdca1361 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -22,7 +22,7 @@ from frappe import _ from frappe.auth import SAFE_HTTP_METHODS, UNSAFE_HTTP_METHODS, HTTPRequest from frappe.middlewares import StaticDataMiddleware from frappe.utils import cint, get_site_name, sanitize_html -from frappe.utils.error import make_error_snapshot +from frappe.utils.error import log_error_snapshot from frappe.website.serve import get_response local_manager = LocalManager(frappe.local) @@ -346,7 +346,7 @@ def handle_exception(e): frappe.local.login_manager.clear_cookies() if http_status_code >= 500: - make_error_snapshot(e) + log_error_snapshot(e) if return_as_message: response = get_response("message", http_status_code=http_status_code) diff --git a/frappe/auth.py b/frappe/auth.py index 29c3e41694..d1259e1aaf 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -349,8 +349,6 @@ class CookieManager: expires = datetime.datetime.now() + datetime.timedelta(days=3) if frappe.session.sid: self.set_cookie("sid", frappe.session.sid, expires=expires, httponly=True) - if frappe.session.session_country: - self.set_cookie("country", frappe.session.session_country) def set_cookie(self, key, value, expires=None, secure=False, httponly=False, samesite="Lax"): if not secure and hasattr(frappe.local, "request"): diff --git a/frappe/build.py b/frappe/build.py index 18ceff1d7a..5a9855ef16 100644 --- a/frappe/build.py +++ b/frappe/build.py @@ -229,6 +229,7 @@ def bundle( verbose=False, skip_frappe=False, files=None, + save_metafiles=False, ): """concat / minify js files""" setup() @@ -248,6 +249,9 @@ def bundle( command += " --run-build-command" + if save_metafiles: + command += " --save-metafiles" + check_node_executable() frappe_app_path = frappe.get_app_path("frappe", "..") frappe.commands.popen(command, cwd=frappe_app_path, env=get_node_env(), raise_err=True) @@ -274,8 +278,8 @@ def watch(apps=None): def check_node_executable(): node_version = Version(subprocess.getoutput("node -v")[1:]) warn = "⚠️ " - if node_version.major < 14: - click.echo(f"{warn} Please update your node version to 14") + if node_version.major < 18: + click.echo(f"{warn} Please update your node version to 18") if not shutil.which("yarn"): click.echo(f"{warn} Please install yarn using below command and try again.\nnpm install -g yarn") click.echo() diff --git a/frappe/commands/scheduler.py b/frappe/commands/scheduler.py index 36fa81f8a5..5d453e3568 100755 --- a/frappe/commands/scheduler.py +++ b/frappe/commands/scheduler.py @@ -215,18 +215,39 @@ def start_worker( ) +@click.command("worker-pool") +@click.option( + "--queue", + type=str, + help="Queue to consume from. Multiple queues can be specified using comma-separated string. If not specified all queues are consumed.", +) +@click.option("--num-workers", type=int, default=2, help="Number of workers to spawn in pool.") +@click.option("--quiet", is_flag=True, default=False, help="Hide Log Outputs") +@click.option("--burst", is_flag=True, default=False, help="Run Worker in Burst mode.") +def start_worker_pool(queue, quiet=False, num_workers=2, burst=False): + """Start a backgrond worker""" + from frappe.utils.background_jobs import start_worker_pool + + start_worker_pool( + queue=queue, + quiet=quiet, + burst=burst, + num_workers=num_workers, + ) + + @click.command("ready-for-migration") @click.option("--site", help="site name") @pass_context def ready_for_migration(context, site=None): - from frappe.utils.doctor import get_pending_jobs + from frappe.utils.doctor import any_job_pending if not site: site = get_site(context) try: frappe.init(site=site) - pending_jobs = get_pending_jobs(site=site) + pending_jobs = any_job_pending(site=site) if pending_jobs: print(f"NOT READY for migration: site {site} has pending background jobs") @@ -251,5 +272,6 @@ commands = [ show_pending_jobs, start_scheduler, start_worker, + start_worker_pool, trigger_scheduler_event, ] diff --git a/frappe/commands/site.py b/frappe/commands/site.py index d606bb78cf..01b3be9590 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -700,9 +700,12 @@ def _use(site, sites_path="."): def use(site, sites_path="."): + from frappe.installer import update_site_config + if os.path.exists(os.path.join(sites_path, site)): - with open(os.path.join(sites_path, "currentsite.txt"), "w") as sitefile: - sitefile.write(site) + sites_path = os.getcwd() + conifg = os.path.join(sites_path, "common_site_config.json") + update_site_config("default_site", site, validate=False, site_config_path=conifg) print(f"Current Site set to {site}") else: print(f"Site {site} does not exist") diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 7e4b1e8e46..e77376b693 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -31,6 +31,12 @@ EXTRA_ARGS_CTX = {"ignore_unknown_options": True, "allow_extra_args": True} @click.option( "--force", is_flag=True, default=False, help="Force build assets instead of downloading available" ) +@click.option( + "--save-metafiles", + is_flag=True, + default=False, + help="Saves esbuild metafiles for built assets. Useful for analyzing bundle size. More info: https://esbuild.github.io/api/#metafile", +) def build( app=None, apps=None, @@ -38,6 +44,7 @@ def build( production=False, verbose=False, force=False, + save_metafiles=False, ): "Compile JS and CSS source files" from frappe.build import bundle, download_frappe_assets @@ -62,7 +69,14 @@ def build( if production: mode = "production" - bundle(mode, apps=apps, hard_link=hard_link, verbose=verbose, skip_frappe=skip_frappe) + bundle( + mode, + apps=apps, + hard_link=hard_link, + verbose=verbose, + skip_frappe=skip_frappe, + save_metafiles=save_metafiles, + ) @click.command("watch") diff --git a/frappe/core/doctype/docshare/test_docshare.py b/frappe/core/doctype/docshare/test_docshare.py index e080b0d4ff..125e829d9b 100644 --- a/frappe/core/doctype/docshare/test_docshare.py +++ b/frappe/core/doctype/docshare/test_docshare.py @@ -33,13 +33,28 @@ class TestDocShare(FrappeTestCase): def test_doc_permission(self): frappe.set_user(self.user) + self.assertFalse(self.event.has_permission()) frappe.set_user("Administrator") frappe.share.add("Event", self.event.name, self.user) frappe.set_user(self.user) - self.assertTrue(self.event.has_permission()) + # PERF: All share permission check should happen with maximum 1 query. + with self.assertRowsRead(1): + self.assertTrue(self.event.has_permission()) + + second_event = frappe.get_doc( + { + "doctype": "Event", + "subject": "test share event 2", + "starts_on": "2015-01-01 10:00:00", + "event_type": "Private", + } + ).insert() + frappe.share.add("Event", second_event.name, self.user) + with self.assertRowsRead(1): + self.assertTrue(self.event.has_permission()) def test_share_permission(self): frappe.share.add("Event", self.event.name, self.user, write=1, share=1) diff --git a/frappe/core/doctype/error_snapshot/__init__.py b/frappe/core/doctype/error_snapshot/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/frappe/core/doctype/error_snapshot/error_object.html b/frappe/core/doctype/error_snapshot/error_object.html deleted file mode 100644 index 450bfacfc6..0000000000 --- a/frappe/core/doctype/error_snapshot/error_object.html +++ /dev/null @@ -1,12 +0,0 @@ -{% if (Object.prototype.toString.call(x) === "[object Object]") { %} - - {% for (var key in x) { %} - - - - - {% } %} -
{{ key }}{{ x[key] }}
-{% } else { %} - {{ x }} -{% } %} diff --git a/frappe/core/doctype/error_snapshot/error_snapshot.html b/frappe/core/doctype/error_snapshot/error_snapshot.html deleted file mode 100644 index 6f449e0fe9..0000000000 --- a/frappe/core/doctype/error_snapshot/error_snapshot.html +++ /dev/null @@ -1,77 +0,0 @@ - -{% function id(){ return id._old_id++; }; id._old_id = 0; %} -

{{ __("Error Report") }}

-

{{ doc.pyver }}

-
-
{{ __("Timestamp") }}:
-
{{ doc.timestamp }}
-
{{ __("Relapsed") }}
-
{{ doc.relapses }}
-
- -

{{ __("Exception") }}

-{{ frappe.render_template("error_object", {x: JSON.parse(doc.exception)}) }} - -

{{ __("Locals") }}

-{{ frappe.render_template("error_object", {x: JSON.parse(doc.locals)} )}} - -

{{ __("Traceback") }}

-{% var frames = JSON.parse(doc.frames); %} -{% for (var i in frames) { %} - {% var frameid = id(), frame = frames[i] %} -

{{ frame.file }}: {{ frame.lnum }} -

-
-
- {% for (var index in frame.lines) { %} - {% var line = frame.lines[index] %} -
- {{ index }} - {{ line }} -
- {% } %} -
-
- - {{ __("Locals") }} - -
-
-
-
-
-

{{ __("Locals") }}

- {{ frappe.render_template("error_object", {x: frame.dump }) }} -
-
-

-{% } %} diff --git a/frappe/core/doctype/error_snapshot/error_snapshot.js b/frappe/core/doctype/error_snapshot/error_snapshot.js deleted file mode 100644 index f8a7e3ded5..0000000000 --- a/frappe/core/doctype/error_snapshot/error_snapshot.js +++ /dev/null @@ -1,20 +0,0 @@ -frappe.ui.form.on("Error Snapshot", "load", function (frm) { - frm.set_read_only(true); -}); - -frappe.ui.form.on("Error Snapshot", "refresh", function (frm) { - frm.set_df_property( - "view", - "options", - frappe.render_template("error_snapshot", { doc: frm.doc }) - ); - - if (frm.doc.relapses) { - frm.add_custom_button(__("Show Relapses"), function () { - frappe.route_options = { - parent_error_snapshot: frm.doc.name, - }; - frappe.set_route("List", "Error Snapshot"); - }); - } -}); diff --git a/frappe/core/doctype/error_snapshot/error_snapshot.json b/frappe/core/doctype/error_snapshot/error_snapshot.json deleted file mode 100644 index b92db8f99a..0000000000 --- a/frappe/core/doctype/error_snapshot/error_snapshot.json +++ /dev/null @@ -1,130 +0,0 @@ -{ - "actions": [], - "creation": "2015-11-28 00:57:39.766888", - "doctype": "DocType", - "document_type": "System", - "engine": "InnoDB", - "field_order": [ - "view", - "seen", - "evalue", - "timestamp", - "relapses", - "etype", - "traceback", - "parent_error_snapshot", - "pyver", - "exception", - "locals", - "frames" - ], - "fields": [ - { - "fieldname": "view", - "fieldtype": "HTML", - "label": "Snapshot View" - }, - { - "default": "0", - "fieldname": "seen", - "fieldtype": "Check", - "hidden": 1, - "in_filter": 1, - "label": "Seen" - }, - { - "fieldname": "evalue", - "fieldtype": "Code", - "hidden": 1, - "in_list_view": 1, - "label": "Friendly Title", - "read_only": 1 - }, - { - "fieldname": "timestamp", - "fieldtype": "Datetime", - "hidden": 1, - "label": "Timestamp", - "read_only": 1 - }, - { - "default": "1", - "fieldname": "relapses", - "fieldtype": "Int", - "hidden": 1, - "in_list_view": 1, - "label": "Relapses", - "read_only": 1 - }, - { - "fieldname": "etype", - "fieldtype": "Data", - "hidden": 1, - "label": "Exception Type", - "read_only": 1 - }, - { - "fieldname": "traceback", - "fieldtype": "Code", - "hidden": 1, - "label": "Traceback", - "read_only": 1 - }, - { - "fieldname": "parent_error_snapshot", - "fieldtype": "Data", - "hidden": 1, - "label": "Parent Error Snapshot" - }, - { - "fieldname": "pyver", - "fieldtype": "Code", - "hidden": 1, - "label": "Pyver", - "read_only": 1 - }, - { - "fieldname": "exception", - "fieldtype": "Code", - "hidden": 1, - "label": "Exception" - }, - { - "fieldname": "locals", - "fieldtype": "Code", - "hidden": 1, - "label": "Locals" - }, - { - "fieldname": "frames", - "fieldtype": "Code", - "hidden": 1, - "label": "Frames" - } - ], - "in_create": 1, - "links": [], - "modified": "2022-08-03 12:20:53.504160", - "modified_by": "Administrator", - "module": "Core", - "name": "Error Snapshot", - "owner": "Administrator", - "permissions": [ - { - "create": 1, - "delete": 1, - "email": 1, - "export": 1, - "print": 1, - "read": 1, - "report": 1, - "role": "Administrator", - "share": 1, - "write": 1 - } - ], - "sort_field": "timestamp", - "sort_order": "DESC", - "states": [], - "title_field": "evalue" -} \ No newline at end of file diff --git a/frappe/core/doctype/error_snapshot/error_snapshot.py b/frappe/core/doctype/error_snapshot/error_snapshot.py deleted file mode 100644 index acc49c78cd..0000000000 --- a/frappe/core/doctype/error_snapshot/error_snapshot.py +++ /dev/null @@ -1,40 +0,0 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and contributors -# License: MIT. See LICENSE - -import frappe -from frappe.model.document import Document -from frappe.query_builder import Interval -from frappe.query_builder.functions import Now - - -class ErrorSnapshot(Document): - no_feed_on_delete = True - - def onload(self): - if not self.parent_error_snapshot: - self.db_set("seen", 1, update_modified=False) - - for relapsed in frappe.get_all("Error Snapshot", filters={"parent_error_snapshot": self.name}): - frappe.db.set_value("Error Snapshot", relapsed.name, "seen", 1, update_modified=False) - - frappe.local.flags.commit = True - - def validate(self): - parent = frappe.get_all( - "Error Snapshot", - filters={"evalue": self.evalue, "parent_error_snapshot": ""}, - fields=["name", "relapses", "seen"], - limit_page_length=1, - ) - - if parent: - parent = parent[0] - self.update({"parent_error_snapshot": parent["name"]}) - frappe.db.set_value("Error Snapshot", parent["name"], "relapses", parent["relapses"] + 1) - if parent["seen"]: - frappe.db.set_value("Error Snapshot", parent["name"], "seen", 0) - - @staticmethod - def clear_old_logs(days=30): - table = frappe.qb.DocType("Error Snapshot") - frappe.db.delete(table, filters=(table.modified < (Now() - Interval(days=days)))) diff --git a/frappe/core/doctype/error_snapshot/error_snapshot_list.js b/frappe/core/doctype/error_snapshot/error_snapshot_list.js deleted file mode 100644 index b331788852..0000000000 --- a/frappe/core/doctype/error_snapshot/error_snapshot_list.js +++ /dev/null @@ -1,19 +0,0 @@ -frappe.listview_settings["Error Snapshot"] = { - add_fields: ["parent_error_snapshot", "relapses", "seen"], - filters: [ - ["parent_error_snapshot", "=", null], - ["seen", "=", false], - ], - get_indicator: function (doc) { - if (doc.parent_error_snapshot && doc.parent_error_snapshot.length) { - return [__("Relapsed"), !doc.seen ? "orange" : "blue", "parent_error_snapshot,!=,"]; - } else { - return [__("First Level"), !doc.seen ? "red" : "green", "parent_error_snapshot,=,"]; - } - }, - onload: function (listview) { - frappe.require("logtypes.bundle.js", () => { - frappe.utils.logtypes.show_log_retention_message(cur_list.doctype); - }); - }, -}; diff --git a/frappe/core/doctype/error_snapshot/test_error_snapshot.py b/frappe/core/doctype/error_snapshot/test_error_snapshot.py deleted file mode 100644 index 4779d56c7b..0000000000 --- a/frappe/core/doctype/error_snapshot/test_error_snapshot.py +++ /dev/null @@ -1,11 +0,0 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors -# License: MIT. See LICENSE -from frappe.tests.utils import FrappeTestCase -from frappe.utils.logger import sanitized_dict - -# test_records = frappe.get_test_records('Error Snapshot') - - -class TestErrorSnapshot(FrappeTestCase): - def test_form_dict_sanitization(self): - self.assertNotEqual(sanitized_dict({"pwd": "SECRET", "usr": "WHAT"}).get("pwd"), "SECRET") diff --git a/frappe/core/doctype/file/file.json b/frappe/core/doctype/file/file.json index d6c4a99bc3..6c64bfe274 100644 --- a/frappe/core/doctype/file/file.json +++ b/frappe/core/doctype/file/file.json @@ -174,7 +174,7 @@ "icon": "fa fa-file", "idx": 1, "links": [], - "modified": "2022-09-13 15:50:15.508251", + "modified": "2023-05-02 15:42:14.274901", "modified_by": "Administrator", "module": "Core", "name": "File", @@ -196,14 +196,8 @@ { "create": 1, "delete": 1, - "email": 1, - "export": 1, - "if_owner": 1, - "print": 1, "read": 1, - "report": 1, "role": "All", - "share": 1, "write": 1 } ], diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index 7770226e79..8778826b56 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -16,6 +16,7 @@ import frappe from frappe import _ from frappe.database.schema import SPECIAL_CHAR_PATTERN from frappe.model.document import Document +from frappe.permissions import get_doctypes_with_read from frappe.utils import call_hook_method, cint, get_files_path, get_hook_method, get_url from frappe.utils.file_manager import is_safe_path from frappe.utils.image import optimize_image, strip_exif_data @@ -718,40 +719,39 @@ def on_doctype_update(): def has_permission(doc, ptype=None, user=None): - has_access = False user = user or frappe.session.user if ptype == "create": - has_access = frappe.has_permission("File", "create", user=user) + return frappe.has_permission("File", "create", user=user) - if not doc.is_private or doc.owner in [user, "Guest"] or user == "Administrator": - has_access = True + if not doc.is_private or doc.owner == user or user == "Administrator": + return True if doc.attached_to_doctype and doc.attached_to_name: attached_to_doctype = doc.attached_to_doctype attached_to_name = doc.attached_to_name - try: - ref_doc = frappe.get_doc(attached_to_doctype, attached_to_name) + ref_doc = frappe.get_doc(attached_to_doctype, attached_to_name) - if ptype in ["write", "create", "delete"]: - has_access = ref_doc.has_permission("write") + if ptype in ["write", "create", "delete"]: + return ref_doc.has_permission("write") + else: + return ref_doc.has_permission("read") - if ptype == "delete" and not has_access: - frappe.throw( - _( - "Cannot delete file as it belongs to {0} {1} for which you do not have permissions" - ).format(doc.attached_to_doctype, doc.attached_to_name), - frappe.PermissionError, - ) - else: - has_access = ref_doc.has_permission("read") - except frappe.DoesNotExistError: - # if parent doc is not created before file is created - # we cannot check its permission so we will use file's permission - pass + return False - return has_access + +def get_permission_query_conditions(user: str = None) -> str: + user = user or frappe.session.user + if user == "Administrator": + return "" + + readable_doctypes = ", ".join(repr(dt) for dt in get_doctypes_with_read()) + return f""" + (`tabFile`.`is_private` = 0) + OR (`tabFile`.`attached_to_doctype` IS NULL AND `tabFile`.`owner` = {user !r}) + OR (`tabFile`.`attached_to_doctype` IN ({readable_doctypes})) + """ # Note: kept at the end to not cause circular, partial imports & maintain backwards compatibility diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py index bbe8bb6d1a..1e7e698062 100644 --- a/frappe/core/doctype/file/test_file.py +++ b/frappe/core/doctype/file/test_file.py @@ -611,46 +611,42 @@ class TestAttachmentsAccess(FrappeTestCase): def setUp(self) -> None: frappe.db.delete("File", {"is_folder": 0}) - def test_attachments_access(self): + def test_list_private_attachments(self): frappe.set_user("test4@example.com") self.attached_to_doctype, self.attached_to_docname = make_test_doc() - frappe.get_doc( - { - "doctype": "File", - "file_name": "test_user.txt", - "attached_to_doctype": self.attached_to_doctype, - "attached_to_name": self.attached_to_docname, - "content": "Testing User", - } + frappe.new_doc( + "File", + file_name="test_user_attachment.txt", + attached_to_doctype=self.attached_to_doctype, + attached_to_name=self.attached_to_docname, + content="Testing User", + is_private=1, ).insert() - frappe.get_doc( - { - "doctype": "File", - "file_name": "test_user_home.txt", - "content": "User Home", - } + frappe.new_doc( + "File", + file_name="test_user_standalone.txt", + content="User Home", + is_private=1, ).insert() frappe.set_user("test@example.com") - frappe.get_doc( - { - "doctype": "File", - "file_name": "test_system_manager.txt", - "attached_to_doctype": self.attached_to_doctype, - "attached_to_name": self.attached_to_docname, - "content": "Testing System Manager", - } + frappe.new_doc( + "File", + file_name="test_sm_attachment.txt", + attached_to_doctype=self.attached_to_doctype, + attached_to_name=self.attached_to_docname, + content="Testing System Manager", + is_private=1, ).insert() - frappe.get_doc( - { - "doctype": "File", - "file_name": "test_sm_home.txt", - "content": "System Manager Home", - } + frappe.new_doc( + "File", + file_name="test_sm_standalone.txt", + content="System Manager Home", + is_private=1, ).insert() system_manager_files = [file.file_name for file in get_files_in_folder("Home")["files"]] @@ -664,15 +660,47 @@ class TestAttachmentsAccess(FrappeTestCase): file.file_name for file in get_files_in_folder("Home/Attachments")["files"] ] - self.assertIn("test_sm_home.txt", system_manager_files) - self.assertNotIn("test_sm_home.txt", user_files) - self.assertIn("test_user_home.txt", system_manager_files) - self.assertIn("test_user_home.txt", user_files) + self.assertIn("test_sm_standalone.txt", system_manager_files) + self.assertNotIn("test_sm_standalone.txt", user_files) - self.assertIn("test_system_manager.txt", system_manager_attachments_files) - self.assertNotIn("test_system_manager.txt", user_attachments_files) - self.assertIn("test_user.txt", system_manager_attachments_files) - self.assertIn("test_user.txt", user_attachments_files) + self.assertIn("test_user_standalone.txt", user_files) + self.assertNotIn("test_user_standalone.txt", system_manager_files) + + self.assertIn("test_sm_attachment.txt", system_manager_attachments_files) + self.assertIn("test_sm_attachment.txt", user_attachments_files) + self.assertIn("test_user_attachment.txt", system_manager_attachments_files) + self.assertIn("test_user_attachment.txt", user_attachments_files) + + def test_list_public_single_file(self): + """Ensure that users are able to list public standalone files.""" + frappe.set_user("test@example.com") + frappe.new_doc( + "File", + file_name="test_public_single.txt", + content="Public single File", + is_private=0, + ).insert() + + frappe.set_user("test4@example.com") + files = [file.file_name for file in get_files_in_folder("Home")["files"]] + self.assertIn("test_public_single.txt", files) + + def test_list_public_attachment(self): + """Ensure that users are able to list public attachments.""" + frappe.set_user("test@example.com") + self.attached_to_doctype, self.attached_to_docname = make_test_doc() + frappe.new_doc( + "File", + file_name="test_public_attachment.txt", + attached_to_doctype=self.attached_to_doctype, + attached_to_name=self.attached_to_docname, + content="Public Attachment", + is_private=0, + ).insert() + + frappe.set_user("test4@example.com") + files = [file.file_name for file in get_files_in_folder("Home/Attachments")["files"]] + self.assertIn("test_public_attachment.txt", files) def tearDown(self) -> None: frappe.set_user("Administrator") diff --git a/frappe/core/doctype/log_settings/log_settings.py b/frappe/core/doctype/log_settings/log_settings.py index 832be49f3c..623e358b6c 100644 --- a/frappe/core/doctype/log_settings/log_settings.py +++ b/frappe/core/doctype/log_settings/log_settings.py @@ -14,7 +14,6 @@ DEFAULT_LOGTYPES_RETENTION = { "Error Log": 30, "Activity Log": 90, "Email Queue": 30, - "Error Snapshot": 30, "Scheduled Job Log": 90, "Route History": 90, "Submission Queue": 30, @@ -45,11 +44,11 @@ def _supports_log_clearing(doctype: str) -> bool: class LogSettings(Document): def validate(self): - self._remove_unsupported_doctypes() + self.remove_unsupported_doctypes() self._deduplicate_entries() self.add_default_logtypes() - def _remove_unsupported_doctypes(self): + def remove_unsupported_doctypes(self): for entry in list(self.logs_to_clear): if _supports_log_clearing(entry.ref_doctype): continue @@ -114,6 +113,7 @@ class LogSettings(Document): def run_log_clean_up(): doc = frappe.get_doc("Log Settings") + doc.remove_unsupported_doctypes() doc.add_default_logtypes() doc.save() doc.clear_logs() @@ -156,7 +156,6 @@ LOG_DOCTYPES = [ "Route History", "Email Queue", "Email Queue Recipient", - "Error Snapshot", "Error Log", ] diff --git a/frappe/core/doctype/log_settings/test_log_settings.py b/frappe/core/doctype/log_settings/test_log_settings.py index d7f43a181d..edee098553 100644 --- a/frappe/core/doctype/log_settings/test_log_settings.py +++ b/frappe/core/doctype/log_settings/test_log_settings.py @@ -62,7 +62,6 @@ class TestLogSettings(FrappeTestCase): "Activity Log", "Email Queue", "Route History", - "Error Snapshot", "Scheduled Job Log", ] diff --git a/frappe/core/doctype/prepared_report/prepared_report.json b/frappe/core/doctype/prepared_report/prepared_report.json index fb3809e481..b64b91c4ef 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.json +++ b/frappe/core/doctype/prepared_report/prepared_report.json @@ -25,7 +25,8 @@ "fieldtype": "Data", "label": "Report Name", "read_only": 1, - "reqd": 1 + "reqd": 1, + "search_index": 1 }, { "default": "Queued", @@ -35,8 +36,9 @@ "in_list_view": 1, "in_standard_filter": 1, "label": "Status", - "options": "Error\nQueued\nCompleted", - "read_only": 1 + "options": "Error\nQueued\nCompleted\nStarted", + "read_only": 1, + "search_index": 1 }, { "fieldname": "column_break_4", @@ -104,7 +106,7 @@ ], "in_create": 1, "links": [], - "modified": "2023-05-19 15:41:03.428589", + "modified": "2023-07-01 18:29:12.700239", "modified_by": "Administrator", "module": "Core", "name": "Prepared Report", diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index ed7f4711aa..30efa8eb91 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -3,6 +3,7 @@ import json +from contextlib import suppress from typing import Any from rq import get_current_job @@ -12,9 +13,13 @@ from frappe.desk.form.load import get_attachments from frappe.desk.query_report import generate_report_result from frappe.model.document import Document from frappe.monitor import add_data_to_monitor -from frappe.utils import gzip_compress, gzip_decompress +from frappe.utils import add_to_date, gzip_compress, gzip_decompress, now from frappe.utils.background_jobs import enqueue +# If prepared report runs for longer than this time it's automatically considered as failed +FAILURE_THRESHOLD = 60 * 60 +REPORT_TIMEOUT = 25 * 60 + class PreparedReport(Document): @property @@ -38,12 +43,21 @@ class PreparedReport(Document): def before_insert(self): self.status = "Queued" + def on_trash(self): + # If job is running then send stop signal. + if self.status != "Started": + return + + with suppress(Exception): + job = frappe.get_doc("RQ Job", self.job_id) + job.stop_job() + def after_insert(self): enqueue( generate_report, queue="long", prepared_report=self.name, - timeout=1500, + timeout=REPORT_TIMEOUT, enqueue_after_commit=True, ) @@ -58,7 +72,7 @@ class PreparedReport(Document): def generate_report(prepared_report): - update_job_id(prepared_report, get_current_job().id) + update_job_id(prepared_report) instance = frappe.get_doc("Prepared Report", prepared_report) report = frappe.get_doc("Report", instance.report_name) @@ -95,8 +109,18 @@ def generate_report(prepared_report): ) -def update_job_id(prepared_report, job_id): - frappe.db.set_value("Prepared Report", prepared_report, "job_id", job_id, update_modified=False) +def update_job_id(prepared_report): + job = get_current_job() + + frappe.db.set_value( + "Prepared Report", + prepared_report, + { + "job_id": job and job.id, + "status": "Started", + }, + ) + frappe.db.commit() @@ -132,7 +156,7 @@ def get_reports_in_queued_state(report_name, filters): filters={ "report_name": report_name, "filters": process_filters_for_prepared_report(filters), - "status": "Queued", + "status": ("in", ("Queued", "Started")), "owner": frappe.session.user, }, ) @@ -151,6 +175,21 @@ def get_completed_prepared_report(filters, user, report_name): ) +def expire_stalled_report(): + frappe.db.set_value( + "Prepared Report", + { + "status": "Started", + "modified": ("<", add_to_date(now(), seconds=-FAILURE_THRESHOLD, as_datetime=True)), + }, + { + "status": "Failed", + "error_message": frappe._("Report timed out."), + }, + update_modified=False, + ) + + @frappe.whitelist() def delete_prepared_reports(reports): reports = frappe.parse_json(reports) diff --git a/frappe/core/doctype/prepared_report/test_prepared_report.py b/frappe/core/doctype/prepared_report/test_prepared_report.py index a864ea73f8..dbd1294cbb 100644 --- a/frappe/core/doctype/prepared_report/test_prepared_report.py +++ b/frappe/core/doctype/prepared_report/test_prepared_report.py @@ -2,10 +2,13 @@ # License: MIT. See LICENSE import json import time +from contextlib import contextmanager import frappe from frappe.desk.query_report import generate_report_result, get_report_doc -from frappe.tests.utils import FrappeTestCase +from frappe.query_builder.utils import db_type_is +from frappe.tests.test_query_builder import run_only_if +from frappe.tests.utils import FrappeTestCase, timeout class TestPreparedReport(FrappeTestCase): @@ -16,11 +19,21 @@ class TestPreparedReport(FrappeTestCase): frappe.db.commit() - def create_prepared_report(self, commit=False): + @timeout(seconds=20) + def wait_for_status(self, report, status): + frappe.db.commit() # Flush changes first + while True: + frappe.db.rollback() # read new data + report.reload() + if report.status == status: + break + time.sleep(0.5) + + def create_prepared_report(self, report=None, commit=True): doc = frappe.get_doc( { "doctype": "Prepared Report", - "report_name": "Database Storage Usage By Tables", + "report_name": report or "Database Storage Usage By Tables", } ).insert() @@ -30,24 +43,50 @@ class TestPreparedReport(FrappeTestCase): return doc def test_queueing(self): - doc_ = self.create_prepared_report() - self.assertEqual("Queued", doc_.status) - self.assertTrue(doc_.queued_at) + doc = self.create_prepared_report() + self.assertEqual("Queued", doc.status) + self.assertTrue(doc.queued_at) - frappe.db.commit() - time.sleep(5) + self.wait_for_status(doc, "Completed") - doc_ = frappe.get_last_doc("Prepared Report") - self.assertEqual("Completed", doc_.status) - self.assertTrue(doc_.job_id) - self.assertTrue(doc_.report_end_time) + doc = frappe.get_last_doc("Prepared Report") + self.assertTrue(doc.job_id) + self.assertTrue(doc.report_end_time) def test_prepared_data(self): - doc_ = self.create_prepared_report(commit=True) - time.sleep(5) + doc = self.create_prepared_report() + self.wait_for_status(doc, "Completed") - prepared_data = json.loads(doc_.get_prepared_data().decode("utf-8")) + prepared_data = json.loads(doc.get_prepared_data().decode("utf-8")) generated_data = generate_report_result(get_report_doc("Database Storage Usage By Tables")) self.assertEqual(len(prepared_data["columns"]), len(generated_data["columns"])) self.assertEqual(len(prepared_data["result"]), len(generated_data["result"])) self.assertEqual(len(prepared_data), len(generated_data)) + + @run_only_if(db_type_is.MARIADB) + def test_start_status_and_kill_jobs(self): + with test_report(report_type="Query Report", query="select sleep(10)") as report: + doc = self.create_prepared_report(report.name) + self.wait_for_status(doc, "Started") + job_id = doc.job_id + + doc.delete() + time.sleep(1) + job = frappe.get_doc("RQ Job", job_id) + self.assertEqual(job.status, "stopped") + + +@contextmanager +def test_report(**args): + try: + report = frappe.new_doc("Report") + report.update(args) + if not report.report_name: + report.report_name = frappe.generate_hash() + if not report.ref_doctype: + report.ref_doctype = "ToDo" + report.insert() + frappe.db.commit() + yield report + finally: + report.delete() diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 8cdbc24074..8bd22b173e 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -49,6 +49,10 @@ class Report(Document): def on_update(self): self.export_doc() + def before_export(self): + self.letterhead = None + self.prepared_report = 0 + def on_trash(self): if ( self.is_standard == "Yes" @@ -121,7 +125,7 @@ class Report(Document): def execute_script_report(self, filters): # save the timestamp to automatically set to prepared - threshold = 30 + threshold = 15 res = [] start_time = datetime.datetime.now() @@ -135,7 +139,7 @@ class Report(Document): # automatically set as prepared execution_time = (datetime.datetime.now() - start_time).total_seconds() if execution_time > threshold and not self.prepared_report: - self.db_set("prepared_report", 1) + frappe.enqueue(enable_prepared_report, report=self.name) frappe.cache.hset("report_execution_time", self.name, execution_time) @@ -382,3 +386,7 @@ def get_group_by_column_label(args, meta): function=sql_fn_map[args.aggregate_function], fieldlabel=aggregate_on_label ) return label + + +def enable_prepared_report(report: str): + frappe.db.set_value("Report", report, "prepared_report", 1) diff --git a/frappe/core/doctype/rq_job/rq_job.py b/frappe/core/doctype/rq_job/rq_job.py index 391ccd8dfd..659c81e5c4 100644 --- a/frappe/core/doctype/rq_job/rq_job.py +++ b/frappe/core/doctype/rq_job/rq_job.py @@ -63,23 +63,15 @@ class RQJob(Document): order_desc = "desc" in args.get("order_by", "") - matched_job_ids = RQJob.get_matching_job_ids(args) + matched_job_ids = RQJob.get_matching_job_ids(args)[start : start + page_length] - jobs = [] - for job_ids in create_batch(matched_job_ids, 100): - jobs.extend( - serialize_job(job) - for job in Job.fetch_many(job_ids=job_ids, connection=get_redis_conn()) - if job and for_current_site(job) - ) - if len(jobs) > start + page_length: - # we have fetched enough. This is inefficient but because of site filtering TINA - break + conn = get_redis_conn() + jobs = [serialize_job(job) for job in Job.fetch_many(job_ids=matched_job_ids, connection=conn)] - return sorted(jobs, key=lambda j: j.modified, reverse=order_desc)[start : start + page_length] + return sorted(jobs, key=lambda j: j.modified, reverse=order_desc) @staticmethod - def get_matching_job_ids(args): + def get_matching_job_ids(args) -> list[str]: filters = make_filter_dict(args.get("filters")) queues = _eval_filters(filters.get("queue"), QUEUES) @@ -92,7 +84,7 @@ class RQJob(Document): for status in statuses: matched_job_ids.extend(fetch_job_ids(queue, status)) - return matched_job_ids + return filter_current_site_jobs(matched_job_ids) @check_permissions def delete(self): @@ -107,8 +99,7 @@ class RQJob(Document): @staticmethod def get_count(args) -> int: - # Can not be implemented efficiently due to site filtering hence ignored. - return 0 + return len(RQJob.get_matching_job_ids(args)) # None of these methods apply to virtual job doctype, overriden for sanity. @staticmethod @@ -155,6 +146,12 @@ def for_current_site(job: Job) -> bool: return job.kwargs.get("site") == frappe.local.site +def filter_current_site_jobs(job_ids: list[str]) -> list[str]: + site = frappe.local.site + + return [j for j in job_ids if j.startswith(site)] + + def _eval_filters(filter, values: list[str]) -> list[str]: if filter: operator, operand = filter @@ -186,10 +183,13 @@ def remove_failed_jobs(): frappe.only_for("System Manager") for queue in get_queues(): fail_registry = queue.failed_job_registry - for job_ids in create_batch(fail_registry.get_job_ids(), 100): - for job in Job.fetch_many(job_ids=job_ids, connection=get_redis_conn()): - if job and for_current_site(job): - fail_registry.remove(job, delete_job=True) + failed_jobs = filter_current_site_jobs(fail_registry.get_job_ids()) + + # Delete in batches to avoid loading too many things in memory + conn = get_redis_conn() + for job_ids in create_batch(failed_jobs, 100): + for job in Job.fetch_many(job_ids=job_ids, connection=conn): + job and fail_registry.remove(job, delete_job=True) def get_all_queued_jobs(): diff --git a/frappe/core/doctype/rq_job/rq_job_list.js b/frappe/core/doctype/rq_job/rq_job_list.js index aa05b411ba..7d140d668f 100644 --- a/frappe/core/doctype/rq_job/rq_job_list.js +++ b/frappe/core/doctype/rq_job/rq_job_list.js @@ -15,7 +15,6 @@ frappe.listview_settings["RQ Job"] = { ); if (listview.list_view_settings) { - listview.list_view_settings.disable_count = 1; listview.list_view_settings.disable_sidebar_stats = 1; } @@ -57,6 +56,6 @@ frappe.listview_settings["RQ Job"] = { } listview.refresh(); - }, 5000); + }, 15000); }, }; diff --git a/frappe/core/doctype/rq_job/test_rq_job.py b/frappe/core/doctype/rq_job/test_rq_job.py index c39717cfd8..f5d5f89ed4 100644 --- a/frappe/core/doctype/rq_job/test_rq_job.py +++ b/frappe/core/doctype/rq_job/test_rq_job.py @@ -97,13 +97,38 @@ class TestRQJob(FrappeTestCase): self.assertIn("quitting", cstr(stderr)) @timeout(20) - def test_job_id_dedup(self): + def test_multi_queue_burst_consumption_worker_pool(self): + for _ in range(3): + for q in ["default", "short"]: + frappe.enqueue(self.BG_JOB, sleep=1, queue=q) + + _, stderr = execute_in_shell( + "bench worker-pool --queue short,default --burst --num-workers=4", check_exit_code=True + ) + self.assertIn("quitting", cstr(stderr)) + + @timeout(20) + def test_job_id_manual_dedup(self): job_id = "test_dedup" job = frappe.enqueue(self.BG_JOB, sleep=5, job_id=job_id) self.assertTrue(is_job_enqueued(job_id)) self.check_status(job, "finished") self.assertFalse(is_job_enqueued(job_id)) + @timeout(20) + def test_auto_job_dedup(self): + job_id = "test_dedup" + job1 = frappe.enqueue(self.BG_JOB, sleep=2, job_id=job_id, deduplicate=True) + job2 = frappe.enqueue(self.BG_JOB, sleep=5, job_id=job_id, deduplicate=True) + self.assertIsNone(job2) + self.check_status(job1, "finished") # wait + + # Failed jobs last longer, subsequent job should still pass with same ID. + job3 = frappe.enqueue(self.BG_JOB, fail=True, job_id=job_id, deduplicate=True) + self.check_status(job3, "failed") + job4 = frappe.enqueue(self.BG_JOB, sleep=1, job_id=job_id, deduplicate=True) + self.check_status(job4, "finished") + @timeout(20) def test_enqueue_after_commit(self): job_id = frappe.generate_hash() diff --git a/frappe/core/doctype/server_script/server_script_list.js b/frappe/core/doctype/server_script/server_script_list.js new file mode 100644 index 0000000000..0df447a0eb --- /dev/null +++ b/frappe/core/doctype/server_script/server_script_list.js @@ -0,0 +1,38 @@ +frappe.listview_settings["Server Script"] = { + onload: function (listview) { + add_github_star_cta(listview); + }, +}; + +function add_github_star_cta(listview) { + try { + const key = "show_github_star_banner"; + if (localStorage.getItem(key) == "false") { + return; + } + + if (listview.github_star_banner) { + listview.github_star_banner.remove(); + } + + const message = "Loving Frappe Framework?"; + const link = "https://github.com/frappe/frappe"; + const cta = "Star us on GitHub"; + + listview.github_star_banner = $(` +
+
+ ${message}
${cta} → +
+
+ + + +
+
+ `).appendTo(listview.page.sidebar); + } catch (error) { + console.error(error); + } +} diff --git a/frappe/core/notifications.py b/frappe/core/notifications.py index 093418e345..26e920bca9 100644 --- a/frappe/core/notifications.py +++ b/frappe/core/notifications.py @@ -11,7 +11,6 @@ def get_notification_config(): "Communication": {"status": "Open", "communication_type": "Communication"}, "ToDo": "frappe.core.notifications.get_things_todo", "Event": "frappe.core.notifications.get_todays_events", - "Error Snapshot": {"seen": 0, "parent_error_snapshot": None}, "Workflow Action": {"status": "Open"}, }, } diff --git a/frappe/core/report/database_storage_usage_by_tables/database_storage_usage_by_tables.json b/frappe/core/report/database_storage_usage_by_tables/database_storage_usage_by_tables.json index 20deb78ad6..773cb7771f 100644 --- a/frappe/core/report/database_storage_usage_by_tables/database_storage_usage_by_tables.json +++ b/frappe/core/report/database_storage_usage_by_tables/database_storage_usage_by_tables.json @@ -9,7 +9,6 @@ "filters": [], "idx": 0, "is_standard": "Yes", - "letter_head": "abc", "modified": "2022-10-19 02:59:00.365307", "modified_by": "Administrator", "module": "Core", @@ -25,4 +24,4 @@ "role": "System Manager" } ] -} \ No newline at end of file +} diff --git a/frappe/core/workspace/build/build.json b/frappe/core/workspace/build/build.json index b917f88e27..12bef0ed89 100644 --- a/frappe/core/workspace/build/build.json +++ b/frappe/core/workspace/build/build.json @@ -155,74 +155,6 @@ "onboard": 0, "type": "Link" }, - { - "hidden": 0, - "is_query_report": 0, - "label": "System Logs", - "link_count": 6, - "onboard": 0, - "type": "Card Break" - }, - { - "hidden": 0, - "is_query_report": 0, - "label": "Background Jobs", - "link_count": 0, - "link_to": "RQ Job", - "link_type": "DocType", - "onboard": 0, - "type": "Link" - }, - { - "hidden": 0, - "is_query_report": 0, - "label": "Scheduled Jobs Logs", - "link_count": 0, - "link_to": "Scheduled Job Log", - "link_type": "DocType", - "onboard": 0, - "type": "Link" - }, - { - "hidden": 0, - "is_query_report": 0, - "label": "Error Logs", - "link_count": 0, - "link_to": "Error Log", - "link_type": "DocType", - "onboard": 0, - "type": "Link" - }, - { - "hidden": 0, - "is_query_report": 0, - "label": "Error Snapshot", - "link_count": 0, - "link_to": "Error Snapshot", - "link_type": "DocType", - "onboard": 0, - "type": "Link" - }, - { - "hidden": 0, - "is_query_report": 0, - "label": "Communication Logs", - "link_count": 0, - "link_to": "Communication", - "link_type": "DocType", - "onboard": 0, - "type": "Link" - }, - { - "hidden": 0, - "is_query_report": 0, - "label": "Activity Log", - "link_count": 0, - "link_to": "Activity Log", - "link_type": "DocType", - "onboard": 0, - "type": "Link" - }, { "hidden": 0, "is_query_report": 0, @@ -331,9 +263,67 @@ "link_type": "DocType", "onboard": 0, "type": "Link" + }, + { + "hidden": 0, + "is_query_report": 0, + "label": "System Logs", + "link_count": 5, + "onboard": 0, + "type": "Card Break" + }, + { + "hidden": 0, + "is_query_report": 0, + "label": "Background Jobs", + "link_count": 0, + "link_to": "RQ Job", + "link_type": "DocType", + "onboard": 0, + "type": "Link" + }, + { + "hidden": 0, + "is_query_report": 0, + "label": "Scheduled Jobs Logs", + "link_count": 0, + "link_to": "Scheduled Job Log", + "link_type": "DocType", + "onboard": 0, + "type": "Link" + }, + { + "hidden": 0, + "is_query_report": 0, + "label": "Error Logs", + "link_count": 0, + "link_to": "Error Log", + "link_type": "DocType", + "onboard": 0, + "type": "Link" + }, + { + "hidden": 0, + "is_query_report": 0, + "label": "Communication Logs", + "link_count": 0, + "link_to": "Communication", + "link_type": "DocType", + "onboard": 0, + "type": "Link" + }, + { + "hidden": 0, + "is_query_report": 0, + "label": "Activity Log", + "link_count": 0, + "link_to": "Activity Log", + "link_type": "DocType", + "onboard": 0, + "type": "Link" } ], - "modified": "2023-05-24 14:47:24.395259", + "modified": "2023-06-28 10:30:17.228167", "modified_by": "Administrator", "module": "Core", "name": "Build", diff --git a/frappe/desk/page/setup_wizard/setup_wizard.js b/frappe/desk/page/setup_wizard/setup_wizard.js index 7d68fd683c..e5c268b24c 100644 --- a/frappe/desk/page/setup_wizard/setup_wizard.js +++ b/frappe/desk/page/setup_wizard/setup_wizard.js @@ -97,10 +97,13 @@ frappe.setup.SetupWizard = class SetupWizard extends frappe.ui.Slides { handle_enter_press(e) { if (e.which === frappe.ui.keyCode.ENTER) { - var $target = $(e.target); - if ($target.hasClass("prev-btn")) { + let $target = $(e.target); + if ($target.hasClass("prev-btn") || $target.hasClass("next-btn")) { $target.trigger("click"); } else { + // hitting enter on autocomplete field shouldn't trigger next slide. + if ($target.data().fieldtype == "Autocomplete") return; + this.container.find(".next-btn").trigger("click"); e.preventDefault(); } @@ -545,15 +548,19 @@ frappe.setup.utils = { slide.get_input("timezone").empty().add_options(data.all_timezones); - // set values if present - if (frappe.wizard.values.country) { - country_field.set_input(frappe.wizard.values.country); - } else if (data.default_country) { - country_field.set_input(data.default_country); - } - slide.get_field("currency").set_input(frappe.wizard.values.currency); slide.get_field("timezone").set_input(frappe.wizard.values.timezone); + + // set values if present + let country = + frappe.wizard.values.country || + data.default_country || + guess_country(frappe.setup.data.regional_data.country_info); + + if (country) { + country_field.set_input(country); + $(country_field.input).change(); + } }, bind_language_events: function (slide) { @@ -630,3 +637,16 @@ frappe.setup.utils = { }); }, }; + +function guess_country(country_info) { + try { + const system_timezone = Intl.DateTimeFormat().resolvedOptions().timeZone; + + for ([country, info] of Object.entries(country_info)) { + let possible_timezones = (info.timezones || []).filter((t) => t == system_timezone); + if (possible_timezones.length) return country; + } + } catch (e) { + console.log("Could not guess country", e); + } +} diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 0df88ebd5c..cae5f76b3d 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -132,7 +132,6 @@ def return_unsubscribed_page(email, doctype, name): def flush(from_test=False): """flush email queue, every time: called from scheduler""" from frappe.email.doctype.email_queue.email_queue import send_mail - from frappe.utils.background_jobs import get_jobs # To avoid running jobs inside unit tests if frappe.are_emails_muted(): @@ -142,24 +141,16 @@ def flush(from_test=False): if cint(frappe.db.get_default("suspend_email_queue")) == 1: return - try: - queued_jobs = set(get_jobs(site=frappe.local.site, key="job_name")[frappe.local.site]) - except Exception: - queued_jobs = set() - for row in get_queue(): try: - job_name = f"email_queue_sendmail_{row.name}" - if job_name not in queued_jobs: - frappe.enqueue( - method=send_mail, - email_queue_name=row.name, - now=from_test, - job_name=job_name, - queue="short", - ) - else: - frappe.logger().debug(f"Not queueing job {job_name} because it is in queue already") + frappe.enqueue( + method=send_mail, + email_queue_name=row.name, + now=from_test, + job_id=f"email_queue_sendmail_{row.name}", + queue="short", + deduplicate=True, + ) except Exception: frappe.get_doc("Email Queue", row.name).log_error() diff --git a/frappe/hooks.py b/frappe/hooks.py index ed2b25bc1b..fe430918d0 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -108,6 +108,7 @@ permission_query_conditions = { "Communication": "frappe.core.doctype.communication.communication.get_permission_query_conditions_for_communication", "Workflow Action": "frappe.workflow.doctype.workflow_action.workflow_action.get_permission_query_conditions", "Prepared Report": "frappe.core.doctype.prepared_report.prepared_report.get_permission_query_condition", + "File": "frappe.core.doctype.file.file.get_permission_query_conditions", } has_permission = { @@ -195,9 +196,9 @@ scheduler_events = { "frappe.email.doctype.email_account.email_account.pull", ], # Hourly but offset by 30 minutes - # "30 * * * *": [ - # - # ], + "30 * * * *": [ + "frappe.core.doctype.prepared_report.prepared_report.expire_stalled_report", + ], # Daily but offset by 45 minutes "45 0 * * *": [ "frappe.core.doctype.log_settings.log_settings.run_log_clean_up", @@ -213,7 +214,6 @@ scheduler_events = { "hourly": [ "frappe.model.utils.link_count.update_link_count", "frappe.model.utils.user_settings.sync_user_settings", - "frappe.utils.error.collect_error_snapshots", "frappe.desk.page.backups.backups.delete_downloadable_backups", "frappe.deferred_insert.save_to_db", "frappe.desk.form.document_follow.send_hourly_updates", diff --git a/frappe/installer.py b/frappe/installer.py index 9c2807d7cd..775e5b9b02 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -287,6 +287,9 @@ def install_app(name, verbose=False, set_as_patched=True, force=False): if out is False: return + for fn in frappe.get_hooks("before_app_install"): + frappe.get_attr(fn)(name) + if name != "frappe": add_module_defs(name, ignore_if_duplicate=force) @@ -302,6 +305,9 @@ def install_app(name, verbose=False, set_as_patched=True, force=False): for after_install in app_hooks.after_install or []: frappe.get_attr(after_install)() + for fn in frappe.get_hooks("after_app_install"): + frappe.get_attr(fn)(name) + sync_jobs() sync_fixtures(name) sync_customizations(name) @@ -369,6 +375,9 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) for before_uninstall in app_hooks.before_uninstall or []: frappe.get_attr(before_uninstall)() + for fn in frappe.get_hooks("before_app_uninstall"): + frappe.get_attr(fn)(app_name) + modules = frappe.get_all("Module Def", filters={"app_name": app_name}, pluck="name") drop_doctypes = _delete_modules(modules, dry_run=dry_run) @@ -382,6 +391,9 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) for after_uninstall in app_hooks.after_uninstall or []: frappe.get_attr(after_uninstall)() + for fn in frappe.get_hooks("after_app_uninstall"): + frappe.get_attr(fn)(app_name) + click.secho(f"Uninstalled App {app_name} from Site {site}", fg="green") frappe.flags.in_uninstall = False @@ -605,7 +617,6 @@ def make_site_dirs(): os.path.join("public", "files"), os.path.join("private", "backups"), os.path.join("private", "files"), - "error-snapshots", "locks", "logs", ]: diff --git a/frappe/integrations/doctype/webhook/webhook.py b/frappe/integrations/doctype/webhook/webhook.py index 6fa24bfb67..5b8c653198 100644 --- a/frappe/integrations/doctype/webhook/webhook.py +++ b/frappe/integrations/doctype/webhook/webhook.py @@ -26,6 +26,7 @@ class Webhook(Document): self.validate_request_url() self.validate_request_body() self.validate_repeating_fields() + self.validate_secret() self.preview_document = None def on_update(self): @@ -74,6 +75,13 @@ class Webhook(Document): if len(webhook_data) != len(set(webhook_data)): frappe.throw(_("Same Field is entered more than once")) + def validate_secret(self): + if self.enable_security: + try: + self.get_password("webhook_secret", False).encode("utf8") + except Exception: + frappe.throw(_("Invalid Webhook Secret")) + @frappe.whitelist() def generate_preview(self): # This function doesn't need to do anything specific as virtual fields @@ -112,16 +120,21 @@ def get_context(doc): def enqueue_webhook(doc, webhook) -> None: - webhook: Webhook = frappe.get_doc("Webhook", webhook.get("name")) - headers = get_webhook_headers(doc, webhook) - data = get_webhook_data(doc, webhook) + try: + webhook: Webhook = frappe.get_doc("Webhook", webhook.get("name")) + headers = get_webhook_headers(doc, webhook) + data = get_webhook_data(doc, webhook) - if webhook.is_dynamic_url: - request_url = frappe.render_template(webhook.request_url, get_context(doc)) - else: - request_url = webhook.request_url + if webhook.is_dynamic_url: + request_url = frappe.render_template(webhook.request_url, get_context(doc)) + else: + request_url = webhook.request_url + + except Exception as e: + frappe.logger().debug({"enqueue_webhook_error": e}) + log_request(webhook.name, doc.name, request_url, headers, data) + return - r = None for i in range(3): try: r = requests.request( diff --git a/frappe/patches.txt b/frappe/patches.txt index c26b1a74d7..ebdda9b220 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -31,7 +31,6 @@ execute:frappe.reload_doc('core', 'doctype', 'user') #2017-10-27 execute:frappe.reload_doc('core', 'doctype', 'report_column') execute:frappe.reload_doc('core', 'doctype', 'report_filter') execute:frappe.reload_doc('core', 'doctype', 'report') #2020-08-25 -execute:frappe.reload_doc('core', 'doctype', 'error_snapshot') execute:frappe.get_doc("User", "Guest").save() execute:frappe.delete_doc("DocType", "Control Panel", force=1) execute:frappe.delete_doc("DocType", "Tag") @@ -42,7 +41,6 @@ execute:frappe.db.sql("delete from `tabProperty Setter` where `property` = 'idx' execute:frappe.db.sql("delete from tabSessions where user is null") execute:frappe.delete_doc("DocType", "Backup Manager") execute:frappe.permissions.reset_perms("Web Page") -execute:frappe.permissions.reset_perms("Error Snapshot") execute:frappe.db.sql("delete from `tabWeb Page` where ifnull(template_path, '')!=''") execute:frappe.core.doctype.language.language.update_language_names() # 2017-04-12 execute:frappe.db.set_value("Print Settings", "Print Settings", "add_draft_heading", 1) @@ -227,3 +225,4 @@ frappe.patches.v15_0.remove_background_jobs_from_dropdown frappe.desk.doctype.form_tour.patches.introduce_ui_tours execute:frappe.delete_doc_if_exists("Workspace", "Customization") execute:frappe.db.set_single_value("Document Naming Settings", "default_amend_naming", "Amend Counter") +execute:frappe.delete_doc_if_exists("DocType", "Error Snapshot") diff --git a/frappe/patches/v14_0/clear_long_pending_stale_logs.py b/frappe/patches/v14_0/clear_long_pending_stale_logs.py index 53127cb197..e419b1e562 100644 --- a/frappe/patches/v14_0/clear_long_pending_stale_logs.py +++ b/frappe/patches/v14_0/clear_long_pending_stale_logs.py @@ -15,7 +15,6 @@ def execute(): "Email Queue": get_current_setting("clear_email_queue_after") or 30, # child table on email queue "Email Queue Recipient": get_current_setting("clear_email_queue_after") or 30, - "Error Snapshot": get_current_setting("clear_error_log_after") or 90, # newly added "Scheduled Job Log": 90, } diff --git a/frappe/permissions.py b/frappe/permissions.py index 633d0e278d..e8ca0ecb3c 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -118,17 +118,24 @@ def has_permission( def false_if_not_shared(): if ptype in ("read", "write", "share", "submit", "email", "print"): - shared = frappe.share.get_shared( - doctype, user, ["read" if ptype in ("email", "print") else ptype] - ) + + rights = ["read" if ptype in ("email", "print") else ptype] if doc: doc_name = get_doc_name(doc) - if doc_name in shared: + shared = frappe.share.get_shared( + doctype, + user, + rights=rights, + filters=[["share_name", "=", doc_name]], + limit=1, + ) + + if shared: if ptype in ("read", "write", "share", "submit") or meta.permissions[0].get(ptype): return True - elif shared: + elif frappe.share.get_shared(doctype, user, rights=rights, limit=1): # if atleast one shared doc of that type, then return True # this is used in db_query to check if permission on DocType return True diff --git a/frappe/public/js/form_builder/components/Column.vue b/frappe/public/js/form_builder/components/Column.vue index 1563d1033e..54dd49d5bf 100644 --- a/frappe/public/js/form_builder/components/Column.vue +++ b/frappe/public/js/form_builder/components/Column.vue @@ -6,7 +6,7 @@ import { ref } from "vue"; import { useStore } from "../store"; import { move_children_to_parent, confirm_dialog } from "../utils"; -let props = defineProps(["section", "column"]); +const props = defineProps(["section", "column"]); let store = useStore(); let hovered = ref(false); diff --git a/frappe/public/js/form_builder/components/EditableInput.vue b/frappe/public/js/form_builder/components/EditableInput.vue index 8964838f4a..21b517af3b 100644 --- a/frappe/public/js/form_builder/components/EditableInput.vue +++ b/frappe/public/js/form_builder/components/EditableInput.vue @@ -3,7 +3,7 @@ import { ref, nextTick, computed } from "vue"; import { useStore } from "../store"; let store = useStore(); -let props = defineProps({ +const props = defineProps({ text: { type: String }, @@ -46,7 +46,7 @@ function focus_on_label() { :disabled="store.read_only" type="text" :placeholder="placeholder" - v-model="text" + :value="text" :style="{ width: hidden_span_width }" @input="event => $emit('update:modelValue', event.target.value)" @keydown.enter="editing = false" diff --git a/frappe/public/js/form_builder/components/Field.vue b/frappe/public/js/form_builder/components/Field.vue index e0230765b5..b67d6db0c9 100644 --- a/frappe/public/js/form_builder/components/Field.vue +++ b/frappe/public/js/form_builder/components/Field.vue @@ -4,7 +4,7 @@ import { ref, computed } from "vue"; import { useStore } from "../store"; import { move_children_to_parent, clone_field } from "../utils"; -let props = defineProps(["column", "field"]); +const props = defineProps(["column", "field"]); let store = useStore(); let hovered = ref(false); diff --git a/frappe/public/js/form_builder/components/Section.vue b/frappe/public/js/form_builder/components/Section.vue index 5131ff25d3..cd675e5958 100644 --- a/frappe/public/js/form_builder/components/Section.vue +++ b/frappe/public/js/form_builder/components/Section.vue @@ -6,7 +6,7 @@ import { ref } from "vue"; import { useStore } from "../store"; import { section_boilerplate, move_children_to_parent, confirm_dialog } from "../utils"; -let props = defineProps(["tab", "section"]); +const props = defineProps(["tab", "section"]); let store = useStore(); let hovered = ref(false); diff --git a/frappe/public/js/form_builder/components/controls/AttachControl.vue b/frappe/public/js/form_builder/components/controls/AttachControl.vue index 86cdf7c5ac..6d8718d5dc 100644 --- a/frappe/public/js/form_builder/components/controls/AttachControl.vue +++ b/frappe/public/js/form_builder/components/controls/AttachControl.vue @@ -1,6 +1,6 @@