From addfd70810a0a67e6f5e35960051acbb0754dfaa Mon Sep 17 00:00:00 2001 From: Ayumi Yu Date: Mon, 31 Oct 2016 12:50:13 -0700 Subject: [PATCH] Limit 24 hour payment notification to once per reconcile period Fix #4481 Auditors: @bsclifton Test Plan: 1. Trigger the "Payment in 24 hours, please review" notification. - Update reconcileStamp to <24 hours from now - Have sufficient funds; OR disable the sufficient funds conditional by adding false to ledger.js L1485 (showEnabledNotifications()) - Change startup notification delay in ledger.js L484 from 15m to 5s 2. Open Brave and observe 24h review notification 3. Close Brave and reopen. Notification should not reappear. (Next 24h notification timestamp is set in Application Support/brave-development/session-store-1) --- app/ledger.js | 55 ++++++++++++++++++++++----------------- docs/state.md | 1 + js/constants/appConfig.js | 3 +++ js/constants/settings.js | 1 + 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/app/ledger.js b/app/ledger.js index c6da3519b37..ea39cf0eabc 100644 --- a/app/ledger.js +++ b/app/ledger.js @@ -105,8 +105,6 @@ let addFundsMessage let reconciliationMessage let notificationPaymentDoneMessage let notificationTryPaymentsMessage -let suppressNotifications = false -let reconciliationNotificationShown = false let notificationTimeout = null // TODO(bridiver) - create a better way to get setting changes @@ -323,9 +321,9 @@ if (ipc) { if (message === addFundsMessage) { appActions.hideMessageBox(message) if (buttonIndex === 0) { - // Don't show notifications for the next 6 hours. - suppressNotifications = true - setTimeout(() => { suppressNotifications = false }, 6 * msecs.hour) + // "Later" -- wait 6 hours to re-show "reconciliation soon" notification. + const nextTime = ledgerInfo.reconcileStamp + 6 * msecs.hour + appActions.changeSetting(settings.PAYMENTS_NOTIFICATION_RECONCILE_SOON_TIMESTAMP, nextTime) } else { // Open payments panel if (win) { @@ -339,9 +337,6 @@ if (ipc) { win.webContents.send(messages.SHORTCUT_NEW_FRAME, 'about:preferences#payments', { singleFrame: true }) } - // If > 24 hours has passed, it might be time to show the reconciliation - // message again - setTimeout(() => { reconciliationNotificationShown = false }, 1 * msecs.day) } else if (message === notificationPaymentDoneMessage) { appActions.hideMessageBox(message) } else if (message === notificationTryPaymentsMessage) { @@ -889,6 +884,7 @@ var ledgerInfo = { creating: false, created: false, + reconcileFrequency: undefined, reconcileStamp: undefined, transactions: @@ -1194,6 +1190,7 @@ var getStateInfo = (state) => { ledgerInfo.created = !!state.properties.wallet ledgerInfo.creating = !ledgerInfo.created + ledgerInfo.reconcileFrequency = state.properties.days ledgerInfo.reconcileStamp = state.reconcileStamp if (info) { @@ -1442,8 +1439,7 @@ var pathName = (name) => { const showNotifications = () => { if (getSetting(settings.PAYMENTS_ENABLED) && - getSetting(settings.PAYMENTS_NOTIFICATIONS) && - !suppressNotifications) { + getSetting(settings.PAYMENTS_NOTIFICATIONS)) { showEnabledNotifications() } else if (!getSetting(settings.PAYMENTS_ENABLED)) { showDisabledNotifications() @@ -1500,24 +1496,35 @@ const showEnabledNotifications = () => { persist: false } }) - } else if (!reconciliationNotificationShown) { - reconciliationMessage = reconciliationMessage || locale.translation('reconciliationNotification') - appActions.showMessageBox({ - greeting: locale.translation('updateHello'), - message: reconciliationMessage, - buttons: [ - {text: locale.translation('reviewSites'), className: 'primary'} - ], - options: { - style: 'greetingStyle', - persist: false - } - }) - reconciliationNotificationShown = true + } else if (shouldShowNotificationReviewPublishers()) { + showNotificationReviewPublishers() } } } +const shouldShowNotificationReviewPublishers = () => { + const nextTime = getSetting(settings.PAYMENTS_NOTIFICATION_RECONCILE_SOON_TIMESTAMP) + return !nextTime || (underscore.now() > nextTime) +} + +const showNotificationReviewPublishers = () => { + const nextTime = ledgerInfo.reconcileStamp + (ledgerInfo.reconcileFrequency - 1) * msecs.day + appActions.changeSetting(settings.PAYMENTS_NOTIFICATION_RECONCILE_SOON_TIMESTAMP, nextTime) + + reconciliationMessage = reconciliationMessage || locale.translation('reconciliationNotification') + appActions.showMessageBox({ + greeting: locale.translation('updateHello'), + message: reconciliationMessage, + buttons: [ + {text: locale.translation('reviewSites'), className: 'primary'} + ], + options: { + style: 'greetingStyle', + persist: false + } + }) +} + // Called from observeTransactions() when we see a new payment (transaction). const showNotificationPaymentDone = (transactionContributionFiat) => { notificationPaymentDoneMessage = locale.translation('notificationPaymentDone') diff --git a/docs/state.md b/docs/state.md index 0601d88ebbe..053c19e679b 100644 --- a/docs/state.md +++ b/docs/state.md @@ -445,6 +445,7 @@ WindowStore ledgerInfo: { creating: boolean, // wallet is being created created: boolean, // wallet is created + reconcileFrequency: number, // duration between each reconciliation in days reconcileStamp: number, // timestamp for the next reconcilation transactions: [ { // contributions reconciling/reconciled viewingId: string, // UUIDv4 for this contribution diff --git a/js/constants/appConfig.js b/js/constants/appConfig.js index a31f0eaa9ff..59d24c17ac9 100644 --- a/js/constants/appConfig.js +++ b/js/constants/appConfig.js @@ -119,6 +119,9 @@ module.exports = { 'bookmarks.toolbar.showOnlyFavicon': false, 'payments.enabled': false, 'payments.notifications': false, + // "Out of money, pls add" / "In 24h we'll pay publishers [Review]" + // After shown, set timestamp to next reconcile time - 1 day. + 'payments.notification-reconcile-soon-timestamp': null, 'payments.notificationTryPaymentsDismissed': false, // True if you dismiss the message or enable Payments 'payments.contribution-amount': 5, // USD 'privacy.autofill-enabled': true, diff --git a/js/constants/settings.js b/js/constants/settings.js index e72b15d66d5..fb3e639b4fa 100644 --- a/js/constants/settings.js +++ b/js/constants/settings.js @@ -45,6 +45,7 @@ const settings = { // Payments Tab PAYMENTS_ENABLED: 'payments.enabled', PAYMENTS_NOTIFICATIONS: 'payments.notifications', + PAYMENTS_NOTIFICATION_RECONCILE_SOON_TIMESTAMP: 'notification-reconcile-soon-timestamp', PAYMENTS_NOTIFICATION_TRY_PAYMENTS_DISMISSED: 'payments.notificationTryPaymentsDismissed', PAYMENTS_CONTRIBUTION_AMOUNT: 'payments.contribution-amount', // Advanced settings