always update costInfo on FilePage componentDidMount #1563

Merged
daovist merged 2 commits from overtaken-claim-price into master 2018-06-08 20:57:47 +02:00
daovist commented 2018-06-07 20:15:44 +02:00 (Migrated from github.com)

#797 @tzarebczan

Call fetchCostInfo whenever <FilePage> mounts. All fetchCostInfo does is update costInfo based on fileInfo; there is no api call. So running it whenever a user visits a claim is not a problem where fetching fileInfo from the daemon could slow things down significantly.

I don't like this solution but refactoring everything about costInfo would be more involved and there may be reasons for this design which are not apparent to me.

#797 @tzarebczan Call `fetchCostInfo` whenever `<FilePage>` mounts. All `fetchCostInfo` does is update `costInfo` based on `fileInfo`; there is no api call. So running it whenever a user visits a claim is not a problem where fetching `fileInfo` from the daemon could slow things down significantly. I don't like this solution but refactoring everything about `costInfo` would be more involved and [there may be reasons for this design which are not apparent to me](https://en.wikipedia.org/wiki/Wikipedia:Chesterton's_fence).
neb-b (Migrated from github.com) reviewed 2018-06-07 20:15:44 +02:00
kauffj commented 2018-06-07 21:52:17 +02:00 (Migrated from github.com)

Chesterson's Fence applies very much to programming :)

The reason for this code was in the original (and likely future) model, there may be an additional cost for data separate from the cost of the content itself.

This feature was turned off since data is free in 99.9+% of cases.

It's debatable whether it's worth retaining this since we expect data fees to return or to just remove it and re-add it when they do.

(@seanyesmunt can make that call)

Chesterson's Fence applies very much to programming :) The reason for this code was in the original (and likely future) model, there may be an additional cost for data separate from the cost of the content itself. This feature was turned off since data is free in 99.9+% of cases. It's debatable whether it's worth retaining this since we expect data fees to return or to just remove it and re-add it when they do. (@seanyesmunt can make that call)
neb-b commented 2018-06-08 20:28:50 +02:00 (Migrated from github.com)

I think this is ok for now. It fixes the bug and is a small enough change that it will be easy to change in the future. If there are any larger changes related to this, then we may want to go a different route. I will add a comment that links to this PR/discussion above this line.

I think this is ok for now. It fixes the bug and is a small enough change that it will be easy to change in the future. If there are any larger changes related to this, then we may want to go a different route. I will add a comment that links to this PR/discussion above this line.
Sign in to join this conversation.
No reviewers
No milestone
No project
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/lbry-desktop#1563
No description provided.