bugsnag / bugsnag-android

BugSnag crash monitoring and reporting tool for Android apps
https://www.bugsnag.com/platforms/android/
Other
1.19k stars 205 forks source link

Access user-id? #265

Closed d4rken closed 4 years ago

d4rken commented 6 years ago

In some cases I would like users to be able to send me their user-id such that I can specifically look into their issues.

AFAIK the user-id is currently not accessible, is there a reason not to add a getter for this?

fractalwrench commented 6 years ago

@d4rken you should be able to set the user ID via: Bugsnag.setUser(final String id, final String email, final String name)

This will attach a user ID to a report. Would you be able to explain the usecase on why a getter would be useful?

d4rken commented 6 years ago

Setting the user-id would overwrite the current one that Bugsnag generates by default, right?

I don't want to change the user-id, just to make it visible in the app, the current user-id.

Setting my own user-id would not enable me to find and view reports that were already send with the old (bugsnag default) ID.

Looking at the code I'd just add client.getUser() and then either make the user fields public final or add getters.

Theoretically I think we should also unclutter the API a bit there and just add a setUser(user) and remove the many overlapping api methods, but that may be another issue/PR.

fractalwrench commented 6 years ago

@d4rken that makes sense. I think uncluttering the API would be a good idea, maybe something like the following would make sense, and we could deprecate older methods:

Bugsnag.setUser(new User("abc", null, null)) User user = Bugsnag.getUser()

d4rken commented 6 years ago

Yeah something like that. Need to check the init call chain a bit more... I could make a PR sometime this week, should I?

For general API uncluttering a deprecation would probably not be sufficient, rather require a breaking change major release. Less setter methods, more builder patterns to modify "config" objects... etc. anyways, different ticket for that 😉

fractalwrench commented 6 years ago

Sure :) if you're happy to make a PR then I can review it, otherwise we'll schedule this internally.

If you have other thoughts for decluttering the API I'd be quite interested to hear about them in a different ticket. There are a few things I'd like to do personally, such as reducing mutable state, and sharing some common code with our Java Notifier, but I'd be interested to hear an outside perspective on this.

d4rken commented 6 years ago

Sure :) if you're happy to make a PR then I can review it, otherwise we'll schedule this internally.

Done

If you have other thoughts for decluttering the API I'd be quite interested to hear about them in a different ticket.

Nothing concrete, mostly related to pulling details into their own classes like I did in #268. I've made a ticket, #269.

There are a few things I'd like to do personally, such as reducing mutable state, and sharing some common code with our Java Notifier, but I'd be interested to hear an outside perspective on this.

User is now immutable 😉 I've added some thoughts on that in #269 too. I have a java/playframework/lagoon project in my mind, maybe I'll take a look at the bugsnag java SDK when starting on it and send a few PR's your way.

fractalwrench commented 6 years ago

Thanks for the contribution and great suggestions @d4rken - I'll have a read through and respond. @kattrali may also help out with the review.

johnkiely1 commented 4 years ago

This has now been fixed in V5.0.0