Preserve Invocation Context

[DragonSenseiGuy]

Added:

• A link to the pastebin for commands executed outside of #bot-commands, to preserve invocation context

Closes #3376

[swfarnsworth]

@jb3, am I correct in my understanding that this copies messages from the server to our pastebin, possibly unexpectedly?

[jb3]

@jb3, am I correct in my understanding that this copies messages from the server to our pastebin, possibly unexpectedly?

Yes, but with it being a bot command I'm less worried about that. Previously we've worried about that when things such as automatically uploading unformatted code have been mentioned, or when we are talking about uploading messages which are not direct interactions with the bot.

In this case, it's reasonable to temporarily upload after a user has directly used our features (bear in mind we automatically upload the results if they are too long).

[swfarnsworth]

Perhaps we can send the user the link to delete the paste? You can largely copy the code here:

bot/bot/exts/utils/attachment_pastebin_uploader.py

Lines 130 to 133 in cb4401b

	# Send the user a DM with the delete link for the paste.
	# The angle brackets around the remove link are required to stop Discord from visiting the URL to produce a
	# preview, thereby deleting the paste
	await message.author.send(content=f"[Click here](<{paste_response.removal}>) to delete your recent paste.")

[DragonSenseiGuy]

Perhaps we can send the user the link to delete the paste? You can largely copy the code here:

bot/bot/exts/utils/attachment_pastebin_uploader.py

Lines 130 to 133 in cb4401b

	# Send the user a DM with the delete link for the paste.
	# The angle brackets around the remove link are required to stop Discord from visiting the URL to produce a
	# preview, thereby deleting the paste
	await message.author.send(content=f"[Click here](<{paste_response.removal}>) to delete your recent paste.")

Added.

[swfarnsworth]

When a string is too long, the preferred format is like this

content = (
    "Lorem ipsum dolor sit amet, consectetur "
    f"adipiscing {elit}, sed do eiusmod tempor "
    "incididunt ut labore et dolore magna aliqua."
)

This leverages implicit string concatenation. Note that segments using f-string need to start with an f. The trailing spaces at the end of each line are literal single spaces that are not line breaks.

[DragonSenseiGuy]

Yeah, Just saw the lint errors, pushed a commit.

[jb3]

Formatting and wording.

[wookie184]

Mostly looks good, thanks for the PR.

[MarkKoz]

Thanks. Looks good besides the comment about exception handling.

Maybe we should only be doing this when RedirectOutput.delete_invocation is true. But in practice this is a global option and the default is true, so it's probably not worth our time. I'm not sure why we even have a config option for that.

[jb3]

Thanks. Looks good besides the comment about exception handling.

Maybe we should only be doing this when RedirectOutput.delete_invocation is true. But in practice this is a global option and the default is true, so it's probably not worth our time. I'm not sure why we even have a config option for that.

If we have the option we may as well implement the functionality. I think just wrapping the network request section in an if statement should cover it? Once the definition for paste_response is added back then as long as that remains set to None then the bot will just form the generic no-link message and send that.

[DragonSenseiGuy]

Thanks. Looks good besides the comment about exception handling.
Maybe we should only be doing this when RedirectOutput.delete_invocation is true. But in practice this is a global option and the default is true, so it's probably not worth our time. I'm not sure why we even have a config option for that.

If we have the option we may as well implement the functionality. I think just wrapping the network request section in an if statement should cover it? Once the definition for paste_response is added back then as long as that remains set to None then the bot will just form the generic no-link message and send that.

So an if statement over the entire aysnc with ClientSession() as session?

[jb3]

[...]

So an if statement over the entire aysnc with ClientSession() as session?

Yes, as well as the variable declaration.

[decorator-factory]

this will break if paste_response is None

[DragonSenseiGuy]

I think the logs stop it from executing?

[jb3]

They don't, as mentioned in wookie's previous review (logs explicitly do not stop execution, nor do we want them to).

This will need wrapping with an if paste_response:

[DragonSenseiGuy]

then no else or is there a fallback option?

[jb3]

We don't need an else because if there isn't a paste_response then we didn't upload their paste.

[DragonSenseiGuy]

Fixed.

[DragonSenseiGuy]

Thanks for the merge @jb3 !

[onerandomusername]

this should use ctx.bot.http_session rather than creating a whole new session.