fix(shipment): apply code review
diff --git a/erpnext/erpnext_integrations/doctype/letmeship/letmeship.json b/erpnext/erpnext_integrations/doctype/letmeship/letmeship.json
index 4a9a70f..94b001e 100644
--- a/erpnext/erpnext_integrations/doctype/letmeship/letmeship.json
+++ b/erpnext/erpnext_integrations/doctype/letmeship/letmeship.json
@@ -24,14 +24,14 @@
   },
   {
    "fieldname": "api_password",
-   "fieldtype": "Data",
+   "fieldtype": "Password",
    "label": "API Password",
    "read_only_depends_on": "eval:doc.enabled == 0"
   }
  ],
  "issingle": 1,
  "links": [],
- "modified": "2020-08-05 16:33:44.548230",
+ "modified": "2020-10-21 10:28:37.607717",
  "modified_by": "Administrator",
  "module": "ERPNext Integrations",
  "name": "LetMeShip",
diff --git a/erpnext/erpnext_integrations/doctype/letmeship/letmeship.py b/erpnext/erpnext_integrations/doctype/letmeship/letmeship.py
index 3ad06db..162c1ed 100644
--- a/erpnext/erpnext_integrations/doctype/letmeship/letmeship.py
+++ b/erpnext/erpnext_integrations/doctype/letmeship/letmeship.py
@@ -20,9 +20,7 @@
 	delivery_address, shipment_parcel, description_of_content, pickup_date,
 	value_of_goods, pickup_contact=None, delivery_contact=None):
 	# Retrieve rates at LetMeShip from specification stated.
-	enabled = frappe.db.get_single_value('LetMeShip','enabled')
-	api_id = frappe.db.get_single_value('LetMeShip','api_id')
-	api_password = frappe.db.get_single_value('LetMeShip','api_password')
+	api_id, api_password, enabled = frappe.db.get_value('LetMeShip', 'LetMeShip', ['enabled', 'api_password', 'api_id'])
 	if not enabled or not api_id or not api_password:
 		return []
 
@@ -41,62 +39,16 @@
 		'Accept': 'application/json',
 		'Access-Control-Allow-Origin': 'string'
 	}
-	payload = {'pickupInfo': {
-		'address': {
-			'countryCode': pickup_address.country_code,
-			'zip': pickup_address.pincode,
-			'city': pickup_address.city,
-			'street': pickup_address.address_line1,
-			'addressInfo1': pickup_address.address_line2,
-			'houseNo': '',
-		},
-		'company': pickup_address.address_title,
-		'person': {
-			'title': pickup_contact.title,
-			'firstname': pickup_contact.first_name,
-			'lastname': pickup_contact.last_name
-		},
-		'phone': {
-			'phoneNumber': pickup_contact.phone,
-			'phoneNumberPrefix': pickup_contact.phone_prefix
-		},
-		'email': pickup_contact.email,
-	}, 'deliveryInfo': {
-		'address': {
-			'countryCode': delivery_address.country_code,
-			'zip': delivery_address.pincode,
-			'city': delivery_address.city,
-			'street': delivery_address.address_line1,
-			'addressInfo1': delivery_address.address_line2,
-			'houseNo': '',
-		},
-		'company': delivery_address.address_title,
-		'person': {
-			'title': delivery_contact.title,
-			'firstname': delivery_contact.first_name,
-			'lastname': delivery_contact.last_name
-		},
-		'phone': {
-			'phoneNumber': delivery_contact.phone,
-			'phoneNumberPrefix': delivery_contact.phone_prefix
-		},
-		'email': delivery_contact.email,
-	}, 'shipmentDetails': {
-		'contentDescription': description_of_content,
-		'shipmentType': 'PARCEL',
-		'shipmentSettings': {
-			'saturdayDelivery': False,
-			'ddp': False,
-			'insurance': False,
-			'pickupOrder': False,
-			'pickupTailLift': False,
-			'deliveryTailLift': False,
-			'holidayDelivery': False,
-		},
-		'goodsValue': value_of_goods,
-		'parcelList': parcel_list,
-		'pickupInterval': {'date': pickup_date},
-	}}
+	payload = generate_payload(
+		pickup_address=pickup_address,
+		pickup_contact=pickup_contact,
+		delivery_address=delivery_address,
+		delivery_contact=delivery_contact,
+		description_of_content=description_of_content,
+		value_of_goods=value_of_goods,
+		parcel_list=parcel_list,
+		pickup_date=pickup_date
+	)
 	try:
 		available_services = []
 		response_data = requests.post(
@@ -128,6 +80,7 @@
 					.format(response_data['message'])
 			)
 	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
 		frappe.msgprint(
 			_('Error occurred while fetching LetMeShip Prices: {0}')
 				.format(str(exc)),
@@ -142,9 +95,7 @@
 	pickup_contact=None, delivery_contact=None):
 	# Create a transaction at LetMeShip
 	# LetMeShip have limit of 30 characters for Company field
-	enabled = frappe.db.get_single_value('LetMeShip','enabled')
-	api_id = frappe.db.get_single_value('LetMeShip','api_id')
-	api_password = frappe.db.get_single_value('LetMeShip','api_password')
+	api_id, api_password, enabled = frappe.db.get_value('LetMeShip', 'LetMeShip', ['enabled', 'api_password', 'api_id'])
 	if not enabled or not api_id or not api_password:
 		return []
 
@@ -162,6 +113,72 @@
 		'Accept': 'application/json',
 		'Access-Control-Allow-Origin': 'string'
 	}
+	payload = generate_payload(
+		pickup_address=pickup_address,
+		pickup_contact=pickup_contact,
+		delivery_address=delivery_address,
+		delivery_contact=delivery_contact,
+		description_of_content=description_of_content,
+		value_of_goods=value_of_goods,
+		parcel_list=parcel_list,
+		pickup_date=pickup_date,
+		service_info=service_info,
+		tracking_notific_email=tracking_notific_email,
+		shipment_notific_email=shipment_notific_email
+	)
+	try:
+		response_data = requests.post(
+			url=url,
+			auth=(api_id, api_password),
+			headers=headers,
+			data=json.dumps(payload)
+		)
+		response_data = json.loads(response_data.text)
+		if 'shipmentId' in response_data:
+			shipment_amount = response_data['service']['priceInfo']['totalPrice']
+			awb_number = ''
+			url = 'https://api.letmeship.com/v1/shipments/{id}'.format(id=response_data['shipmentId'])
+			tracking_response = requests.get(url, auth=(api_id, api_password),headers=headers)
+			tracking_response_data = json.loads(tracking_response.text)
+			if 'trackingData' in tracking_response_data:
+				for parcel in tracking_response_data['trackingData']['parcelList']:
+					if 'awbNumber' in parcel:
+						awb_number = parcel['awbNumber']
+			return {
+				'service_provider': LETMESHIP_PROVIDER,
+				'shipment_id': response_data['shipmentId'],
+				'carrier': service_info['carrier'],
+				'carrier_service': service_info['service_name'],
+				'shipment_amount': shipment_amount,
+				'awb_number': awb_number,
+			}
+		elif 'message' in response_data:
+			frappe.throw(
+				_('Error occurred while creating Shipment: {0}')
+					.format(response_data['message'])
+			)
+	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
+		frappe.msgprint(
+			_('Error occurred while creating Shipment: {0}')
+				.format(str(exc)),
+			indicator='orange',
+			alert=True
+		)
+
+def generate_payload(
+	pickup_address,
+	pickup_contact,
+	delivery_address,
+	delivery_contact,
+	description_of_content,
+	value_of_goods,
+	parcel_list,
+	pickup_date,
+	service_info=None,
+	tracking_notific_email=None,
+	shipment_notific_email=None
+):
 	payload = {
 		'pickupInfo': {
 			'address': {
@@ -205,18 +222,6 @@
 			},
 			'email': delivery_contact.email,
 		},
-		'service': {
-			'baseServiceDetails': {
-				'id': service_info['id'],
-				'name': service_info['service_name'],
-				'carrier': service_info['carrier'],
-				'priceInfo': service_info['price_info'],
-			},
-			'supportedExWorkType': [],
-			'messages': [''],
-			'description': '',
-			'serviceInfo': '',
-		},
 		'shipmentDetails': {
 			'contentDescription': description_of_content,
 			'shipmentType': 'PARCEL',
@@ -233,10 +238,24 @@
 			'parcelList': parcel_list,
 			'pickupInterval': {
 				'date': pickup_date
+			}
+		}
+	}
+
+	if service_info:
+		payload['service'] = {
+			'baseServiceDetails': {
+				'id': service_info['id'],
+				'name': service_info['service_name'],
+				'carrier': service_info['carrier'],
+				'priceInfo': service_info['price_info'],
 			},
-			'contentDescription': description_of_content,
-		},
-		'shipmentNotification': {
+			'supportedExWorkType': [],
+			'messages': [''],
+			'description': '',
+			'serviceInfo': '',
+		}
+		payload['shipmentNotification'] = {
 			'trackingNotification': {
 				'deliveryNotification': True,
 				'problemNotification': True,
@@ -247,77 +266,47 @@
 				'notificationText': '',
 				'emails': [ shipment_notific_email ]
 			}
-		},
-		'labelEmail': True,
-	}
+		}
+		payload['labelEmail'] = True
+	return payload
+
+def get_letmeship_label(shipment_id):
 	try:
