Merge branch 'develop' into export-all-rows-fix

.editorconfig

@@ -9,6 +9,6 @@ trim_trailing_whitespace = true
 charset = utf-8
 
 # python, js indentation settings
-[{*.py,*.js}]
+[{*.py,*.js,*.vue}]
 indent_style = tab
 indent_size = 4

.github/helper/documentation.py

@@ -32,9 +32,9 @@ if __name__ == "__main__":
 
 	if response.ok:
 		payload = response.json()
-		title = payload.get("title", "").lower()
-		head_sha = payload.get("head", {}).get("sha")
-		body = payload.get("body", "").lower()
+		title = (payload.get("title") or "").lower()
+		head_sha = (payload.get("head") or {}).get("sha")
+		body = (payload.get("body") or "").lower()
 
 		if title.startswith("feat") and head_sha and "no-docs" not in body:
 			if docs_link_exists(body):

.github/helper/install_dependencies.sh

@@ -2,11 +2,6 @@
 
 set -e
 
-# python "${GITHUB_WORKSPACE}/.github/helper/roulette.py"
-# if [[ $? != 2 ]];then
-#   exit;
-# fi
-
  # install wkhtmltopdf
 wget -O /tmp/wkhtmltox.tar.xz https://github.com/frappe/wkhtmltopdf/raw/master/wkhtmltox-0.12.3_linux-generic-amd64.tar.xz
 tar -xf /tmp/wkhtmltox.tar.xz -C /tmp

.github/helper/roulette.py

@@ -1,56 +1,68 @@
-# if the script ends with exit code 0, then no tests are run further, else all tests are run
+import json
 import os
 import re
 import shlex
 import subprocess
 import sys
+import urllib.request
 
 
+def get_files_list(pr_number, repo="frappe/frappe"):
+	req = urllib.request.Request(f"https://api.github.com/repos/{repo}/pulls/{pr_number}/files")
+	res = urllib.request.urlopen(req)
+	dump = json.loads(res.read().decode('utf8'))
+	return [change["filename"] for change in dump]
+
 def get_output(command, shell=True):
-    print(command)
-    command = shlex.split(command)
-    return subprocess.check_output(command, shell=shell, encoding="utf8").strip()
+	print(command)
+	command = shlex.split(command)
+	return subprocess.check_output(command, shell=shell, encoding="utf8").strip()
 
 def is_py(file):
-    return file.endswith("py")
+	return file.endswith("py")
 
-def is_js(file):
-    return file.endswith("js")
+def is_ci(file):
+	return ".github" in file
+
+def is_frontend_code(file):
+	return file.endswith((".css", ".scss", ".less", ".sass", ".styl", ".js", ".ts"))
 
 def is_docs(file):
-    regex = re.compile(r'\.(md|png|jpg|jpeg)$|^.github|LICENSE')
-    return bool(regex.search(file))
+	regex = re.compile(r'\.(md|png|jpg|jpeg|csv)$|^.github|LICENSE')
+	return bool(regex.search(file))
 
 
 if __name__ == "__main__":
-    build_type = os.environ.get("TYPE")
-    before = os.environ.get("BEFORE")
-    after = os.environ.get("AFTER")
-    commit_range = before + '...' + after
-    print("Build Type: {}".format(build_type))
-    print("Commit Range: {}".format(commit_range))
+	files_list = sys.argv[1:]
+	build_type = os.environ.get("TYPE")
+	pr_number = os.environ.get("PR_NUMBER")
+	repo = os.environ.get("REPO_NAME")
 
-    try:
-        files_changed = get_output("git diff --name-only {}".format(commit_range), shell=False)
-    except Exception:
-        sys.exit(2)
+	if not files_list and pr_number:
+		files_list = get_files_list(pr_number=pr_number, repo=repo)
 
-    if "fatal" not in files_changed:
-        files_list = files_changed.split()
-        only_docs_changed = len(list(filter(is_docs, files_list))) == len(files_list)
-        only_js_changed = len(list(filter(is_js, files_list))) == len(files_list)
-        only_py_changed = len(list(filter(is_py, files_list))) == len(files_list)
+	if not files_list:
+		print("No files' changes detected. Build is shutting")
+		sys.exit(0)
 
-        if only_docs_changed:
-            print("Only docs were updated, stopping build process.")
-            sys.exit(0)
+	ci_files_changed = any(f for f in files_list if is_ci(f))
+	only_docs_changed = len(list(filter(is_docs, files_list))) == len(files_list)
+	only_frontend_code_changed = len(list(filter(is_frontend_code, files_list))) == len(files_list)
+	only_py_changed = len(list(filter(is_py, files_list))) == len(files_list)
 
-        if only_js_changed and build_type == "server":
-            print("Only JavaScript code was updated; Stopping Python build process.")
-            sys.exit(0)
+	if ci_files_changed:
+		print("CI related files were updated, running all build processes.")
 
-        if only_py_changed and build_type == "ui":
-            print("Only Python code was updated, stopping Cypress build process.")
-            sys.exit(0)
+	elif only_docs_changed:
+		print("Only docs were updated, stopping build process.")
+		sys.exit(0)
 
-    sys.exit(2)
+	elif only_frontend_code_changed and build_type == "server":
+		print("Only Frontend code was updated; Stopping Python build process.")
+		sys.exit(0)
+
+	elif only_py_changed and build_type == "ui":
+		print("Only Python code was updated, stopping Cypress build process.")
+		sys.exit(0)
+
+	os.system('echo "::set-output name=build::strawberry"')

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

@@ -26,10 +26,21 @@ jobs:
         with:
           python-version: 3.7
 
+      - name: Check if build should be run
+        id: check-build
+        run: |
+          python "${GITHUB_WORKSPACE}/.github/helper/roulette.py"
+        env:
+          TYPE: "server"
+          PR_NUMBER: ${{ github.event.number }}
+          REPO_NAME: ${{ github.repository }}
+
       - name: Add to Hosts
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
 
       - name: Cache pip
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         uses: actions/cache@v2
         with:
           path: ~/.cache/pip
@@ -39,6 +50,7 @@ jobs:
             ${{ runner.os }}-
 
       - name: Cache node modules
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         uses: actions/cache@v2
         env:
           cache-name: cache-node-modules
@@ -51,10 +63,12 @@ jobs:
             ${{ runner.os }}-
 
       - name: Get yarn cache directory path
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         id: yarn-cache-dir-path
         run: echo "::set-output name=dir::$(yarn cache dir)"
 
       - uses: actions/cache@v2
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         id: yarn-cache
         with:
           path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
@@ -63,6 +77,7 @@ jobs:
             ${{ runner.os }}-yarn-
 
       - name: Install Dependencies
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh
         env:
           BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
@@ -70,12 +85,14 @@ jobs:
           TYPE: server
 
       - name: Install
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh
         env:
           DB: mariadb
           TYPE: server
 
       - name: Run Patch Tests
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: |
           cd ~/frappe-bench/
           wget https://frappeframework.com/files/v10-frappe.sql.gz

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

@@ -35,17 +35,29 @@ jobs:
         with:
           python-version: 3.7
 
+      - name: Check if build should be run
+        id: check-build
+        run: |
+          python "${GITHUB_WORKSPACE}/.github/helper/roulette.py"
+        env:
+          TYPE: "server"
+          PR_NUMBER: ${{ github.event.number }}
+          REPO_NAME: ${{ github.repository }}
+
       - uses: actions/setup-node@v2
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         with:
           node-version: 14
           check-latest: true
 
       - name: Add to Hosts
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: |
           echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
           echo "127.0.0.1 test_site_producer" | sudo tee -a /etc/hosts
 
       - name: Cache pip
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         uses: actions/cache@v2
         with:
           path: ~/.cache/pip
@@ -55,6 +67,7 @@ jobs:
             ${{ runner.os }}-
 
       - name: Cache node modules
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         uses: actions/cache@v2
         env:
           cache-name: cache-node-modules
@@ -67,10 +80,12 @@ jobs:
             ${{ runner.os }}-
 
       - name: Get yarn cache directory path
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         id: yarn-cache-dir-path
         run: echo "::set-output name=dir::$(yarn cache dir)"
 
       - uses: actions/cache@v2
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         id: yarn-cache
         with:
           path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
@@ -79,6 +94,7 @@ jobs:
             ${{ runner.os }}-yarn-
 
       - name: Install Dependencies
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh
         env:
           BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
@@ -86,18 +102,22 @@ jobs:
           TYPE: server
 
       - name: Install
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh
         env:
           DB: mariadb
           TYPE: server
 
       - name: Run Tests
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator --with-coverage
         env:
           CI_BUILD_ID: ${{ github.run_id }}
           ORCHESTRATOR_URL: http://test-orchestrator.frappe.io
 
       - name: Upload Coverage Data
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
+        id: upload-coverage-data
         run: |
           cp ~/frappe-bench/sites/.coverage ${GITHUB_WORKSPACE}
           cd ${GITHUB_WORKSPACE}
@@ -114,6 +134,7 @@ jobs:
   coveralls:
     name: Coverage Wrap Up
     needs: test
+    if: ${{ needs.test.steps.check-build.build == 'strawberry' }}
     container: python:3-slim
     runs-on: ubuntu-18.04
     steps:

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

@@ -37,17 +37,29 @@ jobs:
         with:
           python-version: 3.7
 
+      - name: Check if build should be run
+        id: check-build
+        run: |
+          python "${GITHUB_WORKSPACE}/.github/helper/roulette.py"
+        env:
+          TYPE: "server"
+          PR_NUMBER: ${{ github.event.number }}
+          REPO_NAME: ${{ github.repository }}
+
       - uses: actions/setup-node@v2
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         with:
           node-version: '14'
           check-latest: true
 
       - name: Add to Hosts
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: |
           echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
           echo "127.0.0.1 test_site_producer" | sudo tee -a /etc/hosts
 
       - name: Cache pip
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         uses: actions/cache@v2
         with:
           path: ~/.cache/pip
@@ -57,6 +69,7 @@ jobs:
             ${{ runner.os }}-
 
       - name: Cache node modules
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         uses: actions/cache@v2
         env:
           cache-name: cache-node-modules
@@ -69,10 +82,12 @@ jobs:
             ${{ runner.os }}-
 
       - name: Get yarn cache directory path
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         id: yarn-cache-dir-path
         run: echo "::set-output name=dir::$(yarn cache dir)"
 
       - uses: actions/cache@v2
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         id: yarn-cache
         with:
           path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
@@ -81,6 +96,7 @@ jobs:
             ${{ runner.os }}-yarn-
 
       - name: Install Dependencies
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh
         env:
           BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
@@ -88,12 +104,14 @@ jobs:
           TYPE: server
 
       - name: Install
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh
         env:
           DB: postgres
           TYPE: server
 
       - name: Run Tests
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --use-orchestrator
         env:
           CI_BUILD_ID: ${{ github.run_id }}

.github/workflows/ui-tests.yml

@@ -35,17 +35,29 @@ jobs:
         with:
           python-version: 3.7
 
+      - name: Check if build should be run
+        id: check-build
+        run: |
+          python "${GITHUB_WORKSPACE}/.github/helper/roulette.py"
+        env:
+          TYPE: "ui"
+          PR_NUMBER: ${{ github.event.number }}
+          REPO_NAME: ${{ github.repository }}
+
       - uses: actions/setup-node@v2
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         with:
           node-version: 14
           check-latest: true
 
       - name: Add to Hosts
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: |
           echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
           echo "127.0.0.1 test_site_producer" | sudo tee -a /etc/hosts
 
       - name: Cache pip
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         uses: actions/cache@v2
         with:
           path: ~/.cache/pip
@@ -55,6 +67,7 @@ jobs:
             ${{ runner.os }}-
 
       - name: Cache node modules
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         uses: actions/cache@v2
         env:
           cache-name: cache-node-modules
@@ -67,10 +80,12 @@ jobs:
             ${{ runner.os }}-
 
       - name: Get yarn cache directory path
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         id: yarn-cache-dir-path
         run: echo "::set-output name=dir::$(yarn cache dir)"
 
       - uses: actions/cache@v2
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         id: yarn-cache
         with:
           path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
@@ -79,6 +94,7 @@ jobs:
             ${{ runner.os }}-yarn-
 
       - name: Cache cypress binary
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         uses: actions/cache@v2
         with:
           path: ~/.cache
@@ -88,6 +104,7 @@ jobs:
             ${{ runner.os }}-
 
       - name: Install Dependencies
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh
         env:
           BEFORE: ${{ env.GITHUB_EVENT_PATH.before }}
@@ -95,15 +112,18 @@ jobs:
           TYPE: ui
 
       - name: Install
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh
         env:
           DB: mariadb
           TYPE: ui
 
       - name: Site Setup
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: cd ~/frappe-bench/ && bench --site test_site execute frappe.utils.install.complete_setup_wizard
 
       - name: UI Tests
+        if: ${{ steps.check-build.outputs.build == 'strawberry' }}
         run: cd ~/frappe-bench/ && bench --site test_site run-ui-tests frappe --headless --parallel --ci-build-id $GITHUB_RUN_ID
         env:
           CYPRESS_RECORD_KEY: 4a48f41c-11b3-425b-aa88-c58048fa69eb

.mergify.yml

@@ -1,4 +1,18 @@
 pull_request_rules:
+  - name: Auto-close PRs on stable branch
+    conditions:
+      - and:
+        - author!=surajshetty3416
+        - or:
+          - base=version-13
+          - base=version-12
+    actions:
+      close:
+      comment:
+          message: |
+            @{{author}}, thanks for the contribution, but we do not accept pull requests on a stable branch. Please raise PR on an appropriate hotfix branch. 
+            https://github.com/frappe/erpnext/wiki/Pull-Request-Checklist#which-branch
+
   - name: Automatic merge on CI success and review
     conditions:
       - status-success=Sider

cypress/integration/awesome_bar.js

