privileges, url: fix privilege check in .urlban command family

[dgw]

Description

This is a possible resolution for #2649.

Maybe there's a better way to do this—maybe it would be better to just use AccessLevel._member_names_ directly—but suggesting an alternative approach when appropriate is what code review's for, right?

The commits are separated out so it'll be easy to bring whatever is useful (especially the new test case) over to a new approach if someone has a better idea.

Checklist

• I have read CONTRIBUTING.md
• I can and do license this contribution under the EFLv2
• No issues are reported by make qa (runs make lint and make test)
  • mypy and pytest pass; only flake8 complains a little (fixed by build: pin urllib3<2.3 to accommodate our old fork of vcrpy #2647).
• I have tested the functionality of the things this change touches

An alternative of my own

Defining these shortcut constants from sopel.plugin (below) in sopel.privileges instead could be useful:

sopel/sopel/plugin.py

Lines 26 to 34 in 99c4a00

	# import and expose privileges as shortcut
	from sopel.privileges import AccessLevel
	VOICE = AccessLevel.VOICE
	HALFOP = AccessLevel.HALFOP
	OP = AccessLevel.OP
	ADMIN = AccessLevel.ADMIN
	OWNER = AccessLevel.OWNER
	OPER = AccessLevel.OPER

I do wonder if we should in fact make sopel.privileges.__all__ export the list of privilege names, and keep AccessLevel as more of an internal thing. It would remove any need for a weird module-level constant (sopel.privileges.names), and simplify adding the aliases back to sopel.plugin (to one line: from sopel.privileges import *).

[SnoopJ]

Nice patch!

[SnoopJ]

Defining these shortcut constants from sopel.plugin (below) in sopel.privileges instead could be useful:

I think it would be better to add them to __all__ so that updates to the Enum automagically end up in the module namespace, but yet another alternative would be to make sopel.privileges into the enum class rather than the module where it is defined. That is probably too controversial in terms of public API though.

[dgw]

That is probably too controversial in terms of public API though.

Yeah it is now. Before 8.0 released, it was still changeable. Now we have to keep it this way until at least 9.0.

What I don't like about making sopel.privileges itself the enum is that nothing else* lives directly in the first-child level of hierarchy like that. It's sopel.bot.Sopel not sopel.bot, sopel.trigger.Trigger not sopel.trigger, etc.

Is there a better way to organize things such that the enum can become sopel._something_.privileges? Probably yes, and if it bothers you enough you should make an issue for 8.1 with other ideas. 😁 (If nothing else feels "righter", we can still have a fallback in that issue to reorganize the shortcuts/__all__ as already suggested, since that doesn't belong in a bugfix.)

---

Yes I know about sopel.version_info. But metadata about the package itself is about the only thing that makes sense in the root 🤷‍♂️

[Exirel]

It would remove any need for a weird module-level constant (sopel.privileges.names), and simplify adding the aliases back to sopel.plugin (to one line: from sopel.privileges import *).

I really dislike the usage of the wildcard * import in Python. It's a code smell that is used in the most foolish ways.

Beside, no ones need names when you can do [e.name for e in AccessLevel] which gives the same thing, and remove the need to expose names in sopel.privileges.

[dgw]

Beside, no ones need names when you can do [e.name for e in AccessLevel] which gives the same thing, and remove the need to expose names in sopel.privileges.

Good point. I knew it was a good idea to keep each module's changes in a separate commit. Very easy to just drop the new attr in sopel.privileges and amend the patch to url, keeping the test unchanged 💪