Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promotion expiration #4035

Merged
merged 1 commit into from
Dec 3, 2019
Merged

Promotion expiration #4035

merged 1 commit into from
Dec 3, 2019

Conversation

masparrow
Copy link
Contributor

@masparrow masparrow commented Nov 20, 2019

Resolves brave/brave-browser#6947

Submitter Checklist:

Test Plan:

  1. Clean install
  2. Join rewards, and claim a grant
  3. Check your balance (17.5 BAT)
  4. Close Brave
  5. Edit the promotion expiresAt in the database publisher_info_db to be in the past (i.e. 946684800 for 00:00:00 01/01/2000)
  6. Launch Brave
  7. Check your balance (should now be 0.0 BAT)
  8. There should be no details button (to show more grants) in wallet panel

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.

@masparrow masparrow added this to the 1.3.x - Nightly milestone Nov 20, 2019
@masparrow masparrow self-assigned this Nov 20, 2019
@masparrow masparrow changed the title Promotion expiration - Resolves https://github.com/brave/brave-browser/issues/6947 Promotion expiration Nov 20, 2019
@masparrow masparrow force-pushed the issues/6497 branch 2 times, most recently from f245b3d to ceefcd5 Compare November 20, 2019 15:33
@NejcZdovc
Copy link
Contributor

can you please add browser test for it?

Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

After step 7 in the test plan I checked if details button is gone. It's gone on settings page, but I still see promotion in the panel.

Panel
image

Settings page
image

@masparrow
Copy link
Contributor Author

masparrow commented Nov 22, 2019

can you please add browser test for it?

Will do, and I'll investigate the promotions thing - I spotted that but thought it was correct as it was a history of the claim. I'm guessing data is cached elsewhere.

@@ -311,8 +311,12 @@ class LEDGER_EXPORT LedgerClient {
ledger::GetAllUnblindedTokensCallback callback) = 0;

virtual void DeleteUnblindedTokens(
const std::vector<std::string>& id_list,
ledger::ResultCallback callback) = 0;
const std::vector<std::string>& id_list,
Copy link
Contributor

Choose a reason for hiding this comment

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

spacing was fixed for this function

@NejcZdovc NejcZdovc self-requested a review December 2, 2019 15:34
Copy link
Collaborator

@kylehickinson kylehickinson left a comment

Choose a reason for hiding this comment

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

iOS db implementation will be migrated to native libs in #4087

@NejcZdovc
Copy link
Contributor

CI failed on audit-network, not related.

Copy link
Collaborator

@tmancey tmancey left a comment

Choose a reason for hiding this comment

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

LGTM++

@NejcZdovc NejcZdovc merged commit 1ca5bf5 into master Dec 3, 2019
@NejcZdovc NejcZdovc deleted the issues/6497 branch December 3, 2019 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Promotion expiration
4 participants