LBRYCommunity/spee.ch
1
0
Fork 0
Code Issues 95 Pull requests 2 Projects Releases 4 Packages Wiki Activity

React router #343

Merged
bones7242 merged 96 commits from react-router into master 2018-02-15 08:02:17 +01:00
Conversation 3 Commits 96 Files changed 111 +3 -17
2 changed files with 3 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 7b4b5da428 - Show all commits

14
helpers/errorHandlers.js
View file

@ -7,20 +7,6 @@ module.exports = {
if (error.code === 'ECONNREFUSED') { if (error.code === 'ECONNREFUSED') {
status = 200; status = 200;
message = 'Connection refused. The daemon may not be running.'; message = 'Connection refused. The daemon may not be running.';
// // check for errors from the daemon
// } else if (error.response) {
// status = error.response.status || 500;
// if (error.response.data) {
// if (error.response.data.message) {
// message = error.response.data.message;
// } else if (error.response.data.error) {
// message = error.response.data.error.message;
// } else {
// message = error.response.data;
// }
// } else {
// message = error.response;
// }
// check for thrown errors // check for thrown errors
} else if (error.message) { } else if (error.message) {
status = 200; status = 200;

6
react/containers/ShowAsset/view.jsx
View file

@ -21,11 +21,11 @@ class ShowAsset extends React.Component {
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
// create request params // create request params
let body = {}; let body = {};
if (modifier) { if (modifier) {
if (modifier.channel) { if (modifier.id) {
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
body['claimId'] = modifier.id;
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
} else {
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
body['channelName'] = modifier.channel.name; body['channelName'] = modifier.channel.name;
body['channelClaimId'] = modifier.channel.id; body['channelClaimId'] = modifier.channel.id;
} else {
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
body['claimId'] = modifier.id;
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
} }
} }
body['claimName'] = name; body['claimName'] = name;

neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.
neb-b commented 2018-02-13 06:13:36 +01:00 (Migrated from github.com)
Review
Copy link

I think you are still creating more work than necessary with this. In my opinion previousRequest shouldn't even exist. In the mapStateToProps you should be able to map the asset from your state into the component. If !asset then make the request.

I also think onShowNewAsset and onNewRequest can be combined. More specifically I don't think onShowNewAsset is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".

I think you are still creating more work than necessary with this. In my opinion `previousRequest` shouldn't even exist. In the `mapStateToProps` you should be able to map the `asset` from your state into the component. If `!asset` then make the request. I also think `onShowNewAsset` and `onNewRequest` can be combined. More specifically I don't think `onShowNewAsset` is needed. It might just be my lack of understanding with the current data flow, but you shouldn't need to manually say "show this asset". A better approach would be "select the asset with this id".
bones7242 commented 2018-02-14 02:17:20 +01:00 (Migrated from github.com)
Review
Copy link

Ok, I think I'm getting closer. I was able to do away with onShowNewAsset and combine the needed logic from its action (retrieving the asset's claim data) into onNewRequest. That allowed me to remove previousRequest from the props I am passing to the <ShowAsset /> component. However, I am still checking for a previousRequest in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full claimId from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.

Ok, I think I'm getting closer. I was able to do away with `onShowNewAsset` and combine the needed logic from its action (retrieving the asset's claim data) into `onNewRequest`. That allowed me to remove `previousRequest` from the props I am passing to the `<ShowAsset />` component. However, I am still checking for a `previousRequest` in the mapStateToProps function. Do you see a way to avoid that step altogether? The reason for storing and checking the previous requests is to avoid having to retrieve new information for a request that was already made (i.e. to avoid having to request the full `claimId` from the server). I'm trying to figure out if that can be skipped or consolidated, but I am not sure how.