@@ -10,9 +10,9 @@ context('Awesome Bar', () => {
 	});
 
 	it('navigates to doctype list', () => {
-		cy.get('#navbar-search').type('todo', { delay: 200 });
-		cy.get('#navbar-search + ul').should('be.visible');
-		cy.get('#navbar-search').type('{downarrow}{enter}', { delay: 100 });
+		cy.findByPlaceholderText('Search or type a command (Ctrl + G)').type('todo', { delay: 200 });
+		cy.get('.awesomplete').findByRole('listbox').should('be.visible');
+		cy.findByPlaceholderText('Search or type a command (Ctrl + G)').type('{downarrow}{enter}', { delay: 100 });
 
 		cy.get('.title-text').should('contain', 'To Do');
 
@@ -20,24 +20,24 @@ context('Awesome Bar', () => {
 	});
 
 	it('find text in doctype list', () => {
-		cy.get('#navbar-search')
+		cy.findByPlaceholderText('Search or type a command (Ctrl + G)')
 			.type('test in todo{downarrow}{enter}', { delay: 200 });
 
 		cy.get('.title-text').should('contain', 'To Do');
 
-		cy.get('[data-original-title="Name"] > .input-with-feedback')
+		cy.findByPlaceholderText('Name')
 			.should('have.value', '%test%');
 	});
 
 	it('navigates to new form', () => {
-		cy.get('#navbar-search')
+		cy.findByPlaceholderText('Search or type a command (Ctrl + G)')
 			.type('new blog post{downarrow}{enter}', { delay: 200 });
 
 		cy.get('.title-text:visible').should('have.text', 'New Blog Post');
 	});
 
 	it('calculates math expressions', () => {
-		cy.get('#navbar-search')
+		cy.findByPlaceholderText('Search or type a command (Ctrl + G)')
 			.type('55 + 32{downarrow}{enter}', { delay: 200 });
 
 		cy.get('.modal-title').should('contain', 'Result');

cypress/integration/control_barcode.js

@@ -20,7 +20,7 @@ context('Control Barcode', () => {
 	it('should generate barcode on setting a value', () => {
 		get_dialog_with_barcode().as('dialog');
 
-		cy.get('.frappe-control[data-fieldname=barcode] input')
+		cy.get('.frappe-control[data-fieldname=barcode]').findByRole('textbox')
 			.focus()
 			.type('123456789')
 			.blur();
@@ -37,11 +37,11 @@ context('Control Barcode', () => {
 	it('should reset when input is cleared', () => {
 		get_dialog_with_barcode().as('dialog');
 
-		cy.get('.frappe-control[data-fieldname=barcode] input')
+		cy.get('.frappe-control[data-fieldname=barcode]').findByRole('textbox')
 			.focus()
 			.type('123456789')
 			.blur();
-		cy.get('.frappe-control[data-fieldname=barcode] input')
+		cy.get('.frappe-control[data-fieldname=barcode]').findByRole('textbox')
 			.clear()
 			.blur();
 		cy.get('.frappe-control[data-fieldname=barcode] svg[data-barcode-value="123456789"]')

cypress/integration/control_icon.js

@@ -17,17 +17,17 @@ context('Control Icon', () => {
 
 	it('should set icon', () => {
 		get_dialog_with_icon().as('dialog');
-		cy.get('.frappe-control[data-fieldname=icon] input').first().click();
+		cy.get('.frappe-control[data-fieldname=icon]').findByRole('textbox').click();
 
 		cy.get('.icon-picker .icon-wrapper[id=active]').first().click();
-		cy.get('.frappe-control[data-fieldname=icon] input').first().should('have.value', 'active');
+		cy.get('.frappe-control[data-fieldname=icon]').findByRole('textbox').should('have.value', 'active');
 		cy.get('@dialog').then(dialog => {
 			let value = dialog.get_value('icon');
 			expect(value).to.equal('active');
 		});
 
 		cy.get('.icon-picker .icon-wrapper[id=resting]').first().click();
-		cy.get('.frappe-control[data-fieldname=icon] input').first().should('have.value', 'resting');
+		cy.get('.frappe-control[data-fieldname=icon]').findByRole('textbox').should('have.value', 'resting');
 		cy.get('@dialog').then(dialog => {
 			let value = dialog.get_value('icon');
 			expect(value).to.equal('resting');
@@ -36,14 +36,14 @@ context('Control Icon', () => {
 
 	it('search for icon and clear search input', () => {
 		let search_text = 'ed';
-		cy.get('.icon-picker input[type=search]').first().click().type(search_text);
+		cy.get('.icon-picker').findByRole('searchbox').click().type(search_text);
 		cy.get('.icon-section .icon-wrapper:not(.hidden)').then(i => {
 			cy.get(`.icon-section .icon-wrapper[id*='${search_text}']`).then(icons => {
 				expect(i.length).to.equal(icons.length);
 			});
 		});
 
-		cy.get('.icon-picker input[type=search]').clear().blur();
+		cy.get('.icon-picker').findByRole('searchbox').clear().blur();
 		cy.get('.icon-section .icon-wrapper').should('not.have.class', 'hidden');
 	});
 

cypress/integration/control_link.js

@@ -35,7 +35,7 @@ context('Control Link', () => {
 		cy.wait('@search_link');
 		cy.get('@input').type('todo for link', { delay: 200 });
 		cy.wait('@search_link');
-		cy.get('.frappe-control[data-fieldname=link] ul').should('be.visible');
+		cy.get('.frappe-control[data-fieldname=link]').findByRole('listbox').should('be.visible');
 		cy.get('.frappe-control[data-fieldname=link] input').type('{enter}', { delay: 100 });
 		cy.get('.frappe-control[data-fieldname=link] input').blur();
 		cy.get('@dialog').then(dialog => {
@@ -71,7 +71,7 @@ context('Control Link', () => {
 			cy.get('@input').type(todos[0]).blur();
 			cy.wait('@validate_link');
 			cy.get('@input').focus();
-			cy.get('.frappe-control[data-fieldname=link] .link-btn')
+			cy.findByTitle('Open Link')
 				.should('be.visible')
 				.click();
 			cy.location('pathname').should('eq', `/app/todo/${todos[0]}`);

cypress/integration/control_select.js

@@ -24,8 +24,10 @@ context('Control Select', () => {
 		cy.get('@control').get('.select-icon').should('exist');
 		cy.get('@control').get('.placeholder').should('have.css', 'display', 'block');
 		cy.get('@select').select('Option 1');
+		cy.findByDisplayValue('Option 1').should('exist');
 		cy.get('@control').get('.placeholder').should('have.css', 'display', 'none');
 		cy.get('@select').invoke('val', '');
+		cy.findByDisplayValue('Option 1').should('not.exist');
 		cy.get('@control').get('.placeholder').should('have.css', 'display', 'block');
 
 

cypress/integration/dashboard_links.js

@@ -0,0 +1,63 @@
+context('Dashboard links', () => {
+	before(() => {
+		cy.visit('/login');
+		cy.login();
+	});
+
+	it('Adding a new contact, checking for the counter on the dashboard and deleting the created contact', () => {
+		cy.visit('/app/contact');
+		cy.clear_filters();
+
+		cy.visit('/app/user');
+		cy.get('.list-row-col > .level-item > .ellipsis').eq(0).click();
+
+		//To check if initially the dashboard contains only the "Contact" link and there is no counter
+		cy.get('[data-doctype="Contact"]').should('contain', 'Contact');
+
+		//Adding a new contact
+		cy.get('.btn[data-doctype="Contact"]').click();
+		cy.get('[data-doctype="Contact"][data-fieldname="first_name"]').type('Admin');
+		cy.findByRole('button', {name: 'Save'}).click();
+		cy.visit('/app/user');
+		cy.get('.list-row-col > .level-item > .ellipsis').eq(0).click();
+
+		//To check if the counter for contact doc is "1" after adding the contact
+		cy.get('[data-doctype="Contact"] > .count').should('contain', '1');
+		cy.get('[data-doctype="Contact"]').contains('Contact').click();
+
+		//Deleting the newly created contact
+		cy.visit('/app/contact');
+		cy.get('.list-subject > .select-like > .list-row-checkbox').eq(0).click();
+		cy.findByRole('button', {name: 'Actions'}).click();
+		cy.get('.actions-btn-group [data-label="Delete"]').click();
+		cy.findByRole('button', {name: 'Yes'}).click({delay: 700});
+
+
+		//To check if the counter from the "Contact" doc link is removed
+		cy.wait(700);
+		cy.visit('/app/user');
+		cy.get('.list-row-col > .level-item > .ellipsis').eq(0).click();
+		cy.get('[data-doctype="Contact"]').should('contain', 'Contact');
+	});
+
+	it('Report link in dashboard', () => {
+		cy.visit('/app/user');
+		cy.visit('/app/user/Administrator');
+		cy.get('[data-doctype="Contact"]').should('contain', 'Contact');
+		cy.findByText('Connections');
+		cy.window()
+			.its('cur_frm')
+			.then(cur_frm => {
+				cur_frm.dashboard.data.reports = [
+					{
+						'label': 'Reports',
+						'items': ['Permitted Documents For User']
+					}
+				];
+				cur_frm.dashboard.render_report_links();
+				cy.get('[data-report="Permitted Documents For User"]').contains('Permitted Documents For User').click();
+				cy.findByText('Permitted Documents For User');
+				cy.findByPlaceholderText('User').should("have.value", "Administrator");
+			});
+	});
+});

cypress/integration/depends_on.js

@@ -62,11 +62,11 @@ context('Depends On', () => {
 	it('should set the field as mandatory depending on other fields value', () => {
 		cy.new_form('Test Depends On');
 		cy.fill_field('test_field', 'Some Value');
-		cy.get('button.primary-action').contains('Save').click();
+		cy.findByRole('button', {name: 'Save'}).click();
 		cy.get('.msgprint-dialog .modal-title').contains('Missing Fields').should('be.visible');
 		cy.hide_dialog();
 		cy.fill_field('test_field', 'Random value');
-		cy.get('button.primary-action').contains('Save').click();
+		cy.findByRole('button', {name: 'Save'}).click();
 		cy.get('.msgprint-dialog .modal-title').contains('Missing Fields').should('not.be.visible');
 	});
 	it('should set the field as read only depending on other fields value', () => {
@@ -84,7 +84,7 @@ context('Depends On', () => {
 		cy.fill_field('dependant_field', 'Some Value');
 		//cy.fill_field('test_field', 'Some Other Value');
 		cy.get('.frappe-control[data-fieldname="child_test_depends_on_field"]').as('table');
-		cy.get('@table').find('button.grid-add-row').click();
+		cy.get('@table').findByRole('button', {name: 'Add Row'}).click();
 		cy.get('@table').find('[data-idx="1"]').as('row1');
 		cy.get('@row1').find('.btn-open-row').click();
 		cy.get('@row1').find('.form-in-grid').as('row1-form_in_grid');

cypress/integration/file_uploader.js

@@ -25,7 +25,7 @@ context('FileUploader', () => {
 
 		cy.get_open_dialog().find('.file-name').should('contain', 'example.json');
 		cy.intercept('POST', '/api/method/upload_file').as('upload_file');
-		cy.get_open_dialog().find('.btn-modal-primary').click();
+		cy.get_open_dialog().findByRole('button', {name: 'Upload'}).click();
 		cy.wait('@upload_file').its('response.statusCode').should('eq', 200);
 		cy.get('.modal:visible').should('not.exist');
 	});
@@ -33,11 +33,11 @@ context('FileUploader', () => {
 	it('should accept uploaded files', () => {
 		open_upload_dialog();
 
-		cy.get_open_dialog().find('.btn-file-upload div:contains("Library")').click();
-		cy.get('.file-filter').type('example.json');
-		cy.get_open_dialog().find('.tree-label:contains("example.json")').first().click();
+		cy.get_open_dialog().findByRole('button', {name: 'Library'}).click();
+		cy.findByPlaceholderText('Search by filename or extension').type('example.json');
+		cy.get_open_dialog().findAllByText('example.json').first().click();
 		cy.intercept('POST', '/api/method/upload_file').as('upload_file');
-		cy.get_open_dialog().find('.btn-primary').click();
+		cy.get_open_dialog().findByRole('button', {name: 'Upload'}).click();
 		cy.wait('@upload_file').its('response.body.message')
 			.should('have.property', 'file_name', 'example.json');
 		cy.get('.modal:visible').should('not.exist');
@@ -46,10 +46,12 @@ context('FileUploader', () => {
 	it('should accept web links', () => {
 		open_upload_dialog();
 
-		cy.get_open_dialog().find('.btn-file-upload div:contains("Link")').click();
-		cy.get_open_dialog().find('.file-web-link input').type('https://github.com', { delay: 100, force: true });
+		cy.get_open_dialog().findByRole('button', {name: 'Link'}).click();
+		cy.get_open_dialog()
+			.findByPlaceholderText('Attach a web link')
+			.type('https://github.com', { delay: 100, force: true });
 		cy.intercept('POST', '/api/method/upload_file').as('upload_file');
-		cy.get_open_dialog().find('.btn-primary').click();
+		cy.get_open_dialog().findByRole('button', {name: 'Upload'}).click();
 		cy.wait('@upload_file').its('response.body.message')
 			.should('have.property', 'file_url', 'https://github.com');
 		cy.get('.modal:visible').should('not.exist');
@@ -62,15 +64,14 @@ context('FileUploader', () => {
 			subjectType: 'drag-n-drop',
 		});
 
-		cy.get_open_dialog().find('.file-name').should('contain', 'sample_image.jpg');
+		cy.get_open_dialog().findAllByText('sample_image.jpg').should('exist');
 		cy.get_open_dialog().find('.btn-crop').first().click();
-		cy.get_open_dialog().find('.image-cropper-actions > .btn-primary').should('contain', 'Crop');
-		cy.get_open_dialog().find('.image-cropper-actions > .btn-primary').click();
-		cy.get_open_dialog().find('.optimize-checkbox').first().should('contain', 'Optimize');
-		cy.get_open_dialog().find('.optimize-checkbox').first().click();
+		cy.get_open_dialog().findByRole('button', {name: 'Crop'}).click();
+		cy.get_open_dialog().findAllByRole('checkbox', {name: 'Optimize'}).should('exist');
+		cy.get_open_dialog().findAllByLabelText('Optimize').first().click();
 
 		cy.intercept('POST', '/api/method/upload_file').as('upload_file');
-		cy.get_open_dialog().find('.btn-modal-primary').click();
+		cy.get_open_dialog().findByRole('button', {name: 'Upload'}).click();
 		cy.wait('@upload_file').its('response.statusCode').should('eq', 200);
 		cy.get('.modal:visible').should('not.exist');
 	});

cypress/integration/form.js

@@ -26,7 +26,7 @@ context('Form', () => {
 		cy.visit('/app/contact');
 		cy.add_filter();
 		cy.get('.filter-field .input-with-feedback.form-control').type('123', { force: true });
-		cy.get('.filter-popover .apply-filters').click({ force: true });
+		cy.findByRole('button', {name: 'Apply Filters'}).click({ force: true });
 		cy.visit('/app/contact/Test Form Contact 3');
 		cy.get('.prev-doc').should('be.visible').click();
 		cy.get('.msgprint-dialog .modal-body').contains('No further records').should('be.visible');

cypress/integration/form_tour.js

@@ -9,7 +9,7 @@ context('Form Tour', () => {
 
 	const open_test_form_tour = () => {
 		cy.visit('/app/form-tour/Test Form Tour');
-		cy.get('button[data-label="Show%20Tour"]').should('be.visible').and('contain', 'Show Tour').as('show_tour');
+		cy.findByRole('button', {name: 'Show Tour'}).should('be.visible').as('show_tour');
 		cy.get('@show_tour').click();
 		cy.wait(500);
 		cy.url().should('include', '/app/contact');
@@ -20,10 +20,10 @@ context('Form Tour', () => {
 	it('navigates a form tour', () => {
 		open_test_form_tour();
 
-		cy.get('#driver-popover-item').should('be.visible');
+		cy.get('.frappe-driver').should('be.visible');
 		cy.get('.frappe-control[data-fieldname="first_name"]').as('first_name');
 		cy.get('@first_name').should('have.class', 'driver-highlighted-element');
-		cy.get('.driver-next-btn').as('next_btn');
+		cy.get('.frappe-driver').findByRole('button', {name: 'Next'}).as('next_btn');
 
 		// next btn shouldn't move to next step, if first name is not entered
 		cy.get('@next_btn').click();
@@ -39,7 +39,7 @@ context('Form Tour', () => {
 		// assert field is highlighted
 		cy.get('.frappe-control[data-fieldname="last_name"]').as('last_name');
 		cy.get('@last_name').should('have.class', 'driver-highlighted-element');
-		
+
 		// after filling the field, next step should be highlighted
 		cy.fill_field('last_name', 'Test Last Name', 'Data');
 		cy.wait(500);
@@ -49,12 +49,12 @@ context('Form Tour', () => {
 		// assert field is highlighted
 		cy.get('.frappe-control[data-fieldname="phone_nos"]').as('phone_nos');
 		cy.get('@phone_nos').should('have.class', 'driver-highlighted-element');
-		
+
 		// move to next step
 		cy.wait(500);
 		cy.get('@next_btn').click();
 		cy.wait(500);
-		
+
 		// assert add row btn is highlighted
 		cy.get('@phone_nos').find('.grid-add-row').as('add_row');
 		cy.get('@add_row').should('have.class', 'driver-highlighted-element');
@@ -68,21 +68,21 @@ context('Form Tour', () => {
 		cy.get('.grid-row-open .frappe-control[data-fieldname="phone"]').as('phone');
 		cy.get('@phone').should('have.class', 'driver-highlighted-element');
 		// enter value in a table field
-		cy.fill_table_field('phone_nos', '1', 'phone', '1234567890');
+		let field = cy.fill_table_field('phone_nos', '1', 'phone', '1234567890');
+		field.blur();
 
 		// move to collapse row step
 		cy.wait(500);
-		cy.get('@next_btn').click();
+		cy.get('.driver-popover-title').contains('Test Title 4').siblings().get('@next_btn').click();
 		cy.wait(500);
-
 		// collapse row
 		cy.get('.grid-row-open .grid-collapse-row').click();
 		cy.wait(500);
-		
+
 		// assert save btn is highlighted
 		cy.get('.primary-action').should('have.class', 'driver-highlighted-element');
-		cy.get('@next_btn').should('contain', 'Save');
+		cy.wait(500);
+		cy.get('.frappe-driver').findByRole('button', {name: 'Save'}).should('be.visible');
 
 	});
 });
-	

cypress/integration/grid_pagination.js

@@ -30,12 +30,12 @@ context('Grid Pagination', () => {
 	it('adds and deletes rows and changes page', () => {
 		cy.visit('/app/contact/Test Contact');
 		cy.get('.frappe-control[data-fieldname="phone_nos"]').as('table');
-		cy.get('@table').find('button.grid-add-row').click();
+		cy.get('@table').findByRole('button', {name: 'Add Row'}).click();
 		cy.get('@table').find('.grid-body .row-index').should('contain', 1001);
 		cy.get('@table').find('.current-page-number').should('contain', '21');
 		cy.get('@table').find('.total-page-number').should('contain', '21');
 		cy.get('@table').find('.grid-body .grid-row .grid-row-check').click({ force: true });
-		cy.get('@table').find('button.grid-remove-rows').click();
+		cy.get('@table').findByRole('button', {name: 'Delete'}).click();
 		cy.get('@table').find('.grid-body .row-index').last().should('contain', 1000);
 		cy.get('@table').find('.current-page-number').should('contain', '20');
 		cy.get('@table').find('.total-page-number').should('contain', '20');

cypress/integration/list_view_settings.js

@@ -17,9 +17,9 @@ context('List View Settings', () => {
 		cy.get('.dropdown-menu li').filter(':visible').contains('List Settings').click();
 		cy.get('.modal-dialog').should('contain', 'DocType Settings');
 
-		cy.get('input[data-fieldname="disable_count"]').check({ force: true });
-		cy.get('input[data-fieldname="disable_sidebar_stats"]').check({ force: true });
-		cy.get('button').filter(':visible').contains('Save').click();
+		cy.findByLabelText('Disable Count').check({ force: true });
+		cy.findByLabelText('Disable Sidebar Stats').check({ force: true });
+		cy.findByRole('button', {name: 'Save'}).click();
 
 		cy.reload({ force: true });
 
@@ -29,8 +29,8 @@ context('List View Settings', () => {
 		cy.get('.menu-btn-group button').click({ force: true });
 		cy.get('.dropdown-menu li').filter(':visible').contains('List Settings').click();
 		cy.get('.modal-dialog').should('contain', 'DocType Settings');
-		cy.get('input[data-fieldname="disable_count"]').uncheck({ force: true });
-		cy.get('input[data-fieldname="disable_sidebar_stats"]').uncheck({ force: true });
-		cy.get('button').filter(':visible').contains('Save').click();
+		cy.findByLabelText('Disable Count').uncheck({ force: true });
+		cy.findByLabelText('Disable Sidebar Stats').uncheck({ force: true });
+		cy.findByRole('button', {name: 'Save'}).click();
 	});
 });

cypress/integration/login.js

@@ -11,13 +11,13 @@ context('Login', () => {
 
 	it('validates password', () => {
 		cy.get('#login_email').type('Administrator');
-		cy.get('.btn-login:visible').click();
+		cy.findByRole('button', {name: 'Login'}).click();
 		cy.location('pathname').should('eq', '/login');
 	});
 
 	it('validates email', () => {
 		cy.get('#login_password').type('qwe');
-		cy.get('.btn-login:visible').click();
+		cy.findByRole('button', {name: 'Login'}).click();
 		cy.location('pathname').should('eq', '/login');
 	});
 
@@ -25,8 +25,8 @@ context('Login', () => {
 		cy.get('#login_email').type('Administrator');
 		cy.get('#login_password').type('qwer');
 
-		cy.get('.btn-login:visible').click();
-		cy.get('.btn-login:visible').contains('Invalid Login. Try again.');
+		cy.findByRole('button', {name: 'Login'}).click();
+		cy.findByRole('button', {name: 'Invalid Login. Try again.'}).should('exist');
 		cy.location('pathname').should('eq', '/login');
 	});
 
@@ -34,7 +34,7 @@ context('Login', () => {
 		cy.get('#login_email').type('Administrator');
 		cy.get('#login_password').type(Cypress.config('adminPassword'));
 
-		cy.get('.btn-login:visible').click();
+		cy.findByRole('button', {name: 'Login'}).click();
 		cy.location('pathname').should('eq', '/app');
 		cy.window().its('frappe.session.user').should('eq', 'Administrator');
 	});
@@ -60,7 +60,7 @@ context('Login', () => {
 		cy.get('#login_email').type('Administrator');
 		cy.get('#login_password').type(Cypress.config('adminPassword'));
 
-		cy.get('.btn-login:visible').click();
+		cy.findByRole('button', {name: 'Login'}).click();
 
 		// verify redirected location and url params after login
 		cy.url().should('include', '/me?' + payload.toString().replace('+', '%20'));

cypress/integration/recorder.js

@@ -16,24 +16,24 @@ context('Recorder', () => {
 	it('Navigate to Recorder', () => {
 		cy.visit('/app');
 		cy.awesomebar('recorder');
-		cy.get('h3').should('contain', 'Recorder');
+		cy.findByTitle('Recorder').should('exist');
 		cy.url().should('include', '/recorder/detail');
 	});
 
 	it('Recorder Empty State', () => {
-		cy.get('.title-text').should('contain', 'Recorder');
+		cy.findByTitle('Recorder').should('exist');
 
 		cy.get('.indicator-pill').should('contain', 'Inactive').should('have.class', 'red');
 
-		cy.get('.primary-action').should('contain', 'Start');
-		cy.get('.btn-secondary').should('contain', 'Clear');
+		cy.findByRole('button', {name: 'Start'}).should('exist');
+		cy.findByRole('button', {name: 'Clear'}).should('exist');
 
 		cy.get('.msg-box').should('contain', 'Inactive');
-		cy.get('.msg-box .btn-primary').should('contain', 'Start Recording');
+		cy.findByRole('button', {name: 'Start Recording'}).should('exist');
 	});
 
 	it('Recorder Start', () => {
-		cy.get('.primary-action').should('contain', 'Start').click();
+		cy.findByRole('button', {name: 'Start'}).click();
 		cy.get('.indicator-pill').should('contain', 'Active').should('have.class', 'green');
 
 		cy.get('.msg-box').should('contain', 'No Requests');
@@ -46,12 +46,12 @@ context('Recorder', () => {
 		cy.get('.list-count').should('contain', '20 of ');
 
 		cy.visit('/app/recorder');
-		cy.get('.title-text').should('contain', 'Recorder');
+		cy.findByTitle('Recorder').should('exist');
 		cy.get('.result-list').should('contain', '/api/method/frappe.desk.reportview.get');
 	});
 
 	it('Recorder View Request', () => {
-		cy.get('.primary-action').should('contain', 'Start').click();
+		cy.findByRole('button', {name: 'Start'}).click();
 
 		cy.visit('/app/List/DocType/List');
 		cy.intercept('POST', '/api/method/frappe.desk.reportview.get').as('list_refresh');

cypress/integration/report_view.js

@@ -23,7 +23,7 @@ context('Report View', () => {
 		let cell = cy.get('.dt-row-0 > .dt-cell--col-4');
 		// select the cell
 		cell.dblclick();
-		cell.find('input[data-fieldname="enabled"]').check({ force: true });
+		cell.findByRole('checkbox').check({ force: true });
 		cy.get('.dt-row-0 > .dt-cell--col-5').click();
 		cy.wait('@value-update');
 		cy.get('@doc').then(doc => {

cypress/integration/timeline.js

@@ -10,26 +10,26 @@ context('Timeline', () => {
 	it('Adding new ToDo, adding new comment, verifying comment addition & deletion and deleting ToDo', () => {
 		//Adding new ToDo
 		cy.click_listview_primary_button('Add ToDo');
-		cy.get('.modal-footer > .custom-actions > .btn').contains('Edit in full page').click();
-		cy.get('.row > .section-body > .form-column > form > .frappe-control > .form-group > .control-input-wrapper > .control-input > .ql-container > .ql-editor').eq(0).type('Test ToDo', {force: true});
+		cy.findByRole('button', {name: 'Edit in full page'}).click();
+		cy.get('[data-fieldname="description"] .ql-editor').eq(0).type('Test ToDo', {force: true});
 		cy.wait(200);
-		cy.get('#page-ToDo > .page-head > .container > .row > .col > .standard-actions > .primary-action').contains('Save').click();
+		cy.findByRole('button', {name: 'Save'}).click();
 		cy.wait(700);
 		cy.visit('/app/todo');
-		cy.get('.list-row > .level-left > .list-subject > .level-item.ellipsis > .ellipsis').eq(0).click();
+		cy.get('.level-item.ellipsis').eq(0).click();
 
 		//To check if the comment box is initially empty and tying some text into it
-		cy.get('.comment-input-container > .frappe-control > .ql-container > .ql-editor').should('contain', '').type('Testing Timeline');
+		cy.get('[data-fieldname="comment"] .ql-editor').should('contain', '').type('Testing Timeline');
 
 		//Adding new comment
-		cy.get('.comment-input-wrapper > .btn').contains('Comment').click();
+		cy.findByRole('button', {name: 'Comment'}).click();
 
 		//To check if the commented text is visible in the timeline content
 		cy.get('.timeline-content').should('contain', 'Testing Timeline');
 
 		//Editing comment
 		cy.click_timeline_action_btn(0);
-		cy.get('.timeline-content > .timeline-message-box > .comment-edit-box > .frappe-control > .ql-container > .ql-editor').first().type(' 123');
+		cy.get('.timeline-content [data-fieldname="comment"] .ql-editor').first().type(' 123');
 		cy.click_timeline_action_btn(0);
 
 		//To check if the edited comment text is visible in timeline content
@@ -37,20 +37,20 @@ context('Timeline', () => {
 
 		//Discarding comment
 		cy.click_timeline_action_btn(0);
-		cy.get('.actions > .btn').eq(1).first().click();
+		cy.findByRole('button', {name: 'Dismiss'}).click();
 
 		//To check if after discarding the timeline content is same as previous
 		cy.get('.timeline-content').should('contain', 'Testing Timeline 123');
 
 		//Deleting the added comment
 		cy.get('.actions > .btn > .icon').first().click();
-		cy.get('.modal-footer > .standard-actions > .btn-primary').contains('Yes').click();
+		cy.findByRole('button', {name: 'Yes'}).click();
 		cy.click_modal_primary_button('Yes');
 
 		//Deleting the added ToDo
-		cy.get('#page-ToDo > .page-head > .container > .row > .col > .standard-actions > .menu-btn-group > .btn').click({force: true});
-		cy.get('.menu-btn-group > .dropdown-menu > li > .grey-link').eq(17).click({force: true});
-		cy.get('.modal.show > .modal-dialog > .modal-content > .modal-footer > .standard-actions > .btn-primary').contains('Yes').click({force: true});
+		cy.get('.menu-btn-group button').eq(1).click();
+		cy.get('.menu-btn-group [data-label="Delete"]').click();
+		cy.findByRole('button', {name: 'Yes'}).click();
 	});
 
 	it('Timeline should have submit and cancel activity information', () => {
@@ -64,31 +64,31 @@ context('Timeline', () => {
 
 		//Adding a new entry for the created custom doctype
 		cy.fill_field('title', 'Test');
-		cy.get('.modal-footer > .standard-actions > .btn-primary').contains('Save').click();
-		cy.get('.modal-footer > .standard-actions > .btn-primary').contains('Submit').click();
+		cy.findByRole('button', {name: 'Save'}).click();
+		cy.findByRole('button', {name: 'Submit'}).click();
 		cy.visit('/app/custom-submittable-doctype');
 		cy.get('.list-subject > .bold > .ellipsis').eq(0).click();
 
 		//To check if the submission of the documemt is visible in the timeline content
 		cy.get('.timeline-content').should('contain', 'Administrator submitted this document');
-		cy.get('.page-actions > .standard-actions > .btn-secondary').contains('Cancel').click({delay: 900});		
-		cy.get('.modal-footer > .standard-actions > .btn-primary').contains('Yes').click();
-		
+		cy.findByRole('button', {name: 'Cancel'}).click({delay: 900});
+		cy.findByRole('button', {name: 'Yes'}).click();
+
 		//To check if the cancellation of the documemt is visible in the timeline content
 		cy.get('.timeline-content').should('contain', 'Administrator cancelled this document');
 
 		//Deleting the document
 		cy.visit('/app/custom-submittable-doctype');
 		cy.get('.list-subject > .select-like > .list-row-checkbox').eq(0).click();
-		cy.get('.page-actions > .standard-actions > .actions-btn-group > .btn').contains('Actions').click();
+		cy.findByRole('button', {name: 'Actions'}).click();
 		cy.get('.actions-btn-group > .dropdown-menu > li > .grey-link').eq(7).click();
 		cy.click_modal_primary_button('Yes', {force: true, delay: 700});
 
 		//Deleting the custom doctype
 		cy.visit('/app/doctype');
 		cy.get('.list-subject > .select-like > .list-row-checkbox').eq(0).click();
-		cy.get('.page-actions > .standard-actions > .actions-btn-group > .btn').contains('Actions').click();
-		cy.get('.actions-btn-group > .dropdown-menu > li > .grey-link').eq(5).click();
+		cy.findByRole('button', {name: 'Actions'}).click();
+		cy.get('.actions-btn-group [data-label="Delete"]').click();
 		cy.click_modal_primary_button('Yes');
 	});
 });

cypress/integration/timeline_email.js

@@ -8,14 +8,13 @@ context('Timeline Email', () => {
 	it('Adding new ToDo, adding email and verifying timeline content for email attachment, deleting attachment and ToDo', () => {
 		//Adding new ToDo
 		cy.click_listview_primary_button('Add ToDo');
-		cy.get('.custom-actions > .btn').trigger('click', {delay: 500});
-		cy.get('.row > .section-body > .form-column > form > .frappe-control > .form-group > .control-input-wrapper > .control-input > .ql-container > .ql-editor').eq(0).type('Test ToDo', {force: true});
+		cy.get('.custom-actions:visible > .btn').contains("Edit in full page").click({delay: 500});
+		cy.fill_field("description", "Test ToDo",  "Text Editor");
 		cy.wait(500);
-		//cy.click_listview_primary_button('Save');
 		cy.get('.primary-action').contains('Save').click({force: true});
 		cy.wait(700);
 		cy.visit('/app/todo');
-		cy.get('.list-row > .level-left > .list-subject > .level-item.ellipsis > .ellipsis').eq(0).click();
+		cy.get('.list-row > .level-left > .list-subject').eq(0).click();
 
 		//Creating a new email
 		cy.get('.timeline-actions > .btn').click();
@@ -47,7 +46,7 @@ context('Timeline Email', () => {
 
 		//Removing the added attachment
 		cy.get('.attachment-row > .data-pill > .remove-btn > .icon').click();
-		cy.get('.modal-dialog > .modal-content > .modal-footer > .standard-actions > .btn-primary').contains('Yes').click();
+		cy.get('.modal-footer:visible > .standard-actions > .btn-primary').contains('Yes').click();
 
 		//To check if the removed attachment is shown in the timeline content
 		cy.get('.timeline-content').should('contain', 'Removed 72402.jpg');
@@ -55,17 +54,17 @@ context('Timeline Email', () => {
 
 		//To check if the discard button functionality in email is working correctly
 		cy.get('.timeline-actions > .btn').click();
-		cy.fill_field('recipients', 'test@example.com', 'MultiSelect'); 
+		cy.fill_field('recipients', 'test@example.com', 'MultiSelect');
 		cy.get('.modal-footer > .standard-actions > .btn-secondary').contains('Discard').click();
 		cy.wait(500);
 		cy.get('.timeline-actions > .btn').click();
 		cy.wait(500);
 		cy.get_field('recipients', 'MultiSelect').should('have.text', '');
-		cy.get('.modal.show > .modal-dialog > .modal-content > .modal-header > .modal-actions > .btn-modal-close > .icon').click();  
+		cy.get('.modal-header:visible > .modal-actions > .btn-modal-close > .icon').click();
 
 		//Deleting the added ToDo
-		cy.get('#page-ToDo > .page-head > .container > .row > .col > .standard-actions > .menu-btn-group > .btn').click();
-		cy.get('.menu-btn-group > .dropdown-menu > li > .grey-link').eq(17).click();
-		cy.get('.modal.show > .modal-dialog > .modal-content > .modal-footer > .standard-actions > .btn-primary').click();
+		cy.get('.menu-btn-group:visible > .btn').click();
+		cy.get('.menu-btn-group:visible > .dropdown-menu > li > .dropdown-item').contains('Delete').click();
+		cy.get('.modal-footer:visible > .standard-actions > .btn-primary').click();
 	});
 });

cypress/integration/workspace.js

@@ -14,7 +14,7 @@ context('Workspace 2.0', () => {
 
 	it('Create Private Page', () => {
 		cy.get('.codex-editor__redactor .ce-block');
-		cy.get('.custom-actions button[data-label="Create%20Page"]').click();
+		cy.get('.custom-actions button[data-label="Create%20Workspace"]').click();
 		cy.fill_field('title', 'Test Private Page', 'Data');
 		cy.fill_field('icon', 'edit', 'Icon');
 		cy.get_open_dialog().find('.modal-header').click();
@@ -29,19 +29,19 @@ context('Workspace 2.0', () => {
 
 		cy.wait(500);
 		cy.get('.codex-editor__redactor .ce-block');
-		cy.get('.standard-actions .btn-secondary[data-label=Customize]').click();
+		cy.get('.standard-actions .btn-secondary[data-label=Edit]').click();
 	});
 
 	it('Add New Block', () => {
 		cy.get('.codex-editor__redactor .ce-block');
 		cy.get('.custom-actions .inner-group-button[data-label="Add%20Block"]').click();
 		cy.get('.custom-actions .inner-group-button .dropdown-menu .block-menu-item-label').contains('Heading').click();
-		cy.get(".ce-block:last").find('h2').click({force: true}).type('Header');
+		cy.get(":focus").type('Header');
 		cy.get(".ce-block:last").find('.ce-header').should('exist');
 
 		cy.get('.custom-actions .inner-group-button[data-label="Add%20Block"]').click();
 		cy.get('.custom-actions .inner-group-button .dropdown-menu .block-menu-item-label').contains('Text').click();
-		cy.get(".ce-block:last").find('.ce-paragraph').click({force: true}).type('Paragraph text');
+		cy.get(":focus").type('Paragraph text');
 		cy.get(".ce-block:last").find('.ce-paragraph').should('exist');
 	});
 
@@ -77,7 +77,7 @@ context('Workspace 2.0', () => {
 
 	it('Delete Private Page', () => {
 		cy.get('.codex-editor__redactor .ce-block');
-		cy.get('.standard-actions .btn-secondary[data-label=Customize]').click();
+		cy.get('.standard-actions .btn-secondary[data-label=Edit]').click();
 
 		cy.get('.sidebar-item-container[item-name="Test Private Page"]').find('.sidebar-item-control .delete-page').click();
 		cy.wait(300);

cypress/support/commands.js

@@ -1,4 +1,5 @@
 import 'cypress-file-upload';
+import '@testing-library/cypress/add-commands';
 // ***********************************************
 // This example commands.js shows you how to
 // create various custom commands and overwrite

frappe/automation/doctype/assignment_rule/test_assignment_rule.py

@@ -76,7 +76,7 @@ class TestAutoAssign(unittest.TestCase):
 		# clear 5 assignments for first user
 		# can't do a limit in "delete" since postgres does not support it
 		for d in frappe.get_all('ToDo', dict(reference_type = 'Note', owner = 'test@example.com'), limit=5):
-			frappe.db.sql("delete from tabToDo where name = %s", d.name)
+			frappe.db.delete("ToDo", {"name": d.name})
 
 		# add 5 more assignments
 		for i in range(5):
@@ -177,7 +177,7 @@ class TestAutoAssign(unittest.TestCase):
 		), 'owner'), 'test@example.com')
 
 	def check_assignment_rule_scheduling(self):
-		frappe.db.sql("DELETE FROM `tabAssignment Rule`")
+		frappe.db.delete("Assignment Rule")
 
 		days_1 = [dict(day = 'Sunday'), dict(day = 'Monday'), dict(day = 'Tuesday')]
 
@@ -204,7 +204,7 @@ class TestAutoAssign(unittest.TestCase):
 		), 'owner'), ['test3@example.com'])
 
 	def test_assignment_rule_condition(self):
-		frappe.db.sql("DELETE FROM `tabAssignment Rule`")
+		frappe.db.delete("Assignment Rule")
 
 		# Add expiry_date custom field
 		from frappe.custom.doctype.custom_field.custom_field import create_custom_field
@@ -253,7 +253,7 @@ class TestAutoAssign(unittest.TestCase):
 		assignment_rule.delete()
 
 def clear_assignments():
-	frappe.db.sql("delete from tabToDo where reference_type = 'Note'")
+	frappe.db.delete("ToDo", {"reference_type": "Note"})
 
 def get_assignment_rule(days, assign=None):
 	frappe.delete_doc_if_exists('Assignment Rule', 'For Note 1')

frappe/automation/doctype/auto_repeat/auto_repeat.js

@@ -30,7 +30,7 @@ frappe.ui.form.on('Auto Repeat', {
 	refresh: function(frm) {
 		// auto repeat message
 		if (frm.is_new()) {
-			let customize_form_link = `<a href="/app/customize form">${__('Customize Form')}</a>`;
+			let customize_form_link = `<a href="/app/customize-form">${__('Customize Form')}</a>`;
 			frm.dashboard.set_headline(__('To configure Auto Repeat, enable "Allow Auto Repeat" from {0}.', [customize_form_link]));
 		}
 

frappe/automation/doctype/milestone_tracker/test_milestone_tracker.py

@@ -7,7 +7,7 @@ import unittest
 
 class TestMilestoneTracker(unittest.TestCase):
 	def test_milestone(self):
-		frappe.db.sql('delete from `tabMilestone Tracker`')
+		frappe.db.delete("Milestone Tracker")
 
 		frappe.cache().delete_key('milestone_tracker_map')
 
@@ -44,5 +44,5 @@ class TestMilestoneTracker(unittest.TestCase):
 		self.assertEqual(milestones[0].value, 'Closed')
 
 		# cleanup
-		frappe.db.sql('delete from tabMilestone')
+		frappe.db.delete("Milestone")
 		milestone_tracker.delete()

frappe/commands/utils.py

@@ -486,15 +486,26 @@ frappe.db.connect()
 
 
 @click.command('console')
+@click.option(
+	'--autoreload',
+	is_flag=True,
+	help="Reload changes to code automatically"
+)
 @pass_context
-def console(context):
+def console(context, autoreload=False):
 	"Start ipython console for a site"
 	site = get_site(context)
 	frappe.init(site=site)
 	frappe.connect()
 	frappe.local.lang = frappe.db.get_default("lang")
 
-	import IPython
+	from IPython.terminal.embed import InteractiveShellEmbed
+
+	terminal = InteractiveShellEmbed()
+	if autoreload:
+		terminal.extension_manager.load_extension("autoreload")
+		terminal.run_line_magic("autoreload", "2")
+
 	all_apps = frappe.get_installed_apps()
 	failed_to_import = []
 
@@ -509,7 +520,9 @@ def console(context):
 	if failed_to_import:
 		print("\nFailed to import:\n{}".format(", ".join(failed_to_import)))
 
-	IPython.embed(display_banner="", header="", colors="neutral")
+	terminal.colors = "neutral"
+	terminal.display_banner = False
+	terminal()
 
 
 @click.command('run-tests')
@@ -524,7 +537,7 @@ def console(context):
 @click.option('--skip-test-records', is_flag=True, default=False, help="Don't create test records")
 @click.option('--skip-before-tests', is_flag=True, default=False, help="Don't run before tests hook")
 @click.option('--junit-xml-output', help="Destination file path for junit xml report")
-@click.option('--failfast', is_flag=True, default=False)
+@click.option('--failfast', is_flag=True, default=False, help="Stop the test run on the first error or failure")
 @pass_context
 def run_tests(context, app=None, module=None, doctype=None, test=(), profile=False,
 		coverage=False, junit_xml_output=False, ui_tests = False, doctype_list_path=None,
@@ -589,24 +602,26 @@ def run_ui_tests(context, app, headless=False, parallel=True, ci_build_id=None):
 	admin_password = frappe.get_conf(site).admin_password
 
 	# override baseUrl using env variable
-	site_env = 'CYPRESS_baseUrl={}'.format(site_url)
-	password_env = 'CYPRESS_adminPassword={}'.format(admin_password) if admin_password else ''
+	site_env = f'CYPRESS_baseUrl={site_url}'
+	password_env = f'CYPRESS_adminPassword={admin_password}' if admin_password else ''
 
 	os.chdir(app_base_path)
 
 	node_bin = subprocess.getoutput("npm bin")
-	cypress_path = "{0}/cypress".format(node_bin)
-	plugin_path = "{0}/../cypress-file-upload".format(node_bin)
+	cypress_path = f"{node_bin}/cypress"
+	plugin_path = f"{node_bin}/../cypress-file-upload"
+	testing_library_path = f"{node_bin}/../@testing-library"
 
 	# check if cypress in path...if not, install it.
 	if not (
 		os.path.exists(cypress_path)
 		and os.path.exists(plugin_path)
+		and os.path.exists(testing_library_path)
 		and cint(subprocess.getoutput("npm view cypress version")[:1]) >= 6
 	):
 		# install cypress
 		click.secho("Installing Cypress...", fg="yellow")
-		frappe.commands.popen("yarn add cypress@^6 cypress-file-upload@^5 --no-lockfile")
+		frappe.commands.popen("yarn add cypress@^6 cypress-file-upload@^5 @testing-library/cypress@^8 --no-lockfile")
 
 	# run for headless mode
 	run_or_open = 'run --browser firefox --record' if headless else 'open'
@@ -617,7 +632,7 @@ def run_ui_tests(context, app, headless=False, parallel=True, ci_build_id=None):
 		formatted_command += ' --parallel'
 
 	if ci_build_id:
-		formatted_command += ' --ci-build-id {}'.format(ci_build_id)
+		formatted_command += f' --ci-build-id {ci_build_id}'
 
 	click.secho("Running Cypress...", fg="yellow")
 	frappe.commands.popen(formatted_command, cwd=app_base_path, raise_err=True)

frappe/core/doctype/access_log/access_log.py

@@ -29,4 +29,5 @@ def make_access_log(doctype=None, document=None, method=None, file_type=None,
 	doc.insert(ignore_permissions=True)
 
 	# `frappe.db.commit` added because insert doesnt `commit` when called in GET requests like `printview`
-	frappe.db.commit()
+	if frappe.request and frappe.request.method == 'GET':
+		frappe.db.commit()

frappe/core/doctype/comment/test_comment.py

@@ -30,7 +30,7 @@ class TestComment(unittest.TestCase):
 		from frappe.website.doctype.blog_post.test_blog_post import make_test_blog
 		test_blog = make_test_blog()
 
-		frappe.db.sql("delete from `tabComment` where reference_doctype = 'Blog Post'")
+		frappe.db.delete("Comment", {"reference_doctype": "Blog Post"})
 
 		from frappe.templates.includes.comments.comments import add_comment
 		add_comment('Good comment with 10 chars', 'test@test.com', 'Good Tester',
@@ -41,7 +41,7 @@ class TestComment(unittest.TestCase):
 			reference_name = test_blog.name
 		))[0].published, 1)
 
-		frappe.db.sql("delete from `tabComment` where reference_doctype = 'Blog Post'")
+		frappe.db.delete("Comment", {"reference_doctype": "Blog Post"})
 
 		add_comment('pleez vizits my site http://mysite.com', 'test@test.com', 'bad commentor',
 			'Blog Post', test_blog.name, test_blog.route)

frappe/core/doctype/feedback/feedback.json

@@ -8,8 +8,8 @@
   "reference_doctype",
   "reference_name",
   "column_break_3",
-  "email",
   "rating",
+  "ip_address",
   "section_break_6",
   "feedback"
  ],
@@ -18,12 +18,6 @@
    "fieldname": "column_break_3",
    "fieldtype": "Column Break"
   },
-  {
-   "fieldname": "email",
-   "fieldtype": "Data",
-   "label": "Email",
-   "reqd": 1
-  },
   {
    "fieldname": "rating",
    "fieldtype": "Float",
@@ -56,11 +50,18 @@
    "label": "Reference Name",
    "options": "reference_doctype",
    "reqd": 1
+  },
+  {
+   "fieldname": "ip_address",
+   "fieldtype": "Data",
+   "hidden": 1,
+   "label": "IP Address",
+   "read_only": 1
   }
  ],
  "index_web_pages_for_search": 1,
  "links": [],
- "modified": "2021-06-14 15:11:26.005805",
+ "modified": "2021-06-23 12:45:42.045696",
  "modified_by": "Administrator",
  "module": "Core",
  "name": "Feedback",

frappe/core/doctype/feedback/test_feedback.py

@@ -9,19 +9,19 @@ class TestFeedback(unittest.TestCase):
 		from frappe.website.doctype.blog_post.test_blog_post import make_test_blog
 		test_blog = make_test_blog()
 
-		frappe.db.sql("delete from `tabFeedback` where reference_doctype = 'Blog Post'")
+		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','test@test.com')
+		feedback = add_feedback('Blog Post', test_blog.name, 5, 'New feedback')
 
 		self.assertEqual(feedback.feedback, 'New feedback')
 		self.assertEqual(feedback.rating, 5)
 
-		updated_feedback = update_feedback('Blog Post', test_blog.name, 6, 'Updated feedback', 'test@test.com')
+		updated_feedback = update_feedback('Blog Post', test_blog.name, 6, 'Updated feedback')
 
 		self.assertEqual(updated_feedback.feedback, 'Updated feedback')
 		self.assertEqual(updated_feedback.rating, 6)
 
-		frappe.db.sql("delete from `tabFeedback` where reference_doctype = 'Blog Post'")
+		frappe.db.delete("Feedback", {"reference_doctype": "Blog Post"})
 
 		test_blog.delete()

frappe/core/doctype/file/file.py

@@ -21,11 +21,11 @@ import zipfile
 import requests
 import requests.exceptions
 from PIL import Image, ImageFile, ImageOps
-from io import StringIO
+from io import BytesIO
 from urllib.parse import quote, unquote
 
 import frappe
-from frappe import _, conf
+from frappe import _, conf, safe_decode
 from frappe.model.document import Document
 from frappe.utils import call_hook_method, cint, cstr, encode, get_files_path, get_hook_method, random_string, strip
 from frappe.utils.image import strip_exif_data, optimize_image
@@ -257,8 +257,7 @@ class File(Document):
 			with open(get_files_path(file_name, is_private=self.is_private), "rb") as f:
 				self.content_hash = get_content_hash(f.read())
 		except IOError:
-			frappe.msgprint(_("File {0} does not exist").format(self.file_url))
-			raise
+			frappe.throw(_("File {0} does not exist").format(self.file_url))
 
 	def on_trash(self):
 		if self.is_home_folder or self.is_attachments_folder:
@@ -270,16 +269,12 @@ class File(Document):
 
 	def make_thumbnail(self, set_as_thumbnail=True, width=300, height=300, suffix="small", crop=False):
 		if self.file_url:
-			if self.file_url.startswith("/files"):
-				try:
+			try:
+				if self.file_url.startswith(("/files", "/private/files")):
 					image, filename, extn = get_local_image(self.file_url)
-				except IOError:
-					return
-
-			else:
-				try:
+				else:
 					image, filename, extn = get_web_image(self.file_url)
-				except (requests.exceptions.HTTPError, requests.exceptions.SSLError, IOError, TypeError):
+			except (requests.exceptions.HTTPError, requests.exceptions.SSLError, IOError, TypeError):
 					return
 
 			size = width, height
@@ -289,16 +284,13 @@ class File(Document):
 				image.thumbnail(size, Image.ANTIALIAS)
 
 			thumbnail_url = filename + "_" + suffix + "." + extn
-
 			path = os.path.abspath(frappe.get_site_path("public", thumbnail_url.lstrip("/")))
 
 			try:
 				image.save(path)
-
 				if set_as_thumbnail:
 					self.db_set("thumbnail_url", thumbnail_url)
 
-				self.db_set("thumbnail_url", thumbnail_url)
 			except IOError:
 				frappe.msgprint(_("Unable to write file format for {0}").format(path))
 				return
@@ -326,12 +318,10 @@ class File(Document):
 
 	def unzip(self):
 		'''Unzip current file and replace it by its children'''
-		if not ".zip" in self.file_name:
-			frappe.msgprint(_("Not a zip file"))
-			return
+		if not self.file_url.endswith(".zip"):
+			frappe.throw(_("{0} is not a zip file").format(self.file_name))
 
-		zip_path = frappe.get_site_path(self.file_url.strip('/'))
-		base_url = os.path.dirname(self.file_url)
+		zip_path = self.get_full_path()
 
 		files = []
 		with zipfile.ZipFile(zip_path) as z:
@@ -359,10 +349,6 @@ class File(Document):
 		return files
 
 
-	def get_file_url(self):
-		data = frappe.db.get_value("File", self.file_data_name, ["file_name", "file_url"], as_dict=True)
-		return data.file_url or data.file_name
-
 	def exists_on_disk(self):
 		exists = os.path.exists(self.get_full_path())
 		return exists
@@ -431,47 +417,6 @@ class File(Document):
 
 		return get_files_path(self.file_name, is_private=self.is_private)
 
-	def get_file_doc(self):
-		'''returns File object (Document) from given parameters or form_dict'''
-		r = frappe.form_dict
-
-		if self.file_url is None: self.file_url = r.file_url
-		if self.file_name is None: self.file_name = r.file_name
-		if self.attached_to_doctype is None: self.attached_to_doctype = r.doctype
-		if self.attached_to_name is None: self.attached_to_name = r.docname
-		if self.attached_to_field is None: self.attached_to_field = r.docfield
-		if self.folder is None: self.folder = r.folder
-		if self.is_private is None: self.is_private = r.is_private
-
-		if r.filedata:
-			file_doc = self.save_uploaded()
-
-		elif r.file_url:
-			file_doc = self.save()
-
-		return file_doc
-
-
-	def save_uploaded(self):
-		self.content = self.get_uploaded_content()
-		if self.content:
-			return self.save()
-		else:
-			raise Exception
-
-	def get_uploaded_content(self):
-		# should not be unicode when reading a file, hence using frappe.form
-		if 'filedata' in frappe.form_dict:
-			if "," in frappe.form_dict.filedata:
-				frappe.form_dict.filedata = frappe.form_dict.filedata.rsplit(",", 1)[1]
-			frappe.uploaded_content = base64.b64decode(frappe.form_dict.filedata)
-			return frappe.uploaded_content
-		elif self.content:
-			return self.content
-		frappe.msgprint(_('No file attached'))
-		return None
-
-
 	def save_file(self, content=None, decode=False, ignore_existing_file_check=False):
 		file_exists = False
 		self.content = content
@@ -539,14 +484,6 @@ class File(Document):
 			'file_url': self.file_url
 		}
 
-	def get_file_data_from_hash(self):
-		for name in frappe.db.sql_list("select name from `tabFile` where content_hash=%s and is_private=%s",
-			(self.content_hash, self.is_private)):
-			b = frappe.get_doc('File', name)
-			return {k: b.get(k) for k in frappe.get_hooks()['write_file_keys']}
-		return False
-
-
 	def check_max_file_size(self):
 		max_file_size = get_max_file_size()
 		file_size = len(self.content)
@@ -621,7 +558,8 @@ def create_new_folder(file_name, folder):
 	file.file_name = file_name
 	file.is_folder = 1
 	file.folder = folder
-	file.insert()
+	file.insert(ignore_if_duplicate=True)
+	return file
 
 @frappe.whitelist()
 def move_file(file_list, new_parent, old_parent):
@@ -672,7 +610,7 @@ def get_local_image(file_url):
 	try:
 		image = Image.open(file_path)
 	except IOError:
-		frappe.msgprint(_("Unable to read file format for {0}").format(file_url), raise_exception=True)
+		frappe.throw(_("Unable to read file format for {0}").format(file_url))
 
 	content = None
 
@@ -704,7 +642,7 @@ def get_web_image(file_url):
 		raise
 
 	try:
-		image = Image.open(StringIO(frappe.safe_decode(r.content)))
+		image = Image.open(BytesIO(r.content))
 	except Exception as e:
 		frappe.msgprint(_("Image link '{0}' is not valid").format(file_url), raise_exception=e)
 
@@ -740,48 +678,12 @@ def delete_file(path):
 			os.remove(path)
 
 
-def remove_file(fid=None, attached_to_doctype=None, attached_to_name=None, from_delete=False, delete_permanently=False):
-	"""Remove file and File entry"""
-	file_name = None
-	if not (attached_to_doctype and attached_to_name):
-		attached = frappe.db.get_value("File", fid,
-			["attached_to_doctype", "attached_to_name", "file_name"])
-		if attached:
-			attached_to_doctype, attached_to_name, file_name = attached
-
-	ignore_permissions, comment = False, None
-	if attached_to_doctype and attached_to_name and not from_delete:
-		doc = frappe.get_doc(attached_to_doctype, attached_to_name)
-		ignore_permissions = doc.has_permission("write") or False
-		if frappe.flags.in_web_form:
-			ignore_permissions = True
-		if not file_name:
-			file_name = frappe.db.get_value("File", fid, "file_name")
-		comment = doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name))
-		frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions, delete_permanently=delete_permanently)
-
-	return comment
 
 
 def get_max_file_size():
 	return cint(conf.get('max_file_size')) or 10485760
 
 
-def remove_all(dt, dn, from_delete=False, delete_permanently=False):
-	"""remove all files in a transaction"""
-	try:
-		for fid in frappe.db.sql_list("""select name from `tabFile` where
-			attached_to_doctype=%s and attached_to_name=%s""", (dt, dn)):
-			if from_delete:
-				# If deleting a doc, directly delete files
-				frappe.delete_doc("File", fid, ignore_permissions=True, delete_permanently=delete_permanently)
-			else:
-				# Removes file and adds a comment in the document it is attached to
-				remove_file(fid=fid, attached_to_doctype=dt, attached_to_name=dn,
-					from_delete=from_delete, delete_permanently=delete_permanently)
-	except Exception as e:
-		if e.args[0]!=1054: raise # (temp till for patched)
-
 
 def has_permission(doc, ptype=None, user=None):
 	has_access = False
@@ -827,6 +729,7 @@ def remove_file_by_url(file_url, doctype=None, name=None):
 		fid = frappe.db.get_value("File", {"file_url": file_url})
 
 	if fid:
+		from frappe.utils.file_manager import remove_file
 		return remove_file(fid=fid)
 
 
@@ -886,15 +789,13 @@ def extract_images_from_html(doc, content):
 		if b"," in content:
 			content = content.split(b",")[1]
 		content = base64.b64decode(content)
-		
+
 		content = optimize_image(content, mtype)
 
 		if "filename=" in headers:
 			filename = headers.split("filename=")[-1]
+			filename = safe_decode(filename).split(";")[0]
 
-			# decode filename
-			if not isinstance(filename, str):
-				filename = str(filename, 'utf-8')
 		else:
 			filename = get_random_filename(content_type=mtype)
 
@@ -922,12 +823,9 @@ def extract_images_from_html(doc, content):
 	return content
 
 
-def get_random_filename(extn=None, content_type=None):
-	if extn:
-		if not extn.startswith("."):
-			extn = "." + extn
-
-	elif content_type:
+def get_random_filename(content_type=None):
+	extn = None
+	if content_type:
 		extn = mimetypes.guess_extension(content_type)
 
 	return random_string(7) + (extn or "")
@@ -938,7 +836,7 @@ def unzip_file(name):
 	'''Unzip the given file and make file records for each of the extracted files'''
 	file_obj = frappe.get_doc('File', name)
 	files = file_obj.unzip()
-	return len(files)
+	return files
 
 @frappe.whitelist()
 def optimize_saved_image(doc_name):
@@ -979,13 +877,6 @@ def get_attached_images(doctype, names):
 	return out
 
 
-@frappe.whitelist()
-def validate_filename(filename):
-	from frappe.utils import now_datetime
-	timestamp = now_datetime().strftime(" %Y-%m-%d %H:%M:%S")
-	fname = get_file_name(filename, timestamp)
-	return fname
-
 @frappe.whitelist()
 def get_files_in_folder(folder, start=0, page_length=20):
 	start = cint(start)

frappe/core/doctype/file/test_file.py

@@ -2,11 +2,12 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 # See license.txt
 import base64
+import json
 import frappe
 import os
 import unittest
 from frappe import _
-from frappe.core.doctype.file.file import move_file, get_files_in_folder
+from frappe.core.doctype.file.file import get_attached_images, move_file, get_files_in_folder, unzip_file
 from frappe.utils import get_files_path
 # test_records = frappe.get_test_records('File')
 
@@ -365,6 +366,80 @@ class TestFile(unittest.TestCase):
 		file1.file_url = '/private/files/parent_dir2.txt'
 		file1.save()
 
+	def test_file_url_validation(self):
+		test_file = frappe.get_doc({
+			"doctype": "File",
+			"file_name": 'logo',
+			"file_url": 'https://frappe.io/files/frappe.png'
+		})
+
+		self.assertIsNone(test_file.validate())
+
+		# bad path
+		test_file.file_url = "/usr/bin/man"
+		self.assertRaisesRegex(frappe.exceptions.ValidationError, "URL must start with http:// or https://", test_file.validate)
+
+		test_file.file_url = None
+		test_file.file_name = "/usr/bin/man"
+		self.assertRaisesRegex(frappe.exceptions.ValidationError, "There is some problem with the file url", test_file.validate)
+
+		test_file.file_url = None
+		test_file.file_name = "_file"
+		self.assertRaisesRegex(IOError, "does not exist", test_file.validate)
+
+		test_file.file_url = None
+		test_file.file_name = "/private/files/_file"
+		self.assertRaisesRegex(IOError, "does not exist", test_file.validate)
+
+	def test_make_thumbnail(self):
+		# test web image
+		test_file = frappe.get_doc({
+			"doctype": "File",
+			"file_name": 'logo',
+			"file_url": frappe.utils.get_url('/_test/assets/image.jpg'),
+		}).insert(ignore_permissions=True)
+
+		test_file.make_thumbnail()
+		self.assertEquals(test_file.thumbnail_url, '/files/image_small.jpg')
+
+		# test local image
+		test_file.db_set('thumbnail_url', None)
+		test_file.reload()
+		test_file.file_url = "/files/image_small.jpg"
+		test_file.make_thumbnail(suffix="xs", crop=True)
+		self.assertEquals(test_file.thumbnail_url, '/files/image_small_xs.jpg')
+
+		frappe.clear_messages()
+		test_file.db_set('thumbnail_url', None)
+		test_file.reload()
+		test_file.file_url = frappe.utils.get_url('unknown.jpg')
+		test_file.make_thumbnail(suffix="xs")
+		self.assertEqual(json.loads(frappe.message_log[0]), {"message": f"File '{frappe.utils.get_url('unknown.jpg')}' not found"})
+		self.assertEquals(test_file.thumbnail_url, None)
+
+	def test_file_unzip(self):
+		file_path = frappe.get_app_path('frappe', 'www/_test/assets/file.zip')
+		public_file_path = frappe.get_site_path('public', 'files')
+		try:
+			import shutil
+			shutil.copy(file_path, public_file_path)
+		except Exception:
+			pass
+
+		test_file = frappe.get_doc({
+			"doctype": "File",
+			"file_url": '/files/file.zip',
+		}).insert(ignore_permissions=True)
+
+		self.assertListEqual([file.file_name for file in unzip_file(test_file.name)],
+			['css_asset.css', 'image.jpg', 'js_asset.min.js'])
+
+		test_file = frappe.get_doc({
+			"doctype": "File",
+			"file_url": frappe.utils.get_url('/_test/assets/image.jpg'),
+		}).insert(ignore_permissions=True)
+		self.assertRaisesRegex(frappe.exceptions.ValidationError, 'not a zip file', test_file.unzip)
+
 class TestAttachment(unittest.TestCase):
 	test_doctype = 'Test For Attachment'
 
@@ -469,3 +544,28 @@ class TestAttachmentsAccess(unittest.TestCase):
 
 		frappe.set_user('Administrator')
 		frappe.db.rollback()
+
+
+class TestFileUtils(unittest.TestCase):
+	def test_extract_images_from_doc(self):
+		# with filename in data URI
+		todo = frappe.get_doc({
+			"doctype": "ToDo",
+			"description": 'Test <img src="data:image/png;filename=pix.png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=">'
+		}).insert()
+		self.assertTrue(frappe.db.exists("File", {"attached_to_name": todo.name}))
+		self.assertIn('<img src="/files/pix.png">', todo.description)
+		self.assertListEqual(get_attached_images('ToDo', [todo.name])[todo.name], ['/files/pix.png'])
+
+		# without filename in data URI
+		todo = frappe.get_doc({
+			"doctype": "ToDo",
+			"description": 'Test <img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=">'
+		}).insert()
+		filename = frappe.db.exists("File", {"attached_to_name": todo.name})
+		self.assertIn(f'<img src="{frappe.get_doc("File", filename).file_url}', todo.description)
+
+	def test_create_new_folder(self):
+		from frappe.core.doctype.file.file import create_new_folder
+		folder = create_new_folder('test_folder', 'Home')
+		self.assertTrue(folder.is_folder)

frappe/core/doctype/prepared_report/prepared_report.py

@@ -11,8 +11,6 @@ from frappe.desk.query_report import generate_report_result
 from frappe.model.document import Document
 from frappe.utils import gzip_compress, gzip_decompress
 from frappe.utils.background_jobs import enqueue
-from frappe.core.doctype.file.file import remove_all
-
 
 class PreparedReport(Document):
 	def before_insert(self):

frappe/core/doctype/role_profile/role_profile.py

@@ -3,6 +3,7 @@
 # For license information, please see license.txt
 
 from frappe.model.document import Document
+import frappe
 
 class RoleProfile(Document):
 	def autoname(self):
@@ -11,5 +12,9 @@ class RoleProfile(Document):
 
 	def on_update(self):
 		""" Changes in role_profile reflected across all its user """
-		from frappe.core.doctype.user.user import update_roles
-		update_roles(self.name)
+		users = frappe.get_all('User', filters={'role_profile_name': self.name})
+		roles = [role.role for role in self.roles]
+		for d in users:
+			user = frappe.get_doc('User', d)
+			user.set('roles', [])
+			user.add_roles(*roles)

frappe/core/doctype/role_profile/test_role_profile.py

@@ -8,6 +8,7 @@ test_dependencies = ['Role']
 
 class TestRoleProfile(unittest.TestCase):
 	def test_make_new_role_profile(self):
+		frappe.delete_doc_if_exists('Role Profile', 'Test 1', force=1)
 		new_role_profile = frappe.get_doc(dict(doctype='Role Profile', role_profile='Test 1')).insert()
 
 		self.assertEqual(new_role_profile.role_profile, 'Test 1')
@@ -19,7 +20,25 @@ class TestRoleProfile(unittest.TestCase):
 		new_role_profile.save()
 		self.assertEqual(new_role_profile.roles[0].role, '_Test Role 2')
 
+		# user with a role profile
+		random_user = frappe.mock("email")
+		random_user_name = frappe.mock("name")
+
+		random_user = frappe.get_doc({
+			"doctype": "User",
+			"email": random_user,
+			"enabled": 1,
+			"first_name": random_user_name,
+			"new_password": "Eastern_43A1W",
+			"role_profile_name": 'Test 1'
+		}).insert(ignore_permissions=True, ignore_if_duplicate=True)
+		self.assertListEqual([role.role for role in random_user.roles], [role.role for role in new_role_profile.roles])
+
 		# clear roles
 		new_role_profile.roles = []
 		new_role_profile.save()
 		self.assertEqual(new_role_profile.roles, [])
+
+		# user roles with the role profile should also be updated
+		random_user.reload()
+		self.assertListEqual(random_user.roles, [])

frappe/core/doctype/translation/test_translation.py

@@ -8,7 +8,7 @@ from frappe import _
 
 class TestTranslation(unittest.TestCase):
 	def setUp(self):
-		frappe.db.sql('delete from tabTranslation')
+		frappe.db.delete("Translation")
 
 	def tearDown(self):
 		frappe.local.lang = 'en'

frappe/core/doctype/user/test_records.json

@@ -70,5 +70,19 @@
     "role": "System Manager"
    }
   ]
- }
+ },
+ {
+    "doctype": "User",
+    "email": "testpassword@example.com",
+    "enabled": 1,
+    "first_name": "_Test",
+    "new_password": "Eastern_43A1W",
+    "roles": [
+     {
+      "doctype": "Has Role",
+      "parentfield": "roles",
+      "role": "System Manager"
+     }
+    ]
+   }
 ]

frappe/core/doctype/user/test_user.py

@@ -1,16 +1,18 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 # MIT License. See license.txt
-import frappe, unittest, uuid
+import json
+import unittest
+from unittest.mock import patch
 
-from frappe.model.delete_doc import delete_doc
-from frappe.utils.data import today, add_to_date
-from frappe import _dict
-from frappe.utils import get_url
-from frappe.core.doctype.user.user import get_total_users
-from frappe.core.doctype.user.user import MaxUsersReachedError, test_password_strength
-from frappe.core.doctype.user.user import extract_mentions
+import frappe
+import frappe.exceptions
+from frappe.core.doctype.user.user import (extract_mentions, reset_password,
+	sign_up, test_password_strength, update_password, verify_password)
 from frappe.frappeclient import FrappeClient
+from frappe.model.delete_doc import delete_doc
+from frappe.utils import get_url
 
+user_module = frappe.core.doctype.user.user
 test_records = frappe.get_test_records('User')
 
 class TestUser(unittest.TestCase):
@@ -23,7 +25,7 @@ class TestUser(unittest.TestCase):
 
 	def test_user_type(self):
 		new_user = frappe.get_doc(dict(doctype='User', email='test-for-type@example.com',
-			first_name='Tester')).insert()
+			first_name='Tester')).insert(ignore_if_duplicate=True)
 		self.assertEqual(new_user.user_type, 'Website User')
 
 		# social login userid for frappe
@@ -52,7 +54,7 @@ class TestUser(unittest.TestCase):
 	def test_delete(self):
 		frappe.get_doc("User", "test@example.com").add_roles("_Test Role 2")
 		self.assertRaises(frappe.LinkExistsError, delete_doc, "Role", "_Test Role 2")
-		frappe.db.sql("""delete from `tabHas Role` where role='_Test Role 2'""")
+		frappe.db.delete("Has Role", {"role": "_Test Role 2"})
 		delete_doc("Role","_Test Role 2")
 
 		if frappe.db.exists("User", "_test@example.com"):
@@ -119,40 +121,9 @@ class TestUser(unittest.TestCase):
 		# system manager now added by Administrator
 		self.assertTrue("System Manager" in [d.role for d in me.get("roles")])
 
-	# def test_deny_multiple_sessions(self):
-	#	from frappe.installer import update_site_config
-	# 	clear_limit('users')
-	#
-	# 	# allow one session
-	# 	user = frappe.get_doc('User', 'test@example.com')
-	# 	user.simultaneous_sessions = 1
-	# 	user.new_password = 'Eastern_43A1W'
-	# 	user.save()
-	#
-	# 	def test_request(conn):
-	# 		value = conn.get_value('User', 'first_name', {'name': 'test@example.com'})
-	# 		self.assertTrue('first_name' in value)
-	#
-	# 	from frappe.frappeclient import FrappeClient
-	# 	update_site_config('deny_multiple_sessions', 0)
-	#
-	# 	conn1 = FrappeClient(get_url(), "test@example.com", "Eastern_43A1W", verify=False)
-	# 	test_request(conn1)
-	#
-	# 	conn2 = FrappeClient(get_url(), "test@example.com", "Eastern_43A1W", verify=False)
-	# 	test_request(conn2)
-	#
-	# 	update_site_config('deny_multiple_sessions', 1)
-	# 	conn3 = FrappeClient(get_url(), "test@example.com", "Eastern_43A1W", verify=False)
-	# 	test_request(conn3)
-	#
-	# 	# first connection should fail
-	# 	test_request(conn1)
-
-
 	def test_delete_user(self):
 		new_user = frappe.get_doc(dict(doctype='User', email='test-for-delete@example.com',
-			first_name='Tester Delete User')).insert()
+			first_name='Tester Delete User')).insert(ignore_if_duplicate=True)
 		self.assertEqual(new_user.user_type, 'Website User')
 
 		# role with desk access
@@ -174,7 +145,7 @@ class TestUser(unittest.TestCase):
 		self.assertFalse(frappe.db.exists('User', new_user.name))
 
 	def test_password_strength(self):
-		# Test Password without Password Strenth Policy
+		# Test Password without Password Strength Policy
 		frappe.db.set_value("System Settings", "System Settings", "enable_password_policy", 0)
 
 		# password policy is disabled, test_password_strength should be ignored
@@ -193,6 +164,17 @@ class TestUser(unittest.TestCase):
 		result = test_password_strength("Eastern_43A1W")
 		self.assertEqual(result['feedback']['password_policy_validation_passed'], True)
 
+
+		# test password strength while saving user with new password
+		user = frappe.get_doc("User", "test@example.com")
+		frappe.flags.in_test = False
+		user.new_password = "password"
+		self.assertRaisesRegex(frappe.exceptions.ValidationError, "Invalid Password", user.save)
+		user.reload()
+		user.new_password = "Eastern_43A1W"
+		user.save()
+		frappe.flags.in_test = True
+
 	def test_comment_mentions(self):
 		comment = '''
 			<span class="mention" data-id="test.comment@example.com" data-value="Test" data-denotation-char="@">
@@ -227,6 +209,7 @@ class TestUser(unittest.TestCase):
 		self.assertEqual(extract_mentions(comment)[0], "test_user@example.com")
 		self.assertEqual(extract_mentions(comment)[1], "test.again@example1.com")
 
+		frappe.delete_doc("User Group", "Team")
 		doc = frappe.get_doc({
 			'doctype': 'User Group',
 			'name': 'Team',
@@ -236,14 +219,18 @@ class TestUser(unittest.TestCase):
 				'user': 'test1@example.com'
 			}]
 		})
-		doc.insert(ignore_if_duplicate=True)
+
+		doc.insert()
 
 		comment = '''
 			<div>
 				Testing comment for
 				<span class="mention" data-id="Team" data-value="Team" data-is-group="true" data-denotation-char="@">
 					<span><span class="ql-mention-denotation-char">@</span>Team</span>
-				</span>
+				</span> and
+				<span class="mention" data-id="Unknown Team" data-value="Unknown Team" data-is-group="true" data-denotation-char="@">
+					<span><span class="ql-mention-denotation-char">@</span>Unknown Team</span>
+				</span><!-- this should be ignored-->
 				please check
 			</div>
 		'''
@@ -267,32 +254,125 @@ class TestUser(unittest.TestCase):
 		self.assertEqual(res1.status_code, 200)
 		self.assertEqual(res2.status_code, 417)
 
-	# def test_user_rollback(self):
-	# 	"""
-	#	FIXME: This is failing with PR #12693 as Rollback can't happen if notifications sent on user creation.
-	#	Make sure that notifications disabled.
-	# 	"""
-	# 	frappe.db.commit()
-	# 	frappe.db.begin()
-	# 	user_id = str(uuid.uuid4())
-	# 	email = f'{user_id}@example.com'
-	# 	try:
-	# 		frappe.flags.in_import = True  # disable throttling
-	# 		frappe.get_doc(dict(
-	# 			doctype='User',
-	# 			email=email,
-	# 			first_name=user_id,
-	# 		)).insert()
-	# 	finally:
-	# 		frappe.flags.in_import = False
+	def test_user_rename(self):
+		old_name = "test_user_rename@example.com"
+		new_name = "test_user_rename_new@example.com"
+		user = frappe.get_doc({
+			"doctype": "User",
+			"email": old_name,
+			"enabled": 1,
+			"first_name": "_Test",
+			"new_password": "Eastern_43A1W",
+			"roles": [
+				{
+					"doctype": "Has Role",
+					"parentfield": "roles",
+					"role": "System Manager"
+				}]
+		}).insert(ignore_permissions=True, ignore_if_duplicate=True)
 
-	# 	# Check user has been added
-	# 	self.assertIsNotNone(frappe.db.get("User", {"email": email}))
+		frappe.rename_doc('User', user.name, new_name)
+		self.assertTrue(frappe.db.exists("Notification Settings", new_name))
+
+		frappe.delete_doc("User", new_name)
+
+	def test_signup(self):
+		import frappe.website.utils
+		random_user = frappe.mock('email')
+		random_user_name = frappe.mock('name')
+		# disabled signup
+		with patch.object(user_module, "is_signup_disabled", return_value=True):
+			self.assertRaisesRegex(frappe.exceptions.ValidationError, "Sign Up is disabled",
+				sign_up, random_user, random_user_name, "/signup")
+
+		self.assertTupleEqual(sign_up(random_user, random_user_name, "/welcome"), (1, "Please check your email for verification"))
+		self.assertEqual(frappe.cache().hget('redirect_after_login', random_user), "/welcome")
+
+		# re-register
+		self.assertTupleEqual(sign_up(random_user, random_user_name, "/welcome"), (0, "Already Registered"))
+
+		# disabled user
+		user = frappe.get_doc("User", random_user)
+		user.enabled = 0
+		user.save()
+
+		self.assertTupleEqual(sign_up(random_user, random_user_name, "/welcome"), (0, "Registered but disabled"))
+
+		# throttle user creation
+		with patch.object(user_module.frappe.db, "get_creation_count", return_value=301):
+			self.assertRaisesRegex(frappe.exceptions.ValidationError, "Throttled",
+				sign_up, frappe.mock('email'), random_user_name, "/signup")
+
+
+	def test_reset_password(self):
+		from frappe.auth import CookieManager, LoginManager
+		from frappe.utils import set_request
+		old_password = "Eastern_43A1W"
+		new_password = "easy_password"
+
+		set_request(path="/random")
+		frappe.local.cookie_manager = CookieManager()
+		frappe.local.login_manager = LoginManager()
+
+		frappe.set_user("testpassword@example.com")
+		test_user = frappe.get_doc("User", "testpassword@example.com")
+		test_user.reset_password()
+		self.assertEqual(update_password(new_password, key=test_user.reset_password_key), "/app")
+		self.assertEqual(update_password(new_password, key="wrong_key"), "The Link specified has either been used before or Invalid")
+
+		# password verification should fail with old password
+		self.assertRaises(frappe.exceptions.AuthenticationError, verify_password, old_password)
+		verify_password(new_password)
+
+		# reset password
+		update_password(old_password, old_password=new_password)
+
+		self.assertRaisesRegex(frappe.exceptions.ValidationError, "Invalid key type", update_password, "test", 1, ['like', '%'])
+
+		password_strength_response = {
+			"feedback": {
+				"password_policy_validation_passed": False,
+				"suggestions": ["Fix password"]
+			}
+		}
+
+		# password strength failure test
+		with patch.object(user_module, "test_password_strength", return_value=password_strength_response):
+			self.assertRaisesRegex(frappe.exceptions.ValidationError, "Fix password", update_password, new_password, 0, test_user.reset_password_key)
+
+
+		# test redirect URL for website users
+		frappe.set_user("test2@example.com")
+		self.assertEqual(update_password(new_password, old_password=old_password), "/")
+		# reset password
+		update_password(old_password, old_password=new_password)
+
+		# test API endpoint
+		with patch.object(user_module.frappe, 'sendmail') as sendmail:
+			frappe.clear_messages()
+			test_user = frappe.get_doc("User", "test2@example.com")
+			self.assertEqual(reset_password(user="test2@example.com"), None)
+			test_user.reload()
+			self.assertEqual(update_password(new_password, key=test_user.reset_password_key), "/")
+			update_password(old_password, old_password=new_password)
+			self.assertEqual(json.loads(frappe.message_log[0]), {"message": "Password reset instructions have been sent to your email"})
+		sendmail.assert_called_once()
+		self.assertEqual(sendmail.call_args[1]["recipients"], "test2@example.com")
+
+		self.assertEqual(reset_password(user="test2@example.com"), None)
+		self.assertEqual(reset_password(user="Administrator"), "not allowed")
+		self.assertEqual(reset_password(user="random"), "not found")
+
+	def test_user_onload_modules(self):
+		from frappe.config import get_modules_from_all_apps
+		from frappe.desk.form.load import getdoc
+		frappe.response.docs = []
+		getdoc("User", "Administrator")
+		doc = frappe.response.docs[0]
+		self.assertListEqual(doc.get("__onload").get('all_modules', []),
+			[m.get("module_name") for m in get_modules_from_all_apps()])
 
-	# 	# Check that rollback works
-	# 	frappe.db.rollback()
-	# 	self.assertIsNone(frappe.db.get("User", {"email": email}))
 
 def delete_contact(user):
-	frappe.db.sql("DELETE FROM `tabContact` WHERE `email_id`= %s", user)
-	frappe.db.sql("DELETE FROM `tabContact Email` WHERE `email_id`= %s", user)
+	frappe.db.delete("Contact", {"email_id": user})
+	frappe.db.delete("Contact Email", {"email_id": user})

frappe/core/doctype/user/user.js

@@ -166,7 +166,7 @@ frappe.ui.form.on('User', {
 
 			frm.add_custom_button(__("Reset OTP Secret"), function() {
 				frappe.call({
-					method: "frappe.core.doctype.user.user.reset_otp_secret",
+					method: "frappe.twofactor.reset_otp_secret",
 					args: {
 						"user": frm.doc.name
 					}

frappe/core/doctype/user/user.py

@@ -15,17 +15,11 @@ from frappe.desk.doctype.notification_settings.notification_settings import crea
 from frappe.utils.user import get_system_managers
 from frappe.website.utils import is_signup_disabled
 from frappe.rate_limiter import rate_limit
-from frappe.utils.background_jobs import enqueue
 from frappe.core.doctype.user_type.user_type import user_linked_with_permission_on_doctype
 
 
 STANDARD_USERS = ("Guest", "Administrator")
 
-
-class MaxUsersReachedError(frappe.ValidationError):
-	pass
-
-
 class User(Document):
 	__new_password = None
 
@@ -56,8 +50,6 @@ class User(Document):
 		frappe.cache().delete_key('enabled_users')
 
 	def validate(self):
-		self.check_demo()
-
 		# clear new password
 		self.__new_password = self.new_password
 		self.new_password = ""
@@ -137,10 +129,6 @@ class User(Document):
 		"""Returns true if current user is the session user"""
 		return self.name == frappe.session.user
 
-	def check_demo(self):
-		if frappe.session.user == 'demo@erpnext.com':
-			frappe.throw(_('Cannot change user details in demo. Please signup for a new account at https://erpnext.com'), title=_('Not Allowed'))
-
 	def set_full_name(self):
 		self.full_name = " ".join(filter(None, [self.first_name, self.last_name]))
 
@@ -398,7 +386,6 @@ class User(Document):
 
 
 	def before_rename(self, old_name, new_name, merge=False):
-		self.check_demo()
 		frappe.clear_cache(user=old_name)
 		self.validate_rename(old_name, new_name)
 
@@ -718,85 +705,6 @@ def get_email_awaiting(user):
 				where parent = %(user)s""",{"user":user})
 		return False
 
-@frappe.whitelist(allow_guest=False)
-def set_email_password(email_account, user, password):
-	account = frappe.get_doc("Email Account", email_account)
-	if account.awaiting_password:
-		account.awaiting_password = 0
-		account.password = password
-		try:
-			account.save(ignore_permissions=True)
-		except Exception:
-			frappe.db.rollback()
-			return False
-
-	return True
-
-def setup_user_email_inbox(email_account, awaiting_password, email_id, enable_outgoing):
-	""" setup email inbox for user """
-	def add_user_email(user):
-		user = frappe.get_doc("User", user)
-		row = user.append("user_emails", {})
-
-		row.email_id = email_id
-		row.email_account = email_account
-		row.awaiting_password = awaiting_password or 0
-		row.enable_outgoing = enable_outgoing or 0
-
-		user.save(ignore_permissions=True)
-
-	udpate_user_email_settings = False
-	if not all([email_account, email_id]):
-		return
-
-	user_names = frappe.db.get_values("User", { "email": email_id }, as_dict=True)
-	if not user_names:
-		return
-
-	for user in user_names:
-		user_name = user.get("name")
-
-		# check if inbox is alreay configured
-		user_inbox = frappe.db.get_value("User Email", {
-			"email_account": email_account,
-			"parent": user_name
-		}, ["name"]) or None
-
-		if not user_inbox:
-			add_user_email(user_name)
-		else:
-			# update awaiting password for email account
-			udpate_user_email_settings = True
-
-	if udpate_user_email_settings:
-		frappe.db.sql("""UPDATE `tabUser Email` SET awaiting_password = %(awaiting_password)s,
-			enable_outgoing = %(enable_outgoing)s WHERE email_account = %(email_account)s""", {
-				"email_account": email_account,
-				"enable_outgoing": enable_outgoing,
-				"awaiting_password": awaiting_password or 0
-			})
-	else:
-		users = " and ".join([frappe.bold(user.get("name")) for user in user_names])
-		frappe.msgprint(_("Enabled email inbox for user {0}").format(users))
-
-	ask_pass_update()
-
-def remove_user_email_inbox(email_account):
-	""" remove user email inbox settings if email account is deleted """
-	if not email_account:
-		return
-
-	users = frappe.get_all("User Email", filters={
-		"email_account": email_account
-	}, fields=["parent as name"])
-
-	for user in users:
-		doc = frappe.get_doc("User", user.get("name"))
-		to_remove = [ row for row in doc.user_emails if row.email_account == email_account ]
-		[ doc.remove(row) for row in to_remove ]
-
-		doc.save(ignore_permissions=True)
-
 def ask_pass_update():
 	# update the sys defaults as to awaiting users
 	from frappe.utils import set_default
@@ -809,24 +717,19 @@ def ask_pass_update():
 
 def _get_user_for_update_password(key, old_password):
 	# verify old password
+	result = frappe._dict()
 	if key:
-		user = frappe.db.get_value("User", {"reset_password_key": key})
-		if not user:
-			return {
-				'message': _("The Link specified has either been used before or Invalid")
-			}
+		result.user = frappe.db.get_value("User", {"reset_password_key": key})
+		if not result.user:
+			result.message = _("The Link specified has either been used before or Invalid")
 
 	elif old_password:
 		# verify old password
 		frappe.local.login_manager.check_password(frappe.session.user, old_password)
 		user = frappe.session.user
+		result.user = user
 
-	else:
-		return
-
-	return {
-		'user': user
-	}
+	return result
 
 def reset_user_data(user):
 	user_doc = frappe.get_doc("User", user)
@@ -848,14 +751,12 @@ def sign_up(email, full_name, redirect_to):
 
 	user = frappe.db.get("User", {"email": email})
 	if user:
-		if user.disabled:
-			return 0, _("Registered but disabled")
-		else:
+		if user.enabled:
 			return 0, _("Already Registered")
+		else:
+			return 0, _("Registered but disabled")
 	else:
-		if frappe.db.sql("""select count(*) from tabUser where
-			HOUR(TIMEDIFF(CURRENT_TIMESTAMP, TIMESTAMP(modified)))=1""")[0][0] > 300:
-
+		if frappe.db.get_creation_count('User', 60) > 300:
 			frappe.respond_as_web_page(_('Temporarily Disabled'),
 				_('Too many users signed up recently, so the registration is disabled. Please try back in an hour'),
 				http_status_code=429)
@@ -1048,91 +949,6 @@ def update_gravatar(name):
 	if gravatar:
 		frappe.db.set_value('User', name, 'user_image', gravatar)
 
-@frappe.whitelist(allow_guest=True)
-def send_token_via_sms(tmp_id,phone_no=None,user=None):
-	try:
-		from frappe.core.doctype.sms_settings.sms_settings import send_request
-	except:
-		return False
-
-	if not frappe.cache().ttl(tmp_id + '_token'):
-		return False
-	ss = frappe.get_doc('SMS Settings', 'SMS Settings')
-	if not ss.sms_gateway_url:
-		return False
-
-	token = frappe.cache().get(tmp_id + '_token')
-	args = {ss.message_parameter: 'verification code is {}'.format(token)}
-
-	for d in ss.get("parameters"):
-		args[d.parameter] = d.value
-
-	if user:
-		user_phone = frappe.db.get_value('User', user, ['phone','mobile_no'], as_dict=1)
-		usr_phone = user_phone.mobile_no or user_phone.phone
-		if not usr_phone:
-			return False
-	else:
-		if phone_no:
-			usr_phone = phone_no
-		else:
-			return False
-
-	args[ss.receiver_parameter] = usr_phone
-	status = send_request(ss.sms_gateway_url, args, use_post=ss.use_post)
-
-	if 200 <= status < 300:
-		frappe.cache().delete(tmp_id + '_token')
-		return True
-	else:
-		return False
-
-@frappe.whitelist(allow_guest=True)
-def send_token_via_email(tmp_id,token=None):
-	import pyotp
-
-	user = frappe.cache().get(tmp_id + '_user')
-	count = token or frappe.cache().get(tmp_id + '_token')
-
-	if ((not user) or (user == 'None') or (not count)):
-		return False
-	user_email = frappe.db.get_value('User',user, 'email')
-	if not user_email:
-		return False
-
-	otpsecret = frappe.cache().get(tmp_id + '_otp_secret')
-	hotp = pyotp.HOTP(otpsecret)
-
-	frappe.sendmail(
-		recipients=user_email,
-		sender=None,
-		subject="Verification Code",
-		template="verification_code",
-		args=dict(code=hotp.at(int(count))),
-		delayed=False,
-		retry=3
-	)
-
-	return True
-
-@frappe.whitelist(allow_guest=True)
-def reset_otp_secret(user):
-	otp_issuer = frappe.db.get_value('System Settings', 'System Settings', 'otp_issuer_name')
-	user_email = frappe.db.get_value('User',user, 'email')
-	if frappe.session.user in ["Administrator", user] :
-		frappe.defaults.clear_default(user + '_otplogin')
-		frappe.defaults.clear_default(user + '_otpsecret')
-		email_args = {
-			'recipients':user_email, 'sender':None, 'subject':'OTP Secret Reset - {}'.format(otp_issuer or "Frappe Framework"),
-			'message':'<p>Your OTP secret on {} has been reset. If you did not perform this reset and did not request it, please contact your System Administrator immediately.</p>'.format(otp_issuer or "Frappe Framework"),
-			'delayed':False,
-			'retry':3
-		}
-		enqueue(method=frappe.sendmail, queue='short', timeout=300, event=None, is_async=True, job_name=None, now=False, **email_args)
-		return frappe.msgprint(_("OTP Secret has been reset. Re-registration will be required on next login."))
-	else:
-		return frappe.throw(_("OTP secret can only be reset by the Administrator."))
-
 def throttle_user_creation():
 	if frappe.flags.in_import:
 		return
@@ -1150,15 +966,6 @@ def get_module_profile(module_profile):
 	module_profile = frappe.get_doc('Module Profile', {'module_profile_name': module_profile})
 	return module_profile.get('block_modules')
 
-def update_roles(role_profile):
-	users = frappe.get_all('User', filters={'role_profile_name': role_profile})
-	role_profile = frappe.get_doc('Role Profile', role_profile)
-	roles = [role.role for role in role_profile.roles]
-	for d in users:
-		user = frappe.get_doc('User', d)
-		user.set('roles', [])
-		user.add_roles(*roles)
-
 def create_contact(user, ignore_links=False, ignore_mandatory=False):
 	from frappe.contacts.doctype.contact.contact import get_contact_name
 	if user.name in ["Administrator", "Guest"]: return
@@ -1217,18 +1024,18 @@ def generate_keys(user):
 
 	:param user: str
 	"""
-	if "System Manager" in frappe.get_roles():
-		user_details = frappe.get_doc("User", user)
-		api_secret = frappe.generate_hash(length=15)
-		# if api key is not set generate api key
-		if not user_details.api_key:
-			api_key = frappe.generate_hash(length=15)
-			user_details.api_key = api_key
-		user_details.api_secret = api_secret
-		user_details.save()
+	frappe.only_for("System Manager")
+	user_details = frappe.get_doc("User", user)
+	api_secret = frappe.generate_hash(length=15)
+	# if api key is not set generate api key
+	if not user_details.api_key:
+		api_key = frappe.generate_hash(length=15)
+		user_details.api_key = api_key
+	user_details.api_secret = api_secret
+	user_details.save()
+
+	return {"api_secret": api_secret}
 
-		return {"api_secret": api_secret}
-	frappe.throw(frappe._("Not Permitted"), frappe.PermissionError)
 
 @frappe.whitelist()
 def switch_theme(theme):

frappe/desk/desktop.py

@@ -368,7 +368,7 @@ def get_desktop_page(page):
 	on desk.
 
 	Args:
-		page (string): page name
+		page (json): page data
 
 	Returns:
 		dict: dictionary of cards, charts and shortcuts to be displayed on website

frappe/desk/doctype/event/test_event.py

@@ -14,7 +14,7 @@ test_records = frappe.get_test_records('Event')
 
 class TestEvent(unittest.TestCase):
 	def setUp(self):
-		frappe.db.sql('delete from tabEvent')
+		frappe.db.delete("Event")
 		make_test_objects('Event', reset=True)
 
 		self.test_records = frappe.get_test_records('Event')

frappe/desk/doctype/note/test_note.py

@@ -8,9 +8,9 @@ test_records = frappe.get_test_records('Note')
 
 class TestNote(unittest.TestCase):
 	def insert_note(self):
-		frappe.db.sql('delete from tabVersion')
-		frappe.db.sql('delete from tabNote')
-		frappe.db.sql('delete from `tabNote Seen By`')
+		frappe.db.delete("Version")
+		frappe.db.delete("Note")
+		frappe.db.delete("Note Seen By")
 
 		return frappe.get_doc(dict(doctype='Note', title='test note',
 			content='test note content')).insert()

frappe/desk/doctype/notification_log/test_notification_log.py

@@ -2,6 +2,7 @@
 # Copyright (c) 2019, Frappe Technologies and Contributors
 # See license.txt
 import frappe
+from frappe.core.doctype.user.user import get_system_users
 from frappe.desk.form.assign_to import add as assign_task
 import unittest
 
@@ -54,7 +55,4 @@ def get_todo():
 	return frappe.get_cached_doc('ToDo', res[0].name)
 
 def get_user():
-	users = frappe.db.get_all('User',
-		filters={'name': ('not in', ['Administrator', 'Guest'])},
-		fields='name', limit=1)
-	return users[0].name
+	return get_system_users(limit=1)[0]

frappe/desk/doctype/system_console/system_console.js

@@ -5,7 +5,7 @@ frappe.ui.form.on('System Console', {
 	onload: function(frm) {
 		frappe.ui.keys.add_shortcut({
 			shortcut: 'shift+enter',
-			action: () => frm.execute_action('Execute'),
+			action: () => frm.page.btn_primary.trigger('click'),
 			page: frm.page,
 			description: __('Execute Console script'),
 			ignore_inputs: true,
@@ -14,8 +14,11 @@ frappe.ui.form.on('System Console', {
 
 	refresh: function(frm) {
 		frm.disable_save();
-		frm.page.set_primary_action(__("Execute"), () => {
-			frm.execute_action('Execute');
+		frm.page.set_primary_action(__("Execute"), $btn => {
+			$btn.text(__('Executing...'));
+			return frm.execute_action("Execute").then(() => {
+				$btn.text(__('Execute'));
+			});
 		});
 	}
 });

frappe/desk/doctype/tag/test_tag.py

@@ -6,7 +6,7 @@ from frappe.desk.doctype.tag.tag import add_tag
 
 class TestTag(unittest.TestCase):
 	def setUp(self) -> None:
-		frappe.db.sql("DELETE from `tabTag`")
+		frappe.db.delete("Tag")
 		frappe.db.sql("UPDATE `tabDocType` set _user_tags=''")
 
 	def test_tag_count_query(self):

frappe/desk/doctype/todo/test_todo.py

@@ -14,7 +14,7 @@ class TestToDo(unittest.TestCase):
 		todo = frappe.get_doc(dict(doctype='ToDo', description='test todo',
 			assigned_by='Administrator')).insert()
 
-		frappe.db.sql('delete from `tabDeleted Document`')
+		frappe.db.delete("Deleted Document")
 		todo.delete()
 
 		deleted = frappe.get_doc('Deleted Document', dict(deleted_doctype=todo.doctype, deleted_name=todo.name))
@@ -27,7 +27,7 @@ class TestToDo(unittest.TestCase):
 			frappe.db.get_value('User', todo.assigned_by, 'full_name'))
 
 	def test_fetch_setup(self):
-		frappe.db.sql('delete from tabToDo')
+		frappe.db.delete("ToDo")
 
 		todo_meta = frappe.get_doc('DocType', 'ToDo')
 		todo_meta.get('fields', dict(fieldname='assigned_by_full_name'))[0].fetch_from = ''
@@ -104,8 +104,8 @@ class TestToDo(unittest.TestCase):
 		clear_permissions_cache('ToDo')
 		frappe.db.rollback()
 
-def test_fetch_if_empty(self):
-		frappe.db.sql('delete from tabToDo')
+	def test_fetch_if_empty(self):
+		frappe.db.delete("ToDo")
 
 		# Allow user changes
 		todo_meta = frappe.get_doc('DocType', 'ToDo')
@@ -122,9 +122,8 @@ def test_fetch_if_empty(self):
 		self.assertEqual(todo.assigned_by_full_name, 'Admin')
 
 		# Overwrite user changes
-		todo_meta = frappe.get_doc('DocType', 'ToDo')
-		todo_meta.get('fields', dict(fieldname='assigned_by_full_name'))[0].fetch_if_empty = 0
-		todo_meta.save()
+		todo.meta.get('fields', dict(fieldname='assigned_by_full_name'))[0].fetch_if_empty = 0
+		todo.meta.save()
 
 		todo.reload()
 		todo.save()

frappe/desk/doctype/todo/todo.py

@@ -29,8 +29,15 @@ class ToDo(Document):
 		else:
 			# NOTE the previous value is only available in validate method
 			if self.get_db_value("status") != self.status:
+				if self.owner == frappe.session.user:
+					removal_message = frappe._("{0} removed their assignment.").format(
+						get_fullname(frappe.session.user))
+				else:
+					removal_message = frappe._("Assignment of {0} removed by {1}").format(
+						get_fullname(self.owner), get_fullname(frappe.session.user))
+
 				self._assignment = {
-					"text": frappe._("Assignment closed by {0}").format(get_fullname(frappe.session.user)),
+					"text": removal_message,
 					"comment_type": "Assignment Completed"
 				}
 

frappe/desk/doctype/workspace/test_workspace.py

@@ -1,8 +1,95 @@
 # -*- coding: utf-8 -*-
 # Copyright (c) 2020, Frappe Technologies and Contributors
 # See license.txt
-# import frappe
+import frappe
 import unittest
-
 class TestWorkspace(unittest.TestCase):
-	pass
+	def setUp(self):
+		create_module("Test Module")
+
+	def tearDown(self):
+		frappe.db.delete("Workspace", {"module": "Test Module"})
+		frappe.db.delete("DocType", {"module": "Test Module"})
+		frappe.delete_doc("Module Def", "Test Module")
+
+	# TODO: FIX ME - flaky test!!!
+	# def test_workspace_with_cards_specific_to_a_country(self):
+	# 	workspace = create_workspace()
+	# 	insert_card(workspace, "Card Label 1", "DocType 1", "DocType 2", "France")
+	# 	insert_card(workspace, "Card Label 2", "DocType A", "DocType B")
+
+	# 	workspace.insert(ignore_if_duplicate = True)
+
+	# 	cards = workspace.get_link_groups()
+
+	# 	if frappe.get_system_settings('country') == "France":
+	# 		self.assertEqual(len(cards), 2)
+	# 	else:
+	# 		self.assertEqual(len(cards), 1)
+
+def create_module(module_name):
+	module = frappe.get_doc({
+		"doctype": "Module Def",
+		"module_name": module_name,
+		"app_name": "frappe"
+	})
+	module.insert(ignore_if_duplicate = True)
+
+	return module
+
+def create_workspace(**args):
+	workspace = frappe.new_doc("Workspace")
+	args = frappe._dict(args)
+
+	workspace.name = args.name or "Test Workspace"
+	workspace.label = args.label or "Test Workspace"
+	workspace.category = args.category or "Modules"
+	workspace.is_standard = args.is_standard or 1
+	workspace.module = "Test Module"
+
+	return workspace
+
+def insert_card(workspace, card_label, doctype1, doctype2, country=None):
+	workspace.append("links", {
+		"type": "Card Break",
+		"label": card_label,
+		"only_for": country
+	})
+
+	create_doctype(doctype1, "Test Module")
+	workspace.append("links", {
+		"type": "Link",
+		"label": doctype1,
+		"only_for": country,
+		"link_type": "DocType",
+		"link_to": doctype1
+	})
+
+	create_doctype(doctype2, "Test Module")
+	workspace.append("links", {
+		"type": "Link",
+		"label": doctype2,
+		"only_for": country,
+		"link_type": "DocType",
+		"link_to": doctype2
+	})
+
+def create_doctype(doctype_name, module):
+	frappe.get_doc({
+		'doctype': 'DocType',
+		'name': doctype_name,
+		'module': module,
+		'custom': 1,
+		'autoname': 'field:title',
+		'fields': [
+			{'label': 'Title', 'fieldname': 'title', 'reqd': 1, 'fieldtype': 'Data'},
+			{'label': 'Description', 'fieldname': 'description', 'fieldtype': 'Small Text'},
+			{'label': 'Date', 'fieldname': 'date', 'fieldtype': 'Date'},
+			{'label': 'Duration', 'fieldname': 'duration', 'fieldtype': 'Duration'},
+			{'label': 'Number', 'fieldname': 'number', 'fieldtype': 'Int'},
+			{'label': 'Number', 'fieldname': 'another_number', 'fieldtype': 'Int'}
+		],
+		'permissions': [
+			{'role': 'System Manager'}
+		]
+	}).insert(ignore_if_duplicate = True)

frappe/desk/doctype/workspace/workspace.json

@@ -28,7 +28,6 @@
   "pin_to_bottom",
   "hide_custom",
   "public",
-  "content_section",
   "content",
   "section_break_2",
   "charts_label",
@@ -39,6 +38,7 @@
   "section_break_18",
   "cards_label",
   "links",
+  "roles_section",
   "roles"
  ],
  "fields": [
@@ -46,6 +46,7 @@
    "fieldname": "label",
    "fieldtype": "Data",
    "label": "Name",
+   "reqd": 1,
    "unique": 1
   },
   {
@@ -232,21 +233,18 @@
   {
    "fieldname": "title",
    "fieldtype": "Data",
-   "label": "Title"
+   "label": "Title",
+   "reqd": 1
   },
   {
    "fieldname": "parent_page",
    "fieldtype": "Data",
    "label": "Parent Page"
   },
-  {
-   "fieldname": "content_section",
-   "fieldtype": "Section Break",
-   "label": "Content"
-  },
   {
    "fieldname": "content",
    "fieldtype": "Long Text",
+   "hidden": 1,
    "label": "Content"
   },
   {
@@ -259,10 +257,15 @@
    "fieldtype": "Table",
    "label": "Roles",
    "options": "Has Role"
+  },
+  {
+   "fieldname": "roles_section",
+   "fieldtype": "Section Break",
+   "label": "Roles"
   }
  ],
  "links": [],
- "modified": "2021-08-05 11:49:09.028243",
+ "modified": "2021-08-19 12:51:00.233017",
  "modified_by": "Administrator",
  "module": "Desk",
  "name": "Workspace",

frappe/desk/doctype/workspace/workspace.py

@@ -17,6 +17,12 @@ class Workspace(Document):
 			frappe.throw(_("You need to be in developer mode to edit this document"))
 		validate_route_conflict(self.doctype, self.name)
 
+		try:
+			if not isinstance(loads(self.content), list):
+				raise
+		except Exception:
+			frappe.throw(_("Content data shoud be a list"))
+
 		duplicate_exists = frappe.db.exists("Workspace", {
 			"name": ["!=", self.name], 'is_default': 1, 'extends': self.extends
 		})
@@ -56,7 +62,7 @@ class Workspace(Document):
 		for link in self.links:
 			link = link.as_dict()
 			if link.type == "Card Break":
-				if card_links and (not current_card.only_for or current_card.only_for == frappe.get_system_settings('country')):
+				if card_links and (not current_card['only_for'] or current_card['only_for'] == frappe.get_system_settings('country')): 
 					current_card['links'] = card_links
 					cards.append(current_card)
 

frappe/desk/form/utils.py

@@ -5,7 +5,7 @@ import frappe, json
 import frappe.desk.form.meta
 import frappe.desk.form.load
 from frappe.desk.form.document_follow import follow_document
-from frappe.utils.file_manager import extract_images_from_html
+from frappe.core.doctype.file.file import extract_images_from_html
 
 from frappe import _
 

frappe/desk/search.py

@@ -265,6 +265,7 @@ def get_users_for_mentions():
 			'name': ['not in', ('Administrator', 'Guest')],
 			'allowed_in_mentions': True,
 			'user_type': 'System User',
+			'enabled': True,
 		})
 
 def get_user_groups():

frappe/email/doctype/email_account/email_account.py

@@ -137,8 +137,6 @@ class EmailAccount(Document):
 
 	def on_update(self):
 		"""Check there is only one default of each type."""
-		from frappe.core.doctype.user.user import setup_user_email_inbox
-
 		self.check_automatic_linking_email_account()
 		self.there_must_be_only_one_default()
 		setup_user_email_inbox(email_account=self.name, awaiting_password=self.awaiting_password,
@@ -532,8 +530,6 @@ class EmailAccount(Document):
 
 	def on_trash(self):
 		"""Clear communications where email account is linked"""
-		from frappe.core.doctype.user.user import remove_user_email_inbox
-
 		frappe.db.sql("update `tabCommunication` set email_account='' where email_account=%s", self.name)
 		remove_user_email_inbox(email_account=self.name)
 
@@ -724,3 +720,84 @@ def get_max_email_uid(email_account):
 	else:
 		max_uid = cint(result[0].get("uid", 0)) + 1
 		return max_uid
+
+
+def setup_user_email_inbox(email_account, awaiting_password, email_id, enable_outgoing):
+	""" setup email inbox for user """
+	from frappe.core.doctype.user.user import ask_pass_update
+
+	def add_user_email(user):
+		user = frappe.get_doc("User", user)
+		row = user.append("user_emails", {})
+
+		row.email_id = email_id
+		row.email_account = email_account
+		row.awaiting_password = awaiting_password or 0
+		row.enable_outgoing = enable_outgoing or 0
+
+		user.save(ignore_permissions=True)
+
+	update_user_email_settings = False
+	if not all([email_account, email_id]):
+		return
+
+	user_names = frappe.db.get_values("User", {"email": email_id}, as_dict=True)
+	if not user_names:
+		return
+
+	for user in user_names:
+		user_name = user.get("name")
+
+		# check if inbox is alreay configured
+		user_inbox = frappe.db.get_value("User Email", {
+			"email_account": email_account,
+			"parent": user_name
+		}, ["name"]) or None
+
+		if not user_inbox:
+			add_user_email(user_name)
+		else:
+			# update awaiting password for email account
+			update_user_email_settings = True
+
+	if update_user_email_settings:
+		frappe.db.sql("""UPDATE `tabUser Email` SET awaiting_password = %(awaiting_password)s,
+			enable_outgoing = %(enable_outgoing)s WHERE email_account = %(email_account)s""", {
+				"email_account": email_account,
+				"enable_outgoing": enable_outgoing,
+				"awaiting_password": awaiting_password or 0
+			})
+	else:
+		users = " and ".join([frappe.bold(user.get("name")) for user in user_names])
+		frappe.msgprint(_("Enabled email inbox for user {0}").format(users))
+	ask_pass_update()
+
+def remove_user_email_inbox(email_account):
+	""" remove user email inbox settings if email account is deleted """
+	if not email_account:
+		return
+
+	users = frappe.get_all("User Email", filters={
+		"email_account": email_account
+	}, fields=["parent as name"])
+
+	for user in users:
+		doc = frappe.get_doc("User", user.get("name"))
+		to_remove = [row for row in doc.user_emails if row.email_account == email_account]
+		[doc.remove(row) for row in to_remove]
+
+		doc.save(ignore_permissions=True)
+
+@frappe.whitelist(allow_guest=False)
+def set_email_password(email_account, user, password):
+	account = frappe.get_doc("Email Account", email_account)
+	if account.awaiting_password:
+		account.awaiting_password = 0
+		account.password = password
+		try:
+			account.save(ignore_permissions=True)
+		except Exception:
+			frappe.db.rollback()
+			return False
+
+	return True

frappe/email/doctype/email_account/test_email_account.py

@@ -34,8 +34,8 @@ class TestEmailAccount(unittest.TestCase):
 	def setUp(self):
 		frappe.flags.mute_emails = False
 		frappe.flags.sent_mail = None
-		frappe.db.sql('delete from `tabEmail Queue`')
-		frappe.db.sql('delete from `tabUnhandled Email`')
+		frappe.db.delete("Email Queue")
+		frappe.db.delete("Unhandled Email")
 
 	def get_test_mail(self, fname):
 		with open(os.path.join(os.path.dirname(__file__), "test_mails", fname), "r") as f:
@@ -60,7 +60,7 @@ class TestEmailAccount(unittest.TestCase):
 		comm = frappe.get_doc("Communication", {"sender": "test_sender@example.com"})
 		comm.db_set("creation", datetime.now() - timedelta(seconds = 30 * 60))
 
-		frappe.db.sql("DELETE FROM `tabEmail Queue`")
+		frappe.db.delete("Email Queue")
 		notify_unreplied()
 		self.assertTrue(frappe.db.get_value("Email Queue", {"reference_doctype": comm.reference_doctype,
 			"reference_name": comm.reference_name, "status":"Not Sent"}))
@@ -183,7 +183,7 @@ class TestEmailAccount(unittest.TestCase):
 
 	def test_threading_by_message_id(self):
 		cleanup()
-		frappe.db.sql("""delete from `tabEmail Queue`""")
+		frappe.db.delete("Email Queue")
 
 		# reference document for testing
 		event = frappe.get_doc(dict(doctype='Event', subject='test-message')).insert()
@@ -242,8 +242,8 @@ class TestInboundMail(unittest.TestCase):
 
 	def setUp(self):
 		cleanup()
-		frappe.db.sql('delete from `tabEmail Queue`')
-		frappe.db.sql('delete from `tabToDo`')
+		frappe.db.delete("Email Queue")
+		frappe.db.delete("ToDo")
 
 	def get_test_mail(self, fname):
 		with open(os.path.join(os.path.dirname(__file__), "test_mails", fname), "r") as f:

frappe/email/doctype/newsletter/exceptions.py

@@ -0,0 +1,13 @@
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
+# MIT License. See LICENSE
+
+from frappe.exceptions import ValidationError
+
+class NewsletterAlreadySentError(ValidationError):
+	pass
+
+class NoRecipientFoundError(ValidationError):
+	pass
+
+class NewsletterNotSavedError(ValidationError):
+	pass

frappe/email/doctype/newsletter/newsletter.py

@@ -1,241 +1,323 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
-# License: GNU General Public License v3. See license.txt
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
+# MIT License. See LICENSE
+
+from typing import Dict, List
 
 import frappe
 import frappe.utils
-from frappe import throw, _
+
+from frappe import _
 from frappe.website.website_generator import WebsiteGenerator
 from frappe.utils.verified_command import get_signed_params, verify_request
 from frappe.email.doctype.email_group.email_group import add_subscribers
-from frappe.utils import parse_addr, now_datetime, markdown, validate_email_address
+
+from .exceptions import NewsletterAlreadySentError, NoRecipientFoundError, NewsletterNotSavedError
+
 
 class Newsletter(WebsiteGenerator):
 	def onload(self):
-		if self.email_sent:
-			self.get("__onload").status_count = dict(frappe.db.sql("""select status, count(name)
-				from `tabEmail Queue` where reference_doctype=%s and reference_name=%s
-				group by status""", (self.doctype, self.name))) or None
+		self.setup_newsletter_status()
 
 	def validate(self):
-		self.route = "newsletters/" + self.name
-		if self.send_from:
-			validate_email_address(self.send_from, True)
+		self.route = f"newsletters/{self.name}"
+		self.validate_sender_address()
+		self.validate_recipient_address()
+
+	@property
+	def newsletter_recipients(self) -> List[str]:
+		if getattr(self, "_recipients", None) is None:
+			self._recipients = self.get_recipients()
+		return self._recipients
 
 	@frappe.whitelist()
-	def test_send(self, doctype="Lead"):
-		self.recipients = frappe.utils.split_emails(self.test_email_id)
-		self.queue_all(test_email=True)
+	def test_send(self):
+		test_emails = frappe.utils.split_emails(self.test_email_id)
+		self.queue_all(test_emails=test_emails)
 		frappe.msgprint(_("Test email sent to {0}").format(self.test_email_id))
 
 	@frappe.whitelist()
 	def send_emails(self):
 		"""send emails to leads and customers"""
+		self.queue_all()
+		frappe.msgprint(_("Email queued to {0} recipients").format(len(self.newsletter_recipients)))
+
+	def setup_newsletter_status(self):
+		"""Setup analytical status for current Newsletter. Can be accessible from desk.
+		"""
 		if self.email_sent:
-			throw(_("Newsletter has already been sent"))
-
-		self.recipients = self.get_recipients()
-
-		if self.recipients:
-			self.queue_all()
-			frappe.msgprint(_("Email queued to {0} recipients").format(len(self.recipients)))
-
-		else:
-			frappe.msgprint(_("Newsletter should have atleast one recipient"))
-
-	def queue_all(self, test_email=False):
-		if not self.get("recipients"):
-			# in case it is called via worker
-			self.recipients = self.get_recipients()
-
-		self.validate_send()
-
-		sender = self.send_from or frappe.utils.get_formatted_email(self.owner)
-
-		if not frappe.flags.in_test:
-			frappe.db.auto_commit_on_many_writes = True
-
-		attachments = []
-		if self.send_attachments:
-			files = frappe.get_all("File", fields=["name"], filters={"attached_to_doctype": "Newsletter",
-				"attached_to_name": self.name}, order_by="creation desc")
-
-			for file in files:
-				try:
-					# these attachments will be attached on-demand
-					# and won't be stored in the message
-					attachments.append({"fid": file.name})
-				except IOError:
-					frappe.throw(_("Unable to find attachment {0}").format(file.name))
-
-		args = {
-			"message": self.get_message(),
-			"name": self.name
-		}
-		frappe.sendmail(recipients=self.recipients, sender=sender,
-			subject=self.subject, message=self.get_message(), template="newsletter",
-			reference_doctype=self.doctype, reference_name=self.name,
-			add_unsubscribe_link=self.send_unsubscribe_link, attachments=attachments,
-			unsubscribe_method="/unsubscribe",
-			unsubscribe_params={"name": self.name},
-			send_priority=0, queue_separately=True, args=args)
-
-		if not frappe.flags.in_test:
-			frappe.db.auto_commit_on_many_writes = False
-
-		if not test_email:
-			self.db_set("email_sent", 1)
-			self.db_set("schedule_send", now_datetime())
-			self.db_set("scheduled_to_send", len(self.recipients))
-
-	def get_message(self):
-		if self.content_type == "HTML":
-			return frappe.render_template(self.message_html, {"doc": self.as_dict()})
-		return {
-			'Rich Text': self.message,
-			'Markdown': markdown(self.message_md)
-		}[self.content_type or 'Rich Text']
-
-	def get_recipients(self):
-		"""Get recipients from Email Group"""
-		recipients_list = []
-		for email_group in get_email_groups(self.name):
-			for d in frappe.db.get_all("Email Group Member", ["email"],
-				{"unsubscribed": 0, "email_group": email_group.email_group}):
-					recipients_list.append(d.email)
-		return list(set(recipients_list))
+			status_count = frappe.get_all("Email Queue",
+				filters={"reference_doctype": self.doctype, "reference_name": self.name},
+				fields=["status", "count(name)"],
+				group_by="status",
+				order_by="status",
+				as_list=True,
+			)
+			self.get("__onload").status_count = dict(status_count)
 
 	def validate_send(self):
-		if self.get("__islocal"):
-			throw(_("Please save the Newsletter before sending"))
+		"""Validate if Newsletter can be sent.
+		"""
+		self.validate_newsletter_status()
+		self.validate_newsletter_recipients()
 
-		if not self.recipients:
-			frappe.throw(_("Newsletter should have at least one recipient"))
+	def validate_newsletter_status(self):
+		if self.email_sent:
+			frappe.throw(_("Newsletter has already been sent"), exc=NewsletterAlreadySentError)
+
+		if self.get("__islocal"):
+			frappe.throw(_("Please save the Newsletter before sending"), exc=NewsletterNotSavedError)
+
+	def validate_newsletter_recipients(self):
+		if not self.newsletter_recipients:
+			frappe.throw(_("Newsletter should have atleast one recipient"), exc=NoRecipientFoundError)
+		self.validate_recipient_address()
+
+	def validate_sender_address(self):
+		"""Validate self.send_from is a valid email address or not.
+		"""
+		if self.send_from:
+			frappe.utils.validate_email_address(self.send_from, throw=True)
+
+	def validate_recipient_address(self):
+		"""Validate if self.newsletter_recipients are all valid email addresses or not.
+		"""
+		for recipient in self.newsletter_recipients:
+			frappe.utils.validate_email_address(recipient, throw=True)
+
+	def get_linked_email_queue(self) -> List[str]:
+		"""Get list of email queue linked to this newsletter.
+		"""
+		return frappe.get_all("Email Queue",
+			filters={
+				"reference_doctype": self.doctype,
+				"reference_name": self.name,
+			},
+			pluck="name",
+		)
+
+	def get_success_recipients(self) -> List[str]:
+		"""Recipients who have already recieved the newsletter.
+
+		Couldn't think of a better name ;)
+		"""
+		return frappe.get_all("Email Queue Recipient",
+			filters={
+				"status": ("in", ["Not Sent", "Sending", "Sent"]),
+				"parentfield": ("in", self.get_linked_email_queue()),
+			},
+			pluck="recipient",
+		)
+
+	def get_pending_recipients(self) -> List[str]:
+		"""Get list of pending recipients of the newsletter. These
+		recipients may not have receive the newsletter in the previous iteration.
+		"""
+		return [
+			x for x in self.newsletter_recipients if x not in self.get_success_recipients()
+		]
+
+	def queue_all(self, test_emails: List[str] = None):
+		"""Queue Newsletter to all the recipients generated from the `Email Group`
+		table
+
+		Args:
+			test_email (List[str], optional): Send test Newsletter to the passed set of emails.
+			Defaults to None.
+		"""
+		if test_emails:
+			for test_email in test_emails:
+				frappe.utils.validate_email_address(test_email, throw=True)
+		else:
+			self.validate()
+			self.validate_send()
+
+		newsletter_recipients = test_emails or self.get_pending_recipients()
+		self.send_newsletter(emails=newsletter_recipients)
+
+		if not test_emails:
+			self.email_sent = True
+			self.schedule_send = frappe.utils.now_datetime()
+			self.scheduled_to_send = len(newsletter_recipients)
+			self.save()
+
+	def get_newsletter_attachments(self) -> List[Dict[str, str]]:
+		"""Get list of attachments on current Newsletter
+		"""
+		attachments = []
+
+		if self.send_attachments:
+			files = frappe.get_all(
+				"File",
+				filters={"attached_to_doctype": "Newsletter", "attached_to_name": self.name},
+				order_by="creation desc",
+				pluck="name",
+			)
+			attachments.extend({"fid": file} for file in files)
+
+		return attachments
+
+	def send_newsletter(self, emails: List[str]):
+		"""Trigger email generation for `emails` and add it in Email Queue.
+		"""
+		# TODO: get rid of this maybe?
+		message = self.get_message()
+		attachments = self.get_newsletter_attachments()
+		sender = self.send_from or frappe.utils.get_formatted_email(self.owner)
+		args = {"message": message, "name": self.name}
+
+		is_auto_commit_set = bool(frappe.db.auto_commit_on_many_writes)
+		frappe.db.auto_commit_on_many_writes = not frappe.flags.in_test
+
+		frappe.sendmail(
+			subject=self.subject,
+			sender=sender,
+			recipients=emails,
+			message=message,
+			attachments=attachments,
+			template="newsletter",
+			add_unsubscribe_link=self.send_unsubscribe_link,
+			unsubscribe_method="/unsubscribe",
+			unsubscribe_params={"name": self.name},
+			reference_doctype=self.doctype,
+			reference_name=self.name,
+			queue_separately=True,
+			send_priority=0,
+			args=args,
+		)
+
+		frappe.db.auto_commit_on_many_writes = is_auto_commit_set
+
+	def get_message(self) -> str:
+		if self.content_type == "HTML":
+			return frappe.render_template(self.message_html, {"doc": self.as_dict()})
+		if self.content_type == "Markdown":
+			return frappe.utils.markdown(self.message_md)
+		# fallback to Rich Text
+		return self.message
+
+	def get_recipients(self) -> List[str]:
+		"""Get recipients from Email Group"""
+		emails = frappe.get_all(
+			"Email Group Member",
+			filters={"unsubscribed": 0, "email_group": ("in", self.get_email_groups())},
+			pluck="email",
+		)
+		return list(set(emails))
+
+	def get_email_groups(self) -> List[str]:
+		# wondering why the 'or'? i can't figure out why both aren't equivalent - @gavin
+		return [
+			x.email_group for x in self.email_group
+		] or frappe.get_all(
+			"Newsletter Email Group",
+			filters={"parent": self.name, "parenttype": "Newsletter"},
+			pluck="email_group",
+		)
+
+	def get_attachments(self) -> List[Dict[str, str]]:
+		return frappe.get_all(
+			"File",
+			fields=["name", "file_name", "file_url", "is_private"],
+			filters={
+				"attached_to_name": self.name,
+				"attached_to_doctype": "Newsletter",
+				"is_private": 0,
+			},
+		)
 
 	def get_context(self, context):
 		newsletters = get_newsletter_list("Newsletter", None, None, 0)
 		if newsletters:
 			newsletter_list = [d.name for d in newsletters]
 			if self.name not in newsletter_list:
-				frappe.redirect_to_message(_('Permission Error'),
-					_("You are not permitted to view the newsletter."))
+				frappe.redirect_to_message(
+					_("Permission Error"), _("You are not permitted to view the newsletter.")
+				)
 				frappe.local.flags.redirect_location = frappe.local.response.location
 				raise frappe.Redirect
 			else:
-				context.attachments = get_attachments(self.name)
+				context.attachments = self.get_attachments()
 		context.no_cache = 1
 		context.show_sidebar = True
 
 
-def get_attachments(name):
-	return frappe.get_all("File",
-			fields=["name", "file_name", "file_url", "is_private"],
-			filters = {"attached_to_name": name, "attached_to_doctype": "Newsletter", "is_private":0})
-
-
-def get_email_groups(name):
-	return frappe.db.get_all("Newsletter Email Group", ["email_group"],{"parent":name, "parenttype":"Newsletter"})
-
-
 @frappe.whitelist(allow_guest=True)
 def confirmed_unsubscribe(email, group):
 	""" unsubscribe the email(user) from the mailing list(email_group) """
-	frappe.flags.ignore_permissions=True
+	frappe.flags.ignore_permissions = True
 	doc = frappe.get_doc("Email Group Member", {"email": email, "email_group": group})
 	if not doc.unsubscribed:
 		doc.unsubscribed = 1
-		doc.save(ignore_permissions = True)
-
-def create_lead(email_id):
-	"""create a lead if it does not exist"""
-	from frappe.model.naming import get_default_naming_series
-	full_name, email_id = parse_addr(email_id)
-	if frappe.db.get_value("Lead", {"email_id": email_id}):
-		return
-
-	lead = frappe.get_doc({
-		"doctype": "Lead",
-		"email_id": email_id,
-		"lead_name": full_name or email_id,
-		"status": "Lead",
-		"naming_series": get_default_naming_series("Lead"),
-		"company": frappe.db.get_default("Company"),
-		"source": "Email"
-	})
-	lead.insert()
+		doc.save(ignore_permissions=True)
 
 
 @frappe.whitelist(allow_guest=True)
-def subscribe(email, email_group=_('Website')):
-	url = frappe.utils.get_url("/api/method/frappe.email.doctype.newsletter.newsletter.confirm_subscription") +\
-		"?" + get_signed_params({"email": email, "email_group": email_group})
+def subscribe(email, email_group=_("Website")):
+	"""API endpoint to subscribe an email to a particular email group. Triggers a confirmation email.
+	"""
 
-	email_template = frappe.db.get_value('Email Group', email_group, ['confirmation_email_template'])
+	# build subscription confirmation URL
+	api_endpoint = frappe.utils.get_url(
+		"/api/method/frappe.email.doctype.newsletter.newsletter.confirm_subscription"
+	)
+	signed_params = get_signed_params({"email": email, "email_group": email_group})
+	confirm_subscription_url = f"{api_endpoint}?{signed_params}"
 
-	content=''
-	if email_template:
-		args = dict(
-			email=email,
-			confirmation_url=url,
-			email_group=email_group
-		)
+	# fetch custom template if available
+	email_confirmation_template = frappe.db.get_value(
+		"Email Group", email_group, "confirmation_email_template"
+	)
 
-		email_template = frappe.get_doc("Email Template", email_template)
+	# build email and send
+	if email_confirmation_template:
+		args = {"email": email, "confirmation_url": confirm_subscription_url, "email_group": email_group}
+		email_template = frappe.get_doc("Email Template", email_confirmation_template)
+		email_subject = email_template.subject
 		content = frappe.render_template(email_template.response, args)
-
-	if not content:
-		messages = (
+	else:
+		email_subject = _("Confirm Your Email")
+		translatable_content = (
 			_("Thank you for your interest in subscribing to our updates"),
 			_("Please verify your Email Address"),
-			url,
-			_("Click here to verify")
+			confirm_subscription_url,
+			_("Click here to verify"),
 		)
-
 		content = """
-		<p>{0}. {1}.</p>
-		<p><a href="{2}">{3}</a></p>
-		""".format(*messages)
+			<p>{0}. {1}.</p>
+			<p><a href="{2}">{3}</a></p>
+		""".format(*translatable_content)
+
+	frappe.sendmail(
+		email,
+		subject=email_subject,
+		content=content,
+		now=True,
+	)
 
-	frappe.sendmail(email, subject=getattr('email_template', 'subject', '') or _("Confirm Your Email"), content=content, now=True)
 
 @frappe.whitelist(allow_guest=True)
-def confirm_subscription(email, email_group=_('Website')):
+def confirm_subscription(email, email_group=_("Website")):
+	"""API endpoint to confirm email subscription.
+	This endpoint is called when user clicks on the link sent to their mail.
+	"""
 	if not verify_request():
 		return
 
 	if not frappe.db.exists("Email Group", email_group):
-		frappe.get_doc({
-			"doctype": "Email Group",
-			"title": email_group
-		}).insert(ignore_permissions=True)
+		frappe.get_doc({"doctype": "Email Group", "title": email_group}).insert(
+			ignore_permissions=True
+		)
 
 	frappe.flags.ignore_permissions = True
 
 	add_subscribers(email_group, email)
 	frappe.db.commit()
 
-	frappe.respond_as_web_page(_("Confirmed"),
+	frappe.respond_as_web_page(
+		_("Confirmed"),
 		_("{0} has been successfully added to the Email Group.").format(email),
-		indicator_color='green')
-
-
-def send_newsletter(newsletter):
-	try:
-		doc = frappe.get_doc("Newsletter", newsletter)
-		doc.queue_all()
-
-	except:
-		frappe.db.rollback()
-
-		# wasn't able to send emails :(
-		doc.db_set("email_sent", 0)
-		frappe.db.commit()
-
-		frappe.log_error(title='Send Newsletter')
-
-		raise
-
-	else:
-		frappe.db.commit()
+		indicator_color="green",
+	)
 
 
 def get_list_context(context=None):
@@ -268,12 +350,35 @@ def get_newsletter_list(doctype, txt, filters, limit_start, limit_page_length=20
 			'''.format(','.join(['%s'] * len(email_group_list)),
 					limit_page_length, limit_start), email_group_list, as_dict=1)
 
+
 def send_scheduled_email():
 	"""Send scheduled newsletter to the recipients."""
-	scheduled_newsletter = frappe.get_all('Newsletter', filters = {
-		'schedule_send': ('<=', now_datetime()),
-		'email_sent': 0,
-		'schedule_sending': 1
-	}, fields = ['name'], ignore_ifnull=True)
+	scheduled_newsletter = frappe.get_all(
+		"Newsletter",
+		filters={
+			"schedule_send": ("<=", frappe.utils.now_datetime()),
+			"email_sent": False,
+			"schedule_sending": True,
+		},
+		ignore_ifnull=True,
+		pluck="name",
+	)
+
 	for newsletter in scheduled_newsletter:
-		send_newsletter(newsletter.name)
+		try:
+			frappe.get_doc("Newsletter", newsletter).queue_all()
+
+		except Exception:
+			frappe.db.rollback()
+
+			# wasn't able to send emails :(
+			frappe.db.set_value("Newsletter", newsletter, "email_sent", 0)
+			message = (
+				f"Newsletter {newsletter} failed to send"
+				"\n\n"
+				f"Traceback: {frappe.get_traceback()}"
+			)
+			frappe.log_error(title="Send Newsletter", message=message)
+
+		if not frappe.flags.in_test:
+			frappe.db.commit()

frappe/email/doctype/newsletter/test_newsletter.py

@@ -1,17 +1,26 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
-# License: GNU General Public License v3. See license.txt
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
+# MIT License. See LICENSE
+
 import unittest
 from random import choice
+from typing import Union
+from unittest.mock import MagicMock, PropertyMock, patch
 
 import frappe
-from frappe.email.doctype.newsletter.newsletter import (
-	confirmed_unsubscribe,
-	send_scheduled_email,
+from frappe.desk.form.load import run_onload
+from frappe.email.doctype.newsletter.exceptions import (
+	NewsletterAlreadySentError, NoRecipientFoundError
+)
+from frappe.email.doctype.newsletter.newsletter import (
+	Newsletter,
+	confirmed_unsubscribe,
+	get_newsletter_list,
+	send_scheduled_email
 )
-from frappe.email.doctype.newsletter.newsletter import get_newsletter_list
 from frappe.email.queue import flush
 from frappe.utils import add_days, getdate
 
+
 test_dependencies = ["Email Group"]
 emails = [
 	"test_subscriber1@example.com",
@@ -19,23 +28,107 @@ emails = [
 	"test_subscriber3@example.com",
 	"test1@example.com",
 ]
+newsletters = []
 
 
-class TestNewsletter(unittest.TestCase):
+def get_dotted_path(obj: type) -> str:
+	klass = obj.__class__
+	module = klass.__module__
+	if module == 'builtins':
+		return klass.__qualname__ # avoid outputs like 'builtins.str'
+	return f"{module}.{klass.__qualname__}"
+
+
+class TestNewsletterMixin:
 	def setUp(self):
 		frappe.set_user("Administrator")
-		frappe.db.sql("delete from `tabEmail Group Member`")
+		self.setup_email_group()
 
+	def tearDown(self):
+		frappe.set_user("Administrator")
+		for newsletter in newsletters:
+			frappe.db.delete("Email Queue", {
+				"reference_doctype": "Newsletter",
+				"reference_name": newsletter,
+			})
+			frappe.delete_doc("Newsletter", newsletter)
+			frappe.db.delete("Newsletter Email Group", newsletter)
+			newsletters.remove(newsletter)
+
+	def setup_email_group(self):
 		if not frappe.db.exists("Email Group", "_Test Email Group"):
-			frappe.get_doc({"doctype": "Email Group", "title": "_Test Email Group"}).insert()
-
-		for email in emails:
 			frappe.get_doc({
-				"doctype": "Email Group Member",
-				"email": email,
-				"email_group": "_Test Email Group"
+				"doctype": "Email Group",
+				"title": "_Test Email Group"
 			}).insert()
 
+		for email in emails:
+			doctype = "Email Group Member"
+			email_filters = {
+				"email": email,
+				"email_group": "_Test Email Group"
+			}
+			try:
+				frappe.get_doc({
+					"doctype": doctype,
+					**email_filters,
+				}).insert()
+			except Exception:
+				frappe.db.update(doctype, email_filters, "unsubscribed", 0)
+
+	def send_newsletter(self, published=0, schedule_send=None) -> Union[str, None]:
+		frappe.db.delete("Email Queue")
+		frappe.db.delete("Email Queue Recipient")
+		frappe.db.delete("Newsletter")
+
+		newsletter_options = {
+			"published": published,
+			"schedule_sending": bool(schedule_send),
+			"schedule_send": schedule_send
+		}
+		newsletter = self.get_newsletter(**newsletter_options)
+
+		if schedule_send:
+			send_scheduled_email()
+		else:
+			newsletter.send_emails()
+			return newsletter.name
+
+	@staticmethod
+	def get_newsletter(**kwargs) -> "Newsletter":
+		"""Generate and return Newsletter object
+		"""
+		doctype = "Newsletter"
+		newsletter_content = {
+			"subject": "_Test Newsletter",
+			"send_from": "Test Sender <test_sender@example.com>",
+			"content_type": "Rich Text",
+			"message": "Testing my news.",
+		}
+		similar_newsletters = frappe.db.get_all(doctype, newsletter_content, pluck="name")
+
+		for similar_newsletter in similar_newsletters:
+			frappe.delete_doc(doctype, similar_newsletter)
+
+		newsletter = frappe.get_doc({"doctype": doctype, **newsletter_content, **kwargs})
+		newsletter.append("email_group", {"email_group": "_Test Email Group"})
+		newsletter.save(ignore_permissions=True)
+		newsletter.reload()
+		newsletters.append(newsletter.name)
+
+		attached_files = frappe.get_all("File", {
+				"attached_to_doctype": newsletter.doctype,
+				"attached_to_name": newsletter.name,
+			},
+			pluck="name",
+		)
+		for file in attached_files:
+			frappe.delete_doc("File", file)
+
+		return newsletter
+
+
+class TestNewsletter(TestNewsletterMixin, unittest.TestCase):
 	def test_send(self):
 		self.send_newsletter()
 
@@ -64,40 +157,15 @@ class TestNewsletter(unittest.TestCase):
 			if email != to_unsubscribe:
 				self.assertTrue(email in recipients)
 
-	@staticmethod
-	def send_newsletter(published=0, schedule_send=None):
-		frappe.db.sql("delete from `tabEmail Queue`")
-		frappe.db.sql("delete from `tabEmail Queue Recipient`")
-		frappe.db.sql("delete from `tabNewsletter`")
-		newsletter = frappe.get_doc({
-			"doctype": "Newsletter",
-			"subject": "_Test Newsletter",
-			"send_from": "Test Sender <test_sender@example.com>",
-			"content_type": "Rich Text",
-			"message": "Testing my news.",
-			"published": published,
-			"schedule_sending": bool(schedule_send),
-			"schedule_send": schedule_send
-		}).insert(ignore_permissions=True)
-
-		newsletter.append("email_group", {"email_group": "_Test Email Group"})
-		newsletter.save()
-		if schedule_send:
-			send_scheduled_email()
-			return
-
-		newsletter.send_emails()
-		return newsletter.name
-
 	def test_portal(self):
-		self.send_newsletter(1)
+		self.send_newsletter(published=1)
 		frappe.set_user("test1@example.com")
-		newsletters = get_newsletter_list("Newsletter", None, None, 0)
-		self.assertEqual(len(newsletters), 1)
+		newsletter_list = get_newsletter_list("Newsletter", None, None, 0)
+		self.assertEqual(len(newsletter_list), 1)
 
 	def test_newsletter_context(self):
 		context = frappe._dict()
-		newsletter_name = self.send_newsletter(1)
+		newsletter_name = self.send_newsletter(published=1)
 		frappe.set_user("test2@example.com")
 		doc = frappe.get_doc("Newsletter", newsletter_name)
 		doc.get_context(context)
@@ -112,3 +180,68 @@ class TestNewsletter(unittest.TestCase):
 		recipients = [e.recipients[0].recipient for e in email_queue_list]
 		for email in emails:
 			self.assertTrue(email in recipients)
+
+	def test_newsletter_test_send(self):
+		"""Test "Test Send" functionality of Newsletter
+		"""
+		newsletter = self.get_newsletter()
+		newsletter.test_email_id = choice(emails)
+		newsletter.test_send()
+
+		self.assertFalse(newsletter.email_sent)
+		newsletter.save = MagicMock()
+		self.assertFalse(newsletter.save.called)
+
+	def test_newsletter_status(self):
+		"""Test for Newsletter's stats on onload event
+		"""
+		newsletter = self.get_newsletter()
+		newsletter.email_sent = True
+		# had to use run_onload as calling .onload directly bought weird errors
+		# like TestNewsletter has no attribute "_TestNewsletter__onload"
+		run_onload(newsletter)
+		self.assertIsInstance(newsletter.get("__onload").status_count, dict)
+
+	def test_already_sent_newsletter(self):
+		newsletter = self.get_newsletter()
+		newsletter.send_emails()
+
+		with self.assertRaises(NewsletterAlreadySentError):
+			newsletter.send_emails()
+
+	def test_newsletter_with_no_recipient(self):
+		newsletter = self.get_newsletter()
+		property_path = f"{get_dotted_path(newsletter)}.newsletter_recipients"
+
+		with patch(property_path, new_callable=PropertyMock) as mock_newsletter_recipients:
+			mock_newsletter_recipients.return_value = []
+			with self.assertRaises(NoRecipientFoundError):
+				newsletter.send_emails()
+
+	def test_send_newsletter_with_attachments(self):
+		newsletter = self.get_newsletter()
+		newsletter.reload()
+		file_attachment = frappe.get_doc({
+			"doctype": "File",
+			"file_name": "test1.txt",
+			"attached_to_doctype": newsletter.doctype,
+			"attached_to_name": newsletter.name,
+			"content": frappe.mock("paragraph")
+		})
+		file_attachment.save()
+		newsletter.send_attachments = True
+		newsletter_attachments = newsletter.get_newsletter_attachments()
+		self.assertEqual(len(newsletter_attachments), 1)
+		self.assertEqual(newsletter_attachments[0]["fid"], file_attachment.name)
+
+	def test_send_scheduled_email_error_handling(self):
+		newsletter = self.get_newsletter(schedule_send=add_days(getdate(), -1))
+		job_path = "frappe.email.doctype.newsletter.newsletter.Newsletter.queue_all"
+		m = MagicMock(side_effect=frappe.OutgoingEmailError)
+
+		with self.assertRaises(frappe.OutgoingEmailError):
+			with patch(job_path, new_callable=m):
+				send_scheduled_email()
+
+		newsletter.reload()
+		self.assertEqual(newsletter.email_sent, 0)

frappe/email/doctype/notification/test_notification.py

@@ -9,7 +9,7 @@ test_dependencies = ["User", "Notification"]
 
 class TestNotification(unittest.TestCase):
 	def setUp(self):
-		frappe.db.sql("""delete from `tabEmail Queue`""")
+		frappe.db.delete("Email Queue")
 		frappe.set_user("test@example.com")
 
 		if not frappe.db.exists('Notification', {'name': 'ToDo Status Update'}, 'name'):
@@ -50,7 +50,7 @@ class TestNotification(unittest.TestCase):
 
 		self.assertTrue(frappe.db.get_value("Email Queue", {"reference_doctype": "Communication",
 			"reference_name": communication.name, "status":"Not Sent"}))
-		frappe.db.sql("""delete from `tabEmail Queue`""")
+		frappe.db.delete("Email Queue")
 
 		communication.reload()
 		communication.content = "test 2"
@@ -189,9 +189,9 @@ class TestNotification(unittest.TestCase):
 
 	def test_cc_jinja(self):
 
-		frappe.db.sql("""delete from `tabUser` where email='test_jinja@example.com'""")
-		frappe.db.sql("""delete from `tabEmail Queue`""")
-		frappe.db.sql("""delete from `tabEmail Queue Recipient`""")
+		frappe.db.delete("User", {"email": "test_jinja@example.com"})
+		frappe.db.delete("Email Queue")
+		frappe.db.delete("Email Queue Recipient")
 
 		test_user = frappe.new_doc("User")
 		test_user.name = 'test_jinja'
@@ -205,9 +205,9 @@ class TestNotification(unittest.TestCase):
 
 		self.assertTrue(frappe.db.get_value("Email Queue Recipient", {"recipient": "test_jinja@example.com"}))
 
-		frappe.db.sql("""delete from `tabUser` where email='test_jinja@example.com'""")
-		frappe.db.sql("""delete from `tabEmail Queue`""")
-		frappe.db.sql("""delete from `tabEmail Queue Recipient`""")
+		frappe.db.delete("User", {"email": "test_jinja@example.com"})
+		frappe.db.delete("Email Queue")
+		frappe.db.delete("Email Queue Recipient")
 
 	def test_notification_to_assignee(self):
 		todo = frappe.new_doc('ToDo')

frappe/integrations/doctype/webhook/test_webhook.py

@@ -11,9 +11,9 @@ class TestWebhook(unittest.TestCase):
 	@classmethod
 	def setUpClass(cls):
 		# delete any existing webhooks
-		frappe.db.sql("DELETE FROM tabWebhook")
+		frappe.db.delete("Webhook")
 		# Delete existing logs if any
-		frappe.db.sql("DELETE FROM `tabWebhook Request Log`")
+		frappe.db.delete("Webhook Request Log")
 		# create test webhooks
 		cls.create_sample_webhooks()
 
@@ -46,7 +46,7 @@ class TestWebhook(unittest.TestCase):
 	@classmethod
 	def tearDownClass(cls):
 		# delete any existing webhooks
-		frappe.db.sql("DELETE FROM tabWebhook")
+		frappe.db.delete("Webhook")
 
 	def setUp(self):
 		# retrieve or create a User webhook for `after_insert`
@@ -168,7 +168,7 @@ class TestWebhook(unittest.TestCase):
 	def test_webhook_req_log_creation(self):
 		if not frappe.db.get_value('User', 'user2@integration.webhooks.test.com'):
 			user = frappe.get_doc({
-				'doctype': 'User', 
+				'doctype': 'User',
 				'email': 'user2@integration.webhooks.test.com',
 				'first_name': 'user2'
 			}).insert()

frappe/model/delete_doc.py

@@ -10,7 +10,7 @@ import frappe.model.meta
 from frappe import _
 from frappe import get_module_path
 from frappe.model.dynamic_links import get_dynamic_link_map
-from frappe.core.doctype.file.file import remove_all
+from frappe.utils.file_manager import remove_all
 from frappe.utils.password import delete_all_passwords_for
 from frappe.model.naming import revert_series_if_last
 from frappe.utils.global_search import delete_for_document
@@ -190,7 +190,7 @@ def delete_from_table(doctype, name, ignore_doctypes, doc):
 	# delete from child tables
 	for t in list(set(tables)):
 		if t not in ignore_doctypes:
-			frappe.db.sql("delete from `tab%s` where parenttype=%s and parent = %s" % (t, '%s', '%s'), (doctype, name))
+			frappe.db.delete(t, {"parenttype": doctype, "parent": name})
 
 def update_flags(doc, flags=None, ignore_permissions=False):
 	if ignore_permissions:
@@ -323,9 +323,10 @@ def delete_dynamic_links(doctype, name):
 
 def delete_references(doctype, reference_doctype, reference_name,
 		reference_doctype_field = 'reference_doctype', reference_name_field = 'reference_name'):
-	frappe.db.sql('''delete from `tab{0}`
-		where {1}=%s and {2}=%s'''.format(doctype, reference_doctype_field, reference_name_field), # nosec
-		(reference_doctype, reference_name))
+	frappe.db.delete(doctype, {
+		reference_doctype_field: reference_doctype,
+		reference_name_field: reference_name
+	})
 
 def clear_references(doctype, reference_doctype, reference_name,
 		reference_doctype_field = 'reference_doctype', reference_name_field = 'reference_name'):

frappe/model/document.py

@@ -385,8 +385,7 @@ class Document(BaseDocument):
 					[self.name, self.doctype, fieldname] + rows)
 			if len(deleted_rows) > 0:
 				# delete rows that do not match the ones in the document
-				frappe.db.sql("""delete from `tab{0}` where name in ({1})""".format(df.options,
-					','.join(['%s'] * len(deleted_rows))), tuple(row[0] for row in deleted_rows))
+				frappe.db.delete(df.options, {"name": ("in", tuple(row[0] for row in deleted_rows))})
 
 		else:
 			# no rows found, delete all rows

frappe/modules/utils.py

@@ -114,8 +114,7 @@ def sync_customizations_for_doctype(data, folder):
 					doc.db_insert()
 
 			if custom_doctype != 'Custom Field':
-				frappe.db.sql('delete from `tab{0}` where `{1}` =%s'.format(
-					custom_doctype, doctype_fieldname), doc_type)
+				frappe.db.delete(custom_doctype, {doctype_fieldname: doc_type})
 
 				for d in data[key]:
 					_insert(d)

frappe/patches.txt

@@ -182,4 +182,4 @@ frappe.patches.v13_0.jinja_hook
 frappe.patches.v13_0.update_notification_channel_if_empty
 frappe.patches.v14_0.drop_data_import_legacy
 frappe.patches.v14_0.rename_cancelled_documents
-frappe.patches.v14_0.update_workspace2
+frappe.patches.v14_0.update_workspace2 # 25.08.2021

frappe/patches/v14_0/rename_cancelled_documents.py

@@ -129,9 +129,9 @@ def update_linked_doctypes(doctype, cancelled_doc_names):
 				update
 					`tab{linked_dt}`
 				set
-					{column}=CONCAT({column}, '-CANC')
+					`{column}`=CONCAT(`{column}`, '-CANC')
 				where
-					{column} in %(cancelled_doc_names)s;
+					`{column}` in %(cancelled_doc_names)s;
 			""".format(linked_dt=linked_dt, column=field),
 				{'cancelled_doc_names': cancelled_doc_names})
 		else:
@@ -151,9 +151,9 @@ def update_dynamic_linked_doctypes(doctype, cancelled_doc_names):
 				update
 					`tab{linked_dt}`
 				set
-					{column}=CONCAT({column}, '-CANC')
+					`{column}`=CONCAT(`{column}`, '-CANC')
 				where
-					{column} in %(cancelled_doc_names)s and {doctype_fieldname}=%(dt)s;
+					`{column}` in %(cancelled_doc_names)s and {doctype_fieldname}=%(dt)s;
 			""".format(linked_dt=linked_dt, column=fieldname, doctype_fieldname=doctype_fieldname),
 				{'cancelled_doc_names': cancelled_doc_names, 'dt': doctype})
 		else:

frappe/patches/v14_0/update_workspace2.py

@@ -50,11 +50,11 @@ def create_content(doc):
 	return content
 
 def update_wspace(doc, seq, content):
-	if not doc.is_standard and not doc.public:
+	if not doc.title and not doc.content and not doc.is_standard and not doc.public:
 		doc.sequence_id = seq + 1
 		doc.content = json.dumps(content)
 		doc.public = 0
-		doc.title = doc.extends
+		doc.title = doc.extends or doc.label
 		doc.extends = ''
 		doc.category = ''
 		doc.onboarding = ''

frappe/public/js/frappe/desk.js

@@ -230,7 +230,7 @@ frappe.Application = class Application {
 			s.fields_dict.checking.$wrapper.html('<i class="fa fa-spinner fa-spin fa-4x"></i>');
 			s.show();
 			frappe.call({
-				method: 'frappe.core.doctype.user.user.set_email_password',
+				method: 'frappe.email.doctype.email_account.email_account.set_email_password',
 				args: {
 					"email_account": email_account[i]["email_account"],
 					"user": user,

frappe/public/js/frappe/form/controls/code.js

@@ -9,12 +9,7 @@ frappe.ui.form.ControlCode = class ControlCode extends frappe.ui.form.ControlTex
 		this.ace_editor_target = $('<div class="ace-editor-target"></div>')
 			.appendTo(this.input_area);
 
-		this.expanded = false;
-		this.$expand_button = $(`<button class="btn btn-xs btn-default">${this.get_button_label()}</button>`).click(() => {
-			this.expanded = !this.expanded;
-			this.refresh_height();
-			this.toggle_label();
-		}).appendTo(this.$input_wrapper);
+
 		// styling
 		this.ace_editor_target.addClass('border rounded');
 		this.ace_editor_target.css('height', 300);
@@ -22,6 +17,21 @@ frappe.ui.form.ControlCode = class ControlCode extends frappe.ui.form.ControlTex
 		// initialize
 		const ace = window.ace;
 		this.editor = ace.edit(this.ace_editor_target.get(0));
+
+		if (this.df.max_lines || this.df.min_lines) {
+			if (this.df.max_lines)
+				this.editor.setOption("maxLines", this.df.max_lines);
+			if (this.df.min_lines)
+				this.editor.setOption("minLines", this.df.min_lines);
+		} else {
+			this.expanded = false;
+			this.$expand_button = $(`<button class="btn btn-xs btn-default">${this.get_button_label()}</button>`).click(() => {
+				this.expanded = !this.expanded;
+				this.refresh_height();
+				this.toggle_label();
+			}).appendTo(this.$input_wrapper);
+		}
+
 		this.editor.setTheme('ace/theme/tomorrow');
 		this.editor.setOption("showPrintMargin", false);
 		this.editor.setOption("wrap", this.df.wrap);

frappe/public/js/frappe/form/controls/data.js

@@ -163,7 +163,7 @@ frappe.ui.form.ControlData = class ControlData extends frappe.ui.form.ControlInp
 			}
 		};
 		this.$input.on("change", change_handler);
-		if (this.constructor.trigger_change_on_input_event) {
+		if (this.constructor.trigger_change_on_input_event && !this.in_grid()) {
 			// debounce to avoid repeated validations on value change
 			this.$input.on("input", frappe.utils.debounce(change_handler, 500));
 		}
@@ -267,4 +267,7 @@ frappe.ui.form.ControlData = class ControlData extends frappe.ui.form.ControlInp
 		let el = this.$input.parents(el_class)[0];
 		if (el) $(el).toggleClass(scroll_class, add);
 	}
+	in_grid() {
+		return this.grid || this.layout && this.layout.grid;
+	}
 };

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

@@ -283,21 +283,13 @@ frappe.ui.form.Dashboard = class FormDashboard {
 		$(frappe.render_template('form_links', this.data))
 			.appendTo(transactions_area_body);
 
-		if (this.data.reports && this.data.reports.length) {
-			$(frappe.render_template('report_links', this.data))
-				.appendTo(transactions_area_body);
-		}
+		this.render_report_links();
 
 		// bind links
 		transactions_area_body.find(".badge-link").on('click', function() {
 			me.open_document_list($(this).closest('.document-link'));
 		});
 
-		// bind reports
-		transactions_area_body.find(".report-link").on('click', function() {
-			me.open_report($(this).parent());
-		});
-
 		// bind open notifications
 		transactions_area_body.find('.open-notification').on('click', function() {
 			me.open_document_list($(this).parent(), true);
@@ -311,6 +303,18 @@ frappe.ui.form.Dashboard = class FormDashboard {
 		this.data_rendered = true;
 	}
 
+	render_report_links() {
+		let parent = this.transactions_area;
+		if (this.data.reports && this.data.reports.length) {
+			$(frappe.render_template('report_links', this.data))
+				.appendTo(parent);
+			// bind reports
+			parent.find(".report-link").on('click', (e) => {
+				this.open_report($(e.target).parent());
+			});
+		}
+	}
+
 	open_report($link) {
 		let report = $link.attr('data-report');
 
@@ -318,6 +322,7 @@ frappe.ui.form.Dashboard = class FormDashboard {
 			? (this.data.non_standard_fieldnames[report] || this.data.fieldname)
 			: this.data.fieldname;
 
+		frappe.provide('frappe.route_options');
 		frappe.route_options[fieldname] = this.frm.doc.name;
 		frappe.set_route("query-report", report);
 	}

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

@@ -339,7 +339,7 @@ frappe.ui.form.Form = class FrappeForm {
 			}
 		}
 		if (action.action_type==='Server Action') {
-			frappe.xcall(action.action, {'doc': this.doc}).then((doc) => {
+			return frappe.xcall(action.action, {'doc': this.doc}).then((doc) => {
 				if (doc.doctype) {
 					// document is returned by the method,
 					// apply the changes locally and refresh
@@ -354,7 +354,7 @@ frappe.ui.form.Form = class FrappeForm {
 				});
 			});
 		} else if (action.action_type==='Route') {
-			frappe.set_route(action.action);
+			return frappe.set_route(action.action);
 		}
 	}
 

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

@@ -82,10 +82,16 @@ frappe.ui.form.FormTour = class FormTour {
 
 	get_step(step_info, on_next) {
 		const { name, fieldname, title, description, position, is_table_field } = step_info;
+		let element = `.frappe-control[data-fieldname='${fieldname}']`;
+
 		const field = this.frm.get_field(fieldname);
-		let element = field ? field.wrapper : `.frappe-control[data-fieldname='${fieldname}']`;
+		if (field) {
+			// wrapper for section breaks returns in a list
+			element = field.wrapper[0] ? field.wrapper[0] : field.wrapper;
+		}
 
 		if (is_table_field) {
+			// TODO: fix wrapper for grid sections
 			element = `.grid-row-open .frappe-control[data-fieldname='${fieldname}']`;
 		}
 

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

@@ -38,7 +38,7 @@ export default class Grid {
 
 		this.is_grid = true;
 		this.debounced_refresh = this.refresh.bind(this);
-		this.debounced_refresh = frappe.utils.debounce(this.debounced_refresh, 500);
+		this.debounced_refresh = frappe.utils.debounce(this.debounced_refresh, 100);
 	}
 
 	allow_on_grid_editing() {
@@ -502,7 +502,7 @@ export default class Grid {
 			this.set_editable_grid_column_disp(fieldname, show);
 		}
 
-		this.refresh(true);
+		this.debounced_refresh();
 	}
 
 	set_editable_grid_column_disp(fieldname, show) {
@@ -546,17 +546,17 @@ export default class Grid {
 
 	toggle_reqd(fieldname, reqd) {
 		this.get_docfield(fieldname).reqd = reqd;
-		this.refresh();
+		this.debounced_refresh();
 	}
 
 	toggle_enable(fieldname, enable) {
 		this.get_docfield(fieldname).read_only = enable ? 0 : 1;
-		this.refresh();
+		this.debounced_refresh();
 	}
 
 	toggle_display(fieldname, show) {
 		this.get_docfield(fieldname).hidden = show ? 0 : 1;
-		this.refresh();
+		this.debounced_refresh();
 	}
 
 	toggle_checkboxes(enable) {

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

@@ -252,14 +252,18 @@ frappe.ui.form.Layout = class Layout {
 		}
 
 		if (document.activeElement) {
-			document.activeElement.focus();
-	
-			if (document.activeElement.tagName == 'INPUT') {
+			if (document.activeElement.tagName == 'INPUT' && this.is_numeric_field_active()) {
 				document.activeElement.select();
 			}
 		}
 	}
 
+	is_numeric_field_active() {
+		const control = $(document.activeElement).closest(".frappe-control");
+		const fieldtype = (control.data() || {}).fieldtype;
+		return frappe.model.numeric_fieldtypes.includes(fieldtype);
+	}
+
 	refresh_sections() {
 		// hide invisible sections
 		this.wrapper.find(".form-section:not(.hide-control)").each(function() {

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

@@ -367,6 +367,7 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList {
 
 		if (
 			!this.settings.hide_name_column &&
+			this.meta.title_field &&
 			this.meta.title_field !== 'name'
 		) {
 			this.columns.push({

frappe/public/js/frappe/recorder/RecorderDetail.vue

@@ -1,5 +1,5 @@
 <template>
-	<div>
+	<div v-cloak @drop.prevent="import_data" @dragover.prevent>
 		<div class="page-form">
 			<div class="filter-list">
 				<div class="tag-filters-area">
@@ -46,7 +46,7 @@
 
 				</div>
 				<div class="result-list">
-					<div class="list-row-container" v-for="(request, index) in paginated(sorted(filtered(requests)))" :key="index" @click="route_to_request_detail(request.uuid)">
+					<div class="list-row-container" v-for="(request, index) in paginated(sorted(filtered(requests)))" :key="index" @click="route_to_request_detail(request)">
 						<div class="level list-row small">
 							<div class="level-left ellipsis">
 								<div class="list-row-col ellipsis list-subject level ">
@@ -71,15 +71,16 @@
 			</div>
 			<div v-if="requests.length == 0" class="no-result text-muted flex justify-center align-center" style="">
 				<div class="msg-box no-border" v-if="status.status == 'Inactive'" >
-					<p>{{ __("Recorder is Inactive") }}</p>
 					<p><button class="btn btn-primary btn-sm btn-new-doc" @click="start()">{{ __("Start Recording") }}</button></p>
+					<p>{{ __("Recorder is Inactive.") }}</p>
+					<p>{{ __("Start recording or drag & drop a previously exported data file to view it.") }}</p>
 				</div>
 				<div class="msg-box no-border" v-if="status.status == 'Active'" >
 					<p>{{ __("No Requests found") }}</p>
 					<p>{{ __("Go make some noise") }}</p>
 				</div>
 			</div>
-			<div v-if="requests.length != 0" class="list-paging-area">
+			<div v-else class="list-paging-area">
 				<div class="row">
 					<div class="col-xs-6">
 						<div class="btn-group btn-group-paging">
@@ -144,7 +145,7 @@ export default {
 			frappe.set_route("recorder");
 			this.clear();
 		});
-
+		this.$root.page.add_menu_item("Export data", () => this.export_data());
 	},
 	computed: {
 		pages: function() {
@@ -239,8 +240,36 @@ export default {
 				});
 			}
 		},
-		route_to_request_detail(id) {
-			this.$router.push({name: 'request-detail', params: {id}});
+		route_to_request_detail(request) {
+			this.$router.push({name: 'request-detail', params: {request, id: request.uuid}});
+		},
+		export_data: function() {
+			if (!this.requests) {
+				return;
+			}
+			frappe.call("frappe.recorder.export_data")
+				.then((r) => {
+					const data = r.message;
+					const filename = `${data[0]['uuid']}..${data[data.length -1]['uuid']}.json`
+
+					const el = document.createElement('a');
+					el.setAttribute('href', 'data:application/json,' + encodeURIComponent(JSON.stringify(data)));
+					el.setAttribute('download', filename);
+					el.click();
+				});
+		},
+		import_data: function(e) {
+			if (this.requests.length > 0) {
+				// don't replace existing capture
+				return;
+			}
+			const request_file = e.dataTransfer.files[0];
+
+			const file_reader = new FileReader();
+			file_reader.readAsText(request_file, 'UTF-8');
+			file_reader.onload = ({target: {result}}) => {
+				 this.requests = JSON.parse(result);
+			}
 		}
 	}
 };

frappe/public/js/frappe/recorder/RequestDetail.vue

@@ -283,14 +283,21 @@ export default {
 			label: __('Recorder'),
 			route: '/app/recorder'
 		});
-		frappe.call({
-			method: "frappe.recorder.get",
-			args: {
-				uuid: this.$route.params.id
-			}
-		}).then( r => {
-			this.request = r.message
-		});
-	}
+
+		const request = this.$route.params.request;
+		if (request.headers || request.form_dict || request.calls) {
+			// complete request data passed as parameter.
+			this.request = request;
+		} else {
+			frappe.call({
+				method: "frappe.recorder.get",
+				args: {
+					uuid: request.uuid
+				}
+			}).then( r => {
+				this.request = r.message
+			});
+		}
+	},
 };
 </script>

frappe/public/js/frappe/router.js

@@ -129,7 +129,7 @@ frappe.router = {
 		if (frappe.workspaces[route[0]]) {
 			// public workspace
 			route = ['Workspaces', frappe.workspaces[route[0]].title];
-		} else if (frappe.workspaces[route[1]]) {
+		} else if (route[0] == 'private' && frappe.workspaces[route[1]]) {
 			// private workspace
 			route = ['Workspaces', 'private', frappe.workspaces[route[1]].title];
 		} else if (this.routes[route[0]]) {
@@ -354,8 +354,8 @@ frappe.router = {
 				return a;
 			}
 		}).join('/');
-
-		return '/app/' + (path_string || 'home');
+		let default_page = frappe.workspaces['home'] ? 'home' : Object.keys(frappe.workspaces)[0];
+		return '/app/' + (path_string || default_page);
 	},
 
 	push_state(url) {

frappe/public/js/frappe/ui/page.js

@@ -251,7 +251,7 @@ frappe.ui.Page = class Page {
 			.prop("disabled", false)
 			.html(opts.label)
 			.on("click", function() {
-				let response = opts.click.apply(this);
+				let response = opts.click.apply(this, [btn]);
 				me.btn_disable_enable(btn, response);
 			});
 

frappe/public/js/frappe/ui/toolbar/awesome_bar.js

@@ -194,9 +194,14 @@ frappe.search.AwesomeBar = class AwesomeBar {
 		var out = [], routes = [];
 		options.forEach(function(option) {
 			if(option.route) {
-				if(option.route[0] === "List" && option.route[2] !== 'Report') {
+				if (
+					option.route[0] === "List" &&
+					option.route[2] !== 'Report' &&
+					option.route[2] !== 'Inbox'
+				) {
 					option.route.splice(2);
 				}
+
 				var str_route = (typeof option.route==='string') ?
 					option.route : option.route.join('/');
 				if(routes.indexOf(str_route)===-1) {

frappe/public/js/frappe/views/inbox/inbox_view.js

@@ -94,19 +94,20 @@ frappe.views.InboxView = class InboxView extends frappe.views.ListView {
 		this.render_list();
 		this.on_row_checked();
 		this.render_count();
-		this.render_tags();
 	}
 
 	get_meta_html(email) {
 		const attachment = email.has_attachment ?
 			`<span class="fa fa-paperclip fa-large" title="${__('Has Attachments')}"></span>` : '';
 
-		const form_link = frappe.utils.get_form_link(email.reference_doctype, email.reference_name);
-		const link = email.reference_doctype && email.reference_doctype !== this.doctype ?
-			`<a class="text-muted grey" href="${form_link}"
+		let link = "";
+		if (email.reference_doctype && email.reference_doctype !== this.doctype) {
+			link = `<a class="text-muted grey"
+				href="${frappe.utils.get_form_link(email.reference_doctype, email.reference_name)}"
 				title="${__('Linked with {0}', [email.reference_doctype])}">
 				<i class="fa fa-link fa-large"></i>
-			</a>` : '';
+			</a>`;
+		}
 
 		const communication_date = comment_when(email.communication_date, true);
 		const status =

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

@@ -854,6 +854,10 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList {
 				}
 			};
 
+			if (this.raw_data.add_total_row) {
+				this.$page.find('.layout-main-section').css('--report-total-height', '310px');
+			}
+
 			if (this.report_settings.get_datatable_options) {
 				datatable_options = this.report_settings.get_datatable_options(datatable_options);
 			}

frappe/public/js/frappe/views/workspace/blocks/card.js

@@ -14,7 +14,7 @@ export default class Card extends Block {
 	constructor({ data, api, config, readOnly, block }) {
 		super({ data, api, config, readOnly, block });
 		this.sections = {};
-		this.col = this.data.col ? this.data.col : "12";
+		this.col = this.data.col ? this.data.col : "4";
 		this.allow_customization = !this.readOnly;
 		this.options = {
 			allow_sorting: this.allow_customization,

frappe/public/js/frappe/views/workspace/blocks/paragraph.js

@@ -123,10 +123,10 @@ export default class Paragraph extends Block {
 		return true;
 	}
 
-	save(toolsContent) {
+	save() {
 		this.wrapper = this._element;
 		return {
-			text: toolsContent.innerText,
+			text: this.wrapper.innerHTML,
 			col: this.get_col(),
 		};
 	}
@@ -155,6 +155,9 @@ export default class Paragraph extends Block {
 		return {
 			text: {
 				br: true,
+				b: true,
+				i: true,
+				a: true
 			}
 		};
 	}

frappe/public/js/frappe/views/workspace/blocks/shortcut.js

@@ -13,7 +13,7 @@ export default class Shortcut extends Block {
 
 	constructor({ data, api, config, readOnly, block }) {
 		super({ data, api, config, readOnly, block });
-		this.col = this.data.col ? this.data.col : "12";
+		this.col = this.data.col ? this.data.col : "4";
 		this.allow_customization = !this.readOnly;
 		this.options = {
 			allow_sorting: this.allow_customization,

frappe/public/js/frappe/views/workspace/workspace.js

@@ -32,8 +32,8 @@ frappe.views.Workspace = class Workspace {
 			'private': {}
 		};
 		this.sidebar_categories = [
-			'Public',
-			frappe.user.first_name() || 'Private'
+			'My Workspaces',
+			'Public'
 		];
 		this.tools = {
 			header: {
@@ -174,10 +174,6 @@ frappe.views.Workspace = class Workspace {
 			$(e.target).parent().find('.sidebar-item-container').toggleClass('hidden');
 		});
 
-		if (!this.current_page.name) {
-			$title.trigger("click");
-		}
-
 		if (Object.keys(root_pages).length === 0) {
 			sidebar_section.addClass('hidden');
 		}
@@ -357,7 +353,7 @@ frappe.views.Workspace = class Workspace {
 		let current_page = pages.filter(p => p.title == page.name)[0];
 
 		if (!this.is_read_only) {
-			this.setup_customization_buttons(current_page.is_editable);
+			this.setup_customization_buttons(current_page);
 			return;
 		}
 
@@ -365,20 +361,20 @@ frappe.views.Workspace = class Workspace {
 		this.page.clear_secondary_action();
 		this.page.clear_inner_toolbar();
 
-		current_page.is_editable && this.page.set_secondary_action(__("Customize"), () => {
+		current_page.is_editable && this.page.set_secondary_action(__("Edit"), () => {
 			if (!this.editor || !this.editor.readOnly) return;
 			this.is_read_only = false;
 			this.editor.readOnly.toggle();
 			this.editor.isReady.then(() => {
 				this.initialize_editorjs_undo();
-				this.setup_customization_buttons(true);
+				this.setup_customization_buttons(current_page);
 				this.show_sidebar_actions();
 				this.make_sidebar_sortable();
 				this.make_blocks_sortable();
 			});
 		});
 
-		this.page.add_inner_button(__("Create Page"), () => {
+		this.page.add_inner_button(__("Create Workspace"), () => {
 			this.initialize_new_page();
 		});
 	}
@@ -389,13 +385,13 @@ frappe.views.Workspace = class Workspace {
 		this.undo.readOnly = false;
 	}
 
-	setup_customization_buttons(is_editable) {
+	setup_customization_buttons(page) {
 		let me = this;
 		this.page.clear_primary_action();
 		this.page.clear_secondary_action();
 		this.page.clear_inner_toolbar();
 
-		is_editable && this.page.set_primary_action(
+		page.is_editable && this.page.set_primary_action(
 			__("Save Customizations"),
 			() => {
 				this.page.clear_primary_action();
@@ -424,6 +420,10 @@ frappe.views.Workspace = class Workspace {
 			}
 		);
 
+		page.name && this.page.add_inner_button(__("Settings"), () => {
+			frappe.set_route(`workspace/${page.name}`);
+		});
+
 		Object.keys(this.blocks).forEach(key => {
 			this.page.add_inner_button(`
 				<span class="block-menu-item-icon">${this.blocks[key].toolbox.icon}</span>
@@ -446,7 +446,7 @@ frappe.views.Workspace = class Workspace {
 			$(`<span class="sidebar-info">${frappe.utils.icon("lock", "sm")}</span>`)
 				.appendTo(sidebar_control);
 			sidebar_control.parent().click(() => {
-				frappe.show_alert({
+				!this.is_read_only && frappe.show_alert({
 					message: __("Only Workspace Manager can sort or edit this page"),
 					indicator: 'info'
 				}, 5);
@@ -498,9 +498,9 @@ frappe.views.Workspace = class Workspace {
 
 	prepare_sorted_sidebar(is_public) {
 		if (is_public) {
-			this.sorted_public_items = this.sort_sidebar(this.sidebar.find('.standard-sidebar-section').first());
+			this.sorted_public_items = this.sort_sidebar(this.sidebar.find('.standard-sidebar-section').last());
 		} else {
-			this.sorted_private_items = this.sort_sidebar(this.sidebar.find('.standard-sidebar-section').last());
+			this.sorted_private_items = this.sort_sidebar(this.sidebar.find('.standard-sidebar-section').first());
 		}
 	}
 
@@ -578,7 +578,7 @@ frappe.views.Workspace = class Workspace {
 				if (!this.validate_page(values)) return;
 				d.hide();
 				this.initialize_editorjs_undo();
-				this.setup_customization_buttons(true);
+				this.setup_customization_buttons({is_editable: true});
 				this.title = values.title;
 				this.icon = values.icon;
 				this.parent = values.parent;
@@ -647,7 +647,7 @@ frappe.views.Workspace = class Workspace {
 		);
 		$sidebar_item.find('.sidebar-item-control .drag-handle').css('margin-right', '8px');
 
-		let $sidebar_section = is_public ? $sidebar[0] : $sidebar[1];
+		let $sidebar_section = is_public ? $sidebar[1] : $sidebar[0];
 
 		if (!parent) {
 			!is_public && $sidebar.last().removeClass('hidden');

frappe/public/scss/desk/desktop.scss

@@ -946,7 +946,11 @@ body {
 						&.new-widget {
 							align-items: inherit;
 						}
-	
+
+						&.ce-paragraph {
+							display: block;
+						}
+
 						.paragraph-control {
 							display: flex;
 							flex-direction: row-reverse;

frappe/public/scss/desk/form.scss

@@ -115,7 +115,7 @@
 		margin-bottom: var(--margin-sm);
 		font-weight: var(--text-bold);
 	}
-	.row:first-child {
+	.form-documents:first-of-type .row:first-child {
 		.form-link-title {
 			margin-top: 0;
 		}

frappe/public/scss/desk/report.scss

@@ -84,8 +84,9 @@
 	margin-bottom: 10px;
 }
 
-.layout-main-section .frappe-card {
+.layout-main-section {
 	--report-filter-height: 0px;
+	--report-total-height: 275px;
 }
 
 .report-wrapper {
@@ -95,7 +96,7 @@
 		height: calc(100vh - var(--report-filter-height) - 205px);
 
 		.dt-scrollable {
-			height: calc(100vh - var(--report-filter-height) - 275px);
+			height: calc(100vh - var(--report-filter-height) - var(--report-total-height));
 		}
 	}
 }

frappe/rate_limiter.py

@@ -103,7 +103,7 @@ def rate_limit(key: str, limit: Union[int, Callable] = 5, seconds: int= 24*60*60
 		def wrapper(*args, **kwargs):
 			# Do not apply rate limits if method is not opted to check
 			if methods != 'ALL' and frappe.request.method.upper() not in methods:
-				return frappe.call(fun, **frappe.form_dict)
+				return frappe.call(fun, **frappe.form_dict or kwargs)
 
 			_limit = limit() if callable(limit) else limit
 
@@ -118,6 +118,6 @@ def rate_limit(key: str, limit: Union[int, Callable] = 5, seconds: int= 24*60*60
 			if value > _limit:
 				frappe.throw(_("You hit the rate limit because of too many requests. Please try after sometime."))
 
-			return frappe.call(fun, **frappe.form_dict)
+			return frappe.call(fun, **frappe.form_dict or kwargs)
 		return wrapper
 	return ratelimit_decorator

frappe/recorder.py

@@ -181,6 +181,13 @@ def get(uuid=None, *args, **kwargs):
 	return result
 
 
+@frappe.whitelist()
+@do_not_record
+@administrator_only
+def export_data(*args, **kwargs):
+	return list(frappe.cache().hgetall(RECORDER_REQUEST_HASH).values())
+
+
 @frappe.whitelist()
 @do_not_record
 @administrator_only

frappe/social/doctype/energy_point_log/test_energy_point_log.py

@@ -13,8 +13,8 @@ class TestEnergyPointLog(unittest.TestCase):
 
 	def tearDown(self):
 		frappe.set_user('Administrator')
-		frappe.db.sql('DELETE FROM `tabEnergy Point Log`')
-		frappe.db.sql('DELETE FROM `tabEnergy Point Rule`')
+		frappe.db.delete("Energy Point Log")
+		frappe.db.delete("Energy Point Rule")
 		frappe.cache().delete_value('energy_point_rule_map')
 
 	def test_user_energy_point(self):

frappe/templates/emails/password_reset.html

@@ -3,5 +3,5 @@
 <p><a class="btn btn-primary" href="{{ link }}">{{_("Reset your password")}}</a></p>
 <p>
 	{{_("Thank you")}},<br>
-	{{ user_fullname }}
+	{{ created_by }}
 </p>

frappe/templates/includes/feedback/feedback.html

@@ -7,9 +7,6 @@
 			<form>
 				<fieldset>
 					<div class="row" style="margin-bottom: 15px;">
-						<div class="col-sm-6">
-							<input class="form-control feedback_email" name="feedback_email" placeholder="{{ _("Your Email Address") }}" type="email">
-						</div>
 						<div class="col-sm-6">
 							<div class="rating">
 								{% for rating in [1, 2, 3, 4, 5 ,6, 7, 8, 9, 10] %}
@@ -41,7 +38,6 @@
 		feedback && $("#submit-feedback").html(__("Update"));
 
 		if (frappe.is_user_logged_in()) {
-			$(".feedback_email").parent().toggleClass("hidden");
 			if (feedback) {
 				$("[name='feedback']").val(feedback);
 				toggle_feedback();
@@ -83,12 +79,12 @@
 
 		$('#submit-feedback').click((ev) => {
 			let update = ev.target.innerText !== __("Submit");
+			let rating = $('.rating').find('.rating-click').length;
 			let args = {
 				reference_doctype: "{{ reference_doctype or doctype }}",
 				reference_name: "{{ reference_name or name }}",
 				rating: rating,
-				feedback: $("[name='feedback']").val(),
-				feedback_email: $("[name='feedback_email']").val() || frappe.user_id
+				feedback: $("[name='feedback']").val()
 			}
 
 			if (args.rating == 0) {
@@ -101,16 +97,14 @@
 				return false;
 			}
 
-			if (args.feedback_email!=='Administrator' && !validate_email(args.feedback_email)) {
-				frappe.msgprint("{{ _("Please enter a valid email address.") }}");
-				return false;
-			}
-
 			if (!update) {
 				frappe.call({
 					method: "frappe.templates.includes.feedback.feedback.add_feedback",
 					args: args,
 					callback: function(r) {
+						if (!r.message) {
+							return
+						}
 						toggle_feedback();
 						if (!frappe.is_user_logged_in()) {
 							$("[name='feedback']").val('');