Allow get_top_role to be optionally hoisted via config.

[scragly]

PR #3014 introduced the behavior to only fetch hoisted top roles, which may be fine for most, but is an unexpected behavior to those who may have a secondary mod-mail server that doesn't bother to hoist the roles, and may specifically not wish to.

The result of no hoisted roles is predictably None being sent for anon reply titles and mod_tag reply footers.

In this PR, I've set this behavior to be configurable through a new config key: use_hoisted_top_role. Considering the majority of setups will be fine with it as-is, I've set it to True by default.

I chose to use a simple hoisted flag on the util function rather than requiring bot be passed just to reference the config. This will also ensure the utility can be used in a generic manner in future for cases that may not want to rely on the same configuration value.

[fourjr]

Thanks for the PR. Haven't looked at it properly yet but could you add the new config option into config_help.json?

[scragly]

Added, just let us know if the wording needs adjusting 👍

[Jerrie-Aries]

Here you can just do:

    if not hoisted:
        return member.top_role

before the for loop. Hmm, but the output still the same tho. 😅

And also, additionally, I'm suggesting if hoisted is True and no hoisted role is found after the for loop, it should fallback to member.top_role. Just to make sure this function returns an appropriate role object. 🤔

[scragly]

Here you can just do:

Thanks for the recommendation. It's a bit of a nitpick perhaps, as in discord.py, it's also iterating over the roles anyway in a similar manner. I'm inclined to leave it as is.

I'm suggesting if hoisted is True and no hoisted role is found after the for loop, it should fallback to member.top_role. Just to make sure this function returns an appropriate role object.

Honestly, this should probably have been the default behavior back when the original PR change occurred, however I didn't want to assume there wasn't considerations behind why it wasn't. Instead, my intention is to not alter the existing behavior as was written by @fourjr originally, and just make it configurable.

I don't want to creep the scope of this PR beyond this intention. If I did, I'd probably end up re-writing the entire project eventually just because I found a whole bunch of things I think should be different.

If the behavior you state is something you want, feel free to create a PR after this is merged.

[fourjr]

lgtm