Merge branch 'develop' into cypress-ui-test

.github/helper/install.sh

@@ -59,4 +59,4 @@ cd ../..
 bench start &
 bench --site test_site reinstall --yes
 if [ "$TYPE" == "server" ]; then bench --site test_site_producer reinstall --yes; fi
-bench build --app frappe
+CI=Yes bench build --app frappe

.github/helper/semgrep_rules/frappe_correctness.yml

@@ -131,3 +131,16 @@ rules:
       key `$X` is uselessly assigned twice. This could be a potential bug.
   languages: [python]
   severity: ERROR
+
+- id: frappe-using-db-sql
+  pattern-either:
+    - pattern: frappe.db.sql(...)
+    - pattern: frappe.db.sql_ddl(...)
+    - pattern: frappe.db.sql_list(...)
+  paths:
+    exclude:
+      - "test_*.py"
+  message: |
+    The PR contains a SQL query that may be re-written with frappe.qb (https://frappeframework.com/docs/user/en/api/query-builder) or the Database API (https://frappeframework.com/docs/user/en/api/database)
+  languages: [python]
+  severity: ERROR

.github/try-on-f-cloud-button.svg

@@ -0,0 +1,32 @@
+<svg width="201" height="60" viewBox="0 0 201 60" fill="none" xmlns="http://www.w3.org/2000/svg">
+<g filter="url(#filter0_dd)">
+<rect x="4" y="2" width="193" height="52" rx="6" fill="#2490EF"/>
+<path d="M28 22.2891H32.8786V35.5H36.2088V22.2891H41.0874V19.5H28V22.2891Z" fill="white"/>
+<path d="M41.6982 35.5H45.0129V28.7109C45.0129 27.2344 46.0866 26.2188 47.5494 26.2188C48.0085 26.2188 48.6388 26.2969 48.95 26.3984V23.4453C48.6543 23.375 48.2419 23.3281 47.9074 23.3281C46.5691 23.3281 45.472 24.1094 45.0362 25.5938H44.9117V23.5H41.6982V35.5Z" fill="white"/>
+<path d="M52.8331 40C55.2996 40 56.6068 38.7344 57.2837 36.7969L61.9289 23.5156L58.4197 23.5L55.9221 32.3125H55.7976L53.3233 23.5H49.8374L54.1247 35.8437L53.9302 36.3516C53.4944 37.4766 52.6619 37.5312 51.4947 37.1719L50.7478 39.6562C51.2224 39.8594 51.9927 40 52.8331 40Z" fill="white"/>
+<path d="M73.6142 35.7344C77.2401 35.7344 79.4966 33.2422 79.4966 29.5469C79.4966 25.8281 77.2401 23.3438 73.6142 23.3438C69.9883 23.3438 67.7319 25.8281 67.7319 29.5469C67.7319 33.2422 69.9883 35.7344 73.6142 35.7344ZM73.6298 33.1562C71.9569 33.1562 71.101 31.6171 71.101 29.5233C71.101 27.4296 71.9569 25.8827 73.6298 25.8827C75.2715 25.8827 76.1274 27.4296 76.1274 29.5233C76.1274 31.6171 75.2715 33.1562 73.6298 33.1562Z" fill="white"/>
+<path d="M84.7253 28.5625C84.7331 27.0156 85.6512 26.1094 86.9895 26.1094C88.3201 26.1094 89.1215 26.9844 89.1137 28.4531V35.5H92.4284V27.8594C92.4284 25.0625 90.7945 23.3438 88.3046 23.3438C86.5306 23.3438 85.2466 24.2187 84.7097 25.6172H84.5697V23.5H81.4106V35.5H84.7253V28.5625Z" fill="white"/>
+<path fill-rule="evenodd" clip-rule="evenodd" d="M102.429 19.5H113.429V22.3141H102.429V19.5ZM102.429 35.5V26.6794H112.699V29.4982H105.94V35.5H102.429Z" fill="white"/>
+<path d="M131.584 24.9625C131.09 21.5057 128.345 19.5 124.785 19.5C120.589 19.5 117.429 22.463 117.429 27.4924C117.429 32.5142 120.55 35.4848 124.785 35.4848C128.604 35.4848 131.137 33.0916 131.584 30.1211L128.651 30.1059C128.282 31.9293 126.745 32.9549 124.824 32.9549C122.22 32.9549 120.354 31.0632 120.354 27.4924C120.354 23.9824 122.204 22.0299 124.832 22.0299C126.784 22.0299 128.314 23.1011 128.651 24.9625H131.584Z" fill="white"/>
+<path d="M136.409 19.7124H133.571V35.2718H136.409V19.7124Z" fill="white"/>
+<path d="M144.031 35.5001C147.56 35.5001 149.803 33.0917 149.803 29.483C149.803 25.8667 147.56 23.4507 144.031 23.4507C140.502 23.4507 138.259 25.8667 138.259 29.483C138.259 33.0917 140.502 35.5001 144.031 35.5001ZM144.047 33.2969C142.094 33.2969 141.137 31.6103 141.137 29.4754C141.137 27.3406 142.094 25.6312 144.047 25.6312C145.968 25.6312 146.925 27.3406 146.925 29.4754C146.925 31.6103 145.968 33.2969 144.047 33.2969Z" fill="white"/>
+<path d="M159.338 30.3641C159.338 32.1419 158.028 33.0232 156.773 33.0232C155.409 33.0232 154.499 32.0887 154.499 30.6072V23.6025H151.66V31.0327C151.66 33.8361 153.307 35.4239 155.675 35.4239C157.479 35.4239 158.749 34.5046 159.298 33.1979H159.424V35.272H162.176V23.6025H159.338V30.3641Z" fill="white"/>
+<path d="M169.014 35.4769C171.084 35.4769 172.017 34.2841 172.464 33.4332H172.637V35.2718H175.429V19.7124H172.582V25.532H172.464C172.033 24.6887 171.147 23.4503 169.022 23.4503C166.238 23.4503 164.05 25.5624 164.05 29.4522C164.05 33.2965 166.175 35.4769 169.014 35.4769ZM169.806 33.2205C167.931 33.2205 166.943 31.6251 166.943 29.437C166.943 27.2642 167.916 25.7067 169.806 25.7067C171.633 25.7067 172.637 27.173 172.637 29.437C172.637 31.701 171.617 33.2205 169.806 33.2205Z" fill="white"/>
+</g>
+<defs>
+<filter id="filter0_dd" x="0" y="0" width="201" height="60" filterUnits="userSpaceOnUse" color-interpolation-filters="sRGB">
+<feFlood flood-opacity="0" result="BackgroundImageFix"/>
+<feColorMatrix in="SourceAlpha" type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0" result="hardAlpha"/>
+<feOffset/>
+<feGaussianBlur stdDeviation="0.25"/>
+<feColorMatrix type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.5 0"/>
+<feBlend mode="normal" in2="BackgroundImageFix" result="effect1_dropShadow"/>
+<feColorMatrix in="SourceAlpha" type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 127 0" result="hardAlpha"/>
+<feOffset dy="2"/>
+<feGaussianBlur stdDeviation="2"/>
+<feColorMatrix type="matrix" values="0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0.13 0"/>
+<feBlend mode="normal" in2="effect1_dropShadow" result="effect2_dropShadow"/>
+<feBlend mode="normal" in="SourceGraphic" in2="effect2_dropShadow" result="shape"/>
+</filter>
+</defs>
+</svg>

.github/workflows/patch-mariadb-tests.yml

@@ -29,7 +29,7 @@ jobs:
       - name: Setup Python
         uses: actions/setup-python@v2
         with:
-          python-version: 3.7
+          python-version: '3.9'
 
       - name: Check if build should be run
         id: check-build

.github/workflows/publish-assets-develop.yml

@@ -18,7 +18,7 @@ jobs:
           node-version: 14
       - uses: actions/setup-python@v2
         with:
-          python-version: '3.7'
+          python-version: '3.9'
       - name: Set up bench and build assets
         run: |
           npm install -g yarn

.github/workflows/publish-assets-releases.yml

@@ -21,7 +21,7 @@ jobs:
           python-version: '12.x'
       - uses: actions/setup-python@v2
         with:
-          python-version: '3.7'
+          python-version: '3.9'
       - name: Set up bench and build assets
         run: |
           npm install -g yarn

.github/workflows/server-mariadb-tests.yml

@@ -38,7 +38,7 @@ jobs:
       - name: Setup Python
         uses: actions/setup-python@v2
         with:
-          python-version: 3.7
+          python-version: '3.9'
 
       - name: Check if build should be run
         id: check-build

.github/workflows/server-postgres-tests.yml

@@ -41,7 +41,7 @@ jobs:
       - name: Setup Python
         uses: actions/setup-python@v2
         with:
-          python-version: 3.7
+          python-version: '3.9'
 
       - name: Check if build should be run
         id: check-build

.github/workflows/ui-tests.yml

@@ -37,7 +37,7 @@ jobs:
       - name: Setup Python
         uses: actions/setup-python@v2
         with:
-          python-version: 3.7
+          python-version: '3.9'
 
       - name: Check if build should be run
         id: check-build

CODEOWNERS

@@ -15,5 +15,6 @@ core/							@surajshetty3416
 database          @gavindsouza
 model             @gavindsouza
 requirements.txt	@gavindsouza
+query_builder/	@gavindsouza
 commands/					@gavindsouza
 workspace         @shariquerik

README.md

@@ -27,7 +27,7 @@
 		<img src='https://www.codetriage.com/frappe/frappe/badges/users.svg'>
 	</a>
 	<a href="https://codecov.io/gh/frappe/frappe">
-	        <img src="https://codecov.io/gh/frappe/frappe/branch/develop/graph/badge.svg?token=XoTa679hIj"/>
+	        <img src="https://codecov.io/gh/frappe/frappe/branch/develop/graph/badge.svg?token=XoTa679hIj&flag=server"/>
 	</a>
 </div>
 
@@ -35,25 +35,36 @@
 
 Full-stack web application framework that uses Python and MariaDB on the server side and a tightly integrated client side library. Built for [ERPNext](https://erpnext.com)
 
-### Table of Contents
-* [Installation](https://frappeframework.com/docs/user/en/installation)
-* [Documentation](https://frappeframework.com/docs)
+<div align="center">
+	<a href="https://frappecloud.com/deploy?apps=frappe&source=frappe_readme">
+		<img src=".github/try-on-f-cloud-button.svg" height="40">
+	</a>
+</div>
+
+## Table of Contents
+* [Installation](#installation)
+* [Contributing](#contributing)
+* [Resources](#resources)
 * [License](#license)
 
-### Installation
+## Installation
 
 * [Install via Docker](https://github.com/frappe/frappe_docker)
 * [Install via Frappe Bench](https://github.com/frappe/bench)
+* [Offical Documentation](https://frappeframework.com/docs/user/en/installation)
+* [Managed Hosting on Frappe Cloud](https://frappecloud.com/deploy?apps=frappe&source=frappe_readme)
 
 ## Contributing
 
+1. [Code of Conduct](CODE_OF_CONDUCT.md)
 1. [Contribution Guidelines](https://github.com/frappe/erpnext/wiki/Contribution-Guidelines)
+1. [Security Policy](SECURITY.md)
 1. [Translations](https://translate.erpnext.com)
 
-### Website
+## Resources
 
-For details and documentation, see the website
-[https://frappeframework.com](https://frappeframework.com)
+1. [frappeframework.com](https://frappeframework.com) - Official documentation of the Frappe Framework.
+1. [frappe.school](https://frappe.school) - Pick from the various courses by the maintainers or from the community.
 
-### License
+## License
 This repository has been released under the [MIT License](LICENSE).

codecov.yml

@@ -10,11 +10,6 @@ coverage:
         threshold: 0.5%
         flags:
           - server
-      ui-tests:
-        target: auto
-        threshold: 0.5%
-        flags:
-          - ui-tests
 
 comment:
   layout: "diff, flags"
@@ -28,4 +23,4 @@ flags:
   ui-tests:
     paths:
       - ".*\\.js"
-    carryforward: true
+    carryforward: true

cypress/integration/discussions.js

@@ -57,7 +57,23 @@ context('Discussions', () => {
 		cy.get('.discussion-on-page:visible .comment-field').should('have.value', '');
 	};
 
+	const single_thread_discussion = () => {
+		cy.visit('/test-single-thread');
+		cy.get('.discussions-sidebar').should('have.length', 0);
+		cy.get('.reply').should('have.length', 0);
+
+		cy.get('.discussion-on-page .comment-field')
+			.type('This comment is being made on a single thread discussion.')
+			.should('have.value', 'This comment is being made on a single thread discussion.');
+
+		cy.get('.discussion-on-page .submit-discussion').click();
+		cy.wait(3000);
+		cy.get('.discussion-on-page').children(".reply-card").eq(-1).children(".reply-text")
+			.should('have.text', 'This comment is being made on a single thread discussion.\n');
+	};
+
 	it('reply through modal', reply_through_modal);
 	it('reply through comment box', reply_through_comment_box);
 	it('cancel and clear comment box', cancel_and_clear_comment_box);
+	it('single thread discussion', single_thread_discussion);
 });

cypress/integration/timeline.js

@@ -44,13 +44,14 @@ context('Timeline', () => {
 		cy.get('.timeline-content').should('contain', 'Testing Timeline 123');
 
 		//Deleting the added comment
-		cy.get('.actions > .btn > .icon').first().click();
+		cy.get('.more-actions > .action-btn').click();
+		cy.get('.more-actions .dropdown-item').contains('Delete').click();
 		cy.findByRole('button', {name: 'Yes'}).click();
 		cy.click_modal_primary_button('Yes');
 
 		//Deleting the added ToDo
-		cy.get('[id="page-ToDo"] .menu-btn-group button').eq(1).click();
-		cy.get('[id="page-ToDo"] .menu-btn-group [data-label="Delete"]').click();
+		cy.get('[id="page-ToDo"] .menu-btn-group [data-original-title="Menu"]').click();
+		cy.get('[id="page-ToDo"] .menu-btn-group .dropdown-item').contains('Delete').click();
 		cy.findByRole('button', {name: 'Yes'}).click();
 	});
 

cypress/support/commands.js

@@ -353,5 +353,5 @@ Cypress.Commands.add('click_listview_primary_button', (btn_name) => {
 });
 
 Cypress.Commands.add('click_timeline_action_btn', (btn_name) => {
-	cy.get('.timeline-content > .timeline-message-box > .justify-between > .actions > .btn').contains(btn_name).click();
+	cy.get('.timeline-message-box .custom-actions > .btn').contains(btn_name).click();
 });

esbuild/esbuild.js

@@ -44,6 +44,11 @@ let argv = yargs
 		type: "boolean",
 		description: "Run in watch mode and rebuild on file changes"
 	})
+	.option("live-reload", {
+		type: "boolean",
+		description: `Automatically reload web pages when assets are rebuilt.
+			Can only be used with the --watch flag.`
+	})
 	.option("production", {
 		type: "boolean",
 		description: "Run build in production mode"
@@ -104,6 +109,9 @@ async function execute() {
 		log_error("There were some problems during build");
 		log();
 		log(chalk.dim(e.stack));
+		if (process.env.CI) {
+			process.kill(process.pid);
+		}
 		return;
 	}
 
@@ -475,7 +483,8 @@ async function notify_redis({ error, success }) {
 	}
 	if (success) {
 		payload = {
-			success: true
+			success: true,
+			live_reload: argv["live-reload"]
 		};
 	}
 
@@ -528,4 +537,4 @@ function log_rebuilt_assets(prev_assets, new_assets) {
 		log("    " + filename);
 	}
 	log();
-}
+}

frappe/build.py

@@ -246,7 +246,7 @@ def bundle(mode, apps=None, hard_link=False, make_copy=False, restore=False, ver
 
 	check_node_executable()
 	frappe_app_path = frappe.get_app_path("frappe", "..")
-	frappe.commands.popen(command, cwd=frappe_app_path, env=get_node_env())
+	frappe.commands.popen(command, cwd=frappe_app_path, env=get_node_env(), raise_err=True)
 
 
 def watch(apps=None):
@@ -257,6 +257,13 @@ def watch(apps=None):
 	if apps:
 		command += " --apps {apps}".format(apps=apps)
 
+	live_reload = frappe.utils.cint(
+		os.environ.get("LIVE_RELOAD", frappe.conf.live_reload)
+	)
+
+	if live_reload:
+		command += " --live-reload"
+
 	check_node_executable()
 	frappe_app_path = frappe.get_app_path("frappe", "..")
 	frappe.commands.popen(command, cwd=frappe_app_path, env=get_node_env())

frappe/commands/utils.py

@@ -12,10 +12,9 @@ from frappe.exceptions import SiteNotSpecifiedError
 from frappe.utils import update_progress_bar, cint
 from frappe.coverage import CodeCoverage
 
-DATA_IMPORT_DEPRECATION = click.style(
+DATA_IMPORT_DEPRECATION = (
 	"[DEPRECATED] The `import-csv` command used 'Data Import Legacy' which has been deprecated.\n"
-	"Use `data-import` command instead to import data via 'Data Import'.",
-	fg="yellow"
+	"Use `data-import` command instead to import data via 'Data Import'."
 )
 
 
@@ -364,7 +363,7 @@ def import_doc(context, path, force=False):
 @click.option('--no-email', default=True, is_flag=True, help='Send email if applicable')
 @pass_context
 def import_csv(context, path, only_insert=False, submit_after_import=False, ignore_encoding_errors=False, no_email=True):
-	click.secho(DATA_IMPORT_DEPRECATION)
+	click.secho(DATA_IMPORT_DEPRECATION, fg="yellow")
 	sys.exit(1)
 
 
@@ -504,6 +503,12 @@ frappe.db.connect()
 	])
 
 
+def _console_cleanup():
+	# Execute rollback_observers on console close
+	frappe.db.rollback()
+	frappe.destroy()
+
+
 @click.command('console')
 @click.option(
 	'--autoreload',
@@ -519,6 +524,9 @@ def console(context, autoreload=False):
 	frappe.local.lang = frappe.db.get_default("lang")
 
 	from IPython.terminal.embed import InteractiveShellEmbed
+	from atexit import register
+
+	register(_console_cleanup)
 
 	terminal = InteractiveShellEmbed()
 	if autoreload:

frappe/core/doctype/communication/communication.py

@@ -255,7 +255,7 @@ class Communication(Document, CommunicationEmailMixin):
 	def set_delivery_status(self, commit=False):
 		'''Look into the status of Email Queue linked to this Communication and set the Delivery Status of this Communication'''
 		delivery_status = None
-		status_counts = Counter(frappe.db.sql_list('''select status from `tabEmail Queue` where communication=%s''', self.name))
+		status_counts = Counter(frappe.get_all("Email Queue", pluck="status", filters={"communication": self.name}))
 		if self.sent_or_received == "Received":
 			return
 

frappe/core/doctype/communication/mixins.py

@@ -217,17 +217,7 @@ class CommunicationEmailMixin:
 		if not emails:
 			return []
 
-		disabled_users = frappe.db.sql_list("""
-			SELECT
-				email
-			FROM
-				`tabUser`
-			where
-				email in %(emails)s
-				and
-				thread_notify=0
-		""", {'emails': tuple(emails)})
-		return disabled_users
+		return frappe.get_all("User", pluck="email", filters={"email": ["in", emails], "thread_notify": 0})
 
 	@staticmethod
 	def filter_disabled_users(emails):
@@ -236,17 +226,7 @@ class CommunicationEmailMixin:
 		if not emails:
 			return []
 
-		disabled_users = frappe.db.sql_list("""
-			SELECT
-				email
-			FROM
-				`tabUser`
-			where
-				email in %(emails)s
-				and
-				enabled=0
-		""", {'emails': tuple(emails)})
-		return disabled_users
+		return frappe.get_all("User", pluck="email", filters={"email": ["in", emails], "enabled": 0})
 
 	def sendmail_input_dict(self, print_html=None, print_format=None,
 			send_me_a_copy=None, print_letterhead=None, is_inbound_mail_communcation=None):

frappe/core/doctype/data_export/exporter.py

@@ -261,6 +261,7 @@ class DataExporter:
 			self.writer.writerow([self.data_keys.data_separator])
 
 	def add_data(self):
+		from frappe.query_builder import DocType
 		if self.template and not self.with_data:
 			return
 
@@ -305,9 +306,15 @@ class DataExporter:
 			if self.all_doctypes:
 				# add child tables
 				for c in self.child_doctypes:
-					for ci, child in enumerate(frappe.db.sql("""select * from `tab{0}`
-						where parent=%s and parentfield=%s order by idx""".format(c['doctype']),
-						(doc.name, c['parentfield']), as_dict=1)):
+					child_doctype_table = DocType(c["doctype"])
+					data_row = (
+						frappe.qb.from_(child_doctype_table)
+						.select("*")
+						.where(child_doctype_table.parent == doc.name)
+						.where(child_doctype_table.parentfield == c["parentfield"])
+						.orderby(child_doctype_table.idx)
+					)
+					for ci, child in enumerate(data_row.run()):
 						self.add_data_row(rows, c['doctype'], c['parentfield'], child, ci)
 
 			for row in rows:

frappe/core/doctype/doctype/doctype.py

@@ -23,6 +23,7 @@ from frappe.modules.import_file import get_file_path
 from frappe.model.meta import Meta
 from frappe.desk.utils import validate_route_conflict
 from frappe.website.utils import clear_cache
+from frappe.query_builder.functions import Concat
 
 class InvalidFieldNameError(frappe.ValidationError): pass
 class UniqueFieldnameError(frappe.ValidationError): pass
@@ -465,7 +466,7 @@ class DocType(Document):
 			return
 
 		# check if atleast 1 record exists
-		if not (frappe.db.table_exists(self.name) and frappe.db.sql("select name from `tab{}` limit 1".format(self.name))):
+		if not (frappe.db.table_exists(self.name) and frappe.get_all(self.name, fields=["name"], limit=1, as_list=True)):
 			return
 
 		existing_property_setter = frappe.db.get_value("Property Setter", {"doc_type": self.name,
@@ -571,17 +572,17 @@ class DocType(Document):
 	def make_amendable(self):
 		"""If is_submittable is set, add amended_from docfields."""
 		if self.is_submittable:
-			if not frappe.db.sql("""select name from tabDocField
-				where fieldname = 'amended_from' and parent = %s""", self.name):
-					self.append("fields", {
-						"label": "Amended From",
-						"fieldtype": "Link",
-						"fieldname": "amended_from",
-						"options": self.name,
-						"read_only": 1,
-						"print_hide": 1,
-						"no_copy": 1
-					})
+			docfield_exists = frappe.get_all("DocField", filters={"fieldname": "amended_from", "parent": self.name}, pluck="name", limit=1)
+			if not docfield_exists:
+				self.append("fields", {
+					"label": "Amended From",
+					"fieldtype": "Link",
+					"fieldname": "amended_from",
+					"options": self.name,
+					"read_only": 1,
+					"print_hide": 1,
+					"no_copy": 1
+				})
 
 	def make_repeatable(self):
 		"""If allow_auto_repeat is set, add auto_repeat custom field."""
@@ -706,12 +707,13 @@ def validate_series(dt, autoname=None, name=None):
 		and (not autoname.startswith('format:')):
 
 		prefix = autoname.split('.')[0]
-		used_in = frappe.db.sql("""
-			SELECT `name`
-			FROM `tabDocType`
-			WHERE `autoname` LIKE CONCAT(%s, '.%%')
-			AND `name`!=%s
-		""", (prefix, name))
+		doctype = frappe.qb.DocType("DocType")
+		used_in = (frappe.qb
+					.from_(doctype)
+					.select(doctype.name)
+					.where(doctype.autoname.like(Concat(prefix,".%")))
+					.where(doctype.name != name)
+					).run()
 		if used_in:
 			frappe.throw(_("Series {0} already used in {1}").format(prefix, used_in[0][0]))
 

frappe/core/doctype/feedback/test_feedback.py

@@ -5,6 +5,13 @@ import frappe
 import unittest
 
 class TestFeedback(unittest.TestCase):
+	def tearDown(self):
+		frappe.form_dict.reference_doctype = None
+		frappe.form_dict.reference_name = None
+		frappe.form_dict.rating = None
+		frappe.form_dict.feedback = None
+		frappe.local.request_ip = None
+
 	def test_feedback_creation_updation(self):
 		from frappe.website.doctype.blog_post.test_blog_post import make_test_blog
 		test_blog = make_test_blog()
@@ -12,7 +19,14 @@ class TestFeedback(unittest.TestCase):
 		frappe.db.delete("Feedback", {"reference_doctype": "Blog Post"})
 
 		from frappe.templates.includes.feedback.feedback import add_feedback, update_feedback
-		feedback = add_feedback('Blog Post', test_blog.name, 5, 'New feedback')
+
+		frappe.form_dict.reference_doctype = 'Blog Post'
+		frappe.form_dict.reference_name = test_blog.name
+		frappe.form_dict.rating = 5
+		frappe.form_dict.feedback = 'New feedback'
+		frappe.local.request_ip = '127.0.0.1'
+
+		feedback = add_feedback()
 
 		self.assertEqual(feedback.feedback, 'New feedback')
 		self.assertEqual(feedback.rating, 5)

frappe/core/doctype/file/file.py

@@ -813,7 +813,7 @@ def extract_images_from_doc(doc, fieldname):
 		doc.set(fieldname, content)
 
 
-def extract_images_from_html(doc, content):
+def extract_images_from_html(doc, content, is_private=False):
 	frappe.flags.has_dataurl = False
 
 	def _save_file(match):
@@ -846,7 +846,8 @@ def extract_images_from_html(doc, content):
 			"attached_to_doctype": doctype,
 			"attached_to_name": name,
 			"content": content,
-			"decode": False
+			"decode": False,
+			"is_private": is_private
 		})
 		_file.save(ignore_permissions=True)
 		file_url = _file.file_url

frappe/core/doctype/file/test_file.py

@@ -204,10 +204,14 @@ class TestFile(unittest.TestCase):
 
 
 	def delete_test_data(self):
-		for f in frappe.db.sql('''select name, file_name from tabFile where
-			is_home_folder = 0 and is_attachments_folder = 0 order by creation desc'''):
-			frappe.delete_doc("File", f[0])
-
+		test_file_data = frappe.db.get_all(
+			"File",
+			pluck="name",
+			filters={"is_home_folder": 0, "is_attachments_folder": 0},
+			order_by="creation desc",
+		)
+		for f in test_file_data:
+			frappe.delete_doc("File", f)
 
 	def upload_file(self):
 		_file = frappe.get_doc({

frappe/core/doctype/language/language.json

@@ -7,6 +7,7 @@
  "document_type": "Setup",
  "engine": "InnoDB",
  "field_order": [
+  "enabled",
   "language_code",
   "language_name",
   "flag",
@@ -39,15 +40,22 @@
    "fieldtype": "Link",
    "label": "Based On",
    "options": "Language"
+  },
+  {
+   "default": "1",
+   "fieldname": "enabled",
+   "fieldtype": "Check",
+   "label": "Enabled"
   }
  ],
  "icon": "fa fa-globe",
  "in_create": 1,
  "links": [],
- "modified": "2020-04-16 22:11:33.066852",
+ "modified": "2021-10-18 14:02:06.818219",
  "modified_by": "Administrator",
  "module": "Core",
  "name": "Language",
+ "naming_rule": "By fieldname",
  "owner": "Administrator",
  "permissions": [
   {

frappe/core/doctype/log_settings/log_settings.py

@@ -38,7 +38,7 @@ def has_unseen_error_log(user):
 			'message': _("You have unseen {0}").format('<a href="/app/List/Error%20Log/List"> Error Logs </a>')
 		}
 
-	if frappe.db.sql_list("select name from `tabError Log` where seen = 0 limit 1"):
+	if frappe.get_all("Error Log", filters={"seen": 0}, limit=1):
 		log_settings = frappe.get_cached_doc('Log Settings')
 
 		if log_settings.users_to_notify:

frappe/core/doctype/navbar_settings/navbar_settings.py

@@ -22,7 +22,6 @@ class NavbarSettings(Document):
 		if not frappe.flags.in_patch and (len(before_save_items) > len(after_save_items)):
 			frappe.throw(_("Please hide the standard navbar items instead of deleting them"))
 
-@frappe.whitelist(allow_guest=True)
 def get_app_logo():
 	app_logo = frappe.db.get_single_value('Navbar Settings', 'app_logo', cache=True)
 	if not app_logo:

frappe/core/doctype/server_script/server_script.py

@@ -94,7 +94,7 @@ class ServerScript(Document):
 		Args:
 			doc (Document): Executes script with for a certain document's events
 		"""
-		safe_exec(self.script, _locals={"doc": doc})
+		safe_exec(self.script, _locals={"doc": doc}, restrict_commit_rollback=True)
 
 	def execute_scheduled_method(self):
 		"""Specific to Scheduled Jobs via Server Scripts

frappe/core/doctype/server_script/test_server_script.py

@@ -59,6 +59,26 @@ conditions = '1 = 1'
 		reference_doctype = 'Note',
 		script = '''
 frappe.method_that_doesnt_exist("do some magic")
+'''
+	),
+	dict(
+		name='test_todo_commit',
+		script_type = 'DocType Event',
+		doctype_event = 'Before Save',
+		reference_doctype = 'ToDo',
+		disabled = 1,
+		script = '''
+frappe.db.commit()
+'''
+	),
+	dict(
+		name='test_cache_methods',
+		script_type = 'DocType Event',
+		doctype_event = 'Before Save',
+		reference_doctype = 'ToDo',
+		disabled = 1,
+		script = '''
+frappe.cache().set_value('test_key', doc.name)
 '''
 	)
 ]
@@ -119,3 +139,24 @@ class TestServerScript(unittest.TestCase):
 
 		self.assertTrue("invalid python code" in str(se.exception).lower(),
 				msg="Python code validation not working")
+
+	def test_commit_in_doctype_event(self):
+		server_script = frappe.get_doc('Server Script', 'test_todo_commit')
+		server_script.disabled = 0
+		server_script.save()
+
+		self.assertRaises(AttributeError, frappe.get_doc(dict(doctype='ToDo', description='test me')).insert)
+
+		server_script.disabled = 1
+		server_script.save()
+
+	def test_cache_methods_in_server_script(self):
+		server_script = frappe.get_doc('Server Script', 'test_cache_methods')
+		server_script.disabled = 0
+		server_script.save()
+
+		todo = frappe.get_doc(dict(doctype='ToDo', description='test me')).insert()
+		self.assertEqual(todo.name, frappe.cache().get_value('test_key'))
+
+		server_script.disabled = 1
+		server_script.save()

frappe/core/doctype/transaction_log/transaction_log.py

@@ -14,10 +14,9 @@ class TransactionLog(Document):
 		self.row_index = index
 		self.timestamp = now_datetime()
 		if index != 1:
-			prev_hash = frappe.db.sql(
-				"SELECT `chaining_hash` FROM `tabTransaction Log` WHERE `row_index` = '{0}'".format(index - 1))
+			prev_hash = frappe.get_all("Transaction Log", filters={"row_index":str(index-1)}, pluck="chaining_hash", limit=1)
 			if prev_hash:
-				self.previous_hash = prev_hash[0][0]
+				self.previous_hash = prev_hash[0]
 			else:
 				self.previous_hash = "Indexing broken"
 		else:

frappe/core/doctype/user/user.json

@@ -202,7 +202,8 @@
    "fieldname": "role_profile_name",
    "fieldtype": "Link",
    "label": "Role Profile",
-   "options": "Role Profile"
+   "options": "Role Profile",
+   "permlevel": 1
   },
   {
    "fieldname": "roles_html",
@@ -670,7 +671,7 @@
   }
  ],
  "max_attachments": 5,
- "modified": "2021-02-02 16:11:06.037543",
+ "modified": "2021-10-18 16:56:05.578379",
  "modified_by": "Administrator",
  "module": "Core",
  "name": "User",

frappe/core/doctype/user_permission/user_permission.py

@@ -54,7 +54,7 @@ class UserPermission(Document):
 			ref_link = frappe.get_desk_link(self.doctype, overlap_exists[0].name)
 			frappe.throw(_("{0} has already assigned default value for {1}.").format(ref_link, self.allow))
 
-@frappe.whitelist(allow_guest=True)
+@frappe.whitelist()
 def get_user_permissions(user=None):
 	'''Get all users permissions for the user as a dict of doctype'''
 	# if this is called from client-side,

frappe/core/page/permission_manager/permission_manager.js

@@ -325,15 +325,15 @@ frappe.PermissionEngine = class PermissionEngine {
 			.attr("data-doctype", d.parent)
 			.attr("data-role", d.role)
 			.attr("data-permlevel", d.permlevel)
-			.click(function () {
+			.on("click", () => {
 				return frappe.call({
 					module: "frappe.core",
 					page: "permission_manager",
 					method: "remove",
 					args: {
-						doctype: $(this).attr("data-doctype"),
-						role: $(this).attr("data-role"),
-						permlevel: $(this).attr("data-permlevel")
+						doctype: d.parent,
+						role: d.role,
+						permlevel: d.permlevel
 					},
 					callback: (r) => {
 						if (r.exc) {

frappe/database/database.py

@@ -113,6 +113,7 @@ class Database(object):
 		query = str(query)
 		if not run:
 			return query
+
 		if re.search(r'ifnull\(', query, flags=re.IGNORECASE):
 			# replaces ifnull in query with coalesce
 			query = re.sub(r'ifnull\(', 'coalesce(', query, flags=re.IGNORECASE)

frappe/database/mariadb/framework_mariadb.sql

@@ -226,6 +226,7 @@ CREATE TABLE `tabDocType` (
   `email_append_to` int(1) NOT NULL DEFAULT 0,
   `subject_field` varchar(255) DEFAULT NULL,
   `sender_field` varchar(255) DEFAULT NULL,
+  `migration_hash` varchar(255) DEFAULT NULL,
   PRIMARY KEY (`name`),
   KEY `parent` (`parent`)
 ) ENGINE=InnoDB ROW_FORMAT=DYNAMIC CHARACTER SET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

frappe/database/postgres/framework_postgres.sql

@@ -231,6 +231,7 @@ CREATE TABLE "tabDocType" (
   "email_append_to" smallint NOT NULL DEFAULT 0,
   "subject_field" varchar(255) DEFAULT NULL,
   "sender_field" varchar(255) DEFAULT NULL,
+  "migration_hash" varchar(255) DEFAULT NULL,
   PRIMARY KEY ("name")
 ) ;
 

frappe/desk/form/load.py

@@ -13,7 +13,7 @@ from frappe.desk.form.document_follow import is_document_followed
 from frappe import _
 from urllib.parse import quote
 
-@frappe.whitelist(allow_guest=True)
+@frappe.whitelist()
 def getdoc(doctype, name, user=None):
 	"""
 	Loads a doclist for a given document. This method is called directly from the client.
@@ -52,7 +52,7 @@ def getdoc(doctype, name, user=None):
 
 	frappe.response.docs.append(doc)
 
-@frappe.whitelist(allow_guest=True)
+@frappe.whitelist()
 def getdoctype(doctype, with_parent=False, cached_timestamp=None):
 	"""load doctype"""
 

frappe/desk/form/utils.py

@@ -66,7 +66,8 @@ def add_comment(reference_doctype, reference_name, content, comment_email, comme
 		comment_type='Comment',
 		comment_by=comment_by
 	))
-	doc.content = extract_images_from_html(doc, content)
+	reference_doc = frappe.get_doc(reference_doctype, reference_name)
+	doc.content = extract_images_from_html(reference_doc, content, is_private=True)
 	doc.insert(ignore_permissions=True)
 
 	follow_document(doc.reference_doctype, doc.reference_name, frappe.session.user)

frappe/desk/listview.py

@@ -2,7 +2,7 @@
 # License: MIT. See LICENSE
 import frappe
 
-@frappe.whitelist(allow_guest=True)
+@frappe.whitelist()
 def get_list_settings(doctype):
 	try:
 		return frappe.get_cached_doc("List View Settings", doctype)

frappe/desk/reportview.py

@@ -14,7 +14,7 @@ from frappe.utils import cstr, format_duration
 from frappe.model.base_document import get_controller
 
 
-@frappe.whitelist(allow_guest=True)
+@frappe.whitelist()
 @frappe.read_only()
 def get():
 	args = get_form_params()
@@ -127,6 +127,8 @@ def setup_group_by(data):
 
 		if frappe.db.has_column(data.aggregate_on_doctype, data.aggregate_on_field):
 			data.fields.append('{aggregate_function}(`tab{aggregate_on_doctype}`.`{aggregate_on_field}`) AS _aggregate_column'.format(**data))
+			if data.aggregate_on_field:
+				data.fields.append(f"`tab{data.aggregate_on_doctype}`.`{data.aggregate_on_field}`")
 		else:
 			raise_invalid_field(data.aggregate_on_field)
 

frappe/email/doctype/auto_email_report/auto_email_report.py

@@ -249,7 +249,7 @@ def make_links(columns, data):
 				if col.options and row.get(col.fieldname) and row.get(col.options):
 					row[col.fieldname] = get_link_to_form(row[col.options], row[col.fieldname])
 			elif col.fieldtype == "Currency" and row.get(col.fieldname):
-				doc = frappe.get_doc(col.parent, doc_name) if doc_name else None
+				doc = frappe.get_doc(col.parent, doc_name) if doc_name and col.parent else None
 				# 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

frappe/email/doctype/notification/test_notification.py

@@ -274,4 +274,7 @@ class TestNotification(unittest.TestCase):
 		self.assertTrue('test2@example.com' in recipients)
 		self.assertTrue('test1@example.com' in recipients)
 
-
+	@classmethod
+	def tearDownClass(cls):
+		frappe.delete_doc_if_exists("Notification", "ToDo Status Update")
+		frappe.delete_doc_if_exists("Notification", "Contact Status Update")

frappe/installer.py

@@ -4,6 +4,8 @@
 import json
 import os
 import sys
+from collections import OrderedDict
+from typing import List, Dict
 
 import frappe
 from frappe.defaults import _clear_cache
@@ -158,7 +160,7 @@ def install_app(name, verbose=False, set_as_patched=True):
 	if name != "frappe":
 		add_module_defs(name)
 
-	sync_for(name, force=True, sync_everything=True, verbose=verbose, reset_permissions=True)
+	sync_for(name, force=True, reset_permissions=True)
 
 	add_to_installed_apps(name)
 
@@ -230,9 +232,29 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False)
 		scheduled_backup(ignore_files=True)
 
 	frappe.flags.in_uninstall = True
-	drop_doctypes = []
 
 	modules = frappe.get_all("Module Def", filters={"app_name": app_name}, pluck="name")
+
+	drop_doctypes = _delete_modules(modules, dry_run=dry_run)
+	_delete_doctypes(drop_doctypes, dry_run=dry_run)
+
+	if not dry_run:
+		remove_from_installed_apps(app_name)
+		frappe.db.commit()
+
+	click.secho(f"Uninstalled App {app_name} from Site {site}", fg="green")
+	frappe.flags.in_uninstall = False
+
+
+def _delete_modules(modules: List[str], dry_run: bool) -> List[str]:
+	""" Delete modules belonging to the app and all related doctypes.
+
+		Note: All record linked linked to Module Def are also deleted.
+
+		Returns: list of deleted doctypes."""
+	drop_doctypes = []
+
+	doctype_link_field_map = _get_module_linked_doctype_field_map()
 	for module_name in modules:
 		print(f"Deleting Module '{module_name}'")
 
@@ -242,45 +264,67 @@ def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False)
 			print(f"* removing DocType '{doctype.name}'...")
 
 			if not dry_run:
-				frappe.delete_doc("DocType", doctype.name, ignore_on_trash=True)
-
-				if not doctype.issingle:
+				if doctype.issingle:
+					frappe.delete_doc("DocType", doctype.name, ignore_on_trash=True)
+				else:
 					drop_doctypes.append(doctype.name)
 
-		linked_doctypes = frappe.get_all(
-			"DocField", filters={"fieldtype": "Link", "options": "Module Def"}, fields=["parent"]
-		)
-		ordered_doctypes = ["Workspace", "Report", "Page", "Web Form"]
-		all_doctypes_with_linked_modules = ordered_doctypes + [
-			doctype.parent
-			for doctype in linked_doctypes
-			if doctype.parent not in ordered_doctypes
-		]
-		doctypes_with_linked_modules = [
-			x for x in all_doctypes_with_linked_modules if frappe.db.exists("DocType", x)
-		]
-		for doctype in doctypes_with_linked_modules:
-			for record in frappe.get_all(doctype, filters={"module": module_name}, pluck="name"):
-				print(f"* removing {doctype} '{record}'...")
-				if not dry_run:
-					frappe.delete_doc(doctype, record, ignore_on_trash=True, force=True)
+		_delete_linked_documents(module_name, doctype_link_field_map, dry_run=dry_run)
 
 		print(f"* removing Module Def '{module_name}'...")
 		if not dry_run:
 			frappe.delete_doc("Module Def", module_name, ignore_on_trash=True, force=True)
 
-	for doctype in set(drop_doctypes):
+	return drop_doctypes
+
+
+def _delete_linked_documents(
+		module_name: str,
+		doctype_linkfield_map: Dict[str, str],
+		dry_run: bool
+	) -> None:
+
+	"""Deleted all records linked with module def"""
+	for doctype, fieldname in doctype_linkfield_map.items():
+		for record in frappe.get_all(doctype, filters={fieldname: module_name}, pluck="name"):
+			print(f"* removing {doctype} '{record}'...")
+			if not dry_run:
+				frappe.delete_doc(doctype, record, ignore_on_trash=True, force=True)
+
+def _get_module_linked_doctype_field_map() -> Dict[str, str]:
+	""" Get all the doctypes which have module linked with them.
+
+		returns ordered dictionary with doctype->link field mapping."""
+
+	# Hardcoded to change order of deletion
+	ordered_doctypes = [
+			("Workspace", "module"),
+			("Report", "module"),
+			("Page", "module"),
+			("Web Form", "module")
+	]
+	doctype_to_field_map = OrderedDict(ordered_doctypes)
+
+	linked_doctypes = frappe.get_all(
+		"DocField", filters={"fieldtype": "Link", "options": "Module Def"}, fields=["parent", "fieldname"]
+	)
+	existing_linked_doctypes = [d for d in linked_doctypes if frappe.db.exists("DocType", d.parent)]
+
+	for d in existing_linked_doctypes:
+		# DocType deletion is handled separately in the end
+		if d.parent not in doctype_to_field_map and d.parent != "DocType":
+			doctype_to_field_map[d.parent] = d.fieldname
+
+	return doctype_to_field_map
+
+
+def _delete_doctypes(doctypes: List[str], dry_run: bool) -> None:
+	for doctype in set(doctypes):
 		print(f"* dropping Table for '{doctype}'...")
 		if not dry_run:
+			frappe.delete_doc("DocType", doctype, ignore_on_trash=True)
 			frappe.db.sql_ddl(f"drop table `tab{doctype}`")
 
-	if not dry_run:
-		remove_from_installed_apps(app_name)
-		frappe.db.commit()
-
-	click.secho(f"Uninstalled App {app_name} from Site {site}", fg="green")
-	frappe.flags.in_uninstall = False
-
 
 def post_install(rebuild_website=False):
 	from frappe.website.utils import clear_website_cache
@@ -456,9 +500,20 @@ def convert_archive_content(sql_file_path):
 	if frappe.conf.db_type == "mariadb":
 		# ever since mariaDB 10.6, row_format COMPRESSED has been deprecated and removed
 		# this step is added to ease restoring sites depending on older mariaDB servers
-		contents = open(sql_file_path).read()
-		with open(sql_file_path, "w") as f:
-			f.write(contents.replace("ROW_FORMAT=COMPRESSED", "ROW_FORMAT=DYNAMIC"))
+		from frappe.utils import random_string
+		from pathlib import Path
+
+		old_sql_file_path = Path(f"{sql_file_path}_{random_string(10)}")
+		sql_file_path = Path(sql_file_path)
+
+		os.rename(sql_file_path, old_sql_file_path)
+		sql_file_path.touch()
+
+		with open(old_sql_file_path) as r, open(sql_file_path, "a") as w:
+			for line in r:
+				w.write(line.replace("ROW_FORMAT=COMPRESSED", "ROW_FORMAT=DYNAMIC"))
+
+		old_sql_file_path.unlink()
 
 
 def extract_sql_gzip(sql_gz_path):

frappe/integrations/doctype/dropbox_settings/dropbox_settings.py

@@ -336,7 +336,6 @@ def dropbox_auth_finish(return_access_token=False):
 		_("Dropbox access is approved!") + close,
 		indicator_color='green')
 
-@frappe.whitelist(allow_guest=True)
 def set_dropbox_access_token(access_token):
 	frappe.db.set_value("Dropbox Settings", None, 'dropbox_access_token', access_token)
 	frappe.db.commit()

frappe/migrate.py

@@ -18,6 +18,7 @@ from frappe.core.doctype.language.language import sync_languages
 from frappe.modules.utils import sync_customizations
 from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs
 from frappe.search.website_search import build_index_for_all_routes
+from frappe.database.schema import add_column
 
 
 def migrate(verbose=True, skip_failing=False, skip_search_index=False):
@@ -26,9 +27,10 @@ def migrate(verbose=True, skip_failing=False, skip_search_index=False):
 	- run patches
 	- sync doctypes (schema)
 	- sync dashboards
+	- sync jobs
 	- sync fixtures
-	- sync desktop icons
-	- sync web pages (from /www)
+	- sync customizations
+	- sync languages
 	- sync web pages (from /www)
 	- run after migrate hooks
 	'''
@@ -51,6 +53,7 @@ Otherwise, check the server logs and ensure that all the required services are r
 		os.remove(touched_tables_file)
 
 	try:
+		add_column(doctype="DocType", column_name="migration_hash", fieldtype="Data")
 		frappe.flags.touched_tables = set()
 		frappe.flags.in_migrate = True
 
@@ -65,7 +68,7 @@ Otherwise, check the server logs and ensure that all the required services are r
 		frappe.modules.patch_handler.run_all(skip_failing)
 
 		# sync
-		frappe.model.sync.sync_all(verbose=verbose)
+		frappe.model.sync.sync_all()
 		frappe.translate.clear_cache()
 		sync_jobs()
 		sync_fixtures()

frappe/model/base_document.py

@@ -267,7 +267,12 @@ class BaseDocument(object):
 				if isinstance(d[fieldname], list) and df.fieldtype not in table_fields:
 					frappe.throw(_('Value for {0} cannot be a list').format(_(df.label)))
 
-			if convert_dates_to_str and isinstance(d[fieldname], (datetime.datetime, datetime.time, datetime.timedelta)):
+			if convert_dates_to_str and isinstance(d[fieldname], (
+				datetime.datetime,
+				datetime.date,
+				datetime.time,
+				datetime.timedelta
+			)):
 				d[fieldname] = str(d[fieldname])
 
 			if d[fieldname] == None and ignore_nulls:

frappe/model/db_query.py

@@ -597,8 +597,8 @@ class DatabaseQuery(object):
 				self.conditions.append(self.get_share_condition())
 
 		else:
-			#if has if_owner permission skip user perm check
-			if role_permissions.get("has_if_owner_enabled") and role_permissions.get("if_owner", {}):
+			# skip user perm check if owner constraint is required
+			if requires_owner_constraint(role_permissions):
 				self.match_conditions.append(
 					f"`tab{self.doctype}`.`owner` = {frappe.db.escape(self.user, percent=False)}"
 				)
@@ -895,3 +895,22 @@ def get_date_range(operator, value):
 	timespan = period_map[operator] + ' ' + timespan_map[value] if operator != 'timespan' else value
 
 	return get_timespan_date_range(timespan)
+
+def requires_owner_constraint(role_permissions):
+	"""Returns True if "select" or "read" isn't available without being creator."""
+
+	if not role_permissions.get("has_if_owner_enabled"):
+		return
+
+	if_owner_perms = role_permissions.get("if_owner")
+	if not if_owner_perms:
+		return
+
+	# has select or read without if owner, no need for constraint
+	for perm_type in ("select", "read"):
+		if role_permissions.get(perm_type) and perm_type not in if_owner_perms:
+			return
+
+	# not checking if either select or read if present in if_owner_perms
+	# because either of those is required to perform a query
+	return True

frappe/model/sync.py

@@ -10,62 +10,67 @@ from frappe.modules.import_file import import_file_by_path
 from frappe.modules.patch_handler import block_user
 from frappe.utils import update_progress_bar
 
-def sync_all(force=0, verbose=False, reset_permissions=False):
+
+def sync_all(force=0, reset_permissions=False):
 	block_user(True)
 
 	for app in frappe.get_installed_apps():
-		sync_for(app, force, verbose=verbose, reset_permissions=reset_permissions)
+		sync_for(app, force, reset_permissions=reset_permissions)
 
 	block_user(False)
 
 	frappe.clear_cache()
 
-def sync_for(app_name, force=0, sync_everything = False, verbose=False, reset_permissions=False):
+
+def sync_for(app_name, force=0, reset_permissions=False):
 	files = []
 
 	if app_name == "frappe":
 		# these need to go first at time of install
-		for d in (("core", "docfield"),
-			("core", "docperm"),
-			("core", "doctype_action"),
-			("core", "doctype_link"),
-			("core", "role"),
-			("core", "has_role"),
-			("core", "doctype"),
-			("core", "user"),
-			("custom", "custom_field"),
-			("custom", "property_setter"),
-			("website", "web_form"),
-			("website", "web_template"),
-			("website", "web_form_field"),
-			("website", "portal_menu_item"),
-			("data_migration", "data_migration_mapping_detail"),
-			("data_migration", "data_migration_mapping"),
-			("data_migration", "data_migration_plan_mapping"),
-			("data_migration", "data_migration_plan"),
-			("desk", "number_card"),
-			("desk", "dashboard_chart"),
-			("desk", "dashboard"),
-			("desk", "onboarding_permission"),
-			("desk", "onboarding_step"),
-			("desk", "onboarding_step_map"),
-			("desk", "module_onboarding"),
-			("desk", "workspace_link"),
-			("desk", "workspace_chart"),
-			("desk", "workspace_shortcut"),
-			("desk", "workspace")):
-			files.append(os.path.join(frappe.get_app_path("frappe"), d[0],
-				"doctype", d[1], d[1] + ".json"))
+
+		FRAPPE_PATH = frappe.get_app_path("frappe")
+
+		for core_module in ["docfield", "docperm", "doctype_action", "doctype_link", "role", "has_role", "doctype"]:
+			files.append(os.path.join(FRAPPE_PATH, "core", "doctype", core_module, f"{core_module}.json"))
+
+		for custom_module in ["custom_field", "property_setter"]:
+			files.append(os.path.join(FRAPPE_PATH, "custom", "doctype", custom_module, f"{custom_module}.json"))
+
+		for website_module in ["web_form", "web_template", "web_form_field", "portal_menu_item"]:
+			files.append(os.path.join(FRAPPE_PATH, "website", "doctype", website_module, f"{website_module}.json"))
+
+		for data_migration_module in [
+			"data_migration_mapping_detail",
+			"data_migration_mapping",
+			"data_migration_plan_mapping",
+			"data_migration_plan",
+		]:
+			files.append(os.path.join(FRAPPE_PATH, "data_migration", "doctype", data_migration_module, f"{data_migration_module}.json"))
+
+		for desk_module in [
+			"number_card",
+			"dashboard_chart",
+			"dashboard",
+			"onboarding_permission",
+			"onboarding_step",
+			"onboarding_step_map",
+			"module_onboarding",
+			"workspace_link",
+			"workspace_chart",
+			"workspace_shortcut",
+			"workspace",
+		]:
+			files.append(os.path.join(FRAPPE_PATH, "desk", "doctype", desk_module, f"{desk_module}.json"))
 
 	for module_name in frappe.local.app_modules.get(app_name) or []:
 		folder = os.path.dirname(frappe.get_module(app_name + "." + module_name).__file__)
-		get_doc_files(files, folder)
+		files = get_doc_files(files=files, start_path=folder)
 
 	l = len(files)
+
 	if l:
 		for i, doc_path in enumerate(files):
-			import_file_by_path(doc_path, force=force, ignore_version=True,
-				reset_permissions=reset_permissions, for_sync=True)
+			import_file_by_path(doc_path, force=force, ignore_version=True, reset_permissions=reset_permissions)
 
 			frappe.db.commit()
 
@@ -75,17 +80,36 @@ def sync_for(app_name, force=0, sync_everything = False, verbose=False, reset_pe
 		# print each progress bar on new line
 		print()
 
+
 def get_doc_files(files, start_path):
 	"""walk and sync all doctypes and pages"""
 
-	# load in sequence - warning for devs
-	document_types = ['doctype', 'page', 'report', 'dashboard_chart_source', 'print_format',
-		'web_page', 'website_theme', 'web_form', 'web_template',
-		'notification', 'print_style',
-		'data_migration_mapping', 'data_migration_plan',
-		'workspace', 'onboarding_step', 'module_onboarding', 'form_tour',
-		'client_script', 'server_script', 'custom_field', 'property_setter']
+	files = files or []
 
+	# load in sequence - warning for devs
+	document_types = [
+		"doctype",
+		"page",
+		"report",
+		"dashboard_chart_source",
+		"print_format",
+		"web_page",
+		"website_theme",
+		"web_form",
+		"web_template",
+		"notification",
+		"print_style",
+		"data_migration_mapping",
+		"data_migration_plan",
+		"workspace",
+		"onboarding_step",
+		"module_onboarding",
+		"form_tour",
+		"client_script",
+		"server_script",
+		"custom_field",
+		"property_setter",
+	]
 	for doctype in document_types:
 		doctype_path = os.path.join(start_path, doctype)
 		if os.path.exists(doctype_path):
@@ -95,3 +119,5 @@ def get_doc_files(files, start_path):
 					if os.path.exists(doc_path):
 						if not doc_path in files:
 							files.append(doc_path)
+
+	return files

frappe/modules/import_file.py

@@ -1,31 +1,53 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 # License: MIT. See LICENSE
-import frappe, os, json
-from frappe.modules import get_module_path, scrub_dt_dn
-from frappe.utils import get_datetime_str
+import hashlib
+import json
+import os
+
+import frappe
 from frappe.model.base_document import get_controller
+from frappe.modules import get_module_path, scrub_dt_dn
+from frappe.query_builder import DocType
+from frappe.utils import get_datetime_str, now
+
+
+def caclulate_hash(path: str) -> str:
+	"""Calculate md5 hash of the file in binary mode
+
+	Args:
+		path (str): Path to the file to be hashed
+
+	Returns:
+		str: The calculated hash
+	"""
+	hash_md5 = hashlib.md5()
+	with open(path, "rb") as f:
+		for chunk in iter(lambda: f.read(4096), b""):
+			hash_md5.update(chunk)
+	return hash_md5.hexdigest()
+
 
 ignore_values = {
 	"Report": ["disabled", "prepared_report", "add_total_row"],
 	"Print Format": ["disabled"],
 	"Notification": ["enabled"],
 	"Print Style": ["disabled"],
-	"Module Onboarding": ['is_complete'],
-	"Onboarding Step": ['is_complete', 'is_skipped']
+	"Module Onboarding": ["is_complete"],
+	"Onboarding Step": ["is_complete", "is_skipped"],
 }
 
 ignore_doctypes = [""]
 
+
 def import_files(module, dt=None, dn=None, force=False, pre_process=None, reset_permissions=False):
 	if type(module) is list:
 		out = []
 		for m in module:
-			out.append(import_file(m[0], m[1], m[2], force=force, pre_process=pre_process,
-				reset_permissions=reset_permissions))
+			out.append(import_file(m[0], m[1], m[2], force=force, pre_process=pre_process, reset_permissions=reset_permissions))
 		return out
 	else:
-		return import_file(module, dt, dn, force=force, pre_process=pre_process,
-			reset_permissions=reset_permissions)
+		return import_file(module, dt, dn, force=force, pre_process=pre_process, reset_permissions=reset_permissions)
+
 
 def import_file(module, dt, dn, force=False, pre_process=None, reset_permissions=False):
 	"""Sync a file from txt if modifed, return false if not updated"""
@@ -33,77 +55,160 @@ def import_file(module, dt, dn, force=False, pre_process=None, reset_permissions
 	ret = import_file_by_path(path, force, pre_process=pre_process, reset_permissions=reset_permissions)
 	return ret
 
+
 def get_file_path(module, dt, dn):
 	dt, dn = scrub_dt_dn(dt, dn)
 
-	path = os.path.join(get_module_path(module),
-		os.path.join(dt, dn, dn + ".json"))
+	path = os.path.join(get_module_path(module), os.path.join(dt, dn, f"{dn}.json"))
 
 	return path
 
-def import_file_by_path(path, force=False, data_import=False, pre_process=None, ignore_version=None,
-		reset_permissions=False, for_sync=False):
+
+def import_file_by_path(path: str,force: bool = False,data_import: bool = False,pre_process = None,ignore_version: bool = None,reset_permissions: bool = False):
+	"""Import file from the given path
+
+	Some conditions decide if a file should be imported or not.
+	Evaluation takes place in the order they are mentioned below.
+
+	- Check if `force` is true. Import the file. If not, move ahead.
+	- Get `db_modified_timestamp`(value of the modified field in the database for the file).
+		If the return is `none,` this file doesn't exist in the DB, so Import the file. If not, move ahead.
+	- Check if there is a hash in DB for that file. If there is, Calculate the Hash of the file to import and compare it with the one in DB if they are not equal.
+		Import the file. If Hash doesn't exist, move ahead.
+	- Check if `db_modified_timestamp` is older than the timestamp in the file; if it is, we import the file.
+
+	If timestamp comparison happens for doctypes, that means the Hash for it doesn't exist.
+	So, even if the timestamp is newer on DB (When comparing timestamps), we import the file and add the calculated Hash to the DB.
+	So in the subsequent imports, we can use hashes to compare. As a precautionary measure, the timestamp is updated to the current time as well.
+
+	Args:
+		path (str): Path to the file.
+		force (bool, optional): Load the file without checking any conditions. Defaults to False.
+		data_import (bool, optional): [description]. Defaults to False.
+		pre_process ([type], optional): Any preprocesing that may need to take place on the doc. Defaults to None.
+		ignore_version (bool, optional): ignore current version. Defaults to None.
+		reset_permissions (bool, optional): reset permissions for the file. Defaults to False.
+
+	Returns:
+		[bool]: True if import takes place. False if it wasn't imported.
+	"""
+	frappe.flags.dt = frappe.flags.dt or []
 	try:
 		docs = read_doc_from_file(path)
 	except IOError:
-		print (path + " missing")
+		print(f"{path} missing")
 		return
 
+	calculated_hash = caclulate_hash(path)
+
 	if docs:
 		if not isinstance(docs, list):
 			docs = [docs]
 
 		for doc in docs:
-			if not force and not is_changed(doc):
-				return False
 
-			original_modified = doc.get("modified")
-
-			import_doc(doc, force=force, data_import=data_import, pre_process=pre_process,
-				ignore_version=ignore_version, reset_permissions=reset_permissions, path=path)
-
-			if original_modified:
-				update_modified(original_modified, doc)
+			# modified timestamp in db, none if doctype's first import
+			db_modified_timestamp = frappe.db.get_value(doc["doctype"], doc["name"], "modified")
+			is_db_timestamp_latest = db_modified_timestamp and doc.get("modified") <= get_datetime_str(db_modified_timestamp)
+
+			if not force or db_modified_timestamp:
+				try:
+					stored_hash = frappe.db.get_value(doc["doctype"], doc["name"], "migration_hash")
+				except Exception:
+					frappe.flags.dt += [doc["doctype"]]
+					stored_hash = None
+
+				# if hash exists and is equal no need to update
+				if stored_hash and stored_hash == calculated_hash:
+					return False
+
+				# if hash doesn't exist, check if db timestamp is same as json timestamp, add hash if from doctype
+				if is_db_timestamp_latest and doc["doctype"] != "DocType":
+					return False
+
+			import_doc(
+				docdict=doc,
+				force=force,
+				data_import=data_import,
+				pre_process=pre_process,
+				ignore_version=ignore_version,
+				reset_permissions=reset_permissions,
+				path=path,
+			)
+
+			if doc["doctype"] == "DocType":
+				doctype_table = DocType("DocType")
+				frappe.qb.update(
+					doctype_table
+				).set(
+					doctype_table.migration_hash, calculated_hash
+				).where(
+					doctype_table.name == doc["name"]
+				).run()
+
+			new_modified_timestamp = doc.get("modified")
+
+			# if db timestamp is newer, hash must have changed, must update db timestamp
+			if is_db_timestamp_latest and doc["doctype"] == "DocType":
+				new_modified_timestamp = now()
+
+			if new_modified_timestamp:
+				update_modified(new_modified_timestamp, doc)
 
 	return True
 
-def is_changed(doc):
+
+def is_timestamp_changed(doc):
 	# check if timestamps match
-	db_modified = frappe.db.get_value(doc['doctype'], doc['name'], 'modified')
-	if db_modified and doc.get('modified')==get_datetime_str(db_modified):
-		return False
-	return True
+	db_modified = frappe.db.get_value(doc["doctype"], doc["name"], "modified")
+	return not (db_modified and doc.get("modified") == get_datetime_str(db_modified))
+
 
 def read_doc_from_file(path):
 	doc = None
 	if os.path.exists(path):
-		with open(path, 'r') as f:
+		with open(path, "r") as f:
 			try:
 				doc = json.loads(f.read())
 			except ValueError:
 				print("bad json: {0}".format(path))
 				raise
 	else:
-		raise IOError('%s missing' % path)
+		raise IOError("%s missing" % path)
 
 	return doc
 
+
 def update_modified(original_modified, doc):
 	# since there is a new timestamp on the file, update timestamp in
-	if doc["doctype"] == doc["name"] and doc["name"]!="DocType":
-		frappe.db.sql("""update tabSingles set value=%s where field="modified" and doctype=%s""",
-			(original_modified, doc["name"]))
+	if doc["doctype"] == doc["name"] and doc["name"] != "DocType":
+		singles_table = DocType("Singles")
+
+		frappe.qb.update(
+			singles_table
+		).set(
+			singles_table.value,original_modified
+		).where(
+			singles_table.field == "modified"
+		).where(
+			singles_table.doctype == doc["name"]
+		).run()
 	else:
-		frappe.db.sql("update `tab%s` set modified=%s where name=%s" % (doc['doctype'],
-			'%s', '%s'), (original_modified, doc['name']))
+		doctype_table = DocType(doc['doctype'])
 
-def import_doc(docdict, force=False, data_import=False, pre_process=None,
-		ignore_version=None, reset_permissions=False, path=None):
+		frappe.qb.update(doctype_table
+		).set(
+			doctype_table.modified, original_modified
+		).where(
+			doctype_table.name == doc["name"]
+		).run()
+
+def import_doc(docdict, force=False, data_import=False, pre_process=None, ignore_version=None, reset_permissions=False, path=None):
 	frappe.flags.in_import = True
 	docdict["__islocal"] = 1
 
-	controller = get_controller(docdict['doctype'])
-	if controller and hasattr(controller, 'prepare_for_import') and callable(getattr(controller, 'prepare_for_import')):
+	controller = get_controller(docdict["doctype"])
+	if controller and hasattr(controller, "prepare_for_import") and callable(getattr(controller, "prepare_for_import")):
 		controller.prepare_for_import(docdict)
 
 	doc = frappe.get_doc(docdict)
@@ -132,15 +237,16 @@ def import_doc(docdict, force=False, data_import=False, pre_process=None,
 
 	return doc
 
+
 def load_code_properties(doc, path):
-	'''Load code files stored in separate files with extensions'''
+	"""Load code files stored in separate files with extensions"""
 	if path:
-		if hasattr(doc, 'get_code_fields'):
+		if hasattr(doc, "get_code_fields"):
 			dirname, filename = os.path.split(path)
 			for key, extn in doc.get_code_fields().items():
-				codefile = os.path.join(dirname, filename.split('.')[0]+'.'+extn)
+				codefile = os.path.join(dirname, filename.split(".")[0] + "." + extn)
 				if os.path.exists(codefile):
-					with open(codefile,'r') as txtfile:
+					with open(codefile, "r") as txtfile:
 						doc.set(key, txtfile.read())
 
 
@@ -164,12 +270,13 @@ def delete_old_doc(doc, reset_permissions):
 
 	doc.flags.ignore_children_type = ignore
 
+
 def reset_tree_properties(doc):
 	# Note on Tree DocTypes:
 	# The tree structure is maintained in the database via the fields "lft" and
 	# "rgt". They are automatically set and kept up-to-date. Importing them
 	# would destroy any existing tree structure.
-	if getattr(doc.meta, 'is_tree', None) and any([doc.lft, doc.rgt]):
+	if getattr(doc.meta, "is_tree", None) and any([doc.lft, doc.rgt]):
 		print('Ignoring values of `lft` and `rgt` for {} "{}"'.format(doc.doctype, doc.name))
 		doc.lft = None
 		doc.rgt = None

frappe/patches/v11_0/sync_user_permission_doctype_before_migrate.py

@@ -1,7 +1,7 @@
-
 import frappe
 
+
 def execute():
 	frappe.flags.in_patch = True
-	frappe.reload_doc('core', 'doctype', 'user_permission')
+	frappe.reload_doc("core", "doctype", "user_permission")
 	frappe.db.commit()

frappe/permissions.py

@@ -107,13 +107,9 @@ def get_doc_permissions(doc, user=None, ptype=None):
 	meta = frappe.get_meta(doc.doctype)
 
 	def is_user_owner():
-		doc_owner = doc.get('owner') or ''
-		doc_owner = doc_owner.lower()
-		session_user = frappe.session.user.lower()
-		return doc_owner == session_user
+		return (doc.get("owner") or "").lower() == frappe.session.user.lower()
 
-
-	if has_controller_permissions(doc, ptype, user=user) == False :
+	if has_controller_permissions(doc, ptype, user=user) is False:
 		push_perm_check_log('Not allowed via controller permission check')
 		return {ptype: 0}
 
@@ -182,22 +178,23 @@ def get_role_permissions(doctype_meta, user=None, is_owner=None):
 
 		applicable_permissions = list(filter(is_perm_applicable, getattr(doctype_meta, 'permissions', [])))
 		has_if_owner_enabled = any(p.get('if_owner', 0) for p in applicable_permissions)
-
 		perms['has_if_owner_enabled'] = has_if_owner_enabled
 
 		for ptype in rights:
 			pvalue = any(p.get(ptype, 0) for p in applicable_permissions)
 			# check if any perm object allows perm type
 			perms[ptype] = cint(pvalue)
-			if (pvalue
-				and has_if_owner_enabled
-				and not has_permission_without_if_owner_enabled(ptype)
-				and ptype != 'create'):
+			if (
+					pvalue
+					and has_if_owner_enabled
+					and not has_permission_without_if_owner_enabled(ptype)
+					and ptype != 'create'
+			):
 				perms['if_owner'][ptype] = cint(pvalue and is_owner)
 				# has no access if not owner
 				# only provide select or read access so that user is able to at-least access list
 				# (and the documents will be filtered based on owner sin further checks)
-				perms[ptype] = 1 if ptype in ['select', 'read'] else 0
+				perms[ptype] = 1 if ptype in ('select', 'read') else 0
 
 		frappe.local.role_permissions[cache_key] = perms
 

frappe/printing/doctype/network_printer_settings/network_printer_settings.json

@@ -41,10 +41,11 @@
  ],
  "index_web_pages_for_search": 1,
  "links": [],
- "modified": "2021-09-17 11:30:16.781655",
+ "modified": "2021-10-07 11:23:13.799402",
  "modified_by": "Administrator",
  "module": "Printing",
  "name": "Network Printer Settings",
+ "naming_rule": "Set by user",
  "owner": "Administrator",
  "permissions": [
   {
@@ -58,6 +59,15 @@
    "role": "System Manager",
    "share": 1,
    "write": 1
+  },
+  {
+   "email": 1,
+   "export": 1,
+   "print": 1,
+   "read": 1,
+   "report": 1,
+   "role": "All",
+   "share": 1
   }
  ],
  "sort_field": "modified",

frappe/printing/doctype/print_format/print_format.js

@@ -36,7 +36,7 @@ frappe.ui.form.on("Print Format", {
 			else if (frm.doc.custom_format && !frm.doc.raw_printing) {
 				frm.set_df_property("html", "reqd", 1);
 			}
-			if (frappe.perm.has_perm('DocType', 0, 'read', frm.doc.doc_type)) {
+			if (frappe.model.can_read(frm.doc.doc_type)) {
 				frappe.db.get_value('DocType', frm.doc.doc_type, 'default_print_format', (r) => {
 					if (r.default_print_format != frm.doc.name) {
 						frm.add_custom_button(__("Set as Default"), function () {

frappe/printing/page/print/print.js

@@ -171,13 +171,13 @@ frappe.ui.form.PrintView = class {
 			});
 		}
 
-		if (frappe.perm.has_perm('Print Format', 0, 'create')) {
+		if (frappe.model.can_create('Print Format')) {
 			this.page.add_menu_item(__('Customize'), () =>
 				this.edit_print_format()
 			);
 		}
 
-		if (this.print_settings.enable_print_server) {
+		if (cint(this.print_settings.enable_print_server)) {
 			this.page.add_menu_item(__('Select Network Printer'), () =>
 				this.network_printer_setting_dialog()
 			);

frappe/public/js/frappe/build_events/BuildSuccess.vue

@@ -3,8 +3,11 @@
 		v-if="is_shown"
 		class="flex justify-between build-success-message align-center"
 	>
-		<div class="mr-4">Compiled successfully</div>
-		<a class="text-white underline" href="/" @click.prevent="reload">
+		Compiled successfully
+		<a
+			v-if="!live_reload"
+			class="ml-4 text-white underline" href="/" @click.prevent="reload"
+		>
 			Refresh
 		</a>
 	</div>
@@ -14,11 +17,17 @@ export default {
 	name: "BuildSuccess",
 	data() {
 		return {
-			is_shown: false
+			is_shown: false,
+			live_reload: false,
 		};
 	},
 	methods: {
-		show() {
+		show(data) {
+			if (data.live_reload) {
+				this.live_reload = true;
+				this.reload();
+			}
+
 			this.is_shown = true;
 			if (this.timeout) {
 				clearTimeout(this.timeout);

frappe/public/js/frappe/build_events/build_events.bundle.js

@@ -13,10 +13,11 @@ frappe.realtime.on("build_event", data => {
 	}
 });
 
-function show_build_success() {
+function show_build_success(data) {
 	if (error) {
 		error.hide();
 	}
+
 	if (!success) {
 		let target = $('<div class="build-success-container">')
 			.appendTo($container)
@@ -27,7 +28,7 @@ function show_build_success() {
 		});
 		success = vm.$children[0];
 	}
-	success.show();
+	success.show(data);
 }
 
 function show_build_error(data) {

frappe/public/js/frappe/form/footer/base_timeline.js

@@ -97,9 +97,13 @@ class BaseTimeline {
 		}
 
 		timeline_item.append(`<div class="timeline-content ${item.is_card ? 'frappe-card' : ''}">`);
-		timeline_item.find('.timeline-content').append(item.content);
+		let timeline_content = timeline_item.find('.timeline-content');
+		timeline_content.append(item.content);
 		if (!item.hide_timestamp && !item.is_card) {
-			timeline_item.find('.timeline-content').append(`<span> - ${comment_when(item.creation)}</span>`);
+			timeline_content.append(`<span> - ${comment_when(item.creation)}</span>`);
+		}
+		if (item.id) {
+			timeline_content.attr("id", item.id);
 		}
 		return timeline_item;
 	}

frappe/public/js/frappe/form/footer/form_timeline.js

@@ -96,6 +96,7 @@ class FormTimeline extends BaseTimeline {
 	render_timeline_items() {
 		super.render_timeline_items();
 		this.set_document_info();
+		frappe.utils.bind_actions_with_object(this.timeline_items_wrapper, this);
 	}
 
 	set_document_info() {
@@ -179,6 +180,7 @@ class FormTimeline extends BaseTimeline {
 				is_card: true,
 				content: this.get_communication_timeline_content(communication),
 				doctype: "Communication",
+				id: `communication-${communication.name}`,
 				name: communication.name
 			});
 		});
@@ -246,6 +248,7 @@ class FormTimeline extends BaseTimeline {
 			creation: comment.creation,
 			is_card: true,
 			doctype: "Comment",
+			id: `comment-${comment.name}`,
 			name: comment.name,
 			content: this.get_comment_timeline_content(comment),
 		};
@@ -394,7 +397,7 @@ class FormTimeline extends BaseTimeline {
 	}
 
 	setup_reply(communication_box, communication_doc) {
-		let actions = communication_box.find('.actions');
+		let actions = communication_box.find('.custom-actions');
 		let reply = $(`<a class="action-btn reply">${frappe.utils.icon('reply', 'md')}</a>`).click(() => {
 			this.compose_mail(communication_doc);
 		});
@@ -446,14 +449,16 @@ class FormTimeline extends BaseTimeline {
 		let edit_wrapper = $(`<div class="comment-edit-box">`).hide();
 		let edit_box = this.make_editable(edit_wrapper);
 		let content_wrapper = comment_wrapper.find('.content');
-
-		let delete_button = $();
+		let more_actions_wrapper = comment_wrapper.find('.more-actions');
 		if (frappe.model.can_delete("Comment")) {
-			delete_button = $(`
-				<button class="btn btn-link action-btn">
-					${frappe.utils.icon('close', 'sm')}
-				</button>
+			const delete_option = $(`
+				<li>
+					<a class="dropdown-item">
+						${__("Delete")}
+					</a>
+				</li>
 			`).click(() => this.delete_comment(doc.name));
+			more_actions_wrapper.find('.dropdown-menu').append(delete_option);
 		}
 
 		let dismiss_button = $(`
@@ -493,15 +498,14 @@ class FormTimeline extends BaseTimeline {
 		edit_button.toggle_edit_mode = () => {
 			edit_button.edit_mode = !edit_button.edit_mode;
 			edit_button.text(edit_button.edit_mode ? __('Save') : __('Edit'));
-			delete_button.toggle(!edit_button.edit_mode);
+			more_actions_wrapper.toggle(!edit_button.edit_mode);
 			dismiss_button.toggle(edit_button.edit_mode);
 			edit_wrapper.toggle(edit_button.edit_mode);
 			content_wrapper.toggle(!edit_button.edit_mode);
 		};
-
-		comment_wrapper.find('.actions').append(edit_button);
-		comment_wrapper.find('.actions').append(dismiss_button);
-		comment_wrapper.find('.actions').append(delete_button);
+		let actions_wrapper = comment_wrapper.find('.custom-actions');
+		actions_wrapper.append(edit_button);
+		actions_wrapper.append(dismiss_button);
 	}
 
 	make_editable(container) {
@@ -559,6 +563,14 @@ class FormTimeline extends BaseTimeline {
 			});
 		});
 	}
+
+	copy_link(ev) {
+		let doc_link = frappe.urllib.get_full_url(
+			frappe.utils.get_form_link(this.frm.doctype, this.frm.docname)
+		);
+		let element_id = $(ev.currentTarget).closest(".timeline-content").attr("id");
+		frappe.utils.copy_to_clipboard(`${doc_link}#${element_id}`);
+	}
 }
 
 export default FormTimeline;

frappe/public/js/frappe/form/form.js

@@ -480,7 +480,11 @@ frappe.ui.form.Form = class FrappeForm {
 			this.layout.show_empty_form_message();
 		}
 
-		this.scroll_to_element();
+		frappe.after_ajax(() => {
+			$(document).ready(() => {
+				this.scroll_to_element();
+			});
+		});
 	}
 
 	set_first_tab_as_active() {
@@ -598,6 +602,8 @@ frappe.ui.form.Form = class FrappeForm {
 		this.validate_form_action(save_action, resolve);
 
 		var after_save = function(r) {
+			// to remove hash from URL to avoid scroll after save
+			history.replaceState(null, null, ' ');
 			if(!r.exc) {
 				if (["Save", "Update", "Amend"].indexOf(save_action)!==-1) {
 					frappe.utils.play_sound("click");
@@ -1195,6 +1201,8 @@ frappe.ui.form.Form = class FrappeForm {
 			if (selector.length) {
 				frappe.utils.scroll_to(selector);
 			}
+		} else if (window.location.hash && $(window.location.hash).length) {
+			frappe.utils.scroll_to(window.location.hash, true, 200, null, null, true);
 		}
 	}
 

frappe/public/js/frappe/form/grid.js

@@ -773,16 +773,18 @@ export default class Grid {
 	}
 
 	setup_user_defined_columns() {
-		let user_settings = frappe.get_user_settings(this.frm.doctype, 'GridView');
-		if (user_settings && user_settings[this.doctype] && user_settings[this.doctype].length) {
-			this.user_defined_columns = user_settings[this.doctype].map(row => {
-				let column = frappe.meta.get_docfield(this.doctype, row.fieldname);
-				if (column) {
-					column.in_list_view = 1;
-					column.columns = row.columns;
-					return column;
-				}
-			});
+		if (this.frm) {
+			let user_settings = frappe.get_user_settings(this.frm.doctype, 'GridView');
+			if (user_settings && user_settings[this.doctype] && user_settings[this.doctype].length) {
+				this.user_defined_columns = user_settings[this.doctype].map(row => {
+					let column = frappe.meta.get_docfield(this.doctype, row.fieldname);
+					if (column) {
+						column.in_list_view = 1;
+						column.columns = row.columns;
+						return column;
+					}
+				});
+			}
 		}
 	}
 

frappe/public/js/frappe/form/grid_row.js

@@ -497,7 +497,7 @@ export default class GridRow {
 	}
 
 	update_user_settings_for_grid() {
-		if (!this.selected_columns_for_grid) {
+		if (!this.selected_columns_for_grid || !this.frm) {
 			return;
 		}
 

frappe/public/js/frappe/form/multi_select_dialog.js

@@ -70,6 +70,7 @@ frappe.ui.form.MultiSelectDialog = class MultiSelectDialog {
 		this.dialog = new frappe.ui.Dialog({
 			title: title,
 			fields: this.fields,
+			size: this.size,
 			primary_action_label: this.primary_action_label || __("Get Items"),
 			secondary_action_label: __("Make {0}", [__(this.doctype)]),
 			primary_action: () => {
@@ -135,7 +136,7 @@ frappe.ui.form.MultiSelectDialog = class MultiSelectDialog {
 			this.get_child_result().then(r => {
 				this.child_results = r.message || [];
 				this.render_child_datatable();
-	
+
 				this.$wrapper.addClass('hidden');
 				this.$child_wrapper.removeClass('hidden');
 				this.dialog.fields_dict.more_btn.$wrapper.hide();

frappe/public/js/frappe/form/script_manager.js

@@ -193,7 +193,7 @@ frappe.ui.form.ScriptManager = class ScriptManager {
 
 		function setup_add_fetch(df) {
 			if ((['Data', 'Read Only', 'Text', 'Small Text', 'Currency', 'Check',
-				'Text Editor', 'Code', 'Link', 'Float', 'Int', 'Date', 'Select'].includes(df.fieldtype) || df.read_only==1)
+				'Text Editor', 'Code', 'Link', 'Float', 'Int', 'Date', 'Select', 'Duration'].includes(df.fieldtype) || df.read_only==1)
 				&& df.fetch_from && df.fetch_from.indexOf(".")!=-1) {
 				var parts = df.fetch_from.split(".");
 				me.frm.add_fetch(parts[0], parts[1], df.fieldname);

frappe/public/js/frappe/form/templates/timeline_message_box.html

@@ -63,6 +63,20 @@
 				</svg>
 			</a>
 			{% } %}
+			<div class="custom-actions"></div>
+			<div class="more-actions">
+				<a type="button" class="action-btn"
+					data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
+					<svg class="icon icon-sm">
+						<use xlink:href="#icon-dot-horizontal"></use>
+					</svg>
+				</a>
+				<ul class="dropdown-menu small">
+					<li>
+						<a class="dropdown-item" data-action="copy_link">{{ __('Copy Link') }}</a>
+					</li>
+				</ul>
+			</div>
 		</span>
 	</span>
 	<div class="content">

frappe/public/js/frappe/list/bulk_operations.js

@@ -78,7 +78,7 @@ export default class BulkOperations {
 			args: {
 				doctype: 'Letter Head',
 				fields: ['name', 'is_default'],
-				limit: 0
+				limit_page_length: 0
 			},
 			async: false,
 			callback (r) {

frappe/public/js/frappe/list/list_filter.js

@@ -26,6 +26,7 @@ export default class ListFilter {
 		this.$input_area = this.wrapper.find('.input-area');
 		this.$list_filters = this.wrapper.find('.list-filters');
 		this.$saved_filters = this.wrapper.find('.saved-filters').hide();
+		this.$saved_filters_preview = this.wrapper.find('.saved-filters-preview');
 		this.saved_filters_hidden = true;
 
 		this.filter_input = frappe.ui.form.make_control({
@@ -57,6 +58,7 @@ export default class ListFilter {
 
 	refresh() {
 		this.get_list_filters().then(() => {
+			this.filters.length ? this.$saved_filters_preview.show() : this.$saved_filters_preview.hide();
 			const html = this.filters.map((filter) => this.filter_template(filter));
 			this.wrapper.find('.filter-pill').remove();
 			this.$saved_filters.append(html);

frappe/public/js/frappe/list/list_settings.js

@@ -114,14 +114,14 @@ export default class ListSettings {
 
 					<div class="row">
 						<div class="col-md-1">
-							<i class="fa fa-bars text-muted sortable-handle ${show_sortable_handle}" aria-hidden="true"></i>
+							${frappe.utils.icon("drag", "xs", "", "", "sortable-handle " + show_sortable_handle)}
 						</div>
 						<div class="col-md-10" style="padding-left:0px;">
 							${me.fields[idx].label}
 						</div>
 						<div class="col-md-1 ${can_remove}">
 							<a class="text-muted remove-field" data-fieldname="${me.fields[idx].fieldname}">
-								<i class="fa fa-trash-o" aria-hidden="true"></i>
+								${frappe.utils.icon("delete", "xs")}
 							</a>
 						</div>
 					</div>

frappe/public/js/frappe/list/list_view.js

@@ -907,7 +907,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList {
 			return this.settings.get_form_link(doc);
 		}
 
-		const docname = doc.name.match(/[%'"\s]/)
+		const docname = doc.name.match(/[%'"#\s]/)
 			? encodeURIComponent(doc.name)
 			: doc.name;
 

frappe/public/js/frappe/router.js

@@ -60,6 +60,7 @@ $('body').on('click', 'a', function(e) {
 		// target has "/app, this is a v2 style route.
 		return override(e.currentTarget.pathname + e.currentTarget.hash);
 	}
+
 });
 
 frappe.router = {
@@ -263,7 +264,9 @@ frappe.router = {
 		return new Promise(resolve => {
 			route = this.get_route_from_arguments(route);
 			route = this.convert_from_standard_route(route);
-			const sub_path = this.make_url(route);
+			let sub_path = this.make_url(route);
+			// replace each # occurrences in the URL with encoded character except for last
+			// sub_path = sub_path.replace(/[#](?=.*[#])/g, "%23");
 			this.push_state(sub_path);
 
 			setTimeout(() => {
@@ -347,7 +350,7 @@ frappe.router = {
 				return null;
 			} else {
 				a = String(a);
-				if (a && a.match(/[%'"\s\t]/)) {
+				if (a && a.match(/[%'"#\s\t]/)) {
 					// if special chars, then encode
 					a = encodeURIComponent(a);
 				}
@@ -374,7 +377,7 @@ frappe.router = {
 		// return clean sub_path from hash or url
 		// supports both v1 and v2 routing
 		if (!route) {
-			route = window.location.pathname + window.location.hash + window.location.search;
+			route = window.location.pathname;
 			if (route.includes('app#')) {
 				// to support v1
 				route = window.location.hash;

frappe/public/js/frappe/ui/dialog.js

@@ -78,6 +78,8 @@ frappe.ui.Dialog = class Dialog extends frappe.ui.FieldGroup {
 		this.$wrapper
 			.on("hide.bs.modal", function() {
 				me.display = false;
+				me.is_minimized = false;
+				me.hide_scrollbar(false);
 
 				if(frappe.ui.open_dialogs[frappe.ui.open_dialogs.length-1]===me) {
 					frappe.ui.open_dialogs.pop();
@@ -96,6 +98,7 @@ frappe.ui.Dialog = class Dialog extends frappe.ui.FieldGroup {
 				window.cur_dialog = me;
 				frappe.ui.open_dialogs.push(me);
 				me.focus_on_first_input();
+				me.hide_scrollbar(true);
 				me.on_page_show && me.on_page_show();
 				$(document).trigger('frappe.ui.Dialog:shown');
 				$(document).off('focusin.modal');
@@ -233,7 +236,11 @@ frappe.ui.Dialog = class Dialog extends frappe.ui.FieldGroup {
 		this.get_minimize_btn().html(frappe.utils.icon(icon));
 		this.on_minimize_toggle && this.on_minimize_toggle(this.is_minimized);
 		this.header.find('.modal-title').toggleClass('cursor-pointer');
-		$("body").css("overflow", this.is_minimized ?  "auto" : "hidden");
+		this.hide_scrollbar(!this.is_minimized);
+	}
+
+	hide_scrollbar(bool) {
+		$("body").css("overflow", bool ?  "hidden" : "auto");
 	}
 
 	add_custom_action(label, action, css_class=null) {

frappe/public/js/frappe/utils/pretty_date.js

@@ -25,11 +25,11 @@ function prettyDate(date, mini) {
 			if (day_diff < 7) {
 				return __("{0} d", [day_diff]);
 			} else if (day_diff < 31) {
-				return __("{0} w", [Math.ceil(day_diff / 7)]);
+				return __("{0} w", [Math.floor(day_diff / 7)]);
 			} else if (day_diff < 365) {
-				return __("{0} M", [Math.ceil(day_diff / 30)]);
+				return __("{0} M", [Math.floor(day_diff / 30)]);
 			} else {
-				return __("{0} y", [Math.ceil(day_diff / 365)]);
+				return __("{0} y", [Math.floor(day_diff / 365)]);
 			}
 		}
 	} else {
@@ -54,15 +54,15 @@ function prettyDate(date, mini) {
 			} else if (day_diff < 14) {
 				return __("1 week ago");
 			} else if (day_diff < 31) {
-				return __("{0} weeks ago", [Math.ceil(day_diff / 7)]);
+				return __("{0} weeks ago", [Math.floor(day_diff / 7)]);
 			} else if (day_diff < 62) {
 				return __("1 month ago");
 			} else if (day_diff < 365) {
-				return __("{0} months ago", [Math.ceil(day_diff / 30)]);
+				return __("{0} months ago", [Math.floor(day_diff / 30)]);
 			} else if (day_diff < 730) {
 				return __("1 year ago");
 			} else {
-				return __("{0} years ago", [Math.ceil(day_diff / 365)]);
+				return __("{0} years ago", [Math.floor(day_diff / 365)]);
 			}
 		}
 	}

frappe/public/js/frappe/utils/utils.js

@@ -268,7 +268,8 @@ Object.assign(frappe.utils, {
 				</a></p>');
 		return content.html();
 	},
-	scroll_to: function(element, animate=true, additional_offset, element_to_be_scrolled, callback) {
+	scroll_to: function(element, animate=true, additional_offset,
+		element_to_be_scrolled, callback, highlight_element=false) {
 		if (frappe.flags.disable_auto_scroll) return;
 
 		element_to_be_scrolled = element_to_be_scrolled || $("html, body");
@@ -291,11 +292,20 @@ Object.assign(frappe.utils, {
 		}
 
 		if (animate) {
-			element_to_be_scrolled.animate({ scrollTop: scroll_top }).promise().then(callback);
+			element_to_be_scrolled.animate({
+				scrollTop: scroll_top
+			}).promise().then(() => {
+				if (highlight_element) {
+					$(element).addClass('highlight');
+					document.addEventListener("click", function() {
+						$(element).removeClass('highlight');
+					}, {once: true});
+				}
+				callback && callback();
+			});
 		} else {
 			element_to_be_scrolled.scrollTop(scroll_top);
 		}
-
 	},
 	get_scroll_position: function(element, additional_offset) {
 		let header_offset = $(".navbar").height() + $(".page-head:visible").height();
@@ -1039,18 +1049,20 @@ Object.assign(frappe.utils, {
 		return duration;
 	},
 
-	seconds_to_duration(value, duration_options) {
-		let secs = value;
-		let total_duration = {
-			days: Math.floor(secs / (3600 * 24)),
-			hours: Math.floor(secs % (3600 * 24) / 3600),
-			minutes: Math.floor(secs % 3600 / 60),
-			seconds: Math.floor(secs % 60)
+	seconds_to_duration(seconds, duration_options) {
+		const round = seconds > 0 ? Math.floor : Math.ceil;
+		const total_duration = {
+			days: round(seconds / 86400), // 60 * 60 * 24
+			hours: round(seconds % 86400 / 3600),
+			minutes: round(seconds % 3600 / 60),
+			seconds: round(seconds % 60)
 		};
+
 		if (duration_options.hide_days) {
-			total_duration.hours = Math.floor(secs / 3600);
+			total_duration.hours = round(seconds / 3600);
 			total_duration.days = 0;
 		}
+
 		return total_duration;
 	},
 
@@ -1123,7 +1135,7 @@ Object.assign(frappe.utils, {
 		}
 	},
 
-	icon(icon_name, size="sm", icon_class="", icon_style="") {
+	icon(icon_name, size="sm", icon_class="", icon_style="", svg_class="") {
 		let size_class = "";
 
 		if (typeof size == "object") {
@@ -1131,7 +1143,7 @@ Object.assign(frappe.utils, {
 		} else {
 			size_class = `icon-${size}`;
 		}
-		return `<svg class="icon ${size_class}" style="${icon_style}">
+		return `<svg class="icon ${svg_class} ${size_class}" style="${icon_style}">
 			<use class="${icon_class}" href="#icon-${icon_name}"></use>
 		</svg>`;
 	},

frappe/public/js/frappe/views/reports/query_report.js

@@ -107,7 +107,6 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 		}
 
 		if (this.report_name !== frappe.get_route()[1]) {
-			// this.toggle_loading(true);
 			// different report
 			this.load_report();
 		}
@@ -556,6 +555,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 	refresh() {
 		this.toggle_message(true);
 		this.toggle_report(false);
+		this.show_loading_screen();
 		let filters = this.get_filter_values(true);
 
 		// only one refresh at a time
@@ -645,6 +645,8 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 
 			this.show_footer_message();
 			frappe.hide_progress();
+		}).finally(() => {
+			this.hide_loading_screen();
 		});
 	}
 
@@ -869,6 +871,24 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 		}
 	}
 
+	show_loading_screen() {
+		const loading_state = `<div class="msg-box no-border">
+			<div>
+				<img src="/assets/frappe/images/ui-states/list-empty-state.svg" alt="Generic Empty State" class="null-state">
+			</div>
+			<p>${__('Loading')}...</p>
+		</div>`;
+
+		this.$loading.find('div').html(loading_state);
+		this.$report.hide();
+		this.$loading.show();
+	}
+
+	hide_loading_screen() {
+		this.$loading.hide();
+		this.$report.show();
+	}
+
 	get_chart_options(data) {
 		let options = this.report_settings.get_chart_data
 			? this.report_settings.get_chart_data(data.columns, data.result)
@@ -1679,6 +1699,8 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 			.hide().appendTo(this.page.main);
 
 		this.$chart = $('<div class="chart-wrapper">').hide().appendTo(this.page.main);
+
+		this.$loading = $(this.message_div('')).hide().appendTo(this.page.main);
 		this.$report = $('<div class="report-wrapper">').appendTo(this.page.main);
 		this.$message = $(this.message_div('')).hide().appendTo(this.page.main);
 	}
@@ -1738,11 +1760,6 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 		this.refresh();
 	}
 
-	toggle_loading(flag) {
-		this.toggle_message(flag, __('Loading') + '...');
-	}
-
-
 	toggle_nothing_to_show(flag) {
 		let message = this.prepared_report
 			? __('This is a background report. Please set the appropriate filters and then generate a new one.')

frappe/public/scss/common/css_variables.scss

@@ -209,6 +209,8 @@
 	--highlight-color: var(--gray-50);
 	--yellow-highlight-color: var(--yellow-50);
 
+	--highlight-shadow: 1px 1px 10px var(--blue-50), 0px 0px 4px var(--blue-600);
+
 	// Border Sizes
 	--border-radius-sm: 4px;
 	--border-radius: 6px;

frappe/public/scss/common/modal.scss

@@ -169,7 +169,7 @@ body.modal-open[style^="padding-right"] {
 			border-radius: var(--border-radius-md);
 			border-bottom-right-radius: 0;
 			border-bottom-left-radius: 0;
-			box-shadow: -10px 10px rgba(0, 0, 0, 0.100661);
+			box-shadow: var(--shadow-lg);
 		}
 
 		@include media-breakpoint-down(sm) {

frappe/public/scss/desk/dark.scss

@@ -75,6 +75,8 @@
 	--highlight-color: var(--gray-700);
 	--yellow-highlight-color: var(--yellow-700);
 
+	--highlight-shadow: 1px 1px 10px var(--blue-900), 0px 0px 4px var(--blue-500);
+
 	// input
 	--input-disabled-bg: none;
 

frappe/public/scss/desk/desktop.scss

@@ -164,12 +164,11 @@ body {
 
 			.drag-handle {
 				cursor: all-scroll;
-				cursor: -webkit-grabbing;
+				cursor: grabbing;
 
 				&:active {
+					cursor: all-scroll;
 					cursor: grabbing;
-					cursor: -moz-grabbing;
-					cursor: -webkit-grabbing;
 				}
 			}
 
@@ -813,7 +812,7 @@ body {
 
 			.drag-handle {
 				cursor: all-scroll;
-				cursor: -webkit-grabbing;
+				cursor: grabbing;
 				display: none;
 			}
 
@@ -966,7 +965,7 @@ body {
 
 							.drag-handle {
 								cursor: all-scroll;
-								cursor: -webkit-grabbing;
+								cursor: grabbing;
 							}
 						}
 					}

frappe/public/scss/desk/filters.scss

@@ -8,6 +8,7 @@
 	min-width: 500px;
 	min-height: 50px;
 	font-size: var(--text-md);
+	z-index: 1019;
 }
 
 .filter-area {

frappe/public/scss/desk/global.scss

@@ -561,6 +561,19 @@ details > summary:focus {
 	display: none;
 }
 
+.highlight {
+	transition: 0.5s ease background-color;
+	box-shadow: var(--highlight-shadow) !important;
+}
+
+.dropdown-menu.small {
+	font-size: var(--text-sm);
+	min-width: 140px;
+	.dropdown-item {
+		padding: var(--padding-xs);
+	}
+}
+
 // REDESIGN TODO: Handling of broken images?
 // img.no-image:before {
 // 	.img-background();

frappe/public/scss/desk/list.scss

@@ -228,6 +228,11 @@ input.list-check-all, input.list-row-checkbox {
 		z-index: 500;
 		top: 0;
 	}
+
+	.sortable-handle {
+		cursor: all-scroll;
+		cursor: grabbing;
+	}
 }
 
 .list-items {

frappe/public/scss/desk/timeline.scss

@@ -117,7 +117,7 @@ $threshold: 34;
 
 			.actions {
 				display: flex;
-				> * {
+				> *:not(.indicator-pill) {
 					color: var(--text-muted);
 				}
 			}

frappe/public/scss/website/index.scss

@@ -192,8 +192,8 @@ h5.modal-title {
 }
 
 .hidden-xs {
-	@extend .d-none;
-	@extend .d-sm-block;
+	@extend .d-block;
+	@extend .d-sm-none;
 }
 
 .visible-xs {
@@ -216,6 +216,10 @@ h5.modal-title {
 	float: right;
 }
 
+.pull-left {
+	float: left;
+}
+
 .image-with-blur {
 	transition: filter 300ms ease-in-out;
 	filter: blur(1.5rem);

frappe/query_builder/__init__.py

@@ -1,2 +1,2 @@
 from pypika import *
-from frappe.query_builder.utils import Column, get_query_builder, patch_query_execute
+from frappe.query_builder.utils import Column, DocType, get_query_builder, patch_query_execute

frappe/query_builder/builder.py

@@ -2,6 +2,7 @@ from pypika import MySQLQuery, Order, PostgreSQLQuery, terms
 from pypika.queries import Schema, Table
 from frappe.utils import get_table_name
 from pypika.terms import Function
+
 class Base:
 	terms = terms
 	desc = Order.desc
@@ -27,6 +28,17 @@ class MariaDB(Base, MySQLQuery):
 			table = cls.DocType(table)
 		return super().from_(table, *args, **kwargs)
 
+	@classmethod
+	def into(cls, table, *args, **kwargs):
+		if isinstance(table, str):
+			table = cls.DocType(table)
+		return super().into(table, *args, **kwargs)
+
+	@classmethod
+	def update(cls, table, *args, **kwargs):
+		if isinstance(table, str):
+			table = cls.DocType(table)
+		return super().update(table, *args, **kwargs)
 
 class Postgres(Base, PostgreSQLQuery):
 	field_translation = {"table_name": "relname", "table_rows": "n_tup_ins"}
@@ -57,3 +69,15 @@ class Postgres(Base, PostgreSQLQuery):
 			table = cls.DocType(table)
 
 		return super().from_(table, *args, **kwargs)
+
+	@classmethod
+	def into(cls, table, *args, **kwargs):
+		if isinstance(table, str):
+			table = cls.DocType(table)
+		return super().into(table, *args, **kwargs)
+
+	@classmethod
+	def update(cls, table, *args, **kwargs):
+		if isinstance(table, str):
+			table = cls.DocType(table)
+		return super().update(table, *args, **kwargs)

frappe/query_builder/utils.py

@@ -44,14 +44,20 @@ def get_attr(method_string):
 	methodname = method_string.split('.')[-1]
 	return getattr(import_module(modulename), methodname)
 
+def DocType(*args, **kwargs):
+	return frappe.qb.DocType(*args, **kwargs)
+
 def patch_query_execute():
 	"""Patch the Query Builder with helper execute method
 	This excludes the use of `frappe.db.sql` method while
 	executing the query object
 	"""
 
-	def execute_query(query, **kwargs):
-		return frappe.db.sql(query, **kwargs)
+	def execute_query(query, *args, **kwargs):
+		query = str(query)
+		if frappe.flags.in_safe_exec and not query.lower().strip().startswith("select"):
+			raise frappe.PermissionError('Only SELECT SQL allowed in scripting')
+		return frappe.db.sql(query, *args, **kwargs)
 
 	query_class = get_attr(str(frappe.qb).split("'")[1])
 	builder_class = get_type_hints(query_class._builder).get('return')

frappe/sessions.py

@@ -17,8 +17,8 @@ import redis
 from urllib.parse import unquote
 from frappe.cache_manager import clear_user_cache
 
-@frappe.whitelist(allow_guest=True)
-def clear(user=None):
+@frappe.whitelist()
+def clear():
 	frappe.local.session_obj.update(force=True)
 	frappe.local.db.commit()
 	clear_user_cache(frappe.session.user)

frappe/templates/discussions/comment_box.html

@@ -1,32 +1,35 @@
 <form class="discussion-form">
 
-		<div class="form-group">
-			<div class="control-input-wrapper">
-				<div class="control-input">
-					<input type="text" autocomplete="off" class="input-with-feedback form-control topic-title" data-fieldtype="Data"
-						data-fieldname="feedback_comments" placeholder="{{ _('Type title') }}" spellcheck="false"></input>
-				</div>
+	{% if not single_thread %}
+	<div class="form-group">
+		<div class="control-input-wrapper">
+			<div class="control-input">
+				<input type="text" autocomplete="off" class="input-with-feedback form-control topic-title"
+					data-fieldtype="Data" data-fieldname="feedback_comments" placeholder="{{ _('Type title') }}"
+					spellcheck="false"></input>
 			</div>
 		</div>
+	</div>
+	{% endif %}
 
-		<div class="form-group">
-			<div class="control-input-wrapper">
-				<div class="control-input">
-					<textarea type="text" autocomplete="off" class="input-with-feedback form-control comment-field"
-						data-fieldtype="Text" data-fieldname="feedback_comments" placeholder="{{ _('Type here. Use markdown to format.') }}"
-						spellcheck="false"></textarea>
-				</div>
+	<div class="form-group">
+		<div class="control-input-wrapper">
+			<div class="control-input">
+				<textarea type="text" autocomplete="off" class="input-with-feedback form-control comment-field"
+					data-fieldtype="Text" data-fieldname="feedback_comments"
+					placeholder="{{ _('Type here. Use markdown to format.') }}" spellcheck="false"></textarea>
 			</div>
 		</div>
+	</div>
 
-		<div class="comment-footer">
-			<div class="small flex-grow-1">
-				{{ _("Press Cmd+Enter to post your comment") }}
-			</div>
+	<div class="comment-footer">
+		<div class="small flex-grow-1">
+			{{ _("Press Cmd+Enter to post your comment") }}
+		</div>
 
-			<a class="dark-links cancel-comment hide"> {{ _("Cancel") }} </a>
-			<div class="button is-default submit-discussion pull-right mb-1" data-doctype="{{ doctype | urlencode }}"
-				data-docname="{{ docname | urlencode }}">
-				{{ _("Post") }} </div>
+		<a class="dark-links cancel-comment hide"> {{ _("Cancel") }} </a>
+		<div class="button is-default submit-discussion pull-right mb-1">
+			{{ _("Post") }}
 		</div>
-	</form>
+	</div>
+</form>

frappe/templates/discussions/discussions.js

@@ -87,29 +87,43 @@ const setup_socket_io = () => {
 };
 
 const publish_message = (data) => {
-
-	if ($(`.discussion-on-page[data-topic=${data.topic_info.name}]`).length) {
+	const doctype = decodeURIComponent($(".discussions-parent").attr("data-doctype"));
+	const docname = decodeURIComponent($(".discussions-parent").attr("data-docname"));
+	const topic = data.topic_info;
+	const single_thread = $(".is-single-thread").length;
+	const first_topic = !$(".reply-card").length;
+	const document_match_found = doctype == topic.reference_doctype && docname == topic.reference_docname;
+
+	if ($(`.discussion-on-page[data-topic=${topic.name}]`).length) {
 		post_message_cleanup();
-		$('<div class="card-divider-dark mb-8"></div>' + data.template).insertBefore(`.discussion-on-page[data-topic=${data.topic_info.name}] .discussion-form`);
-	} else if ((decodeURIComponent($(".discussions-parent .discussions-card").attr("data-doctype")) == data.topic_info.reference_doctype
-		&& decodeURIComponent($(".discussions-parent .discussions-card").attr("data-docname")) == data.topic_info.reference_docname)) {
+		data.template = style_avatar_frame(data.template);
+		$('<div class="card-divider-dark mb-8"></div>' + data.template)
+			.insertBefore(`.discussion-on-page[data-topic=${topic.name}] .discussion-form`);
 
+	} else if (!first_topic && !single_thread && document_match_found) {
 		post_message_cleanup();
 		data.new_topic_template = style_avatar_frame(data.new_topic_template);
 
 		$(data.sidebar).insertAfter(`.discussions-sidebar .form-group`);
 		$(`#discussion-group`).prepend(data.new_topic_template);
 
-		if (data.topic_info.owner == frappe.session.user) {
-			$(".discussion-on-page").collapse();
+		if (topic.owner == frappe.session.user) {
+			$(".discussion-on-page") && $(".discussion-on-page").collapse();
 			$(".sidebar-topic").first().click();
 		}
-	} else if (data.topic_info.owner == frappe.session.user) {
+
+	} else if (single_thread && document_match_found) {
+		post_message_cleanup();
+		data.template = style_avatar_frame(data.template);
+		$(data.template).insertBefore(`.discussion-form`);
+		$(".discussion-on-page").attr("data-topic", topic.name);
+
+	} else if (topic.owner == frappe.session.user && document_match_found) {
 		post_message_cleanup();
 		window.location.reload();
 	}
 
-	update_reply_count(data.topic_info.name);
+	update_reply_count(topic.name);
 };
 
 const post_message_cleanup = () => {
@@ -191,10 +205,10 @@ const submit_discussion = (e) => {
 	const reply = $(".comment-field:visible").val().trim();
 
 	if (reply) {
-		let doctype = $(e.currentTarget).attr("data-doctype");
+		let doctype = $(e.currentTarget).closest(".discussions-parent").attr("data-doctype");
 		doctype = doctype ? decodeURIComponent(doctype) : doctype;
 
-		let docname = $(e.currentTarget).attr("data-docname");
+		let docname = $(e.currentTarget).closest(".discussions-parent").attr("data-docname");
 		docname = docname ? decodeURIComponent(docname) : docname;
 
 		frappe.call({
@@ -232,7 +246,8 @@ const get_color_from_palette = (element) => {
 
 const style_avatar_frame = (template) => {
 	const $template = $(template);
-	$template.find(".avatar-frame").css(get_color_from_palette($template.find(".avatar-frame")));
+	$template.find(".avatar-frame").length
+		&& $template.find(".avatar-frame").css(get_color_from_palette($template.find(".avatar-frame")));
 	return $template.prop("outerHTML");
 };
 

frappe/templates/discussions/discussions_section.html

@@ -1,22 +1,26 @@
 {% set topics = frappe.get_all("Discussion Topic",
 {"reference_doctype": doctype, "reference_docname": docname}, ["name", "title", "owner", "creation"]) %}
 
-{% include "frappe/templates/discussions/topic_modal.html" %}
 
-<div class="discussions-parent">
+<div class="discussions-parent {% if single_thread %} is-single-thread {% endif %}"
+	data-doctype="{{ doctype | urlencode }}" data-docname="{{ docname | urlencode }}">
+
+	{% include "frappe/templates/discussions/topic_modal.html" %}
+
 	<div class="discussions-header">
 		<span class="course-home-headings">{{ _(title) }}</span>
-		{% if topics %}
+		{% if topics and not single_thread %}
 		{% include "frappe/templates/discussions/button.html" %}
 		{% endif %}
 	</div>
 
-	{% if topics %}
-	<div class="common-card-style thread-card discussions-card" data-doctype="{{ doctype }}"
-		data-docname="{{ docname }}">
+	<div class="common-card-style thread-card {% if topics | length and not single_thread %} discussions-card {% endif %} ">
+		{% if topics and not single_thread %}
+
 
 		<div class="discussions-sidebar">
 			{% include "frappe/templates/discussions/search.html" %}
+
 			{% for topic in topics %}
 			{% set replies = frappe.get_all("Discussion Reply", {"topic": topic.name})%}
 			{% include "frappe/templates/discussions/sidebar.html" %}
@@ -24,13 +28,17 @@
 		</div>
 
 		<div class="mr-2" id="discussion-group">
+			{% for topic in topics %}
 			{% include "frappe/templates/discussions/reply_section.html" %}
+			{% endfor %}
 		</div>
-	</div>
-	{% else %}
 
-	<div id="no-discussions" class="common-card-style thread-card">
-		<div class="no-discussions">
+		{% elif single_thread %}
+		{% set topic = topics[0] if topics | length else None %}
+		{% include "frappe/templates/discussions/reply_section.html" %}
+
+		{% else %}
+		<div class="no-discussions" id="no-discussions">
 			<div class="font-weight-bold">No {{ title }}</div>
 			<div class="small mt-3 mb-3">There are no {{ title | lower }} for this {{ doctype | lower }}, why don't you start
 				one! </div>

frappe/templates/discussions/reply_card.html

@@ -2,10 +2,10 @@
 <div class="reply-card">
 	{% set member = frappe.db.get_value("User", reply.owner, ["name", "full_name", "username"], as_dict=True) %}
 	<div class="d-flex align-items-center small mb-2">
-		{% if loop.index == 1 %}
+		{% if loop.index == 1 or single_thread %}
 		{{ avatar(reply.owner) }}
 		{% endif %}
-		<a class="button-links {% if loop.index == 1 %} ml-2 {% endif %}" {% if get_profile_url %} href="{{ get_profile_url(member.username) }}" {% endif %}>
+		<a class="button-links {% if loop.index == 1 or single_thread %} ml-2 {% endif %}" {% if get_profile_url %} href="{{ get_profile_url(member.username) }}" {% endif %}>
 			{{ member.full_name }}
 		</a>
 		<div class="ml-3 frappe-timestamp" data-timestamp="{{ reply.creation }}"> {{ frappe.utils.pretty_date(reply.creation) }} </div>

frappe/templates/discussions/reply_section.html

@@ -1,16 +1,23 @@
-{% for topic in topics %}
+{% if topic %}
+
 {% set replies = frappe.get_all("Discussion Reply", {"topic": topic.name},
 ["reply", "owner", "creation"], order_by="creation")%}
 
-{% if replies %}
-<div class="collapse discussion-on-page" id="t{{ topic.name }}" data-topic="{{ topic.name }}"
-	data-parent="#discussion-group">
+{% endif %}
+
+<div class="collapse discussion-on-page" data-parent="#discussion-group"
+	{% if topic %} id="t{{ topic.name }}" data-topic="{{ topic.name }}" {% endif %}>
 
+	{% if not single_thread %}
 	<div class="button is-default back">
 		{{ _("Back") }}
 	</div>
+	{% endif %}
 
+	{% if topic and topic.title %}
 	<div class="course-home-headings p-0">{{ topic.title }}</div>
+	{% endif %}
+
 	{% for reply in replies %}
 	{% include "frappe/templates/discussions/reply_card.html" %}
 
@@ -34,5 +41,3 @@
 	{% endif %}
 
 </div>
-{% endif %}
-{% endfor %}

frappe/templates/includes/feedback/feedback.py

@@ -3,27 +3,18 @@
 from __future__ import unicode_literals
 
 import frappe
-from frappe.utils import add_to_date, now
 
 from frappe import _
+from frappe.rate_limiter import rate_limit
+from frappe.website.doctype.blog_settings.blog_settings import get_feedback_limit
 
 @frappe.whitelist(allow_guest=True)
+@rate_limit(key='reference_name', limit=get_feedback_limit, seconds=60*60)
 def add_feedback(reference_doctype, reference_name, rating, feedback):
 	doc = frappe.get_doc(reference_doctype, reference_name)
 	if doc.disable_feedback == 1:
 		return
 
-	feedback_count = frappe.db.count("Feedback", {
-		"reference_doctype": reference_doctype,
-		"reference_name": reference_name,
-		"ip_address": frappe.local.request_ip,
-		"creation": (">", add_to_date(now(), hours=-1))
-	})
-
-	if feedback_count > 20:
-		frappe.msgprint(_('Hourly feedback limit reached'))
-		return
-
 	doc = frappe.new_doc('Feedback')
 	doc.reference_doctype = reference_doctype
 	doc.reference_name = reference_name

frappe/templates/styles/discussion_style.css

@@ -3,7 +3,7 @@
 	padding: 1rem;
 }
 
-.discussions-parent .form-control {
+.thread-card .form-control {
 	background-color: #FFFFFF;
 	font-size: inherit;
 	color: inherit;
@@ -246,6 +246,6 @@
 }
 
 .card-divider-dark {
-	border: 1px solid var(--gray-400);
-	margin-bottom: 16px;
+	border: 1px solid var(--gray-300);
+	margin-bottom: 1rem;
 }

frappe/tests/test_commands.py

@@ -13,7 +13,7 @@ import glob
 # imports - module imports
 import frappe
 import frappe.recorder
-from frappe.installer import add_to_installed_apps
+from frappe.installer import add_to_installed_apps, remove_app
 from frappe.utils import add_to_date, get_bench_relative_path, now
 from frappe.utils.backups import fetch_latest_backups
 
@@ -465,3 +465,50 @@ class TestCommands(BaseTestCommands):
 		self.execute("bench --site {site} set-admin-password test2")
 		self.assertEqual(self.returncode, 0)
 		self.assertEqual(check_password('Administrator', 'test2'), 'Administrator')
+
+
+class RemoveAppUnitTests(unittest.TestCase):
+	def test_delete_modules(self):
+		from frappe.installer import (
+				_delete_doctypes,
+				_delete_modules,
+				_get_module_linked_doctype_field_map,
+		)
+
+		test_module = frappe.new_doc("Module Def")
+
+		test_module.update({"module_name": "RemoveThis", "app_name": "frappe"})
+		test_module.save()
+
+		module_def_linked_doctype = frappe.get_doc({
+			"doctype": "DocType",
+			"name": "Doctype linked with module def",
+			"module": "RemoveThis",
+			"custom": 1,
+			"fields": [{
+				"label": "Modulen't",
+				"fieldname": "notmodule",
+				"fieldtype": "Link",
+				"options": "Module Def"
+			}]
+		}).insert()
+
+		doctype_to_link_field_map = _get_module_linked_doctype_field_map()
+
+		self.assertIn("Report", doctype_to_link_field_map)
+		self.assertIn(module_def_linked_doctype.name, doctype_to_link_field_map)
+		self.assertEqual(doctype_to_link_field_map[module_def_linked_doctype.name], "notmodule")
+		self.assertNotIn("DocType", doctype_to_link_field_map)
+
+		doctypes_to_delete = _delete_modules([test_module.module_name], dry_run=False)
+		self.assertEqual(len(doctypes_to_delete), 1)
+
+		_delete_doctypes(doctypes_to_delete, dry_run=False)
+		self.assertFalse(frappe.db.exists("Module Def", test_module.module_name))
+		self.assertFalse(frappe.db.exists("DocType", module_def_linked_doctype.name))
+
+	def test_dry_run(self):
+		"""Check if dry run in not destructive."""
+
+		# nothing to assert, if this fails rest of the test suite will crumble.
+		remove_app("frappe", dry_run=True, yes=True, no_backup=True)

frappe/tests/test_db_query.py

@@ -446,6 +446,25 @@ class TestReportview(unittest.TestCase):
 		user.remove_roles("Blogger", "Website Manager")
 		user.add_roles(*user_roles)
 
+	def test_reportview_get_aggregation(self):
+		# test aggregation based on child table field
+		frappe.local.form_dict = frappe._dict({
+			"doctype": "DocType",
+			"fields": """["`tabDocField`.`label` as field_label","`tabDocField`.`name` as field_name"]""",
+			"filters": "[]",
+			"order_by": "_aggregate_column desc",
+			"start": 0,
+			"page_length": 20,
+			"view": "Report",
+			"with_comment_count": 0,
+			"group_by": "field_label, field_name",
+			"aggregate_on_field": "columns",
+			"aggregate_on_doctype": "DocField",
+			"aggregate_function": "sum"
+		})
+
+		response = execute_cmd("frappe.desk.reportview.get")
+		self.assertListEqual(response["keys"], ["field_label", "field_name", "_aggregate_column", 'columns'])
 
 def add_child_table_to_blog_post():
 	child_table = frappe.get_doc({

frappe/tests/test_permissions.py

@@ -493,6 +493,34 @@ class TestPermissions(unittest.TestCase):
 		frappe.set_user("test2@example.com")
 		self.assertRaises(frappe.PermissionError, getdoc, 'Blog Post', doc.name)
 
+	def test_if_owner_permission_on_get_list(self):
+		doc = frappe.get_doc({
+			"doctype": "Blog Post",
+			"blog_category": "-test-blog-category",
+			"blogger": "_Test Blogger 1",
+			"title": "_Test If Owner Permissions on Get List",
+			"content": "_Test Blog Post Content"
+		})
+
+		doc.insert(ignore_if_duplicate=True)
+
+		update('Blog Post', 'Blogger', 0, 'if_owner', 1)
+		update('Blog Post', 'Blogger', 0, 'read', 1)
+		user = frappe.get_doc("User", "test2@example.com")
+		user.add_roles("Website Manager")
+		frappe.clear_cache(doctype="Blog Post")
+
+		frappe.set_user("test2@example.com")
+		self.assertIn(doc.name, frappe.get_list("Blog Post", pluck="name"))
+
+		# Become system manager to remove role
+		frappe.set_user("test1@example.com")
+		user.remove_roles("Website Manager")
+		frappe.clear_cache(doctype="Blog Post")
+
+		frappe.set_user("test2@example.com")
+		self.assertNotIn(doc.name, frappe.get_list("Blog Post", pluck="name"))
+
 	def test_if_owner_permission_on_delete(self):
 		update('Blog Post', 'Blogger', 0, 'if_owner', 1)
 		update('Blog Post', 'Blogger', 0, 'read', 1)

frappe/tests/test_safe_exec.py

@@ -23,4 +23,12 @@ class TestSafeExec(unittest.TestCase):
 		safe_exec('''out = frappe.db.sql("select name from tabDocType where name='DocType'")''', None, _locals)
 		self.assertEqual(_locals['out'][0][0], 'DocType')
 
-		self.assertRaises(frappe.PermissionError, safe_exec, 'frappe.db.sql("update tabToDo set description=NULL")')
+		self.assertRaises(frappe.PermissionError, safe_exec, 'frappe.db.sql("update tabToDo set description=NULL")')
+
+	def test_query_builder(self):
+		_locals = dict(out=None)
+		safe_exec(script='''out = frappe.qb.from_("User").select(frappe.qb.terms.PseudoColumn("Max(name)")).run()''', _globals=None, _locals=_locals)
+		self.assertEqual(frappe.db.sql("SELECT Max(name) FROM tabUser"), _locals["out"])
+
+	def test_safe_query_builder(self):
+		self.assertRaises(frappe.PermissionError, safe_exec, '''frappe.qb.from_("User").delete().run()''')

frappe/tests/test_website.py

@@ -4,6 +4,7 @@ import frappe
 from frappe.utils import set_request
 from frappe.website.serve import get_response, get_response_content
 from frappe.website.utils import (build_response, clear_website_cache, get_home_page)
+from tenacity import retry, stop_after_attempt, retry_if_exception_type
 
 
 class TestWebsite(unittest.TestCase):
@@ -196,6 +197,11 @@ class TestWebsite(unittest.TestCase):
 		delattr(frappe.hooks, 'page_renderer')
 		frappe.cache().delete_key('app_hooks')
 
+	# TODO: Get rid of this retry logic
+	# Added since test is flaky and we can't figure out why at this point
+	@retry(
+		stop=stop_after_attempt(5), retry=retry_if_exception_type(AssertionError),
+	)
 	def test_printview_page(self):
 		content = get_response_content('/Language/en')
 		self.assertIn('<div class="print-format">', content)