Merge pull request #26198 from nextchamp-saqib/pos-fixes-9-v13

refactor(pos): use pos invoice item name as unique identifier

erpnext/selling/page/point_of_sale/pos_controller.js

@@ -241,8 +241,8 @@ erpnext.PointOfSale.Controller = class {
 			events: {
 				get_frm: () => this.frm,
 
-				cart_item_clicked: (item_code, batch_no, uom, rate) => {
-					const item_row = this.get_item_from_frm(item_code, batch_no, uom, rate);
+				cart_item_clicked: (item) => {
+					const item_row = this.get_item_from_frm(item);
 					this.item_details.toggle_item_details_section(item_row);
 				},
 
@@ -273,17 +273,15 @@ erpnext.PointOfSale.Controller = class {
 					this.cart.toggle_numpad(minimize);
 				},
 
-				form_updated: (cdt, cdn, fieldname, value) => {
-					const item_row = frappe.model.get_doc(cdt, cdn);
-					if (item_row && item_row[fieldname] != value) {
-
-						const { item_code, batch_no, uom, rate } = this.item_details.current_item;
-						const event = {
-							field: fieldname,
+				form_updated: (item, field, value) => {
+					const item_row = frappe.model.get_doc(item.doctype, item.name);
+					if (item_row && item_row[field] != value) {
+						const args = {
+							field,
 							value,
-							item: { item_code, batch_no, uom, rate }
-						}
-						return this.on_cart_update(event)
+							item: this.item_details.current_item
+						};
+						return this.on_cart_update(args);
 					}
 
 					return Promise.resolve();
@@ -300,19 +298,18 @@ erpnext.PointOfSale.Controller = class {
 				set_value_in_current_cart_item: (selector, value) => {
 					this.cart.update_selector_value_in_cart_item(selector, value, this.item_details.current_item);
 				},
-				clone_new_batch_item_in_frm: (batch_serial_map, current_item) => {
+				clone_new_batch_item_in_frm: (batch_serial_map, item) => {
 					// called if serial nos are 'auto_selected' and if those serial nos belongs to multiple batches
 					// for each unique batch new item row is added in the form & cart
 					Object.keys(batch_serial_map).forEach(batch => {
-						const { item_code, batch_no } = current_item;
-						const item_to_clone = this.frm.doc.items.find(i => i.item_code === item_code && i.batch_no === batch_no);
+						const item_to_clone = this.frm.doc.items.find(i => i.name == item.name);
 						const new_row = this.frm.add_child("items", { ...item_to_clone });
 						// update new serialno and batch
 						new_row.batch_no = batch;
 						new_row.serial_no = batch_serial_map[batch].join(`\n`);
 						new_row.qty = batch_serial_map[batch].length;
 						this.frm.doc.items.forEach(row => {
-							if (item_code === row.item_code) {
+							if (item.item_code === row.item_code) {
 								this.update_cart_html(row);
 							}
 						});
@@ -321,8 +318,8 @@ erpnext.PointOfSale.Controller = class {
 				remove_item_from_cart: () => this.remove_item_from_cart(),
 				get_item_stock_map: () => this.item_stock_map,
 				close_item_details: () => {
-					this.item_details.toggle_item_details_section(undefined);
-					this.cart.prev_action = undefined;
+					this.item_details.toggle_item_details_section(null);
+					this.cart.prev_action = null;
 					this.cart.toggle_item_highlight();
 				},
 				get_available_stock: (item_code, warehouse) => this.get_available_stock(item_code, warehouse)
@@ -506,50 +503,47 @@ erpnext.PointOfSale.Controller = class {
 		let item_row = undefined;
 		try {
 			let { field, value, item } = args;
-			const { item_code, batch_no, serial_no, uom, rate } = item;
-			item_row = this.get_item_from_frm(item_code, batch_no, uom, rate);
-
-			const item_selected_from_selector = field === 'qty' && value === "+1"
+			item_row = this.get_item_from_frm(item);
+			const item_row_exists = !$.isEmptyObject(item_row);
 
-			if (item_row) {
-				item_selected_from_selector && (value = item_row.qty + flt(value))
+			const from_selector = field === 'qty' && value === "+1";
+			if (from_selector)
+				value = flt(item_row.qty) + flt(value);
 
-				field === 'qty' && (value = flt(value));
+			if (item_row_exists) {
+				if (field === 'qty')
+					value = flt(value);
 
 				if (['qty', 'conversion_factor'].includes(field) && value > 0 && !this.allow_negative_stock) {
 					const qty_needed = field === 'qty' ? value * item_row.conversion_factor : item_row.qty * value;
 					await this.check_stock_availability(item_row, qty_needed, this.frm.doc.set_warehouse);
 				}
 
-				if (this.is_current_item_being_edited(item_row) || item_selected_from_selector) {
+				if (this.is_current_item_being_edited(item_row) || from_selector) {
 					await frappe.model.set_value(item_row.doctype, item_row.name, field, value);
 					this.update_cart_html(item_row);
 				}
 
 			} else {
-				if (!this.frm.doc.customer) {
-					frappe.dom.unfreeze();
-					frappe.show_alert({
-						message: __('You must select a customer before adding an item.'),
-						indicator: 'orange'
-					});
-					frappe.utils.play_sound("error");
-					return;
-				}
-				if (!item_code) return;
+				if (!this.frm.doc.customer) 
+					return this.raise_customer_selection_alert();
 
-				item_selected_from_selector && (value = flt(value))
+				const { item_code, batch_no, serial_no, rate } = item;
 
-				const args = { item_code, batch_no, rate, [field]: value };
+				if (!item_code)
+					return;
+
+				const new_item = { item_code, batch_no, rate, [field]: value };
 
 				if (serial_no) {
 					await this.check_serial_no_availablilty(item_code, this.frm.doc.set_warehouse, serial_no);
-					args['serial_no'] = serial_no;
+					new_item['serial_no'] = serial_no;
 				}
 
-				if (field === 'serial_no') args['qty'] = value.split(`\n`).length || 0;
+				if (field === 'serial_no')
+					new_item['qty'] = value.split(`\n`).length || 0;
 
-				item_row = this.frm.add_child('items', args);
+				item_row = this.frm.add_child('items', new_item);
 
 				if (field === 'qty' && value !== 0 && !this.allow_negative_stock)
 					await this.check_stock_availability(item_row, value, this.frm.doc.set_warehouse);
@@ -558,8 +552,11 @@ erpnext.PointOfSale.Controller = class {
 				
 				this.update_cart_html(item_row);
 
-				this.item_details.$component.is(':visible') && this.edit_item_details_of(item_row);
-				this.check_serial_batch_selection_needed(item_row) && this.edit_item_details_of(item_row);
+				if (this.item_details.$component.is(':visible'))
+					this.edit_item_details_of(item_row);
+
+				if (this.check_serial_batch_selection_needed(item_row))
+					this.edit_item_details_of(item_row);
 			}
 
 		} catch (error) {
@@ -570,14 +567,33 @@ erpnext.PointOfSale.Controller = class {
 		}
 	}
 
-	get_item_from_frm(item_code, batch_no, uom, rate) {
-		const has_batch_no = batch_no;
-		return this.frm.doc.items.find(
-			i => i.item_code === item_code
-				&& (!has_batch_no || (has_batch_no && i.batch_no === batch_no))
-				&& (i.uom === uom)
-				&& (i.rate == rate)
-		);
+	raise_customer_selection_alert() {
+		frappe.dom.unfreeze();
+		frappe.show_alert({
+			message: __('You must select a customer before adding an item.'),
+			indicator: 'orange'
+		});
+		frappe.utils.play_sound("error");
+	}
+
+	get_item_from_frm({ name, item_code, batch_no, uom, rate }) {
+		let item_row = null;
+		if (name) {
+			item_row = this.frm.doc.items.find(i => i.name == name);
+		} else {
+			// if item is clicked twice from item selector
+			// then "item_code, batch_no, uom, rate" will help in getting the exact item
+			// to increase the qty by one
+			const has_batch_no = batch_no;
+			item_row = this.frm.doc.items.find(
+				i => i.item_code === item_code
+					&& (!has_batch_no || (has_batch_no && i.batch_no === batch_no))
+					&& (i.uom === uom)
+					&& (i.rate == rate)
+			);
+		}
+
+		return item_row || {};
 	}
 
 	edit_item_details_of(item_row) {
@@ -585,9 +601,7 @@ erpnext.PointOfSale.Controller = class {
 	}
 
 	is_current_item_being_edited(item_row) {
-		const { item_code, batch_no } = this.item_details.current_item;
-
-		return item_code !== item_row.item_code || batch_no != item_row.batch_no ? false : true;
+		return item_row.name == this.item_details.current_item.name;
 	}
 
 	update_cart_html(item_row, remove_item) {
@@ -669,7 +683,7 @@ erpnext.PointOfSale.Controller = class {
 
 	update_item_field(value, field_or_action) {
 		if (field_or_action === 'checkout') {
-			this.item_details.toggle_item_details_section(undefined);
+			this.item_details.toggle_item_details_section(null);
 		} else if (field_or_action === 'remove') {
 			this.remove_item_from_cart();
 		} else {
@@ -688,7 +702,7 @@ erpnext.PointOfSale.Controller = class {
 			.then(() => {
 				frappe.model.clear_doc(doctype, name);
 				this.update_cart_html(current_item, true);
-				this.item_details.toggle_item_details_section(undefined);
+				this.item_details.toggle_item_details_section(null);
 				frappe.dom.unfreeze();
 			})
 			.catch(e => console.log(e));

erpnext/selling/page/point_of_sale/pos_item_cart.js

@@ -181,11 +181,8 @@ erpnext.PointOfSale.ItemCart = class {
 				me.$totals_section.find(".edit-cart-btn").click();
 			}
 
-			const item_code = unescape($cart_item.attr('data-item-code'));
-			const batch_no = unescape($cart_item.attr('data-batch-no'));
-			const uom = unescape($cart_item.attr('data-uom'));
-			const rate = unescape($cart_item.attr('data-rate'));
-			me.events.cart_item_clicked(item_code, batch_no, uom, rate);
+			const item_row_name = unescape($cart_item.attr('data-row-name'));
+			me.events.cart_item_clicked({ name: item_row_name });
 			this.numpad_value = '';
 		});
 
@@ -521,25 +518,14 @@ erpnext.PointOfSale.ItemCart = class {
 		}
 	}
 
-	get_cart_item({ item_code, batch_no, uom, rate }) {
-		const batch_attr = `[data-batch-no="${escape(batch_no)}"]`;
-		const item_code_attr = `[data-item-code="${escape(item_code)}"]`;
-		const uom_attr = `[data-uom="${escape(uom)}"]`;
-		const rate_attr = `[data-rate="${escape(rate)}"]`;
-
-		const item_selector = batch_no ?
-			`.cart-item-wrapper${batch_attr}${uom_attr}${rate_attr}` : `.cart-item-wrapper${item_code_attr}${uom_attr}${rate_attr}`;
-
+	get_cart_item({ name }) {
+		const item_selector = `.cart-item-wrapper[data-row-name="${escape(name)}"]`;
 		return this.$cart_items_wrapper.find(item_selector);
 	}
 
 	get_item_from_frm(item) {
 		const doc = this.events.get_frm().doc;
-		const { item_code, batch_no, uom, rate } = item;
-		const search_field = batch_no ? 'batch_no' : 'item_code';
-		const search_value = batch_no || item_code;
-
-		return doc.items.find(i => i[search_field] === search_value && i.uom === uom && i.rate === rate);
+		return doc.items.find(i => i.name == item.name);
 	}
 
 	update_item_html(item, remove_item) {
@@ -564,10 +550,7 @@ erpnext.PointOfSale.ItemCart = class {
 
 		if (!$item_to_update.length) {
 			this.$cart_items_wrapper.append(
-				`<div class="cart-item-wrapper"
-						data-item-code="${escape(item_data.item_code)}" data-uom="${escape(item_data.uom)}"
-						data-batch-no="${escape(item_data.batch_no || '')}" data-rate="${escape(item_data.rate)}">
-				</div>
+				`<div class="cart-item-wrapper" data-row-name="${escape(item_data.name)}"></div>
 				<div class="seperator"></div>`
 			)
 			$item_to_update = this.get_cart_item(item_data);
@@ -642,7 +625,7 @@ erpnext.PointOfSale.ItemCart = class {
 
 		function get_item_image_html() {
 			const { image, item_name } = item_data;
-			if (image) {
+			if (!me.hide_images && image) {
 				return `
 					<div class="item-image">
 						<img

erpnext/selling/page/point_of_sale/pos_item_details.js

@@ -2,6 +2,7 @@ erpnext.PointOfSale.ItemDetails = class {
 	constructor({ wrapper, events, settings }) {
 		this.wrapper = wrapper;
 		this.events = events;
+		this.hide_images = settings.hide_images;
 		this.allow_rate_change = settings.allow_rate_change;
 		this.allow_discount_change = settings.allow_discount_change;
 		this.current_item = {};
@@ -54,36 +55,28 @@ erpnext.PointOfSale.ItemDetails = class {
 		this.$dicount_section = this.$component.find('.discount-section');
 	}
 
-	has_item_has_changed(item) {
-		const { item_code, batch_no, uom, rate } = this.current_item;
-		const item_code_is_same = item && item_code === item.item_code;
-		const batch_is_same = item && batch_no == item.batch_no;
-		const uom_is_same = item && uom === item.uom;
-		const rate_is_same = item && rate === item.rate;
-		
-		if (!item)
-			return false;
-
-		if (item_code_is_same && batch_is_same && uom_is_same && rate_is_same)
-			return false;
-
-		return true;
+	compare_with_current_item(item) {
+		// returns true if `item` is currently being edited
+		return item && item.name == this.current_item.name;
 	}
 
 	toggle_item_details_section(item) {
-		this.item_has_changed = this.has_item_has_changed(item);
+		const current_item_changed = !this.compare_with_current_item(item);
 
-		this.events.toggle_item_selector(this.item_has_changed);
-		this.toggle_component(this.item_has_changed);
+		// if item is null or highlighted cart item is clicked twice
+		const hide_item_details = !Boolean(item) || !current_item_changed;
+		
+		this.events.toggle_item_selector(!hide_item_details);
+		this.toggle_component(!hide_item_details);
 
-		if (this.item_has_changed) {
+		if (item && current_item_changed) {
 			this.doctype = item.doctype;
 			this.item_meta = frappe.get_meta(this.doctype);
 			this.name = item.name;
 			this.item_row = item;
 			this.currency = this.events.get_frm().doc.currency;
 
-			this.current_item = { item_code: item.item_code, batch_no: item.batch_no, uom: item.uom, rate: item.rate };
+			this.current_item = item;
 
 			this.render_dom(item);
 			this.render_discount_dom(item);
@@ -132,7 +125,7 @@ erpnext.PointOfSale.ItemDetails = class {
 		this.$item_name.html(item_name);
 		this.$item_description.html(get_description_html());
 		this.$item_price.html(format_currency(price_list_rate, this.currency));
-		if (image) {
+		if (!this.hide_images && image) {
 			this.$item_image.html(
 				`<img 
 					onerror="cur_pos.item_details.handle_broken_image(this)"
@@ -180,7 +173,7 @@ erpnext.PointOfSale.ItemDetails = class {
 				df: {
 					...field_meta,
 					onchange: function() {
-						me.events.form_updated(me.doctype, me.name, fieldname, this.value);
+						me.events.form_updated(me.current_item, fieldname, this.value);
 					}
 				},
 				parent: this.$form_container.find(`.${fieldname}-control`),
@@ -218,22 +211,17 @@ erpnext.PointOfSale.ItemDetails = class {
 	bind_custom_control_change_event() {
 		const me = this;
 		if (this.rate_control) {
-			if (this.allow_rate_change) {
-				this.rate_control.df.onchange = function() {
-					if (this.value || flt(this.value) === 0) {
-						me.events.set_value_in_current_cart_item('rate', this.value);
-						me.events.form_updated(me.doctype, me.name, 'rate', this.value).then(() => {
-							const item_row = frappe.get_doc(me.doctype, me.name);
-							const doc = me.events.get_frm().doc;
-							me.$item_price.html(format_currency(item_row.rate, doc.currency));
-							me.render_discount_dom(item_row);
-						});
-						me.current_item.rate = this.value;
-					}
-				};
-			} else {
-				this.rate_control.df.read_only = 1;
-			}
+			this.rate_control.df.onchange = function() {
+				if (this.value || flt(this.value) === 0) {
+					me.events.form_updated(me.current_item, 'rate', this.value).then(() => {
+						const item_row = frappe.get_doc(me.doctype, me.name);
+						const doc = me.events.get_frm().doc;
+						me.$item_price.html(format_currency(item_row.rate, doc.currency));
+						me.render_discount_dom(item_row);
+					});
+				}
+			};
+			this.rate_control.df.read_only = !this.allow_rate_change;
 			this.rate_control.refresh();
 		}
 
@@ -246,7 +234,7 @@ erpnext.PointOfSale.ItemDetails = class {
 			this.warehouse_control.df.reqd = 1;
 			this.warehouse_control.df.onchange = function() {
 				if (this.value) {
-					me.events.form_updated(me.doctype, me.name, 'warehouse', this.value).then(() => {
+					me.events.form_updated(me.current_item, 'warehouse', this.value).then(() => {
 						me.item_stock_map = me.events.get_item_stock_map();
 						const available_qty = me.item_stock_map[me.item_row.item_code][this.value];
 						if (available_qty === undefined) {
@@ -278,7 +266,7 @@ erpnext.PointOfSale.ItemDetails = class {
 			this.serial_no_control.df.reqd = 1;
 			this.serial_no_control.df.onchange = async function() {
 				!me.current_item.batch_no && await me.auto_update_batch_no();
-				me.events.form_updated(me.doctype, me.name, 'serial_no', this.value);
+				me.events.form_updated(me.current_item, 'serial_no', this.value);
 			}
 			this.serial_no_control.refresh();
 		}
@@ -295,19 +283,12 @@ erpnext.PointOfSale.ItemDetails = class {
 					}
 				}
 			};
-			this.batch_no_control.df.onchange = function() {
-				me.events.set_value_in_current_cart_item('batch-no', this.value);
-				me.events.form_updated(me.doctype, me.name, 'batch_no', this.value);
-				me.current_item.batch_no = this.value;
-			}
 			this.batch_no_control.refresh();
 		}
 
 		if (this.uom_control) {
 			this.uom_control.df.onchange = function() {
-				me.events.set_value_in_current_cart_item('uom', this.value);
-				me.events.form_updated(me.doctype, me.name, 'uom', this.value);
-				me.current_item.uom = this.value;
+				me.events.form_updated(me.current_item, 'uom', this.value);
 
 				const item_row = frappe.get_doc(me.doctype, me.name);
 				me.conversion_factor_control.df.read_only = (item_row.stock_uom == this.value);
@@ -317,9 +298,9 @@ erpnext.PointOfSale.ItemDetails = class {
 
 		frappe.model.on("POS Invoice Item", "*", (fieldname, value, item_row) => {
 			const field_control = this[`${fieldname}_control`];
-			const item_is_same = !this.has_item_has_changed(item_row);
+			const item_row_is_being_edited = this.compare_with_current_item(item_row);
 
-			if (item_is_same && field_control && field_control.get_value() !== value) {
+			if (item_row_is_being_edited && field_control && field_control.get_value() !== value) {
 				field_control.set_value(value);
 				cur_pos.update_cart_html(item_row);
 			}
@@ -337,7 +318,9 @@ erpnext.PointOfSale.ItemDetails = class {
 				fields: ["batch_no", "name"]
 			});
 			const batch_serial_map = serials_with_batch_no.reduce((acc, r) => {
-				acc[r.batch_no] || (acc[r.batch_no] = []);
+				if (!acc[r.batch_no]) {
+					acc[r.batch_no] = [];
+				}
 				acc[r.batch_no] = [...acc[r.batch_no], r.name];
 				return acc;
 			}, {});
@@ -353,12 +336,10 @@ erpnext.PointOfSale.ItemDetails = class {
 			if (serial_nos_belongs_to_other_batch) {
 				this.serial_no_control.set_value(batch_serial_nos);
 				this.qty_control.set_value(batch_serial_map[batch_no].length);
-			}
-
-			delete batch_serial_map[batch_no];
 
-			if (serial_nos_belongs_to_other_batch)
+				delete batch_serial_map[batch_no];
 				this.events.clone_new_batch_item_in_frm(batch_serial_map, this.current_item);
+			}
 		}
 	}
 

erpnext/selling/page/point_of_sale/pos_item_selector.js

@@ -232,7 +232,11 @@ erpnext.PointOfSale.ItemSelector = class {
 			uom = uom === "undefined" ? undefined : uom;
 			rate = rate === "undefined" ? undefined : rate;
 
-			me.events.item_selected({ field: 'qty', value: "+1", item: { item_code, batch_no, serial_no, uom, rate }});
+			me.events.item_selected({
+				field: 'qty',
+				value: "+1",
+				item: { item_code, batch_no, serial_no, uom, rate }
+			});
 			me.set_search_value('');
 		});