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

Implemented fix to show publisher toggle for PDFs #14028

Merged
merged 1 commit into from
May 4, 2018
Merged

Implemented fix to show publisher toggle for PDFs #14028

merged 1 commit into from
May 4, 2018

Conversation

jasonrsadler
Copy link
Contributor

@jasonrsadler jasonrsadler commented May 4, 2018

Fixes #14025

Payment button is still not being enabled after time requirements satisified.

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:

  1. Clear profile, start browser, and enable payments.
  2. Visit orimi.com/pdf-test.pdf and verify that publisher toggle button renders.
  3. Wait the minimum amount of time and verify that publisher toggle becomes enabled.
  4. Verify that the publisher toggle includes and excludes from the ledger table appropriately.

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

@jasonrsadler jasonrsadler self-assigned this May 4, 2018
@jasonrsadler jasonrsadler changed the title Implemented fix to show publisher toggle Implemented fix to show publisher toggle for PDFs May 4, 2018
@NejcZdovc NejcZdovc added this to the 0.22.x Release 3 (Beta channel) milestone May 4, 2018
@codecov-io
Copy link

codecov-io commented May 4, 2018

Codecov Report

Merging #14028 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #14028      +/-   ##
==========================================
- Coverage   56.51%   56.46%   -0.05%     
==========================================
  Files         284      284              
  Lines       29357    29357              
  Branches     4880     4880              
==========================================
- Hits        16590    16577      -13     
- Misses      12767    12780      +13
Flag Coverage Δ
#unittest 56.46% <100%> (-0.05%) ⬇️
Impacted Files Coverage Δ
app/common/lib/publisherUtil.js 100% <100%> (ø) ⬆️
.../renderer/components/navigation/publisherToggle.js 81.41% <100%> (ø) ⬆️
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%) ⬇️
app/browser/api/ledger.js 63.47% <0%> (-0.18%) ⬇️

Updated publisherToggle to grab the real URL

lint

Renamed publisherState name to publisherUtil
Copy link
Contributor

@ryanml ryanml left a comment

Choose a reason for hiding this comment

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

Good fix, works well

@jasonrsadler jasonrsadler merged commit 507839d into brave:master May 4, 2018
jasonrsadler pushed a commit that referenced this pull request May 4, 2018
Implemented fix to show publisher toggle for PDFs
jasonrsadler pushed a commit that referenced this pull request May 4, 2018
Implemented fix to show publisher toggle for PDFs
@jasonrsadler
Copy link
Contributor Author

jasonrsadler commented May 4, 2018

master: 507839d
0.23: 0eef9e1
0.22-release3: 80ec523

@jasonrsadler jasonrsadler deleted the fix/#14025-pdf-payment branch May 4, 2018 22:49
NejcZdovc added a commit that referenced this pull request May 7, 2018
Added test for shouldShowPublisherButton for PR #14028
NejcZdovc added a commit that referenced this pull request May 7, 2018
Added test for shouldShowPublisherButton for PR #14028
NejcZdovc added a commit that referenced this pull request May 7, 2018
Added test for shouldShowPublisherButton for PR #14028
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants