esi / esi-issues

Issue tracking and feature requests for ESI
https://esi.evetech.net/
208 stars 23 forks source link

Expand functions of POST /fleets/{fleet_id}/members/ #897

Open Aidansavage opened 6 years ago

Aidansavage commented 6 years ago

Pending approval of #887, expand the functions of the POST side of the fleet invite endpoint to allow players to send a join request to the fleet (with appropriate prompts if fleet is set to "approve invites"). This will allow Entities in the game to create, or modify, tools to allow them to send out notifications with links and such that allows people to join fleets without having to actively search for them through the ingame fleet finder. This would also provide an alternative to the current limits of the endpoint of sending invites to an explicit list of characters.

SpeedProg commented 6 years ago

Isn't this already possible if your fleet is registered with the website (which for this endpoint to work would need to be too).

Like you send out a link for a fleet and when they visit it your website sends an invite to the player that visited your website (he needs to be logged in with your website so it knows who to send it to).

Might be that I am missing an important difference here. :/

Aidansavage commented 6 years ago

While not wrong, the immediate difference is that joining a fleet doesnt give the player a prompt, while being invited to it would.

DaneelTrevize commented 6 years ago

Try again with the correct template

Assumed scope required: esi-fleets.write_fleet.v1 but a new style of use-case where the authenticated user isn't the fleet boss.
This FR surely makes more sense to be for POST /characters/{character_id}/fleet/, because you'd only be able to POST 1 Character ID to /fleets/{fleet_id}/members/ while not the boss, that ID being your own. And it would tie in with #887's proposal to modify /characters/{character_id}/fleet/.

Aidansavage commented 6 years ago

Considering it's an expansion of the available functions of POST /fleets/{fleet_id}/members/, it should not need to be stated what the required scope is. That much should already be relevant from the fact it's an existing endpoint.

No, it does not make more sense for it to be /characters/*/ over expanding the functions. This is smack in the middle of a fleet endpoint's operation umbrella, and should stay within the relevant scope proposed in the original post. The limitations of what's being POSTed is pretty irrelevant. Even more so when creating yet another endpoint (in the wrong category no less, which would mean it WOULDNT be an esi-fleet.*.v* scope to begin with) would be fundamentally against what TechCo was trying to do- deduplicate and reduce redundancy. Thus raising the potential of expanding the features of an existing endpoint rather than frivolously asking for a new one to be made. If TechCo finds out that it's unfeasible to expand the functions of this endpoint in such a way, and would require splitting it off into it's own endpoint, we'll cross that ocean when we get there.

Finally, if you would actually read the original issue more thoroughly next time, you would have noticed that it already states:

Pending approval of #887

DaneelTrevize commented 6 years ago

/characters/{character_id}/fleet/ already exists.
The reading of it already involves a esi-fleet.*.v* scope.
My suggestion is consistent with that.

A person adding themselves to a fleet involves both character and fleet entities, there's at least as much reason to add this under one existing endpoint than the other.
Adding yourself to a fleet (possibly pending FC approval) is a rather different kind of action to the FC sending an invite to a (set of) character(s). In that case they're pre-approved, but may be offline, could be invited directly to a fleet hierarchy position iirc, etc.

Should I be assuming you actually speak for TechCo?