From 393d19ca85e9cd6231b16d9731730496dc578c4a Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 8 Jun 2006 00:16:35 +0000
Subject: [PATCH] Fix SQL error on middle creator remove

Added creatorTypeID to the PK in itemCreators, because a creator could conceivably have two roles on an item

Throw an error if attempt to save() an item with two identical creator/creatorTypeID combinations, since, assuming this shouldn't be allowed (i.e. we don't have a four-column PK), there's really no way to handle this elegantly on my end -- interface code can use new method Item.creatorExists(firstName, lastName, creatorTypeID, skipIndex), with skipIndex set to the current index, to make sure this isn't done
---
 .../content/scholar/xpcom/data_access.js      | 66 +++++++++++++------
 .../content/scholar/xpcom/schema.js           |  8 +--
 schema.sql                                    |  4 +-
 3 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/chrome/chromeFiles/content/scholar/xpcom/data_access.js b/chrome/chromeFiles/content/scholar/xpcom/data_access.js
index e2543f58e..9e80bf1be 100644
--- a/chrome/chromeFiles/content/scholar/xpcom/data_access.js
+++ b/chrome/chromeFiles/content/scholar/xpcom/data_access.js
@@ -204,11 +204,12 @@ Scholar.Item.prototype.setCreator = function(orderIndex, firstName, lastName, cr
 		lastName = '';
 	}
 	
