Skip to content
This repository was archived by the owner on Dec 11, 2019. It is now read-only.

Adds promo #13667

Merged
merged 1 commit into from
Apr 5, 2018
Merged

Adds promo #13667

merged 1 commit into from
Apr 5, 2018

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Mar 30, 2018

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

  • create folder brave-development and add promoCode file
  • start browser and make sure that new tab with promo page is opened
  • visit partner page and make sure that partner header is in the request via dev tools

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@codecov-io
Copy link

codecov-io commented Mar 30, 2018

Codecov Report

Merging #13667 into master will increase coverage by 0.09%.
The diff coverage is 71.17%.

@@            Coverage Diff             @@
##           master   #13667      +/-   ##
==========================================
+ Coverage   56.65%   56.74%   +0.09%     
==========================================
  Files         285      285              
  Lines       28920    29038     +118     
  Branches     4782     4803      +21     
==========================================
+ Hits        16385    16479      +94     
- Misses      12535    12559      +24
Flag Coverage Δ
#unittest 56.74% <71.17%> (+0.09%) ⬆️
Impacted Files Coverage Δ
js/constants/appConstants.js 100% <ø> (ø) ⬆️
js/actions/appActions.js 19.03% <100%> (+0.43%) ⬆️
app/browser/reducers/ledgerReducer.js 43.59% <16.66%> (-0.3%) ⬇️
app/browser/api/ledger.js 61.92% <67.92%> (+0.31%) ⬆️
app/siteHacks.js 75.75% <74.72%> (+26.82%) ⬆️
app/browser/reducers/tabsReducer.js 40.05% <77.77%> (+0.05%) ⬆️
js/stores/appStoreRenderer.js 91.66% <0%> (-8.34%) ⬇️
app/renderer/components/reduxComponent.js 57.75% <0%> (-3.45%) ⬇️
js/stores/windowStore.js 28.46% <0%> (-0.3%) ⬇️
... and 2 more

@NejcZdovc NejcZdovc self-assigned this Apr 1, 2018
@NejcZdovc NejcZdovc force-pushed the feature/promo branch 2 times, most recently from 8c0c3e7 to 6bfd999 Compare April 1, 2018 09:51
@NejcZdovc NejcZdovc force-pushed the feature/promo branch 4 times, most recently from 4f8afbd to cf3babb Compare April 2, 2018 16:06
@bsclifton bsclifton added this to the 0.22.x Release 2 (Beta Channel) milestone Apr 2, 2018
@NejcZdovc NejcZdovc force-pushed the feature/promo branch 2 times, most recently from ce5601e to c44ea1e Compare April 5, 2018 13:12
@NejcZdovc NejcZdovc requested review from bsclifton and diracdeltas and removed request for bsclifton April 5, 2018 13:40
@NejcZdovc NejcZdovc force-pushed the feature/promo branch 2 times, most recently from f592a7e to 98ddc15 Compare April 5, 2018 19:43
Auditors:

Test Plan:
Copy link

@tildelowengrimm tildelowengrimm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review is complete in brave/internal#250. r+

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

@bsclifton bsclifton merged commit f8c2de5 into brave:master Apr 5, 2018
bsclifton added a commit that referenced this pull request Apr 5, 2018
bsclifton added a commit that referenced this pull request Apr 5, 2018
bsclifton added a commit that referenced this pull request Apr 5, 2018
@bsclifton
Copy link
Member

master f8c2de5
0.23.x 0a7a128
0.22.x e5dd36a
0.22.x-promo 08b54da

if (!mainFrameUrl) {
return {
resourceName
const domain = urlParse(mainFrameUrl).hostname.split('.').slice(-2).join('.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't work for domains that have subdomains, ex: play.spotify.com

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use getBaseDomain to get the full domain

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

though it looks like it also does this on master

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this code is on the master as well, where I only moved this code into named funtion

bsclifton added a commit that referenced this pull request Apr 12, 2018
@NejcZdovc
Copy link
Contributor Author

NejcZdovc commented Apr 18, 2018

0.22.x-c66 2e1adbd

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants