Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
all: test

run-dev: config.py lib
dev_appserver.py dispatch.yaml app.yaml worker.yaml
dev_appserver.py --enable_console true dispatch.yaml app.yaml worker.yaml

deploy: deploy_build
# If you are running into permission issues and see a message like this:
Expand Down
2 changes: 2 additions & 0 deletions config-example.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@
GRAVATAR = 'backup'

ORG_TITLE = 'All Company'
TEAMS_TITLE = 'All Teams'
OFFICES_TITLE = 'All Offices'
6 changes: 3 additions & 3 deletions import/employees.csv.example
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
username,first_name,last_name,department,photo_url
john,John,Doe,,https://placehold.it/100x100
janet,Janet,Doe,,https://placehold.it/100x100
username,first_name,last_name,department,office,photo_url
john,John,Doe,,San Francisco,https://placehold.it/100x100
janet,Janet,Doe,,San Francisco,https://placehold.it/100x100
2 changes: 2 additions & 0 deletions import/employees.json.example
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
"first_name": "John",
"last_name": "Doe",
"department": "",
"office": "San Francisco",
"photo_url": "https://placehold.it/100x100"
},
{
"username": "janet",
"first_name": "Janet",
"last_name": "Doe",
"department": "",
"office": "San Francisco",
"photo_url": "https://placehold.it/100x100"
}
]
26 changes: 14 additions & 12 deletions logic/love_count.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from models.toggle import LOVE_SENDING_ENABLED


