From b4522535e86dafa0648a932b428e899ec6b76ec3 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Fri, 19 Jun 2020 04:38:39 -0400
Subject: [PATCH] Add _hasFieldChanged()/_getChangedField/_getLatestField() to
 DataObject

Make it easier to use the new `_changedData` approach to object
modifications (currently used only for `tags` and `deleted`) where
changed data is stored in a separate object rather than in the primary
variables. _getLatestField() can be used to return either the new
unsaved value or the current saved value.
---
 .../content/zotero/xpcom/data/dataObject.js   | 20 ++++++++++++++++++-
 chrome/content/zotero/xpcom/data/item.js      | 14 ++++++-------
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js
index ba9f7fe3e5..27db4e6e85 100644
--- a/chrome/content/zotero/xpcom/data/dataObject.js
+++ b/chrome/content/zotero/xpcom/data/dataObject.js
@@ -735,11 +735,26 @@ Zotero.DataObject.prototype._markAllDataTypeLoadStates = function (loaded) {
 	}
 }
 
+Zotero.DataObject.prototype._hasFieldChanged = function (field) {
+	return field in this._changedData;
+};
+
+Zotero.DataObject.prototype._getChangedField = function (field) {
+	return this._changedData[field];
+};
+
 /**
  * Get either the unsaved value of a field or the saved value if unchanged since the last save
  */
 Zotero.DataObject.prototype._getLatestField = function (field) {
-        return this._changedData[field] !== undefined ? this._changedData[field] : this['_' + field];
+	return this._changedData[field] || this['_' + field];
+};
+
+/**
+ * Get either the unsaved value of a field or the saved value if unchanged since the last save
+ */
+Zotero.DataObject.prototype._getLatestField = function (field) {
+	return this._changedData[field] !== undefined ? this._changedData[field] : this['_' + field];
 };
 
 /**
@@ -753,6 +768,9 @@ Zotero.DataObject.prototype._markFieldChange = function (field, value) {
 		if (Array.isArray(value)) {
 			this._changedData[field] = [...value];
 		}
+		else if (typeof value === 'object' && value !== null) {
+			this._changedData[field] = Object.assign({}, value);
+		}
 		else {
 			this._changedData[field] = value;
 		}
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 43b763a4c2..f55f6ba54d 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -1758,9 +1758,9 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 	}
 	
 	// Tags
-	if (this._changedData.tags) {
+	if (this._hasFieldChanged('tags')) {
 		let oldTags = this._tags;
-		let newTags = this._changedData.tags;
+		let newTags = this._getChangedField('tags');
 		this._clearChanged('tags');
 		this._markForReload('tags');
 		
@@ -3355,7 +3355,7 @@ Zotero.Item.prototype.clearBestAttachmentState = function () {
 Zotero.Item.prototype.getTags = function () {
 	this._requireData('tags');
 	// BETTER DEEP COPY?
-	return JSON.parse(JSON.stringify(this._changedData.tags || this._tags));
+	return JSON.parse(JSON.stringify(this._getLatestField('tags')));
 };
 
 
@@ -3367,7 +3367,7 @@ Zotero.Item.prototype.getTags = function () {
  */
 Zotero.Item.prototype.hasTag = function (tagName) {
 	this._requireData('tags');
-	var tags = this._changedData.tags || this._tags;
+	var tags = this._getLatestField('tags');
 	return tags.some(tagData => tagData.tag == tagName);
 }
 
@@ -3377,7 +3377,7 @@ Zotero.Item.prototype.hasTag = function (tagName) {
  */
 Zotero.Item.prototype.getTagType = function (tagName) {
 	this._requireData('tags');
-	var tags = this._changedData.tags || this._tags;
+	var tags = this._getLatestField('tags');
 	for (let tag of tags) {
 		if (tag.tag === tagName) {
 			return tag.type ? tag.type : 0;
@@ -3397,7 +3397,7 @@ Zotero.Item.prototype.getTagType = function (tagName) {
  */
 Zotero.Item.prototype.setTags = function (tags) {
 	this._requireData('tags');
-	var oldTags = this._changedData.tags || this._tags;
+	var oldTags = this._getLatestField('tags');
 	var newTags = tags.concat()
 		// Allow array of strings
 		.map(tag => typeof tag == 'string' ? { tag } : tag);
@@ -3505,7 +3505,7 @@ Zotero.Item.prototype.replaceTag = function (oldTag, newTag) {
  */
 Zotero.Item.prototype.removeTag = function(tagName) {
 	this._requireData('tags');
-	var oldTags = this._changedData.tags || this._tags;
+	var oldTags = this._getLatestField('tags');
 	var newTags = oldTags.filter(tagData => tagData.tag !== tagName);
 	if (newTags.length == oldTags.length) {
 		Zotero.debug('Cannot remove missing tag ' + tagName + ' from item ' + this.libraryKey);