Move editable and filesEditable props to libraries table [DB reupgrade]

And add group.fromJSON(json, userID), which sets editable and
filesEditable properties based on the group JSON (libraryReading, role
lists, etc.) and the given user

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

@@ -101,9 +101,6 @@ Zotero.Group.prototype._set = function (field, val) {
 }
 
 
-
-
-
 /*
  * Build group from database
  */
@@ -140,8 +137,8 @@ Zotero.Group.prototype.loadFromRow = function(row) {
 	this._libraryID = row.libraryID;
 	this._name = row.name;
 	this._description = row.description;
-	this._editable = !!row.editable;
+	this._editable = Zotero.Libraries.isEditable(row.libraryID);
-	this._filesEditable = !!row.filesEditable;
+	this._filesEditable = Zotero.Libraries.isFilesEditable(row.libraryID);
 	this._version = row.version;
 }
 
@@ -209,21 +206,23 @@ Zotero.Group.prototype.save = Zotero.Promise.coroutine(function* () {
 			'groupID',
 			'name',
 			'description',
-			'editable',
-			'filesEditable',
 			'version'
 		];
 		var sqlValues = [
 			this.id,
 			this.name,
 			this.description,
-			this.editable ? 1 : 0,
-			this.filesEditable ? 1 : 0,
 			this.version
 		];
 		
 		if (isNew) {
-			var { id: libraryID } = yield Zotero.Libraries.add('group');
+			let { id: libraryID } = yield Zotero.Libraries.add(
+				'group',
+				{
+					editable: this.editable,
+					filesEditable: this.filesEditable
+				}
+			);
 			sqlColumns.push('libraryID');
 			sqlValues.push(libraryID);
 			
@@ -238,6 +237,9 @@ Zotero.Group.prototype.save = Zotero.Promise.coroutine(function* () {
 			let sql = "UPDATE groups SET " + sqlColumns.map(function (val) val + '=?').join(', ')
 				+ " WHERE groupID=?";
 			yield Zotero.DB.queryAsync(sql, sqlValues);
+			
+			yield Zotero.Libraries.setEditable(this.libraryID, this.editable);
+			yield Zotero.Libraries.setFilesEditable(this.libraryID, this.filesEditable);
 		}
 		
 		if (isNew) {
@@ -258,64 +260,75 @@ Zotero.Group.prototype.save = Zotero.Promise.coroutine(function* () {
 * Deletes group and all descendant objects
 **/
 Zotero.Group.prototype.erase = Zotero.Promise.coroutine(function* () {
-	yield Zotero.DB.executeTransaction(function* () {
+	Zotero.DB.requireTransaction();
-		var notifierData = {};
-		notifierData[this.id] = this.serialize(); // TODO: Replace with JSON
-		
-		var sql, ids, obj;
-		
-		// Delete items
-		var types = ['item', 'collection', 'search'];
-		for (let type of types) {
-			let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type);
-			let sql = "SELECT " + objectsClass.idColumn + " FROM " + objectsClass.table
-				+ " WHERE libraryID=?";
-			ids = yield Zotero.DB.columnQueryAsync(sql, this.libraryID);
-			for (let i = 0; i < ids.length; i++) {
-				let id = ids[i];
-				let obj = yield objectsClass.getAsync(id, { noCache: true });
-				// Descendent object may have already been deleted
-				if (!obj) {
-					continue;
-				}
-				yield obj.erase({
-					skipNotifier: true
-				});
-			}
-		}
-		
-		// Delete library row, which deletes from tags, syncDeleteLog, syncedSettings, and groups
-		// tables via cascade. If any of those gain caching, they should be deleted separately.
-		sql = "DELETE FROM libraries WHERE libraryID=?";
-		yield Zotero.DB.queryAsync(sql, this.libraryID)
-		
-		Zotero.Groups.unregister(this.id);
-		Zotero.Notifier.queue('delete', 'group', this.id, notifierData);
-	}.bind(this));
 	
-	yield Zotero.purgeDataObjects();
+	// Delete items
+	var types = ['item', 'collection', 'search'];
+	for (let type of types) {
+		let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(type);
+		let sql = "SELECT " + objectsClass.idColumn + " FROM " + objectsClass.table
+			+ " WHERE libraryID=?";
+		ids = yield Zotero.DB.columnQueryAsync(sql, this.libraryID);
+		for (let i = 0; i < ids.length; i++) {
+			let id = ids[i];
+			let obj = yield objectsClass.getAsync(id, { noCache: true });
+			// Descendent object may have already been deleted
+			if (!obj) {
+				continue;
+			}
+			yield obj.erase({
+				skipNotifier: true
+			});
+		}
+	}
+	
+	// Delete library row, which deletes from tags, syncDeleteLog, syncedSettings, and groups
+	// tables via cascade. If any of those gain caching, they should be deleted separately.
+	var sql = "DELETE FROM libraries WHERE libraryID=?";
+	yield Zotero.DB.queryAsync(sql, this.libraryID)
+	
+	Zotero.DB.addCurrentCallback('commit', function () {
+		Zotero.Groups.unregister(this.id);
+		//yield Zotero.purgeDataObjects();
+	}.bind(this))
+	Zotero.Notifier.queue('delete', 'group', this.id);
 });
 
 
-Zotero.Group.prototype.serialize = function() {
+Zotero.Group.prototype.fromJSON = function (json, userID) {
-	var obj = {
+	this._requireLoad();
-		primary: {
+	
-			groupID: this.id,
+	this.name = json.name;
-			libraryID: this.libraryID
+	this.description = json.description;
-		},
+	
-		fields: {
+	var editable = false;
-			name: this.name,
+	var filesEditable = false;
-			description: this.description,
+	if (userID) {
-			editable: this.editable,
+		// If user is owner or admin, make library editable, and make files editable unless they're
-			filesEditable: this.filesEditable
+		// disabled altogether
+		if (json.owner == userID || json.admins.indexOf(userID) != -1) {
+			editable = true;
+			if (json.fileEditing != 'none') {
+				filesEditable = true;
+			}
 		}
-	};
+		// If user is member, make library and files editable if they're editable by all members
-	return obj;
+		else if (json.members.indexOf(userID) != -1) {
+			if (json.libraryEditing == 'members') {
+				editable = true;
+				if (json.fileEditing == 'members') {
+					filesEditable = true;
+				}
+			}
+		}
+	}
+	this.editable = editable;
+	this.filesEditable = filesEditable;
 }
 
 
 Zotero.Group.prototype._requireLoad = function () {
-	if (!this._loaded && Zotero.Groups.exists(this.id)) {
+	if (!this._loaded && Zotero.Groups.exists(this._id)) {
 		throw new Error("Group has not been loaded");
 	}
 }

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

@@ -70,14 +70,25 @@ Zotero.Libraries = new function () {
 	}
 	
 	
+	/**
+	 * @return {Integer[]} - All library IDs
+	 */
 	this.getAll = function () {
 		return [for (x of Object.keys(_libraryData)) parseInt(x)]
 	}
 	
 	
-	this.add = Zotero.Promise.coroutine(function* (type) {
+	/**
+	 * @param {String} type - Library type
+	 * @param {Object} [options] - Library properties to set
+	 * @param {Boolean} [options.editable]
+	 * @param {Boolean} [options.filesEditable]
+	 */
+	this.add = Zotero.Promise.coroutine(function* (type, options) {
 		Zotero.DB.requireTransaction();
 		
+		options = options || {};
+		
 		switch (type) {
 			case 'group':
 				break;
@@ -88,8 +99,18 @@ Zotero.Libraries = new function () {
 		
 		var libraryID = yield Zotero.ID.get('libraries');
 		
-		var sql = "INSERT INTO libraries (libraryID, libraryType) VALUES (?, ?)";
+		var sql = "INSERT INTO libraries (libraryID, libraryType";
-		yield Zotero.DB.queryAsync(sql, [libraryID, type]);
+		var params = [libraryID, type];
+		if (options.editable) {
+			sql += ", editable";
+			params.push(options.editable ? 1 : 0);
+			if (options.filesEditable) {
+				sql += ", filesEditable";
+				params.push(options.filesEditable ? 1 : 0);
+			}
+		}
+		sql += ") VALUES (" + params.map(p => "?").join(", ") + ")";
+		yield Zotero.DB.queryAsync(sql, params);
 		
 		// Re-fetch from DB to get auto-filled defaults
 		var sql = "SELECT * FROM libraries WHERE libraryID=?";
@@ -159,48 +180,48 @@ Zotero.Libraries = new function () {
 	/**
 	 * @param {Integer} libraryID
 	 * @param {Date} lastSyncTime
+	 * @return {Promise}
      */
 	this.setLastSyncTime = function (libraryID, lastSyncTime) {
 		var lastSyncTime = Math.round(lastSyncTime.getTime() / 1000);
-		return Zotero.DB.valueQueryAsync(
+		_libraryData[libraryID].lastSyncTime = lastSyncTime;
+		return Zotero.DB.queryAsync(
 			"UPDATE libraries SET lastsync=? WHERE libraryID=?", [lastSyncTime, libraryID]
 		);
 	};
 	
-	
 	this.isEditable = function (libraryID) {
-		var type = this.getType(libraryID);
+		return _libraryData[libraryID].editable;
-		switch (type) {
-			case 'user':
-			case 'publications':
-				return true;
-			
-			case 'group':
-				var groupID = Zotero.Groups.getGroupIDFromLibraryID(libraryID);
-				var group = Zotero.Groups.get(groupID);
-				return group.editable;
-			
-			default:
-				throw new Error("Unsupported library type '" + type + "' in Zotero.Libraries.getName()");
-		}
 	}
 	
+	/**
+	 * @return {Promise}
+	 */
+	this.setEditable = function (libraryID, editable) {
+		if (editable == this.isEditable(libraryID)) {
+			return Zotero.Promise.resolve();
+		}
+		_libraryData[libraryID].editable = !!editable;
+		return Zotero.DB.queryAsync(
+			"UPDATE libraries SET editable=? WHERE libraryID=?", [editable ? 1 : 0, libraryID]
+		);
+	}
 	
 	this.isFilesEditable = function (libraryID) {
-		var type = this.getType(libraryID);
+		return _libraryData[libraryID].filesEditable;
-		switch (type) {
+	}
-			case 'user':
+	
-			case 'publications':
+	/**
-				return true;
+	 * @return {Promise}
-			
+	 */
-			case 'group':
+	this.setFilesEditable = function (libraryID, filesEditable) {
-				var groupID = Zotero.Groups.getGroupIDFromLibraryID(libraryID);
+		if (filesEditable == this.isFilesEditable(libraryID)) {
-				var group = Zotero.Groups.get(groupID);
+			return Zotero.Promise.resolve();
-				return group.filesEditable;
-			
-			default:
-				throw new Error("Unsupported library type '" + type + "' in Zotero.Libraries.getName()");
 		}
+		_libraryData[libraryID].filesEditable = !!filesEditable;
+		return Zotero.DB.queryAsync(
+			"UPDATE libraries SET filesEditable=? WHERE libraryID=?", [filesEditable ? 1 : 0, libraryID]
+		);
 	}
 	
 	this.isGroupLibrary = function (libraryID) {
@@ -215,6 +236,8 @@ Zotero.Libraries = new function () {
 		return {
 			id: row.libraryID,
 			type: row.libraryType,
+			editable: row.editable,
+			filesEditable: row.filesEditable,
 			version: row.version,
 			lastSyncTime: row.lastsync != 0 ? new Date(row.lastsync * 1000) : false
 		};

chrome/content/zotero/xpcom/schema.js

@@ -1465,8 +1465,11 @@ Zotero.Schema = new function(){
 			});
 			yield _updateDBVersion('compatibility', _maxCompatibility);
 			
-			yield Zotero.DB.queryAsync("INSERT INTO libraries (libraryID, libraryType) VALUES (?, 'user')", userLibraryID);
+			var sql = "INSERT INTO libraries (libraryID, libraryType, editable, filesEditable) "
-			yield Zotero.DB.queryAsync("INSERT INTO libraries (libraryID, libraryType) VALUES (2, 'publications')");
+				+ "VALUES "
+				+ "(?, 'user', 1, 1), "
+				+ "(2, 'publications', 1, 1)"
+			yield Zotero.DB.queryAsync(sql, userLibraryID);
 			
 			if (!Zotero.Schema.skipDefaultData) {
 				// Quick Start Guide web page item
@@ -1935,14 +1938,15 @@ Zotero.Schema = new function(){
 			if (i == 80) {
 				yield _updateDBVersion('compatibility', 1);
 				
-				yield Zotero.DB.queryAsync("INSERT INTO libraries VALUES (1, 'user')");
+				yield Zotero.DB.queryAsync("ALTER TABLE libraries RENAME TO librariesOld");
-				yield Zotero.DB.queryAsync("INSERT INTO libraries VALUES (2, 'publications')");
+				yield Zotero.DB.queryAsync("CREATE TABLE libraries (\n    libraryID INTEGER PRIMARY KEY,\n    libraryType TEXT NOT NULL,\n    editable INT NOT NULL,\n    filesEditable INT NOT NULL,\n    version INT NOT NULL DEFAULT 0,\n    lastsync INT NOT NULL DEFAULT 0\n)");
+				yield Zotero.DB.queryAsync("INSERT INTO libraries (libraryID, libraryType, editable, filesEditable) VALUES (1, 'user', 1, 1)");
+				yield Zotero.DB.queryAsync("INSERT INTO libraries (libraryID, libraryType, editable, filesEditable) VALUES (2, 'publications', 1, 1)");
+				yield Zotero.DB.queryAsync("INSERT INTO libraries SELECT libraryID, libraryType, editable, filesEditable, 0, 0 FROM librariesOld JOIN groups USING (libraryID)");
 				
 				yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO syncObjectTypes VALUES (7, 'setting')");
 				yield Zotero.DB.queryAsync("DELETE FROM version WHERE schema IN ('userdata2', 'userdata3')");
 				
-				yield Zotero.DB.queryAsync("ALTER TABLE libraries ADD COLUMN version INT NOT NULL DEFAULT 0");
-				yield Zotero.DB.queryAsync("ALTER TABLE libraries ADD COLUMN lastsync INT NOT NULL DEFAULT 0");
 				yield Zotero.DB.queryAsync("CREATE TABLE syncCache (\n    libraryID INT NOT NULL,\n    key TEXT NOT NULL,\n    syncObjectTypeID INT NOT NULL,\n    version INT NOT NULL,\n    data TEXT,\n    PRIMARY KEY (libraryID, key, syncObjectTypeID, version),\n    FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE,\n    FOREIGN KEY (syncObjectTypeID) REFERENCES syncObjectTypes(syncObjectTypeID)\n)");
 				
 				yield Zotero.DB.queryAsync("DROP TABLE translatorCache");
@@ -2170,8 +2174,8 @@ Zotero.Schema = new function(){
 				yield _migrateUserData_80_relations();
 				
 				yield Zotero.DB.queryAsync("ALTER TABLE groups RENAME TO groupsOld");
-				yield Zotero.DB.queryAsync("CREATE TABLE groups (\n    groupID INTEGER PRIMARY KEY,\n    libraryID INT NOT NULL UNIQUE,\n    name TEXT NOT NULL,\n    description TEXT NOT NULL,\n    editable INT NOT NULL,\n    filesEditable INT NOT NULL,\n    version INT NOT NULL,\n    FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)");
+				yield Zotero.DB.queryAsync("CREATE TABLE groups (\n    groupID INTEGER PRIMARY KEY,\n    libraryID INT NOT NULL UNIQUE,\n    name TEXT NOT NULL,\n    description TEXT NOT NULL,\n    version INT NOT NULL,\n    FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE\n)");
-				yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO groups SELECT groupID, libraryID, name, description, editable, filesEditable, 0 FROM groupsOld");
+				yield Zotero.DB.queryAsync("INSERT OR IGNORE INTO groups SELECT groupID, libraryID, name, description, 0 FROM groupsOld");
 				
 				yield Zotero.DB.queryAsync("ALTER TABLE groupItems RENAME TO groupItemsOld");
 				yield Zotero.DB.queryAsync("CREATE TABLE groupItems (\n    itemID INTEGER PRIMARY KEY,\n    createdByUserID INT,\n    lastModifiedByUserID INT,\n    FOREIGN KEY (itemID) REFERENCES items(itemID) ON DELETE CASCADE,\n    FOREIGN KEY (createdByUserID) REFERENCES users(userID) ON DELETE SET NULL,\n    FOREIGN KEY (lastModifiedByUserID) REFERENCES users(userID) ON DELETE SET NULL\n)");
@@ -2253,6 +2257,7 @@ Zotero.Schema = new function(){
 				yield Zotero.DB.queryAsync("DROP TABLE creatorData");
 				yield Zotero.DB.queryAsync("DROP TABLE itemsOld");
 				yield Zotero.DB.queryAsync("DROP TABLE tagsOld");
+				yield Zotero.DB.queryAsync("DROP TABLE librariesOld");
 			}
 		}
 		

chrome/content/zotero/xpcom/storage.js

@@ -1874,6 +1874,7 @@ Zotero.Sync.Storage = new function () {
 				);
 				
 				if (index == 0) {
+					// TODO: transaction
 					group.erase();
 					Zotero.Sync.Server.resetClient();
 					Zotero.Sync.Storage.resetAllSyncStates();

resource/schema/userdata.sql

@@ -247,6 +247,8 @@ CREATE INDEX deletedItems_dateDeleted ON deletedItems(dateDeleted);
 CREATE TABLE libraries (
     libraryID INTEGER PRIMARY KEY,
     libraryType TEXT NOT NULL,
+    editable INT NOT NULL,
+    filesEditable INT NOT NULL,
     version INT NOT NULL DEFAULT 0,
     lastsync INT NOT NULL DEFAULT 0
 );
@@ -261,8 +263,6 @@ CREATE TABLE groups (
     libraryID INT NOT NULL UNIQUE,
     name TEXT NOT NULL,
     description TEXT NOT NULL,
-    editable INT NOT NULL,
-    filesEditable INT NOT NULL,
     version INT NOT NULL,
     FOREIGN KEY (libraryID) REFERENCES libraries(libraryID) ON DELETE CASCADE
 );

test/content/support.js

@@ -229,8 +229,13 @@ function waitForCallback(cb, interval, timeout) {
 /**
  * Get a default group used by all tests that want one, creating one if necessary
  */
-var getGroup = Zotero.lazy(function () {
+var _defaultGroup;
-	return createGroup({
+var getGroup = Zotero.Promise.method(function () {
+	// Cleared in resetDB()
+	if (_defaultGroup) {
+		return _defaultGroup;
+	}
+	return _defaultGroup = createGroup({
 		name: "My Group"
 	});
 });
@@ -239,7 +244,7 @@ var getGroup = Zotero.lazy(function () {
 var createGroup = Zotero.Promise.coroutine(function* (props) {
 	props = props || {};
 	var group = new Zotero.Group;
-	group.id = Zotero.Utilities.rand(10000, 1000000);
+	group.id = props.id || Zotero.Utilities.rand(10000, 1000000);
 	group.name = props.name || "Test " + Zotero.Utilities.randomString();
 	group.description = props.description || "";
 	group.editable = props.editable || true;
@@ -363,6 +368,7 @@ function resetDB() {
 	var db = Zotero.getZoteroDatabase();
 	return Zotero.reinit(function() {
 		db.remove(false);
+		_defaultGroup = null;
 	}).then(function() {
 		return Zotero.Schema.schemaUpdatePromise;
 	});

test/tests/collectionTreeViewTest.js

@@ -331,7 +331,9 @@ describe("Zotero.CollectionTreeView", function() {
 			linked = yield attachment.getLinkedItem(group.libraryID);
 			assert.equal(linked.id, treeRow.ref.id);
 			
-			yield group.erase()
+			yield Zotero.DB.executeTransaction(function* () {
+				return group.erase();
+			})
 		})
 		
 		it("should not copy an item or its attachment to a group twice", function* () {

test/tests/groupTest.js

@@ -0,0 +1,136 @@
+"use strict";
+
+describe("Zotero.Group", function () {
+	describe("#erase()", function () {
+		it("should unregister group", function* () {
+			var group = yield createGroup();
+			var id = group.id;
+			yield Zotero.DB.executeTransaction(function* () {
+				return group.erase()
+			}.bind(this));
+			assert.isFalse(Zotero.Groups.exists(id));
+		})
+	})
+	
+	describe("#fromJSON()", function () {
+		it("should set permissions for owner", function* () {
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'admins',
+				fileEditing: 'admins'
+			}, 1);
+			assert.isTrue(group.editable);
+			assert.isTrue(group.filesEditable);
+			
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'members',
+				fileEditing: 'members'
+			}, 1);
+			assert.isTrue(group.editable);
+			assert.isTrue(group.filesEditable);
+			
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'admins',
+				fileEditing: 'none'
+			}, 1);
+			assert.isTrue(group.editable);
+			assert.isFalse(group.filesEditable);
+		})
+		
+		it("should set permissions for admin", function* () {
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'admins',
+				fileEditing: 'admins',
+				admins: [2]
+			}, 2);
+			assert.isTrue(group.editable);
+			assert.isTrue(group.filesEditable);
+			
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'members',
+				fileEditing: 'members',
+				admins: [2]
+			}, 2);
+			assert.isTrue(group.editable);
+			assert.isTrue(group.filesEditable);
+			
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'admins',
+				fileEditing: 'none',
+				admins: [2]
+			}, 2);
+			assert.isTrue(group.editable);
+			assert.isFalse(group.filesEditable);
+		})
+		
+		it("should set permissions for member", function* () {
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'members',
+				fileEditing: 'members',
+				admins: [2],
+				members: [3]
+			}, 3);
+			assert.isTrue(group.editable);
+			assert.isTrue(group.filesEditable);
+			
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'admins',
+				fileEditing: 'admins',
+				admins: [2],
+				members: [3]
+			}, 3);
+			assert.isFalse(group.editable);
+			assert.isFalse(group.filesEditable);
+			
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'admins',
+				fileEditing: 'members', // Shouldn't be possible
+				admins: [2],
+				members: [3]
+			}, 3);
+			assert.isFalse(group.editable);
+			assert.isFalse(group.filesEditable);
+			
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'members',
+				fileEditing: 'none',
+				admins: [2],
+				members: [3]
+			}, 3);
+			assert.isTrue(group.editable);
+			assert.isFalse(group.filesEditable);
+		})
+		
+		it("should set permissions for non-member", function* () {
+			var group = new Zotero.Group;
+			group.fromJSON({
+				owner: 1,
+				libraryEditing: 'members',
+				fileEditing: 'members',
+				admins: [2],
+				members: [3]
+			});
+			assert.isFalse(group.editable);
+			assert.isFalse(group.filesEditable);
+		})
+	})
+})

test/tests/groupsTest.js

@@ -7,7 +7,9 @@ describe("Zotero.Groups", function () {
 			}
 			finally {
 				if (group) {
-					yield group.erase();
+					yield Zotero.DB.executeTransaction(function* () {
+						return group.erase();
+					})
 				}
 			}
 		})