Implement report assignment

[esjee]

https://github.com/oreoshake/hackerone-client/issues/2

Usage:

# Assign to user:
> report.assign_to('esjee')

# Assign to group
> report.assign_to('Admin')

# Remove assignee
> report.assign_to('nobody')

assign_to username doesn't appear to work: it causes a 500 Server Error. (Edit: the reason was that you can't assign to API users. This causes a 500 Server Error). I cheated with the YML files to get that working.

Other than that, there are some edge cases which don't work. If you have a user and a group with the exact same name, then this code will assume you want to assign to a user. Similarly, if you happen to have a group called "nobody", this code will assume you actually want to remove the assignee. I don't believe these are real-world use cases, so I believe it's okay to sacrifice that usecase for the sake of a cleaner interface.

[Willianvdv]

@esjee wouldn't it make sense to name the assign methods assign_to_user and assing_to_group? Fixing the internals after @hacker0x01 pushed a fix for this bug is way easier than changing this API's interface.

[esjee]

Hey @Willianvdv, thanks for taking a look at this PR!

To be honest, I'm not sure. On one hand, I think you have a point. Having the API of this gem closely resemble the API of HackerOne means that it should contain less surprises, and be easier for developers to pick up. On the other hand, I think the assign_to username_or_groupname is a slightly cleaner interface.

Of course, we could also support both! Though I'm not sure that's a good idea, either. :sweat_smile:

[martijnrusschen]

Consider adding this functionality to the readme as well.

[Willianvdv]

I think the assign_to username_or_groupname is a slightly cleaner interface.

@esjee can you elaborate? IMO it's pretty confusing that you have an operation that does something. There's no way of telling you're about to assign a group or a user; it depends on how you call your users and groups in HackerOne.

[mvgijssel]

Assign to user or (no) group in a single method actually results in collisions: https://hackerone.com/nobody

[esjee]

@mvgijssel Right. As mentioned in the top post:

Other than that, there are some edge cases which don't work. If you have a user and a group with the exact same name, then this code will assume you want to assign to a user. Similarly, if you happen to have a group called "nobody", this code will assume you actually want to remove the assignee. I don't believe these are real-world use cases, so I believe it's okay to sacrifice that usecase for the sake of a cleaner interface.

You think this is a valid enough usecase to abandon this attempt at creating a cleaner interface?

[esjee]

@oreoshake What do you think? What direction do you want to take this feature?

[oreoshake]

I wasn't aware that a group and a user could have the same name and the nobody collision is 😆. Given that groups don't seem to be namespaced in any way I think we need the separate APIs for assign groups or users since the client shouldn't make any guesses.

[esjee]

I removed the friendly error messages. Note that there's still a discrepancy where this PR asks for a username or group name, whereas the HackerOne API asks for an id. Based on the earlier discussion, I suppose you would want to see that differently as well? I'm a bit concerned that hampers the usability of the gem.

[oreoshake]

username or group name, whereas the HackerOne API asks for an id

Can there be a collision between names and IDs? If IDs are sufficiently long, could we accept both?

[esjee]

There cannot -- usernames are not allowed to start with a number, and ids are always integers.

[oreoshake]

What do folks think about heuristically guessing whether it's an ID or username? I agree this would be very helpful.

[oreoshake]

What do folks think about heuristically guessing whether it's an ID or username? I agree this would be very helpful.

No sense blocking on this. Let's go with what's here! Thank you much @esjee for the back and forth.

[oreoshake]

:ship: in 0.5.0