+	// If creator at this position hasn't changed, cancel
 	if (this._creators.has(orderIndex) &&
 		this._creators.get(orderIndex)['firstName']==firstName &&
 		this._creators.get(orderIndex)['lastName']==lastName &&
 		this._creators.get(orderIndex)['creatorTypeID']==creatorTypeID){
-		return true;
+		return false;
 	}
 	
 	if (!creatorTypeID){
@@ -250,6 +251,23 @@ Scholar.Item.prototype.removeCreator = function(orderIndex){
 }
 
 
+Scholar.Item.prototype.creatorExists = function(firstName, lastName, creatorTypeID, skipIndex){
+	for (var j=0, len=this.numCreators(); j<len; j++){
+		if (typeof skipIndex!='undefined' && skipIndex==j){
+			continue;
+		}
+		var creator2 = this.getCreator(j);
+		
+		if (firstName==creator2['firstName'] &&
+			lastName==creator2['lastName'] &&
+			creatorTypeID==creator2['creatorTypeID']){
+			return true;
+		}
+	}
+	return false;
+}
+
+
 /*
  * Retrieves (and loads from DB, if necessary) an itemData field value
  *
@@ -365,7 +383,7 @@ Scholar.Item.prototype.save = function(){
 			
 			// Always update modified time
 			sql += "dateModified=CURRENT_TIMESTAMP ";
-			sql += "WHERE itemID=?;\n";
+			sql += "WHERE itemID=?";
 			sqlValues.push({'int':this.getID()});
 			
 			Scholar.DB.query(sql, sqlValues);
@@ -374,17 +392,28 @@ Scholar.Item.prototype.save = function(){
 			// Creators
 			//
 			if (this._changedCreators.length){
+				for (var i=0, len=this.numCreators(); i<len; i++){
+					var creator = this.getCreator(i);
+					if (this.creatorExists(creator['firstName'],
+							creator['lastName'], creator['creatorTypeID'], i)){
+						throw('Cannot add duplicate creator/creatorType '
+							+ 'to item ' + this.getID());
+					}
+				}
+				
 				for (orderIndex in this._changedCreators.items){
 					Scholar.debug('Creator ' + orderIndex + ' has changed', 4);
 					
 					var creator = this.getCreator(orderIndex);
 					
-					// If empty, delete at position
+					// Delete at position
+					sql2 = 'DELETE FROM itemCreators'
+						+ ' WHERE itemID=' + this.getID()
+						+ ' AND orderIndex=' + orderIndex;
+					Scholar.DB.query(sql2);
+					
+					// If empty, move on
 					if (!creator['firstName'] && !creator['lastName']){
-						sql2 = 'DELETE FROM itemCreators '
-							+ ' WHERE itemID=' + this.getID()
-							+ ' AND orderIndex=' + orderIndex;
-						Scholar.DB.query(sql2);
 						continue;
 					}
 					
@@ -402,23 +431,22 @@ Scholar.Item.prototype.save = function(){
 						);
 					}
 					
-					
-					// If there's a creator at this position, update
-					// with new creator data
+					// If this creator and creatorType exists elsewhere, move it
 					sql2 = 'SELECT COUNT(*) FROM itemCreators'
 						+ ' WHERE itemID=' + this.getID()
-						+ ' AND orderIndex=' + orderIndex;
+						+ ' AND creatorID=' + creatorID
+						+ ' AND creatorTypeID=' + creator['creatorTypeID'];
 					
 					if (Scholar.DB.valueQuery(sql2)){
-						sql = 'UPDATE itemCreators SET creatorID=?, '
-							+ 'creatorTypeID=? WHERE itemID=?'
-							+ " AND orderIndex=?";
+						sql = 'UPDATE itemCreators SET orderIndex=? '
+							+ "WHERE itemID=? AND creatorID=? AND "
+							+ "creatorTypeID=?";
 							
 						sqlValues = [
-							{'int':creatorID},
-							{'int':creator['creatorTypeID']},
+							{'int':orderIndex},
 							{'int':this.getID()},
-							{'int':orderIndex}
+							{'int':creatorID},
+							{'int':creator['creatorTypeID']}
 						];
 						
 						Scholar.DB.query(sql, sqlValues);
@@ -553,7 +581,7 @@ Scholar.Item.prototype.save = function(){
 			for (var i=0; i<sqlValues.length; i++){
 				sql += '?,';
 			}
-			sql = sql.substring(0,sql.length-1) + ");\n";
+			sql = sql.substring(0,sql.length-1) + ")";
 			
 			// Save basic data to items table and get new ID
 			var itemID = Scholar.DB.query(sql,sqlValues);
@@ -616,7 +644,7 @@ Scholar.Item.prototype.save = function(){
 					sql = 'INSERT INTO itemCreators VALUES ('
 						+ itemID + ',' + creatorID + ','
 						+ creator['creatorTypeID'] + ', ' + orderIndex
-						+ ");\n";
+						+ ")";
 					Scholar.DB.query(sql);
 				}
 			}
diff --git a/chrome/chromeFiles/content/scholar/xpcom/schema.js b/chrome/chromeFiles/content/scholar/xpcom/schema.js
index 965e72997..c523aca23 100644
--- a/chrome/chromeFiles/content/scholar/xpcom/schema.js
+++ b/chrome/chromeFiles/content/scholar/xpcom/schema.js
@@ -62,7 +62,7 @@ Scholar.Schema = new function(){
 		if (Scholar.DB.tableExists('version')){
 			try {
 				var dbVersion = Scholar.DB.valueQuery("SELECT version FROM "
-					+ " version WHERE schema='" + schema + "'");
+					+ "version WHERE schema='" + schema + "'");
 			}
 			// DEBUG: this is temporary to handle version table schema change
 			catch(e){
@@ -224,7 +224,7 @@ Scholar.Schema = new function(){
 		//
 		// Change this value to match the schema version
 		//
-		var toVersion = 15;
+		var toVersion = 16;
 		
 		if (toVersion != _getSchemaSQLVersion()){
 			throw('Schema version does not match version in _migrateSchema()');
@@ -239,8 +239,8 @@ Scholar.Schema = new function(){
 		// Each block performs the changes necessary to move from the
 		// previous revision to that one.
 		for (var i=parseInt(fromVersion) + 1; i<=toVersion; i++){
-			if (i==15){
-				// do stuff
+			if (i==16){
+				_initializeSchema();
 			}
 		}
 		
diff --git a/schema.sql b/schema.sql
index c9d7dfc2b..fb63d9368 100644
--- a/schema.sql
+++ b/schema.sql
@@ -1,4 +1,4 @@
--- 15
+-- 16
 
     DROP TABLE IF EXISTS version;
     CREATE TABLE version (
@@ -99,7 +99,7 @@
         creatorID INT,
         creatorTypeID INT DEFAULT 1,
         orderIndex INT DEFAULT 0,
-        PRIMARY KEY (itemID, creatorID),
+        PRIMARY KEY (itemID, creatorID, creatorTypeID),
         FOREIGN KEY (itemID) REFERENCES items(itemID),
         FOREIGN KEY (creatorID) REFERENCES creators(creatorID)
         FOREIGN KEY (creatorTypeID) REFERENCES creatorTypes(creatorTypeID)