Reader: clean up cardpicking logic / move off sitestore/feedstore #11618

samouri · 2017-02-27T19:37:19Z

now that refresh is out the door we have an opportunity to simplify our cardpicking process

The idea here is to consolidate all of the card-picking to the post-lifecycle so that you don't need to go searching through multiple cardfactories/adapters depending on which stream you are looking at to figure out which card is getting chosen (there is still one more layer that happens in the post-normalizer, but thats separate)

Changes this introduces:

feed-stream-store now adds is_recommendation: true to any post that comes from a recommendation api. I think ideally in the future we can fully separate out the notion of where the data comes from, how to display it, and how to report metrics on it.
delete SearchCardAdapter, and all CardFactories from Search/Stream. Which card to choose for search results (or empty recs) now lives fully in post-lifecycle.
moves the class for EmptySearchRecs from search-stream to its own file. I'm hoping to somehow merge this with our RecBlock class someday

There is still more to do in later prs e.g.:

consolidate our two different recs? if not then at least ~~create a new component for empty search result recommendations~~ done
make the clickHandlers / page calls easier to follow.
generalize fluxPostAdapter to work for all postKeys

To Test:

KeyboardShortcuts, especially on a gap
tag pages, search page empty recs, all x-posts, placeholders, in-stream-recs, most things

matticbot · 2017-02-27T19:37:20Z

Test live: https://calypso.live/?branch=update/reader/reduxify-routing

blowery · 2017-02-27T19:53:58Z

client/reader/list-gap/index.jsx

@@ -13,9 +13,10 @@ var FeedStreamStoreActions = require( 'lib/feed-stream-store/actions' ),
 var Gap = React.createClass( {

 	propTypes: {
-		gap: React.PropTypes.object.isRequired,
+		gap: React.PropTypes.object,
+		postKey: React.PropTypes.object,


how would a gap have a postKey?

according to client/lib/feed-stream-store/feed-stream.js, a gap is a type of postKey. I'm still a bit iffy on this logic though

samouri · 2017-02-28T19:27:42Z

client/reader/stream/post-lifecycle.jsx

+	IN_STREAM_RECOMMENDATION,
+} from 'reader/follow-button/follow-sources';
+
+function EmptySearchRecommendedPosts( { post } ) {


I wonder if its possible to somehow combine this with our other type of RecommendedPost

matticbot · 2017-03-02T08:45:00Z

@samouri This PR needs a rebase

blowery · 2017-03-07T21:23:51Z

client/reader/utils.js

 		const { blogId, postId } = post.referral;
 		query += `ref_blog=${ blogId }&ref_post=${ postId }`;
 	}

 	const method = replaceHistory ? 'replace' : 'show';
-	if ( post.feed_ID && post.feed_item_ID ) {
-		page[ method ]( `/read/feeds/${ post.feed_ID }/posts/${ post.feed_item_ID }${ hashtag }${ query }` );
+	if ( postKey.blogId && postKey.postId ) {


we should prefer the feed item, not the blog item. blog items are less reliable for jetpack sites.

though maybe we make this smarter and default to blogs when it's not a vip or something... hrm.

there's prefer_feed on site objects now. maybe we could use that? See https://public-api.wordpress.com/rest/v1.2/read/sites/fivethirtyeight.com

blowery · 2017-03-07T21:24:32Z

client/lib/feed-stream-store/index.js

-	};
-}
+// by throwing out blogId if feed_item_ID exist then we know if its a feed/blog from here on out
+const keyMaker = post => ( {


won't this require a change to the post store?

also, we already know if it's a feed or a blog. if it's a blog, it has a blog id. otherwise, it's a feed.

blowery · 2017-03-07T21:31:04Z

client/reader/stream/post-lifecycle.jsx

+		if ( props.postKey.isRecommendationBlock ) {
+			return null;
+		}
+
 		const post = PostStore.get( props.postKey );
 		if ( ! post || post._state === 'minimal' ) {
 			defer( () => PostStoreActions.fetchPost( props.postKey ) );


this'll need to be updated to understand the new post key

no more new postKey logic

blowery · 2017-03-28T19:01:11Z

samouri added the [Status] In Progress label Feb 27, 2017

samouri self-assigned this Feb 27, 2017

matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Feb 27, 2017

samouri force-pushed the update/reader/reduxify-routing branch from ce5656d to bb617c8 Compare February 27, 2017 19:45

matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Feb 27, 2017

blowery reviewed Feb 27, 2017

View reviewed changes

samouri commented Feb 28, 2017

View reviewed changes

samouri added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Feb 28, 2017

matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Feb 28, 2017

samouri added [Status] In Progress and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 28, 2017

matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Feb 28, 2017

samouri added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] Reader The reader site on Calypso. [Type] Janitorial and removed [Status] In Progress labels Feb 28, 2017

matticbot added [Status] Needs Rebase [Size] XL Probably needs to be broken down into multiple smaller issues labels Mar 2, 2017

bluefuton removed the [Status] Needs Rebase label Mar 3, 2017

samouri force-pushed the update/reader/reduxify-routing branch from 106dab8 to b0eb67a Compare March 7, 2017 16:25

matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Mar 7, 2017

samouri force-pushed the update/reader/reduxify-routing branch from b0eb67a to 944e235 Compare March 7, 2017 16:52

matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Mar 7, 2017

blowery reviewed Mar 7, 2017

View reviewed changes

mtias changed the title ~~Clean up cardpicking logic~~ Reader: clean up cardpicking logic Mar 9, 2017

samouri merged commit cd6589a into master Mar 28, 2017

samouri deleted the update/reader/reduxify-routing branch March 28, 2017 18:57

blowery removed the [Status] Ready to Merge label Mar 28, 2017

samouri mentioned this pull request Mar 28, 2017

Fix: keyboard clicks on gaps. also es6ify #12604

Merged

matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Mar 29, 2017

This was referenced Dec 29, 2020

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#50

Open

[Snyk] Security upgrade webpack from 4.20.2 to 5.0.0 ajesse11x/wp-calypso#36

Open

awaisab172 mentioned this pull request Mar 15, 2021

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#62

Open

ajesse11x mentioned this pull request Nov 4, 2022

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#83

Open

mdno mentioned this pull request Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities mdno/wp-calypso#113

Open

mayuba mentioned this pull request Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities mayuba/wp-calypso#72

Open

awaisab172 mentioned this pull request Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#134

Open

anarh mentioned this pull request Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities anarh/wp-calypso#102

Open

apmarshall mentioned this pull request Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities NonProphets/wp-calypso#53

Open

ajesse11x mentioned this pull request Dec 25, 2022

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#87

Open

mdno mentioned this pull request Dec 25, 2022

[Snyk] Fix for 1 vulnerabilities mdno/wp-calypso#119

Open

apmarshall mentioned this pull request Dec 25, 2022

[Snyk] Fix for 1 vulnerabilities NonProphets/wp-calypso#57

Open

mayuba mentioned this pull request Dec 26, 2022

[Snyk] Fix for 1 vulnerabilities mayuba/wp-calypso#76

Open

awaisab172 mentioned this pull request Dec 26, 2022

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#141

Open

anarh mentioned this pull request Dec 26, 2022

[Snyk] Fix for 1 vulnerabilities anarh/wp-calypso#108

Open

ajesse11x mentioned this pull request Jan 9, 2023

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#88

Open

awaisab172 mentioned this pull request Jan 10, 2023

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#145

Open

anarh mentioned this pull request Nov 28, 2023

[Snyk] Fix for 13 vulnerabilities anarh/wp-calypso#115

Open

mdno mentioned this pull request Nov 28, 2023

[Snyk] Fix for 13 vulnerabilities mdno/wp-calypso#127

Open

ajesse11x mentioned this pull request Nov 29, 2023

[Snyk] Fix for 20 vulnerabilities ajesse11x/wp-calypso#96

Open

awaisab172 mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities awaisab172/wp-calypso#196

Open

mdno mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities mdno/wp-calypso#134

Open

anarh mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities anarh/wp-calypso#122

Open

ajesse11x mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities ajesse11x/wp-calypso#106

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reader: clean up cardpicking logic / move off sitestore/feedstore #11618

Reader: clean up cardpicking logic / move off sitestore/feedstore #11618

samouri commented Feb 27, 2017 •

edited

Loading

matticbot commented Feb 27, 2017

blowery Feb 27, 2017

samouri Feb 27, 2017 •

edited

Loading

samouri Feb 28, 2017

matticbot commented Mar 2, 2017

blowery Mar 7, 2017

blowery Mar 7, 2017

blowery Mar 7, 2017 •

edited

Loading

blowery Mar 7, 2017

blowery Mar 7, 2017

samouri Mar 23, 2017

blowery commented Mar 28, 2017

Reader: clean up cardpicking logic / move off sitestore/feedstore #11618

Reader: clean up cardpicking logic / move off sitestore/feedstore #11618

Conversation

samouri commented Feb 27, 2017 • edited Loading

matticbot commented Feb 27, 2017

blowery Feb 27, 2017

Choose a reason for hiding this comment

samouri Feb 27, 2017 • edited Loading

Choose a reason for hiding this comment

samouri Feb 28, 2017

Choose a reason for hiding this comment

matticbot commented Mar 2, 2017

blowery Mar 7, 2017

Choose a reason for hiding this comment

blowery Mar 7, 2017

Choose a reason for hiding this comment

blowery Mar 7, 2017 • edited Loading

Choose a reason for hiding this comment

blowery Mar 7, 2017

Choose a reason for hiding this comment

blowery Mar 7, 2017

Choose a reason for hiding this comment

samouri Mar 23, 2017

Choose a reason for hiding this comment

blowery commented Mar 28, 2017

samouri commented Feb 27, 2017 •

edited

Loading

samouri Feb 27, 2017 •

edited

Loading

blowery Mar 7, 2017 •

edited

Loading