diff --git a/.github/workflows/patch-mariadb-tests.yml b/.github/workflows/patch-mariadb-tests.yml
index 82be4d06b5..e8627a01fb 100644
--- a/.github/workflows/patch-mariadb-tests.yml
+++ b/.github/workflows/patch-mariadb-tests.yml
@@ -1,11 +1,6 @@
name: Patch
-on:
- pull_request:
- paths-ignore:
- - '**.js'
- - '**.md'
- workflow_dispatch:
+on: [pull_request, workflow_dispatch]
jobs:
test:
diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml
index 8d5bd690a1..2476102e3d 100644
--- a/.github/workflows/server-mariadb-tests.yml
+++ b/.github/workflows/server-mariadb-tests.yml
@@ -2,15 +2,9 @@ name: Server
on:
pull_request:
- paths-ignore:
- - '**.js'
- - '**.md'
workflow_dispatch:
push:
branches: [ develop ]
- paths-ignore:
- - '**.js'
- - '**.md'
jobs:
test:
diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-postgres-tests.yml
index 8c97c7f84b..4325eebaad 100644
--- a/.github/workflows/server-postgres-tests.yml
+++ b/.github/workflows/server-postgres-tests.yml
@@ -2,9 +2,6 @@ name: Server
on:
pull_request:
- paths-ignore:
- - '**.js'
- - '**.md'
workflow_dispatch:
jobs:
diff --git a/.github/workflows/ui-tests.yml b/.github/workflows/ui-tests.yml
index d76e5e77ea..f342c0709e 100644
--- a/.github/workflows/ui-tests.yml
+++ b/.github/workflows/ui-tests.yml
@@ -2,8 +2,6 @@ name: UI
on:
pull_request:
- paths-ignore:
- - '**.md'
workflow_dispatch:
push:
branches: [ develop ]
diff --git a/.mergify.yml b/.mergify.yml
index 1a81a28594..8c7a7dc95d 100644
--- a/.mergify.yml
+++ b/.mergify.yml
@@ -1,9 +1,11 @@
pull_request_rules:
- name: Auto-close PRs on stable branch
conditions:
- - or:
- - base=version-13
- - base=version-12
+ - and:
+ - author!=surajshetty3416
+ - or:
+ - base=version-13
+ - base=version-12
actions:
close:
comment:
diff --git a/cypress/integration/form_tour.js b/cypress/integration/form_tour.js
index d2d39679a8..ab7ada9034 100644
--- a/cypress/integration/form_tour.js
+++ b/cypress/integration/form_tour.js
@@ -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-popover-item').findByRole('button', {name: 'Next'}).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();
@@ -68,13 +68,13 @@ 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);
@@ -82,7 +82,7 @@ context('Form Tour', () => {
// assert save btn is highlighted
cy.get('.primary-action').should('have.class', 'driver-highlighted-element');
cy.wait(500);
- cy.get('#driver-popover-item').findByRole('button', {name: 'Save'}).should('be.visible');
+ cy.get('.frappe-driver').findByRole('button', {name: 'Save'}).should('be.visible');
});
});
diff --git a/cypress/integration/workspace.js b/cypress/integration/workspace.js
index f18e48aadc..65586366e6 100644
--- a/cypress/integration/workspace.js
+++ b/cypress/integration/workspace.js
@@ -36,12 +36,12 @@ context('Workspace 2.0', () => {
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');
});
diff --git a/frappe/automation/doctype/assignment_rule/test_assignment_rule.py b/frappe/automation/doctype/assignment_rule/test_assignment_rule.py
index e287b83965..dfefd091fb 100644
--- a/frappe/automation/doctype/assignment_rule/test_assignment_rule.py
+++ b/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')
diff --git a/frappe/automation/doctype/milestone_tracker/test_milestone_tracker.py b/frappe/automation/doctype/milestone_tracker/test_milestone_tracker.py
index 21b2779018..1683e94827 100644
--- a/frappe/automation/doctype/milestone_tracker/test_milestone_tracker.py
+++ b/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()
\ No newline at end of file
diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py
index be8304e45d..6de803475a 100644
--- a/frappe/commands/utils.py
+++ b/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')
diff --git a/frappe/core/doctype/access_log/access_log.py b/frappe/core/doctype/access_log/access_log.py
index 2ea014f981..82db450b4a 100644
--- a/frappe/core/doctype/access_log/access_log.py
+++ b/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()
diff --git a/frappe/core/doctype/comment/test_comment.py b/frappe/core/doctype/comment/test_comment.py
index 13db92e7a8..12fe027fba 100644
--- a/frappe/core/doctype/comment/test_comment.py
+++ b/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)
diff --git a/frappe/core/doctype/feedback/test_feedback.py b/frappe/core/doctype/feedback/test_feedback.py
index 2a96d86874..c7551420c3 100644
--- a/frappe/core/doctype/feedback/test_feedback.py
+++ b/frappe/core/doctype/feedback/test_feedback.py
@@ -9,7 +9,7 @@ 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')
@@ -22,6 +22,6 @@ class TestFeedback(unittest.TestCase):
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()
\ No newline at end of file
diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py
index 369179eece..e747ca2b91 100755
--- a/frappe/core/doctype/file/file.py
+++ b/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
@@ -334,12 +326,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:
@@ -367,10 +357,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
@@ -439,47 +425,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
@@ -547,14 +492,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)
@@ -660,7 +597,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):
@@ -711,7 +649,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
@@ -743,7 +681,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)
@@ -779,48 +717,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
@@ -866,6 +768,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)
@@ -930,10 +833,8 @@ def extract_images_from_html(doc, content):
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)
@@ -961,12 +862,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 "")
@@ -977,7 +875,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()
@@ -1002,13 +900,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)
diff --git a/frappe/core/doctype/file/test_file.py b/frappe/core/doctype/file/test_file.py
index 649010c468..5478d7ab85 100644
--- a/frappe/core/doctype/file/test_file.py
+++ b/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
'
+ }).insert()
+ self.assertTrue(frappe.db.exists("File", {"attached_to_name": todo.name}))
+ self.assertIn('
', 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 '
+ }).insert()
+ filename = frappe.db.exists("File", {"attached_to_name": todo.name})
+ self.assertIn(f'
@@ -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 = '''
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.
'.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): diff --git a/frappe/desk/doctype/event/test_event.py b/frappe/desk/doctype/event/test_event.py index 77211946a9..8f56d11da3 100644 --- a/frappe/desk/doctype/event/test_event.py +++ b/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') diff --git a/frappe/desk/doctype/note/test_note.py b/frappe/desk/doctype/note/test_note.py index 1bb1730357..3207fa9b8d 100644 --- a/frappe/desk/doctype/note/test_note.py +++ b/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() diff --git a/frappe/desk/doctype/notification_log/test_notification_log.py b/frappe/desk/doctype/notification_log/test_notification_log.py index af4dee8df3..bedb10b495 100644 --- a/frappe/desk/doctype/notification_log/test_notification_log.py +++ b/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] diff --git a/frappe/desk/doctype/system_console/system_console.js b/frappe/desk/doctype/system_console/system_console.js index c7eac39490..48dd2ba108 100644 --- a/frappe/desk/doctype/system_console/system_console.js +++ b/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')); + }); }); } }); diff --git a/frappe/desk/doctype/tag/test_tag.py b/frappe/desk/doctype/tag/test_tag.py index 6eb7219c26..b9c6e0b744 100644 --- a/frappe/desk/doctype/tag/test_tag.py +++ b/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): diff --git a/frappe/desk/doctype/todo/test_todo.py b/frappe/desk/doctype/todo/test_todo.py index b38e4a059a..f6371c5921 100644 --- a/frappe/desk/doctype/todo/test_todo.py +++ b/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() diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index 09297b4e5e..754b94cdcb 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/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" } diff --git a/frappe/desk/doctype/workspace/workspace.json b/frappe/desk/doctype/workspace/workspace.json index e2ae38faf1..020f3153df 100644 --- a/frappe/desk/doctype/workspace/workspace.json +++ b/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", diff --git a/frappe/desk/doctype/workspace/workspace.py b/frappe/desk/doctype/workspace/workspace.py index 0821ae03c4..7795d02616 100644 --- a/frappe/desk/doctype/workspace/workspace.py +++ b/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 }) diff --git a/frappe/desk/form/utils.py b/frappe/desk/form/utils.py index bfceee6ea2..d7ac940d21 100644 --- a/frappe/desk/form/utils.py +++ b/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 _ diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index ecd59f42bb..fb7349adba 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/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 \ No newline at end of file diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index 35cacac45a..da03a5959e 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/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: diff --git a/frappe/email/doctype/newsletter/exceptions.py b/frappe/email/doctype/newsletter/exceptions.py new file mode 100644 index 0000000000..a6c688dbe8 --- /dev/null +++ b/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 diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 97d77549b7..a118240488 100755 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/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 = """ -{0}. {1}.
- - """.format(*messages) +{0}. {1}.
+ + """.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() diff --git a/frappe/email/doctype/newsletter/test_newsletter.py b/frappe/email/doctype/newsletter/test_newsletter.py index 3abd339ed9..abbcc6440c 100644 --- a/frappe/email/doctype/newsletter/test_newsletter.py +++ b/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
{{_("Thank you")}},
- {{ user_fullname }}
+ {{ created_by }}
Your OTP secret on {0} has been reset. If you did not perform this reset and did not request it, please contact your System Administrator immediately.
').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.")) \ No newline at end of file diff --git a/frappe/utils/csvutils.py b/frappe/utils/csvutils.py index 734d68fe8a..69b7f6f2d3 100644 --- a/frappe/utils/csvutils.py +++ b/frappe/utils/csvutils.py @@ -6,16 +6,7 @@ import json import csv import requests from io import StringIO -from frappe.utils import encode, cstr, cint, flt, comma_or - -def read_csv_content_from_uploaded_file(ignore_encoding=False): - if getattr(frappe, "uploaded_file", None): - with open(frappe.uploaded_file, "r") as upfile: - fcontent = upfile.read() - else: - _file = frappe.new_doc("File") - fcontent = _file.get_uploaded_content() - return read_csv_content(fcontent, ignore_encoding) +from frappe.utils import cstr, cint, flt, comma_or def read_csv_content_from_attached_file(doc): fileid = frappe.get_all("File", fields = ["name"], filters = {"attached_to_doctype": doc.doctype, diff --git a/frappe/utils/file_manager.py b/frappe/utils/file_manager.py index 79a5423d8b..b1e088d641 100644 --- a/frappe/utils/file_manager.py +++ b/frappe/utils/file_manager.py @@ -213,28 +213,22 @@ def write_file(content, fname, is_private=0): return get_files_path(fname, is_private=is_private) -def remove_all(dt, dn, from_delete=False): +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)): - remove_file(fid, dt, dn, from_delete) + 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 remove_file_by_url(file_url, doctype=None, name=None): - if doctype and name: - fid = frappe.db.get_value("File", {"file_url": file_url, - "attached_to_doctype": doctype, "attached_to_name": name}) - else: - fid = frappe.db.get_value("File", {"file_url": file_url}) - - if fid: - return remove_file(fid) - - -def remove_file(fid, attached_to_doctype=None, attached_to_name=None, from_delete=False): +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): @@ -252,8 +246,7 @@ def remove_file(fid, attached_to_doctype=None, attached_to_name=None, from_delet 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) + frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions, delete_permanently=delete_permanently) return comment @@ -372,76 +365,6 @@ def download_file(file_url): frappe.local.response.filecontent = filedata frappe.local.response.type = "download" -def extract_images_from_doc(doc, fieldname): - content = doc.get(fieldname) - content = extract_images_from_html(doc, content) - if frappe.flags.has_dataurl: - doc.set(fieldname, content) - - -def extract_images_from_html(doc, content): - frappe.flags.has_dataurl = False - - def _save_file(match): - data = match.group(1) - data = data.split("data:")[1] - headers, content = data.split(",") - mtype = headers.split(";")[0] - - if isinstance(content, str): - content = content.encode("utf-8") - 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] - - # decode filename - if not isinstance(filename, str): - filename = str(filename, 'utf-8') - else: - filename = get_random_filename(content_type=mtype) - - doctype = doc.parenttype if doc.parent else doc.doctype - name = doc.parent or doc.name - - if doc.doctype == "Comment": - doctype = doc.reference_doctype - name = doc.reference_name - - # TODO fix this - file_url = save_file(filename, content, doctype, name, decode=False).get("file_url") - if not frappe.flags.has_dataurl: - frappe.flags.has_dataurl = True - - return '