Notify users when contribution date is pushed back #14006

NejcZdovc · 2018-05-03T13:22:47Z

Resolves #14000

new notification

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:

add one site to the ledger table (with time spent less then 30min)
add funds
close browser and change reconcile stamp to yesterday
make sure that date is pushed back for 3 days from the stamp that you set and that you see notification as is shown in the print screen (it can be pushed back for 6 days after restart)

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

NejcZdovc · 2018-05-03T13:25:07Z

blocked on brave-intl/bat-client#66

jasonrsadler · 2018-05-07T14:15:56Z

~~~Waited approximately 2 minutes for the switchover~~~
~~~Let me know if this is intended.~~~
See comment below.

jasonrsadler

EDIT
Something went haywire with npm and gave me an old version of bclient from a clean build.]


So I get the '30 minute' message but I get pushed out 6 days following that test plan.

jasonrsadler · 2018-05-07T14:40:11Z

@bradleyrichter for wording

Pushing time threshold to new PR

NejcZdovc · 2018-05-07T17:02:22Z

Updated text based on @bradleyrichter feedback

ryanml · 2018-05-07T17:07:40Z

js/constants/appConstants.js

@@ -192,6 +192,7 @@ const appConstants = {
  APP_ON_LEDGER_PIN_PUBLISHER: _,
  APP_ON_LEDGER_NOTIFICATION_INTERVAL: _,
  APP_ON_LEDGER_MEDIA_DATA: _,
+  APP_ON_FUZZING: _,


Could this be changed to something like APP_ON_LEDGER_FUZZING?

ryanml · 2018-05-07T17:10:04Z

js/actions/appActions.js

@@ -1981,6 +1981,13 @@ const appActions = {
    })
  },

