Repository: rollbar/rollbar-go
Pull request #96 (2022-06-17 07:07:58)

feat: Add support for extra fields on Person details
## Description of the change
(duplicate of https://github.com/rollbar/rollbar-go/pull/91)
This is to help achieve parody with the PHP SDK which allows for
additional fields to be specified on the Person object. We use this to
provide details like what account a specific user belongs to or other
additional internal metadata that is helpful when debugging user/account
level errors.

This is mainly mirroring what I saw in the PHP SDK here: 

https://github.com/rollbar/rollbar-php/blob/89c9069e8532acb638f2fb9efd36b529ec25354f/src/Payload/Person.php#L68-L80

```php
    public function serialize()
    {
        $result = array(
            "id" => $this->id,
            "username" => $this->username,
            "email" => $this->email,
        );
        foreach ($this->extra as $key => $val) {
            $result[$key] = $val;
        }
        
        return $this->utilities()->serializeForRollbarInternal($result, array_keys($this->extra));
    }
```

## Type of change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Maintenance
- [ ] New release

## Related issues

From what I've seen there are not any GitHub issues for this.

## Checklists

### Development

- [x] Lint rules pass locally
      _(I don't see any editor config or linting documentation in the README or anywhere else in the repo?)_
- [x] The code changed/added as part of this pull request has been covered with tests
      _(This code should be covered by existing unit tests, but happy to add a specific test for this code path)_
- [x] All tests related to the changed code pass in development

### Code review

- [x] This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
- [x] "Ready for review" label attached to the PR and reviewers assigned
- [x] Issue from task tracker has a link to this pull request
- [x] Changes have been reviewed by at least one other engineer



Patch:
diff --git a/.gitignore b/.gitignore
index 24c5d82..228bd1a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,3 +2,4 @@ rollbar.test
 .env
 cover.out
 cover.html
+.idea
diff --git a/client.go b/client.go
index e4be3d4..6c9d9d5 100644
--- a/client.go
+++ b/client.go
@@ -135,12 +135,25 @@ func (c *client) SetCustom(custom map[string]interface{}) {
 // SetPerson information for identifying a user associated with
 // any subsequent errors or messages. Only id is required to be
 // non-empty.
-func (c *client) SetPerson(id, username, email string) {
+
+type personOption func(*Person)
+
+func WithPersonExtra(extra map[string]string) personOption {
+	return func(p *Person) {
+		p.Extra = extra
+	}
+}
+func (c *client) SetPerson(id, username, email string, opts ...personOption) {
 	person := Person{
 		Id:       id,
 		Username: username,
 		Email:    email,
 	}
+	for _, opt := range opts {
+		// Call the option giving the instantiated
+		// *Person as the argument
+		opt(&person)
+	}
 
 	c.configuration.person = person
 }
@@ -634,6 +647,7 @@ type Person struct {
 	Id       string
 	Username string
 	Email    string
+	Extra    map[string]string
 }
 
 type pkey int
diff --git a/rollbar.go b/rollbar.go
index 8699442..51bf1a7 100644
--- a/rollbar.go
+++ b/rollbar.go
@@ -231,8 +231,8 @@ func SetCheckIgnore(checkIgnore func(string) bool) {
 // SetPerson information for identifying a user associated with
 // any subsequent errors or messages. Only id is required to be
 // non-empty.
-func SetPerson(id, username, email string) {
-	std.SetPerson(id, username, email)
+func SetPerson(id, username, email string, opts ...personOption) {
+	std.SetPerson(id, username, email, opts...)
 }
 
 // ClearPerson clears any previously set person information. See `SetPerson` for more information.
diff --git a/transforms.go b/transforms.go
index 336168a..e23d38d 100644
--- a/transforms.go
+++ b/transforms.go
@@ -50,11 +50,19 @@ func buildBody(ctx context.Context, configuration configuration, diagnostic diag
 		person = &configuration.person
 	}
 	if person.Id != "" {
-		data["person"] = map[string]string{
+		personData := map[string]string{
 			"id":       person.Id,
 			"username": person.Username,
 			"email":    person.Email,
 		}
+		for key, value := range person.Extra {
+			// If the field on the extra map is already specified then skip it.
+			// This will prevent the extra map from overwriting fields like ID, Username or Email.
+			if _, ok := personData[key]; !ok {
+				personData[key] = value
+			}
+		}
+		data["person"] = personData
 	}
 
 	return map[string]interface{}{


Test patch:
diff --git a/client_test.go b/client_test.go
index 86546bd..a843771 100644
--- a/client_test.go
+++ b/client_test.go
@@ -538,7 +538,9 @@ func TestSetPerson(t *testing.T) {
 	client := testClient()
 	id, username, email := "42", "bork", "bork@foobar.com"
 
-	client.SetPerson(id, username, email)
+	client.SetPerson(id, username, email, WithPersonExtra(map[string]string{
+		"person_extra1": "value1", "person_extra2": "value2", "id": "43"}))
+
 	client.ErrorWithLevel(ERR, errors.New("Person Bork"))
 
 	if transport, ok := client.Transport.(*TestTransport); ok {
@@ -554,6 +556,8 @@ func TestSetPerson(t *testing.T) {
 		errorIfNotEqual(id, person["id"], t)
 		errorIfNotEqual(username, person["username"], t)
 		errorIfNotEqual(email, person["email"], t)
+		errorIfNotEqual("value1", person["person_extra1"], t)
+		errorIfNotEqual("value2", person["person_extra2"], t)
 
 		configuredOptions := configuredOptionsFromData(data)
 		configuredPerson := configuredOptions["person"].(map[string]string)
diff --git a/rollbar_test.go b/rollbar_test.go
index ee0140b..5162d89 100644
--- a/rollbar_test.go
+++ b/rollbar_test.go
@@ -62,6 +62,7 @@ func TestErrorClass(t *testing.T) {
 func TestEverything(t *testing.T) {
 	SetToken(os.Getenv("TOKEN"))
 	SetEnvironment("test")
+	SetPerson("1", "user", "email")
 	if Token() != os.Getenv("TOKEN") {
 		t.Error("Token should be as set")
 	}
@@ -106,6 +107,7 @@ type someNonstandardTypeForLogFailing struct{}
 func TestEverythingGeneric(t *testing.T) {
 	SetToken(os.Getenv("TOKEN"))
 	SetEnvironment("test")
+	SetPerson("1", "user", "email", WithPersonExtra(map[string]string{"v": "k"}))
 	SetCaptureIp(CaptureIpAnonymize)
 	if Token() != os.Getenv("TOKEN") {
 		t.Error("Token should be as set")