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

Fix #2539: Super Referrer Improvements (Clipboard) #2535

Merged
merged 16 commits into from May 25, 2020
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Comment cleanup.

  • Loading branch information
jhreis committed May 20, 2020
commit 576ae4a30275adfb10b469ddfcb8d652959121f4
@@ -87,6 +87,8 @@ public class UserReferralProgram {
Preferences.URP.referralCode.value = refCode
This conversation was marked as resolved by jhreis

This comment has been minimized.

Copy link
@kylehickinson

kylehickinson May 11, 2020

Contributor

This pref is not new right? Is this the only place we can set this?

This comment has been minimized.

Copy link
@jhreis

jhreis May 11, 2020

Author Contributor

Not new, and no. This only gets set for ref codes passed in via clipboard.
I'll add comment here too.

It also gets set if the server passes down a ref code.

}

// Since ref-code method may not be repeatable (e.g. clipboard was cleared), this should be retrieved from prefs,
// and not use the passed in referral code.
service.referralCodeLookup(refCode: UserReferralProgram.getReferralCode(), completion: referralBlock)
}

@@ -198,7 +200,7 @@ public class UserReferralProgram {

// +1 to strip off `:` that proceeds the defined prefix
input.removeFirst(self.clipboardPrefix.count + 1)
This conversation was marked as resolved by jhreis

This comment has been minimized.

Copy link
@kylehickinson

kylehickinson May 11, 2020

Contributor

Understand this probably set in a spec somewhere, but maybe a comment about why + 1?

This comment has been minimized.

Copy link
@jhreis

jhreis May 11, 2020

Author Contributor

Nah, lol, yeah, will do!

This conversation was marked as resolved by Brandon-T

This comment has been minimized.

Copy link
@Brandon-T

Brandon-T May 20, 2020

Collaborator

If input is exactly the same length as clipboardPrefix, then removing count + 1 is undefined behaviour and could potentially cause a crash.

The number of elements to remove from the collection. k must be greater than or equal to zero and must not exceed the number of elements in the collection.

We can add to the guard statement that input.count > self.clipboardPrefix.count + 1
Is it possible that the ref-code after the prefix could potentially be blank? I'm not sure, but I think some people might try to manipulate it :D

This comment has been minimized.

Copy link
@jhreis

jhreis May 20, 2020

Author Contributor

Good catch, thanks @Brandon-T

This comment has been minimized.

Copy link
@jhreis

jhreis May 20, 2020

Author Contributor

I added a unit test to cover this too, thanks for pointing out.

let valid = input.range(of: #"\b[A-Z]{3}[0-9]{3}\b"#, options: .regularExpression) != nil // true
let valid = input.range(of: #"\b[A-Z]{3}[0-9]{3}\b"#, options: .regularExpression) != nil

// Both conditions must be met
return valid ? input : nil
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.