LBRYCommunity/lbryinc
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity

Common subscriptions implementation #19

Merged
akinwale merged 9 commits from subscriptions into master 2019-02-15 10:02:35 +01:00
Conversation 2 Commits 9 Files changed 13 +8366 -8
akinwale commented 2018-12-28 14:23:48 +01:00 (Migrated from github.com)
Copy link
No description provided.
skhameneh (Migrated from github.com) reviewed 2019-01-03 02:29:23 +01:00
package.json
@ -29,6 +29,7 @@
},
"dependencies": {
"lbry-redux": "lbryio/lbry-redux#84b7d396934d57a37802aadbef71db91230a9404",
"bluebird": "^3.5.1",
skhameneh (Migrated from github.com) commented 2019-01-03 02:28:30 +01:00
Copy link

What does Bluebird provide that native promises and async/await don't?
It looks like much of Bluebird was not developed with async/await in mind.

I do not see Bluebird utilized in this PR

What does Bluebird provide that native promises and `async`/`await` don't? It looks like much of Bluebird was not developed with `async`/`await` in mind. **I do not see Bluebird utilized in this PR**
skhameneh (Migrated from github.com) reviewed 2019-01-03 02:31:19 +01:00
src/constants/action_types.js
@ -3,1 +4,4 @@
export const GENERATE_AUTH_TOKEN_SUCCESS = 'GENERATE_AUTH_TOKEN_SUCCESS';
// Claims
export const FETCH_FEATURED_CONTENT_STARTED = 'FETCH_FEATURED_CONTENT_STARTED';
skhameneh (Migrated from github.com) commented 2019-01-03 02:30:41 +01:00
Copy link

We have so many of these repeated, we should consider using generators for _STARTED/_COMPLETED/_FAILED

We have so many of these repeated, we should consider using generators for `_STARTED`/`_COMPLETED`/`_FAILED`
skhameneh (Migrated from github.com) reviewed 2019-01-03 02:33:23 +01:00
src/redux/selectors/subscriptions.js
@ -0,0 +48,4 @@
// We also only care about top_subscribed and featured
// top_bid could just be porn or a channel with no content
const topSubscribedSuggestions = swapKeyAndValue(suggested[SUGGESTED_TOP_SUBSCRIBED]);
skhameneh (Migrated from github.com) commented 2019-01-03 02:33:19 +01:00
Copy link

I need to take a closer look at why swapKeyAndValue is required in this context for sorting, I will revisit this PR

I need to take a closer look at why `swapKeyAndValue` is required in this context for sorting, I will revisit this PR
neb-b commented 2019-01-03 17:34:31 +01:00 (Migrated from github.com)
Copy link

The app currently uses it's own doFetchClaimsByChannel to set the latest piece of content for a channel. We use that to know if we should download new content for a user.

https://github.com/lbryio/lbry-desktop/blob/master/src/renderer/redux/actions/content.js#L289-L331

Maybe we should create a new function in lbryinc that uses doFetchClaimsByChannel from lbry-redux with a callback, then sets the subscription? I'm not sure of the best way to do it.

// lbry-redux
export function doFetchClaimsByChannel(uri, page, pageSize, cb) { 
...
if (cb) cb(latestContent)
}

// lbryinc
export function doFetchClaimsByChannelAndUpdateSubscriptions(uri, page, pageSize) {
  doFetchClaimsByChannel(uri, page, pageSize, (latestContent) => {
       setSubscriptionLatest(latestcontent);
  }
}

Also, does android even need to do this? I know you are more pressed on storage than the desktop app is.

The app currently uses it's own `doFetchClaimsByChannel` to set the latest piece of content for a channel. We use that to know if we should download new content for a user. https://github.com/lbryio/lbry-desktop/blob/master/src/renderer/redux/actions/content.js#L289-L331 Maybe we should create a new function in `lbryinc` that uses `doFetchClaimsByChannel` from `lbry-redux` with a callback, then sets the subscription? I'm not sure of the best way to do it. ``` // lbry-redux export function doFetchClaimsByChannel(uri, page, pageSize, cb) { ... if (cb) cb(latestContent) } // lbryinc export function doFetchClaimsByChannelAndUpdateSubscriptions(uri, page, pageSize) { doFetchClaimsByChannel(uri, page, pageSize, (latestContent) => { setSubscriptionLatest(latestcontent); } } ``` Also, does android even need to do this? I know you are more pressed on storage than the desktop app is.
akinwale (Migrated from github.com) reviewed 2019-01-04 15:24:20 +01:00
package.json
@ -29,6 +29,7 @@
},
"dependencies": {
"lbry-redux": "lbryio/lbry-redux#84b7d396934d57a37802aadbef71db91230a9404",
"bluebird": "^3.5.1",
akinwale (Migrated from github.com) commented 2019-01-04 15:24:19 +01:00
Copy link

It's used in src/redux/actions/subscriptions.js.

It's used in `src/redux/actions/subscriptions.js`.
akinwale (Migrated from github.com) reviewed 2019-01-04 15:25:12 +01:00
src/constants/action_types.js
@ -3,1 +4,4 @@
export const GENERATE_AUTH_TOKEN_SUCCESS = 'GENERATE_AUTH_TOKEN_SUCCESS';
// Claims
export const FETCH_FEATURED_CONTENT_STARTED = 'FETCH_FEATURED_CONTENT_STARTED';
akinwale (Migrated from github.com) commented 2019-01-04 15:25:12 +01:00
Copy link

These were taken directly from the lbry-desktop repo. I suppose we could address these changes in a future PR.

These were taken directly from the `lbry-desktop` repo. I suppose we could address these changes in a future PR.
akinwale (Migrated from github.com) reviewed 2019-01-04 15:26:18 +01:00
src/redux/selectors/subscriptions.js
@ -0,0 +48,4 @@
// We also only care about top_subscribed and featured
// top_bid could just be porn or a channel with no content
const topSubscribedSuggestions = swapKeyAndValue(suggested[SUGGESTED_TOP_SUBSCRIBED]);
akinwale (Migrated from github.com) commented 2019-01-04 15:26:18 +01:00
Copy link

@seanyesmunt Any insight on this one? It's from the lbry-desktop repo, so I assume there's a reason this is being used.

@seanyesmunt Any insight on this one? It's from the `lbry-desktop` repo, so I assume there's a reason this is being used.
neb-b (Migrated from github.com) reviewed 2019-01-04 16:12:37 +01:00
src/redux/selectors/subscriptions.js
@ -0,0 +48,4 @@
// We also only care about top_subscribed and featured
// top_bid could just be porn or a channel with no content
const topSubscribedSuggestions = swapKeyAndValue(suggested[SUGGESTED_TOP_SUBSCRIBED]);
neb-b (Migrated from github.com) commented 2019-01-04 16:12:37 +01:00
Copy link

The comment is pretty much all there is to it. Swapping the value and key so uri was the key made it a lot simpler (but still confusing) because I could just check if the subscription existed with

alreadySubscribedToSuggestedSubscription = topSubscribedSuggestions[uri]

Without swapping I wasn't sure how to do that without a bunch of loops every time I needed to check.

The comment is pretty much all there is to it. Swapping the value and key so `uri` was the key made it a lot simpler (but still confusing) because I could just check if the subscription existed with ``` alreadySubscribedToSuggestedSubscription = topSubscribedSuggestions[uri] ``` Without swapping I wasn't sure how to do that without a bunch of loops every time I needed to check.
neb-b (Migrated from github.com) reviewed 2019-01-04 16:13:49 +01:00
src/redux/selectors/subscriptions.js
@ -0,0 +48,4 @@
// We also only care about top_subscribed and featured
// top_bid could just be porn or a channel with no content
const topSubscribedSuggestions = swapKeyAndValue(suggested[SUGGESTED_TOP_SUBSCRIBED]);
neb-b (Migrated from github.com) commented 2019-01-04 16:13:49 +01:00
Copy link

Very much open to new ideas/reworking this if you know a simpler way.

Very much open to new ideas/reworking this if you know a simpler way.
neb-b (Migrated from github.com) approved these changes 2019-01-25 20:04:53 +01:00
neb-b commented 2019-01-25 20:55:52 +01:00 (Migrated from github.com)
Copy link

Just realized there are a couple things commented out. I will update the app/fix these lines so that they are only performed based on params passed in.

Just realized there are a couple things commented out. I will update the app/fix these lines so that they are only performed based on params passed in.
Sign in to join this conversation.
Reviewers
No reviewers
neb-b
Labels
Clear labels   
area: devops

   
area: discovery

   
area: docs

   
area: livestream

   
area: proposal

Work is part of a proposal

  
consider soon

Discuss this issue at the next planning meeting, then remove this label

  
dependencies

Pull requests that update a dependency file

  
Epic

   
good first issue

Good for newcomers

  
hacktoberfest

Welcome to Hacktoberfest

  
help wanted

Extra attention is needed

  
icebox

Long-term storage

  
level: 1

No knowledge of the existing code required

  
level: 2

Some knowledge of the existing code is recommended

  
level: 3

Significant knowledge of the existing code is recommended

  
level: 4

Intimate knowledge of the existing code is recommended

  
needs: exploration

Solution unclear, needs research

  
needs: grooming

Issue needs to be groomed before work can start

  
needs: priority

Priority level needs to be defined

  
needs: repro

Issue needs step-by-step instructions on how to reproduce in latest code

  
needs: tech design

Needs technical design signoff before implementation can begin

  
on hold

Temporarily paused

  
priority: blocker

Issue is blocking release, do ASAP

  
priority: high

Work needs to be moved into sprint ASAP

  
priority: low

Work should be done but can stay in the backlog for now

  
priority: medium

Work needs to be done within 2-3 sprints

  
resilience

   
Tom's Wishlist

   
type: bug

Existing functionality is wrong or broken

  
type: discussion

A conversation. No specific changes requested

  
type: improvement

Existing (or partially existing) functionality needs to be changed

  
type: new feature

New functionality that does not exist yet

  
type: refactor

Minimal user-visible changes, but significant internal work

  
type: task

Either work that's not related to the code, or a small chore that does not fit into other categories

  
type: testing

Solution needs additional user testing

  
unplanned

Work that was not planned into the spirnt, took priority over planned work

No labels
area: devops
area: discovery
area: docs
area: livestream
area: proposal
consider soon
dependencies
Epic
good first issue
hacktoberfest
help wanted
icebox
level: 1
level: 2
level: 3
level: 4
needs: exploration
needs: grooming
needs: priority
needs: repro
needs: tech design
on hold
priority: blocker
priority: high
priority: low
priority: medium
resilience
Tom's Wishlist
type: bug
type: discussion
type: improvement
type: new feature
type: refactor
type: task
type: testing
unplanned
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbryinc#19
No description provided.
Delete branch "subscriptions"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?