-		response_data = requests.post(
-			url=url,
-			auth=(api_id, api_password),
-			headers=headers,
-			data=json.dumps(payload)
+		# Retrieve shipment label from LetMeShip
+		api_id = frappe.db.get_single_value('LetMeShip','api_id')
+		api_password = frappe.db.get_single_value('LetMeShip','api_password')
+		headers = {
+			'Content-Type': 'application/json',
+			'Accept': 'application/json',
+			'Access-Control-Allow-Origin': 'string'
+		}
+		url = 'https://api.letmeship.com/v1/shipments/{id}/documents?types=LABEL'\
+			.format(id=shipment_id)
+		shipment_label_response = requests.get(
+			url,
+			auth=(api_id,api_password),
+			headers=headers
 		)
-		response_data = json.loads(response_data.text)
-		if 'shipmentId' in response_data:
-			shipment_amount = response_data['service']['priceInfo']['totalPrice']
-			awb_number = ''
-			url = 'https://api.letmeship.com/v1/shipments/{id}'.format(id=response_data['shipmentId'])
-			tracking_response = requests.get(url, auth=(api_id, api_password),headers=headers)
-			tracking_response_data = json.loads(tracking_response.text)
-			if 'trackingData' in tracking_response_data:
-				for parcel in tracking_response_data['trackingData']['parcelList']:
-					if 'awbNumber' in parcel:
-						awb_number = parcel['awbNumber']
-			return {
-				'service_provider': LETMESHIP_PROVIDER,
-				'shipment_id': response_data['shipmentId'],
-				'carrier': service_info['carrier'],
-				'carrier_service': service_info['service_name'],
-				'shipment_amount': shipment_amount,
-				'awb_number': awb_number,
-			}
-		elif 'message' in response_data:
+		shipment_label_response_data = json.loads(shipment_label_response.text)
+		if 'documents' in shipment_label_response_data:
+			for label in shipment_label_response_data['documents']:
+				if 'data' in label:
+					return json.dumps(label['data'])
+		else:
 			frappe.throw(
-				_('Error occurred while creating Shipment: {0}')
-					.format(response_data['message'])
+				_('Error occurred while printing Shipment: {0}')
+					.format(shipment_label_response_data['message'])
 			)
 	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
 		frappe.msgprint(
-			_('Error occurred while creating Shipment: {0}')
+			_('Error occurred while printing Shipment: {0}')
 				.format(str(exc)),
 			indicator='orange',
 			alert=True
 		)
 
 
-def get_letmeship_label(shipment_id):
-	# Retrieve shipment label from LetMeShip
-	api_id = frappe.db.get_single_value('LetMeShip','api_id')
-	api_password = frappe.db.get_single_value('LetMeShip','api_password')
-	headers = {
-		'Content-Type': 'application/json',
-		'Accept': 'application/json',
-		'Access-Control-Allow-Origin': 'string'
-	}
-	url = 'https://api.letmeship.com/v1/shipments/{id}/documents?types=LABEL'\
-		.format(id=shipment_id)
-	shipment_label_response = requests.get(
-		url,
-		auth=(api_id,api_password),
-		headers=headers
-	)
-	shipment_label_response_data = json.loads(shipment_label_response.text)
-	if 'documents' in shipment_label_response_data:
-		for label in shipment_label_response_data['documents']:
-			if 'data' in label:
-				return json.dumps(label['data'])
-	else:
-		frappe.throw(
-			_('Error occurred while printing Shipment: {0}')
-				.format(shipment_label_response_data['message'])
-		)
-
-
 def get_letmeship_tracking_data(shipment_id):
 	# return letmeship tracking data
 	api_id = frappe.db.get_single_value('LetMeShip','api_id')
@@ -359,6 +348,7 @@
 					.format(tracking_data['message'])
 			)
 	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
 		frappe.msgprint(
 			_('Error occurred while updating Shipment: {0}')
 				.format(str(exc)),
diff --git a/erpnext/erpnext_integrations/doctype/packlink/packlink.py b/erpnext/erpnext_integrations/doctype/packlink/packlink.py
index 7fdb053..1db08c3 100644
--- a/erpnext/erpnext_integrations/doctype/packlink/packlink.py
+++ b/erpnext/erpnext_integrations/doctype/packlink/packlink.py
@@ -72,6 +72,7 @@
 
 		return available_services
 	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
 		frappe.msgprint(
 			_('Error occurred on Packlink: {0}')
 				.format(str(exc)), indicator='orange',
@@ -152,6 +153,7 @@
 				'awb_number': '',
 			}
 	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
 		frappe.msgprint(
 			_('Error occurred while creating Shipment: {0}')
 				.format(str(exc)),
@@ -215,6 +217,7 @@
 				'tracking_url': tracking_url
 			}
 	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
 		frappe.msgprint(_('Error occurred while updating Shipment: {0}').format(
 			str(exc)), indicator='orange', alert=True)
 	return []
diff --git a/erpnext/erpnext_integrations/doctype/sendcloud/sendcloud.json b/erpnext/erpnext_integrations/doctype/sendcloud/sendcloud.json
index dab54cb..37b6898 100644
--- a/erpnext/erpnext_integrations/doctype/sendcloud/sendcloud.json
+++ b/erpnext/erpnext_integrations/doctype/sendcloud/sendcloud.json
@@ -24,7 +24,7 @@
   },
   {
    "fieldname": "api_secret",
-   "fieldtype": "Data",
+   "fieldtype": "Password",
    "label": "API Secret",
    "read_only_depends_on": "eval:doc.enabled == 0"
   }
@@ -32,7 +32,7 @@
  "index_web_pages_for_search": 1,
  "issingle": 1,
  "links": [],
- "modified": "2020-08-18 09:48:50.836233",
+ "modified": "2020-10-21 10:28:57.710549",
  "modified_by": "Administrator",
  "module": "ERPNext Integrations",
  "name": "SendCloud",
diff --git a/erpnext/erpnext_integrations/doctype/sendcloud/sendcloud.py b/erpnext/erpnext_integrations/doctype/sendcloud/sendcloud.py
index 85c9438..d30af15 100644
--- a/erpnext/erpnext_integrations/doctype/sendcloud/sendcloud.py
+++ b/erpnext/erpnext_integrations/doctype/sendcloud/sendcloud.py
@@ -16,9 +16,7 @@
 
 def get_sendcloud_available_services(delivery_address, shipment_parcel):
 	# Retrieve rates at SendCloud from specification stated.
-	enabled = frappe.db.get_single_value('SendCloud', 'enabled')
-	api_key = frappe.db.get_single_value('SendCloud', 'api_key')
-	api_secret = frappe.db.get_single_value('SendCloud', 'api_secret')
+	api_key, api_secret, enabled = frappe.db.get_value('SendCloud', 'SendCloud', ['enabled', 'api_key', 'api_secret'])
 	if not enabled or not api_key or not api_secret:
 		return []
 
@@ -40,6 +38,7 @@
 					available_services.append(available_service)
 		return available_services
 	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
 		frappe.msgprint(_('Error occurred on SendCloud: {0}').format(
 			str(exc)), indicator='orange', alert=True)
 
@@ -53,9 +52,7 @@
 	value_of_goods
 ):
 	# Create a transaction at SendCloud
-	enabled = frappe.db.get_single_value('SendCloud', 'enabled')
-	api_key = frappe.db.get_single_value('SendCloud', 'api_key')
-	api_secret = frappe.db.get_single_value('SendCloud', 'api_secret')
+	api_key, api_secret, enabled = frappe.db.get_value('SendCloud', 'SendCloud', ['enabled', 'api_key', 'api_secret'])
 	if not enabled or not api_key or not api_secret:
 		return []
 
@@ -105,6 +102,7 @@
 				'awb_number': awb_number
 			}
 	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
 		frappe.msgprint(_('Error occurred while creating Shipment: {0}').format(
 			str(exc)), indicator='orange', alert=True)
 
@@ -130,17 +128,15 @@
 		api_key = frappe.db.get_single_value('SendCloud', 'api_key')
 		api_secret = frappe.db.get_single_value('SendCloud', 'api_secret')
 		shipment_id_list = shipment_id.split(', ')
-		tracking_url = ''
 		awb_number = []
 		tracking_status = []
 		tracking_status_info = []
+		tracking_urls = []
 		for ship_id in shipment_id_list:
 			tracking_data_response = \
 				requests.get('https://panel.sendcloud.sc/api/v2/parcels/{id}'.format(id=ship_id), auth=(api_key, api_secret))
 			tracking_data = json.loads(tracking_data_response.text)
