diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index f1d5553c07d..4552fb22b12 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -51,7 +51,7 @@ jobs: python $GITHUB_WORKSPACE/.github/helper/documentation.py $PR_NUMBER linter: - name: 'Frappe Linter' + name: 'Semgrep Rules' runs-on: ubuntu-latest if: github.event_name == 'pull_request' @@ -61,7 +61,6 @@ jobs: with: python-version: '3.10' cache: pip - - uses: pre-commit/action@v3.0.0 - name: Download Semgrep rules run: git clone --depth 1 https://github.com/frappe/semgrep-rules.git frappe-semgrep-rules diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml new file mode 100644 index 00000000000..32ececc78aa --- /dev/null +++ b/.github/workflows/pre-commit.yml @@ -0,0 +1,26 @@ +name: Pre-commit + +on: + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: precommit-frappe-${{ github.event_name }}-${{ github.event.number }} + cancel-in-progress: true + +jobs: + linter: + name: 'precommit' + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.10' + cache: pip + - uses: pre-commit/action@v3.0.0 diff --git a/.github/workflows/server-tests.yml b/.github/workflows/server-tests.yml index 4ef2066be65..efea6e90a72 100644 --- a/.github/workflows/server-tests.yml +++ b/.github/workflows/server-tests.yml @@ -139,7 +139,18 @@ jobs: - name: Show bench output if: ${{ always() }} - run: cat ~/frappe-bench/bench_start.log || true + run: | + cd ~/frappe-bench + cat bench_start.log || true + cd logs + for f in ./*.log*; do + echo "Printing log: $f"; + cat $f + done + + - name: Setup tmate session + uses: mxschmitt/action-tmate@v3 + if: ${{ contains( github.event.pull_request.labels.*.name, 'debug-gha') }} # This is required because github still doesn't understand knowingly skipped tests faux-test: diff --git a/frappe/app.py b/frappe/app.py index 61f626bfcd3..555752a13b1 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -399,11 +399,7 @@ def handle_exception(e): def sync_database(rollback: bool) -> bool: # if HTTP method would change server state, commit if necessary - if ( - frappe.db - and (frappe.local.flags.commit or frappe.local.request.method in UNSAFE_HTTP_METHODS) - and frappe.db.transaction_writes - ): + if frappe.db and (frappe.local.flags.commit or frappe.local.request.method in UNSAFE_HTTP_METHODS): frappe.db.commit() rollback = False elif frappe.db: diff --git a/frappe/core/doctype/module_def/module_def_list.js b/frappe/core/doctype/module_def/module_def_list.js new file mode 100644 index 00000000000..4ff0c203e07 --- /dev/null +++ b/frappe/core/doctype/module_def/module_def_list.js @@ -0,0 +1,16 @@ +frappe.listview_settings["Module Def"] = { + onload: function (list_view) { + frappe.call({ + method: "frappe.core.doctype.module_def.module_def.get_installed_apps", + callback: (r) => { + const field = list_view.page.fields_dict.app_name; + if (!field) return; + + const options = JSON.parse(r.message); + options.unshift(""); // Add empty option + field.df.options = options; + field.set_options(); + }, + }); + }, +}; diff --git a/frappe/core/doctype/prepared_report/prepared_report.json b/frappe/core/doctype/prepared_report/prepared_report.json index b64b91c4ef6..ec86237d7fb 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.json +++ b/frappe/core/doctype/prepared_report/prepared_report.json @@ -83,10 +83,9 @@ }, { "fieldname": "job_id", - "fieldtype": "Link", + "fieldtype": "Data", "label": "Job ID", "no_copy": 1, - "options": "RQ Job", "read_only": 1 }, { @@ -106,7 +105,7 @@ ], "in_create": 1, "links": [], - "modified": "2023-07-01 18:29:12.700239", + "modified": "2024-03-07 12:01:58.209879", "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 5cf7829460e..4781b2e8e97 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -31,7 +31,7 @@ class PreparedReport(Document): error_message: DF.Text | None filters: DF.SmallText | None - job_id: DF.Link | None + job_id: DF.Data | None queued_at: DF.Datetime | None queued_by: DF.Data | None report_end_time: DF.Datetime | None diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index d492781795a..ac645fff536 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -16,6 +16,7 @@ toggle_notifications, ) from frappe.desk.notifications import clear_notifications +from frappe.model.delete_doc import check_if_doc_is_linked from frappe.model.document import Document from frappe.query_builder import DocType from frappe.rate_limiter import rate_limit @@ -552,6 +553,15 @@ def on_trash(self): frappe.db.delete("OAuth Authorization Code", {"user": self.name}) frappe.db.delete("Token Cache", {"user": self.name}) + # Delete EPS data + frappe.db.delete("Energy Point Log", {"user": self.name}) + + # Ask user to disable instead if document is still linked + try: + check_if_doc_is_linked(self) + except frappe.LinkExistsError: + frappe.throw(_("You can disable the user instead of deleting it."), frappe.LinkExistsError) + def before_rename(self, old_name, new_name, merge=False): frappe.clear_cache(user=old_name) self.validate_rename(old_name, new_name) diff --git a/frappe/custom/doctype/custom_field/custom_field.json b/frappe/custom/doctype/custom_field/custom_field.json index fc802b79850..32c4552f19b 100644 --- a/frappe/custom/doctype/custom_field/custom_field.json +++ b/frappe/custom/doctype/custom_field/custom_field.json @@ -361,7 +361,7 @@ "label": "Ignore XSS Filter" }, { - "default": "1", + "default": "0", "depends_on": "eval:['Data', 'Select', 'Text', 'Small Text', 'Text Editor'].includes(doc.fieldtype)", "fieldname": "translatable", "fieldtype": "Check", @@ -457,7 +457,7 @@ "idx": 1, "index_web_pages_for_search": 1, "links": [], - "modified": "2024-02-21 18:15:19.384933", + "modified": "2024-03-07 17:34:47.167183", "modified_by": "Administrator", "module": "Custom", "name": "Custom Field", diff --git a/frappe/custom/doctype/property_setter/patches/__init__.py b/frappe/custom/doctype/property_setter/patches/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/frappe/custom/doctype/property_setter/patches/remove_invalid_fetch_from_expressions.py b/frappe/custom/doctype/property_setter/patches/remove_invalid_fetch_from_expressions.py new file mode 100644 index 00000000000..777e2af2d86 --- /dev/null +++ b/frappe/custom/doctype/property_setter/patches/remove_invalid_fetch_from_expressions.py @@ -0,0 +1,26 @@ +import frappe + + +def execute(): + """Remove invalid fetch from expressions""" + + property_setters = frappe.get_all( + "Property Setter", {"doctype_or_field": "DocField", "property": "fetch_from"}, ["name", "value"] + ) + for ps in property_setters: + if not is_valid_expression(ps.value): + frappe.db.delete("Property Setter", {"name": ps.name}) + + custom_fields = frappe.get_all("Custom Field", {"fetch_from": ("is", "set")}, ["name", "fetch_from"]) + for cf in custom_fields: + if not is_valid_expression(cf.fetch_from): + frappe.db.set_value("Custom Field", cf.name, "fetch_from", "") + + +def is_valid_expression(expr) -> bool: + if not expr or "." not in expr: + return False + source_field, target_field = expr.split(".") + if not source_field or not target_field: + return False + return True diff --git a/frappe/database/database.py b/frappe/database/database.py index 9bc241f8a1a..1156890ca62 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -194,7 +194,7 @@ def sql( """ if isinstance(query, MySQLQueryBuilder | PostgreSQLQueryBuilder): - frappe.errprint("Use run method to execute SQL queries generated by Query Engine") + frappe.log("Use run method to execute SQL queries generated by Query Engine") debug = debug or getattr(self, "debug", False) query = str(query) @@ -234,7 +234,7 @@ def sql( self._cursor.execute(query, values) except Exception as e: if self.is_syntax_error(e): - frappe.errprint(f"Syntax error in query:\n{query} {values or ''}") + frappe.log(f"Syntax error in query:\n{query} {values or ''}") elif self.is_deadlocked(e): raise frappe.QueryDeadlockError(e) from e @@ -254,13 +254,13 @@ def sql( # TODO: added temporarily elif self.db_type == "postgres": traceback.print_stack() - frappe.errprint(f"Error in query:\n{e}") + frappe.log(f"Error in query:\n{e}") raise elif isinstance(e, self.ProgrammingError): if frappe.conf.developer_mode: traceback.print_stack() - frappe.errprint(f"Error in query:\n{query, values}") + frappe.log(f"Error in query:\n{query, values}") raise if not ( @@ -271,7 +271,7 @@ def sql( if debug: time_end = time() - frappe.errprint(f"Execution time: {time_end - time_start:.2f} sec") + frappe.log(f"Execution time: {time_end - time_start:.2f} sec") self.log_query(query, values, debug, explain) @@ -343,7 +343,7 @@ def _log_query( _query = _query or str(mogrified_query) if explain and is_query_type(_query, "select"): self.explain_query(_query) - frappe.errprint(_query) + frappe.log(_query) if frappe.conf.logging == 2: _query = _query or str(mogrified_query) @@ -391,14 +391,14 @@ def lazy_mogrify(self, query: Query, values: QueryValues) -> LazyMogrify: def explain_query(self, query, values=None): """Print `EXPLAIN` in error log.""" - frappe.errprint("--- query explain ---") + frappe.log("--- query explain ---") try: self._cursor.execute(f"EXPLAIN {query}", values) except Exception as e: - frappe.errprint(f"error in query explain: {e}") + frappe.log(f"error in query explain: {e}") else: - frappe.errprint(json.dumps(self.fetch_as_dict(), indent=1)) - frappe.errprint("--- query explain end ---") + frappe.log(json.dumps(self.fetch_as_dict(), indent=1)) + frappe.log("--- query explain end ---") def sql_list(self, query, values=(), debug=False, **kwargs): """Return data as list of single elements (first column). diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index 3fb5756ae7e..4c2fcd716c5 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -436,9 +436,8 @@ def updatedb(self, doctype, meta=None): db_table = MariaDBTable(doctype, meta) db_table.validate() - self.commit() db_table.sync() - self.begin() + self.commit() def get_database_list(self): return self.sql("SHOW DATABASES", pluck=True) diff --git a/frappe/database/mariadb/framework_mariadb.sql b/frappe/database/mariadb/framework_mariadb.sql index 4c69eafdedd..91ef69645c0 100644 --- a/frappe/database/mariadb/framework_mariadb.sql +++ b/frappe/database/mariadb/framework_mariadb.sql @@ -176,6 +176,7 @@ CREATE TABLE `tabDocType` ( `idx` int(8) NOT NULL DEFAULT 0, `search_fields` varchar(255) DEFAULT NULL, `issingle` int(1) NOT NULL DEFAULT 0, + `is_virtual` int(1) NOT NULL DEFAULT 0, `is_tree` int(1) NOT NULL DEFAULT 0, `istable` int(1) NOT NULL DEFAULT 0, `editable_grid` int(1) NOT NULL DEFAULT 1, diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index 552ad88f209..8eee72d835b 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -60,7 +60,7 @@ def create(self): CHARACTER SET=utf8mb4 COLLATE=utf8mb4_unicode_ci""" - frappe.db.sql(query) + frappe.db.sql_ddl(query) def alter(self): for col in self.columns.values(): @@ -102,7 +102,7 @@ def alter(self): if query_parts: query_body = ", ".join(query_parts) query = f"ALTER TABLE `{self.table_name}` {query_body}" - frappe.db.sql(query) + frappe.db.sql_ddl(query) except Exception as e: if query := locals().get("query"): # this weirdness is to avoid potentially unbounded vars diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index 7545cfee427..d21c6c5e51b 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -323,7 +323,6 @@ def updatedb(self, doctype, meta=None): db_table = PostgresTable(doctype, meta) db_table.validate() - self.commit() db_table.sync() self.begin() diff --git a/frappe/database/postgres/framework_postgres.sql b/frappe/database/postgres/framework_postgres.sql index 37605be0f63..e27cbffe0f8 100644 --- a/frappe/database/postgres/framework_postgres.sql +++ b/frappe/database/postgres/framework_postgres.sql @@ -179,6 +179,7 @@ CREATE TABLE "tabDocType" ( "idx" bigint NOT NULL DEFAULT 0, "search_fields" varchar(255) DEFAULT NULL, "issingle" smallint NOT NULL DEFAULT 0, + "is_virtual" smallint NOT NULL DEFAULT 0, "is_tree" smallint NOT NULL DEFAULT 0, "istable" smallint NOT NULL DEFAULT 0, "editable_grid" smallint NOT NULL DEFAULT 1, diff --git a/frappe/desk/form/linked_with.py b/frappe/desk/form/linked_with.py index 7fd865d165e..e36b067231a 100644 --- a/frappe/desk/form/linked_with.py +++ b/frappe/desk/form/linked_with.py @@ -613,6 +613,10 @@ def get_linked_fields(doctype, without_ignore_user_permissions_enabled=False): if options in ret: del ret[options] + virtual_doctypes = frappe.get_all("DocType", {"is_virtual": 1}, pluck="name") + for dt in virtual_doctypes: + ret.pop(dt, None) + return ret @@ -639,7 +643,11 @@ def get_dynamic_linked_fields(doctype, without_ignore_user_permissions_enabled=F if is_single(df.doctype): continue - is_child = frappe.get_meta(df.doctype).istable + meta = frappe.get_meta(df.doctype) + if meta.is_virtual: + continue + + is_child = meta.istable possible_link = frappe.get_all( df.doctype, filters={df.doctype_fieldname: doctype}, diff --git a/frappe/email/doctype/auto_email_report/auto_email_report.py b/frappe/email/doctype/auto_email_report/auto_email_report.py index 4c96dacf913..990dfe6e6f9 100644 --- a/frappe/email/doctype/auto_email_report/auto_email_report.py +++ b/frappe/email/doctype/auto_email_report/auto_email_report.py @@ -345,7 +345,9 @@ def make_links(columns, data): if col.options and row.get(col.options): row[col.fieldname] = get_link_to_form(row[col.options], row[col.fieldname]) elif col.fieldtype == "Currency": - doc = frappe.get_doc(col.parent, doc_name) if doc_name and col.get("parent") else None + doc = None + if doc_name and col.get("parent") and not frappe.get_meta(col.parent).istable: + doc = frappe.get_doc(col.parent, doc_name) # Pass the Document to get the currency based on docfield option row[col.fieldname] = frappe.format_value(row[col.fieldname], col, doc=doc) return columns, data diff --git a/frappe/handler.py b/frappe/handler.py index fdee4d86881..956c4ffeb5d 100644 --- a/frappe/handler.py +++ b/frappe/handler.py @@ -286,7 +286,6 @@ def get_attr(cmd): f"Calling shorthand for {cmd} is deprecated, please specify full path in RPC call." ) method = globals()[cmd] - frappe.log("method:" + cmd) return method diff --git a/frappe/hooks.py b/frappe/hooks.py index ddb3fa31c94..2315293ff8a 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -419,6 +419,8 @@ "Unhandled Email", "Webhook Request Log", "Workspace", + "Route History", + "Access Log", ] # Request Hooks diff --git a/frappe/installer.py b/frappe/installer.py index 52c04cbe462..8c65a0cba12 100644 --- a/frappe/installer.py +++ b/frappe/installer.py @@ -357,6 +357,14 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False) click.secho(f"App {app_name} not installed on Site {site}", fg="yellow") return + # Don't allow uninstalling if we have dependent apps installed + for app in frappe.get_installed_apps(): + if app != app_name: + hooks = frappe.get_hooks(app_name=app) + if hooks.required_apps and any(app_name in required_app for required_app in hooks.required_apps): + click.secho(f"App {app_name} is a dependency of {app}. Uninstall {app} first.", fg="yellow") + return + print(f"Uninstalling App {app_name} from Site {site}...") if not dry_run and not yes: diff --git a/frappe/model/document.py b/frappe/model/document.py index 8a3cab5bb0f..00052e33317 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1536,11 +1536,16 @@ def queue_action(self, action, **kwargs): primary_action=primary_action, ) + enqueue_after_commit = kwargs.pop("enqueue_after_commit", None) + if enqueue_after_commit is None: + enqueue_after_commit = True + return enqueue( "frappe.model.document.execute_action", __doctype=self.doctype, __name=self.name, __action=action, + enqueue_after_commit=enqueue_after_commit, **kwargs, ) diff --git a/frappe/patches.txt b/frappe/patches.txt index f1bbeacbc34..d86f4f5f95c 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -233,3 +233,4 @@ frappe.patches.v15_0.set_file_type frappe.core.doctype.data_import.patches.remove_stale_docfields_from_legacy_version frappe.patches.v15_0.validate_newsletter_recipients frappe.patches.v15_0.sanitize_workspace_titles +frappe.custom.doctype.property_setter.patches.remove_invalid_fetch_from_expressions diff --git a/frappe/public/js/frappe/desk.js b/frappe/public/js/frappe/desk.js index 1bce44dfa66..475c5f70ca3 100644 --- a/frappe/public/js/frappe/desk.js +++ b/frappe/public/js/frappe/desk.js @@ -507,7 +507,7 @@ frappe.Application = class Application { frappe.set_route("Form", newdoc.doctype, newdoc.name); frappe.dom.unfreeze(); }); - res && res.fail(frappe.dom.unfreeze); + res && res.fail?.(frappe.dom.unfreeze); }); } } catch (e) { diff --git a/frappe/public/js/frappe/form/templates/timeline_message_box.html b/frappe/public/js/frappe/form/templates/timeline_message_box.html index 830e4cd39f6..9a36c630b79 100644 --- a/frappe/public/js/frappe/form/templates/timeline_message_box.html +++ b/frappe/public/js/frappe/form/templates/timeline_message_box.html @@ -23,7 +23,7 @@ {% } %}