From 4abe8715e0d2129b5b7f907c820cd23783db4db3 Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Sun, 11 Oct 2020 20:03:54 -0700 Subject: [PATCH 01/12] Adds pairing feature to rocket --- app/scheduler/__init__.py | 7 +++- app/scheduler/modules/pairing.py | 63 ++++++++++++++++++++++++++++++++ app/server.py | 7 +--- config/__init__.py | 4 +- docker-compose.yml | 1 + factory/__init__.py | 8 ++++ interface/slack.py | 37 +++++++++++++++++++ 7 files changed, 119 insertions(+), 8 deletions(-) create mode 100644 app/scheduler/modules/pairing.py diff --git a/app/scheduler/__init__.py b/app/scheduler/__init__.py index 8a003fac..83273602 100644 --- a/app/scheduler/__init__.py +++ b/app/scheduler/__init__.py @@ -3,7 +3,9 @@ from flask import Flask from apscheduler.schedulers.background import BackgroundScheduler from .modules.random_channel import RandomChannelPromoter +from .modules.pairing import Pairing from .modules.base import ModuleBase +from db.facade import DBFacade from typing import Tuple, List from config import Config @@ -13,10 +15,12 @@ class Scheduler: def __init__(self, scheduler: BackgroundScheduler, - args: Tuple[Flask, Config]): + args: Tuple[Flask, Config], + facade: DBFacade): """Initialize scheduler class.""" self.scheduler = scheduler self.args = args + self.facade = facade self.modules: List[ModuleBase] = [] self.__init_periodic_tasks() @@ -35,3 +39,4 @@ def __add_job(self, module: ModuleBase): def __init_periodic_tasks(self): """Add jobs that fire every interval.""" self.__add_job(RandomChannelPromoter(*self.args)) + self.__add_job(Pairing(*self.args, self.facade)) diff --git a/app/scheduler/modules/pairing.py b/app/scheduler/modules/pairing.py new file mode 100644 index 00000000..8ed8cf8f --- /dev/null +++ b/app/scheduler/modules/pairing.py @@ -0,0 +1,63 @@ +"""Match two Launchpad member for a private conversation""" +from slack import WebClient +from interface.slack import Bot +from random import shuffle +from .base import ModuleBase +from typing import Dict, List, Tuple, Any +from flask import Flask +from config import Config +from db.facade import DBFacade +import logging + + +class Pairing(ModuleBase): + """Module that matches 2 launchpad members each week""" + + NAME = 'Match launch pad members randomly' + + def __init__(self, + flask_app: Flask, + config: Config, + facade: DBFacade): + """Initialize the object.""" + self.bot = Bot(WebClient(config.slack_api_token), + config.slack_notification_channel) + self.channel_id = self.bot.get_channel_id(config.slack_pairing_channel) + self.facade = facade + + + def get_job_args(self) -> Dict[str, Any]: + """Get job configuration arguments for apscheduler.""" + return {'trigger': 'cron', + 'minute': '*', + 'name': self.NAME} + + def do_it(self): + """Pair users together, and create a private chat for them""" + users = self.bot.get_channel_users(self.channel_id) + logging.debug(f"users of the pairing channel are {users}") + matched_user_pairs = self.__pair_users(users) + for pair in matched_user_pairs: + group_name = self.bot.create_private_chat(pair) + logging.info(f"The name of the created group name is {group_name}") + self.bot.send_to_channel("Hello! You have been matched by Rocket. " + + "please use this channel to get to know each other!", group_name) + + def __pair_users(self, users: List[str]) -> List[List[str]]: + """ + Creates pairs of users that haven't been matched before + """ + shuffle(users) + pairs = [] + pair = [] + for i, user in enumerate(users): + pair.append(user) + if i % 2 != 0: + pairs.append(pair) + pair = [] + # If we have an odd number of people that is not 1 + # We put the odd person out in one of the groups + # So we might have a group of 3 + if len(pair) == 1 and len(pairs) > 0: + pairs[len(pairs) - 1].append(pair[0]) + return pairs diff --git a/app/server.py b/app/server.py index 0052239d..613537b0 100644 --- a/app/server.py +++ b/app/server.py @@ -1,15 +1,13 @@ """Flask server instance.""" from factory import make_command_parser, make_github_webhook_handler, \ - make_slack_events_handler, make_github_interface + make_slack_events_handler, make_github_interface, make_event_scheduler from flask import Flask, request from logging.config import dictConfig from slackeventsapi import SlackEventAdapter -from apscheduler.schedulers.background import BackgroundScheduler import logging import structlog from flask_talisman import Talisman from config import Config -from app.scheduler import Scheduler from interface.slack import Bot from slack import WebClient from boto3.session import Session @@ -81,8 +79,7 @@ slack_events_adapter = SlackEventAdapter(config.slack_signing_secret, "/slack/events", app) -sched = Scheduler(BackgroundScheduler(timezone="America/Los_Angeles"), - (app, config)) +sched = make_event_scheduler(app, config) sched.start() bot = Bot(WebClient(config.slack_api_token), diff --git a/config/__init__.py b/config/__init__.py index 866abb91..00de4ad6 100644 --- a/config/__init__.py +++ b/config/__init__.py @@ -17,7 +17,7 @@ class Config: 'SLACK_API_TOKEN': 'slack_api_token', 'SLACK_NOTIFICATION_CHANNEL': 'slack_notification_channel', 'SLACK_ANNOUNCEMENT_CHANNEL': 'slack_announcement_channel', - + 'SLACK_PAIRING_CHANNEL': 'slack_pairing_channel', 'GITHUB_APP_ID': 'github_app_id', 'GITHUB_ORG_NAME': 'github_org_name', 'GITHUB_DEFAULT_TEAM_NAME': 'github_team_all', @@ -89,7 +89,7 @@ def _set_attrs(self): self.slack_api_token = '' self.slack_notification_channel = '' self.slack_announcement_channel = '' - + self.slack_pairing_channel = '' self.github_app_id = '' self.github_org_name = '' self.github_team_all = '' diff --git a/docker-compose.yml b/docker-compose.yml index b4176389..0db186f4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,6 +22,7 @@ services: environment: - SLACK_NOTIFICATION_CHANNEL=${SLACK_NOTIFICATION_CHANNEL} - SLACK_ANNOUNCEMENT_CHANNEL=${SLACK_ANNOUNCEMENT_CHANNEL} + - SLACK_PAIRING_CHANNEL=${SLACK_PAIRING_CHANNEL} - SLACK_SIGNING_SECRET=${SLACK_SIGNING_SECRET} - SLACK_API_TOKEN=${SLACK_API_TOKEN} - GITHUB_APP_ID=${GITHUB_APP_ID} diff --git a/factory/__init__.py b/factory/__init__.py index caf61563..a305ce29 100644 --- a/factory/__init__.py +++ b/factory/__init__.py @@ -20,6 +20,9 @@ from google.oauth2 import service_account as gcp_service_account from googleapiclient.discovery import build as gcp_build from typing import Optional +from flask import Flask +from app.scheduler import Scheduler +from apscheduler.schedulers.background import BackgroundScheduler def make_dbfacade(config: Config) -> DBFacade: @@ -85,6 +88,11 @@ def make_gcp_client(config: Config) -> Optional[GCPInterface]: drive = gcp_build('drive', 'v3', credentials=credentials) return GCPInterface(drive, subject=config.gcp_service_account_subject) +def make_event_scheduler(app: Flask, config: Config) -> Scheduler: + background_scheduler = BackgroundScheduler(timezone="America/Los_Angeles") + facade = make_dbfacade(config) + return Scheduler(background_scheduler, (app, config), facade) + def create_signing_token() -> str: """Create a new, random signing token.""" diff --git a/interface/slack.py b/interface/slack.py index 63aad99a..da7e02b9 100644 --- a/interface/slack.py +++ b/interface/slack.py @@ -110,6 +110,43 @@ def send_event_notif(self, message): logging.error("Webhook notif failed to send due to {} error.". format(se.error)) + def create_private_chat(self, users: List[str]) -> str: + """ + Create a private chat with the given users + + :param users: the list of users to add to the private chat + + :raise SlackAPIError if the slack API returned error openning the chat + + :return The name of of the private chat created + """ + logging.debug(f"Attempting to open a private conversation with users {users}") + response = self.sc.conversations_open(users = users) + if response['ok']: + logging.debug(f"Successfly opened a converation with the name {response['channel']['name']}") + return response['channel']['name'] + raise SlackAPIError(response['error']) + + def get_channel_id(self, channel_name: str) -> str: + """ + Retrieves a channel's id given it's name + + :param channel_name: The name of the channel + + :raise SlackAPIError if no channels were found with the name `channel_name` + + :return the slack id of the channel + """ + # We strip away the "#" in case it was provided with the channel name + channel_name = channel_name.replace("#", "") + logging.debug(f"Attempting to get the id of channel {channel_name}") + channels = list(filter(lambda c: c['name'] == channel_name, self.get_channels())) + if len(channels) == 0: + raise SlackAPIError(f"No channels found with the name{channel_name}") + if len(channels) != 1: + logging.warning(f"Somehow there is more than one channel with the name {channel_name}") + return channels[0]['id'] + class SlackAPIError(Exception): """Exception representing an error while calling Slack API.""" From 0c9f9b215d52ab013fc7c9b8f150b17ca402decd Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Tue, 13 Oct 2020 10:44:56 -0700 Subject: [PATCH 02/12] Add pairings table to dynamodb, with ttl stub --- config/__init__.py | 2 ++ db/dynamodb.py | 25 ++++++++++++++++++++++--- docker-compose.yml | 1 + sample-env | 1 + 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/config/__init__.py b/config/__init__.py index 00de4ad6..31627ef9 100644 --- a/config/__init__.py +++ b/config/__init__.py @@ -31,6 +31,7 @@ class Config: 'AWS_SECRET_KEY': 'aws_secret_key', 'AWS_USERS_TABLE': 'aws_users_tablename', 'AWS_TEAMS_TABLE': 'aws_teams_tablename', + 'AWS_PAIRINGS_TABLE': 'aws_pairings_tablename', 'AWS_PROJECTS_TABLE': 'aws_projects_tablename', 'AWS_REGION': 'aws_region', 'AWS_LOCAL': 'aws_local', @@ -103,6 +104,7 @@ def _set_attrs(self): self.aws_secret_key = '' self.aws_users_tablename = '' self.aws_teams_tablename = '' + self.aws_pairings_tablename = '' self.aws_projects_tablename = '' self.aws_region = '' self.aws_local: bool = False diff --git a/db/dynamodb.py b/db/dynamodb.py index 51d1744d..35731dea 100644 --- a/db/dynamodb.py +++ b/db/dynamodb.py @@ -3,7 +3,7 @@ from boto3.dynamodb.conditions import Attr from functools import reduce, wraps -from app.model import User, Team, Project +from app.model import User, Team, Project, Pairing from typing import Tuple, List, Type, TypeVar from config import Config from db.facade import DBFacade @@ -44,6 +44,7 @@ def __init__(self, config: Config): self.users_table: str = config.aws_users_tablename self.teams_table: str = config.aws_teams_tablename self.projects_table: str = config.aws_projects_tablename + self.pairings_table: str = config.aws_pairings_tablename def get_table_name(self, cls: Type[T]) -> str: """ @@ -59,6 +60,8 @@ def get_table_name(self, cls: Type[T]) -> str: return self.teams_table elif cls == Project: return self.projects_table + elif cls == Pairing: + return self.pairings_table else: raise TypeError('Type of class one of [User, Team, Project]') @@ -76,6 +79,8 @@ def get_key(self, table_name: str) -> str: return 'github_team_id' elif table_name == self.projects_table: return 'project_id' + elif table_name == self.pairings_table: + return 'pairing_id else: raise TypeError('Table name does not correspond to anything') @@ -87,7 +92,7 @@ def get_set_attrs(self, table_name: str) -> List[str]: :raise: TypeError if table does not exist :return: list of strings of set attributes """ - if table_name == self.users_table: + if table_name == self.users_table or table_name == self.projects_table: return [] elif table_name == self.teams_table: return ['team_leads', 'members'] @@ -117,6 +122,7 @@ def __init__(self, config: Config): self.users_table = config.aws_users_tablename self.teams_table = config.aws_teams_tablename self.projects_table = config.aws_projects_tablename + self.pairings_table = config.aws_pairings_tablename self.CONST = DynamoDB.Const(config) if config.aws_local: @@ -143,6 +149,9 @@ def __init__(self, config: Config): self.__create_table(self.teams_table) if not self.check_valid_table(self.projects_table): self.__create_table(self.projects_table) + if not self.check_valid_table(self.pairings_table): + self.__create_table(self.pairings_table) + self.__enable_ttl(self.pairings_table) def __create_table(self, table_name: str, key_type: str = 'S'): """ @@ -152,7 +161,6 @@ def __create_table(self, table_name: str, key_type: str = 'S'): only be called on initialization. :param table_name: name of the table to create - :param primary_key: name of the primary key for the table :param key_type: type of primary key (S, N, B) """ logging.info(f"Creating table '{table_name}'") @@ -177,6 +185,17 @@ def __create_table(self, table_name: str, key_type: str = 'S'): } ) + def __enable_ttl(self, table_name: str): + """ + Enable automatically dropping entries on TTL expiry + + **Note**: This function should **not** be called externally, and should + only be called on initialization. + + :param table_name: name of the table to enable ttl for + """ + # Stub + def check_valid_table(self, table_name: str) -> bool: """ Check if table with ``table_name`` exists. diff --git a/docker-compose.yml b/docker-compose.yml index 0db186f4..8ae16a04 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -37,6 +37,7 @@ services: - AWS_SECRET_KEY=${AWS_SECRET_KEY} - AWS_USERS_TABLE=${AWS_USERS_TABLE} - AWS_TEAMS_TABLE=${AWS_TEAMS_TABLE} + - AWS_PAIRINGS_TABLE=${AWS_PAIRINGS_TABLE} - AWS_PROJECTS_TABLE=${AWS_PROJECTS_TABLE} - AWS_REGION=${AWS_REGION} - AWS_LOCAL=${AWS_LOCAL} diff --git a/sample-env b/sample-env index 334ef7bc..c6ae3ddc 100644 --- a/sample-env +++ b/sample-env @@ -18,6 +18,7 @@ AWS_ACCESS_KEYID='53' AWS_SECRET_KEY='itsa secret' AWS_USERS_TABLE='users' AWS_TEAMS_TABLE='teams' +AWS_PAIRINGS_TABLE='pairings' AWS_PROJECTS_TABLE='projects' AWS_REGION='us-west-2' AWS_LOCAL='False' # set to 'True' to use local DynamoDB From f76a14a27cb723a5ccbb520b34ce2cf4e8d845dc Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Tue, 13 Oct 2020 11:00:27 -0700 Subject: [PATCH 03/12] Adds pairing model --- app/model/__init__.py | 2 + app/model/pairing.py | 111 ++++++++++++++++++++++++++++++++++++++++++ db/dynamodb.py | 2 +- 3 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 app/model/pairing.py diff --git a/app/model/__init__.py b/app/model/__init__.py index f15e13f0..7a2f769b 100644 --- a/app/model/__init__.py +++ b/app/model/__init__.py @@ -3,10 +3,12 @@ import app.model.team as team import app.model.permissions as permissions import app.model.project as project +import app.model.pairing as pairing import app.model.base as base User = user.User Team = team.Team Permissions = permissions.Permissions Project = project.Project +Pairing = pairing.Pairing BaseModel = base.RocketModel diff --git a/app/model/pairing.py b/app/model/pairing.py new file mode 100644 index 00000000..a9e6c7eb --- /dev/null +++ b/app/model/pairing.py @@ -0,0 +1,111 @@ +"""Represent a Pairing between two users.""" +from typing import List, Dict, Any, TypeVar, Type +import uuid +from app.model.base import RocketModel + +T = TypeVar('T', bound='Pairing') + + +class Pairing(RocketModel): + """Represent a pairing and related fields and methods.""" + + def __init__(self, + user1_slack_id: str, + user2_slack_id: str): + """ + Initialize the pairing. + + Pairing ID is a UUID generated by ``uuid.uuid4()``. + + :param user1_slack_id: The slack ID of the first user + :param user2_slack_id: The slack ID of the second user + """ + self.pairing_id = str(uuid.uuid4()) + self.user1_slack_id = user1_slack_id + self.user2_slack_id = user2_slack_id + self.ttl = "" # TODO + + def get_attachment(self) -> Dict[str, Any]: + """Return slack-formatted attachment (dictionary) for pairing.""" + text_pairs = [ + ('Pairing ID', self.pairing_id), + ('User 1 Slack ID', self.user1_slack_id), + ('User 2 Slack ID', self.user2_slack_id), + ('TTL', self.ttl) + ] + + fields = [{'title': t, 'value': v if v else 'n/a', 'short': True} + for t, v in text_pairs] + fallback = str('\n'.join(map(str, text_pairs))) + + return {'fallback': fallback, 'fields': fields} + + @classmethod + def from_dict(cls: Type[T], d: Dict[str, Any]) -> T: + """ + Return a pairing from a dict object. + + :param d: the dictionary (usually from DynamoDB) + :return: a Pairing object + """ + p = cls(d['user1_slack_id'], d['user2_slack_id']) + p.pairing_id = d['pairing_id'] + p.ttl = d['ttl'] + return p + + @classmethod + def to_dict(cls: Type[T], p: T) -> Dict[str, Any]: + """ + Return a dict object representing a pairing. + + The difference with the in-built ``self.__dict__`` is that this is more + compatible with storing into NoSQL databases like DynamoDB. + + :param p: the Pairing object + :return: a dictionary representing a pairing + """ + def place_if_filled(name: str, field: Any): + """Populate ``udict`` if ``field`` isn't empty.""" + if field: + udict[name] = field + + udict = { + 'pairing_id': p.pairing_id, + 'user1_slack_id': p.user1_slack_id, + 'user2_slack_id': p.user2_slack_id + } + place_if_filled('ttl', p.ttl) + + return udict + + @classmethod + def is_valid(cls: Type[T], p: T) -> bool: + """ + Return true if this pairing has no missing fields. + + Required fields for database to accept: + - ``__pairing_id`` + - ``__user1_slack_id`` + - ``__user2_slack_id`` + + :param pairing: pairing to check + :return: true if this pairing has no missing fields + """ + return len(p.pairing_id) > 0 and\ + len(p.user1_slack_id) > 0 and len(p.user2_slack_id) + + def __eq__(self, other: object) -> bool: + """Return true if this pairing is equal to the other pairing.""" + return isinstance(other, Pairing) and str(self) == str(other) + + def __ne__(self, other: object) -> bool: + """Return true if this pairing isn't equal to the other pairing.""" + return not (self == other) + + def __str__(self) -> str: + """Return all fields of this pairing, JSON format.""" + return str(self.__dict__) + + def __hash__(self) -> int: + """Hash the pairing class using a dictionary.""" + return self.__str__().__hash__() diff --git a/db/dynamodb.py b/db/dynamodb.py index 35731dea..2035b0b0 100644 --- a/db/dynamodb.py +++ b/db/dynamodb.py @@ -80,7 +80,7 @@ def get_key(self, table_name: str) -> str: elif table_name == self.projects_table: return 'project_id' elif table_name == self.pairings_table: - return 'pairing_id + return 'pairing_id' else: raise TypeError('Table name does not correspond to anything') From eb3ff7da6bc4fa1520a4e054dc3611dd97c6248c Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Tue, 13 Oct 2020 12:18:33 -0700 Subject: [PATCH 04/12] Adds persistance to pairing algorithm --- app/model/pairing.py | 2 +- app/scheduler/__init__.py | 4 +-- app/scheduler/modules/pairing.py | 58 ++++++++++++++++++++++++++------ db/dynamodb.py | 12 +++++-- db/facade.py | 10 ++++++ 5 files changed, 71 insertions(+), 15 deletions(-) diff --git a/app/model/pairing.py b/app/model/pairing.py index a9e6c7eb..332221b0 100644 --- a/app/model/pairing.py +++ b/app/model/pairing.py @@ -23,7 +23,7 @@ def __init__(self, self.pairing_id = str(uuid.uuid4()) self.user1_slack_id = user1_slack_id self.user2_slack_id = user2_slack_id - self.ttl = "" # TODO + self.ttl = "TODO" # TODO def get_attachment(self) -> Dict[str, Any]: """Return slack-formatted attachment (dictionary) for pairing.""" diff --git a/app/scheduler/__init__.py b/app/scheduler/__init__.py index 83273602..4d4d0ba8 100644 --- a/app/scheduler/__init__.py +++ b/app/scheduler/__init__.py @@ -3,7 +3,7 @@ from flask import Flask from apscheduler.schedulers.background import BackgroundScheduler from .modules.random_channel import RandomChannelPromoter -from .modules.pairing import Pairing +from .modules.pairing import PairingSchedule from .modules.base import ModuleBase from db.facade import DBFacade from typing import Tuple, List @@ -39,4 +39,4 @@ def __add_job(self, module: ModuleBase): def __init_periodic_tasks(self): """Add jobs that fire every interval.""" self.__add_job(RandomChannelPromoter(*self.args)) - self.__add_job(Pairing(*self.args, self.facade)) + self.__add_job(PairingSchedule(*self.args, self.facade)) diff --git a/app/scheduler/modules/pairing.py b/app/scheduler/modules/pairing.py index 8ed8cf8f..0bf1094f 100644 --- a/app/scheduler/modules/pairing.py +++ b/app/scheduler/modules/pairing.py @@ -3,14 +3,15 @@ from interface.slack import Bot from random import shuffle from .base import ModuleBase -from typing import Dict, List, Tuple, Any +from typing import Dict, List, Tuple, Any, Set from flask import Flask from config import Config from db.facade import DBFacade +from app.model import Pairing import logging -class Pairing(ModuleBase): +class PairingSchedule(ModuleBase): """Module that matches 2 launchpad members each week""" NAME = 'Match launch pad members randomly' @@ -46,18 +47,55 @@ def do_it(self): def __pair_users(self, users: List[str]) -> List[List[str]]: """ Creates pairs of users that haven't been matched before + + :param users: A list of slack ids of all users to match + + If a pairing cannot be done, then the history of pairings is + purged, and the algorithm is run again. """ + # TODO: Clean this up into a more concrete algorithm shuffle(users) + already_added = set() pairs = [] - pair = [] for i, user in enumerate(users): - pair.append(user) - if i % 2 != 0: - pairs.append(pair) - pair = [] + if user in already_added: + continue + previously_paired = self.__get_previous_pairs(user) + for j in range(i + 1, len(users)): + other_user = users[j] + if other_user not in previously_paired and other_user not in already_added: + self.__persist_pairing(user, other_user) + pairs.append([user, other_user]) + already_added.add(user) + already_added.add(other_user) + break + not_paired = list(filter(lambda user: user not in already_added, users)) # If we have an odd number of people that is not 1 # We put the odd person out in one of the groups # So we might have a group of 3 - if len(pair) == 1 and len(pairs) > 0: - pairs[len(pairs) - 1].append(pair[0]) - return pairs + if len(not_paired) == 1 and len(pairs) > 0: + pairs[len(pairs) - 1].append(not_paired[0]) + elif len(not_paired) > 1: + self.__purge_pairings() + return self.__pair_users(users) + else: + return pairs + + def __get_previous_pairs(self, user: str) -> Set[str]: + logging.info(f"Getting previous pairs for {user}") + pairings = self.facade.query_or(Pairing, [('user1_slack_id', user), ('user2_slack_id', user)]) + res = set() + for pairing in pairings: + other = pairing.user1_slack_id if pairing.user2_slack_id == user else pairing.user2_slack_id + res.add(other) + logging.info(f"Previous pairings are {res}") + return res + + def __persist_pairing(self, user1_slack_id: str, user2_slack_id: str): + pairing = Pairing(user1_slack_id, user2_slack_id) + reverse_pairing = Pairing(user2_slack_id, user1_slack_id) + self.facade.store(pairing) + self.facade.store(reverse_pairing) + + def __purge_pairings(self): + self.facade.delete_all(Pairing) \ No newline at end of file diff --git a/db/dynamodb.py b/db/dynamodb.py index 2035b0b0..b2c19168 100644 --- a/db/dynamodb.py +++ b/db/dynamodb.py @@ -92,7 +92,7 @@ def get_set_attrs(self, table_name: str) -> List[str]: :raise: TypeError if table does not exist :return: list of strings of set attributes """ - if table_name == self.users_table or table_name == self.projects_table: + if table_name == self.users_table or table_name == self.pairings_table: return [] elif table_name == self.teams_table: return ['team_leads', 'members'] @@ -209,7 +209,7 @@ def check_valid_table(self, table_name: str) -> bool: def store(self, obj: T) -> bool: Model = obj.__class__ - if Model not in [User, Team, Project]: + if Model not in [User, Team, Project, Pairing]: logging.error(f"Cannot store object {str(obj)}") raise RuntimeError(f'Cannot store object{str(obj)}') @@ -309,3 +309,11 @@ def delete(self, Model: Type[T], k: str): self.CONST.get_key(table_name): k } ) + + def delete_all(self, Model: Type[T]): + logging.info(f"Deleting {Model.__name__}") + table_name = self.CONST.get_table_name(Model) + table = self.ddb.Table(table_name) + table.delete() + table.wait_until_not_exists() + self.__create_table(table_name) diff --git a/db/facade.py b/db/facade.py index c2af3add..bcde0184 100644 --- a/db/facade.py +++ b/db/facade.py @@ -142,3 +142,13 @@ def delete(self, Model: Type[T], k: str): :param k: ID or key of the object to remove (must be primary key) """ raise NotImplementedError + + @abstractmethod + def delete_all(self, Model: Type[T]): + """ + Remove all entries from a table + + :param Model: The table to remove all entries of + """ + + raise NotImplementedError From 6c07dc3db01277335b5b36c8d2863fca14143454 Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Sat, 31 Oct 2020 22:44:05 -0700 Subject: [PATCH 05/12] Adds tests for slack api additions --- app/model/pairing.py | 6 +++--- app/scheduler/modules/pairing.py | 34 +++++++++++++++++++++----------- db/dynamodb.py | 5 +++-- db/facade.py | 5 +++-- factory/__init__.py | 1 + interface/slack.py | 28 +++++++++++++++++--------- tests/app/scheduler/base_test.py | 16 +++++++++------ tests/config/config_test.py | 2 ++ tests/db/dynamodb_test.py | 2 ++ tests/interface/slack_test.py | 34 ++++++++++++++++++++++++++++++++ tests/memorydb.py | 9 +++++++-- 11 files changed, 106 insertions(+), 36 deletions(-) diff --git a/app/model/pairing.py b/app/model/pairing.py index 332221b0..ff3cf3c9 100644 --- a/app/model/pairing.py +++ b/app/model/pairing.py @@ -1,5 +1,5 @@ """Represent a Pairing between two users.""" -from typing import List, Dict, Any, TypeVar, Type +from typing import Dict, Any, TypeVar, Type import uuid from app.model.base import RocketModel @@ -23,7 +23,7 @@ def __init__(self, self.pairing_id = str(uuid.uuid4()) self.user1_slack_id = user1_slack_id self.user2_slack_id = user2_slack_id - self.ttl = "TODO" # TODO + self.ttl = "TODO" # TODO def get_attachment(self) -> Dict[str, Any]: """Return slack-formatted attachment (dictionary) for pairing.""" @@ -92,7 +92,7 @@ def is_valid(cls: Type[T], p: T) -> bool: :return: true if this pairing has no missing fields """ return len(p.pairing_id) > 0 and\ - len(p.user1_slack_id) > 0 and len(p.user2_slack_id) + len(p.user1_slack_id) > 0 and len(p.user2_slack_id) > 0 def __eq__(self, other: object) -> bool: """Return true if this pairing is equal to the other pairing.""" diff --git a/app/scheduler/modules/pairing.py b/app/scheduler/modules/pairing.py index 0bf1094f..720f5337 100644 --- a/app/scheduler/modules/pairing.py +++ b/app/scheduler/modules/pairing.py @@ -3,7 +3,7 @@ from interface.slack import Bot from random import shuffle from .base import ModuleBase -from typing import Dict, List, Tuple, Any, Set +from typing import Dict, List, Any, Set from flask import Flask from config import Config from db.facade import DBFacade @@ -26,7 +26,6 @@ def __init__(self, self.channel_id = self.bot.get_channel_id(config.slack_pairing_channel) self.facade = facade - def get_job_args(self) -> Dict[str, Any]: """Get job configuration arguments for apscheduler.""" return {'trigger': 'cron', @@ -41,8 +40,10 @@ def do_it(self): for pair in matched_user_pairs: group_name = self.bot.create_private_chat(pair) logging.info(f"The name of the created group name is {group_name}") - self.bot.send_to_channel("Hello! You have been matched by Rocket. " + - "please use this channel to get to know each other!", group_name) + self.bot.send_to_channel("Hello! \ + You have been matched by Rocket \ + please use this channel to get to know each other!", + group_name) def __pair_users(self, users: List[str]) -> List[List[str]]: """ @@ -54,6 +55,7 @@ def __pair_users(self, users: List[str]) -> List[List[str]]: purged, and the algorithm is run again. """ # TODO: Clean this up into a more concrete algorithm + logging.info("Running pairing algorithm") shuffle(users) already_added = set() pairs = [] @@ -63,34 +65,41 @@ def __pair_users(self, users: List[str]) -> List[List[str]]: previously_paired = self.__get_previous_pairs(user) for j in range(i + 1, len(users)): other_user = users[j] - if other_user not in previously_paired and other_user not in already_added: + if other_user not in previously_paired and \ + other_user not in already_added: self.__persist_pairing(user, other_user) pairs.append([user, other_user]) already_added.add(user) already_added.add(other_user) break - not_paired = list(filter(lambda user: user not in already_added, users)) + not_paired = list( + filter(lambda user: user not in already_added, users)) # If we have an odd number of people that is not 1 # We put the odd person out in one of the groups # So we might have a group of 3 if len(not_paired) == 1 and len(pairs) > 0: pairs[len(pairs) - 1].append(not_paired[0]) + # In the case the algorithm failed, purge pairings and repeat elif len(not_paired) > 1: + logging.info("Failed to pair users, purging and trying again..") self.__purge_pairings() return self.__pair_users(users) - else: - return pairs + logging.info("Done pairing algorithm") + return pairs def __get_previous_pairs(self, user: str) -> Set[str]: logging.info(f"Getting previous pairs for {user}") - pairings = self.facade.query_or(Pairing, [('user1_slack_id', user), ('user2_slack_id', user)]) + pairings = self.facade.query_or(Pairing, + [('user1_slack_id', user), + ('user2_slack_id', user)]) res = set() for pairing in pairings: - other = pairing.user1_slack_id if pairing.user2_slack_id == user else pairing.user2_slack_id + other = pairing.user1_slack_id if pairing.user2_slack_id == user \ + else pairing.user2_slack_id res.add(other) logging.info(f"Previous pairings are {res}") return res - + def __persist_pairing(self, user1_slack_id: str, user2_slack_id: str): pairing = Pairing(user1_slack_id, user2_slack_id) reverse_pairing = Pairing(user2_slack_id, user1_slack_id) @@ -98,4 +107,5 @@ def __persist_pairing(self, user1_slack_id: str, user2_slack_id: str): self.facade.store(reverse_pairing) def __purge_pairings(self): - self.facade.delete_all(Pairing) \ No newline at end of file + logging.info("Deleting all pairings") + self.facade.delete_all(Pairing) diff --git a/db/dynamodb.py b/db/dynamodb.py index b2c19168..a82d099c 100644 --- a/db/dynamodb.py +++ b/db/dynamodb.py @@ -8,7 +8,7 @@ from config import Config from db.facade import DBFacade -T = TypeVar('T', User, Team, Project) +T = TypeVar('T', User, Team, Project, Pairing) def fragment(items_per_call=100): @@ -92,7 +92,8 @@ def get_set_attrs(self, table_name: str) -> List[str]: :raise: TypeError if table does not exist :return: list of strings of set attributes """ - if table_name == self.users_table or table_name == self.pairings_table: + if table_name == self.users_table or \ + table_name == self.pairings_table: return [] elif table_name == self.teams_table: return ['team_leads', 'members'] diff --git a/db/facade.py b/db/facade.py index bcde0184..2a6decdc 100644 --- a/db/facade.py +++ b/db/facade.py @@ -2,10 +2,11 @@ from app.model.user import User from app.model.team import Team from app.model.project import Project +from app.model.pairing import Pairing from typing import List, Tuple, TypeVar, Type from abc import ABC, abstractmethod -T = TypeVar('T', User, Team, Project) +T = TypeVar('T', User, Team, Project, Pairing) class DBFacade(ABC): @@ -142,7 +143,7 @@ def delete(self, Model: Type[T], k: str): :param k: ID or key of the object to remove (must be primary key) """ raise NotImplementedError - + @abstractmethod def delete_all(self, Model: Type[T]): """ diff --git a/factory/__init__.py b/factory/__init__.py index a305ce29..654214c2 100644 --- a/factory/__init__.py +++ b/factory/__init__.py @@ -88,6 +88,7 @@ def make_gcp_client(config: Config) -> Optional[GCPInterface]: drive = gcp_build('drive', 'v3', credentials=credentials) return GCPInterface(drive, subject=config.gcp_service_account_subject) + def make_event_scheduler(app: Flask, config: Config) -> Scheduler: background_scheduler = BackgroundScheduler(timezone="America/Los_Angeles") facade = make_dbfacade(config) diff --git a/interface/slack.py b/interface/slack.py index da7e02b9..346748b2 100644 --- a/interface/slack.py +++ b/interface/slack.py @@ -110,7 +110,7 @@ def send_event_notif(self, message): logging.error("Webhook notif failed to send due to {} error.". format(se.error)) - def create_private_chat(self, users: List[str]) -> str: + def create_private_chat(self, users: List[str]): """ Create a private chat with the given users @@ -120,31 +120,41 @@ def create_private_chat(self, users: List[str]) -> str: :return The name of of the private chat created """ - logging.debug(f"Attempting to open a private conversation with users {users}") - response = self.sc.conversations_open(users = users) + logging.debug( + f"Attempting to open a private conversation with users {users}") + response = self.sc.conversations_open(users=users) if response['ok']: - logging.debug(f"Successfly opened a converation with the name {response['channel']['name']}") + logging.debug( + f"Successfly opened a converation with the name \ + {response['channel']['name']}") return response['channel']['name'] raise SlackAPIError(response['error']) - def get_channel_id(self, channel_name: str) -> str: + def get_channel_id(self, channel_name: str): """ Retrieves a channel's id given it's name :param channel_name: The name of the channel - :raise SlackAPIError if no channels were found with the name `channel_name` + :raise SlackAPIError if no channels were found with the name + `channel_name` :return the slack id of the channel """ # We strip away the "#" in case it was provided with the channel name channel_name = channel_name.replace("#", "") logging.debug(f"Attempting to get the id of channel {channel_name}") - channels = list(filter(lambda c: c['name'] == channel_name, self.get_channels())) + channels = list( + filter(lambda c: c['name'] == channel_name, + self.get_channels())) if len(channels) == 0: - raise SlackAPIError(f"No channels found with the name{channel_name}") + raise SlackAPIError( + f"No channels found with the name {channel_name}") if len(channels) != 1: - logging.warning(f"Somehow there is more than one channel with the name {channel_name}") + logging.warning( + f"Somehow there is more than one channel\ + with the name {channel_name}" + ) return channels[0]['id'] diff --git a/tests/app/scheduler/base_test.py b/tests/app/scheduler/base_test.py index 188e0843..25c96a4c 100644 --- a/tests/app/scheduler/base_test.py +++ b/tests/app/scheduler/base_test.py @@ -1,10 +1,12 @@ """Test Scheduler base class.""" from unittest import TestCase -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from flask import Flask from apscheduler.schedulers.background import BackgroundScheduler from app.scheduler import Scheduler +from interface.slack import Bot from config import Config +from tests.memorydb import MemoryDB class TestScheduler(TestCase): @@ -16,14 +18,16 @@ def setUp(self): self.flask = MagicMock(Flask) self.args = (self.flask, self.config) self.bgsched = MagicMock(BackgroundScheduler) - + self.db = MemoryDB() self.config.slack_announcement_channel = "#general" self.config.slack_notification_channel = "#general" self.config.slack_api_token = "sometoken.exe" + self.config.slack_pairing_channel = "#general" - def test_proper_initialization(self): + @patch.object(Bot, 'get_channel_id') + def test_proper_initialization(self, other): """Test proper initialization with proper arguments.""" - s = Scheduler(self.bgsched, self.args) + s = Scheduler(self.bgsched, self.args, self.db) - self.assertEqual(self.bgsched.add_job.call_count, 1) - self.assertEqual(len(s.modules), 1) + self.assertEqual(self.bgsched.add_job.call_count, 2) + self.assertEqual(len(s.modules), 2) diff --git a/tests/config/config_test.py b/tests/config/config_test.py index 92975e68..a8086d4a 100644 --- a/tests/config/config_test.py +++ b/tests/config/config_test.py @@ -14,6 +14,7 @@ def setUp(self): 'SLACK_API_TOKEN': 'some token idk', 'SLACK_NOTIFICATION_CHANNEL': '#rocket2', 'SLACK_ANNOUNCEMENT_CHANNEL': '#ot-random', + 'SLACK_PAIRING_CHANNEL': "#pairing", 'GITHUB_APP_ID': '2024', 'GITHUB_ORG_NAME': 'ubclaunchpad', @@ -27,6 +28,7 @@ def setUp(self): 'AWS_USERS_TABLE': 'users', 'AWS_TEAMS_TABLE': 'teams', 'AWS_PROJECTS_TABLE': 'projects', + 'AWS_PAIRINGS_TABLE': 'pairings', 'AWS_REGION': 'us-west-2', 'AWS_LOCAL': 'True', diff --git a/tests/db/dynamodb_test.py b/tests/db/dynamodb_test.py index 9210018a..4d8aed7c 100644 --- a/tests/db/dynamodb_test.py +++ b/tests/db/dynamodb_test.py @@ -19,6 +19,7 @@ def setUp(self): self.config.aws_users_tablename = 'users' self.config.aws_teams_tablename = 'teams' self.config.aws_projects_tablename = 'projects' + self.config.aws_pairings_tablename = 'pairings' self.const = DynamoDB.Const(self.config) def test_get_bad_table_name(self): @@ -43,6 +44,7 @@ def setUp(self): self.config.aws_users_tablename = 'users_test' self.config.aws_teams_tablename = 'teams_test' self.config.aws_projects_tablename = 'projects_test' + self.config.aws_pairings_tablename = 'pairings_test' self.config.aws_local = True self.ddb = DynamoDB(self.config) diff --git a/tests/interface/slack_test.py b/tests/interface/slack_test.py index 725d2c44..e6201331 100644 --- a/tests/interface/slack_test.py +++ b/tests/interface/slack_test.py @@ -143,3 +143,37 @@ def test_create_same_channel_thrice(self): self.bot.create_channel(name) except SlackAPIError as e: assert e.error == "invalid_name" + + def test_create_private_chat(self): + """Test create_private_chat()""" + users = ["user1", "user2", "user3"] + name = "ChannelName" + self.mock_sc.conversations_open.return_value = \ + {"ok": True, "channel": {"name": name}} + error_name = "InvalidABC" + assert self.bot.create_private_chat(users) == name + try: + self.mock_sc.conversations_open.return_value = \ + {"ok": False, "error": error_name} + self.bot.create_private_chat(users) + except SlackAPIError as e: + assert e.error == error_name + + def test_get_channel_id(self): + """Test get_channel_id()""" + channel_name = "#happy" + id = "nice_id" + resp = \ + { + 'ok': True, + 'channels': + [{'name': 'happy', 'id': id}, {'name': 'sad', 'id': 'bad_id'}] + } + self.mock_sc.conversations_list.return_value = resp + assert self.bot.get_channel_id(channel_name) == id + try: + resp['channels'][0]['name'] = 'angry' + self.mock_sc.conversations_list.return_value = resp + self.bot.get_channel_id(channel_name) + except SlackAPIError as e: + assert e.error == "No channels found with the name happy" diff --git a/tests/memorydb.py b/tests/memorydb.py index 99333a44..e67b464e 100644 --- a/tests/memorydb.py +++ b/tests/memorydb.py @@ -1,8 +1,8 @@ from db.facade import DBFacade -from app.model import User, Team, Project, Permissions +from app.model import User, Team, Project, Permissions, Pairing from typing import TypeVar, List, Type, Tuple, cast, Set -T = TypeVar('T', User, Team, Project) +T = TypeVar('T', User, Team, Project, Pairing) # Convert DB field names to python attribute names @@ -91,6 +91,8 @@ def get_key(m: T) -> str: return cast(Team, m).github_team_id elif isinstance(m, Project): return cast(Project, m).project_id + elif isinstance(m, Pairing): + return cast(Pairing, m).pairing_id class MemoryDB(DBFacade): @@ -173,3 +175,6 @@ def delete(self, Model: Type[T], k: str): d = self.get_db(Model) if k in d: d.pop(k) + + def delete_all(self, Model: Type[T]): + pass From 8b478280fa75df83c364b579020c0603c5a3553b Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Sat, 31 Oct 2020 22:45:59 -0700 Subject: [PATCH 06/12] Change launchpad to launch pad --- app/scheduler/modules/pairing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scheduler/modules/pairing.py b/app/scheduler/modules/pairing.py index 720f5337..365c4641 100644 --- a/app/scheduler/modules/pairing.py +++ b/app/scheduler/modules/pairing.py @@ -1,4 +1,4 @@ -"""Match two Launchpad member for a private conversation""" +"""Match two Launch Pad member for a private conversation""" from slack import WebClient from interface.slack import Bot from random import shuffle @@ -12,7 +12,7 @@ class PairingSchedule(ModuleBase): - """Module that matches 2 launchpad members each week""" + """Module that matches 2 Launch Pad members each week""" NAME = 'Match launch pad members randomly' From 09d3bdbd5f01443bff74a0198da2d771eec1acad Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Sat, 31 Oct 2020 23:54:53 -0700 Subject: [PATCH 07/12] Adds tests for the scheduler --- interface/slack.py | 2 +- tests/app/scheduler/modules/pairing_test.py | 82 +++++++++++++++++++++ tests/memorydb.py | 18 ++++- 3 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 tests/app/scheduler/modules/pairing_test.py diff --git a/interface/slack.py b/interface/slack.py index 5888ddb8..9b3c172f 100644 --- a/interface/slack.py +++ b/interface/slack.py @@ -123,7 +123,7 @@ def create_private_chat(self, users: List[str]): """ logging.debug( f"Attempting to open a private conversation with users {users}") - response = self.sc.conversations_open(users=users) + response = cast(SlackResponse, self.sc.conversations_open(users=users)) if response['ok']: logging.debug( f"Successfly opened a converation with the name \ diff --git a/tests/app/scheduler/modules/pairing_test.py b/tests/app/scheduler/modules/pairing_test.py new file mode 100644 index 00000000..e0248554 --- /dev/null +++ b/tests/app/scheduler/modules/pairing_test.py @@ -0,0 +1,82 @@ +"""Test how pairing users should work""" +from unittest import mock, TestCase +from app.scheduler.modules.pairing import PairingSchedule +from tests.memorydb import MemoryDB +from app.model import Pairing +from interface.slack import Bot + + +class TestPairingSchedule(TestCase): + """Test cases for execution of random channel selector.""" + + @mock.patch.object(Bot, 'get_channel_id') + def setUp(self, channel_id_mock): + """Set up necessary spec'd components for testing later.""" + self.config = mock.Mock() + self.config.slack_api_token = '' + self.config.slack_notification_channel = '' + self.config.slack_announcement_channel = '' + self.config.slack_pairing_channel = '' + self.app = mock.Mock() + self.slackbot = mock.Mock() + self.db = MemoryDB() + channel_id_mock.return_value = "id" + + self.pairing = PairingSchedule(self.app, self.config, self.db) + self.pairing.bot = self.slackbot + + def test_pairing_simple(self): + """Test pairing two people together""" + self.db.delete_all(Pairing) + self.slackbot.get_channel_users.return_value = [ + 'user1', 'user2' + ] + + self.pairing.do_it() + self.slackbot.create_private_chat.assert_called_with( + ['user1', 'user2'] + ) + + def test_pairing_many(self): + """Test pairing many people together""" + users = [] + self.db.delete_all(Pairing) + for i in range(100): + users.append("user" + str(i)) + self.slackbot.get_channel_users.return_value = users + self.pairing.do_it() + assert self.slackbot.create_private_chat.call_count == 50 + seen = set() + for arg in self.slackbot.create_private_chat.call_args_list: + user_args = arg.args[0] + for user in user_args: + if user in seen: + self.fail("User was matched twice!") + return + seen.add(user) + + def test_pairing_purged(self): + """Test purging the DB once it is impossible to pair users""" + self.db.delete_all(Pairing) + users = ['user1', 'user2'] + self.slackbot.get_channel_users.return_value = users + self.pairing.do_it() + self.slackbot.create_private_chat.assert_called_with( + ['user1', 'user2'] + ) + # Now the db has the pairing of user1 and user2 + self.pairing.do_it() + # Since they are the only ones, they get matched again + self.slackbot.create_private_chat.assert_called_with( + ['user1', 'user2'] + ) + + def test_pairing_odd(self): + """Test pairing users, when one group has an odd number of users""" + self.db.delete_all(Pairing) + users = ['user1', 'user2', 'user3'] + self.slackbot.get_channel_users.return_value = users + self.pairing.do_it() + self.slackbot.create_private_chat.assert_called_with( + users + ) diff --git a/tests/memorydb.py b/tests/memorydb.py index 017554d2..b8b16bf7 100644 --- a/tests/memorydb.py +++ b/tests/memorydb.py @@ -44,6 +44,12 @@ 'playstore_url': 'playstore_url' } +PAIRING_ATTRS = { + 'pairing_id': 'pairing_id', + 'user1_slack_id': 'user1_slack_id', + 'user2_slack_id': 'user2_slack_id' +} + def field_is_set(Model: Type[T], field: str) -> bool: if Model is Team: @@ -61,6 +67,8 @@ def field_to_attr(Model: Type[T], field: str) -> str: return TEAM_ATTRS[field] elif Model is Project: return PROJ_ATTRS[field] + elif Model is Pairing: + return PAIRING_ATTRS[field] return field @@ -110,17 +118,20 @@ class MemoryDB(DBFacade): def __init__(self, users: List[User] = [], teams: List[Team] = [], - projs: List[Project] = []): + projs: List[Project] = [], + pairings: List[Pairing] = []): """ Initialize with lists of objects. :param users: list of users to initialize the db :param teams: list of teams to initialize the db :param projs: list of projects to initialize the db + :param pairings: list of pairings to initialize the db """ self.users = {u.slack_id: u for u in users} self.teams = {t.github_team_id: t for t in teams} self.projs = {p.project_id: p for p in projs} + self.pairings = {p.pairing_id: p for p in pairings} def get_db(self, Model: Type[T]): if Model is User: @@ -129,6 +140,8 @@ def get_db(self, Model: Type[T]): return self.teams elif Model is Project: return self.projs + elif Model is Pairing: + return self.pairings return {} def store(self, obj: T) -> bool: @@ -184,4 +197,5 @@ def delete(self, Model: Type[T], k: str): d.pop(k) def delete_all(self, Model: Type[T]): - pass + d = self.get_db(Model) + d.clear() From 0d35701dbe6dd5f8b6e4d4df910b971f018b22d8 Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Sun, 1 Nov 2020 00:05:26 -0700 Subject: [PATCH 08/12] Adds tests for model --- tests/app/model/pairing_test.py | 23 ++++++++++++++++ tests/app/scheduler/modules/pairing_test.py | 29 ++++++++++++--------- 2 files changed, 40 insertions(+), 12 deletions(-) create mode 100644 tests/app/model/pairing_test.py diff --git a/tests/app/model/pairing_test.py b/tests/app/model/pairing_test.py new file mode 100644 index 00000000..40a92661 --- /dev/null +++ b/tests/app/model/pairing_test.py @@ -0,0 +1,23 @@ +"""Test the data model for a project.""" +from app.model import Pairing +from unittest import TestCase + + +class TestPairingModel(TestCase): + """Test pairing model functions.""" + + def setUp(self): + """Set up pairing variables for testing.""" + self.p0 = Pairing('user1', 'user2') + self.p1 = Pairing('user1', 'user3') + self.p2 = Pairing('user2', 'user3') + + def test_valid_pairing(self): + """Test the pairing static method is_valid().""" + self.assertTrue(Pairing.is_valid(self.p0)) + + def test_pairing_equalities(self): + """Test pairing __eq__ and __ne__ methods.""" + self.assertNotEqual(self.p0, self.p1) + self.assertNotEqual(self.p0, self.p2) + self.assertNotEqual(self.p1, self.p2) diff --git a/tests/app/scheduler/modules/pairing_test.py b/tests/app/scheduler/modules/pairing_test.py index e0248554..f3480cd2 100644 --- a/tests/app/scheduler/modules/pairing_test.py +++ b/tests/app/scheduler/modules/pairing_test.py @@ -33,9 +33,10 @@ def test_pairing_simple(self): ] self.pairing.do_it() - self.slackbot.create_private_chat.assert_called_with( - ['user1', 'user2'] - ) + args = self.slackbot.create_private_chat.call_args.args[0] + assert 'user1' in args + assert 'user2' in args + assert len(args) == 2 def test_pairing_many(self): """Test pairing many people together""" @@ -61,15 +62,17 @@ def test_pairing_purged(self): users = ['user1', 'user2'] self.slackbot.get_channel_users.return_value = users self.pairing.do_it() - self.slackbot.create_private_chat.assert_called_with( - ['user1', 'user2'] - ) + args = self.slackbot.create_private_chat.call_args.args[0] + assert 'user1' in args + assert 'user2' in args + assert len(args) == 2 # Now the db has the pairing of user1 and user2 self.pairing.do_it() # Since they are the only ones, they get matched again - self.slackbot.create_private_chat.assert_called_with( - ['user1', 'user2'] - ) + args = self.slackbot.create_private_chat.call_args.args[0] + assert 'user1' in args + assert 'user2' in args + assert len(args) == 2 def test_pairing_odd(self): """Test pairing users, when one group has an odd number of users""" @@ -77,6 +80,8 @@ def test_pairing_odd(self): users = ['user1', 'user2', 'user3'] self.slackbot.get_channel_users.return_value = users self.pairing.do_it() - self.slackbot.create_private_chat.assert_called_with( - users - ) + args = self.slackbot.create_private_chat.call_args.args[0] + assert 'user1' in args + assert 'user2' in args + assert 'user3' in args + assert len(args) == 3 From 128ae061dbd6d4520f303548866cd18a13f99995 Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Sun, 1 Nov 2020 12:18:14 -0800 Subject: [PATCH 09/12] Apply suggestions from code review Co-authored-by: Robert Lin --- app/model/pairing.py | 2 +- app/scheduler/modules/pairing.py | 4 ++-- db/dynamodb.py | 2 +- interface/slack.py | 3 ++- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/model/pairing.py b/app/model/pairing.py index ff3cf3c9..2f8c22c1 100644 --- a/app/model/pairing.py +++ b/app/model/pairing.py @@ -15,7 +15,7 @@ def __init__(self, """ Initialize the pairing. - Pairing ID is a UUID generated by ``uuid.uuid4()``. + The generated ``pairing_id`` property is meant to uniquely represent a pairing. :param user1_slack_id: The slack ID of the first user :param user2_slack_id: The slack ID of the second user diff --git a/app/scheduler/modules/pairing.py b/app/scheduler/modules/pairing.py index 365c4641..dc6613d5 100644 --- a/app/scheduler/modules/pairing.py +++ b/app/scheduler/modules/pairing.py @@ -12,9 +12,9 @@ class PairingSchedule(ModuleBase): - """Module that matches 2 Launch Pad members each week""" + """Module that pairs members each ``SLACK_PAIRING_FREQUENCY``""" - NAME = 'Match launch pad members randomly' + NAME = 'Pair members randomly for meetups' def __init__(self, flask_app: Flask, diff --git a/db/dynamodb.py b/db/dynamodb.py index f4f347fe..e1e3c543 100644 --- a/db/dynamodb.py +++ b/db/dynamodb.py @@ -201,7 +201,7 @@ def __enable_ttl(self, table_name: str): :param table_name: name of the table to enable ttl for """ - # Stub + # TODO def check_valid_table(self, table_name: str) -> bool: """ diff --git a/interface/slack.py b/interface/slack.py index 9b3c172f..e43bf70f 100644 --- a/interface/slack.py +++ b/interface/slack.py @@ -142,7 +142,8 @@ def get_channel_id(self, channel_name: str): :return the slack id of the channel """ - # We strip away the "#" in case it was provided with the channel name + # We strip away the "#" in case it was provided with the channel name, + # such as in values returned from Slack's ``conversations_list`` API. channel_name = channel_name.replace("#", "") logging.debug(f"Attempting to get the id of channel {channel_name}") channels = list( From 4d39fe4b427e6ba3bd752a70415af8fa647a4e28 Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Sun, 1 Nov 2020 12:33:30 -0800 Subject: [PATCH 10/12] Feature toggle the pairings --- app/model/pairing.py | 3 ++- app/scheduler/__init__.py | 5 ++++- config/__init__.py | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/model/pairing.py b/app/model/pairing.py index 2f8c22c1..de2520bb 100644 --- a/app/model/pairing.py +++ b/app/model/pairing.py @@ -15,7 +15,8 @@ def __init__(self, """ Initialize the pairing. - The generated ``pairing_id`` property is meant to uniquely represent a pairing. + The generated ``pairing_id`` property is meant to uniquely + represent a pairing. :param user1_slack_id: The slack ID of the first user :param user2_slack_id: The slack ID of the second user diff --git a/app/scheduler/__init__.py b/app/scheduler/__init__.py index 4d4d0ba8..11172531 100644 --- a/app/scheduler/__init__.py +++ b/app/scheduler/__init__.py @@ -39,4 +39,7 @@ def __add_job(self, module: ModuleBase): def __init_periodic_tasks(self): """Add jobs that fire every interval.""" self.__add_job(RandomChannelPromoter(*self.args)) - self.__add_job(PairingSchedule(*self.args, self.facade)) + # Feature toggle the pairings based on the + # existence of the `SLACK_PAIRING_CHANNEL` env variable + if len(self.args[1].slack_pairing_channel) != 0: + self.__add_job(PairingSchedule(*self.args, self.facade)) diff --git a/config/__init__.py b/config/__init__.py index 32dacd0f..bc817de7 100644 --- a/config/__init__.py +++ b/config/__init__.py @@ -46,6 +46,8 @@ class Config: 'GITHUB_LEADS_TEAM_NAME': '', 'GCP_SERVICE_ACCOUNT_CREDENTIALS': '', 'GCP_SERVICE_ACCOUNT_SUBJECT': '', + 'SLACK_PAIRING_CHANNEL': '', + 'AWS_PAIRINGS_TABLE': '', } def __init__(self): From 0b9f76a1821ddc4b1bf628a40c5fb5fcf4c589ae Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Sun, 1 Nov 2020 12:54:38 -0800 Subject: [PATCH 11/12] Makes frequency an environment variable --- app/scheduler/modules/pairing.py | 11 +++++++++-- config/__init__.py | 5 ++++- docker-compose.yml | 1 + tests/app/scheduler/base_test.py | 1 + tests/app/scheduler/modules/pairing_test.py | 1 + tests/config/config_test.py | 2 +- 6 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/scheduler/modules/pairing.py b/app/scheduler/modules/pairing.py index dc6613d5..8c9b891b 100644 --- a/app/scheduler/modules/pairing.py +++ b/app/scheduler/modules/pairing.py @@ -24,13 +24,20 @@ def __init__(self, self.bot = Bot(WebClient(config.slack_api_token), config.slack_notification_channel) self.channel_id = self.bot.get_channel_id(config.slack_pairing_channel) + self.config = config self.facade = facade def get_job_args(self) -> Dict[str, Any]: """Get job configuration arguments for apscheduler.""" + logging.info(f"Running pairing at cron job: {self.config.slack_pairing_frequency}") + cron_frequency = self.config.slack_pairing_frequency.split(' ') return {'trigger': 'cron', - 'minute': '*', - 'name': self.NAME} + 'minute': cron_frequency[0], + 'hour': cron_frequency[1], + 'day': cron_frequency[2], + 'month': cron_frequency[3], + 'day_of_week': cron_frequency[4], + 'name': self.NAME} def do_it(self): """Pair users together, and create a private chat for them""" diff --git a/config/__init__.py b/config/__init__.py index bc817de7..21210f87 100644 --- a/config/__init__.py +++ b/config/__init__.py @@ -18,6 +18,7 @@ class Config: 'SLACK_NOTIFICATION_CHANNEL': 'slack_notification_channel', 'SLACK_ANNOUNCEMENT_CHANNEL': 'slack_announcement_channel', 'SLACK_PAIRING_CHANNEL': 'slack_pairing_channel', + 'SLACK_PAIRING_FREQUENCY': 'slack_pairing_frequency', 'GITHUB_APP_ID': 'github_app_id', 'GITHUB_ORG_NAME': 'github_org_name', 'GITHUB_DEFAULT_TEAM_NAME': 'github_team_all', @@ -47,7 +48,8 @@ class Config: 'GCP_SERVICE_ACCOUNT_CREDENTIALS': '', 'GCP_SERVICE_ACCOUNT_SUBJECT': '', 'SLACK_PAIRING_CHANNEL': '', - 'AWS_PAIRINGS_TABLE': '', + 'AWS_PAIRINGS_TABLE': 'pairings', + 'SLACK_PAIRING_FREQUENCY': '', } def __init__(self): @@ -93,6 +95,7 @@ def _set_attrs(self): self.slack_notification_channel = '' self.slack_announcement_channel = '' self.slack_pairing_channel = '' + self.slack_pairing_frequency = '' self.github_app_id = '' self.github_org_name = '' self.github_team_all = '' diff --git a/docker-compose.yml b/docker-compose.yml index 0ee4b3bf..cd4ad389 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -23,6 +23,7 @@ services: - SLACK_NOTIFICATION_CHANNEL=${SLACK_NOTIFICATION_CHANNEL} - SLACK_ANNOUNCEMENT_CHANNEL=${SLACK_ANNOUNCEMENT_CHANNEL} - SLACK_PAIRING_CHANNEL=${SLACK_PAIRING_CHANNEL} + - SLACK_PAIRING_FREQUENCY=${SLACK_PAIRING_FREQUENCY} - SLACK_SIGNING_SECRET=${SLACK_SIGNING_SECRET} - SLACK_API_TOKEN=${SLACK_API_TOKEN} - GITHUB_APP_ID=${GITHUB_APP_ID} diff --git a/tests/app/scheduler/base_test.py b/tests/app/scheduler/base_test.py index 25c96a4c..dd357689 100644 --- a/tests/app/scheduler/base_test.py +++ b/tests/app/scheduler/base_test.py @@ -23,6 +23,7 @@ def setUp(self): self.config.slack_notification_channel = "#general" self.config.slack_api_token = "sometoken.exe" self.config.slack_pairing_channel = "#general" + self.config.slack_pairing_frequency = "* * * * *" @patch.object(Bot, 'get_channel_id') def test_proper_initialization(self, other): diff --git a/tests/app/scheduler/modules/pairing_test.py b/tests/app/scheduler/modules/pairing_test.py index f3480cd2..87a520bd 100644 --- a/tests/app/scheduler/modules/pairing_test.py +++ b/tests/app/scheduler/modules/pairing_test.py @@ -17,6 +17,7 @@ def setUp(self, channel_id_mock): self.config.slack_notification_channel = '' self.config.slack_announcement_channel = '' self.config.slack_pairing_channel = '' + self.config.slack_pairing_frequency = '* * * * *' self.app = mock.Mock() self.slackbot = mock.Mock() self.db = MemoryDB() diff --git a/tests/config/config_test.py b/tests/config/config_test.py index a8086d4a..f4d392fd 100644 --- a/tests/config/config_test.py +++ b/tests/config/config_test.py @@ -15,7 +15,7 @@ def setUp(self): 'SLACK_NOTIFICATION_CHANNEL': '#rocket2', 'SLACK_ANNOUNCEMENT_CHANNEL': '#ot-random', 'SLACK_PAIRING_CHANNEL': "#pairing", - + 'SLACK_PAIRING_FREQUENCY': '* * * * *', 'GITHUB_APP_ID': '2024', 'GITHUB_ORG_NAME': 'ubclaunchpad', 'GITHUB_WEBHOOK_ENDPT': '/webhook', From d2e2b87ac39b2bce062b596d041fa5b3e7d428cb Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Sun, 1 Nov 2020 12:59:39 -0800 Subject: [PATCH 12/12] Fix linting line too long --- app/scheduler/modules/pairing.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/scheduler/modules/pairing.py b/app/scheduler/modules/pairing.py index 8c9b891b..8a27c3fd 100644 --- a/app/scheduler/modules/pairing.py +++ b/app/scheduler/modules/pairing.py @@ -29,7 +29,8 @@ def __init__(self, def get_job_args(self) -> Dict[str, Any]: """Get job configuration arguments for apscheduler.""" - logging.info(f"Running pairing at cron job: {self.config.slack_pairing_frequency}") + logging.info(f"Running pairing at cron job: \ + {self.config.slack_pairing_frequency}") cron_frequency = self.config.slack_pairing_frequency.split(' ') return {'trigger': 'cron', 'minute': cron_frequency[0],