From 37116bf3fcfbb47910c813e6246c1f0dc03b1496 Mon Sep 17 00:00:00 2001
From: prssanna <prssud@gmail.com>
Date: Sat, 29 Feb 2020 13:32:48 +0530
Subject: [PATCH 1/4] feat: allow child table fields in aggregate on

---
 .../public/js/frappe/ui/group_by/group_by.js  | 62 ++++++++++++-------
 .../js/frappe/views/reports/report_view.js    |  2 +-
 2 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/frappe/public/js/frappe/ui/group_by/group_by.js b/frappe/public/js/frappe/ui/group_by/group_by.js
index 98d4531c5e..8303ebb119 100644
--- a/frappe/public/js/frappe/ui/group_by/group_by.js
+++ b/frappe/public/js/frappe/ui/group_by/group_by.js
@@ -51,21 +51,30 @@ frappe.ui.GroupBy = class {
 	}
 
 	show_hide_aggregate_on() {
-		this.report_view.meta.fields.forEach((field) => {
-			let fn = this.aggregate_function_select.val();
-			if(fn === 'sum' || fn === 'avg') {
-				// pick numeric fields for sum / avg
-				if(frappe.model.is_numeric_field(field.fieldtype)) {
-					this.aggregate_on_select.append(
-						$('<option>', { value : field.fieldname })
-							.text(field.label));
+
+		for (let doctype in this.all_fields) {
+			const doctype_fields = this.all_fields[doctype];
+			doctype_fields.forEach(field => {
+				let fn = this.aggregate_function_select.val();
+				if(fn === 'sum' || fn === 'avg') {
+					// pick numeric fields for sum / avg
+					if(frappe.model.is_numeric_field(field.fieldtype)) {
+						let option_text = doctype == this.doctype
+						? field.label
+						: `${field.label} (${doctype})`
+						this.aggregate_on_select.append(
+							$(`<option data-doctype="${doctype}" 
+								value="${field.fieldname}">>`,
+								{ value : field.fieldname })
+								.text(option_text));
+					}
+					this.aggregate_on_select.show();
+				} else {
+					// count, so no aggregate function
+					this.aggregate_on_select.hide();
 				}
-				this.aggregate_on_select.show();
-			} else {
-				// count, so no aggregate function
-				this.aggregate_on_select.hide();
-			}
-		});
+			});
+		}
 	}
 
 	get_settings() {
@@ -108,6 +117,7 @@ frappe.ui.GroupBy = class {
 
 	apply_group_by() {
 		this.group_by_doctype = this.groupby_select.find(':selected').attr('data-doctype');
+		this.aggregate_on_doctype = this.aggregate_on_select.find(':selected').attr('data-doctype');
 		this.group_by_field = this.groupby_select.val();
 		this.group_by = '`tab' + this.group_by_doctype + '`.`' + this.group_by_field + '`';
 		this.aggregate_function = this.aggregate_function_select.val();
@@ -144,10 +154,11 @@ frappe.ui.GroupBy = class {
 	set_args(args) {
 		if (this.aggregate_function && this.group_by) {
 			let aggregate_column;
-			if(this.aggregate_function === 'count') {
+			if (this.aggregate_function === 'count') {
 				aggregate_column = 'count(`tab'+ this.doctype + '`.`name`)';
 			} else {
-				aggregate_column = `${this.aggregate_function}(\`tab${this.doctype}\`.\`${this.aggregate_on}\`)`;
+				aggregate_column = 
+					`${this.aggregate_function}(\`tab${this.aggregate_on_doctype}\`.\`${this.aggregate_on}\`)`;
 			}
 
 			this.report_view.group_by = this.group_by;
@@ -167,7 +178,9 @@ frappe.ui.GroupBy = class {
 			args.fields = this.report_view.get_fields();
 
 			// add aggregate column in both query args and report view
-			this.report_view.fields.push(['_aggregate_column', this.doctype]);
+			this.report_view.fields.push(['_aggregate_column', this.aggregate_on_doctype]);
+			let aggregate_on_field = '`tab' + this.aggregate_on_doctype + '`.`' + this.aggregate_on + '`';
+			args.fields.push(aggregate_on_field);
 			args.fields.push(aggregate_column + ' as _aggregate_column');
 
 			// setup columns in datatable
@@ -195,7 +208,7 @@ frappe.ui.GroupBy = class {
 			};
 		} else {
 			// get properties of "aggregate_on", for example Net Total
-			docfield = Object.assign({}, frappe.meta.docfield_map[this.doctype][this.aggregate_on]);
+			docfield = Object.assign({}, frappe.meta.docfield_map[this.aggregate_on_doctype][this.aggregate_on]);
 			if (this.aggregate_function === 'sum') {
 				docfield.label = __('Sum of {0}', [docfield.label]);
 			} else {
@@ -230,9 +243,12 @@ frappe.ui.GroupBy = class {
 	}
 
 	get_group_by_fields() {
-		let group_by_fields = {};
+		this.group_by_fields = {};
+		this.all_fields = {};
+
 		let fields = this.report_view.meta.fields.filter(f => ["Select", "Link", "Data", "Int"].includes(f.fieldtype));
-		group_by_fields[this.doctype] = fields;
+		this.group_by_fields[this.doctype] = fields;
+		this.all_fields[this.doctype] = this.report_view.meta.fields;
 
 		const standard_fields_filter = df =>
 			!in_list(frappe.model.no_value_type, df.fieldtype) && !df.report_hide;
@@ -243,10 +259,10 @@ frappe.ui.GroupBy = class {
 		table_fields.forEach(df => {
 			const cdt = df.options;
 			const child_table_fields = frappe.meta.get_docfields(cdt).filter(standard_fields_filter);
-			group_by_fields[cdt] = child_table_fields;
+			this.group_by_fields[cdt] = child_table_fields;
+			this.all_fields[cdt] = child_table_fields;
 		});
-
-		return group_by_fields;
+		return this.group_by_fields;
 	}
 
 };
diff --git a/frappe/public/js/frappe/views/reports/report_view.js b/frappe/public/js/frappe/views/reports/report_view.js
index 59b37f504d..602c88c3b4 100644
--- a/frappe/public/js/frappe/views/reports/report_view.js
+++ b/frappe/public/js/frappe/views/reports/report_view.js
@@ -1034,7 +1034,7 @@ frappe.views.ReportView = class ReportView extends frappe.views.ListView {
 				return {
 					name: name,
 					doctype: col.docfield.parent,
-					content: d[cdt_field(col.field)],
+					content: d[cdt_field(col.field)] || d[col.field],
 					editable: Boolean(name && this.is_editable(col.docfield, d)),
 					format: value => {
 						return frappe.format(value, col.docfield, { always_show_decimals: true }, d);

From b9387cc449a16903e6b440fdf92174d91fb3f721 Mon Sep 17 00:00:00 2001
From: prssanna <prssud@gmail.com>
Date: Sat, 29 Feb 2020 13:45:05 +0530
Subject: [PATCH 2/4] fix: only set aggregate on doctype if function is not sum

---
 frappe/public/js/frappe/ui/group_by/group_by.js | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/frappe/public/js/frappe/ui/group_by/group_by.js b/frappe/public/js/frappe/ui/group_by/group_by.js
index 8303ebb119..573479cfa3 100644
--- a/frappe/public/js/frappe/ui/group_by/group_by.js
+++ b/frappe/public/js/frappe/ui/group_by/group_by.js
@@ -117,15 +117,16 @@ frappe.ui.GroupBy = class {
 
 	apply_group_by() {
 		this.group_by_doctype = this.groupby_select.find(':selected').attr('data-doctype');
-		this.aggregate_on_doctype = this.aggregate_on_select.find(':selected').attr('data-doctype');
 		this.group_by_field = this.groupby_select.val();
 		this.group_by = '`tab' + this.group_by_doctype + '`.`' + this.group_by_field + '`';
 		this.aggregate_function = this.aggregate_function_select.val();
 
 		if (this.aggregate_function === 'count') {
 			this.aggregate_on = 'name';
+			this.aggregate_on_doctype = null;
 		} else {
 			this.aggregate_on = this.aggregate_on_select.val();
+			this.aggregate_on_doctype = this.aggregate_on_select.find(':selected').attr('data-doctype');
 		}
 
 
@@ -153,12 +154,13 @@ frappe.ui.GroupBy = class {
 
 	set_args(args) {
 		if (this.aggregate_function && this.group_by) {
-			let aggregate_column;
+			let aggregate_column, aggregate_on_field;
 			if (this.aggregate_function === 'count') {
 				aggregate_column = 'count(`tab'+ this.doctype + '`.`name`)';
 			} else {
 				aggregate_column = 
 					`${this.aggregate_function}(\`tab${this.aggregate_on_doctype}\`.\`${this.aggregate_on}\`)`;
+				aggregate_on_field = '`tab' + this.aggregate_on_doctype + '`.`' + this.aggregate_on + '`';
 			}
 
 			this.report_view.group_by = this.group_by;
@@ -177,12 +179,14 @@ frappe.ui.GroupBy = class {
 			// rebuild fields for group by
 			args.fields = this.report_view.get_fields();
 
-			// add aggregate column in both query args and report view
-			this.report_view.fields.push(['_aggregate_column', this.aggregate_on_doctype]);
-			let aggregate_on_field = '`tab' + this.aggregate_on_doctype + '`.`' + this.aggregate_on + '`';
-			args.fields.push(aggregate_on_field);
+			// add aggregate column in both query args and report views
+			this.report_view.fields.push(['_aggregate_column', this.aggregate_on_doctype || this.doctype]);
 			args.fields.push(aggregate_column + ' as _aggregate_column');
 
+			if (aggregate_on_field) {
+				args.fields.push(aggregate_on_field);
+			}
+
 			// setup columns in datatable
 			this.report_view.setup_columns();
 

From 16a47820a3ce8175bb2928849e9b65caa1cbc898 Mon Sep 17 00:00:00 2001
From: prssanna <prssud@gmail.com>
Date: Sat, 29 Feb 2020 14:26:52 +0530
Subject: [PATCH 3/4] fix: codacy

---
 frappe/public/js/frappe/ui/group_by/group_by.js | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/frappe/public/js/frappe/ui/group_by/group_by.js b/frappe/public/js/frappe/ui/group_by/group_by.js
index 573479cfa3..a90f03ba55 100644
--- a/frappe/public/js/frappe/ui/group_by/group_by.js
+++ b/frappe/public/js/frappe/ui/group_by/group_by.js
@@ -60,13 +60,12 @@ frappe.ui.GroupBy = class {
 					// pick numeric fields for sum / avg
 					if(frappe.model.is_numeric_field(field.fieldtype)) {
 						let option_text = doctype == this.doctype
-						? field.label
-						: `${field.label} (${doctype})`
+							? field.label
+							: `${field.label} (${doctype})`;
 						this.aggregate_on_select.append(
 							$(`<option data-doctype="${doctype}" 
-								value="${field.fieldname}">>`,
-								{ value : field.fieldname })
-								.text(option_text));
+							value="${field.fieldname}">>`,
+							{ value: field.fieldname }).text(option_text));
 					}
 					this.aggregate_on_select.show();
 				} else {

From ecde921c8a0142c965c61b7407a94ef45a6da37b Mon Sep 17 00:00:00 2001
From: prssanna <prssud@gmail.com>
Date: Mon, 2 Mar 2020 21:19:04 +0530
Subject: [PATCH 4/4] fix: fix aggregate on select field html

---
 .../public/js/frappe/ui/group_by/group_by.js  | 45 ++++++++++---------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/frappe/public/js/frappe/ui/group_by/group_by.js b/frappe/public/js/frappe/ui/group_by/group_by.js
index a90f03ba55..6141d5a89a 100644
--- a/frappe/public/js/frappe/ui/group_by/group_by.js
+++ b/frappe/public/js/frappe/ui/group_by/group_by.js
@@ -25,7 +25,7 @@ frappe.ui.GroupBy = class {
 		this.groupby_select = this.groupby_edit_area.find('select.groupby');
 		this.aggregate_function_select = this.groupby_edit_area.find('select.aggregate-function');
 		this.aggregate_on_select = this.groupby_edit_area.find('select.aggregate-on');
-
+		this.aggregate_on_html = ``;
 		// set default to count
 		this.aggregate_function_select.val("count");
 		this.page.wrapper.find(".frappe-list").append(
@@ -51,28 +51,31 @@ frappe.ui.GroupBy = class {
 	}
 
 	show_hide_aggregate_on() {
+		let fn = this.aggregate_function_select.val();
+		if (fn === 'sum' || fn === 'avg') {
+			if (!this.aggregate_on_html.length) {
+				this.aggregate_on_html = `<option value="" disabled selected>
+					${__("Select Field...")}</option>`;
 
-		for (let doctype in this.all_fields) {
-			const doctype_fields = this.all_fields[doctype];
-			doctype_fields.forEach(field => {
-				let fn = this.aggregate_function_select.val();
-				if(fn === 'sum' || fn === 'avg') {
-					// pick numeric fields for sum / avg
-					if(frappe.model.is_numeric_field(field.fieldtype)) {
-						let option_text = doctype == this.doctype
-							? field.label
-							: `${field.label} (${doctype})`;
-						this.aggregate_on_select.append(
-							$(`<option data-doctype="${doctype}" 
-							value="${field.fieldname}">>`,
-							{ value: field.fieldname }).text(option_text));
-					}
-					this.aggregate_on_select.show();
-				} else {
-					// count, so no aggregate function
-					this.aggregate_on_select.hide();
+				for (let doctype in this.all_fields) {
+					const doctype_fields = this.all_fields[doctype];
+					doctype_fields.forEach(field => {
+						// pick numeric fields for sum / avg
+						if (frappe.model.is_numeric_field(field.fieldtype)) {
+							let option_text = doctype == this.doctype
+								? field.label
+								: `${field.label} (${doctype})`;
+							this.aggregate_on_html+= `<option data-doctype="${doctype}"
+								value="${field.fieldname}">${option_text}</option>`;
+						}
+					});
 				}
-			});
+			}
+			this.aggregate_on_select.html(this.aggregate_on_html);
+			this.aggregate_on_select.show();
+		} else {
+			// count, so no aggregate function
+			this.aggregate_on_select.hide();
 		}
 	}