Restructure the flow in which we apply infractions

[SebastiaanZ]

Currently, the flow with which we register infractions may take up to four requests to our API. I'd like to propose some changes that should bring this number of API requests down to one in most situations. This is especially handy in situations where a lot of infractions are issued in a really short time frame, such as during a raid, to not overload our API.

Note: The changes proposed in this issue also require changes to our API endpoints

Background

Let's take a "mute" infraction, for instance. Issuing a single mute infraction may take up to 4 requests to our API:

1. We first poll if the member already has an active "mute" infraction
2. If not, we then try to register the infraction with an API request
3. If the API replies that it doesn't know the user yet, we register the user using users endpoint
4. We then try to register the infraction again

For a "ban" infraction, a similar flow exists, except that we occasionally need to fetch the user from Discord for step 3 if the user has never been a member of our server (or otherwise not registered in our database). A permanent "ban" infraction may also require an additional request to our API to inactivate a current tempban.

Even if we ignoring the race condition that rarely happens (except during raids) where multiple near-simultaneously issued infractions make it past step 1 (verifying if there's no active infraction of that type) and then all, except one, fail on step 2, I think we can decrease the number of requests we need to register a infraction with our own API by making a few changes.

1. Asking for forgiveness instead of permission Instead of polling the API for active infractions of the type we're trying to issue, we just try to set the infraction. This avoids the rare race condition mentioned before and we can still handle "double" infractions by looking at the exception we'd get for a non-OK response.

Alternatively, we could decide that a more severe infraction (longer duration/permanent) should replace a shorter infraction automatically. My suggestion would be to do that at the very least for when the new infraction is permanent while a temporary infraction was already active. This wouldn't be a change in behavior, just a change in how this is achieved; we do it with a single request instead of three requests.

Additionally, if the second infraction has a later expiry date, we could also make that one automatically "replace" the infraction with a closer expiry date. To prevent moderators accidentally overwriting another infraction "just" given by another moderator in another channel, we could add a requirement that the second infraction has to have been issued a "substantial" time period after the first infraction (say, 2 hours). (We can always decide to undo the first to set a more severe one or to edit the existing one.)

Regardless, this reduces the two/three requests to one request.

2. Providing enough user information to register "unknown" users Our API occasionally complains about users that have not yet been registered to the API. One situation in which this may happen is if we issue an infraction on a user that has never been a member of our guild. I've also observed it when raid bots got themselves auto-muted so quickly after joining the server that the on_member_join syncer hadn't caught up yet. (And the latter is precisely a moment in which API requests are precious.)

To prevent this, we could send enough information along with the infraction to register the user in the database in a new, optional API endpoint field. If provided, rather than returning an error for the unknown user, the API simply registers the user and then sets the infraction.

Obviously, we can only do this if know that we haven't made a mistake with the ID (e.g., message id instead of user id). That's why I'm suggesting that the field is optional: If the infraction is issued with a Member or User instance, we know that it should belong to a "real" user and we can safely register them.

That means that instead of issuing a request to set the infraction, getting an error for "missing user", sending a second request to register the user, and then finally sending a third request to try the infraction another time, there's typically only one request.

Limitations

There's is one catch for proposal 2: If we try to ban a user that is not a member of our guild, we don't have easy access to a Member or User instance for that user to provide enough information for the proposed "auto-registration feature". We also can't determine if registering the user is needed without making a request, as we'd have to poll the API first to know if the user is "known" (e.g., a former member of the server).

However, this isn't a huge problem, considering the roughly two situations that may occur:

1. We're issuing a ban to a former member of the server; the API already "knows" them
2. We're issuing a ban to a completely unknown user; the API needs info to register them

It's nearly always the case that we're in situation 1, where registering the user isn't required. We often ban people after they have already left the server on their own; we rarely ban users that have never been a member in the first place.

This means that the best option is just to not fetch the User object from the heavily rate-limited Discord endpoint to fetch "non-guild users". We just assume that the user is already known, we don't provide additional registration information, and only if the request fails do we "fetch the user from Discord". We can then make a second infraction request with the user information attached.

[MarkKoz]

Since it seems related, I'll also tack on that infractions should only be posted after the action has been taken on the user (e.g. mute role added, nickname changed, etc.). This order allows for the infraction to not be posted if the action fails.

Edit: I realise this interferes with your proposal. Since failure for actions is rare, do you think it'd be better to send a delete request if the action fails? I do think asking for forgiveness has more benefits than my above proposal.

[MarkKoz]

I generally agree with all of your proposals. However, I have some questions below. I also worry that the features I will quote also increase complexity of the API (more so its behaviour than its interface).

Alternatively, we could decide that a more severe infraction (longer duration/permanent) should replace a shorter infraction automatically.

Additionally, if the second infraction has a later expiry date, we could also make that one automatically "replace" the infraction with a closer expiry date.

How do you propose to achieve this? You say it'd be one request, which makes me think that this would need to be handled on the Django side.