Reader: Fix the url we send to the follow button in Full Post View

client/lib/feed-store/actions.js

@@ -1,25 +1,23 @@
 var ActionType = require( './constants' ).action,
 	Dispatcher = require( 'dispatcher' ),
-	FeedStore = require( './' ),
-	FeedState = require( './constants' ).state,
+	inflight = require( 'lib/inflight' ),
 	wpcom = require( 'lib/wp' );
 
-var FeedStoreActions = {
+function requestKey( feedId ) {
+	return `feed-${feedId}`;
+}
+
+const FeedStoreActions = {
 	fetch: function( feedId ) {
-		const feed = FeedStore.get( feedId );
+		const key = requestKey( feedId );
 
-		if ( feed && feed.state === FeedState.PENDING ) {
+		if ( inflight.requestInflight( key ) ) {
 			return;
 		}
 
-		Dispatcher.handleViewAction( {
-			type: ActionType.FETCH,
-			feedId: feedId
-		} );
-
 		wpcom.undocumented().readFeed(
 			{ ID: feedId, meta: 'site' },
-			FeedStoreActions.receiveFeedFetch.bind( FeedStoreActions, feedId )
+			inflight.requestTracker( key, FeedStoreActions.receiveFeedFetch.bind( FeedStoreActions, feedId ) )
 		);
 	},
 

client/lib/feed-store/constants.js

@@ -1,11 +1,10 @@
 module.exports = {
-  action: {
-    FETCH: 'FEED_FETCH',
-    RECEIVE_FETCH: 'RECEIVE_FEED_FETCH'
-  },
-  state: {
-    PENDING: 1,
-    COMPLETE: 2,
-    ERROR: 4
-  }
+	action: {
+		FETCH: 'FEED_FETCH',
+		RECEIVE_FETCH: 'RECEIVE_FEED_FETCH'
+	},
+	state: {
+		COMPLETE: 2,
+		ERROR: 4
+	}
 };

client/lib/feed-store/index.js

@@ -27,16 +27,6 @@ var feeds = {},
 		error: undefined
 	} );
 
-function changeState( feedId, newState ) {
-	var feed = feeds[ feedId ];
-	if ( ! feed ) {
-		feed = new FeedRecord( { feed_ID: feedId } );
-	}
-
-	feed = feed.set( 'state', newState );
-	setFeed( feedId, feed );
-}
-
 function setFeed( feedId, feed ) {
 	if ( feed !== feeds[ feedId ] ) {
 		feeds[ feedId ] = feed;
@@ -88,9 +78,6 @@ FeedStore.dispatchToken = Dispatcher.register( function( payload ) {
 	}
 
 	switch ( action.type ) {
-		case ActionType.FETCH:
-			changeState( action.feedId, State.PENDING );
-			break;
 		case ActionType.RECEIVE_FETCH:
 			if ( action.error ) {
 				receiveError( action.feedId, action.error );

client/lib/feed-store/test/feed-store-tests.js

@@ -10,7 +10,6 @@ var FeedState = require( '../constants' ).state,
 	FeedStore, FeedStoreActions;
 
 describe( 'FeedStore', function() {
-
 	var readFeedStub,
 		mockWpcom = {
 			undocumented: function() {
@@ -42,7 +41,6 @@ describe( 'FeedStore', function() {
 		changeSpy.reset();
 	} );
 
-
 	it( 'should have a dispatch token', function() {
 		expect( FeedStore ).to.have.property( 'dispatchToken' );
 	} );
@@ -51,23 +49,12 @@ describe( 'FeedStore', function() {
 		expect( FeedStore.get( 'UNKNOWN' ) ).to.be.undefined;
 	} );
 
-	it( 'should create a pending record when sent REFRESH', function() {
-		FeedStoreActions.fetch( 'KNOWN' );
-
-		expect( readFeedStub.callCount ).to.equal( 1 );
-		expect( readFeedStub.args[ 0 ][ 0 ] ).to.eql( { ID: 'KNOWN', meta: 'site' } );
-
-		expect( FeedStore.get( 'KNOWN' ).state ).to.equal( FeedState.PENDING );
-		expect( changeSpy.callCount ).to.equal( 1 );
-	} );
-
-	it( 'should pass through the pending state and prevent double fetches', function() {
-
+	it( 'should prevent double fetches', function() {
 		FeedStoreActions.fetch( 'twice' );
 		FeedStoreActions.fetch( 'twice' );
 
 		expect( readFeedStub.callCount ).to.equal( 1 );
-		expect( changeSpy.callCount ).to.equal( 1 );
+		expect( changeSpy.callCount ).to.equal( 0 );
 	} );
 
 	it( 'should accept a good response', function() {
@@ -84,7 +71,7 @@ describe( 'FeedStore', function() {
 
 		FeedStoreActions.fetch( 1234 );
 
-		expect( changeSpy.callCount ).to.equal( 2 );
+		expect( changeSpy.callCount ).to.equal( 1 );
 
 		feedFromStore = FeedStore.get( 1234 );
 
@@ -94,7 +81,6 @@ describe( 'FeedStore', function() {
 		forOwn( feed, function( val, key ) {
 			expect( feedFromStore[ key ] ).to.eql( val );
 		} );
-
 	} );
 
 	it( 'should accept an error', function() {
@@ -104,13 +90,12 @@ describe( 'FeedStore', function() {
 
 		FeedStoreActions.fetch( 'BAD' );
 
-		expect( changeSpy.callCount ).to.equal( 2 );
+		expect( changeSpy.callCount ).to.equal( 1 );
 
 		feedFromStore = FeedStore.get( 'BAD' );
 
 		expect( feedFromStore ).to.be.ok;
 		expect( feedFromStore.state ).to.equal( FeedState.ERROR );
 		expect( feedFromStore.error ).to.equal( error );
 	} );
-
 } );

client/lib/react-smart-set-state/index.js

@@ -1,12 +1,15 @@
 /**
  * A function that only sets the new state if it's different from the current state
  * @param  {object} newState The new state to set
+ * @returns {Boolean} True if new values found, false if not
  */
 export default function smartSetState( newState ) {
 	const hasNewValues = Object.keys( newState ).some( function( key ) {
 		return ( ! ( this.state && this.state.hasOwnProperty( key ) ) || ( this.state[ key ] !== newState[ key ] ) );
 	}, this );
 	if ( hasNewValues ) {
 		this.setState( newState );
+		return true;
 	}
+	return false;
 }

client/lib/reader-feed-subscriptions/index.js

@@ -10,7 +10,9 @@ const Dispatcher = require( 'dispatcher' ),
 	last = require( 'lodash/last' ),
 	Immutable = require( 'immutable' ),
 	clone = require( 'lodash/clone' ),
-	map = require( 'lodash/map' );
+	map = require( 'lodash/map' ),
+	moment = require( 'moment' ),
+	forEach = require( 'lodash/forEach' );
 
 // Internal Dependencies
 const Emitter = require( 'lib/mixins/emitter' ),
@@ -31,7 +33,7 @@ var subscriptions = clone( subscriptionsTemplate ),
 	currentPage = 0,
 	isLastPage = false,
 	isFetching = false,
-	totalSubscriptions,
+	totalSubscriptions = 0,
 	subscriptionTemplate = Immutable.Map( { // eslint-disable-line new-cap
 		state: States.SUBSCRIBED
 	} );
@@ -124,10 +126,6 @@ var FeedSubscriptionStore = {
 	},
 
 	receiveSubscriptions: function( data ) {
-		var currentSubscriptions = subscriptions,
-			newSubscriptions,
-			combinedSubscriptions;
-
 		if ( ! data.subscriptions ) {
 			return;
 		}
@@ -138,36 +136,39 @@ var FeedSubscriptionStore = {
 			return subscriptionTemplate.merge( sub );
 		} );
 
-		newSubscriptions = Immutable.List( subscriptionsWithState ); // eslint-disable-line new-cap
-
 		// Is it the last page?
 		if ( data.number === 0 ) {
 			isLastPage = true;
 		}
 
-		// Is it a new page of results?
-		if ( currentPage > 0 && data.page > currentPage ) {
-			combinedSubscriptions = currentSubscriptions.list.concat( newSubscriptions );
-
-			subscriptions = {
-				count: combinedSubscriptions.size,
-				list: combinedSubscriptions
-			};
-		} else {
-			// Looks like the first results we've received...
-			subscriptions = {
-				count: newSubscriptions.size,
-				list: newSubscriptions
-			};
-		}
+		subscriptions.list = subscriptions.list.asMutable();
+		forEach( subscriptionsWithState, function( subscription ) {
+			_acceptSubscription( subscription, false );
+		} );
+		subscriptions.list = subscriptions.list.sort( function( a, b ) {
+			const aDate = moment( a.get( 'date_subscribed' ) ),
+				bDate = moment( b.get( 'date_subscribed' ) );
+
+			if ( aDate.isAfter( bDate ) ) {
+				return -1;
+			}
+
+			if ( aDate.isBefore( bDate ) ) {
+				return 1;
+			}
+
+			return b.get( 'feed_ID' ) - a.get( 'feed_ID' );
+		} );
+
+		subscriptions.list = subscriptions.list.asImmutable();
 
 		// Set the current page
 		currentPage = data.page;
 
-		// Set total subscriptions for user on the first page only (we keep track of it in the store after that)
 		if ( currentPage === 1 ) {
 			totalSubscriptions = data.total_subscriptions;
 		}
+		subscriptions.count = subscriptions.list.count();
 
 		FeedSubscriptionStore.emit( 'change' );
 	},
@@ -231,6 +232,7 @@ var FeedSubscriptionStore = {
 
 	clearSubscriptions: function() {
 		subscriptions = clone( subscriptionsTemplate );
+		totalSubscriptions = 0;
 	},
 
 	isLastPage: function() {
@@ -250,7 +252,25 @@ var FeedSubscriptionStore = {
 	}
 };
 
-function addSubscription( subscription ) {
+function _acceptSubscription( subscription, addToTop = true ) {
+	let subs = subscriptions.list;
+	const existingIndex = subs.findIndex( value => value.get( 'URL' ) === subscription.get( 'URL' ) );
+	if ( existingIndex !== -1 ) {
+		// update the existing subscription by merging together
+		subs = subs.mergeIn( [ existingIndex ], subscription );
+		if ( subs !== subscriptions.list ) {
+			subscriptions.list = subs;
+			return true;
+		}
+		return false;
+	}
+
+	// new subscription
+	subscriptions.list = subs[ addToTop ? 'unshift' : 'push' ]( subscription );
+	return true;
+}
+
+function addSubscription( subscription, addToTop = true ) {
 	if ( ! subscription || ! subscription.URL ) {
 		return;
 	}
@@ -268,12 +288,9 @@ function addSubscription( subscription ) {
 	// Otherwise, create a new subscription
 	subscription.URL = preparedSiteUrl;
 	const newSubscription = subscriptionTemplate.merge( subscription );
-	subscriptions.list = subscriptions.list.unshift( newSubscription );
+	_acceptSubscription( newSubscription, addToTop );
 	subscriptions.count++;
-
-	if ( totalSubscriptions > 0 ) {
-		totalSubscriptions++;
-	}
+	totalSubscriptions++;
 
 	return true;
 }
@@ -289,7 +306,7 @@ function updateSubscription( url, newSubscriptionInfo ) {
 		return item.get( 'URL' ) === preparedSiteUrl;
 	} );
 
-	if ( isNaN( subscriptionIndex ) ) {
+	if ( subscriptionIndex === -1 ) {
 		return;
 	}
 
@@ -319,7 +336,7 @@ function updateSubscription( url, newSubscriptionInfo ) {
 
 	subscriptions.list = updatedSubscriptionsList;
 
-	if ( totalSubscriptions > 0 && existingSubscription.get( 'state' ) === States.UNSUBSCRIBED && updatedSubscription.get( 'state' ) === States.SUBSCRIBED ) {
+	if ( existingSubscription.get( 'state' ) === States.UNSUBSCRIBED && updatedSubscription.get( 'state' ) === States.SUBSCRIBED ) {
 		totalSubscriptions++;
 	}
 
@@ -385,4 +402,4 @@ FeedSubscriptionStore.dispatchToken = Dispatcher.register( function( payload ) {
 	}
 } );
 
-module.exports = FeedSubscriptionStore;
+export default FeedSubscriptionStore;