From fd0564953b0d5dd32d563617b1894e48d0aa8c55 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 15:30:46 +0530 Subject: [PATCH 1/4] fix(Kanban Board): Clear user settings & dpctype cache on_change Deletion of kanban boards may lead to 404s on clicking on the Kanban option in the views. This is due to the cache not being synced with the DB changes. Changed the hook to on_change since it gets triggered even in db_set. --- frappe/desk/doctype/kanban_board/kanban_board.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/desk/doctype/kanban_board/kanban_board.py b/frappe/desk/doctype/kanban_board/kanban_board.py index 97f529a061..a53045b096 100644 --- a/frappe/desk/doctype/kanban_board/kanban_board.py +++ b/frappe/desk/doctype/kanban_board/kanban_board.py @@ -12,8 +12,9 @@ class KanbanBoard(Document): def validate(self): self.validate_column_name() - def on_update(self): + def on_change(self): frappe.clear_cache(doctype=self.reference_doctype) + frappe.cache().delete_keys("_user_settings") def before_insert(self): for column in self.columns: From 540b34347c688b32ecc5538296f4c6a95dbab1ae Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 15:39:29 +0530 Subject: [PATCH 2/4] fix: Don't update saved filters "silently" Blame on the filters would not be maintained since update_modified was unset. Also, this API allowed those without access to Kanban Boards to possibly pass "bad" filters and cause system to fail (possible DOS). Using client's set_value does a better job at tackling this. --- frappe/desk/doctype/kanban_board/kanban_board.py | 7 ------- frappe/public/js/frappe/views/kanban/kanban_view.js | 8 +------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/frappe/desk/doctype/kanban_board/kanban_board.py b/frappe/desk/doctype/kanban_board/kanban_board.py index a53045b096..ddb1b9ffec 100644 --- a/frappe/desk/doctype/kanban_board/kanban_board.py +++ b/frappe/desk/doctype/kanban_board/kanban_board.py @@ -239,10 +239,3 @@ def set_indicator(board_name, column_name, indicator): board.save() return board - - -@frappe.whitelist() -def save_filters(board_name, filters): - '''Save filters silently''' - frappe.db.set_value('Kanban Board', board_name, 'filters', - filters, update_modified=False) diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index 89d1d41836..5ce6da9d55 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -107,13 +107,7 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { save_kanban_board_filters() { const filters = this.filter_area.get(); - frappe.call({ - method: 'frappe.desk.doctype.kanban_board.kanban_board.save_filters', - args: { - board_name: this.board_name, - filters: filters - } - }).then(r => { + frappe.db.set_value("Kanban Board", this.board_name, "filters", filters).then(r => { if (r.exc) { frappe.show_alert({ indicator: 'red', From 87c6cbeee2845e0b73b323afcf53c3d5143996e2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 17:30:17 +0530 Subject: [PATCH 3/4] fix: Check if kanban exists before switching route --- .../public/js/frappe/list/list_view_select.js | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view_select.js b/frappe/public/js/frappe/list/list_view_select.js index 54e88ea05b..475019d6c1 100644 --- a/frappe/public/js/frappe/list/list_view_select.js +++ b/frappe/public/js/frappe/list/list_view_select.js @@ -262,19 +262,27 @@ frappe.views.ListViewSelect = class ListViewSelect { setup_kanban_boards() { const last_opened_kanban = - frappe.model.user_settings[this.doctype]["Kanban"] && frappe.model.user_settings[this.doctype]["Kanban"] - .last_kanban_board; - if (last_opened_kanban) { - frappe.set_route( - "list", + ?.last_kanban_board; + + if (!last_opened_kanban) { + return frappe.views.KanbanView.show_kanban_dialog( this.doctype, - "kanban", - last_opened_kanban + true ); - } else { - frappe.views.KanbanView.show_kanban_dialog(this.doctype, true); } + frappe.db.exists("Kanban Board", last_opened_kanban).then(exists => { + if (exists) { + frappe.set_route( + "list", + this.doctype, + "kanban", + last_opened_kanban + ); + } else { + frappe.views.KanbanView.show_kanban_dialog(this.doctype, true); + } + }); } get_calendars() { From 13922966996d1013a3dcbdf937009800ef21adf5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 5 Apr 2022 17:31:13 +0530 Subject: [PATCH 4/4] refactor: new Kanban dialog * Hide 'Save' button if no select fields exist * Show 'Customize Form' as primary button when no selects exist --- .../js/frappe/views/kanban/kanban_view.js | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index 5ce6da9d55..3bf3a16189 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -247,25 +247,36 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) { } function new_kanban_dialog(kanbans, show_existing) { + /* Kanban dialog can show either "Save" or "Customize Form" option depending if any Select fields exist in the DocType for Kanban creation + */ if (dialog) return dialog; - const fields = get_fields_for_dialog(kanbans.map(kanban => kanban.name), show_existing); - - let primary_action_label = __('Save'); + const dialog_fields = get_fields_for_dialog(kanbans.map(kanban => kanban.name), show_existing); + const select_fields = frappe.get_meta(doctype).fields.filter(df => { + return (df.fieldtype === 'Select') && (df.fieldname !== 'kanban_column') + }); + const to_save = select_fields.length > 0; + const primary_action_label = to_save ? __('Save') : __('Customize Form'); let primary_action = () => { - const values = dialog.get_values(); - if (!values.selected_kanban || values.selected_kanban == 'Create New Board') { - make_kanban_board(values.board_name, values.field_name, values.project) - .then(() => dialog.hide(), (err) => frappe.msgprint(err)); + if (to_save) { + const values = dialog.get_values(); + if (!values.selected_kanban || values.selected_kanban == 'Create New Board') { + make_kanban_board(values.board_name, values.field_name, values.project).then( + () => dialog.hide(), + (err) => frappe.msgprint(err) + ); + } else { + frappe.set_route(kanbans.find(kanban => kanban.name == values.selected_kanban).route); + } } else { - frappe.set_route(kanbans.find(kanban => kanban.name == values.selected_kanban).route); + frappe.set_route("Form", "Customize Form", {"doc_type": doctype}); } }; dialog = new frappe.ui.Dialog({ title: __('New Kanban Board'), - fields, + fields: dialog_fields, primary_action_label, primary_action }); @@ -274,6 +285,9 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) { function get_fields_for_dialog(kanban_options, show_existing = false) { kanban_options.push('Create New Board'); + const select_fields = frappe.get_meta(doctype).fields.filter(df => { + return df.fieldtype === 'Select' && df.fieldname !== 'kanban_column'; + }); let fields = [ { @@ -284,6 +298,7 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) { depends_on: `eval: ${show_existing}`, mandatory_depends_on: `eval: ${show_existing}`, options: kanban_options, + default: kanban_options[0] }, { fieldname: 'new_kanban_board_sb', @@ -309,13 +324,6 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) { }); } - const select_fields = - frappe.get_meta(doctype).fields - .filter(df => { - return df.fieldtype === 'Select' && - df.fieldname !== 'kanban_column'; - }); - if (select_fields.length > 0) { fields.push({ fieldtype: 'Select', @@ -333,9 +341,6 @@ frappe.views.KanbanView.show_kanban_dialog = function (doctype, show_existing) {

${__('No fields found that can be used as a Kanban Column. Use the Customize Form to add a Custom Field of type "Select".')}

- - ${__('Customize Form')} - ` }];