Reader: fix feed searching having incorrect following state (Automatt…

…ic#13351)

* new getfeedUrl helper

* fix tests

* couple quick fixes to the displayed url. open in new tab and use siteUrl

* run off of subscribe_url if it exists

* move to feed_URL by adding it to data-layer for both follows and feed searches

* fix test

client/blocks/reader-subscription-list-item/index.jsx

@@ -15,9 +15,10 @@ import { getStreamUrl } from 'reader/route';
 import EmailSettings from './email-settings';
 import {
 	getSiteName,
-	getSiteUrl,
 	getSiteDescription,
-	getSiteAuthorName
+	getSiteAuthorName,
+	getFeedUrl,
+	getSiteUrl,
 } from 'reader/get-helpers';
 import untrailingslashit from 'lib/route/untrailingslashit';
 
@@ -44,7 +45,8 @@ function ReaderSubscriptionListItem( {
 	const siteIcon = get( site, 'icon.img' );
 	const feedIcon = get( feed, 'image' );
 	const streamUrl = getStreamUrl( feedId, siteId );
-	const siteUrl = url || getSiteUrl( { feed, site } );
+	const feedUrl = url || getFeedUrl( { feed, site } );
+	const siteUrl = getSiteUrl( { feed, site } );
 	const isFollowing = ( site && site.is_following ) || ( feed && feed.is_following );
 
 	return (
@@ -77,12 +79,14 @@ function ReaderSubscriptionListItem( {
 				}
 			{ siteUrl && (
 				<div className="reader-subscription-list-item__site-url">
-					<a href={ siteUrl }> { stripUrl( siteUrl ) } </a>
+					<a href={ siteUrl } target="_blank" rel="noopener noreferrer">
+						{ stripUrl( siteUrl ) }
+					</a>
 				</div>
 			) }
 			</div>
 			<div className="reader-subscription-list-item__options">
-				<FollowButton siteUrl={ siteUrl } followSource={ followSource } />
+				<FollowButton siteUrl={ feedUrl } followSource={ followSource } />
 				{ isFollowing && ! isEmailBlocked && <EmailSettings siteId={ siteId } /> }
 			</div>
 		</div>

client/reader/following-manage/sites-window-scroller.jsx

@@ -46,7 +46,7 @@ class SitesWindowScroller extends Component {
 				{ ( { measure } ) => (
 					<div key={ key } style={ style } className="following-manage__sites-window-scroller-row-wrapper" >
 							<ConnectedSubscriptionListItem
-								url={ site.URL }
+								url={ site.feed_URL }
 								feedId={ +site.feed_ID }
 								siteId={ +site.blog_ID }
 								onLoad={ measure }

client/reader/get-helpers.js

@@ -11,15 +11,31 @@ import { trim } from 'lodash';
 import { decodeEntities } from 'lib/formatting';
 
 /**
- * Given a feed, site, or post: return the url. return false if one could not be found.
+ * Given a feed, site, or post: return the site url. return false if one could not be found.
  *
  * @param {*} options - an object containing a feed, site, and post. all optional.
  * @returns {string} the site url
  */
 export const getSiteUrl = ( { feed, site, post } = {} ) => {
-	const siteUrl = ( !! site ) && ( site.URL );
-	const feedUrl = ( !! feed ) && ( feed.URL || feed.feed_URL );
-	const postUrl = ( !! post ) && ( post.site_URL || post.feed_URL );
+	const siteUrl = ( !! site ) && ( site.URL || site.domain );
+	const feedUrl = ( !! feed ) && feed.URL;
+	const postUrl = ( !! post ) && post.site_URL;
+
+	return siteUrl || feedUrl || postUrl;
+};
+
+/**
+ * Given a feed, site, or post: return the feed url. return false if one could not be found.
+ * The feed url is different from the site url in that it is unique per feed. A single siteUrl may
+ * be home to many feeds
+ *
+ * @param {*} options - an object containing a feed, site, and post. all optional.
+ * @returns {string} the site url
+ */
+export const getFeedUrl = ( { feed, site, post } = {} ) => {
+	const siteUrl = ( !! site ) && site.feed_URL;
+	const feedUrl = ( !! feed ) && ( feed.feed_URL || feed.URL );
+	const postUrl = ( !! post ) && post.feed_URL;
 
 	return siteUrl || feedUrl || postUrl;
 };

client/reader/test/get-helpers.js

@@ -30,7 +30,15 @@ describe( '#getSiteUrl', () => {
 		expect( siteUrl ).eql( feedWithUrl.URL );
 
 		const siteUrl2 = getSiteUrl( { feed: feedWithFeedUrl } );
-		expect( siteUrl2 ).eql( feedWithFeedUrl.feed_URL );
+		expect( siteUrl2 ).eql( false );
+	} );
+
+	it( 'should not use the feed_URL', () => {
+		const siteUrl2 = getSiteUrl( { feed: feedWithFeedUrl } );
+		expect( siteUrl2 ).not.ok;
+
+		const feedUrl = getSiteUrl( { post: postWithFeedUrl } );
+		expect( feedUrl ).not.ok;
 	} );
 
 	it( 'should get title from site if feed does not exist', () => {
@@ -41,9 +49,6 @@ describe( '#getSiteUrl', () => {
 	it( 'should grab url from post if its there', () => {
 		const siteUrl = getSiteUrl( { post: postWithSiteUrl } );
 		expect( siteUrl ).eql( postWithSiteUrl.site_URL );
-
-		const feedUrl = getSiteUrl( { post: postWithFeedUrl } );
-		expect( feedUrl ).eql( postWithFeedUrl.feed_URL );
 	} );
 
 	it( 'should return false if cannot find a reasonable url', () => {
@@ -69,7 +74,6 @@ describe( '#getSiteName', () => {
 	const feedWithTitleAndName = { ...feedWithTitle, ...feedWithName };
 	const feedWithError = { is_error: true };
 	const feedWithUrl = { URL: 'http://feedWithUrl.com' };
-	const feedWithFeedUrl = { feed_URL: 'http://feedwithFeedUrl.com/hello' };
 	const allFeeds = [ feedWithName, feedWithTitle, feedWithTitleAndName, feedWithError ];
 	const postWithSiteName = { site_name: 'postSiteName' };
 
@@ -103,10 +107,6 @@ describe( '#getSiteName', () => {
 			getSiteName( { site: siteWithDomain, post: {} } )
 		).eql( siteWithDomain.domain );
 
-		expect(
-			getSiteName( { feed: feedWithFeedUrl } )
-		).eql( 'feedwithfeedurl.com' );
-
 		expect(
 			getSiteName( { feed: feedWithUrl } )
 		).eql( 'feedwithurl.com' );

client/state/data-layer/wpcom/read/feed/index.js

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { map, identity } from 'lodash';
+import { map } from 'lodash';
 
 /**
  * Internal dependencies
@@ -32,7 +32,10 @@ export function initiateFeedSearch( store, action, next ) {
 }
 
 export function receiveFeeds( store, action, next, apiResponse ) {
-	const feeds = map( apiResponse.feeds, identity );
+	const feeds = map( apiResponse.feeds, feed => ( {
+		...feed,
+		feed_URL: feed.subscribe_URL,
+	} ) );
 
 	store.dispatch(
 		receiveFeedSearch( action.payload.query, feeds )

client/state/data-layer/wpcom/read/feed/test/index.js

@@ -21,7 +21,7 @@ import { http } from 'state/data-layer/wpcom-http/actions';
 import { NOTICE_CREATE } from 'state/action-types';
 
 const feeds = freeze( [
-	{ blog_ID: 'IM A BLOG' },
+	{ blog_ID: 'IM A BLOG', subscribe_URL: 'feedUrl' },
 ] );
 
 const query = 'okapis r us';
@@ -69,7 +69,11 @@ describe( 'wpcom-api', () => {
 
 				expect( dispatch ).to.have.been.calledOnce;
 				expect( dispatch ).to.have.been.calledWith(
-					receiveFeedSearch( query, feeds )
+					receiveFeedSearch( query, [ {
+						blog_ID: 'IM A BLOG',
+						feed_URL: 'feedUrl',
+						subscribe_URL: 'feedUrl',
+					} ] )
 				);
 			} );
 		} );

client/state/data-layer/wpcom/read/following/mine/index.js

@@ -40,6 +40,7 @@ export const subscriptionsFromApi = apiResponse => {
 		return omitBy( {
 			ID: Number( subscription.ID ),
 			URL: subscription.URL,
+			feed_URL: subscription.URL,
 			blog_ID: toValidId( subscription.blog_ID ),
 			feed_ID: toValidId( subscription.feed_ID ),
 			date_subscribed: Date.parse( subscription.date_subscribed ),

client/state/data-layer/wpcom/read/following/mine/test/index.js

@@ -150,12 +150,14 @@ describe( 'get follow subscriptions', () => {
 					ID: 12345,
 					blog_ID: 122463145,
 					URL: 'http://readerpostcards.wordpress.com',
+					feed_URL: 'http://readerpostcards.wordpress.com',
 					date_subscribed: Date.parse( '2017-01-12T03:55:45+00:00' ),
 				},
 				{
 					ID: 123456,
 					blog_ID: 64146350,
 					URL: 'https://fivethirtyeight.com/',
+					feed_URL: 'https://fivethirtyeight.com/',
 					date_subscribed: Date.parse( '2016-01-12T03:55:45+00:00' ),
 				}
 			];