+  onFuzzing: function (newStamp) {


Following other comment, maybe this should be onLedgerFuzzing

ryanml

One other comment left, besides the issue that @jasonrsadler logged to what will be another PR, lgtm

ryanml · 2018-05-07T17:24:37Z

app/browser/reducers/ledgerReducer.js

@@ -503,6 +503,15 @@ const ledgerReducer = (state, action, immutableAction) => {
        state = ledgerApi.onFetchReferralHeaders(state, action.get('error'), action.get('response'), action.get('body'))
        break
      }
+    case appConstants.APP_ON_LEDGER_FUZZING:
+      {
+        state = ledgerState.setAboutProp(state, 'status', ledgerStatuses.FUZZING)


Should we set this prop after newStamp has been validated on line 510?

I think that as soon as fuzzing is triggered we should display notification, even if something was not send over correctly. wdyt?

That sounds fine yeah, I just assumed that if the check failed the process to that point would be so quick it may not be worth it.

jasonrsadler · 2018-05-07T17:30:44Z

app/browser/api/ledger.js

@@ -2386,6 +2386,10 @@ const onInitRead = (state, parsedData) => {
  return state
 }

+const onFuzzing = () => {
+  appActions.onLedgerFuzzing(client.state.reconcileStamp)


Check for client.state != null

null check added

jasonrsadler

Do we need to check client.state or client.state.reconcileStamp == null?

Resolves brave#14000 Auditors: Test Plan:

ryanml

LGTM, thanks for the changes

please re-review

Notify users when contribution date is pushed back

NejcZdovc · 2018-05-08T07:49:30Z

master bced563
0.23 ef994a7
0.22-release3 187f889

NejcZdovc added feature/rewards PR/blocked labels May 3, 2018

NejcZdovc added this to the 0.22.x Release 3 (Beta channel) milestone May 3, 2018

NejcZdovc self-assigned this May 3, 2018

NejcZdovc added the PR/work-in-progress ⚒ label May 3, 2018

NejcZdovc removed the PR/work-in-progress ⚒ label May 5, 2018

NejcZdovc force-pushed the fix/#14000-notify branch 2 times, most recently from 869a3bf to e1934d9 Compare May 5, 2018 17:30

jasonrsadler removed the PR/blocked label May 5, 2018

NejcZdovc mentioned this pull request May 5, 2018

Added test for shouldShowPublisherButton for PR #14028 #14034

Merged

10 tasks

NejcZdovc added the PR/work-in-progress ⚒ label May 7, 2018

NejcZdovc requested review from ryanml, jasonrsadler and bradleyrichter May 7, 2018 07:59

NejcZdovc removed the PR/work-in-progress ⚒ label May 7, 2018

NejcZdovc force-pushed the fix/#14000-notify branch from e1934d9 to 837dd92 Compare May 7, 2018 08:00

jasonrsadler previously requested changes May 7, 2018

View reviewed changes

jasonrsadler self-requested a review May 7, 2018 16:03

jasonrsadler mentioned this pull request May 7, 2018

Reconcile date is pushing back from last reconcileStamp rather than currentDate #14042

Closed

NejcZdovc force-pushed the fix/#14000-notify branch from 837dd92 to 3ed6420 Compare May 7, 2018 17:01

ryanml reviewed May 7, 2018

View reviewed changes

NejcZdovc force-pushed the fix/#14000-notify branch from 3ed6420 to 548a096 Compare May 7, 2018 17:18

ryanml previously approved these changes May 7, 2018

View reviewed changes

jasonrsadler reviewed May 7, 2018

View reviewed changes

jasonrsadler self-requested a review May 7, 2018 18:50

jasonrsadler previously requested changes May 7, 2018

View reviewed changes

Notify users when contribution date is pushed back

da0cca6

Resolves brave#14000 Auditors: Test Plan:

NejcZdovc requested a review from jasonrsadler May 8, 2018 05:05

NejcZdovc dismissed ryanml’s stale review via da0cca6 May 8, 2018 05:05

NejcZdovc force-pushed the fix/#14000-notify branch from 548a096 to da0cca6 Compare May 8, 2018 05:05

NejcZdovc requested a review from ryanml May 8, 2018 05:06

ryanml approved these changes May 8, 2018

View reviewed changes

NejcZdovc merged commit bced563 into brave:master May 8, 2018

NejcZdovc added a commit that referenced this pull request May 8, 2018

Merge pull request #14006 from NejcZdovc/fix/#14000-notify

ef994a7

Notify users when contribution date is pushed back

NejcZdovc added a commit that referenced this pull request May 8, 2018

Merge pull request #14006 from NejcZdovc/fix/#14000-notify

187f889

Notify users when contribution date is pushed back

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Notify users when contribution date is pushed back #14006

Notify users when contribution date is pushed back #14006

NejcZdovc commented May 3, 2018 •

edited

Loading

NejcZdovc commented May 3, 2018 •

edited

Loading

jasonrsadler commented May 7, 2018 •

edited

Loading

jasonrsadler left a comment •

edited

Loading

jasonrsadler commented May 7, 2018

NejcZdovc commented May 7, 2018

ryanml May 7, 2018

NejcZdovc May 7, 2018

ryanml May 7, 2018

NejcZdovc May 7, 2018

ryanml left a comment

ryanml May 7, 2018

NejcZdovc May 7, 2018

ryanml May 7, 2018

jasonrsadler May 7, 2018

NejcZdovc May 8, 2018

jasonrsadler left a comment

ryanml left a comment

NejcZdovc commented May 8, 2018

Notify users when contribution date is pushed back #14006

Notify users when contribution date is pushed back #14006

Conversation

NejcZdovc commented May 3, 2018 • edited Loading

Submitter Checklist:

Test Plan:

Reviewer Checklist:

NejcZdovc commented May 3, 2018 • edited Loading

jasonrsadler commented May 7, 2018 • edited Loading

jasonrsadler left a comment • edited Loading

Choose a reason for hiding this comment

jasonrsadler commented May 7, 2018

NejcZdovc commented May 7, 2018

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

ryanml left a 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 left a comment

Choose a reason for hiding this comment

ryanml left a comment

Choose a reason for hiding this comment

NejcZdovc commented May 8, 2018

NejcZdovc commented May 3, 2018 •

edited

Loading

NejcZdovc commented May 3, 2018 •

edited

Loading

jasonrsadler commented May 7, 2018 •

edited

Loading

jasonrsadler left a comment •

edited

Loading