-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reader: load a new recommendation after an interaction with a card #5652
Changes from all commits
07504f1
b444818
2c7d464
f539fd8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,9 @@ | |
* External dependencies | ||
*/ | ||
import { combineReducers } from 'redux'; | ||
import keyBy from 'lodash/keyBy'; | ||
import union from 'lodash/union'; | ||
import find from 'lodash/find'; | ||
import filter from 'lodash/filter'; | ||
|
||
/** | ||
* Internal dependencies | ||
|
@@ -12,31 +14,39 @@ import { | |
READER_START_RECOMMENDATIONS_REQUEST, | ||
READER_START_RECOMMENDATIONS_REQUEST_SUCCESS, | ||
READER_START_RECOMMENDATIONS_REQUEST_FAILURE, | ||
READER_START_RECOMMENDATION_INTERACTION, | ||
SERIALIZE, | ||
DESERIALIZE, | ||
} from 'state/action-types'; | ||
import { itemsSchema } from './schema'; | ||
import { isValidStateWithSchema } from 'state/utils'; | ||
|
||
/** | ||
* Tracks all known list objects, indexed by list ID. | ||
* | ||
* @param {Object} state Current state | ||
* @param {Array} state Current state | ||
* @param {Object} action Action payload | ||
* @return {Object} Updated state | ||
* @return {Array} Updated state | ||
*/ | ||
export function items( state = {}, action ) { | ||
export function items( state = [], action ) { | ||
switch ( action.type ) { | ||
case READER_START_RECOMMENDATIONS_RECEIVE: | ||
return Object.assign( {}, state, keyBy( action.recommendations, 'ID' ) ); | ||
// Filter out any recommendations we already have | ||
const newRecommendations = filter( action.recommendations, ( incomingRecommendation ) => { | ||
return ! find( state, { ID: incomingRecommendation.ID } ); | ||
} ); | ||
|
||
// No new recommendations? Just return the existing ones. | ||
if ( newRecommendations.length < 1 ) { | ||
return state; | ||
} | ||
|
||
return state.concat( newRecommendations ); | ||
|
||
// Always return default state - we don't want to serialize recommendations | ||
case SERIALIZE: | ||
return state; | ||
case DESERIALIZE: | ||
if ( ! isValidStateWithSchema( state, itemsSchema ) ) { | ||
return {}; | ||
} | ||
return state; | ||
return []; | ||
} | ||
|
||
return state; | ||
} | ||
|
||
|
@@ -62,7 +72,21 @@ export function isRequestingRecommendations( state = false, action ) { | |
return state; | ||
} | ||
|
||
export function recommendationsInteractedWith( state = [], action ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is the order of interaction important? Could this just be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not in the UI, but it's pretty useful for debugging at the moment :) I'll leave it as an array for now. |
||
switch ( action.type ) { | ||
case READER_START_RECOMMENDATION_INTERACTION: | ||
return union( state, [ action.recommendationId ] ); | ||
|
||
case SERIALIZE: | ||
case DESERIALIZE: | ||
return []; | ||
} | ||
|
||
return state; | ||
} | ||
|
||
export default combineReducers( { | ||
items, | ||
isRequestingRecommendations | ||
isRequestingRecommendations, | ||
recommendationsInteractedWith | ||
} ); |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
number
doesn't currently do anything in the API endpoint FYI.The
LIMIT
in the SQL query is currently hard-coded to 20.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
number=
is supported now (thanks for adding @TooTallNate!)