Merge branch 'frappe:develop' into revert-single-scrollbar-pr

This commit is contained in:
Shariq Ansari 2021-10-19 19:28:55 +05:30 committed by GitHub
commit 9673822e2e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
27 changed files with 202 additions and 103 deletions

View file

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

View file

@ -3,6 +3,7 @@ codecov:
coverage:
status:
patch: off
project:
default: false
server:
@ -10,11 +11,6 @@ coverage:
threshold: 0.5%
flags:
- server
ui-tests:
target: auto
threshold: 0.5%
flags:
- ui-tests
comment:
layout: "diff, flags"
@ -28,4 +24,4 @@ flags:
ui-tests:
paths:
- ".*\\.js"
carryforward: true
carryforward: true

View file

@ -44,6 +44,11 @@ let argv = yargs
type: "boolean",
description: "Run in watch mode and rebuild on file changes"
})
.option("live-reload", {
type: "boolean",
description: `Automatically reload web pages when assets are rebuilt.
Can only be used with the --watch flag.`
})
.option("production", {
type: "boolean",
description: "Run build in production mode"
@ -478,7 +483,8 @@ async function notify_redis({ error, success }) {
}
if (success) {
payload = {
success: true
success: true,
live_reload: argv["live-reload"]
};
}

View file

@ -257,6 +257,13 @@ def watch(apps=None):
if apps:
command += " --apps {apps}".format(apps=apps)
live_reload = frappe.utils.cint(
os.environ.get("LIVE_RELOAD", frappe.conf.live_reload)
)
if live_reload:
command += " --live-reload"
check_node_executable()
frappe_app_path = frappe.get_app_path("frappe", "..")
frappe.commands.popen(command, cwd=frappe_app_path, env=get_node_env())

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -14,7 +14,7 @@ from frappe.utils import cstr, format_duration
from frappe.model.base_document import get_controller
@frappe.whitelist(allow_guest=True)
@frappe.whitelist()
@frappe.read_only()
def get():
args = get_form_params()

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -1049,18 +1049,20 @@ Object.assign(frappe.utils, {
return duration;
},
seconds_to_duration(value, duration_options) {
let secs = value;
let total_duration = {
days: Math.floor(secs / (3600 * 24)),
hours: Math.floor(secs % (3600 * 24) / 3600),
minutes: Math.floor(secs % 3600 / 60),
seconds: Math.floor(secs % 60)
seconds_to_duration(seconds, duration_options) {
const round = seconds > 0 ? Math.floor : Math.ceil;
const total_duration = {
days: round(seconds / 86400), // 60 * 60 * 24
hours: round(seconds % 86400 / 3600),
minutes: round(seconds % 3600 / 60),
seconds: round(seconds % 60)
};
if (duration_options.hide_days) {
total_duration.hours = Math.floor(secs / 3600);
total_duration.hours = round(seconds / 3600);
total_duration.days = 0;
}
return total_duration;
},

View file

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

View file

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

View file

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

View file

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