Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New permission for wiki space #217

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
69 changes: 47 additions & 22 deletions wiki/wiki/doctype/wiki_page/wiki_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ def update_page(self, title, content, edit_message, raised_by=None):
self.save()

def verify_permission(self, permtype):
permitted = frappe.has_permission(self.doctype, permtype, self)
if permtype == "read" and self.allow_guest:
return True
permitted = frappe.has_permission(self.doctype, permtype, self)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change may represent a potential performance regression for no obvious reason. has_permission is not the cheapest of methods to run on anonymous requests.

if not permitted:
action = permtype
if action == "write":
Expand Down Expand Up @@ -216,6 +216,23 @@ def calculate_toc_html(self, html):
def get_context(self, context):
self.verify_permission("read")
self.set_breadcrumbs(context)
ccc = frappe.db.sql(""" select user, allow, for_value,name from `tabUser Permission` where user=%s and allow='Wiki Space' """,(frappe.session.user))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ccc = frappe.db.sql(""" select user, allow, for_value,name from `tabUser Permission` where user=%s and allow='Wiki Space' """,(frappe.session.user))
ccc = frappe.db.get_value("User Permission", {"user": frappe.session.user, "allow": "Wiki Space"}, ["user", "allow", "for_value", "name"])

Try to use database api and query builder as much as possible instead of raw sql
Also use better variable names. ccc is very ambigous

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have to rewrite it everywhere you're using frappe.db.sql

print(self.name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove print statements

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing for everywhere you've used print statements

match_found = False
for c in ccc:
ws = frappe.db.sql(""" select w.wiki_page from `tabWiki Group Item` as w where parent=%s""",(c[2]))
for a in ws:
print(a[0])
if a[0] == self.name:
match_found = True
break
if match_found:
print("ok")
else:
frappe.local.response["type"] = "redirect"
frappe.local.response["location"] = "/"
raise frappe.Redirect
print("++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++")
wiki_settings = frappe.get_single("Wiki Settings")
context.navbar_search = wiki_settings.add_search_bar
context.add_dark_mode = wiki_settings.add_dark_mode
Expand Down Expand Up @@ -283,6 +300,7 @@ def get_context(self, context):

def get_items(self, sidebar_items):
topmost = frappe.get_value("Wiki Group Item", {"wiki_page": self.name}, ["parent"])
wikiSpace = frappe.db.sql("""select name from `tabWiki Space`""")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I said please don't use raw sql unless absolutely necessary.
Also wouldn't this fetch all wiki spaces? Do we need it?

Also try to use snake case for variables. ie wiki_space.


sidebar_html = frappe.cache().hget("wiki_sidebar", topmost)
if not sidebar_html or frappe.conf.disable_website_cache or frappe.conf.developer_mode:
Expand All @@ -294,6 +312,7 @@ def get_items(self, sidebar_items):
context.current_route = self.route
context.collapse_sidebar_groups = wiki_settings.collapse_sidebar_groups
context.sidebar_items = sidebar_items
context.wikiSpace = wikiSpace
context.wiki_search_scope = self.get_space_route()
sidebar_html = frappe.render_template(
"wiki/wiki/doctype/wiki_page/templates/web_sidebar.html", context
Expand All @@ -304,30 +323,36 @@ def get_items(self, sidebar_items):

def get_sidebar_items(self):
wiki_sidebar = frappe.get_doc("Wiki Space", {"route": self.get_space_route()}).wiki_sidebars
check = frappe.db.sql(""" select user, allow, for_value,name from `tabUser Permission` where user=%s and allow='Wiki Space' """,(frappe.session.user))
print("==================================================================")
sidebar = {}

for sidebar_item in wiki_sidebar:
wiki_page = frappe.get_doc("Wiki Page", sidebar_item.wiki_page)
if sidebar_item.parent_label not in sidebar:
sidebar[sidebar_item.parent_label] = [
{
"name": wiki_page.name,
"type": "Wiki Page",
"title": wiki_page.title,
"route": wiki_page.route,
"group_name": sidebar_item.parent_label,
}
]
else:
sidebar[sidebar_item.parent_label] += [
{
"name": wiki_page.name,
"type": "Wiki Page",
"title": wiki_page.title,
"route": wiki_page.route,
"group_name": sidebar_item.parent_label,
}
]
for c in check:
if c and c[2] and c[2] == sidebar_item.parent:
wiki_page = frappe.get_doc("Wiki Page", sidebar_item.wiki_page)
if sidebar_item.parent_label not in sidebar:
sidebar[sidebar_item.parent_label] = [
{
"name": wiki_page.name,
"type": "Wiki Page",
"title": wiki_page.title,
"route": wiki_page.route,
"group_name": sidebar_item.parent_label,
}
]
else:
sidebar[sidebar_item.parent_label] += [
{
"name": wiki_page.name,
"type": "Wiki Page",
"title": wiki_page.title,
"route": wiki_page.route,
"group_name": sidebar_item.parent_label,
}
]
else:
print("_________________________________________________")

return self.get_items(sidebar)

Expand Down