From e6ede4b36f5cf27affada98246c770252d9b411b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adomas=20Ven=C4=8Dkauskas?= <adomas.ven@gmail.com>
Date: Wed, 13 Jan 2016 13:13:29 +0000
Subject: [PATCH] Various feeds changes

And move Z.Attachments.cleanAttachmentURI() to Z.Utilities.cleanURL()
---
 chrome/content/zotero/feedSettings.js         |  44 ++---
 chrome/content/zotero/feedSettings.xul        |   6 +-
 chrome/content/zotero/xpcom/attachments.js    |  31 +---
 .../zotero/xpcom/collectionTreeView.js        |  19 +-
 chrome/content/zotero/xpcom/data/feed.js      | 117 +++++++-----
 chrome/content/zotero/xpcom/data/feedItem.js  |  37 +++-
 chrome/content/zotero/xpcom/data/feedItems.js |  24 ++-
 chrome/content/zotero/xpcom/data/feeds.js     |  24 +--
 chrome/content/zotero/xpcom/data/libraries.js |   2 +-
 chrome/content/zotero/xpcom/feedReader.js     |  45 ++---
 chrome/content/zotero/xpcom/itemTreeView.js   |  43 +++++
 chrome/content/zotero/xpcom/uri.js            |   2 +-
 chrome/content/zotero/xpcom/utilities.js      |  32 ++++
 chrome/content/zotero/zoteroPane.js           |   9 +-
 chrome/locale/en-US/zotero/zotero.dtd         |   4 +-
 chrome/locale/en-US/zotero/zotero.properties  |   2 +-
 test/content/support.js                       |  18 ++
 test/tests/collectionTreeViewTest.js          |   7 +
 test/tests/data/feedModified.rss              |  42 +++++
 test/tests/feedItemTest.js                    |  49 ++++-
 test/tests/feedItemsTest.js                   |  67 ++++++-
 test/tests/feedReaderTest.js                  |  12 +-
 test/tests/feedTest.js                        | 172 ++++++++++++++++--
 test/tests/feedsTest.js                       | 101 ++++++++--
 24 files changed, 698 insertions(+), 211 deletions(-)
 create mode 100644 test/tests/data/feedModified.rss