-			tracking_url_template = \
-				'<a href="{{ tracking_url }}" target="_blank"><b>{{ _("Click here to Track Shipment") }}</b></a><br>'
-			tracking_url += frappe.render_template(tracking_url_template, {'tracking_url': tracking_data['parcel']['tracking_url']})
+			tracking_urls.append(tracking_data['parcel']['tracking_url'])
 			awb_number.append(tracking_data['parcel']['tracking_number'])
 			tracking_status.append(tracking_data['parcel']['status']['message'])
 			tracking_status_info.append(tracking_data['parcel']['status']['message'])
@@ -148,9 +144,10 @@
 			'awb_number': ', '.join(awb_number),
 			'tracking_status': ', '.join(tracking_status),
 			'tracking_status_info': ', '.join(tracking_status_info),
-			'tracking_url': tracking_url
+			'tracking_url': ', '.join(tracking_urls)
 		}
 	except Exception as exc:
+		frappe.log_error(frappe.get_traceback())
 		frappe.msgprint(_('Error occurred while updating Shipment: {0}').format(
 			str(exc)), indicator='orange', alert=True)
 
diff --git a/erpnext/erpnext_integrations/utils.py b/erpnext/erpnext_integrations/utils.py
index e7ef4c8..362f6cf 100644
--- a/erpnext/erpnext_integrations/utils.py
+++ b/erpnext/erpnext_integrations/utils.py
@@ -68,6 +68,4 @@
 	url_reference = frappe.get_value('Parcel Service', carrier, 'url_reference')
 	if url_reference:
 		tracking_url = frappe.render_template(url_reference, {'tracking_number': tracking_number})
-		tracking_url_template =  '<a href="{{ tracking_url }}" target="_blank"><b>{{ _("Click here to Track Shipment") }}</a></b>'
-		tracking_url = frappe.render_template(tracking_url_template, {'tracking_url': tracking_url})
 	return tracking_url
diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py
index 00a66fa..26e4f16 100644
--- a/erpnext/stock/doctype/delivery_note/delivery_note.py
+++ b/erpnext/stock/doctype/delivery_note/delivery_note.py
@@ -575,22 +575,24 @@
 		user = frappe.db.get_value("User", frappe.session.user, ['email', 'full_name', 'phone', 'mobile_no'], as_dict=1)
 		target.pickup_contact_email = user.email
 		pickup_contact_display = '{}'.format(user.full_name)
-		if user.email:
-			pickup_contact_display += '<br>' + user.email
-		if user.phone:
-			pickup_contact_display += '<br>' + user.phone
-		if user.mobile_no and not user.phone:
-			pickup_contact_display += '<br>' + user.mobile_no
+		if user:
+			if user.email:
+				pickup_contact_display += '<br>' + user.email
+			if user.phone:
+				pickup_contact_display += '<br>' + user.phone
+			if user.mobile_no and not user.phone:
+				pickup_contact_display += '<br>' + user.mobile_no
 		target.pickup_contact = pickup_contact_display
 
 		contact = frappe.db.get_value("Contact", source.contact_person, ['email_id', 'phone', 'mobile_no'], as_dict=1)
 		delivery_contact_display = '{}'.format(source.contact_display)
