[fix] filters for calendars frappe/erpnext#9850 (#3686)

* [fix] filters for calendars frappe/erpnext#9850

* [add] tests

frappe/core/doctype/test_runner/test_runner.js

@@ -62,7 +62,6 @@ frappe.ui.form.on('Test Runner', {
 
 			QUnit.done(({ total, failed, passed, runtime }) => {
 				// flag for selenium that test is done
-				$('<div id="frappe-qunit-done"></div>').appendTo($('body'));
 
 				console.log( `Total: ${total}, Failed: ${failed}, Passed: ${passed}, Runtime: ${runtime}` );  // eslint-disable-line
 
@@ -72,6 +71,9 @@ frappe.ui.form.on('Test Runner', {
 					console.log('Tests Passed'); // eslint-disable-line
 				}
 				frappe.set_route('Form', 'Test Runner', 'Test Runner');
+
+				$('<div id="frappe-qunit-done"></div>').appendTo($('body'));
+
 			});
 		});
 

frappe/desk/calendar.py

@@ -19,16 +19,8 @@ def update_event(args, field_map):
 
 def get_event_conditions(doctype, filters=None):
 	"""Returns SQL conditions with user permissions and filters for event queries"""
-	from frappe.desk.reportview import build_match_conditions
+	from frappe.desk.reportview import get_filters_cond
 	if not frappe.has_permission(doctype):
 		frappe.throw(_("Not Permitted"), frappe.PermissionError)
 
-	conditions = build_match_conditions(doctype)
-	conditions = conditions and (" and " + conditions) or ""
-	if filters:
-		filters = json.loads(filters)
-		for key in filters:
-			if filters[key]:
-				conditions += 'and `{0}` = "{1}"'.format(frappe.db.escape(key), frappe.db.escape(filters[key]))
-
-	return conditions
+	return get_filters_cond(doctype, filters, [], with_match_conditions = True)

frappe/desk/reportview.py

@@ -334,7 +334,7 @@ def build_match_conditions(doctype, as_condition=True):
 	else:
 		return match_conditions
 
-def get_filters_cond(doctype, filters, conditions, ignore_permissions=None):
+def get_filters_cond(doctype, filters, conditions, ignore_permissions=None, with_match_conditions=False):
 	if filters:
 		flt = filters
 		if isinstance(filters, dict):
@@ -350,6 +350,10 @@ def get_filters_cond(doctype, filters, conditions, ignore_permissions=None):
 		query = DatabaseQuery(doctype)
 		query.filters = flt
 		query.conditions = conditions
+
+		if with_match_conditions:
+			query.build_match_conditions()
+
 		query.build_filter_conditions(flt, conditions, ignore_permissions)
 
 		cond = ' and ' + ' and '.join(query.conditions)

frappe/public/js/frappe/ui/page.js

@@ -113,13 +113,20 @@ frappe.ui.Page = Class.extend({
 	},
 
 	set_action: function(btn, opts) {
+		let me = this;
 		if (opts.icon) {
 			opts.label = this.get_icon_label(opts.icon, opts.label);
 		}
 
 		this.clear_action_of(btn);
 
-		btn.removeClass("hide").prop("disabled", false).html(opts.label).on("click", opts.click);
+		btn.removeClass("hide")
+			.prop("disabled", false)
+			.html(opts.label)
+			.on("click", function() {
+				let response = opts.click.apply(this);
+				me.btn_disable_enable(btn, response);
+			});
 
 		if (opts.working_label) {
 			btn.attr("data-working-label", opts.working_label);
@@ -250,31 +257,35 @@ frappe.ui.Page = Class.extend({
 		this.get_inner_group_button(label).find("button").removeClass("btn-default").addClass("btn-primary");
 	},
 
+	btn_disable_enable: function(btn, response) {
+		if (response && response.then) {
+			btn.prop('disabled', true);
+			response.then(() => {
+				btn.prop('disabled', false);
+			})
+		} else if (response && response.always) {
+			btn.prop('disabled', true);
+			response.always(() => {
+				btn.prop('disabled', false);
+			});
+		}
+	},
+
 	add_inner_button: function(label, action, group) {
 		let _action = function() {
 			let btn = $(this);
-			let _ret = action();
-			if (_ret && _ret.then) {
-				// returns a promise
-				btn.attr('disabled', true);
-				_ret.then(() => {
-					btn.attr('disabled', false);
-				})
-			}
-			if (_ret && _ret.always) {
-				// returns frappe.call ($.ajax)
-				btn.attr('disabled', true);
-				_ret.always(() => {
-					btn.attr('disabled', false);
-				});
-			}
-		}
+			let response = action();
+			this.btn_disable_enable(btn, response);
+		};
 		if(group) {
 			var $group = this.get_inner_group_button(group);
-			return $('<li><a>'+label+'</a></li>').on('click', _action).appendTo($group.find(".dropdown-menu"));
+			return $('<li><a>'+label+'</a></li>')
+				.on('click', _action)
+				.appendTo($group.find(".dropdown-menu"));
 		} else {
 			return $('<button class="btn btn-default btn-xs" style="margin-left: 10px;">'+__(label)+'</btn>')
-				.on("click", _action).appendTo(this.inner_toolbar.removeClass("hide"))
+				.on("click", _action)
+				.appendTo(this.inner_toolbar.removeClass("hide"));
 		}
 	},
 

frappe/public/js/frappe/views/calendar/calendar.js

@@ -12,7 +12,7 @@ frappe.views.CalendarView = frappe.views.ListRenderer.extend({
 			doctype: this.doctype,
 			parent: this.wrapper,
 			page: this.list_view.page,
-			filter_vals: this.list_view.filter_list.get_filters()
+			list_view: this.list_view
 		}
 		$.extend(options, frappe.views.calendar[this.doctype]);
 		this.calendar = new frappe.views.Calendar(options);
@@ -45,6 +45,7 @@ frappe.views.Calendar = Class.extend({
 		var me = this;
 
 		// add links to other calendars
+		me.page.clear_user_actions();
 		$.each(frappe.boot.calendars, function(i, doctype) {
 			if(frappe.model.can_read(doctype)) {
 				me.page.add_menu_item(__(doctype), function() {
@@ -196,19 +197,11 @@ frappe.views.Calendar = Class.extend({
 		}
 	},
 	get_args: function(start, end) {
-		if(this.filter_vals) {
-			var filters = {};
-			this.filter_vals.forEach(function(f) {
-				if(f[2]==="=") {
-					filters[f[1]] = f[3];
-				}
-			});
-		}
 		var args = {
 			doctype: this.doctype,
 			start: this.get_system_datetime(start),
 			end: this.get_system_datetime(end),
-			filters: filters
+			filters: this.list_view.filter_list.get_filters()
 		};
 		return args;
 	},
@@ -309,50 +302,5 @@ frappe.views.Calendar = Class.extend({
 			event.start = event.start ? $.fullCalendar.moment(event.start).stripTime() : null;
 			event.end = event.end ? $.fullCalendar.moment(event.end).add(1, "day").stripTime() : null;
 		}
-	},
-	add_filters: function() {
-		var me = this;
-		if(this.filters) {
-			$.each(this.filters, function(i, df) {
-				df.change = function() {
-					me.refresh();
-				};
-				me.page.add_field(df);
-			});
-		}
-	},
-	set_filter: function(doctype, value) {
-		var me = this;
-		if(this.filters) {
-			$.each(this.filters, function(i, df) {
-				if(df.options===value)
-					me.page.fields_dict[df.fieldname].set_input(value);
-				return false;
-			});
-		}
-	},
-	get_filters: function() {
-		var filter_vals = {},
-			me = this;
-		if(this.filters) {
-			$.each(this.filters, function(i, df) {
-				filter_vals[df.fieldname || df.label] =
-					me.page.fields_dict[df.fieldname || df.label].get_value();
-			});
-		}
-		return filter_vals;
-	},
-	set_filters_from_route_options: function() {
-		var me = this;
-		if(frappe.route_options) {
-			$.each(frappe.route_options, function(k, value) {
-				if(me.page.fields_dict[k]) {
-					me.page.fields_dict[k].set_input(value);
-				}
-			})
-			frappe.route_options = null;
-			me.refresh();
-			return false;
-		}
 	}
 })

frappe/tests/ui/test_calendar_view.js

@@ -1,88 +1,81 @@
 QUnit.module('views');
 
 QUnit.test("Calendar View Tests", function(assert) {
-	assert.expect(6);
+	assert.expect(4);
 	let done = assert.async();
-	let random_text = frappe.utils.get_random(10);
+	let random_text = frappe.utils.get_random(3);
 	let today = frappe.datetime.get_today()+" 16:20:35"; //arbitrary value taken to prevent cases like 12a for 12:00am and 12h to 24h conversion
 	let visible_time = () => {
-		// Method to return the start-time (hours) of the event visible  
-		return $('.fc-time').text().split('p')[0]; // 'p' because the arbitrary time is pm 
+		// Method to return the start-time (hours) of the event visible
+		return $('.fc-time').text().split('p')[0]; // 'p' because the arbitrary time is pm
 	};
 	let event_title_text = () => {
-		// Method to return the title of the event visible  
+		// Method to return the title of the event visible
 		return $('.fc-title:visible').text();
 	};
 
 	frappe.run_serially([
-		// Create an event using the frappe API
+		// create 2 events, one private, one public
 		() => frappe.tests.make("Event", [
-			{subject: random_text},
+			{subject: random_text + ':Pri'},
 			{starts_on: today},
 			{event_type: 'Private'}
 		]),
-		
+
+		() => frappe.tests.make("Event", [
+			{subject: random_text + ':Pub'},
+			{starts_on: today},
+			{event_type: 'Public'}
+		]),
+
 		// Goto Calendar view
 		() => frappe.set_route(["List", "Event", "Calendar"]),
-		() => frappe.tests.click_page_head_item("Refresh"),
+		() => {
+			// clear filter
+			$('[data-fieldname="event_type"]').val('').trigger('change');
+		},
 		() => frappe.timeout(2),
 		// Check if event is created
 		() => {
 			// Check if the event exists and if its title matches with the one created
-			assert.ok(event_title_text().includes(random_text), "Event title verified");
+			assert.ok(event_title_text().includes(random_text + ':Pri'),
+				"Event title verified");
 			// Check if time of event created is correct
-			assert.ok(visible_time().includes("4:20"), "Event start time verified");
+			assert.ok(visible_time().includes("4:20"),
+				"Event start time verified");
 		},
 
-		// Delete event 
-		// Goto Calendar view
-		() => frappe.set_route(["List", "Event", "Calendar"]),
-		() => frappe.timeout(1),
-		// Open the event to be deleted
-		() => frappe.tests.click_generic_text(random_text),
-		() => frappe.tests.click_page_head_item('Menu'),
-		() => frappe.tests.click_dropdown_item('Delete'),
-		() => frappe.tests.click_page_head_item('Yes'),
-		() => frappe.timeout(1),
-		() => frappe.tests.click_page_head_item("Refresh"),
-		() => frappe.timeout(1),
-		// Goto Calendar View
-		() => frappe.set_route(["List", "Event", "Calendar"]),
+		// check filter
+		() => {
+			$('[data-fieldname="event_type"]').val('Public').trigger('change');
+		},
 		() => frappe.timeout(1),
+		() => {
+			// private event should be hidden
+			assert.notOk(event_title_text().includes(random_text + ':Pri'),
+				"Event title verified");
+		},
 
-		// Check if all menu items redirect to correct locations
-		// Check if clicking on 'Import' redirects you to ["data-import-tool"]
-		() => frappe.tests.click_page_head_item('Menu'),
-		() => frappe.tests.click_dropdown_item('Import'),
-		() => assert.deepEqual(["data-import-tool"], frappe.get_route(), "Routed to 'data-import-tool' by clicking on 'Import'"),
-		() => frappe.set_route(["List", "Event", "Calendar"]),
-		() => frappe.timeout(1),
-			
-		// Check if clicking on 'User Permissions Manager' redirects you to ["user-permissions"]
-		() => frappe.tests.click_page_head_item('Menu'),
-		() => frappe.tests.click_dropdown_item('User Permissions Manager'),
-		() => assert.deepEqual(["user-permissions"], frappe.get_route(), "Routed to 'user-permissions' by clicking on 'User Permissions Manager'"),
+		// Delete event
+		// Goto Calendar view
 		() => frappe.set_route(["List", "Event", "Calendar"]),
 		() => frappe.timeout(1),
-			
-		// Check if clicking on 'Role Permissions Manager' redirects you to ["permission-manager"]
-		() => frappe.tests.click_page_head_item('Menu'),
-		() => frappe.tests.click_dropdown_item('Role Permissions Manager'),
-		() => assert.deepEqual(["permission-manager"], frappe.get_route(), "Routed to 'permission-manager' by clicking on 'Role Permissions Manager'"),
+		// delete event
+		() => frappe.tests.click_generic_text(random_text + ':Pub'),
+		() => {
+			frappe.tests.click_page_head_item('Menu');
+			frappe.tests.click_dropdown_item('Delete');
+		},
+		() => frappe.timeout(0.5),
+		() => frappe.tests.click_button('Yes'),
+		() => frappe.timeout(2),
 		() => frappe.set_route(["List", "Event", "Calendar"]),
+		() => frappe.tests.click_button("Refresh"),
 		() => frappe.timeout(1),
-			
-		// // Check if clicking on 'Customize' redirects you to ["Form", "Customize Form"]
-		// *** ERROR HERE IN FRAPPE: UNCOMMENT THIS ONCE THAT IS RESOLVED *** //
-		// () => frappe.tests.click_page_head_item('Menu'),
-		// () => frappe.tests.click_dropdown_item('Customize'),
-		// () => assert.deepEqual(["Form", "Customize Form"], frappe.get_route(), "Routed to 'Form, Customize Form' by clicking on 'Customize'"),
-		// () => frappe.set_route(["List", "Event", "Calendar"]),
-		// () => frappe.timeout(3),
 
 		// Check if event is deleted
-		() => assert.notOk(event_title_text().includes(random_text), "Event deleted"),
-
+		() => assert.notOk(event_title_text().includes(random_text + ':Pub'),
+			"Event deleted"),
 		() => done()
 	]);
 });