def top_lovers_and_lovees(utc_week_start, dept=None, limit=20):
def top_lovers_and_lovees(utc_week_start, dept=None, office=None, limit=20):
"""Synchronously return a list of (employee key, sent love count) and a list of
(employee key, received love count), each sorted in descending order of love sent
or received.
Expand All @@ -22,11 +22,12 @@ def top_lovers_and_lovees(utc_week_start, dept=None, limit=20):
if c.sent_count == 0:
continue
employee_key = c.key.parent()
if dept:
employee = employee_key.get()
if employee.meta_department == dept or employee.department == dept:
lovers.append((employee_key, c.sent_count))
else:
employee = employee_key.get()
if (
Copy link
Member

Choose a reason for hiding this comment

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

Not blocking but I think this could need a comment saying what condition we are looking for to append an employee to the list.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe even better to extract this into a function with a little docstring to keep things DRY. Thoughts?

not dept or (employee.meta_department == dept or employee.department == dept)
) and (
not office or employee.office.startswith(office)
):
lovers.append((employee_key, c.sent_count))

received = sorted(sent, key=lambda c: c.received_count, reverse=True)
Expand All @@ -37,14 +38,15 @@ def top_lovers_and_lovees(utc_week_start, dept=None, limit=20):
if c.received_count == 0:
continue
employee_key = c.key.parent()
if dept:
employee = employee_key.get()
if employee.meta_department == dept or employee.department == dept:
lovees.append((employee_key, c.received_count))
else:
employee = employee_key.get()
if (
Copy link
Member

Choose a reason for hiding this comment

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

Not blocking but I think this could need a comment saying what condition we are looking for to append en employee to the list.

not dept or (employee.meta_department == dept or employee.department == dept)
) and (
not office or employee.office.startswith(office)
):
lovees.append((employee_key, c.received_count))

return (lovers, lovees)
return lovers, lovees


def rebuild_love_count():
Expand Down
6 changes: 6 additions & 0 deletions logic/office.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# -*- coding: utf-8 -*-
import yaml


_offices = yaml.safe_load(open('offices.yaml'))
OFFICES = set(_offices['Offices'])
2 changes: 2 additions & 0 deletions models/employee.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class Employee(ndb.Model, Pagination):
timestamp = ndb.DateTimeProperty(auto_now_add=True)
user = ndb.UserProperty()
username = ndb.StringProperty()
office = ndb.StringProperty(indexed=False)

@classmethod
def get_current_employee(cls):
Expand Down Expand Up @@ -79,6 +80,7 @@ def update_from_dict(self, d):
self.photo_url = d.get('photo_url')
self.department = d.get('department')
self.meta_department = get_meta_department(self.department)
self.office = d.get('office')

def get_gravatar(self):
"""Creates gravatar URL from email address."""
Expand Down
3 changes: 3 additions & 0 deletions offices.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Offices:
- Office Location 1
- Office Location 2
2 changes: 2 additions & 0 deletions testing/factories/employee.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ def create_employee(
department='Engineering',
first_name='John',
last_name='Doe',
office='San Francisco',
photo_url=None,
):

Expand All @@ -18,5 +19,6 @@ def create_employee(
'department': department,
'first_name': first_name,
'last_name': last_name,
'office': office,
'photo_url': photo_url,
})
1 change: 1 addition & 0 deletions tests/models/employee_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ def test_create_from_dict(self, mock_config):
first_name='John',
last_name='Doe',
department='Accounting',
office='San Francisco',
photos=[]
)
employee = Employee.create_from_dict(employee_dict)
Expand Down
2 changes: 2 additions & 0 deletions tests/views/web_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,10 @@ def test_leaderboard(self):
self.assertIsNotNone(response.context['top_lovers'])
self.assertIsNotNone(response.context['departments'])
self.assertIsNotNone(response.context['sub_departments'])
self.assertIsNotNone(response.context['offices'])
self.assertIsNone(response.context['selected_dept'])
self.assertIsNotNone(response.context['selected_timespan'])
self.assertIsNone(response.context['selected_office'])


class ExploreTest(LoggedInUserBaseTest):
Expand Down
11 changes: 10 additions & 1 deletion themes/default/templates/leaderboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ <h1>Lovers of the Week</h1>
<div class="form-group">
<label class="sr-only" for="department">Department</label>
<select name="department" class="form-control" id="department" style="font-size: 18px;">
<option value="" {% if not selected_dept %}selected{% endif%}>{{ org_title }}</option>
<option value="" {% if not selected_dept %}selected{% endif%}>{{ teams_title }}</option>
{% for meta_dept in departments|sort %}
<option {% if selected_dept == meta_dept %}selected{% endif %} value="{{ meta_dept }}">{{ meta_dept }}</option>
{% if meta_dept in sub_departments %}
Expand All @@ -32,6 +32,15 @@ <h1>Lovers of the Week</h1>
{% endfor %}
</select>
</div>
<div class="form-group">
<label class="sr-only" for="office">Department</label>
Copy link
Member

Choose a reason for hiding this comment

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

I assume this should be Office not Department?

<select name="office" class="form-control" id="office" style="font-size: 18px;">
<option value="" {% if not selected_office %}selected{% endif%}>{{ offices_title }}</option>
{% for office in offices|sort %}
<option {% if selected_office == office %}selected{% endif %} value="{{ office }}">{{ office }}</option>
{% endfor %}
</select>
</div>
</form>
</div>
</div>
Expand Down
13 changes: 11 additions & 2 deletions views/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import logic.love_link
import logic.love_count
import logic.subscription
import logic.office
from errors import NoSuchEmployee
from errors import NoSuchLoveLink
from errors import TaintedLove
Expand Down Expand Up @@ -159,6 +160,7 @@ def explore():
def leaderboard():
timespan = request.args.get('timespan', TIMESPAN_THIS_WEEK)
department = request.args.get('department', None)
office = request.args.get('office', None)

# If last week, we need to subtract *before* getting the week limits to
# avoid being off by one hour on weeks that include a DST transition
Expand All @@ -167,7 +169,11 @@ def leaderboard():
utc_now -= timedelta(days=7)
utc_week_start, _ = utc_week_limits(utc_now)

top_lovers, top_lovees = logic.love_count.top_lovers_and_lovees(utc_week_start, dept=department)
top_lovers, top_lovees = logic.love_count.top_lovers_and_lovees(
utc_week_start,
dept=department,
office=office
)

top_lover_dicts = [
{
Expand Down Expand Up @@ -197,9 +203,12 @@ def leaderboard():
top_lovers=top_lover_dicts,
departments=logic.department.META_DEPARTMENTS,
sub_departments=logic.department.META_DEPARTMENT_MAP,
offices=logic.office.OFFICES,
selected_dept=department,
selected_timespan=timespan,
selected_timespan=timespan, selected_office=office,
org_title=config.ORG_TITLE,
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if org_title is still being used? If not it should be removed here and in the config.

teams_title=config.TEAMS_TITLE,
offices_title=config.OFFICES_TITLE
)


Expand Down