-		if contact.email_id:
-			delivery_contact_display += '<br>' + contact.email_id
-		if contact.phone:
-			delivery_contact_display += '<br>' + contact.phone
-		if contact.mobile_no and not contact.phone:
-			delivery_contact_display += '<br>' + contact.mobile_no
+		if contact:
+			if contact.email_id:
+				delivery_contact_display += '<br>' + contact.email_id
+			if contact.phone:
+				delivery_contact_display += '<br>' + contact.phone
+			if contact.mobile_no and not contact.phone:
+				delivery_contact_display += '<br>' + contact.mobile_no
 		target.delivery_contact = delivery_contact_display
 
 	doclist = get_mapped_doc("Delivery Note", source_name, 	{
@@ -612,7 +614,7 @@
 			}
 		},
 		"Delivery Note Item": {
-			"doctype": "Shipment Delivery Notes",
+			"doctype": "Shipment Delivery Note",
 			"field_map": {
 				"name": "prevdoc_detail_docname",
 				"parent": "prevdoc_docname",
diff --git a/erpnext/stock/doctype/shipment/api/utils.py b/erpnext/stock/doctype/shipment/api/utils.py
deleted file mode 100644
index 1153933..0000000
--- a/erpnext/stock/doctype/shipment/api/utils.py
+++ /dev/null
@@ -1,67 +0,0 @@
-# -*- coding: utf-8 -*-
-# Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and contributors
-# For license information, please see license.txt
-
-from __future__ import unicode_literals
-import frappe
-from frappe import _
-import re
-
-def get_address(address_name):
-	address = frappe.db.get_value('Address', address_name, [
-		'address_title',
-		'address_line1',
-		'address_line2',
-		'city',
-		'pincode',
-		'country',
-	], as_dict=1)
-	address.country_code = frappe.db.get_value('Country', address.country, 'code').upper()
-	if not address.pincode or address.pincode == '':
-		frappe.throw(_("Postal Code is mandatory to continue. </br> \
-			Please set Postal Code for Address <a href='#Form/Address/{0}'>{1}</a>"
-		).format(address_name, address_name))
-	address.pincode = address.pincode.replace(' ', '')
-	address.city = address.city.strip()
-	return address
-
-def get_contact(contact_name):
-	contact = frappe.db.get_value('Contact', contact_name, [
-		'first_name',
-		'last_name',
-		'email_id',
-		'phone',
-		'mobile_no',
-		'gender',
-	], as_dict=1)
-	if not contact.last_name:
-		frappe.throw(_("Last Name is mandatory to continue. </br> \
-			Please set Last Name for Contact <a href='#Form/Contact/{0}'>{1}</a>"
-		).format(contact_name, contact_name))
-	if not contact.phone:
-		contact.phone = contact.mobile_no
-	contact.phone_prefix = contact.phone[:3]
-	contact.phone = re.sub('[^A-Za-z0-9]+', '', contact.phone[3:])
-	contact.email = contact.email_id
-	contact.title = 'MS'
-	if contact.gender == 'Male':
-		contact.title = 'MR'
-	return contact
-
-def get_company_contact():
-	contact = frappe.db.get_value('User', frappe.session.user, [
-		'first_name',
-		'last_name',
-		'email',
-		'phone',
-		'mobile_no',
-		'gender',
-	], as_dict=1)
-	if not contact.phone:
-		contact.phone = contact.mobile_no
-	contact.phone_prefix = contact.phone[:3]
-	contact.phone = re.sub('[^A-Za-z0-9]+', '', contact.phone[3:])
-	contact.title = 'MS'
-	if contact.gender == 'Male':
-		contact.title = 'MR'
-	return contact
diff --git a/erpnext/stock/doctype/shipment/shipment.js b/erpnext/stock/doctype/shipment/shipment.js
index e9f4484..fc0b05f 100644
--- a/erpnext/stock/doctype/shipment/shipment.js
+++ b/erpnext/stock/doctype/shipment/shipment.js
@@ -2,11 +2,6 @@
 // For license information, please see license.txt
 
 frappe.ui.form.on('Shipment', {
-	setup: function(frm) {
-		if (frm.doc.__islocal) {
-			frm.trigger('pickup_type');
-		}
-	},
 	address_query: function(frm, link_doctype, link_name, is_your_company_address) {
 		return {
 			query: 'frappe.contacts.doctype.address.address.address_query',
@@ -99,7 +94,7 @@
 			}
 			return frm.events.contact_query(frm, link_doctype, link_name);
 		});
-		frm.set_query("delivery_note", "shipment_delivery_notes", function() {
+		frm.set_query("delivery_note", "shipment_delivery_note", function() {
 			let customer = '';
 			if (frm.doc.delivery_to_type == "Customer") {
 				customer = frm.doc.delivery_customer;
@@ -127,27 +122,22 @@
 		if (frm.doc.shipment_id) {
 			frm.add_custom_button(__('Print Shipping Label'), function() {
 				return frm.events.print_shipping_label(frm);
-			});
+			}, __('Tools'));
 			if (frm.doc.tracking_status != 'Delivered') {
 				frm.add_custom_button(__('Update Tracking'), function() {
 					return frm.events.update_tracking(frm, frm.doc.service_provider, frm.doc.shipment_id);
-				});
+				}, __('Tools'));
+
+				frm.add_custom_button(__('Track Status'), function() {
+					const urls = frm.doc.tracking_url.split(', ');
+					urls.forEach(url => window.open(url));
+				}, __('View'));
 			}
 		}
 		$('div[data-fieldname=pickup_address] > div > .clearfix').hide();
 		$('div[data-fieldname=pickup_contact] > div > .clearfix').hide();
 		$('div[data-fieldname=delivery_address] > div > .clearfix').hide();
 		$('div[data-fieldname=delivery_contact] > div > .clearfix').hide();
-
-		if (frm.doc.delivery_from_type != 'Company') {
-			frm.set_df_property("delivery_contact_name", "reqd", 1);
-		}
-		if (frm.doc.pickup_from_type != 'Company') {
-			frm.set_df_property("pickup_contact_name", "reqd", 1);
-		}
-		else {
-			frm.toggle_display("pickup_contact_name", false);
-		}
 	},
 	before_save: function(frm) {
 		if (frm.doc.delivery_to_type == 'Company') {
@@ -188,14 +178,10 @@
 	pickup_from_type: function(frm) {
 		if (frm.doc.pickup_from_type == 'Company') {
 			frm.set_value("pickup_company", frappe.defaults.get_default('company'));
-			frm.set_df_property("pickup_contact_name", "reqd", 0);
 			frm.set_value("pickup_customer", '');
 			frm.set_value("pickup_supplier", '');
-			frm.toggle_display("pickup_contact_name", false);
 		}
 		else {
-			frm.set_df_property("pickup_contact_name", "reqd", 1);
-			frm.toggle_display("pickup_contact_name", true);
 			frm.trigger('clear_pickup_fields');
 		}
 		if (frm.doc.pickup_from_type == 'Customer') {
@@ -206,20 +192,16 @@
 			frm.set_value("pickup_customer", '');
 			frm.set_value("pickup_company", '');
 		}
-		frm.events.remove_notific_child_table(frm, 'shipment_notification_subscriptions', 'Pickup');
-		frm.events.remove_notific_child_table(frm, 'shipment_status_update_subscriptions', 'Pickup');
+		frm.events.remove_notific_child_table(frm, 'shipment_notification_subscription', 'Pickup');
+		frm.events.remove_notific_child_table(frm, 'shipment_status_update_subscription', 'Pickup');
 	},
 	delivery_to_type: function(frm) {
 		if (frm.doc.delivery_to_type == 'Company') {
 			frm.set_value("delivery_company", frappe.defaults.get_default('company'));
-			frm.set_df_property("delivery_contact_name", "reqd", 0);
 			frm.set_value("delivery_customer", '');
 			frm.set_value("delivery_supplier", '');
-			frm.toggle_display("delivery_contact_name", false);
 		}
 		else {
-			frm.set_df_property("delivery_contact_name", "reqd", 1);
-			frm.toggle_display("delivery_contact_name", true);
 			frm.trigger('clear_delivery_fields');
 		}
 		if (frm.doc.delivery_to_type == 'Customer') {
@@ -229,13 +211,13 @@
 		if (frm.doc.delivery_to_type == 'Supplier') {
 			frm.set_value("delivery_customer", '');
 			frm.set_value("delivery_company", '');
-			frm.toggle_display("shipment_delivery_notes", false);
+			frm.toggle_display("shipment_delivery_note", false);
 		}
 		else {
-			frm.toggle_display("shipment_delivery_notes", true);
+			frm.toggle_display("shipment_delivery_note", true);
 		}
-		frm.events.remove_notific_child_table(frm, 'shipment_notification_subscriptions', 'Delivery');
-		frm.events.remove_notific_child_table(frm, 'shipment_status_update_subscriptions', 'Delivery');
+		frm.events.remove_notific_child_table(frm, 'shipment_notification_subscription', 'Delivery');
+		frm.events.remove_notific_child_table(frm, 'shipment_status_update_subscription', 'Delivery');
 	},
 	delivery_address_name: function(frm) {
 		if (frm.doc.delivery_to_type == 'Company') {
@@ -307,6 +289,51 @@
 			frm.events.get_contact_display(frm, frm.doc.pickup_contact_name, 'Pickup');
 		}
 	},
+	pickup_contact_person: function(frm) {
+		if (frm.doc.pickup_contact_person) {
+			frappe.call({
+				method: "erpnext.stock.doctype.shipment.shipment.get_company_contact",
+				args: { user: frm.doc.pickup_contact_person },
+				callback: function({ message }) {
+					const r = message;
+					let contact_display = `${r.first_name} ${r.last_name}`;
+					if (r.email) {
+						contact_display += `<br>${ r.email }`;
+						frm.set_value('pickup_contact_email', r.email);
+					}
+					if (r.phone) {
+						contact_display += `<br>${ r.phone }`;
+					}
+					if (r.mobile_no && !r.phone) {
+						contact_display += `<br>${ r.mobile_no }`;
+					}
+					frm.set_value('pickup_contact', contact_display);
+				}
+			});
+		} else {
+			if (frm.doc.pickup_from_type === 'Company') {
+				frappe.call({
+					method: "erpnext.stock.doctype.shipment.shipment.get_company_contact",
+					args: { user: frappe.session.user },
+					callback: function({ message }) {
+						const r = message;
+						let contact_display = `${r.first_name} ${r.last_name}`;
+						if (r.email) {
+							contact_display += `<br>${ r.email }`;
+							frm.set_value('pickup_contact_email', r.email);
+						}
+						if (r.phone) {
+							contact_display += `<br>${ r.phone }`;
+						}
+						if (r.mobile_no && !r.phone) {
+							contact_display += `<br>${ r.mobile_no }`;
+						}
+						frm.set_value('pickup_contact', contact_display);
+					}
+				});
+			}
+		}
+	},
 	set_company_contact: function(frm, delivery_type) {
 		frappe.db.get_value('User', { name: frappe.session.user }, ['full_name', 'last_name', 'email', 'phone', 'mobile_no'], (r) => {
 			if (!(r.last_name && r.email && (r.phone || r.mobile_no))) {
@@ -344,6 +371,7 @@
 				}
 			}
 		});
+		frm.set_value('pickup_contact_person', frappe.session.user);
 	},
 	pickup_company: function(frm) {
 		if (frm.doc.pickup_from_type == 'Company'  && frm.doc.pickup_company) {
@@ -372,14 +400,12 @@
 		}
 	},
 	pickup_customer: function(frm) {
-		frm.trigger('clear_pickup_fields');
 		if (frm.doc.pickup_customer) {
 			frm.events.set_address_name(frm,'Customer',frm.doc.pickup_customer, 'Pickup');
 			frm.events.set_contact_name(frm,'Customer',frm.doc.pickup_customer, 'Pickup');
 		}
 	},
 	pickup_supplier: function(frm) {
-		frm.trigger('clear_pickup_fields');
 		if (frm.doc.pickup_supplier) {
 			frm.events.set_address_name(frm,'Supplier',frm.doc.pickup_supplier, 'Pickup');
 			frm.events.set_contact_name(frm,'Supplier',frm.doc.pickup_supplier, 'Pickup');
@@ -438,7 +464,7 @@
 	},
 	pickup_date: function(frm) {
 		if (frm.doc.pickup_date < frappe.datetime.get_today()) {
-			frappe.throw(__("Pickup Date cannot be in the past"));
+			frappe.throw(__("Pickup Date cannot be before this day"));
 		}
 		if (frm.doc.pickup_date == frappe.datetime.get_today()) {
 			var pickup_time = frm.events.get_pickup_time(frm);
@@ -487,65 +513,63 @@
 		frm.set_value("pickup_to", pickup_to);
 	},
 	clear_pickup_fields: function(frm) {
-		frm.set_value("pickup_address_name", '');
-		frm.set_value("pickup_contact_name", '');
-		frm.set_value("pickup_address", '');
-		frm.set_value("pickup_contact", '');
-		frm.set_value("pickup_contact_email", '');
+		let fields = ["pickup_address_name", "pickup_contact_name", "pickup_address", "pickup_contact", "pickup_contact_email", "pickup_contact_person"];
+		for (let field of fields){
+			frm.set_value(field,  '');
+		}
 	},
 	clear_delivery_fields: function(frm) {
-		frm.set_value("delivery_address_name", '');
-		frm.set_value("delivery_contact_name", '');
-		frm.set_value("delivery_address", '');
-		frm.set_value("delivery_contact", '');
-		frm.set_value("delivery_contact_email", '');
+		let fields = ["delivery_address_name", "delivery_contact_name", "delivery_address", "delivery_contact", "delivery_contact_email"];
+		for (let field of fields){
+			frm.set_value(field,  '');
+		}
 	},
 	pickup_from_send_shipping_notification: function(frm, cdt, cdn) {
 		if (frm.doc.pickup_contact_email && frm.doc.pickup_from_send_shipping_notification
-				&& !validate_duplicate(frm, 'shipment_notification_subscriptions', frm.doc.pickup_contact_email, locals[cdt][cdn].idx)) {
-			let row = frappe.model.add_child(frm.doc, "Shipment Notification Subscriptions", "shipment_notification_subscriptions");
+				&& !validate_duplicate(frm, 'shipment_notification_subscription', frm.doc.pickup_contact_email, locals[cdt][cdn].idx)) {
+			let row = frappe.model.add_child(frm.doc, "Shipment Notification Subscription", "shipment_notification_subscription");
 			row.email = frm.doc.pickup_contact_email;
-			frm.refresh_fields("shipment_notification_subscriptions");
+			frm.refresh_fields("shipment_notification_subscription");
 		}
 		if (!frm.doc.pickup_from_send_shipping_notification) {
-			frm.events.remove_email_row(frm, 'shipment_notification_subscriptions', frm.doc.pickup_contact_email);
-			frm.refresh_fields("shipment_notification_subscriptions");
+			frm.events.remove_email_row(frm, 'shipment_notification_subscription', frm.doc.pickup_contact_email);
+			frm.refresh_fields("shipment_notification_subscription");
 		}
 	},
 	pickup_from_subscribe_to_status_updates: function(frm, cdt, cdn) {
 		if (frm.doc.pickup_contact_email && frm.doc.pickup_from_subscribe_to_status_updates
-				&& !validate_duplicate(frm, 'shipment_status_update_subscriptions', frm.doc.pickup_contact_email, locals[cdt][cdn].idx)) {
-			let row = frappe.model.add_child(frm.doc, "Shipment Status Update Subscriptions", "shipment_status_update_subscriptions");
+				&& !validate_duplicate(frm, 'shipment_status_update_subscription', frm.doc.pickup_contact_email, locals[cdt][cdn].idx)) {
+			let row = frappe.model.add_child(frm.doc, "Shipment Status Update Subscription", "shipment_status_update_subscription");
 			row.email = frm.doc.pickup_contact_email;
-			frm.refresh_fields("shipment_status_update_subscriptions");
+			frm.refresh_fields("shipment_status_update_subscription");
 		}
 		if (!frm.doc.pickup_from_subscribe_to_status_updates) {
-			frm.events.remove_email_row(frm, 'shipment_status_update_subscriptions', frm.doc.pickup_contact_email);
-			frm.refresh_fields("shipment_status_update_subscriptions");
+			frm.events.remove_email_row(frm, 'shipment_status_update_subscription', frm.doc.pickup_contact_email);
+			frm.refresh_fields("shipment_status_update_subscription");
 		}
 	},
 	delivery_to_send_shipping_notification: function(frm, cdt, cdn) {
 		if (frm.doc.delivery_contact_email && frm.doc.delivery_to_send_shipping_notification
-				&& !validate_duplicate(frm, 'shipment_notification_subscriptions', frm.doc.delivery_contact_email, locals[cdt][cdn].idx)){
-			let row = frappe.model.add_child(frm.doc, "Shipment Notification Subscriptions", "shipment_notification_subscriptions");
+				&& !validate_duplicate(frm, 'shipment_notification_subscription', frm.doc.delivery_contact_email, locals[cdt][cdn].idx)){
+			let row = frappe.model.add_child(frm.doc, "Shipment Notification Subscription", "shipment_notification_subscription");
 			row.email = frm.doc.delivery_contact_email;
-			frm.refresh_fields("shipment_notification_subscriptions");
+			frm.refresh_fields("shipment_notification_subscription");
 		}
 		if (!frm.doc.delivery_to_send_shipping_notification) {
-			frm.events.remove_email_row(frm, 'shipment_notification_subscriptions', frm.doc.delivery_contact_email);
-			frm.refresh_fields("shipment_notification_subscriptions");
+			frm.events.remove_email_row(frm, 'shipment_notification_subscription', frm.doc.delivery_contact_email);
+			frm.refresh_fields("shipment_notification_subscription");
 		}
 	},
 	delivery_to_subscribe_to_status_updates: function(frm, cdt, cdn) {
 		if (frm.doc.delivery_contact_email && frm.doc.delivery_to_subscribe_to_status_updates
-				&& !validate_duplicate(frm, 'shipment_status_update_subscriptions', frm.doc.delivery_contact_email, locals[cdt][cdn].idx)) {
-			let row = frappe.model.add_child(frm.doc, "Shipment Status Update Subscriptions", "shipment_status_update_subscriptions");
+				&& !validate_duplicate(frm, 'shipment_status_update_subscription', frm.doc.delivery_contact_email, locals[cdt][cdn].idx)) {
+			let row = frappe.model.add_child(frm.doc, "Shipment Status Update Subscription", "shipment_status_update_subscription");
 			row.email = frm.doc.delivery_contact_email;
-			frm.refresh_fields("shipment_status_update_subscriptions");
+			frm.refresh_fields("shipment_status_update_subscription");
 		}
 		if (!frm.doc.delivery_to_subscribe_to_status_updates) {
-			frm.events.remove_email_row(frm, 'shipment_status_update_subscriptions', frm.doc.delivery_contact_email);
-			frm.refresh_fields("shipment_status_update_subscriptions");
+			frm.events.remove_email_row(frm, 'shipment_status_update_subscription', frm.doc.delivery_contact_email);
+			frm.refresh_fields("shipment_status_update_subscription");
 		}
 	},
 	remove_email_row: function(frm, table, fieldname) {
@@ -589,7 +613,7 @@
 					shipment_parcel: frm.doc.shipment_parcel,
 					description_of_content: frm.doc.description_of_content,
 					pickup_date: frm.doc.pickup_date,
-					pickup_contact_name: frm.doc.pickup_contact_name,
+					pickup_contact_name: frm.doc.pickup_from_type === 'Company' ? frm.doc.pickup_contact_person : frm.doc.pickup_contact_name,
 					delivery_contact_name: frm.doc.delivery_contact_name,
 					value_of_goods: frm.doc.value_of_goods
 				},
@@ -639,7 +663,7 @@
 	},
 	update_tracking: function(frm, service_provider, shipment_id) {
 		let delivery_notes = [];
-		(frm.doc.shipment_delivery_notes || []).forEach((d) => {
+		(frm.doc.shipment_delivery_note || []).forEach((d) => {
 			delivery_notes.push(d.delivery_note);
 		});
 		frappe.call({
@@ -661,14 +685,13 @@
 	}
 });
 
-frappe.ui.form.on('Shipment Delivery Notes', {
+frappe.ui.form.on('Shipment Delivery Note', {
 	delivery_note: function(frm, cdt, cdn) {
 		let row = locals[cdt][cdn];
 		if (row.delivery_note) {
 			let row_index = row.idx - 1;
-			if(validate_duplicate(frm, 'shipment_delivery_notes', row.delivery_note, row_index)) {
-				cur_frm.get_field('shipment_delivery_notes').grid.grid_rows[row_index].remove();
-				frappe.throw(__(`You have entered duplicate Delivery Notes. Please rectify and try again.`));
+			if(validate_duplicate(frm, 'shipment_delivery_note', row.delivery_note, row_index)) {
+				frappe.throw(__(`You have entered a duplicate Delivery Note on Row ${row.idx}. Please rectify and try again.`));
 			}
 		}
 	},
@@ -683,29 +706,27 @@
 });
 
 var validate_duplicate =  function(frm, table, fieldname, index){
-	let duplicate = false;
-	$.each(frm.doc[table], function(i, detail) {
-		// Email duplicate validation
-		if(detail.email === fieldname && !(index === i)) {
-			duplicate = true;
-			return;
-		}
-
-		// Delivery Note duplicate validation
-		if(detail.delivery_note === fieldname && !(index === i)) {
-			duplicate = true;
-			return;
-		}
-	});
-	return duplicate;
+	return (
+		table === 'shipment_delivery_note'
+			? frm.doc[table].some((detail, i) => detail.delivery_note === fieldname && !(index === i))
+			: frm.doc[table].some((detail, i) => detail.email === fieldname && !(index === i))
+	);
 };
 
 function select_from_available_services(frm, available_services) {
 	var headers = [ __("Service Provider"), __("Carrier"), __("Carrier’s Service"), __("Price"), "" ];
 	cur_frm.render_available_services = function(d, headers, data){
+		const arranged_data = data.reduce((prev, curr) => {
+			if (curr.is_preferred) {
+				prev.preferred_services.push(curr);
+			} else {
+				prev.other_services.push(curr);
+			}
+			return prev;
+		}, { preferred_services: [], other_services: [] });
 		d.fields_dict.available_services.$wrapper.html(
 			frappe.render_template('shipment_service_selector',
-				{'header_columns': headers, 'data': data}
+				{'header_columns': headers, 'data': arranged_data}
 			)
 		);
 	};
@@ -722,18 +743,18 @@
 	cur_frm.render_available_services(d, headers, available_services);
 	let shipment_notific_email = [];
 	let tracking_notific_email = [];
-	(frm.doc.shipment_notification_subscriptions || []).forEach((d) => {
+	(frm.doc.shipment_notification_subscription || []).forEach((d) => {
 		if (!d.unsubscribed) {
 			shipment_notific_email.push(d.email);
 		}
 	});
-	(frm.doc.shipment_status_update_subscriptions || []).forEach((d) => {
+	(frm.doc.shipment_status_update_subscription || []).forEach((d) => {
 		if (!d.unsubscribed) {
 			tracking_notific_email.push(d.email);
 		}
 	});
 	let delivery_notes = [];
-	(frm.doc.shipment_delivery_notes || []).forEach((d) => {
+	(frm.doc.shipment_delivery_note || []).forEach((d) => {
 		delivery_notes.push(d.delivery_note);
 	});
 	cur_frm.select_row = function(service_data){
@@ -750,7 +771,7 @@
 				shipment_parcel: frm.doc.shipment_parcel,
 				description_of_content: frm.doc.description_of_content,
 				pickup_date: frm.doc.pickup_date,
-				pickup_contact_name: frm.doc.pickup_contact_name,
+				pickup_contact_name: frm.doc.pickup_from_type === 'Company' ? frm.doc.pickup_contact_person : frm.doc.pickup_contact_name,
 				delivery_contact_name: frm.doc.delivery_contact_name,
 				value_of_goods: frm.doc.value_of_goods,
 				service_data: service_data,
diff --git a/erpnext/stock/doctype/shipment/shipment.json b/erpnext/stock/doctype/shipment/shipment.json
index b6656a2..bbfbb71 100644
--- a/erpnext/stock/doctype/shipment/shipment.json
+++ b/erpnext/stock/doctype/shipment/shipment.json
@@ -14,6 +14,7 @@
   "pickup",
   "pickup_address_name",
   "pickup_address",
+  "pickup_contact_person",
   "pickup_contact_name",
   "pickup_contact_email",
   "pickup_contact",
@@ -32,17 +33,17 @@
   "notification_details_section",
   "pickup_from_send_shipping_notification",
   "pickup_from_subscribe_to_status_updates",
-  "shipment_notification_subscriptions",
+  "shipment_notification_subscription",
   "column_break_27",
   "delivery_to_send_shipping_notification",
   "delivery_to_subscribe_to_status_updates",
-  "shipment_status_update_subscriptions",
+  "shipment_status_update_subscription",
   "parcels_section",
   "shipment_parcel",
   "parcel_template",
   "add_template",
   "column_break_28",
-  "shipment_delivery_notes",
+  "shipment_delivery_note",
   "shipment_details_section",
   "pallets",
   "value_of_goods",
@@ -125,10 +126,11 @@
    "read_only": 1
   },
   {
-   "depends_on": "eval: doc.pickup_customer || doc.pickup_supplier || doc.pickup_from_type == \"Company\"",
+   "depends_on": "eval: doc.pickup_customer || doc.pickup_supplier || doc.pickup_from_type !== \"Company\"",
    "fieldname": "pickup_contact_name",
    "fieldtype": "Link",
    "label": "Contact",
+   "mandatory_depends_on": "eval: doc.pickup_from_type !== 'Company'",
    "options": "Contact"
   },
   {
@@ -206,6 +208,7 @@
    "fieldname": "delivery_contact_name",
    "fieldtype": "Link",
    "label": "Contact",
+   "mandatory_depends_on": "eval: doc.delivery_from_type !== 'Company'",
    "options": "Contact"
   },
   {
@@ -240,12 +243,6 @@
    "label": "Subscribe to status updates"
   },
   {
-   "fieldname": "shipment_notification_subscriptions",
-   "fieldtype": "Table",
-   "label": "Shipment Notification Subscriptions",
-   "options": "Shipment Notification Subscriptions"
-  },
-  {
    "fieldname": "column_break_27",
    "fieldtype": "Column Break"
   },
@@ -262,12 +259,6 @@
    "label": "Subscribe to status updates"
   },
   {
-   "fieldname": "shipment_status_update_subscriptions",
-   "fieldtype": "Table",
-   "label": "Shipment Status Update Subscriptions",
-   "options": "Shipment Status Update Subscriptions"
-  },
-  {
    "fieldname": "parcels_section",
    "fieldtype": "Section Break",
    "label": "Parcels"
@@ -294,12 +285,6 @@
    "fieldtype": "Column Break"
   },
   {
-   "fieldname": "shipment_delivery_notes",
-   "fieldtype": "Table",
-   "label": "Shipment Delivery Notes",
-   "options": "Shipment Delivery Notes"
-  },
-  {
    "fieldname": "shipment_details_section",
    "fieldtype": "Section Break",
    "label": "Shipment details"
@@ -328,16 +313,14 @@
   {
    "default": "09:00",
    "fieldname": "pickup_from",
-   "fieldtype": "Select",
-   "label": "Pickup from",
-   "options": "09:00\n09:30\n10:00\n10:30\n11:00\n11:30\n12:00\n12:30\n13:00\n13:30\n14:00\n14:30\n15:00\n15:30\n16:00\n16:30\n17:00\n17:30\n18:00\n18:30\n19:00"
+   "fieldtype": "Time",
+   "label": "Pickup from"
   },
   {
    "default": "17:00",
    "fieldname": "pickup_to",
-   "fieldtype": "Select",
-   "label": "Pickup to",
-   "options": "09:00\n09:30\n10:00\n10:30\n11:00\n11:30\n12:00\n12:30\n13:00\n13:30\n14:00\n14:30\n15:00\n15:30\n16:00\n16:30\n17:00\n17:30\n18:00\n18:30\n19:00"
+   "fieldtype": "Time",
+   "label": "Pickup to"
   },
   {
    "fieldname": "column_break_36",
@@ -374,13 +357,15 @@
   },
   {
    "fieldname": "service_provider",
-   "fieldtype": "Read Only",
-   "label": "Service Provider"
+   "fieldtype": "Data",
+   "label": "Service Provider",
+   "read_only": 1
   },
   {
    "fieldname": "shipment_id",
-   "fieldtype": "Read Only",
-   "label": "Shipment ID"
+   "fieldtype": "Data",
+   "label": "Shipment ID",
+   "read_only": 1
   },
   {
    "fieldname": "shipment_amount",
@@ -399,23 +384,27 @@
   {
    "fieldname": "tracking_url",
    "fieldtype": "Small Text",
+   "hidden": 1,
    "label": "Tracking URL",
    "read_only": 1
   },
   {
    "fieldname": "carrier",
-   "fieldtype": "Read Only",
-   "label": "Carrier"
+   "fieldtype": "Data",
+   "label": "Carrier",
+   "read_only": 1
   },
   {
    "fieldname": "carrier_service",
-   "fieldtype": "Read Only",
-   "label": "Carrier Service"
+   "fieldtype": "Data",
+   "label": "Carrier Service",
+   "read_only": 1
   },
   {
    "fieldname": "awb_number",
-   "fieldtype": "Read Only",
-   "label": "AWB Number"
+   "fieldtype": "Data",
+   "label": "AWB Number",
+   "read_only": 1
   },
   {
    "fieldname": "tracking_status",
@@ -449,11 +438,37 @@
    "fieldtype": "Select",
    "label": "Incoterm",
    "options": "EXW (Ex Works)\nFCA (Free Carrier)\nCPT (Carriage Paid To)\nCIP (Carriage and Insurance Paid to)\nDPU (Delivered At Place Unloaded)\nDAP (Delivered At Place)\nDDP (Delivered Duty Paid)"
+  },
+  {
+   "fieldname": "shipment_delivery_note",
+   "fieldtype": "Table",
+   "label": "Shipment Delivery Note",
+   "options": "Shipment Delivery Note"
+  },
+  {
+   "fieldname": "shipment_notification_subscription",
+   "fieldtype": "Table",
+   "label": "Shipment Notification Subscription",
+   "options": "Shipment Notification Subscription"
+  },
+  {
+   "fieldname": "shipment_status_update_subscription",
+   "fieldtype": "Table",
+   "label": "Shipment Status Update Subscription",
+   "options": "Shipment Status Update Subscription"
+  },
+  {
+   "depends_on": "eval:doc.pickup_from_type === 'Company'",
+   "fieldname": "pickup_contact_person",
+   "fieldtype": "Link",
+   "label": "Pickup Contact Person",
+   "mandatory_depends_on": "eval:doc.pickup_from_type === 'Company'",
+   "options": "User"
   }
  ],
  "is_submittable": 1,
  "links": [],
- "modified": "2020-07-24 11:44:30.904612",
+ "modified": "2020-09-29 13:59:50.241744",
  "modified_by": "Administrator",
  "module": "Stock",
  "name": "Shipment",
diff --git a/erpnext/stock/doctype/shipment/shipment.py b/erpnext/stock/doctype/shipment/shipment.py
index e059bac..9b3c976 100644
--- a/erpnext/stock/doctype/shipment/shipment.py
+++ b/erpnext/stock/doctype/shipment/shipment.py
@@ -6,6 +6,7 @@
 import frappe
 import json
 from frappe import _
+from frappe.utils import flt
 from frappe.model.document import Document
 from erpnext.accounts.party import get_party_shipping_address
 from frappe.contacts.doctype.contact.contact import get_default_contact
@@ -32,7 +33,7 @@
 
 	def validate_weight(self):
 		for parcel in self.shipment_parcel:
-			if parcel.weight <= 0:
+			if flt(parcel.weight) <= 0:
 				frappe.throw(_('Parcel weight cannot be 0'))
 
 @frappe.whitelist()
@@ -52,12 +53,12 @@
 		if pickup_from_type != 'Company':
 			pickup_contact = get_contact(pickup_contact_name)
 		else:
-			pickup_contact = get_company_contact()
+			pickup_contact = get_company_contact(user=pickup_contact_name)
 		
 		if delivery_to_type != 'Company':
 			delivery_contact = get_contact(delivery_contact_name)
 		else:
-			delivery_contact = get_company_contact()
+			delivery_contact = get_company_contact(user=pickup_contact_name)
 		letmeship_prices = get_letmeship_available_services(
 			delivery_to_type=delivery_to_type,
 			pickup_address=pickup_address,
@@ -104,12 +105,12 @@
 	if pickup_from_type != 'Company':
 		pickup_contact = get_contact(pickup_contact_name)
 	else:
-		pickup_contact = get_company_contact()
+		pickup_contact = get_company_contact(user=pickup_contact_name)
 	
 	if delivery_to_type != 'Company':
 		delivery_contact = get_contact(delivery_contact_name)
 	else:
-		delivery_contact = get_company_contact()
+		delivery_contact = get_company_contact(user=pickup_contact_name)
 	if service_info['service_provider'] == LETMESHIP_PROVIDER:
 		shipment_info = create_letmeship_shipment(
 			pickup_address=pickup_address,
@@ -150,12 +151,9 @@
 		)
 
 	if shipment_info:
-		frappe.db.set_value('Shipment', shipment, 'service_provider', shipment_info.get('service_provider'))
-		frappe.db.set_value('Shipment', shipment, 'carrier', shipment_info.get('carrier'))
-		frappe.db.set_value('Shipment', shipment, 'carrier_service', shipment_info.get('carrier_service'))
-		frappe.db.set_value('Shipment', shipment, 'shipment_id', shipment_info.get('shipment_id'))
-		frappe.db.set_value('Shipment', shipment, 'shipment_amount', shipment_info.get('shipment_amount'))
-		frappe.db.set_value('Shipment', shipment, 'awb_number', shipment_info.get('awb_number'))
+		fields = ['service_provider', 'carrier', 'carrier_service', 'shipment_id', 'shipment_amount', 'awb_number']
+		for field in fields:
+			frappe.db.set_value('Shipment', shipment, field, shipment_info.get(field))
 		frappe.db.set_value('Shipment', shipment, 'status', 'Booked')
 		if delivery_notes:
 			update_delivery_note(delivery_notes=delivery_notes, shipment_info=shipment_info)
@@ -277,9 +275,17 @@
 		contact.phone = contact.mobile_no
 	return contact
 
+def match_parcel_service_type_carrier(shipment_prices, reference):
+	for idx, prices in enumerate(shipment_prices):
+		service_name = match_parcel_service_type_alias(prices.get(reference[0]), prices.get(reference[1]))
+		is_preferred = frappe.db.get_value('Parcel Service Type', service_name, 'show_in_preferred_services_list')
+		shipment_prices[idx].service_name = service_name
+		shipment_prices[idx].is_preferred = is_preferred
+	return shipment_prices
 
-def get_company_contact():
-	contact = frappe.db.get_value('User', frappe.session.user, [
+@frappe.whitelist()
+def get_company_contact(user):
+	contact = frappe.db.get_value('User', user, [
 		'first_name',
 		'last_name',
 		'email',
@@ -289,12 +295,4 @@
 	], as_dict=1)
 	if not contact.phone:
 		contact.phone = contact.mobile_no
-	return contact
-
-def match_parcel_service_type_carrier(shipment_prices, reference):
-	for idx, prices in enumerate(shipment_prices):
-		service_name = match_parcel_service_type_alias(prices.get(reference[0]), prices.get(reference[1]))
-		is_preferred = frappe.db.get_value('Parcel Service Type', service_name, 'show_in_preferred_services_list')
-		shipment_prices[idx].service_name = service_name
-		shipment_prices[idx].is_preferred = is_preferred
-	return shipment_prices
+	return contact
\ No newline at end of file
diff --git a/erpnext/stock/doctype/shipment/shipment_service_selector.html b/erpnext/stock/doctype/shipment/shipment_service_selector.html
index ed9b8bf..4ccbe34 100644
--- a/erpnext/stock/doctype/shipment/shipment_service_selector.html
+++ b/erpnext/stock/doctype/shipment/shipment_service_selector.html
@@ -1,59 +1,63 @@
-{% if (data.length) { %}
-	<div style="overflow-x:scroll;height: 550px;">
+{% if (data.preferred_services.length || data.other_services.length) { %}
+	<div style="overflow-x:scroll; height: 550px;">
 		<h5>{{ __("Preferred Services") }}</h5>
-		<table class="table table-bordered table-hover">
-			<thead class="grid-heading-row">
-				<tr style="text-align: center">
-					{% for (var i = 0; i < header_columns.length; i++) { %} 
-						<th style="text-align: center">{{ header_columns[i] }}</th>
-					{% } %}
-				</tr>
-			</thead>
-			<tbody>
-				{% for (var i = 0; i < data.length; i++) { %} 
-					{% if (data[i].is_preferred) { %} 
+		{% if (data.preferred_services.length) { %}
+			<table class="table table-bordered table-hover">
+				<thead class="grid-heading-row">
+					<tr style="text-align: center">
+						{% for (var i = 0; i < header_columns.length; i++) { %} 
+							<th style="text-align: center">{{ header_columns[i] }}</th>
+						{% } %}
+					</tr>
+				</thead>
+				<tbody>
+					{% for (var i = 0; i < data.preferred_services.length; i++) { %}
 						<tr style="text-align: center" id="data-list-{{i}}">
-							<td style="width:20%">{{ data[i].service_provider }}</td>
-							<td style="width:20%">{{ data[i].carrier }}</td>
-							<td style="width:40%">{{ data[i].service_name }}</td>
-							<td style="width:10%">{{ format_currency(data[i].total_price, 'EUR', 2) }}</td>
+							<td style="width:20%">{{ data.preferred_services[i].service_provider }}</td>
+							<td style="width:20%">{{ data.preferred_services[i].carrier }}</td>
+							<td style="width:40%">{{ data.preferred_services[i].service_name }}</td>
+							<td style="width:10%">{{ format_currency(data.preferred_services[i].total_price, 'EUR', 2) }}</td>
 							<td style="width:10%">
-								<button id="data-list-{{i}}" onclick='cur_frm.select_row({{JSON.stringify(data[i])}})' type="button" class="btn btn-success">
-									<span class="glyphicon glyphicon-check"></span>
+								<button id="data-list-{{i}}" onclick='cur_frm.select_row({{JSON.stringify(data.preferred_services[i])}})' type="button" class="btn btn-primary">
+									<span class="glyphicon glyphicon-ok"></span>
 								</button>
 							</td>
 						</tr>
 					{% } %}
-				{% } %}
-			</tbody>
-		</table>
+				</tbody>
+			</table>
+		{% } else { %}
+			<div><strong class="text-muted">{{ __("No Preferred Services Available") }}</strong></div>
+		{% } %}
 		<h5>{{ __("Other Services") }}</h5>
-		<table class="table table-bordered table-hover">
-			<thead class="grid-heading-row">
-				<tr style="text-align: center">
-					{% for (var i = 0; i < header_columns.length; i++) { %}
-						<th style="text-align: center" >{{ header_columns[i] }}</th>
-					{% } %}
-				</tr>
-			</thead>
-			<tbody>
-				{% for (var i = 0; i < data.length; i++) { %}
-					{% if (!data[i].is_preferred) { %}
+		{% if (data.other_services.length) { %}
+			<table class="table table-bordered table-hover">
+				<thead class="grid-heading-row">
+					<tr style="text-align: center">
+						{% for (var i = 0; i < header_columns.length; i++) { %}
+							<th style="text-align: center" >{{ header_columns[i] }}</th>
+						{% } %}
+					</tr>
+				</thead>
+				<tbody>
+					{% for (var i = 0; i < data.other_services.length; i++) { %}
 						<tr style="text-align: center" id="data-list-{{i}}">
-							<td style="width:20%">{{ data[i].service_provider }}</td>
-							<td style="width:20%">{{ data[i].carrier }}</td>
-							<td style="width:40%">{{ data[i].service_name }}</td>
-							<td style="width:10%">{{ format_currency(data[i].total_price, 'EUR', 2) }}</td>
+							<td style="width:20%">{{ data.other_services[i].service_provider }}</td>
+							<td style="width:20%">{{ data.other_services[i].carrier }}</td>
+							<td style="width:40%">{{ data.other_services[i].service_name }}</td>
+							<td style="width:10%">{{ format_currency(data.other_services[i].total_price, 'EUR', 2) }}</td>
 							<td style="width:10%">
-								<button id="data-list-{{i}}" onclick='cur_frm.select_row({{JSON.stringify(data[i])}})' type="button" class="btn btn-success">
-									<span class="glyphicon glyphicon-check"></span>
+								<button id="data-list-{{i}}" onclick='cur_frm.select_row({{JSON.stringify(data.other_services[i])}})' type="button" class="btn btn-primary">
+									<span class="glyphicon glyphicon-ok"></span>
 								</button>
 							</td>
 						</tr>
 					{% } %}
-				{% } %}
-			</tbody>
-		</table>
+				</tbody>
+			</table>
+		{% } else { %}
+			<div><strong class="text-muted">{{ __("No Services Available") }}</strong></div>
+		{% } %}
 	</div>
 {% } else { %}
 	<div><strong class="text-muted">{{ __("No Services Available") }}</strong></div>
diff --git a/erpnext/stock/doctype/shipment_delivery_notes/__init__.py b/erpnext/stock/doctype/shipment_delivery_note/__init__.py
similarity index 100%
rename from erpnext/stock/doctype/shipment_delivery_notes/__init__.py
rename to erpnext/stock/doctype/shipment_delivery_note/__init__.py
diff --git a/erpnext/stock/doctype/shipment_delivery_notes/shipment_delivery_notes.json b/erpnext/stock/doctype/shipment_delivery_note/shipment_delivery_note.json
similarity index 95%
rename from erpnext/stock/doctype/shipment_delivery_notes/shipment_delivery_notes.json
rename to erpnext/stock/doctype/shipment_delivery_note/shipment_delivery_note.json
index fbc01d9..9651e3f 100644
--- a/erpnext/stock/doctype/shipment_delivery_notes/shipment_delivery_notes.json
+++ b/erpnext/stock/doctype/shipment_delivery_note/shipment_delivery_note.json
@@ -31,7 +31,7 @@
  "modified": "2020-07-09 12:55:01.134270",
  "modified_by": "Administrator",
  "module": "Stock",
- "name": "Shipment Delivery Notes",
+ "name": "Shipment Delivery Note",
  "owner": "Administrator",
  "permissions": [],
  "quick_entry": 1,
diff --git a/erpnext/stock/doctype/shipment_delivery_notes/shipment_delivery_notes.py b/erpnext/stock/doctype/shipment_delivery_note/shipment_delivery_note.py
similarity index 85%
rename from erpnext/stock/doctype/shipment_delivery_notes/shipment_delivery_notes.py
rename to erpnext/stock/doctype/shipment_delivery_note/shipment_delivery_note.py
index ed936c6..4342151 100644
--- a/erpnext/stock/doctype/shipment_delivery_notes/shipment_delivery_notes.py
+++ b/erpnext/stock/doctype/shipment_delivery_note/shipment_delivery_note.py
@@ -6,5 +6,5 @@
 # import frappe
 from frappe.model.document import Document
 
-class ShipmentDeliveryNotes(Document):
+class ShipmentDeliveryNote(Document):
 	pass
diff --git a/erpnext/stock/doctype/shipment_notification_subscriptions/__init__.py b/erpnext/stock/doctype/shipment_notification_subscription/__init__.py
similarity index 100%
rename from erpnext/stock/doctype/shipment_notification_subscriptions/__init__.py
rename to erpnext/stock/doctype/shipment_notification_subscription/__init__.py
diff --git a/erpnext/stock/doctype/shipment_notification_subscriptions/shipment_notification_subscriptions.json b/erpnext/stock/doctype/shipment_notification_subscription/shipment_notification_subscription.json
similarity index 93%
rename from erpnext/stock/doctype/shipment_notification_subscriptions/shipment_notification_subscriptions.json
rename to erpnext/stock/doctype/shipment_notification_subscription/shipment_notification_subscription.json
index bd9b800..d927d99 100644
--- a/erpnext/stock/doctype/shipment_notification_subscriptions/shipment_notification_subscriptions.json
+++ b/erpnext/stock/doctype/shipment_notification_subscription/shipment_notification_subscription.json
@@ -30,7 +30,7 @@
  "modified": "2020-07-09 12:55:14.217387",
  "modified_by": "Administrator",
  "module": "Stock",
- "name": "Shipment Notification Subscriptions",
+ "name": "Shipment Notification Subscription",
  "owner": "Administrator",
  "permissions": [],
  "quick_entry": 1,
diff --git a/erpnext/stock/doctype/shipment_notification_subscriptions/shipment_notification_subscriptions.py b/erpnext/stock/doctype/shipment_notification_subscription/shipment_notification_subscription.py
similarity index 82%
rename from erpnext/stock/doctype/shipment_notification_subscriptions/shipment_notification_subscriptions.py
rename to erpnext/stock/doctype/shipment_notification_subscription/shipment_notification_subscription.py
index 28ead7f..c816e43 100644
--- a/erpnext/stock/doctype/shipment_notification_subscriptions/shipment_notification_subscriptions.py
+++ b/erpnext/stock/doctype/shipment_notification_subscription/shipment_notification_subscription.py
@@ -6,5 +6,5 @@
 # import frappe
 from frappe.model.document import Document
 
-class ShipmentNotificationSubscriptions(Document):
+class ShipmentNotificationSubscription(Document):
 	pass
diff --git a/erpnext/stock/doctype/shipment_parcel_template/shipment_parcel_template.json b/erpnext/stock/doctype/shipment_parcel_template/shipment_parcel_template.json
index ec2bb1c..4735d9f 100644
--- a/erpnext/stock/doctype/shipment_parcel_template/shipment_parcel_template.json
+++ b/erpnext/stock/doctype/shipment_parcel_template/shipment_parcel_template.json
@@ -1,12 +1,12 @@
 {
  "actions": [],
- "autoname": "field:preset_name",
+ "autoname": "field:parcel_template_name",
  "creation": "2020-07-09 11:43:43.470339",
  "doctype": "DocType",
  "editable_grid": 1,
  "engine": "InnoDB",
  "field_order": [
-  "preset_name",
+  "parcel_template_name",
   "length",
   "width",
   "height",
@@ -14,14 +14,6 @@
  ],
  "fields": [
   {
-   "fieldname": "preset_name",
-   "fieldtype": "Data",
-   "in_list_view": 1,
-   "label": "Preset Name",
-   "reqd": 1,
-   "unique": 1
-  },
-  {
    "fieldname": "length",
    "fieldtype": "Int",
    "in_list_view": 1,
@@ -49,10 +41,18 @@
    "label": "Weight (kg)",
    "precision": "1",
    "reqd": 1
+  },
+  {
+   "fieldname": "parcel_template_name",
+   "fieldtype": "Data",
+   "in_list_view": 1,
+   "label": "Parcel Template Name",
+   "reqd": 1,
+   "unique": 1
   }
  ],
  "links": [],
- "modified": "2020-07-10 12:53:22.772826",
+ "modified": "2020-09-28 12:51:00.320421",
  "modified_by": "Administrator",
  "module": "Stock",
  "name": "Shipment Parcel Template",
diff --git a/erpnext/stock/doctype/shipment_status_update_subscriptions/__init__.py b/erpnext/stock/doctype/shipment_status_update_subscription/__init__.py
similarity index 100%
rename from erpnext/stock/doctype/shipment_status_update_subscriptions/__init__.py
rename to erpnext/stock/doctype/shipment_status_update_subscription/__init__.py
diff --git a/erpnext/stock/doctype/shipment_status_update_subscriptions/shipment_status_update_subscriptions.json b/erpnext/stock/doctype/shipment_status_update_subscription/shipment_status_update_subscription.json
similarity index 92%
rename from erpnext/stock/doctype/shipment_status_update_subscriptions/shipment_status_update_subscriptions.json
rename to erpnext/stock/doctype/shipment_status_update_subscription/shipment_status_update_subscription.json
index 3b86b40..a7fe4a4 100644
--- a/erpnext/stock/doctype/shipment_status_update_subscriptions/shipment_status_update_subscriptions.json
+++ b/erpnext/stock/doctype/shipment_status_update_subscription/shipment_status_update_subscription.json
@@ -30,7 +30,7 @@
  "modified": "2020-07-09 12:55:27.615463",
  "modified_by": "Administrator",
  "module": "Stock",
- "name": "Shipment Status Update Subscriptions",
+ "name": "Shipment Status Update Subscription",
  "owner": "Administrator",
  "permissions": [],
  "quick_entry": 1,
diff --git a/erpnext/stock/doctype/shipment_notification_subscriptions/shipment_notification_subscriptions.py b/erpnext/stock/doctype/shipment_status_update_subscription/shipment_status_update_subscription.py
similarity index 82%
copy from erpnext/stock/doctype/shipment_notification_subscriptions/shipment_notification_subscriptions.py
copy to erpnext/stock/doctype/shipment_status_update_subscription/shipment_status_update_subscription.py
index 28ead7f..1b006d7 100644
--- a/erpnext/stock/doctype/shipment_notification_subscriptions/shipment_notification_subscriptions.py
+++ b/erpnext/stock/doctype/shipment_status_update_subscription/shipment_status_update_subscription.py
@@ -6,5 +6,5 @@
 # import frappe
 from frappe.model.document import Document
 
-class ShipmentNotificationSubscriptions(Document):
+class ShipmentStatusUpdateSubscription(Document):
 	pass
diff --git a/erpnext/stock/doctype/shipment_status_update_subscriptions/shipment_status_update_subscriptions.py b/erpnext/stock/doctype/shipment_status_update_subscriptions/shipment_status_update_subscriptions.py
deleted file mode 100644
index a8e31ea..0000000
--- a/erpnext/stock/doctype/shipment_status_update_subscriptions/shipment_status_update_subscriptions.py
+++ /dev/null
@@ -1,10 +0,0 @@
-# -*- coding: utf-8 -*-
-# Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and contributors
-# For license information, please see license.txt
-
-from __future__ import unicode_literals
-# import frappe
-from frappe.model.document import Document
-
-class ShipmentStatusUpdateSubscriptions(Document):
-	pass