fetch featured channels #1064

Closed
daovist wants to merge 22 commits from featured-channels into master
6 changed files with 19244 additions and 22 deletions

19155
package-lock.json generated Normal file

File diff suppressed because it is too large Load diff

View file

@ -33,7 +33,9 @@ class UserPhoneVerify extends React.PureComponent {
<Form onSubmit={this.handleSubmit.bind(this)}>
<p>
{__(
`Please enter the verification code sent to +${countryCode}${phone}. Didn't receive it? `
`Please enter the verification code sent to +${countryCode}${
phone
}. Didn't receive it? `
)}
<Link onClick={this.reset.bind(this)} label="Go back." />
</p>

View file

@ -2,11 +2,18 @@ import React from 'react';
import { connect } from 'react-redux';
import { doFetchFeaturedUris } from 'redux/actions/content';
import { selectFeaturedUris, selectFetchingFeaturedUris } from 'redux/selectors/content';
liamcardenas commented 2018-03-15 21:38:09 +01:00 (Migrated from github.com)
Review

you don't use this

you don't use this
import { selectClaimsById, selectAllClaimsByChannel } from 'redux/selectors/claims';
import { selectDiscover } from 'redux/selectors/discover';
import DiscoverPage from './view';
const select = state => ({
featuredUris: selectFeaturedUris(state),
fetchingFeaturedUris: selectFetchingFeaturedUris(state),
categories: selectDiscover(state),
// we shouldn't need these
featuredUris: selectFeaturedUris(state),
liamcardenas commented 2018-03-27 19:47:25 +02:00 (Migrated from github.com)
Review

if we don't need them, can you take them out?

if we don't need them, can you take them out?
claimsById: selectClaimsById(state),
claimsByChannel: selectAllClaimsByChannel(state),
});
const perform = dispatch => ({

View file

@ -222,13 +222,21 @@ export class FeaturedCategory extends React.PureComponent {
}
class DiscoverPage extends React.PureComponent {
constructor(props) {
kauffj commented 2018-03-08 23:54:42 +01:00 (Migrated from github.com)
Review

Can this logic be either moved into <FeaturedCategory> and/or moved into a selector that is called by <FeaturedCategory>?

Can this logic be either moved into `<FeaturedCategory>` and/or moved into a selector that is called by `<FeaturedCategory>`?
daovist commented 2018-03-09 15:01:30 +01:00 (Migrated from github.com)
Review

I moved all this logic into selectDiscover and am calling it from the Discover page and it's working correctly. Calling it from <FeaturedCategory> doesn't make sense to me because the selector returns an object which is mapped to many <FeaturedCategory>s in the view.

I can kind of see doing to logic for each category separately but I would need to think about it and it doesn't seem like it will be nearly as clean as this.

I moved all this logic into `selectDiscover` and am calling it from the `Discover` page and it's working correctly. Calling it from `<FeaturedCategory>` doesn't make sense to me because the selector returns an object which is mapped to many `<FeaturedCategory>`s in the view. I can kind of see doing to logic for each category separately but I would need to think about it and it doesn't seem like it will be nearly as clean as this.
super(props);
}
componentWillMount() {
this.props.fetchFeaturedUris();
}
render() {
const { featuredUris, fetchingFeaturedUris } = this.props;
const hasContent = typeof featuredUris === 'object' && Object.keys(featuredUris).length,
const {
fetchingFeaturedUris,
categories
} = this.props;
const hasContent = typeof categories === 'object' && Object.keys(categories).length,
failedToLoad = !fetchingFeaturedUris && !hasContent;
return (
@ -240,14 +248,23 @@ class DiscoverPage extends React.PureComponent {
<SubHeader fullWidth smallMargin />
{!hasContent && fetchingFeaturedUris && <BusyMessage message={__('Fetching content')} />}
{hasContent &&
Object.keys(featuredUris).map(
Object.keys(categories).map(
category =>
featuredUris[category].length ? (
<FeaturedCategory
key={category}
category={category}
names={featuredUris[category]}
/>
categories[category].length ? (
category.indexOf('@') === 0 ? (
<FeaturedCategory
key={category}
category={category.split('#')[0]}
categoryLink={category}
names={categories[category]}
/>
) : (
<FeaturedCategory
key={category}
category={category}
names={categories[category]}
/>
)
) : (
''
)

View file

@ -78,21 +78,25 @@ export function doFetchFeaturedUris() {
});
const success = ({ Uris }) => {
Uris['@LupoTV'] = [];
liamcardenas commented 2018-03-27 19:23:58 +02:00 (Migrated from github.com)
Review

I appreciate you leaving this in for testing purposes, obviously remove this when it is ready to merge

I appreciate you leaving this in for testing purposes, obviously remove this when it is ready to merge
let urisToResolve = [];
let actions = [];
liamcardenas commented 2018-03-27 19:16:17 +02:00 (Migrated from github.com)
Review

use const, when possible

use const, when possible
Object.keys(Uris).forEach(category => {
urisToResolve = [...urisToResolve, ...Uris[category]];
if (category.indexOf('@') === 0 && !Uris[category].length) {
actions.push(doFetchClaimsByChannel(category, 1));
} else {
urisToResolve = [...urisToResolve, ...Uris[category]];
}
});
const actions = [
doResolveUris(urisToResolve),
{
type: ACTIONS.FETCH_FEATURED_CONTENT_COMPLETED,
data: {
uris: Uris,
success: true,
},
actions.push(doResolveUris(urisToResolve));
actions.push({
type: ACTIONS.FETCH_FEATURED_CONTENT_COMPLETED,
data: {
uris: Uris,
success: true,
},
];
});
dispatch(batchActions(...actions));
};
@ -277,7 +281,9 @@ export function doLoadVideo(uri) {
});
dispatch(
doAlertError(
`Failed to download ${uri}, please try again. If this problem persists, visit https://lbry.io/faq/support for support.`
`Failed to download ${
uri
}, please try again. If this problem persists, visit https://lbry.io/faq/support for support.`
)
);
});

View file

@ -0,0 +1,35 @@
import { createSelector } from 'reselect';
import { selectFeaturedUris } from './content';
import { selectClaimsById, selectAllClaimsByChannel } from './claims';
export const selectDiscover = createSelector(
[selectFeaturedUris, selectClaimsById, selectAllClaimsByChannel],
(featuredUris, claimsById, claimsByChannel) => {
let categories = featuredUris;
if (!!categories && !!claimsByChannel) {
liamcardenas commented 2018-03-27 19:27:18 +02:00 (Migrated from github.com)
Review

unnecessary to do !!

unnecessary to do `!!`
let channels = [];
Object.keys(categories).forEach(key => {
if (key.indexOf('@') === 0) {
channels.push(key);
}
});
Object.keys(claimsByChannel).forEach(key => {
if (channels.includes(key)) {
liamcardenas commented 2018-03-27 19:35:24 +02:00 (Migrated from github.com)
Review

you could take out the other forEach loop if you change this line to
if(categories[key].indexOf('@') === 0)

you could take out the other forEach loop if you change this line to `if(categories[key].indexOf('@') === 0)`
delete categories[key];
const ids = claimsByChannel[key][1];
let uris = [];
const newKey = `${key}#${claimsById[ids[0]].value.publisherSignature.certificateId}`;
liamcardenas commented 2018-03-27 19:37:27 +02:00 (Migrated from github.com)
Review

instead of using string interpolation, the proper way to do with is with the buildURI function

(I know I told you this method was fine earlier, sorry about that)

instead of using string interpolation, the proper way to do with is with the `buildURI` function (I know I told you this method was fine earlier, sorry about that)
ids.forEach(id => {
liamcardenas commented 2018-03-27 19:44:41 +02:00 (Migrated from github.com)
Review

instead of all of these loops, you should learn how to use map, reduce, filter, etc.

i dont think this would block merging, however, here is a rough sketch of how i would do this

uri = ids.filter(id => claimsById[id]).map(id => `${claimsById[id.name}#${claimsById[id.claim_id}`);

and in fact you could even set

categories[newKey] = ids.filter(id => claimsById[id]).map(id => `${claimsById[id.name}#${claimsById[id.claim_id}`);

this could cut out a lot of this code and make it shorter. It's something you should play around with.

instead of all of these loops, you should learn how to use `map`, `reduce`, `filter`, etc. i dont think this would block merging, however, here is a rough sketch of how i would do this ``` uri = ids.filter(id => claimsById[id]).map(id => `${claimsById[id.name}#${claimsById[id.claim_id}`); ``` and in fact you could even set ``` categories[newKey] = ids.filter(id => claimsById[id]).map(id => `${claimsById[id.name}#${claimsById[id.claim_id}`); ``` this could cut out a lot of this code and make it shorter. It's something you should play around with.
const claim = claimsById[id] ? claimsById[id] : null;
if (claim) {
uris.push(`${claim.name}#${claim.claim_id}`);
liamcardenas commented 2018-03-27 19:37:43 +02:00 (Migrated from github.com)
Review

same here, use buildURI

same here, use buildURI
}
});
categories[newKey] = uris;
}
});
}
console.log("select:", categories);
liamcardenas commented 2018-03-27 19:44:56 +02:00 (Migrated from github.com)
Review

obviously remove when ready to merge :)

obviously remove when ready to merge :)
return categories;
}
);