Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #2378: iOS DB Migration & Rebase to brave-core master #2389

Merged
merged 1 commit into from
Mar 10, 2020

Conversation

kylehickinson
Copy link
Collaborator

@kylehickinson kylehickinson commented Mar 4, 2020

Summary of Changes

This pull request fixes #2378

By moving BraveRewards framework build up to brave-core/master this issue also fixes #2200

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()

Test Plan:

  • Run through previous version rewards, claim a grant, visit some pages, make some tips, etc.
  • Upgrade to version with this PR merged and verify that all your activity + BAT is still available

Screenshots:

iOS 12 Failure Screenshots
Light Dark
iOS 12 2 - light iOS 12 2 - dark
iOS 13 Failure Screenshots
Light Dark
iOS 13 3 - light iOS 13 3 - dark

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • release-notes/(include|exclude)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue is assigned to a milestone (should happen at merge time).

@kylehickinson kylehickinson marked this pull request as ready for review March 4, 2020 20:03
@kylehickinson kylehickinson linked an issue Mar 4, 2020 that may be closed by this pull request
@iccub iccub added the blocked If a ticket is blocked for some reason, if not using a sub-block label, please provide info in issue label Mar 5, 2020
@@ -93,7 +94,10 @@ extension RewardsSummaryProtocol {
})
}

let reservedAmount = BATValue(state.ledger.reservedAmount)
state.ledger.pendingContributionsTotal { amount in

Copy link
Contributor

Choose a reason for hiding this comment

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

why this empty?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops, left this in here when I moved the logic for getting it out (now passed in the arg since its async)

I'll delete it :)

self.rewardsSummaryView.disclaimerView = WalletDisclaimerView().then {
$0.labels = labels
$0.labels.forEach {
$0.onLinkedTapped = { [weak self] link in
Copy link
Contributor

Choose a reason for hiding this comment

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

this $0 refers to labels or to WalletDisclaimerView? idk, maybe name one of the params to avoid confusion

@@ -78,7 +78,7 @@ final class AutoContributeExclusionListController: UIViewController {
}

@objc private func tappedRestoreAll(_ sender: UIBarButtonItem) {
let numberOfExcludedSites = state.ledger.numberOfExcludedPublishers
let numberOfExcludedSites = publishers.count
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure about naming but shouldn't this be some kind of excluded publishers count?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We are in a controller which only shows excluded publishers, so any publisher in here is already excluded

@kylehickinson kylehickinson requested a review from iccub March 5, 2020 20:49
@kylehickinson kylehickinson removed the blocked If a ticket is blocked for some reason, if not using a sub-block label, please provide info in issue label Mar 5, 2020
Copy link
Contributor

@iccub iccub left a comment

Choose a reason for hiding this comment

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

Looks good, I would wait with merging a day or two until 1.15 is approved

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iOS DB Migration (& rebase to brave-core master) entire a-c not going through due to missing delays
3 participants