k-capehart / go-salesforce

Salesforce REST API client written in Go
https://pkg.go.dev/github.com/k-capehart/go-salesforce/v2
MIT License
24 stars 4 forks source link

refactor: expose auth field in Salesforce struct for enhanced integration #44

Closed iamber12 closed 1 month ago

iamber12 commented 1 month ago

Addresses #43

This PR exposes the auth field in the Salesforce struct to allow direct access to the authentication token, essential for specific API calls such as retrieving ContentVersion data. This change improves the client's flexibility and integration capabilities. I have run the test suite, and all tests pass successfully. No changes in test coverage.

k-capehart commented 1 month ago

@iamber12 Hey Amber, thank you! I was hesitant to make this public, it opens the door for users to edit the field values of the Authentication struct, which doesn't feel right. What if instead we created a method on the Salesforce type GetAccessToken() that just returned the value of sf.auth.AccessToken? That way the values remains read-only.

Let me know your thoughts on that, I'm open to my mind being changed.

iamber12 commented 1 month ago

@k-capehart I hadn't thought of it that way, but it makes complete sense. I agree with your point, and I'll make the changes to create a GetAccessToken() method instead. I'll update the PR shortly.

iamber12 commented 1 month ago

@k-capehart I added it to the Salesforce struct for simplicity. Please let me know if this approach works for you.

k-capehart commented 1 month ago

@k-capehart I added it to the Salesforce struct for simplicity. Please let me know if this approach works for you.

@iamber12 I have once concern with this specific solution, but maybe you have a different perspective.

I'd be worried that there could be potential drift between the AccessToken in the Salesforce struct, and the AccessToken on the Authentication struct. Since one of them is public and therefore can be edited. So you could authenticate, then do something like sf.AccessToken = "1234". Your operations would still work, but then your public facing AccessToken differs from the private one, with no means of retrieving the private one.

Going back to what I said earlier, I think the best solution is still to have something like this.

func (sf *Salesforce) GetAccessToken() string {
    return sf.auth.AccessToken
}
iamber12 commented 1 month ago

@k-capehart while having it in the Salesforce struct would have been more convenient for certain operations, I understand the concern about maintaining consistency and avoiding potential issues.

I've made the necessary changes and updated the PR accordingly.

k-capehart commented 1 month ago

@iamber12 Awesome, thanks for spending the time, it's greatly appreciated! 🏆

I'll merge this in and create a new release shortly.

I also invited you to the repository. No pressure on more contributions or anything, but I think it'd be great to have you listed as a contributor.

iamber12 commented 1 month ago

@k-capehart Thank you for the invitation to the repository. I’m happy to accept and look forward to contributing more in the future.