Fix group saving and copying attachments between libraries

chrome/content/zotero/bindings/tagselector.xml

@@ -226,9 +226,11 @@
 						var emptyRegular = true;
 						var tagsToggleBox = this.id('tags-toggle');
 						
-						var tagColors = yield Zotero.Tags.getColors(this.libraryID);
+						var tagColors = yield Zotero.Tags.getColors(this.libraryID)
+							.tap(() => Zotero.Promise.check(this.item));
 						if (fetch || this._dirty) {
-							this._tags = yield Zotero.Tags.getAll(this.libraryID, this._types);
+							this._tags = yield Zotero.Tags.getAll(this.libraryID, this._types)
+								.tap(() => Zotero.Promise.check(this.item));
 							
 							// Remove children
 							tagsToggleBox.textContent = "";

chrome/content/zotero/xpcom/attachments.js

@@ -1115,6 +1115,8 @@ Zotero.Attachments = new function(){
 			throw ("Attachment is already in library " + libraryID);
 		}
 		
+		Zotero.DB.requireTransaction();
+		
 		attachment.loadItemData();
 		var newAttachment = yield attachment.clone(libraryID);
 		if (attachment.isImportedAttachment()) {
@@ -1124,7 +1126,7 @@ Zotero.Attachments = new function(){
 		if (parentItemID) {
 			newAttachment.parentID = parentItemID;
 		}
-		yield newAttachment.saveTx();
+		yield newAttachment.save();
 		
 		// Copy over files if they exist
 		if (newAttachment.isImportedAttachment() && attachment.getFile()) {

chrome/content/zotero/xpcom/data/group.js

@@ -52,7 +52,6 @@ Zotero.Group.prototype.__defineGetter__('objectType', function () { return 'grou
 Zotero.Group.prototype.__defineGetter__('id', function () { return this._get('id'); });
 Zotero.Group.prototype.__defineSetter__('id', function (val) { this._set('id', val); });
 Zotero.Group.prototype.__defineGetter__('libraryID', function () { return this._get('libraryID'); });
-Zotero.Group.prototype.__defineSetter__('libraryID', function (val) { return this._set('libraryID', val); });
 Zotero.Group.prototype.__defineGetter__('name', function () { return this._get('name'); });
 Zotero.Group.prototype.__defineSetter__('name', function (val) { this._set('name', val); });
 Zotero.Group.prototype.__defineGetter__('description', function () { return this._get('description'); });
@@ -221,11 +220,7 @@ Zotero.Group.prototype.hasItem = function (item) {
 
 Zotero.Group.prototype.save = Zotero.Promise.coroutine(function* () {
 	if (!this.id) {
-		throw ("ID not set in Zotero.Group.save()");
+		throw new Error("Group id not set");
-	}
-	
-	if (!this.libraryID) {
-		throw ("libraryID not set in Zotero.Group.save()");
 	}
 	
 	if (!this._changed) {
@@ -240,7 +235,6 @@ Zotero.Group.prototype.save = Zotero.Promise.coroutine(function* () {
 		
 		var sqlColumns = [
 			'groupID',
-			'libraryID',
 			'name',
 			'description',
 			'editable',
@@ -249,7 +243,6 @@ Zotero.Group.prototype.save = Zotero.Promise.coroutine(function* () {
 		];
 		var sqlValues = [
 			this.id,
-			this.libraryID,
 			this.name,
 			this.description,
 			this.editable ? 1 : 0,
@@ -258,28 +251,34 @@ Zotero.Group.prototype.save = Zotero.Promise.coroutine(function* () {
 		];
 		
 		if (isNew) {
-			if (!Zotero.Libraries.exists(this.libraryID)) {
+			var { id: libraryID } = yield Zotero.Libraries.add('group');
-				Zotero.Libraries.add(this.libraryID, 'group');
+			sqlColumns.push('libraryID');
-			}
+			sqlValues.push(libraryID);
 			
-			var sql = "INSERT INTO groups (" + sqlColumns.join(', ') + ") "
+			let sql = "INSERT INTO groups (" + sqlColumns.join(', ') + ") "
 						+ "VALUES (" + sqlColumns.map(() => '?').join(', ') + ")";
 			yield Zotero.DB.queryAsync(sql, sqlValues);
 		}
 		else {
 			sqlColumns.shift();
-			sqlValues.shift();
+			sqlValues.push(sqlValues.shift());
 			
-			var sql = "UPDATE groups SET "
+			let sql = "UPDATE groups SET " + sqlColumns.map(function (val) val + '=?').join(', ')
-				+ sqlColumns.map(function (val) val + '=?').join(', ')
 				+ " WHERE groupID=?";
-			sqlValues.push(this.id);
 			yield Zotero.DB.queryAsync(sql, sqlValues);
 		}
+		
+		if (isNew) {
+			Zotero.DB.addCurrentCallback("commit", Zotero.Promise.coroutine(function* () {
+				yield this.load();
+				Zotero.Groups.register(this)
+			}.bind(this)));
+			Zotero.Notifier.trigger('add', 'group', this.id);
+		}
+		else {
+			Zotero.Notifier.trigger('modify', 'group', this.id);
+		}
 	}.bind(this));
-	
-	Zotero.Groups.register(this);
-	Zotero.Notifier.trigger('add', 'group', this.id);
 });
 
 

chrome/content/zotero/xpcom/data/libraries.js

@@ -75,23 +75,26 @@ Zotero.Libraries = new function () {
 	}
 	
 	
-	this.add = Zotero.Promise.coroutine(function* (libraryID, type) {
+	this.add = Zotero.Promise.coroutine(function* (type) {
+		Zotero.DB.requireTransaction();
+		
 		switch (type) {
 			case 'group':
 				break;
 			
 			default:
-				throw new Error("Invalid library type '" + type + "' in Zotero.Libraries.add()");
+				throw new Error("Invalid library type '" + type + "'");
 		}
 		
+		var libraryID = yield Zotero.ID.get('libraries');
+		
 		var sql = "INSERT INTO libraries (libraryID, libraryType) VALUES (?, ?)";
 		yield Zotero.DB.queryAsync(sql, [libraryID, type]);
 		
 		// Re-fetch from DB to get auto-filled defaults
 		var sql = "SELECT * FROM libraries WHERE libraryID=?";
-		var row = Zotero.DB.rowQueryAsync(sql, [libraryID]);
+		var row = yield Zotero.DB.rowQueryAsync(sql, [libraryID]);
-		_libraryData[row.libraryID] = parseDBRow(row);
+		return _libraryData[row.libraryID] = parseDBRow(row);
-		return row;
 	});
 	
 	
@@ -210,6 +213,7 @@ Zotero.Libraries = new function () {
 	
 	function parseDBRow(row) {
 		return {
+			id: row.libraryID,
 			type: row.libraryType,
 			version: row.version,
 			lastSyncTime: row.lastsync != 0 ? new Date(row.lastsync * 1000) : false

test/tests/collectionTreeViewTest.js

@@ -209,5 +209,59 @@ describe("Zotero.CollectionTreeView", function() {
 			var treeRow = itemsView.getRow(0);
 			assert.equal(treeRow.ref.id, item.id);
 		})
+		
+		it("should add an item to a library", function* () {
+			var group = new Zotero.Group;
+			group.id = 75161251;
+			group.name = "Test";
+			group.description = "";
+			group.editable = true;
+			group.filesEditable = true;
+			group.version = 1234;
+			yield group.save();
+			
+			var item = yield createDataObject('item', false, {
+				skipSelect: true
+			});
+			var file = getTestDataDirectory();
+			file.append('test.png');
+			var attachmentID = yield Zotero.Attachments.importFromFile({
+				file: file,
+				parentItemID: item.id
+			});
+			
+			var row = collectionsView.getRowByID("L" + group.libraryID);
+			
+			// Simulate a drag and drop
+			var stub = sinon.stub(Zotero.DragDrop, "getDragTarget");
+			stub.returns(collectionsView.getRow(row));
+			collectionsView.drop(row, 0, {
+				dropEffect: 'copy',
+				effectAllowed: 'copy',
+				mozSourceNode: win.document.getElementById('zotero-items-tree'),
+				types: {
+					contains: function (type) {
+						return type == 'zotero/item';
+					}
+				},
+				getData: function (type) {
+					if (type == 'zotero/item') {
+						return "" + item.id;
+					}
+				}
+			});
+			
+			// Add observer to wait for collection add
+			var ids = yield waitForItemEvent("add");
+			
+			stub.restore();
+			yield collectionsView.selectLibrary(group.libraryID);
+			yield waitForItemsLoad(win);
+			
+			var itemsView = win.ZoteroPane.itemsView
+			assert.equal(itemsView.rowCount, 1);
+			var treeRow = itemsView.getRow(0);
+			assert.equal(treeRow.ref.id, ids[0]);
+		})
 	})
 })