-
-
Notifications
You must be signed in to change notification settings - Fork 333
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
Add name and geo to User
#2710
Merged
Merged
Add name and geo to User
#2710
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
5da7181
add name to user
denrase 2ccf64e
Merge branch 'main' into feat/user-name-and-geo
denrase 59ed2ae
add geo
denrase 7dcc3ce
add geo to user
denrase 9f36ea3
add changelog entry
denrase c7de7cd
Format code
getsentry-bot 83abdfd
add files to project and fix tests
denrase 7c61c2d
Merge branch 'feat/user-name-and-geo' of github.com:getsentry/sentry-…
denrase afd5daf
change prop to strong
denrase 2f6437e
use correct nullable syntax
denrase d8e4d8e
copy geo
denrase ba0f3f5
use assignment syntax
denrase f5dd593
use shorter init syntax
denrase a687e50
Merge branch 'main' into feat/user-name-and-geo
denrase c4d82c6
move entry to unreleased section
denrase dde6b42
Merge branch 'main' into feat/user-name-and-geo
denrase 7d6693c
use diefferent nullability syntax
denrase f497206
add newline
denrase a76140e
Merge branch 'main' into feat/user-name-and-geo
denrase 2bf9294
remove redundant init override
denrase f3aef79
Merge branch 'main' into feat/user-name-and-geo
denrase 990c8ea
remove synchronized blocks in geo, handle name in user
denrase 3c261e0
Format code
getsentry-bot f41a14e
Merge branch 'main' into feat/user-name-and-geo
denrase ba655db
refactor code
denrase 7d04382
add sample data in comment
denrase 70cc238
Format code
getsentry-bot d929410
update changelog entry
denrase 6cc9013
Merge branch 'main' into feat/user-name-and-geo
denrase File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| #import "SentryDefines.h" | ||
| #import "SentrySerializable.h" | ||
| #import <Foundation/Foundation.h> | ||
|
|
||
| NS_ASSUME_NONNULL_BEGIN | ||
|
|
||
| /// Approximate geographical location of the end user or device. | ||
| /// | ||
| /// Example of serialized data: | ||
| /// { | ||
| /// "geo": { | ||
| /// "country_code": "US", | ||
| /// "city": "Ashburn", | ||
| /// "region": "San Francisco" | ||
| /// } | ||
| /// } | ||
| NS_SWIFT_NAME(Geo) | ||
| @interface SentryGeo : NSObject <SentrySerializable, NSCopying> | ||
|
|
||
| /** | ||
| * Optional: Human readable city name. | ||
| */ | ||
| @property (nullable, atomic, copy) NSString *city; | ||
|
|
||
| /** | ||
| * Optional: Two-letter country code (ISO 3166-1 alpha-2). | ||
| */ | ||
| @property (nullable, atomic, copy) NSString *countryCode; | ||
|
|
||
| /** | ||
| * Optional: Human readable region name or code. | ||
| */ | ||
| @property (nullable, atomic, copy) NSString *region; | ||
denrase marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| - (BOOL)isEqual:(id _Nullable)other; | ||
|
|
||
| - (BOOL)isEqualToGeo:(SentryGeo *)geo; | ||
|
|
||
| - (NSUInteger)hash; | ||
|
|
||
| @end | ||
|
|
||
| NS_ASSUME_NONNULL_END | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| #import "SentryGeo.h" | ||
|
|
||
| #import <Foundation/Foundation.h> | ||
|
|
||
| NS_ASSUME_NONNULL_BEGIN | ||
|
|
||
| @implementation SentryGeo | ||
|
|
||
| - (id)copyWithZone:(nullable NSZone *)zone | ||
| { | ||
| SentryGeo *copy = [[[self class] allocWithZone:zone] init]; | ||
|
|
||
| if (copy != nil) { | ||
| copy.city = self.city; | ||
| copy.countryCode = self.countryCode; | ||
| copy.region = self.region; | ||
| } | ||
|
|
||
| return copy; | ||
| } | ||
|
|
||
| - (NSDictionary<NSString *, id> *)serialize | ||
| { | ||
| return @{ @"city" : self.city, @"country_code" : self.countryCode, @"region" : self.region }; | ||
| } | ||
|
|
||
| - (BOOL)isEqual:(id _Nullable)other | ||
| { | ||
| if (other == self) { | ||
| return YES; | ||
| } | ||
| if (!other || ![[other class] isEqual:[self class]]) { | ||
| return NO; | ||
| } | ||
|
|
||
| return [self isEqualToGeo:other]; | ||
| } | ||
|
|
||
| - (BOOL)isEqualToGeo:(SentryGeo *)geo | ||
| { | ||
| if (self == geo) { | ||
| return YES; | ||
| } | ||
| if (geo == nil) { | ||
| return NO; | ||
| } | ||
|
|
||
| NSString *otherCity = geo.city; | ||
| if (self.city != otherCity && ![self.city isEqualToString:otherCity]) { | ||
| return NO; | ||
| } | ||
|
|
||
| NSString *otherCountryCode = geo.countryCode; | ||
| if (self.countryCode != otherCountryCode | ||
| && ![self.countryCode isEqualToString:otherCountryCode]) { | ||
| return NO; | ||
| } | ||
|
|
||
| NSString *otherRegion = geo.region; | ||
| if (self.region != otherRegion && ![self.region isEqualToString:otherRegion]) { | ||
| return NO; | ||
| } | ||
|
|
||
| return YES; | ||
| } | ||
|
|
||
| - (NSUInteger)hash | ||
| { | ||
| NSUInteger hash = 17; | ||
|
|
||
| hash = hash * 23 + [self.city hash]; | ||
| hash = hash * 23 + [self.countryCode hash]; | ||
| hash = hash * 23 + [self.region hash]; | ||
|
|
||
| return hash; | ||
| } | ||
|
|
||
| @end | ||
|
|
||
| NS_ASSUME_NONNULL_END |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import XCTest | ||
|
|
||
| class SentryGeoTests: XCTestCase { | ||
philipphofmann marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| func testSerializationWithAllProperties() { | ||
| let geo = TestData.geo.copy() as! Geo | ||
| let actual = geo.serialize() | ||
|
|
||
| // Changing the original doesn't modify the serialized | ||
| geo.city = "" | ||
| geo.countryCode = "" | ||
| geo.region = "" | ||
|
|
||
| XCTAssertEqual(TestData.geo.city, actual["city"] as? String) | ||
| XCTAssertEqual(TestData.geo.countryCode, actual["country_code"] as? String) | ||
| XCTAssertEqual(TestData.geo.region, actual["region"] as? String) | ||
| } | ||
|
|
||
| func testHash() { | ||
| XCTAssertEqual(TestData.geo.hash(), TestData.geo.hash()) | ||
|
|
||
| let geo2 = TestData.geo | ||
| geo2.city = "Berlin" | ||
| XCTAssertNotEqual(TestData.geo.hash(), geo2.hash()) | ||
| } | ||
|
|
||
| func testIsEqualToSelf() { | ||
| XCTAssertEqual(TestData.geo, TestData.geo) | ||
| XCTAssertTrue(TestData.geo.isEqual(to: TestData.geo)) | ||
| } | ||
|
|
||
| func testIsNotEqualToOtherClass() { | ||
| XCTAssertFalse(TestData.geo.isEqual(1)) | ||
| } | ||
|
|
||
| func testIsEqualToCopy() { | ||
| XCTAssertEqual(TestData.geo, TestData.geo.copy() as! Geo) | ||
| } | ||
|
|
||
| func testNotIsEqual() { | ||
| testIsNotEqual { geo in geo.city = "" } | ||
| testIsNotEqual { geo in geo.countryCode = "" } | ||
| testIsNotEqual { geo in geo.region = "" } | ||
| } | ||
|
|
||
| func testIsNotEqual(block: (Geo) -> Void ) { | ||
| let geo = TestData.geo.copy() as! Geo | ||
| block(geo) | ||
| XCTAssertNotEqual(TestData.geo, geo) | ||
| } | ||
|
|
||
| func testCopyWithZone_CopiesDeepCopy() { | ||
| let geo = TestData.geo | ||
| let copiedGeo = geo.copy() as! Geo | ||
|
|
||
| // Modifying the original does not change the copy | ||
| geo.city = "" | ||
| geo.countryCode = "" | ||
| geo.region = "" | ||
|
|
||
| XCTAssertEqual(TestData.geo, copiedGeo) | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can't just grab the
NSLocaleobject that would probably be used to populate this, so we can get any other info from that we need at a later date, as well as ensure the right value is actually set in ourserializeimplementation (there's a lot going on in NSLocale and some people might not get it right).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As i understood this is filled out manually by users, so i'm not sure if we should do things automatically to populate these fields. What other info that we might need are you refering to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose even if we know sure we won't want anything other than country code, we could still just get it directly from the NSLocale. I've seen plenty of mistakes mixing up country codes, language codes and locale identifiers. I would ask for some validation logic, but that would be more work than us just using the locale object in a known safe way.
Do you mean end-users of apps consuming this SDK? Or users of the SDK, as in app developers? If the former, even more reason to validate input, but again, that's more work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the server infers that automatically from the IP if provided, this should only be set if the user wanna override the inferred value by the server as explained here #2710 (comment)