Skip to content

Commit

Permalink
feat: let users modify hook resolution order (backport #19653) (#19774)
Browse files Browse the repository at this point in the history
* feat: let users modify hook resolution order

Since hook resolution depends on the order in which apps were installed
on site, it should be made configurable as escape hatch in case a
different resolution order is desired.

Keep in mind that changing order affects every hook, page, customization
so you can't pick and choose priority for individual hooks as of now.

Separate proposals are welcome for such configurabilty.

(cherry picked from commit 1796cae)

# Conflicts:
#	frappe/core/doctype/installed_applications/installed_applications.js
#	frappe/core/doctype/installed_applications/test_installed_applications.py

* fix: log changes made to installed_apps order

(cherry picked from commit c14379c)

* chore: conflicts

* fix(UX): drag handles on app resolution order table (#19672)

[skip ci]

Co-Authored-By: Suraj Shetty <surajshetty3416@gmail.com>

Co-authored-by: Suraj Shetty <surajshetty3416@gmail.com>

Co-authored-by: Ankush Menat <ankush@frappe.io>
Co-authored-by: Suraj Shetty <surajshetty3416@gmail.com>
  • Loading branch information
3 people committed Jan 25, 2023
1 parent 30729a2 commit 76e0974
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,67 @@
// Copyright (c) 2020, Frappe Technologies and contributors
// For license information, please see license.txt

frappe.ui.form.on('Installed Applications', {
// refresh: function(frm) {
frappe.ui.form.on("Installed Applications", {
refresh: function (frm) {
frm.add_custom_button(__("Update Hooks Resolution Order"), () => {
frm.trigger("show_update_order_dialog");
});
},

// }
show_update_order_dialog() {
const dialog = new frappe.ui.Dialog({
title: __("Update Hooks Resolution Order"),
fields: [
{
fieldname: "apps",
fieldtype: "Table",
label: __("Installed Apps"),
cannot_add_rows: true,
cannot_delete_rows: true,
in_place_edit: true,
data: [],
fields: [
{
fieldtype: "Data",
fieldname: "app_name",
label: __("App Name"),
in_list_view: 1,
read_only: 1,
},
],
},
],
primary_action: function () {
const new_order = this.get_values()["apps"].map((row) => row.app_name);
frappe.call({
method: "frappe.core.doctype.installed_applications.installed_applications.update_installed_apps_order",
freeze: true,
args: {
new_order: new_order,
},
});
this.hide();
},
primary_action_label: __("Update Order"),
});

frappe
.xcall(
"frappe.core.doctype.installed_applications.installed_applications.get_installed_app_order"
)
.then((data) => {
data.forEach((app) => {
dialog.fields_dict.apps.df.data.push({
app_name: app,
});
});

dialog.fields_dict.apps.grid.refresh();
// hack: change checkboxes to drag handles.
let grid = $(dialog.fields_dict.apps.grid.parent);
grid.find(".grid-row-check:first").remove() &&
grid.find(".grid-row-check").replaceWith(frappe.utils.icon("menu"));
dialog.show();
});
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,17 @@

from __future__ import unicode_literals

import json

import frappe
from frappe import _
from frappe.model.document import Document


class InvalidAppOrder(frappe.ValidationError):
pass


class InstalledApplications(Document):
def update_versions(self):
self.delete_key("installed_applications")
Expand All @@ -21,3 +28,51 @@ def update_versions(self):
},
)
self.save()


@frappe.whitelist()
def update_installed_apps_order(new_order: list[str] | str):
"""Change the ordering of `installed_apps` global
This list is used to resolve hooks and by default it's order of installation on site.
Sometimes it might not be the ordering you want, so thie function is provided to override it.
"""
frappe.only_for("System Manager")

if isinstance(new_order, str):
new_order = json.loads(new_order)

frappe.local.request_cache and frappe.local.request_cache.clear()
existing_order = frappe.get_installed_apps(_ensure_on_bench=True)

if set(existing_order) != set(new_order) or not isinstance(new_order, list):
frappe.throw(
_("You are only allowed to update order, do not remove or add apps."), exc=InvalidAppOrder
)

# Ensure frappe is always first regardless of user's preference.
if "frappe" in new_order:
new_order.remove("frappe")
new_order.insert(0, "frappe")

frappe.db.set_global("installed_apps", json.dumps(new_order))

_create_version_log_for_change(existing_order, new_order)


def _create_version_log_for_change(old, new):
version = frappe.new_doc("Version")
version.ref_doctype = "DefaultValue"
version.docname = "installed_apps"
version.data = frappe.as_json({"changed": [["current", json.dumps(old), json.dumps(new)]]})
version.flags.ignore_links = True # This is a fake doctype
version.flags.ignore_permissions = True
version.insert()


@frappe.whitelist()
def get_installed_app_order() -> list[str]:
frappe.only_for("System Manager")

return frappe.get_installed_apps(_ensure_on_bench=True)
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
# -*- coding: utf-8 -*-
# Copyright (c) 2020, Frappe Technologies and Contributors
# See license.txt
from __future__ import unicode_literals
# License: MIT. See LICENSE

# import frappe
import unittest
import frappe
from frappe.core.doctype.installed_applications.installed_applications import (
InvalidAppOrder,
update_installed_apps_order,
)
from frappe.tests.utils import FrappeTestCase


class TestInstalledApplications(unittest.TestCase):
pass
class TestInstalledApplications(FrappeTestCase):
def test_order_change(self):
update_installed_apps_order(["frappe"])
self.assertRaises(InvalidAppOrder, update_installed_apps_order, [])
self.assertRaises(InvalidAppOrder, update_installed_apps_order, ["frappe", "deepmind"])

0 comments on commit 76e0974

Please sign in to comment.