Reddit users can now receive direct contributions (tips) via Brave Rewards panel #2624
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jasonrsadler
force-pushed
the
reddit_panel
branch
2 times, most recently
from
6be111d to
9be02c0
Compare
jasonrsadler
requested review from
NejcZdovc,
emerick,
SergeyZhukovsky,
ryanml and
tmancey
jasonrsadler
force-pushed
the
reddit_panel
branch
from
9be02c0 to
208ae86
Compare
jasonrsadler
changed the title
emerick
reviewed
Jun 6, 2019
test/BUILD.gn
Outdated
| @@ -118,6 +118,7 @@ test("brave_unit_tests") { | |||
| if (brave_rewards_enabled) { | |||
| sources += [ | |||
| "//brave/vendor/bat-native-ledger/src/bat/ledger/internal/media/helper_unittest.cc", | |||
| "//brave/vendor/bat-native-ledger/src/bat/ledger/internal/media/reddit_unittest.cc", | |||
There was a problem hiding this comment.
Nit: Sometimes hard to tell in a diff, but looks like extra spacing at the beginning of this line.
There was a problem hiding this comment.
There was. Removed.
| // Canonicalize away 'old.reddit.com' and replace with 'www.reddit.com'. | ||
| std::string new_host = reddit_url.host().substr( | ||
| 0, reddit_url.host().length()); | ||
| new_host.replace(new_host.find("old"), 3, "www"); |
There was a problem hiding this comment.
Do we need to call find here? Seems like it would always return 0.
There was a problem hiding this comment.
We do not. Removed.
| GURL reddit_url(url); | ||
| if (reddit_url.DomainIs(OLD_REDDIT_DOMAIN)) { | ||
| // Canonicalize away 'old.reddit.com' and replace with 'www.reddit.com'. | ||
| std::string new_host = reddit_url.host().substr( |
There was a problem hiding this comment.
Can't you just do a direct assignment here?
std::string new_host = reddit_url.host();
There was a problem hiding this comment.
Yes.
Thanks for the catches.
NejcZdovc
reviewed
Jun 6, 2019
| std::string url = REDDIT_TLD; | ||
| std::string name = REDDIT_MEDIA_TYPE; | ||
|
|
||
| if (!url.empty()) { |
There was a problem hiding this comment.
nit: switch if and return, so that we don't have this big if
NejcZdovc
reviewed
Jun 6, 2019
| void MediaReddit::OnMediaActivityError( | ||
| const ledger::VisitData& visit_data, | ||
| uint64_t window_id) { | ||
| std::string url = REDDIT_TLD; |
NejcZdovc
reviewed
Jun 6, 2019
| const ledger::VisitData& visit_data, | ||
| uint64_t window_id) { | ||
| std::string url = REDDIT_TLD; | ||
| std::string name = REDDIT_MEDIA_TYPE; |
NejcZdovc
reviewed
Jun 6, 2019
| void MediaReddit::UserPath( | ||
| uint64_t window_id, | ||
| const ledger::VisitData& visit_data) { | ||
| std::string user = GetUserNameFromUrl(visit_data.path); |
NejcZdovc
reviewed
Jun 6, 2019
| return; | ||
| } | ||
|
|
||
| std::string media_key = (std::string)REDDIT_MEDIA_TYPE + "_" + user; |
NejcZdovc
reviewed
Jun 6, 2019
| return std::string(); | ||
| } | ||
|
|
||
| std::vector<std::string> parts = base::SplitString( |
NejcZdovc
reviewed
Jun 6, 2019
| uint64_t window_id, | ||
| const ledger::VisitData& visit_data, | ||
| const std::string& publisher_key) { | ||
| auto filter = ledger_->CreateActivityFilter( |
jasonrsadler
force-pushed
the
reddit_panel
branch
from
208ae86 to
164f16b
Compare
jasonrsadler
force-pushed
the
reddit_panel
branch
from
164f16b to
7c46c4c
Compare
|
Updated with merged |
emerick
previously approved these changes
Jun 6, 2019
jasonrsadler
force-pushed
the
reddit_panel
branch
from
7c46c4c to
e3006e9
Compare
jasonrsadler
force-pushed
the
reddit_panel
branch
from
e3006e9 to
47b456a
Compare
|
Removed package-lock |
This was referenced Jun 14, 2019
fmarier
pushed a commit
that referenced
this pull request
Oct 29, 2019
Jenkins pipeline for building PRs on Linux
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes brave/brave-browser#4745
Brave-UI: brave/brave-ui#495
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests && npm run test-security) onnpm run lint)git rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
old.reddit.comRepeat all steps above for unverified publishers and ensure panel shows correctly (e. g. standard reddit icon, etc)
Reviewer Checklist:
After-merge Checklist:
changes has landed on.