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

Fix Brave Ads state-level targeting crash #2693

Merged

Conversation

tmancey
Copy link
Collaborator

@tmancey tmancey commented Jul 2, 2020

Summary of Changes

This pull request fixes #2692

Submitter Checklist:

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

Test Plan:

Screenshots:

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).

@tmancey tmancey requested a review from kylehickinson July 2, 2020 18:06
@tmancey tmancey self-assigned this Jul 2, 2020
@tmancey tmancey added this to the 1.20 milestone Jul 2, 2020
guard let selectedIndex = subdivisionTargetingOptions.firstIndex(where: { $0.0 == adsSubdivisionTargetingCode }) else { fatalError() }
cell.accessoryLabel?.text = subdivisionTargetingOptions[selectedIndex].1
if adsSubdivisionTargetingCode == "DISABLED" {
cell.accessoryLabel?.text = "Disabled"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This has to be localized

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@tmancey tmancey requested a review from kylehickinson July 7, 2020 08:12
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.

Still needs to be localized (put static text in entry in Strings.swift, then use that there)

@tmancey tmancey force-pushed the brave-core-1.12.x-fix-state-level-targeting-crash branch from d5ded3c to 2f6fba3 Compare July 7, 2020 17:58
@tmancey tmancey requested a review from kylehickinson July 7, 2020 18:00
@tmancey
Copy link
Collaborator Author

tmancey commented Jul 7, 2020

@kylehickinson Fixed localization issues

@tmancey tmancey modified the milestones: 1.20, 1.21 Jul 7, 2020
@kylehickinson kylehickinson removed this from the 1.21 milestone Jul 7, 2020
@tmancey tmancey merged commit bb0238c into brave-core-1.12.x Jul 7, 2020
@tmancey tmancey deleted the brave-core-1.12.x-fix-state-level-targeting-crash branch July 7, 2020 18:40
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.

2 participants