Skip to content

Commit

Permalink
Add the three new commands for network-wide blacklisting, add code to…
Browse files Browse the repository at this point in the history 
… fetch network ID of a user given either a per-site URL or just their SE-wide profile URL
  • Loading branch information
@CoconutMacaroon
CoconutMacaroon committed Dec 23, 2022
1 parent a9505a4 commit 2db2bcc
Showing 1 changed file with 62 additions and 0 deletions.
62 changes: 62 additions & 0 deletions chatcommands.py
View file
Expand Up @@ -39,6 +39,8 @@
from dns.exception import DNSException
import number_homoglyphs
import phone_numbers
# TODO: add 'tldextract' to requirements.txt
import tldextract


# TODO: Do we need uid == -2 check? Turn into "is_user_valid" check
Expand All @@ -53,7 +55,67 @@ def null():
return None


def per_site_account_to_network_id(site_uid: int, site: str) -> int:
"""Given a site name and the per-site ID number, fetch the SE ID for said account"""
# TODO: use the API key that we have, also we should likely do something w/ the backoff paramater
return int(requests.get(
"https://api.stackexchange.com/2.3/users/" + str(site_uid) + "/?site=" + site
).json()["items"][0]["account_id"])

This comment has been minimized.

Sign in to view

Copy link
@makyen

makyen Dec 24, 2022

Contributor

Unfortunately, there is substantially more to using the SE API within SD than just making a requests.get() call. Primarily, it's handling backoff. It is, currently, substantially more complex than it should be. I have, partially, written code to be an abstraction for the process, such that it does end up being just a single call which then handles backoff and at least a portion of errors. Finishing that up, even if what I first PR is substantially less than the full handling I'd desire, is a high priority at this point.

This comment has been minimized.

Sign in to view

Copy link
@makyen

makyen Dec 24, 2022

Contributor

Let me see if I can get that done and a PR made in the next couple of days.


def getnetid (url):

This comment has been minimized.

Sign in to view

Copy link
@makyen

makyen Dec 24, 2022

Contributor

If this isn't a chat command, and thus the function name isn't being used directly in chat, then the name should be in snake case, so get_net_id.

This comment has been minimized.

Sign in to view

Copy link
@CoconutMacaroon

CoconutMacaroon Dec 25, 2022

Author Contributor

Yep - also note that I haven't formatted the rest of the changes, so there are also some other formatting/naming convention errors. That is something I'll check. Related question: although I don't use it there, should I be using type hints? I notice that SD doesn't use them in other functions. I did a search in GH Issues, but I don't see any mention of it. In CHQ, I found a discussion about it, but I'm still not sure if I should use them or not

This comment has been minimized.

Sign in to view

Copy link
@makyen

makyen Dec 26, 2022

Contributor

As to type hints: We currently don't use them in SD. It would be reasonable to open an issue on GitHub to discuss if we want to start doing so.
"""Takes a URL for a user account on a network site and returns their network-wide SE ID number"""

This comment has been minimized.

Sign in to view

Copy link
@makyen

makyen Dec 26, 2022

Contributor

Also, this is something that's a general purpose function, which might be desirable to use elsewhere in SD's code at some point. So, it seems like it would be more appropriate to have this function in helpers.py, which would allow it to be imported from there, rather than potentially requiring the import of chatcommands.py just to use this function.
# TODO: validate the URL we're given somehow
#
# TODO: looks like there is already a regex for this in the function
# get_user_from_url in parsing.py, can some of that logic be re-used
# here even though we also need to be able to extract the user ID from
# an SE URL and not just a per-site account URL

This comment has been minimized.

Sign in to view

Copy link
@makyen

makyen Dec 24, 2022

Contributor

As I understand it, main SE user URLs have the same structure as user IDs for the rest of SE sites. Thus, get_user_from_url should be able to be used directly to extract the user and site from all such URLs. If it doesn't work, then that function should be changed such that it does work. At least in my, limited, testing of that function, it does work with all SE user URLs. Well, most such URLs, even for regular sites. There are at least some moderator-only URLs which, effectively, identify a user which I haven't checked to see if it handles.

This comment has been minimized.

Sign in to view

Copy link
@CoconutMacaroon

CoconutMacaroon Dec 25, 2022

Author Contributor

