Reddit inline tips link on post menu will now appear #2665

jasonrsadler · 2019-06-11T18:27:48Z

Fixes brave/brave-browser#4915

brave-ui: brave/brave-ui#498 brave/brave-ui#501

Submitter Checklist:

Test Plan:

Visit reddit.com with Rewards enabled and ensure that tip link shows on reddit posts' flat menu
Go to rewards settings->tip settings and uncheck the box for 'reddit' and ensure that inline tip links do not show on reddit.
Ensure tipping banner is correct when using reddit inline tipping link.

Reviewer Checklist:

New files have MPL-2.0 license header.
Request a security/privacy review as needed.
Adequate test coverage exists to prevent regressions
Verify test plan is specified in PR before merging to source

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on.
All relevant documentation has been updated.

vendor/bat-native-ledger/src/bat/ledger/internal/media/reddit.cc

NejcZdovc · 2019-06-20T15:40:12Z

@jasonrsadler I asked for refactor and I stopped at that point. I didn't do a full review, sorry other things are going on. Today I did a full review

jasonrsadler · 2019-06-20T16:22:41Z

Thanks. refactor was pushed to post PR as was discussed before your original review so not sure why review here was partial. Understand lots going on.

vendor/bat-native-ledger/src/bat/ledger/internal/media/reddit.cc

NejcZdovc · 2019-06-21T08:39:11Z

components/brave_rewards/resources/tip/components/tipRedditUser.tsx

+        redditMetaData.postText.length > 0
+        ? 'redditTipTitle'
+        : 'redditTipTitleEmpty'
+    publisher.title = 'u/' + getLocale(key, { user: redditMetaData.userName })


shouldn't this be like publisher.title = getLocale(key, { user: 'u/' + redditMetaData.userName })?

NejcZdovc · 2019-06-21T08:47:02Z

@jasonrsadler are we not displaying reddit box for posts? Is this intentionally?

Post:

Banner:

NejcZdovc · 2019-06-21T08:50:02Z

@jasonrsadler we need tweet out on thank you page on reddit as well, like we have it on twitter tips, right?

If I tip through the panel I see tweet box

NejcZdovc · 2019-06-21T09:29:33Z

vendor/bat-native-ledger/src/bat/ledger/internal/media/reddit.cc

