Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add route to get a member's data for helper review #451

Merged
merged 12 commits into from
Mar 16, 2021
18 changes: 17 additions & 1 deletion postgres/init.sql
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,28 @@ INSERT INTO users VALUES (
current_timestamp
);

CREATE TABLE channels (
id varchar,
name varchar,
primary key(id)
);

INSERT INTO channels VALUES(
'267659945086812160',
'python-general'
);

INSERT INTO channels VALUES(
'1234',
'zebra'
);

CREATE TABLE messages (
id varchar,
author_id varchar references users(id),
is_deleted boolean,
created_at timestamp,
channel_id varchar,
channel_id varchar references channels(id),
primary key(id)
);

Expand Down
40 changes: 40 additions & 0 deletions pydis_site/apps/api/models/bot/metricity.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import List, Tuple

from django.db import connections

BLOCK_INTERVAL = 10 * 60 # 10 minute blocks
Expand Down Expand Up @@ -89,3 +91,41 @@ def total_message_blocks(self, user_id: str) -> int:
raise NotFound()

return values[0]

def top_channel_activity(self, user_id: str) -> List[Tuple[str, int]]:
"""
Query the top three channels in which the user is most active.

Help channels are grouped under "the help channels",
and off-topic channels are grouped under "off-topic".
"""
self.cursor.execute(
"""
SELECT
CASE
WHEN channels.name ILIKE 'help-%%' THEN 'the help channels'
WHEN channels.name ILIKE 'ot%%' THEN 'off-topic'
Comment on lines +106 to +107
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar here is irking me a bit.

Suggested change
WHEN channels.name ILIKE 'help-%%' THEN 'the help channels'
WHEN channels.name ILIKE 'ot%%' THEN 'off-topic'
WHEN channels.name ILIKE 'help-%%' THEN 'Help channels'
WHEN channels.name ILIKE 'ot%%' THEN 'Off topic'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of those names is to be used in a sentence, rather than titles, although I agree it's not the prettiest

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, gotcha, I thought it would be more a sort of list like ```
Help channels: 24,404 messages
Off topic: 12,000 messages
...: ... messages

ELSE channels.name
END,
COUNT(1)
FROM
messages
LEFT JOIN channels ON channels.id = messages.channel_id
WHERE
author_id = '%s' AND NOT messages.is_deleted
GROUP BY
1
ORDER BY
2 DESC
LIMIT
3;
""",
[user_id]
)

values = self.cursor.fetchall()

if not values:
raise NotFound()

return values
35 changes: 30 additions & 5 deletions pydis_site/apps/api/tests/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ def test_get_metricity_data(self):
joined_at = "foo"
total_messages = 1
total_blocks = 1
self.mock_metricity_user(joined_at, total_messages, total_blocks)
self.mock_metricity_user(joined_at, total_messages, total_blocks, [])

# When
url = reverse('bot:user-metricity-data', args=[0], host='api')
Expand All @@ -431,18 +431,21 @@ def test_no_metricity_user(self):

# When
url = reverse('bot:user-metricity-data', args=[0], host='api')
response = self.client.get(url)
response1 = self.client.get(url)
url = reverse('bot:user-metricity-review-data', args=[0], host='api')
response2 = self.client.get(url)

# Then
self.assertEqual(response.status_code, 404)
self.assertEqual(response1.status_code, 404)
self.assertEqual(response2.status_code, 404)
mbaruh marked this conversation as resolved.
Show resolved Hide resolved

def test_metricity_voice_banned(self):
cases = [
{'exception': None, 'voice_banned': True},
{'exception': ObjectDoesNotExist, 'voice_banned': False},
]

self.mock_metricity_user("foo", 1, 1)
self.mock_metricity_user("foo", 1, 1, [["bar", 1]])

for case in cases:
with self.subTest(exception=case['exception'], voice_banned=case['voice_banned']):
Expand All @@ -455,14 +458,35 @@ def test_metricity_voice_banned(self):
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json()["voice_banned"], case["voice_banned"])

def mock_metricity_user(self, joined_at, total_messages, total_blocks):
def test_metricity_review_data(self):
# Given
joined_at = "foo"
total_messages = 10
total_blocks = 1
channel_activity = [["bar", 4], ["buzz", 6]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be subtests to test each case in the query (i.e. you need to test off topic and help channels too).

self.mock_metricity_user(joined_at, total_messages, total_blocks, channel_activity)

# When
url = reverse('bot:user-metricity-review-data', args=[0], host='api')
response = self.client.get(url)

# Then
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json(), {
"joined_at": joined_at,
"top_channel_activity": channel_activity,
"total_messages": total_messages
})

def mock_metricity_user(self, joined_at, total_messages, total_blocks, top_channel_activity):
patcher = patch("pydis_site.apps.api.viewsets.bot.user.Metricity")
self.metricity = patcher.start()
self.addCleanup(patcher.stop)
self.metricity = self.metricity.return_value.__enter__.return_value
self.metricity.user.return_value = dict(joined_at=joined_at)
self.metricity.total_messages.return_value = total_messages
self.metricity.total_message_blocks.return_value = total_blocks
self.metricity.top_channel_activity.return_value = top_channel_activity

def mock_no_metricity_user(self):
patcher = patch("pydis_site.apps.api.viewsets.bot.user.Metricity")
Expand All @@ -472,3 +496,4 @@ def mock_no_metricity_user(self):
self.metricity.user.side_effect = NotFound()
self.metricity.total_messages.side_effect = NotFound()
self.metricity.total_message_blocks.side_effect = NotFound()
self.metricity.top_channel_activity.side_effect = NotFound()
15 changes: 15 additions & 0 deletions pydis_site/apps/api/viewsets/bot/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,3 +262,18 @@ def metricity_data(self, request: Request, pk: str = None) -> Response:
except NotFound:
return Response(dict(detail="User not found in metricity"),
status=status.HTTP_404_NOT_FOUND)

@action(detail=True)
def metricity_review_data(self, request: Request, pk: str = None) -> Response:
mbaruh marked this conversation as resolved.
Show resolved Hide resolved
"""Request handler for metricity_review_data endpoint."""
user = self.get_object()

with Metricity() as metricity:
try:
data = metricity.user(user.id)
data["total_messages"] = metricity.total_messages(user.id)
data["top_channel_activity"] = metricity.top_channel_activity(user.id)
return Response(data, status=status.HTTP_200_OK)
except NotFound:
return Response(dict(detail="User not found in metricity"),
status=status.HTTP_404_NOT_FOUND)