diff --git a/chrome/content/zotero/feedSettings.js b/chrome/content/zotero/feedSettings.js
index 000c81433..56555a1d6 100644
--- a/chrome/content/zotero/feedSettings.js
+++ b/chrome/content/zotero/feedSettings.js
@@ -36,33 +36,15 @@ var Zotero_Feed_Settings = new function() {
 		urlTainted = false;
 	
 	let cleanURL = function(url) {
-		url = url.trim();
-		if (!url) return;
-		
-		let ios = Components.classes["@mozilla.org/network/io-service;1"]
-			.getService(Components.interfaces.nsIIOService);
-		
-		let cleanUrl;
-		try {
-			let uri = ios.newURI(url, null, null);
-			if (uri.scheme != 'http' && uri.scheme != 'https') {
+		let cleanUrl = Zotero.Utilities.cleanURL(url, true);
+
+		if (cleanUrl) {
+			if (/^https?:\/\/[^\/\s]+\/\S/.test(cleanUrl)) {
+				return cleanUrl;
+			} else {
 				Zotero.debug(uri.scheme + " is not a supported protocol for feeds.");
 			}
-			
-			cleanUrl = uri.spec;
-		} catch (e) {
-			if (e.result == Components.results.NS_ERROR_MALFORMED_URI) {
-				// Assume it's a URL missing "http://" part
-				try {
-					cleanUrl = ios.newURI('http://' + url, null, null).spec;
-				} catch (e) {}
-			}
-			throw e;
 		}
-		
-		if (!cleanUrl) return;
-		
-		if (/^https?:\/\/[^\/\s]+\/\S/.test(cleanUrl)) return cleanUrl;
 	};
 	
 	this.init = function() {
@@ -93,9 +75,9 @@ var Zotero_Feed_Settings = new function() {
 		}
 		document.getElementById('feed-ttl').value = ttl;
 		
-		let cleanAfter = data.cleanAfter;
-		if (cleanAfter === undefined) cleanAfter = 2;
-		document.getElementById('feed-cleanAfter').value = cleanAfter;
+		let cleanupAfter = data.cleanupAfter;
+		if (cleanupAfter === undefined) cleanupAfter = 2;
+		document.getElementById('feed-cleanupAfter').value = cleanupAfter;
 		
 		if (data.url && !data.urlIsValid) {
 			this.validateUrl();
@@ -114,7 +96,7 @@ var Zotero_Feed_Settings = new function() {
 		urlIsValid = false;
 		document.getElementById('feed-title').disabled = true;
 		document.getElementById('feed-ttl').disabled = true;
-		document.getElementById('feed-cleanAfter').disabled = true;
+		document.getElementById('feed-cleanupAfter').disabled = true;
 		document.documentElement.getButton('accept').disabled = true;
 	};
 	
@@ -132,6 +114,8 @@ var Zotero_Feed_Settings = new function() {
 			let fr = feedReader = new Zotero.FeedReader(url);
 			yield fr.process();
 			let feed = fr.feedProperties;
+			// Prevent progress if textbox changes triggered another call to
+			// validateUrl / invalidateUrl (old session)
 			if (feedReader !== fr || urlTainted) return;
 			
 			let title = document.getElementById('feed-title');
@@ -149,7 +133,7 @@ var Zotero_Feed_Settings = new function() {
 			urlIsValid = true;
 			title.disabled = false;
 			ttl.disabled = false;
-			document.getElementById('feed-cleanAfter').disabled = false;
+			document.getElementById('feed-cleanupAfter').disabled = false;
 			document.documentElement.getButton('accept').disabled = false;
 		}
 		catch (e) {
@@ -164,7 +148,7 @@ var Zotero_Feed_Settings = new function() {
 		data.url = document.getElementById('feed-url').value;
 		data.title = document.getElementById('feed-title').value;
 		data.ttl = document.getElementById('feed-ttl').value * 60;
-		data.cleanAfter = document.getElementById('feed-cleanAfter').value * 1;
+		data.cleanupAfter = document.getElementById('feed-cleanupAfter').value * 1;
 		return true;
 	};
 	
diff --git a/chrome/content/zotero/feedSettings.xul b/chrome/content/zotero/feedSettings.xul
index 45ebf8014..2550c9ca7 100644
--- a/chrome/content/zotero/feedSettings.xul
+++ b/chrome/content/zotero/feedSettings.xul
@@ -40,9 +40,9 @@
 					<label value="&zotero.feedSettings.refresh.label2;" control="feed-ttl"/>
 				</hbox>
 				<hbox align="center">
-					<label value="&zotero.feedSettings.cleanAfter.label1;" control="feed-cleanAfter"/>
-					<textbox id="feed-cleanAfter" type="number" min="0" increment="1" size="2"/>
-					<label value="&zotero.feedSettings.cleanAfter.label2;" control="feed-cleanAfter"/>
+					<label value="&zotero.feedSettings.cleanupAfter.label1;" control="feed-cleanupAfter"/>
+					<textbox id="feed-cleanupAfter" type="number" min="0" increment="1" size="2"/>
+					<label value="&zotero.feedSettings.cleanupAfter.label2;" control="feed-cleanupAfter"/>
 				</hbox>
 			</vbox>
 		</vbox>
diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js
index 24ddfedbb..2f2ec45da 100644
--- a/chrome/content/zotero/xpcom/attachments.js
+++ b/chrome/content/zotero/xpcom/attachments.js
@@ -683,32 +683,13 @@ Zotero.Attachments = new function(){
 		
 		return attachmentItem;
 	});
-	
-	
+
+
+	/**
+	 * @deprecated Use Zotero.Utilities.cleanURL instead
+	 */
 	this.cleanAttachmentURI = function (uri, tryHttp) {
-		uri = uri.trim();
-		if (!uri) return false;
-		
-		var ios = Components.classes["@mozilla.org/network/io-service;1"]
-			.getService(Components.interfaces.nsIIOService);
-		try {
-			return ios.newURI(uri, null, null).spec // Valid URI if succeeds
-		} catch (e) {
-			if (e instanceof Components.Exception
-				&& e.result == Components.results.NS_ERROR_MALFORMED_URI
-			) {
-				if (tryHttp && /\w\.\w/.test(uri)) {
-					// Assume it's a URL missing "http://" part
-					try {
-						return ios.newURI('http://' + uri, null, null).spec;
-					} catch (e) {}
-				}
-				
-				Zotero.debug('cleanAttachmentURI: Invalid URI: ' + uri, 2);
-				return false;
-			}
-			throw e;
-		}
+		return Zotero.Utilities.cleanURL(uri, tryHttp);
 	}
 	
 	
diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js
index ce9d06d48..1136b191f 100644
--- a/chrome/content/zotero/xpcom/collectionTreeView.js
+++ b/chrome/content/zotero/xpcom/collectionTreeView.js
@@ -490,8 +490,12 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
 				
 				break;
 			
-			case 'feed':
 			case 'group':
+				yield this.reload();
+				yield this.selectByID(currentTreeRow.id);
+				break;
+				
+			case 'feed':
 				yield this.reload();
 				yield this.selectByID("L" + id);
 				break;
@@ -790,9 +794,6 @@ Zotero.CollectionTreeView.prototype.isContainerEmpty = function(row)
 				&& this._unfiledLibraries.indexOf(libraryID) == -1
 				&& this.hideSources.indexOf('trash') != -1;
 	}
-	if (treeRow.isFeed()) {
-		return false; // If it's shown, it has something
-	}
 	if (treeRow.isCollection()) {
 		return !treeRow.ref.hasChildCollections();
 	}
@@ -1107,6 +1108,7 @@ Zotero.CollectionTreeView.prototype.deleteSelection = Zotero.Promise.coroutine(f
 			yield treeRow.ref.eraseTx({
 				deleteItems: true
 			});
+		}
 		if (treeRow.isCollection() || treeRow.isFeed()) {
 			yield treeRow.ref.erase(deleteItems);
 		}
@@ -1139,7 +1141,7 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi
 	var isCollection = treeRow.isCollection();
 	var libraryID = treeRow.ref.libraryID;
 	
-	if (treeRow.isPublications()) {
+	if (treeRow.isPublications() || treeRow.isFeed()) {
 		return false;
 	}
 	
@@ -1335,7 +1337,7 @@ Zotero.CollectionTreeView.prototype._rememberOpenStates = Zotero.Promise.corouti
 		var open = this.isContainerOpen(i);
 		
 		// Collections and feeds default to closed
-		if (!open && treeRow.isCollection() && treeRow.isFeed()) {
+		if (!open && treeRow.isCollection() || treeRow.isFeed()) {
 			delete state[treeRow.id];
 			continue;
 		}
@@ -1434,6 +1436,11 @@ Zotero.CollectionTreeView.prototype.canDropCheck = function (row, orient, dataTr
 			return false;
 		}
 		
+		if (treeRow.isFeed()) {
+			Zotero.debug("Cannot drop into feeds");
+			return false;
+		}
+		
 		if (dataType == 'zotero/item') {
 			var ids = data;
 			var items = Zotero.Items.get(ids);
diff --git a/chrome/content/zotero/xpcom/data/feed.js b/chrome/content/zotero/xpcom/data/feed.js
index b3dfa2055..dde99ff80 100644
--- a/chrome/content/zotero/xpcom/data/feed.js
+++ b/chrome/content/zotero/xpcom/data/feed.js
@@ -23,6 +23,19 @@
     ***** END LICENSE BLOCK *****
 */
 
+/**
+ * Zotero.Feed, extends Zotero.Library
+ * 
+ * Custom parameters:
+ * - name - name of the feed displayed in the collection tree
+ * - url
+ * - cleanupAfter - number of days after which read items should be removed
+ * - refreshInterval - in terms of hours
+ * 
+ * @param params
+ * @returns Zotero.Feed
+ * @constructor
+ */
 Zotero.Feed = function(params = {}) {
 	params.libraryType = 'feed';
 	Zotero.Feed._super.call(this, params);
@@ -30,8 +43,8 @@ Zotero.Feed = function(params = {}) {
 	this._feedCleanupAfter = null;
 	this._feedRefreshInterval = null;
 
-	// Feeds are editable by the user. Remove the setter
-	this.editable = true;
+	// Feeds are not editable by the user. Remove the setter
+	this.editable = false;
 	Zotero.defineProperty(this, 'editable', {
 		get: function() this._get('_libraryEditable')
 	});
@@ -42,8 +55,8 @@ Zotero.Feed = function(params = {}) {
 		get: function() this._get('_libraryFilesEditable')
 	});
 	
-	Zotero.Utilities.assignProps(this, params, ['name', 'url', 'refreshInterval',
-		'cleanupAfter']);
+	Zotero.Utilities.assignProps(this, params, 
+		['name', 'url', 'refreshInterval', 'cleanupAfter']);
 	
 	// Return a proxy so that we can disable the object once it's deleted
 	return new Proxy(this, {
@@ -63,6 +76,8 @@ Zotero.Feed._colToProp = function(c) {
 	return "_feed" + Zotero.Utilities.capitalize(c);
 }
 
+Zotero.extendClass(Zotero.Library, Zotero.Feed);
+
 Zotero.defineProperty(Zotero.Feed, '_unreadCountSQL', {
 	value: "(SELECT COUNT(*) FROM items I JOIN feedItems FeI USING (itemID)"
 			+ " WHERE I.libraryID=F.libraryID AND FeI.readTime IS NULL) AS _feedUnreadCount"
@@ -86,8 +101,6 @@ Zotero.defineProperty(Zotero.Feed, '_rowSQL', {
 		+ " FROM feeds F JOIN libraries L USING (libraryID)"
 });
 
-Zotero.extendClass(Zotero.Library, Zotero.Feed);
-
 Zotero.defineProperty(Zotero.Feed.prototype, '_objectType', {
 	value: 'feed'
 });
@@ -103,12 +116,7 @@ Zotero.defineProperty(Zotero.Feed.prototype, 'unreadCount', {
 	get: function() this._feedUnreadCount
 });
 Zotero.defineProperty(Zotero.Feed.prototype, 'updating', {
-	get: function() this._updating,
-	set: function(v) {
-		if (!v == !this._updating) return; // Unchanged
-		this._updating = !!v;
-		Zotero.Notifier.trigger('statusChanged', 'feed', this.id);
-	}
+	get: function() !!this._updating,
 });
 
 (function() {
@@ -291,10 +299,10 @@ Zotero.Feed.prototype._finalizeSave = Zotero.Promise.coroutine(function* (env) {
 
 Zotero.Feed.prototype.getExpiredFeedItemIDs = Zotero.Promise.coroutine(function* () {
 	let sql = "SELECT itemID AS id FROM feedItems "
-		+ "LEFT JOIN libraryItems LI USING (itemID)"
-		+ "WHERE LI.libraryID=?"
-		+ "WHERE readTime IS NOT NULL "
-		+ "AND (julianday(readTime, 'utc') + (?) - julianday('now', 'utc')) > 0";
+		+ "LEFT JOIN items I USING (itemID) "
+		+ "WHERE I.libraryID=? "
+		+ "AND readTime IS NOT NULL "
+		+ "AND julianday('now', 'utc') - (julianday(readTime, 'utc') + ?) > 0";
 	let expiredIDs = yield Zotero.DB.queryAsync(sql, [this.id, {int: this.cleanupAfter}]);
 	return expiredIDs.map(row => row.id);
 });
@@ -307,7 +315,7 @@ Zotero.Feed.prototype.clearExpiredItems = Zotero.Promise.coroutine(function* ()
 			Zotero.debug("Cleaning up read feed items...");
 			if (expiredItems.length) {
 				Zotero.debug(expiredItems.join(', '));
-				yield Zotero.FeedItems.eraseTx(expiredItems);
+				yield Zotero.FeedItems.forceErase(expiredItems);
 			} else {
 				Zotero.debug("No expired feed items");
 			}
@@ -319,26 +327,36 @@ Zotero.Feed.prototype.clearExpiredItems = Zotero.Promise.coroutine(function* ()
 });
 
 Zotero.Feed.prototype._updateFeed = Zotero.Promise.coroutine(function* () {
-	this.updating = true;
-	this.lastCheckError = null;
+	var toAdd = [];
+	if (this._updating) {
+		return this._updating;
+	}
+	let deferred = Zotero.Promise.defer();
+	this._updating = deferred.promise;
+	Zotero.Notifier.trigger('statusChanged', 'feed', this.id);
+	this._set('_feedLastCheckError', null);
 	
 	yield this.clearExpiredItems();
 	try {
 		let fr = new Zotero.FeedReader(this.url);
 		yield fr.process();
 		let itemIterator = new fr.ItemIterator();
-		let item, toAdd = [], processedGUIDs = [];
+		let item, processedGUIDs = [];
 		while (item = yield itemIterator.next().value) {
+			// NOTE: Might cause issues with feeds that set pubDate for publication date of the item
+			// rather than the date the item was added to the feed.
 			if (item.dateModified && this.lastUpdate
 				&& item.dateModified < this.lastUpdate
 			) {
-				Zotero.debug("Item modification date before last update date (" + this._feedLastCheck + ")");
+				Zotero.debug("Item modification date before last update date (" + this.lastCheck + ")");
 				Zotero.debug(item);
 				// We can stop now
 				fr.terminate();
 				break;
 			}
 			
+			// Append id at the end to prevent same item collisions from different feeds
+			item.guid += ":" + this.id;
 			if (processedGUIDs.indexOf(item.guid) != -1) {
 				Zotero.debug("Feed item " + item.guid + " already processed from feed.");
 				continue;
@@ -356,8 +374,8 @@ Zotero.Feed.prototype._updateFeed = Zotero.Promise.coroutine(function* () {
 			} else {
 				Zotero.debug("Feed item " + item.guid + " already in library.");
 				
-				if (item.dateModified && feedItem.dateModified
-					&& feedItem.dateModified == item.dateModified
+				if (!item.dateModified || 
+					(feedItem.dateModified && feedItem.dateModified == item.dateModified)
 				) {
 					Zotero.debug("Modification date has not changed. Skipping update.");
 					continue;
@@ -370,39 +388,46 @@ Zotero.Feed.prototype._updateFeed = Zotero.Promise.coroutine(function* () {
 			
 			// Delete invalid data
 			delete item.guid;
+			delete item.dateAdded;
 			
 			feedItem.fromJSON(item);
 			toAdd.push(feedItem);
 		}
-		
-		// Save in reverse order
-		let savePromises = new Array(toAdd.length);
-		for (let i=toAdd.length-1; i>=0; i--) {
-			// Saving currently has to happen sequentially so as not to violate the
-			// unique constraints in dataValues (FIXME)
-			yield toAdd[i].save({skipEditCheck: true});
-		}
-		
-		this.lastUpdate = Zotero.Date.dateToSQL(new Date(), true);
 	}
 	catch (e) {
 		if (e.message) {
 			Zotero.debug("Error processing feed from " + this.url);
 			Zotero.debug(e);
 		}
-		this.lastCheckError = e.message || 'Error processing feed';
+		this._set('_feedLastCheckError', e.message || 'Error processing feed');
 	}
-	this.lastCheck = Zotero.Date.dateToSQL(new Date(), true);
-	yield this.saveTx({skipEditCheck: true});
-	this.updating = false;
+	if (toAdd.length) {
+		yield Zotero.DB.executeTransaction(function* () {
+			// Save in reverse order
+			for (let i=toAdd.length-1; i>=0; i--) {
+				// Saving currently has to happen sequentially so as not to violate the
+				// unique constraints in itemDataValues (FIXME)
+				yield toAdd[i].save({skipEditCheck: true});
+			}
+		});
+		this._set('_feedLastUpdate', Zotero.Date.dateToSQL(new Date(), true));
+	}
+	this._set('_feedLastCheck', Zotero.Date.dateToSQL(new Date(), true));
+	yield this.saveTx();
+	yield this.updateUnreadCount();
+	deferred.resolve();
+	this._updating = false;
+	Zotero.Notifier.trigger('statusChanged', 'feed', this.id);
 });
 
-Zotero.Feed.prototype.updateFeed = function() {
-	return this._updateFeed()
-	.finally(function() {
+Zotero.Feed.prototype.updateFeed = Zotero.Promise.coroutine(function* () {
+	try {
+		let result = yield this._updateFeed();
+		return result;
+	} finally {
 		Zotero.Feeds.scheduleNextFeedCheck();
-	});
-}
+	}
+});
 
 Zotero.Feed.prototype._finalizeErase = Zotero.Promise.coroutine(function* (){
 	let notifierData = {};
@@ -411,14 +436,14 @@ Zotero.Feed.prototype._finalizeErase = Zotero.Promise.coroutine(function* (){
 	};
 	Zotero.Notifier.trigger('delete', 'feed', this.id, notifierData);
 	Zotero.Feeds.unregister(this.libraryID);
-	return Zotero.Feed._super.prototype._finalizeErase.call(this);
+	return Zotero.Feed._super.prototype._finalizeErase.apply(this, arguments);
 });
 
-Zotero.Feed.prototype.erase = Zotero.Promise.coroutine(function* (deleteItems) {
+Zotero.Feed.prototype.erase = Zotero.Promise.coroutine(function* (options = {}) {
 	let childItemIDs = yield Zotero.FeedItems.getAll(this.id, false, false, true);
-	yield Zotero.FeedItems.erase(childItemIDs);
+	yield Zotero.FeedItems.forceErase(childItemIDs);
 	
-	yield Zotero.Feed._super.prototype.erase.call(this);
+	yield Zotero.Feed._super.prototype.erase.call(this, options);
 });
 
 Zotero.Feed.prototype.updateUnreadCount = Zotero.Promise.coroutine(function* () {
diff --git a/chrome/content/zotero/xpcom/data/feedItem.js b/chrome/content/zotero/xpcom/data/feedItem.js
index d68f50aa7..7cdc131ae 100644
--- a/chrome/content/zotero/xpcom/data/feedItem.js
+++ b/chrome/content/zotero/xpcom/data/feedItem.js
@@ -89,6 +89,32 @@ Zotero.FeedItem.prototype.setField = function(field, value) {
 	return Zotero.FeedItem._super.prototype.setField.apply(this, arguments);
 }
 
+Zotero.FeedItem.prototype.fromJSON = function(json) {
+	// Handle weird formats in feedItems
+	let dateFields = ['accessDate', 'dateAdded', 'dateModified'];
+	for (let dateField of dateFields) {
+		let val = json[dateField];
+		if (val) {
+			let d = new Date(val);
+			if (isNaN(d.getTime())) {
+				d = Zotero.Date.sqlToDate(val, true);
+			}
+			if (!d || isNaN(d.getTime())) {
+				d = Zotero.Date.strToDate(val);
+				d = new Date(d.year, d.month, d.day);
+				Zotero.debug(dateField + " " + JSON.stringify(d), 1);
+			}
+			if (!d) {
+				Zotero.logError("Discarding invalid " + field + " '" + val
+					+ "' for item " + this.libraryKey);
+				delete json[dateField];
+				continue;
+			}
+			json[dateField] = d.toISOString();
+		}
+	}
+	Zotero.FeedItem._super.prototype.fromJSON.apply(this, arguments);
+}
 
 Zotero.FeedItem.prototype._initSave = Zotero.Promise.coroutine(function* (env) {
 	if (!this.guid) {
@@ -124,6 +150,11 @@ Zotero.FeedItem.prototype.forceSaveTx = function(options) {
 	return this.saveTx(newOptions);
 }
 
+Zotero.FeedItem.prototype.save = function(options = {}) {
+	options.skipDateModifiedUpdate = true;
+	return Zotero.FeedItem._super.prototype.save.apply(this, arguments)
+}
+
 Zotero.FeedItem.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 	yield Zotero.FeedItem._super.prototype._saveData.apply(this, arguments);
 	
@@ -138,12 +169,12 @@ Zotero.FeedItem.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 Zotero.FeedItem.prototype.toggleRead = Zotero.Promise.coroutine(function* (state) {
 	state = state !== undefined ? !!state : !this.isRead;
 	let changed = this.isRead != state;
-	this.isRead = state;
 	if (changed) {
-		yield this.save({skipEditCheck: true, skipDateModifiedUpdate: true});
+		this.isRead = state;
+		yield this.saveTx({skipEditCheck: true, skipDateModifiedUpdate: true});
 		
 		let feed = Zotero.Feeds.get(this.libraryID);
-		feed.updateUnreadCount();
+		yield feed.updateUnreadCount();
 	}
 });
 
diff --git a/chrome/content/zotero/xpcom/data/feedItems.js b/chrome/content/zotero/xpcom/data/feedItems.js
index fd492d629..b99292a7a 100644
--- a/chrome/content/zotero/xpcom/data/feedItems.js
+++ b/chrome/content/zotero/xpcom/data/feedItems.js
@@ -95,6 +95,7 @@ Zotero.FeedItems = new Proxy(function() {
 	});
 	
 	this.toggleReadByID = Zotero.Promise.coroutine(function* (ids, state) {
+		var feedsToUpdate = new Set();
 		if (!Array.isArray(ids)) {
 			if (typeof ids != 'string') throw new Error('ids must be a string or array in Zotero.FeedItems.toggleReadByID');
 			
@@ -104,20 +105,33 @@ Zotero.FeedItems = new Proxy(function() {
 		
 		if (state == undefined) {
 			// If state undefined, toggle read if at least one unread
-			state = true;
+			state = false;
 			for (let item of items) {
-				if (item.isRead) {
-					state = false;
+				if (!item.isRead) {
+					state = true;
 					break;
 				}
 			}
 		}
 		
-		for (let i=0; i<items.length; i++) {
-			items[i].toggleRead(state);
+		yield Zotero.DB.executeTransaction(function() {
+			for (let i=0; i<items.length; i++) {
+				items[i].isRead = state;
+				yield items[i].save({skipEditCheck: true});
+				feedsToUpdate.add(items[i].libraryID);
+			}
+		});
+		for (let feedID of feedsToUpdate) {
+			let feed = Zotero.Feeds.get(feedID);
+			yield feed.updateUnreadCount();
 		}
 	});
 	
+	this.forceErase = function(ids, options = {}) {
+		options.skipEditCheck = true;
+		return this.erase(ids, options);
+	};
+	
 	return this;
 }.call({}),
 
diff --git a/chrome/content/zotero/xpcom/data/feeds.js b/chrome/content/zotero/xpcom/data/feeds.js
index 150152c0e..909636e04 100644
--- a/chrome/content/zotero/xpcom/data/feeds.js
+++ b/chrome/content/zotero/xpcom/data/feeds.js
@@ -118,8 +118,8 @@ Zotero.Feeds = new function() {
 		Zotero.debug("Scheduling next feed update.");
 		let sql = "SELECT ( CASE "
 			+ "WHEN lastCheck IS NULL THEN 0 "
-			+ "ELSE julianday(lastCheck, 'utc') + (refreshInterval/1440.0) - julianday('now', 'utc') "
-			+ "END ) * 1440 AS nextCheck "
+			+ "ELSE strftime('%s', lastCheck) + refreshInterval*3600 - strftime('%s', 'now') "
+			+ "END ) AS nextCheck "
 			+ "FROM feeds WHERE refreshInterval IS NOT NULL "
 			+ "ORDER BY nextCheck ASC LIMIT 1";
 		var nextCheck = yield Zotero.DB.valueQueryAsync(sql);
@@ -130,9 +130,9 @@ Zotero.Feeds = new function() {
 		}
 
 		if (nextCheck !== false) {
-			nextCheck = nextCheck > 0 ? Math.ceil(nextCheck * 60000) : 0;
-			Zotero.debug("Next feed check in " + nextCheck/60000 + " minutes");
-			this._nextFeedCheck = Zotero.Promise.delay(nextCheck).cancellable();
+			nextCheck = nextCheck > 0 ? nextCheck * 1000 : 0;
+			Zotero.debug("Next feed check in " + nextCheck / 60000 + " minutes");
+			this._nextFeedCheck = Zotero.Promise.delay(nextCheck);
 			Zotero.Promise.all([this._nextFeedCheck, globalFeedCheckDelay])
 			.then(() => {
 				globalFeedCheckDelay = Zotero.Promise.delay(60000); // Don't perform auto-updates more than once per minute
@@ -157,16 +157,12 @@ Zotero.Feeds = new function() {
 				+ "OR (julianday(lastCheck, 'utc') + (refreshInterval/1440.0) - julianday('now', 'utc')) <= 0 )";
 		let needUpdate = (yield Zotero.DB.queryAsync(sql)).map(row => row.id);
 		Zotero.debug("Running update for feeds: " + needUpdate.join(', '));
-		let feeds = Zotero.Libraries.get(needUpdate);
-		let updatePromises = [];
-		for (let i=0; i<feeds.length; i++) {
-			updatePromises.push(feeds[i]._updateFeed());
+		for (let i=0; i<needUpdate.length; i++) {
+			let feed = Zotero.Feeds.get(needUpdate[i]);
+			yield feed._updateFeed();
 		}
 		
-		return Zotero.Promise.settle(updatePromises)
-		.then(() => {
-			Zotero.debug("All feed updates done.");
-			this.scheduleNextFeedCheck()
-		});
+		Zotero.debug("All feed updates done.");
+		this.scheduleNextFeedCheck();
 	});
 }
diff --git a/chrome/content/zotero/xpcom/data/libraries.js b/chrome/content/zotero/xpcom/data/libraries.js
index d0a5177ee..39cc8ea1c 100644
--- a/chrome/content/zotero/xpcom/data/libraries.js
+++ b/chrome/content/zotero/xpcom/data/libraries.js
@@ -298,7 +298,7 @@ Zotero.Libraries = new function () {
 		this._ensureExists(libraryID);
 		return Zotero.Libraries.get(libraryID).filesEditable;
 	};
-
+	
 	/**
 	 * @deprecated
 	 * 
diff --git a/chrome/content/zotero/xpcom/feedReader.js b/chrome/content/zotero/xpcom/feedReader.js
index 794d33cff..f74fc1a4d 100644
--- a/chrome/content/zotero/xpcom/feedReader.js
+++ b/chrome/content/zotero/xpcom/feedReader.js
@@ -35,6 +35,7 @@
  * http://rss.sciencedirect.com/publication/science/20925212
  * http://www.ncbi.nlm.nih.gov/entrez/eutils/erss.cgi?rss_guid=1fmfIeN4X5Q8HemTZD5Rj6iu6-FQVCn7xc7_IPIIQtS1XiD9bf
  * http://export.arxiv.org/rss/astro-ph
+ * http://fhs.dukejournals.org/rss_feeds/recent.xml TODO: refreshing unreads all items
  */
 
 /**
@@ -104,7 +105,6 @@ Zotero.FeedReader = function(url) {
 		
 		this._feedProperties = info;
 		this._feed = feed;
-		return info;
 	}.bind(this)).then(function(){
 		let items = this._feed.items;
 		if (items && items.length) {
@@ -119,9 +119,11 @@ Zotero.FeedReader = function(url) {
 				this._feedItems.push(Zotero.Promise.defer()); // Push a new deferred promise so an iterator has something to return
 				lastItem.resolve(feedItem);
 			}
-
-		this._feedProcessed.resolve();
 		}
+		this._feedProcessed.resolve();
+	}.bind(this)).catch(function(e) {
+		Zotero.debug("Feed processing failed " + e.message);
+		this._feedProcessed.reject(e);
 	}.bind(this)).finally(function() {
 		// Make sure the last promise gets resolved to null
 		let lastItem = this._feedItems[this._feedItems.length - 1];
@@ -229,6 +231,10 @@ Zotero.defineProperty(Zotero.FeedReader.prototype, 'ItemIterator', {
 			};
 		};
 		
+		iterator.prototype.last = function() {
+			return items[items.length-1];
+		}
+		
 		return iterator;
 	}
 }, {lazy: true});
@@ -304,7 +310,8 @@ Zotero.FeedReader._processCreators = function(feedEntry, field, role) {
 				names.push(name);
 			}
 		}
-	} catch(e) {
+	} 
+	catch(e) {
 		if (e.result != Components.results.NS_ERROR_FAILURE) throw e;
 		
 		if (field != 'authors') return [];
@@ -372,20 +379,21 @@ Zotero.FeedReader._getFeedItem = function(feedEntry, feedInfo) {
 	if (feedEntry.updated) item.dateModified = new Date(feedEntry.updated);
 	
 	if (feedEntry.published) {
-		let date = new Date(feedEntry.published);
+		var date = new Date(feedEntry.published);
 		
 		if (!date.getUTCSeconds() && !(date.getUTCHours() && date.getUTCMinutes())) {
 			// There was probably no time, but there may have been a a date range,
 			// so something could have ended up in the hour _or_ minute field
-			item.date = getFeedField(feedEntry, null, 'pubDate')
+			date = getFeedField(feedEntry, 'pubDate')
 				/* In case it was magically pulled from some other field */
 				|| ( date.getUTCFullYear() + '-'
 					+ (date.getUTCMonth() + 1) + '-'
 					+  date.getUTCDate() );
-		} else {
-			item.date = Zotero.FeedReader._formatDate(date);
-			// Add time zone
+		} 
+		else {
+			date = Zotero.Date.dateToSQL(date, true);
 		}
+		item.dateAdded = date;
 		
 		if (!item.dateModified) {
 			items.dateModified = date;
@@ -395,16 +403,16 @@ Zotero.FeedReader._getFeedItem = function(feedEntry, feedInfo) {
 	if (!item.dateModified) {
 		// When there's no reliable modification date, we can assume that item doesn't get updated
 		Zotero.debug("FeedReader: Feed item missing a modification date (" + item.guid + ")");
+	} else {
+		// Convert date modified to string, since those are directly comparable
+		item.dateModified = Zotero.Date.dateToSQL(item.dateModified, true);
 	}
 	
-	if (!item.date && item.dateModified) {
+	if (!item.dateAdded && item.dateModified) {
 		// Use lastModified date
-		item.date = Zotero.FeedReader._formatDate(item.dateModified);
+		item.dateAdded = item.dateModified;
 	}
 	
-	// Convert date modified to string, since those are directly comparable
-	if (item.dateModified) item.dateModified = Zotero.Date.dateToSQL(item.dateModified, true);
-	
 	if (feedEntry.rights) item.rights = Zotero.FeedReader._getRichText(feedEntry.rights, 'rights');
 	
 	item.creators = Zotero.FeedReader._processCreators(feedEntry, 'authors', 'author');
@@ -421,7 +429,7 @@ Zotero.FeedReader._getFeedItem = function(feedEntry, feedInfo) {
 	
 	/** Done with basic metadata, now look for better data **/
 	
-	let date = Zotero.FeedReader._getFeedField(feedEntry, 'publicationDate', 'prism')
+	date = Zotero.FeedReader._getFeedField(feedEntry, 'publicationDate', 'prism')
 		|| Zotero.FeedReader._getFeedField(feedEntry, 'date', 'dc');
 	if (date) item.date = date;
 	
@@ -499,13 +507,6 @@ Zotero.FeedReader._getRichText = function(feedText, field) {
 	return Zotero.Utilities.dom2text(domFragment, field);
 };
 
-/*
- * Format JS date as SQL date
- */
-Zotero.FeedReader._formatDate = function(date) {
-	return Zotero.Date.dateToSQL(date, true);
-}
-
 /*
  * Get field value from feed entry by namespace:fieldName
  */
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index 960510c31..bc0d8fa55 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -98,6 +98,7 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree
 		}
 		
 		this._treebox = treebox;
+		this.setSortColumn();
 		
 		if (this._ownerDocument.defaultView.ZoteroPane_Local) {
 			this._ownerDocument.defaultView.ZoteroPane_Local.setItemsPaneMessage(Zotero.getString('pane.items.loading'));
@@ -259,6 +260,48 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree
 });
 
 
+Zotero.ItemTreeView.prototype.setSortColumn = function() {
+	var dir, col, currentCol, currentDir;
+	
+	for (let i=0, len=this._treebox.columns.count; i<len; i++) {
+		let column = this._treebox.columns.getColumnAt(i);
+		if (column.element.getAttribute('sortActive')) {
+			currentCol = column;
+			currentDir = column.element.getAttribute('sortDirection');
+			column.element.removeAttribute('sortActive');
+			column.element.removeAttribute('sortDirection');
+			break;
+		}
+	}
+	
+	let colId = Zotero.Prefs.get('itemTree.sortColumnId');
+	// Restore previous sort setting (feed -> non-feed)
+	if (! this.collectionTreeRow.isFeed() && colId) {
+		col = this._treebox.columns.getNamedColumn(colId);
+		dir = Zotero.Prefs.get('itemTree.sortDirection');
+		Zotero.Prefs.clear('itemTree.sortColumnId');
+		Zotero.Prefs.clear('itemTree.sortDirection');
+	// Sort Feeds by dateAdded (anything -> feed)
+	} else if (this.collectionTreeRow.isFeed()) {
+		col = this._treebox.columns.getNamedColumn("zotero-items-column-dateAdded");
+		dir = 'descending';
+		// No previous sort setting stored, so store it (non-feed -> feed)
+		if (!colId && currentCol) {
+			Zotero.Prefs.set('itemTree.sortColumnId', currentCol.id);
+			Zotero.Prefs.set('itemTree.sortDirection', currentDir);
+		}
+	// Retain current sort setting (non-feed -> non-feed)
+	} else {
+		col = currentCol;
+		dir = currentDir;
+	}
+	if (col) {
+		col.element.setAttribute('sortActive', true);
+		col.element.setAttribute('sortDirection', dir);
+	}
+}
+
+
 /**
  *  Reload the rows from the data access methods
  *  (doesn't call the tree.invalidate methods, etc.)
diff --git a/chrome/content/zotero/xpcom/uri.js b/chrome/content/zotero/xpcom/uri.js
index 7cc2e9bc6..e5abba6d7 100644
--- a/chrome/content/zotero/xpcom/uri.js
+++ b/chrome/content/zotero/xpcom/uri.js
@@ -188,7 +188,7 @@ Zotero.URI = new function () {
 			return path;
 		}
 		
-		if (obj instanceof Zotero.Item || obj instanceof Zotero.Feed) {
+		if (obj instanceof Zotero.Item) {
 			return path + '/items/' + obj.key;
 		}
 		
diff --git a/chrome/content/zotero/xpcom/utilities.js b/chrome/content/zotero/xpcom/utilities.js
index d623696f2..6cb530c3f 100644
--- a/chrome/content/zotero/xpcom/utilities.js
+++ b/chrome/content/zotero/xpcom/utilities.js
@@ -252,6 +252,38 @@ Zotero.Utilities = {
 		var x = x.replace(/^[\x00-\x27\x29-\x2F\x3A-\x40\x5B-\x60\x7B-\x7F\s]+/, "");
 		return x.replace(/[\x00-\x28\x2A-\x2F\x3A-\x40\x5B-\x60\x7B-\x7F\s]+$/, "");
 	},
+
+	/**
+	 * Cleans a http url string
+	 * @param url {String}
+	 * @params tryHttp {Boolean} Attempt prepending 'http://' to the url
+	 * @returns {String}
+	 */
+	cleanURL: function(url, tryHttp=false) {
+		url = url.trim();
+		if (!url) return false;
+		
+		var ios = Components.classes["@mozilla.org/network/io-service;1"]
+			.getService(Components.interfaces.nsIIOService);
+		try {
+			return ios.newURI(url, null, null).spec; // Valid URI if succeeds
+		} catch (e) {
+			if (e instanceof Components.Exception
+				&& e.result == Components.results.NS_ERROR_MALFORMED_URI
+			) {
+				if (tryHttp && /\w\.\w/.test(url)) {
+					// Assume it's a URL missing "http://" part
+					try {
+						return ios.newURI('http://' + url, null, null).spec;
+					} catch (e) {}
+				}
+				
+				Zotero.debug('cleanURL: Invalid URI: ' + url, 2);
+				return false;
+			}
+			throw e;
+		}
+	},
 	
 	/**
 	 * Eliminates HTML tags, replacing &lt;br&gt;s with newlines
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
index b473774eb..f19cf670d 100644
--- a/chrome/content/zotero/zoteroPane.js
+++ b/chrome/content/zotero/zoteroPane.js
@@ -860,8 +860,8 @@ var ZoteroPane = new function()
 			feed.url = data.url;
 			feed.name = data.title;
 			feed.refreshInterval = data.ttl;
-			feed.cleanupAfter = data.cleanAfter;
-			yield feed.save({skipEditCheck: true});
+			feed.cleanupAfter = data.cleanupAfter;
+			yield feed.saveTx();
 			yield feed.updateFeed();
 		}
 	});
@@ -1927,7 +1927,7 @@ var ZoteroPane = new function()
 		feed.name = data.title;
 		feed.refreshInterval = data.ttl;
 		feed.cleanupAfter = data.cleanAfter;
-		yield feed.save({skipEditCheck: true});
+		yield feed.saveTx();
 	});
 	
 	this.refreshFeed = function() {
@@ -4285,8 +4285,7 @@ var ZoteroPane = new function()
 		}
 		
 		let feedItem;
-		itemReadTimeout = Zotero.FeedItems.getAsync(feedItemID)
-		.cancellable()
+		itemReadTimeout = Zotero.FeedItems.getAsync(feedItemID) 
 		.then(function(newFeedItem) {
 			if (!newFeedItem) {
 				throw new Zotero.Promise.CancellationError('Not a FeedItem');
diff --git a/chrome/locale/en-US/zotero/zotero.dtd b/chrome/locale/en-US/zotero/zotero.dtd
index 44ce77703..7549bbb2f 100644
--- a/chrome/locale/en-US/zotero/zotero.dtd
+++ b/chrome/locale/en-US/zotero/zotero.dtd
@@ -265,8 +265,8 @@
 <!ENTITY zotero.feedSettings.refresh.label1             "Refresh Interval:">
 <!ENTITY zotero.feedSettings.refresh.label2             "hour(s)">
 <!ENTITY zotero.feedSettings.title.label                "Title">
-<!ENTITY zotero.feedSettings.cleanAfter.label1         "Remove read articles after ">
-<!ENTITY zotero.feedSettings.cleanAfter.label2         "day(s)">
+<!ENTITY zotero.feedSettings.cleanupAfter.label1         "Remove read articles after ">
+<!ENTITY zotero.feedSettings.cleanupAfter.label2         "day(s)">
 
 
 <!ENTITY zotero.recognizePDF.recognizing.label			"Retrieving Metadata…">
diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties
index b1ab2206c..fc537182e 100644
--- a/chrome/locale/en-US/zotero/zotero.properties
+++ b/chrome/locale/en-US/zotero/zotero.properties
@@ -184,7 +184,7 @@ pane.collections.duplicate		= Duplicate Items
 
 pane.collections.menu.rename.collection			= Rename Collection…
 pane.collections.menu.edit.savedSearch			= Edit Saved Search…
-pane.collections.menu.edit.savedSearch			= Edit Feed…
+pane.collections.menu.edit.feed					= Edit Feed…
 pane.collections.menu.delete.collection			= Delete Collection…
 pane.collections.menu.delete.collectionAndItems	= Delete Collection and Items…
 pane.collections.menu.delete.savedSearch			= Delete Saved Search…
diff --git a/test/content/support.js b/test/content/support.js
index acb45c2d4..760bb705d 100644
--- a/test/content/support.js
+++ b/test/content/support.js
@@ -305,6 +305,24 @@ var createGroup = Zotero.Promise.coroutine(function* (props = {}) {
 	return group;
 });
 
+var createFeed = Zotero.Promise.coroutine(function* (props = {}) {
+	var feed = new Zotero.Feed;
+	feed.name = props.name || "Test " + Zotero.Utilities.randomString();
+	feed.description = props.description || "";
+	feed.url = props.url || 'http://www.' + Zotero.Utilities.randomString() + '.com/feed.rss';
+	feed.refreshInterval = props.refreshInterval || 12;
+	feed.cleanupAfter = props.cleanupAfter || 2;
+	yield feed.saveTx();
+	return feed;
+});
+
+var clearFeeds = Zotero.Promise.coroutine(function* () {
+	let feeds = Zotero.Feeds.getAll();
+	for (let i=0; i<feeds.length; i++) {
+		yield feeds[i].eraseTx();
+	}
+});
+
 //
 // Data objects
 //
diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js
index 959b1f72a..f94a964b3 100644
--- a/test/tests/collectionTreeViewTest.js
+++ b/test/tests/collectionTreeViewTest.js
@@ -292,6 +292,13 @@ describe("Zotero.CollectionTreeView", function() {
 				spy.restore();
 			}
 		})
+		
+		it("should select a new feed", function* () {
+			var feed = yield createFeed();
+			// Library should still be selected
+			assert.equal(cv.getSelectedLibraryID(), feed.id);
+		})
+		
 	})
 	
 	describe("#drop()", function () {
diff --git a/test/tests/data/feedModified.rss b/test/tests/data/feedModified.rss
new file mode 100644
index 000000000..9e7e0fff1
--- /dev/null
+++ b/test/tests/data/feedModified.rss
@@ -0,0 +1,42 @@
+<?xml version="1.0"?>
+<!-- Lifted from http://cyber.law.harvard.edu/rss/examples/rss2sample.xml -->
+<rss version="2.0">
+   <channel>
+      <title>Liftoff News</title>
+      <link>http://liftoff.msfc.nasa.gov/</link>
+      <description>Liftoff to Space Exploration.</description>
+      <language>en-us</language>
+      <pubDate>Tue, 03 Jun 2037 09:39:21 GMT</pubDate>
+      <lastBuildDate>Tue, 03 Jun 2037 09:39:21 GMT</lastBuildDate>
+      <docs>http://blogs.law.harvard.edu/tech/rss</docs>
+      <generator>Weblog Editor 2.0</generator>
+      <managingEditor>editor@example.com</managingEditor>
+      <webMaster>webmaster@example.com</webMaster>
+      <item>
+         <title>Star City</title>
+         <link>http://liftoff.msfc.nasa.gov/news/2003/news-starcity.asp</link>
+         <description>How do Americans get ready to work with Russians aboard the International Space Station? They take a crash course in culture, language and protocol at Russia's &lt;a href="http://howe.iki.rssi.ru/GCTC/gctc_e.htm"&gt;Star City&lt;/a&gt;.</description>
+         <pubDate>Tue, 03 Jun 2037 09:39:21 GMT</pubDate>
+         <guid>http://liftoff.msfc.nasa.gov/2003/06/03.html#item573</guid>
+      </item>
+      <item>
+         <description>Sky watchers in Europe, Asia, and parts of Alaska and Canada will experience a &lt;a href="http://science.nasa.gov/headlines/y2003/30may_solareclipse.htm"&gt;partial eclipse of the Sun&lt;/a&gt; on Saturday, May 31st.</description>
+         <pubDate>Fri, 30 May 2003 11:06:42 GMT</pubDate>
+         <guid>http://liftoff.msfc.nasa.gov/2003/05/30.html#item572</guid>
+      </item>
+      <item>
+         <title>The Engine That Does More</title>
+         <link>http://liftoff.msfc.nasa.gov/news/2003/news-VASIMR.asp</link>
+         <description>Before man travels to Mars, NASA hopes to design new engines that will let us fly through the Solar System more quickly.  The proposed VASIMR engine would do that.</description>
+         <pubDate>Tue, 27 May 2003 08:37:32 GMT</pubDate>
+         <guid>http://liftoff.msfc.nasa.gov/2003/05/27.html#item571</guid>
+      </item>
+      <item>
+         <title>Astronauts' Dirty Laundry</title>
+         <link>http://liftoff.msfc.nasa.gov/news/2003/news-laundry.asp</link>
+         <description>Compared to earlier spacecraft, the International Space Station has many luxuries, but laundry facilities are not one of them.  Instead, astronauts have other options.</description>
+         <pubDate>Tue, 20 May 2003 08:56:02 GMT</pubDate>
+         <guid>http://liftoff.msfc.nasa.gov/2003/05/20.html#item570</guid>
+      </item>
+   </channel>
+</rss>
\ No newline at end of file
diff --git a/test/tests/feedItemTest.js b/test/tests/feedItemTest.js
index dbf069771..c8717a901 100644
--- a/test/tests/feedItemTest.js
+++ b/test/tests/feedItemTest.js
@@ -1,12 +1,12 @@
 describe("Zotero.FeedItem", function () {
 	let feed, libraryID;
 	before(function* () {
-		feed = new Zotero.Feed({ name: 'Test ' + Zotero.randomString(), url: 'http://' + Zotero.randomString() + '.com/' });
+		feed = yield createFeed({ name: 'Test ' + Zotero.randomString(), url: 'http://' + Zotero.randomString() + '.com/' });
 		yield feed.saveTx();
 		libraryID = feed.libraryID;
 	});
 	after(function() {
-		return feed.eraseTx();
+		return clearFeeds();
 	});
 	
 	it("should be an instance of Zotero.Item", function() {
@@ -90,6 +90,22 @@ describe("Zotero.FeedItem", function () {
 			assert.closeTo(readTime, expectedTimestamp, 2000, 'read timestamp is correct in the DB');
 		});
 	});
+	describe("#fromJSON()", function() {
+		it("should attempt to parse non ISO-8601 dates", function* () {
+			var json = {
+				itemType: "journalArticle",
+				accessDate: "2015-06-07 20:56:00",
+				dateAdded: "18-20 June 2015", // magically parsed by `new Date()`
+				dateModified: "07/06/2015", // US
+			};
+			var item = new Zotero.FeedItem;
+			item.fromJSON(json);
+			assert.strictEqual(item.getField('accessDate'), '2015-06-07 20:56:00');
+			assert.strictEqual(item.getField('dateAdded'), '2015-06-18 20:00:00');
+			// sets a timezone specific hour when new Date parses from strings without hour specified.
+			assert.strictEqual(item.getField('dateModified'), Zotero.Date.dateToSQL(new Date(2015, 6, 6), true)); 
+		})
+	});
 	describe("#save()", function() {
 		it("should require edit check override", function* () {
 			let feedItem = new Zotero.FeedItem('book', { guid: Zotero.randomString() });
@@ -175,4 +191,33 @@ describe("Zotero.FeedItem", function () {
 			yield assert.isRejected(feedItem.eraseTx(), /^Error: Cannot edit feedItem in read-only library/);
 		});
 	});
+	
+	describe("#toggleRead()", function() {
+		it('should toggle state', function* () {
+			feed = yield createFeed();
+			
+			let item = yield createDataObject('feedItem', { guid: Zotero.randomString(), libraryID: feed.id });
+			item.isRead = false;
+			yield item.forceSaveTx();
+			
+			yield item.toggleRead();
+			assert.isTrue(item.isRead, "item is toggled to read state");
+		});
+		it('should save if specified state is different from current', function* (){
+			feed = yield createFeed();
+
+			let item = yield createDataObject('feedItem', { guid: Zotero.randomString(), libraryID: feed.id });
+			item.isRead = false;
+			yield item.forceSaveTx();
+			sinon.spy(item, 'save');
+
+			yield item.toggleRead(true);
+			assert.isTrue(item.save.called, "item was saved on toggle read");
+			
+			item.save.reset();
+			
+			yield item.toggleRead(true);
+			assert.isFalse(item.save.called, "item was not saved on toggle read to same state");
+		});
+	});
 });
diff --git a/test/tests/feedItemsTest.js b/test/tests/feedItemsTest.js
index bb8065b59..4626adef9 100644
--- a/test/tests/feedItemsTest.js
+++ b/test/tests/feedItemsTest.js
@@ -1,11 +1,10 @@
 describe("Zotero.FeedItems", function () {
 	let feed;
-	before(function() {
-		feed = new Zotero.Feed({ name: 'foo', url: 'http://' + Zotero.randomString() + '.com' });
-		return feed.saveTx();
+	before(function* () {
+		feed = yield createFeed({ name: 'foo', url: 'http://' + Zotero.randomString() + '.com' });
 	});
 	after(function() {
-		return feed.eraseTx();
+		return clearFeeds();
 	});
 	
 	describe("#getIDFromGUID()", function() {
@@ -35,4 +34,64 @@ describe("Zotero.FeedItems", function () {
 			assert.isFalse(feedItem);
 		});
 	});
+	describe("#toggleReadByID()", function() {
+		var save, feed, items, ids;
+		
+		before(function() {
+			save = sinon.spy(Zotero.FeedItem.prototype, 'save');
+		});
+		
+		beforeEach(function* (){
+			feed = yield createFeed();
+
+			items = [];
+			for (let i = 0; i < 10; i++) {
+				let item = yield createDataObject('feedItem', { guid: Zotero.randomString(), libraryID: feed.id });
+				item.isRead = true;
+				yield item.forceSaveTx();
+				items.push(item);
+			}
+			ids = Array.map(items, (i) => i.id);
+		});
+		
+		after(function() {
+			save.restore();
+		});
+		
+		afterEach(function* () {
+			save.reset();
+			
+			yield clearFeeds();
+		});
+	
+		it('should toggle all items read if at least one unread', function* () {
+			items[0].isRead = false;
+			yield items[0].forceSaveTx();
+			
+			yield Zotero.FeedItems.toggleReadByID(ids);
+			
+			for(let i = 0; i < 10; i++) {
+				assert.isTrue(save.thisValues[i].isRead, "#toggleRead called with true");
+			}
+		});
+
+		it('should toggle all items unread if all read', function* () {
+			yield Zotero.FeedItems.toggleReadByID(ids);
+
+			for(let i = 0; i < 10; i++) {
+				assert.isFalse(save.thisValues[i].isRead, "#toggleRead called with false");
+			}
+		});
+
+		it('should toggle all items unread if unread state specified', function* () {
+			items[0].isRead = false;
+			yield items[0].forceSaveTx();
+
+			yield Zotero.FeedItems.toggleReadByID(ids, false);
+
+			for(let i = 0; i < 10; i++) {
+				assert.isFalse(save.thisValues[i].isRead, "#toggleRead called with true");
+			}
+		});
+	});
 });
diff --git a/test/tests/feedReaderTest.js b/test/tests/feedReaderTest.js
index 441246808..c262a8d75 100644
--- a/test/tests/feedReaderTest.js
+++ b/test/tests/feedReaderTest.js
@@ -30,6 +30,10 @@ describe("Zotero.FeedReader", function () {
 		language: 'en'
 	};
 
+	after(function* () {
+		yield clearFeeds();
+	});
+
 	describe('FeedReader()', function () {
 		it('should throw if url not provided', function() {
 			assert.throw(() => new Zotero.FeedReader())
@@ -108,7 +112,7 @@ describe("Zotero.FeedReader", function () {
 				abstractNote: 'How do Americans get ready to work with Russians aboard the International Space Station? They take a crash course in culture, language and protocol at Russia\'s Star City.',
 				url: 'http://liftoff.msfc.nasa.gov/news/2003/news-starcity.asp',
 				dateModified: '2003-06-03 09:39:21',
-				date: '2003-06-03 09:39:21',
+				dateAdded: '2003-06-03 09:39:21',
 				creators: [{
 					firstName: '',
 					lastName: 'editor@example.com',
@@ -127,12 +131,13 @@ describe("Zotero.FeedReader", function () {
 		});
 		
 		it('should parse items correctly for a detailed feed', function* () {
-			let expected = { guid: 'http://www.example.com/item1',
+			let expected = {
+				guid: 'http://www.example.com/item1',
 				title: 'Title 1',
 				abstractNote: 'Description 1',
 				url: 'http://www.example.com/item1',
 				dateModified: '2016-01-07 00:00:00',
-				date: '2016-01-07',
+				dateAdded: '2016-01-07 00:00:00',
 				creators: [
 					{ firstName: 'Author1 A. T.', lastName: 'Rohtua', creatorType: 'author' },
 					{ firstName: 'Author2 A.', lastName: 'Auth', creatorType: 'author' },
@@ -141,6 +146,7 @@ describe("Zotero.FeedReader", function () {
 					{ firstName: 'Contributor2 C.', lastName: 'Contrib', creatorType: 'contributor' },
 					{ firstName: 'Contributor3', lastName: 'Contr', creatorType: 'contributor' }
 				],
+				date: '2016-01-07',
 				publicationTitle: 'Publication',
 				ISSN: '0000-0000',
 				publisher: 'Publisher',
diff --git a/test/tests/feedTest.js b/test/tests/feedTest.js
index 52d1d13a5..bf7ebad91 100644
--- a/test/tests/feedTest.js
+++ b/test/tests/feedTest.js
@@ -1,24 +1,7 @@
 describe("Zotero.Feed", function() {
-	let createFeed = Zotero.Promise.coroutine(function* (props = {}) {
-		let feed = new Zotero.Feed({
-			name: props.name || 'Test ' + Zotero.randomString(),
-			url: props.url || 'http://www.' + Zotero.randomString() + '.com',
-			refreshInterval: props.refreshInterval,
-			cleanupAfter: props.cleanupAfter
-		});
-		
-		yield feed.saveTx();
-		return feed;
-	});
-	
 	// Clean up after after tests
 	after(function* () {
-		let feeds = Zotero.Feeds.getAll();
-		yield Zotero.DB.executeTransaction(function* () {
-			for (let i=0; i<feeds.length; i++) {
-				yield feeds[i].erase();
-			}
-		});
+		yield clearFeeds();
 	});
 	
 	it("should be an instance of Zotero.Library", function() {
@@ -93,7 +76,7 @@ describe("Zotero.Feed", function() {
 				name: 'Test ' + Zotero.randomString(),
 				url: 'http://' + Zotero.randomString() + '.com/',
 				refreshInterval: 30,
-				cleanupAfter: 2
+				cleanupAfter: 1
 			};
 			
 			let feed = yield createFeed(props);
@@ -179,6 +162,157 @@ describe("Zotero.Feed", function() {
 		});
 	});
 	
+	describe("#clearExpiredItems()", function() {
+		var feed, expiredFeedItem, readFeedItem, feedItem, feedItemIDs;
+		
+		before(function* (){
+			feed = yield createFeed({cleanupAfter: 1});
+			
+			expiredFeedItem = yield createDataObject('feedItem', { libraryID: feed.libraryID });
+			// Read 2 days ago
+			expiredFeedItem.isRead = true;
+			expiredFeedItem._feedItemReadTime = Zotero.Date.dateToSQL(
+					new Date(Date.now() - 2 * 24*60*60*1000), true);
+			yield expiredFeedItem.forceSaveTx();
+			
+			readFeedItem = yield createDataObject('feedItem', { libraryID: feed.libraryID });
+			readFeedItem.isRead = true;
+			yield readFeedItem.forceSaveTx();
+			
+			feedItem = yield createDataObject('feedItem', { libraryID: feed.libraryID });
+			
+			feedItemIDs = yield Zotero.FeedItems.getAll(feed.libraryID).map((row) => row.id);
+			
+			assert.include(feedItemIDs, feedItem.id, "feed contains unread feed item");
+			assert.include(feedItemIDs, readFeedItem.id, "feed contains read feed item");
+			assert.include(feedItemIDs, expiredFeedItem.id, "feed contains expired feed item");
+			
+			yield feed.clearExpiredItems();
+			
+			feedItemIDs = yield Zotero.FeedItems.getAll(feed.libraryID).map((row) => row.id);
+		});
+	
+		it('should clear expired items', function() {
+			assert.notInclude(feedItemIDs, expiredFeedItem.id, "feed no longer contain expired feed item");	
+		});
+		
+		it('should not clear read items that have not expired yet', function() {
+			assert.include(feedItemIDs, readFeedItem.id, "feed still contains new feed item");
+		})
+		
+		it('should not clear unread items', function() {
+			assert.include(feedItemIDs, feedItem.id, "feed still contains new feed item");
+		});
+	});
+	
+	describe('#updateFeed()', function() {
+		var feedUrl = getTestDataItemUrl("feed.rss");
+		var modifiedFeedUrl = getTestDataItemUrl("feedModified.rss");
+		
+		afterEach(function* () {
+			yield clearFeeds();
+		});
+		
+		it('should schedule next feed check', function* () {
+			let scheduleNextFeedCheck = sinon.stub(Zotero.Feeds, 'scheduleNextFeedCheck');
+			
+			let feed = yield createFeed();
+			feed._feedUrl = feedUrl;
+			yield feed.updateFeed();
+			assert.equal(scheduleNextFeedCheck.called, true);
+			
+			scheduleNextFeedCheck.restore();
+		});
+		
+		it('should add new feed items', function* () {
+			let feed = yield createFeed();
+			feed._feedUrl = feedUrl;
+			yield feed.updateFeed();
+			
+			let feedItems = yield Zotero.FeedItems.getAll(feed.id);
+			assert.equal(feedItems.length, 4);
+		});
+		
+		it('should set lastCheck and lastUpdated values', function* () {
+			let feed = yield createFeed();
+			feed._feedUrl = feedUrl;
+			
+			assert.notOk(feed.lastCheck);
+			assert.notOk(feed.lastUpdate);
+			
+			yield feed.updateFeed();
+			
+			assert.ok(feed.lastCheck >= Zotero.Date.dateToSQL(new Date(Date.now() - 1000*60), true));
+			assert.ok(feed.lastUpdate >= Zotero.Date.dateToSQL(new Date(Date.now() - 1000*60), true));
+		});
+		it('should update modified items and set unread', function* () {
+			let feed = yield createFeed();
+			feed._feedUrl = feedUrl;
+			yield feed.updateFeed();
+
+			let feedItem = yield Zotero.FeedItems.getAsyncByGUID("http://liftoff.msfc.nasa.gov/2003/06/03.html#item573");
+			feedItem.isRead = true;
+			yield feedItem.forceSaveTx();
+			feedItem = yield Zotero.FeedItems.getAsyncByGUID("http://liftoff.msfc.nasa.gov/2003/06/03.html#item573");
+			assert.isTrue(feedItem.isRead);
+			
+			let oldDateModified = feedItem.dateModified;
+			
+			feed._feedUrl = modifiedFeedUrl;
+			yield feed.updateFeed();
+			
+			feedItem = yield Zotero.FeedItems.getAsyncByGUID("http://liftoff.msfc.nasa.gov/2003/06/03.html#item573");
+			
+			assert.notEqual(oldDateModified, feedItem.dateModified);
+			assert.isFalse(feedItem.isRead)
+		});
+		it('should skip items that are not modified', function* () {
+			let feed = yield createFeed();
+			feed._feedUrl = feedUrl;
+			yield feed.updateFeed();
+
+			let feedItems = yield Zotero.FeedItems.getAll(feed.id);
+			let datesAdded = [], datesModified = [];
+			for(let feedItem of feedItems) {
+				datesAdded.push(feedItem.dateAdded);
+				datesModified.push(feedItem.dateModified);
+			}
+			
+			feed._feedUrl = modifiedFeedUrl;
+			yield feed.updateFeed();
+			
+			feedItems = yield Zotero.FeedItems.getAll(feed.id);
+			
+			let changedCount = 0;
+			for (let i = 0; i < feedItems.length; i++) {
+				assert.equal(feedItems[i].dateAdded, datesAdded[i]);
+				if (feedItems[i].dateModified != datesModified[i]) {
+					changedCount++;
+				}
+			}
+			
+			assert.equal(changedCount, 1);
+		});
+		it('should update unread count', function* () {
+			let feed = yield createFeed();
+			feed._feedUrl = feedUrl;
+			yield feed.updateFeed();
+			
+			assert.equal(feed.unreadCount, 4);
+
+			let feedItems = yield Zotero.FeedItems.getAll(feed.id);
+			for (let feedItem of feedItems) {
+				feedItem.isRead = true;
+				yield feedItem.forceSaveTx();
+			}
+			
+			feed._feedUrl = modifiedFeedUrl;
+			yield feed.updateFeed();
+			
+			assert.equal(feed.unreadCount, 1);
+		});
+	});
+	
 	describe("Adding items", function() {
 		let feed;
 		before(function* () {
diff --git a/test/tests/feedsTest.js b/test/tests/feedsTest.js
index 780b6034f..f93afb1d0 100644
--- a/test/tests/feedsTest.js
+++ b/test/tests/feedsTest.js
@@ -1,25 +1,9 @@
 describe("Zotero.Feeds", function () {
-	let createFeed = Zotero.Promise.coroutine(function* (props = {}) {
-		let feed = new Zotero.Feed({
-			name: props.name || 'Test ' + Zotero.randomString(),
-			url: props.url || 'http://www.' + Zotero.randomString() + '.com',
-			refreshInterval: props.refreshInterval,
-			cleanupAfter: props.cleanupAfter
-		});
-		
-		yield feed.saveTx();
-		return feed;
-	});
 	
-	let clearFeeds = Zotero.Promise.coroutine(function* () {
-		let feeds = Zotero.Feeds.getAll();
-		yield Zotero.DB.executeTransaction(function* () {
-			for (let i=0; i<feeds.length; i++) {
-				yield feeds[i].erase();
-			}
-		});
+	after(function* () {
+		yield clearFeeds();
 	});
-	
+
 	describe("#haveFeeds()", function() {
 		it("should return false for a DB without feeds", function* () {
 			yield clearFeeds();
@@ -56,4 +40,83 @@ describe("Zotero.Feeds", function () {
 			assert.sameMembers(feeds, [feed1, feed2]);
 		});
 	});
+	describe('#updateFeeds', function() {
+		var freshFeed, recentFeed, oldFeed;
+		var _updateFeed;
+	
+		before(function* () {
+			yield clearFeeds();
+		
+			sinon.stub(Zotero.Feeds, 'scheduleNextFeedCheck');
+			_updateFeed = sinon.stub(Zotero.Feed.prototype, '_updateFeed').resolves();
+			let url = getTestDataItemUrl("feed.rss");
+			
+			freshFeed = yield createFeed({refreshInterval: 2});
+			freshFeed._feedUrl = url;
+			freshFeed.lastCheck = null;
+			yield freshFeed.saveTx();
+			
+			recentFeed = yield createFeed({refreshInterval: 2});
+			recentFeed._feedUrl = url;
+			recentFeed.lastCheck = Zotero.Date.dateToSQL(new Date(), true);
+			yield recentFeed.saveTx();
+			
+			oldFeed = yield createFeed({refreshInterval: 2});
+			oldFeed._feedUrl = url;
+			oldFeed.lastCheck = Zotero.Date.dateToSQL(new Date(Date.now() - 1000*60*60*6), true);
+			yield oldFeed.saveTx();
+			
+			yield Zotero.Feeds.updateFeeds();
+			assert.isTrue(_updateFeed.called);
+		});
+		
+		after(function() {
+			Zotero.Feeds.scheduleNextFeedCheck.restore();
+			_updateFeed.restore();
+		});
+		
+		it('should update feeds that have never been updated', function() {
+			for (var feed of _updateFeed.thisValues) {
+				if (feed.id == freshFeed.id) {
+					break;
+				}
+			}
+			assert.isTrue(feed._updateFeed.called);
+		});
+		it('should update feeds that need updating since last check', function() {
+			for (var feed of _updateFeed.thisValues) {
+				if (feed.id == oldFeed.id) {
+					break;
+				}
+			}
+			assert.isTrue(feed._updateFeed.called);
+		});
+		it("should not update feeds that don't need updating", function() {
+			for (var feed of _updateFeed.thisValues) {
+				if (feed.id != recentFeed.id) {
+					break;
+				}
+				// should never reach
+				assert.isOk(null, "does not update feed that did not need updating")
+			}
+		});
+	});
+	describe('#scheduleNextFeedCheck()', function() {
+		it('schedules next feed check', function* () {
+			sinon.spy(Zotero.Feeds, 'scheduleNextFeedCheck');
+			sinon.spy(Zotero.Promise, 'delay');
+			
+			yield clearFeeds();
+			let feed = yield createFeed({refreshInterval: 1});
+			feed._set('_feedLastCheck', Zotero.Date.dateToSQL(new Date(), true));
+			yield feed.saveTx();
+
+			yield Zotero.Feeds.scheduleNextFeedCheck();
+			
+			assert.equal(Zotero.Promise.delay.args[0][0], 1000*60*60);
+			
+			Zotero.Feeds.scheduleNextFeedCheck.restore();
+			Zotero.Promise.delay.restore();
+		});
+	})
 })