+  }
+  GURL url(REDDIT_USER_URL + ledger_->URIEncode(user_name));
+  if (!url.is_valid()) {
+    callback(ledger::Result::TIP_ERROR, std::move(publisher_info));


should we return info here or just null if we have an error?

based on other callbacks with errors I would say just return nullptr

NejcZdovc · 2019-06-21T09:30:49Z

vendor/bat-native-ledger/src/bat/ledger/internal/media/reddit.cc

+  const std::string favicon_url = GetProfileImageUrl(data);
+  const std::string media_key = GetMediaKey(user_name, REDDIT_MEDIA_TYPE);
+
+  if (publisher_key.empty()) {


nit: maybe try to exit as soon as possible

So when we get publisher_key on line 320 we check if valid, before continue parsing other things

NejcZdovc · 2019-06-21T09:31:53Z

vendor/bat-native-ledger/src/bat/ledger/internal/media/reddit.cc

+  ledger_->SaveMediaVisit(publisher_key,
+                          *visit_data,
+                          0,
+                          0,


we need to pass window_id into SavePublisherInfo so that correct panel is updated

jasonrsadler · 2019-06-21T11:37:14Z

For posts, there was a dom change. That is fixed.

For tweet on tip, the original thinking was that it wouldn't be available for inline reddit. However, never got an official word so it is restored.

jasonrsadler force-pushed the reddit-inline branch 3 times, most recently from 2befd94 to 1da9930 Compare June 11, 2019 18:34

mihaiplesa added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Jun 11, 2019

jasonrsadler force-pushed the reddit-inline branch 13 times, most recently from 40d333e to 225ccb9 Compare June 13, 2019 13:26

jasonrsadler self-assigned this Jun 13, 2019

jasonrsadler force-pushed the reddit-inline branch 4 times, most recently from 722cae1 to e2a818d Compare June 13, 2019 20:25

jasonrsadler mentioned this pull request Jun 14, 2019

Twitter metadata needs to be generalized for other media publishers brave/brave-browser#4907

Closed

jasonrsadler force-pushed the reddit-inline branch from e2a818d to d02d8f5 Compare June 14, 2019 00:57

jasonrsadler removed CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Jun 14, 2019

jasonrsadler force-pushed the reddit-inline branch from 982ca24 to d1f861f Compare June 20, 2019 13:41

NejcZdovc reviewed Jun 20, 2019

View reviewed changes

vendor/bat-native-ledger/src/bat/ledger/internal/media/reddit.cc Show resolved Hide resolved

jasonrsadler force-pushed the reddit-inline branch 2 times, most recently from db8f9a5 to 67c7aa3 Compare June 20, 2019 16:17

jasonrsadler requested a review from NejcZdovc June 20, 2019 16:24

jasonrsadler force-pushed the reddit-inline branch from 67c7aa3 to 7aedb98 Compare June 20, 2019 17:11

NejcZdovc reviewed Jun 20, 2019

View reviewed changes

vendor/bat-native-ledger/src/bat/ledger/internal/media/reddit.cc Show resolved Hide resolved

jasonrsadler force-pushed the reddit-inline branch 2 times, most recently from 13bb864 to 49d7004 Compare June 21, 2019 01:45

jasonrsadler requested a review from NejcZdovc June 21, 2019 01:46

jasonrsadler force-pushed the reddit-inline branch from eaa3a4f to 93fc6aa Compare June 21, 2019 02:00

NejcZdovc reviewed Jun 21, 2019

View reviewed changes

jasonrsadler force-pushed the reddit-inline branch 2 times, most recently from 61b65e1 to f9ad478 Compare June 21, 2019 11:35

jasonrsadler requested a review from NejcZdovc June 21, 2019 11:37

Fixes brave/brave-browser#4915

2c44c08

jasonrsadler force-pushed the reddit-inline branch from f9ad478 to 2c44c08 Compare June 21, 2019 16:17

NejcZdovc approved these changes Jun 21, 2019

View reviewed changes

jasonrsadler merged commit e14f2a6 into master Jun 21, 2019

NejcZdovc removed the PR/Pending Review label Jun 21, 2019

jasonrsadler deleted the reddit-inline branch June 21, 2019 20:01

GeetaSarvadnya mentioned this pull request Jul 24, 2019

Implement Reddit inline tipping for flat menu brave/brave-browser#4915

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reddit inline tips link on post menu will now appear #2665

Reddit inline tips link on post menu will now appear #2665

jasonrsadler commented Jun 11, 2019 •

edited

Loading

NejcZdovc commented Jun 20, 2019

jasonrsadler commented Jun 20, 2019 •

edited

Loading

NejcZdovc Jun 21, 2019

NejcZdovc Jun 21, 2019

jasonrsadler Jun 21, 2019

NejcZdovc commented Jun 21, 2019 •

edited

Loading

NejcZdovc commented Jun 21, 2019 •

edited

Loading

NejcZdovc Jun 21, 2019

NejcZdovc Jun 21, 2019

jasonrsadler Jun 21, 2019

NejcZdovc Jun 21, 2019

jasonrsadler Jun 21, 2019

NejcZdovc Jun 21, 2019

jasonrsadler Jun 21, 2019

jasonrsadler commented Jun 21, 2019

Reddit inline tips link on post menu will now appear #2665

Reddit inline tips link on post menu will now appear #2665

Conversation

jasonrsadler commented Jun 11, 2019 • edited Loading

Submitter Checklist:

Test Plan:

Reviewer Checklist:

After-merge Checklist:

NejcZdovc commented Jun 20, 2019

jasonrsadler commented Jun 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NejcZdovc commented Jun 21, 2019 • edited Loading

NejcZdovc commented Jun 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonrsadler commented Jun 21, 2019

jasonrsadler commented Jun 11, 2019 •

edited

Loading

jasonrsadler commented Jun 20, 2019 •

edited

Loading

NejcZdovc commented Jun 21, 2019 •

edited

Loading

NejcZdovc commented Jun 21, 2019 •

edited

Loading