interactions.py

interactions.py copied to clipboard

Problem Description

The recorder can filter recorded users by id, acting as a whitelist/allowlist. If you want it to function as a blacklist/denylist, then you are forced to whitelist all guild members minus the desired filtered users. This is very inconvenient, especially considering you would need to have an event handler to update the filter on every guild join event.

Proposed Solution

Refactor the way the recorder does filtering and add a recording mode to allow changing between allowlist and denylist behaviors.

Alternatives Considered

No response

Additional Information

No response

Code of Conduct

• [X] I agree to follow the contribution requirements.

To clarify: it should be as simple as adding an extra field to the class to serve as a blacklist of members (and adding basic logic to filter out said members).

To clarify: it should be as simple as adding an extra field to the class to serve as a blacklist of members (and adding basic logic to filter out said members).

There are two reasons I had for thinking a recording mode is better.

1. You cannot have a whitelist and a blacklist both being used simultaneously. If someone is not on the whitelist, they are inherently on the blacklist and vice-versa.
2. The usage could stay the same, namely in start_recording, but also in filter.

    async def start_recording(
        self,
        *user_ids: Snowflake_Type,
        recording_mode: RecordingMode | None = None,
        output_dir: str | None = None,
    ) -> None:
        ...

    def filter(self, *user_ids: Snowflake_Type, recording_mode: RecordingMode | None = None) -> None:
        """
        Filter the users that are being recorded.

        Args:
            *user_ids: The user_ids to either record or not record, depending on the recording mode, if not specified everyone will be included
            recording_mode: The recording mode to use, if not specified the recording list will act as an allowlist

        """
        self.recording_list = to_snowflake_list(unpack_helper(user_ids))

        if recording_mode is not None:
            self.recording_mode = recording_mode

    def is_filtered(self, user_id: Snowflake_Type) -> bool:
        """
        Checks if a user is filtered from being recorded.

        Args:
            user_id: The user_id to check

        Returns:
            True if the user is filtered from being recorded, otherwise False

        """
        match self.recording_mode:
            case RecordingMode.ALLOW:
                return self.recording_list and user_id not in self.recording_list
            case RecordingMode.DENY:
                return not self.recording_list or user_id in self.recording_list
            case _:
                return False

With the lists separate, the only way that start_recording can be changed to support starting in either whitelist or blacklist mode is to add a default parameter which can be used to infer which list to set from the *args. At that point, it may as well be one list with a recording mode as I was originally thinking.

    async def start_recording(
        self,
        *user_ids: Snowflake_Type,
        is_whitelist: bool = True,
        output_dir: str | None = None,
    ) -> None:
        ...

    def set_whitelist(self, *user_ids: Snowflake_Type) -> None:
        self.recording_blacklist = []
        self.recording_whitelist = to_snowflake_list(unpack_helper(user_ids))

    def set_blacklist(self, *user_ids: Snowflake_Type) -> None:
        self.recording_whitelist = []
        self.recording_blacklist = to_snowflake_list(unpack_helper(user_ids))

    def filter(self, *user_ids: Snowflake_Type, is_whitelist: bool = True) -> None:
        if is_whitelist:
            self.set_whitelist(*user_ids)
        else
            self.set_blacklist(*user_ids)

    def is_filtered(self, user_id: Snowflake_Type) -> bool:
        """
        Checks if a user is filtered from being recorded.

        Args:
            user_id: The user_id to check

        Returns:
            True if the user is filtered from being recorded, otherwise False

        """
        return (self.recording_whitelist and user_id not in self.recording_whitelist) or (
            self.recording_blacklist and user_id in self.recording_blacklist
        )

Honestly, that's the idea I had (more or less). Though I'd also rather not have the add_black/whitelist to prevent people from running into unexpected behavior (lots of debate can be had if whitelists and blacklists are mutually exclusive in context - in this case it is, but in some it's possible for it not to be).

Okay, I can inline those and swap things over to using the two lists on my feature branch.