I'll give that function a try - I hadn't found it until after I wrote that code, but I'll check it out. It would also remove the requirement on tldextract as well
# extact the user ID from the URL
uid: int = int(regex.search(r"(?<=users\/)[0-9]+", url)[0])

# if we're given an SE account URL (not a per-site account)
if "stackexchange.com" in url and ".stackexchange.com" not in url:
return uid
# if we're given a per-site account
else:
# extract the URL information
extracted_url_data = tldextract.extract(url)

# if there is no subdomain, our site has a dedicated domain (i.e., askubuntu.com)
if extracted_url_data.subdomain == '':
sitename = extracted_url_data.domain
else:
# otherwise, our site is a subdomain of SE (i.e., sitename.stackexchange.com)
sitename = extracted_url_data.subdomain
return per_site_account_to_network_id(uid, sitename)


# --- Blacklist Functions --- #
# TODO: document these three new commands in the Wiki
@command(str)

This comment has been minimized.

Sign in to view

Copy link
@makyen

makyen Dec 24, 2022

Contributor

To a large extent, I'd expect that these should be leveraging the code in the existing isblu, addblu, and rmblu, either by being aliases for those, which are recognized and the network URL obtained prior to proceeding, or as separate functions, which then call those functions. Using an alias is probably easier. Calling the functions may be a bit more complex than just calling them, due to the @command decorator. What I did in get_number_bisect() is probably more than will be needed, but it's what was needed in order to call the bisect_number() command from another command. An alternative would be to put all the existing code in a separate function, which isn't an @command and then calling that from both functions, but using an alias seems easier.

This comment has been minimized.

Sign in to view

Copy link
@CoconutMacaroon

CoconutMacaroon Dec 25, 2022 •
edited

Author Contributor

Although I agree that most of that code can be used, I was thinking it would be better to have the !!/isblu (and similar) function do what they already do if they are given a per-site URL (of course), but if !!/isblu gts an SE profile URL, it does a site-wide blacklist, but a separate command like !!/isnetblu will just call isblu if it gets an SE URL, but !!/isnetblu could also accept a per-site URL and convert it to an SE URL so one doesn't have to manually find the SE URL for a user, and they can just use those separate commands with a normal per-site URL and it would do the lookup automatically

This comment has been minimized.

Sign in to view

Copy link
@makyen

makyen Dec 25, 2022

Contributor

Charcoal user's shouldn't need to run multiple commands to see if a user is actually blacklisted. It's fine/good to have one, or both, aliases which only provide a response for the specified site (e.g. !!/isblu-site) or the SE Network (!!/isblu-net and !!/isblu-network), but the normal !!/isblu <some site user URL> should respond with information for both the site user and network user. So, I'd expect the response to be something like:

User is not blacklisted (<site userID> on <site>) and not blacklisted network wide (<SE userID> on stackexchange.com).

or

User is blacklisted (<site userID> on <site>), but not network wide (<SE userID> on stackexchange.com).

or

User is blacklisted network wide (<SE userID> on stackexchange.com), which effectively blacklists them as user <site userID> on <site>, but they are not explicitly blacklisted on <site> and typically shouldn't be separately blacklisted on individual sites.

or

User is blacklisted (<site userID> on <site>) and blacklisted network wide (<SE userID> on stackexchange.com).

depending on the state of the user's blacklisting on the supplied site and blacklisting network wide.

If !!/isblu <SE network user URL>, then the response should just be about the network user, because we, obviously, can't know which site the user might be interested in. Alternately, we could check every site where the user has an account. Hmmm... that's probably desirable, or at least some way to do so.

We should probably also have users automatically removed from the blacklist as per-site when they are added network wide. Although, that's debatable.
def isnetblu(url):
# FIXME: This doesn't actually check anything currently (currently it
# just serves to verify that I can correctly add commands and that the
# functinality to lookup network IDs works correctly)
net_id = getnetid(url)
return "Blacklisted network ID " + str(net_id)
# remove that network ID from the blacklist


@command(str)
def addnetblu(url):
net_id = getnetid(url)
# add that network ID to the blacklist


@command(str)
def rmnetblu(url):
net_id = getnetid(url)
# check if that network ID is on the blacklist


# noinspection PyIncorrectDocstring,PyMissingTypeHints
@command(str, whole_msg=True, privileged=True)
def addblu(msg, user):
Expand Down

0 comments on commit 2db2bcc

Please sign in to comment.