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

Sort shakes alphabetically #758

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
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
56 changes: 37 additions & 19 deletions models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -585,19 +585,33 @@ def shakes(self, include_managed=False, include_only_group_shakes=False):
it will not return any 'user' shakes that the user owns.
"""
managed_shakes = []
params = [self.id] # Guaranteed will need to substitute at least user_id value.
user_where_clause = 'user_id = %s '
if include_only_group_shakes:
user_where_clause += "AND type = 'group' AND deleted = 0"
shakes_sql = """
SELECT shake.*
FROM shake
WHERE %s
""" % user_where_clause
if include_managed:
sql = """SELECT shake.* from shake, shake_manager
WHERE shake_manager.user_id = %s
AND shake.id = shake_manager.shake_id
AND shake.deleted = 0
AND shake_manager.deleted = 0
ORDER BY shake_manager.shake_id
shakes_sql += """
UNION
SELECT shake.*
FROM shake, shake_manager
WHERE shake_manager.user_id = %s
AND shake.id = shake_manager.shake_id
AND shake.deleted = 0
AND shake_manager.deleted = 0
"""
managed_shakes = shake.Shake.object_query(sql, self.id)
user_shakes_sql = 'user_id=%s ORDER BY id'
if include_only_group_shakes:
user_shakes_sql = "user_id=%s and type='group' and deleted=0 ORDER BY id"
return shake.Shake.where(user_shakes_sql, self.id) + managed_shakes
# Only include the second user_id param if we include managed shakes clause.
params.append(self.id)

# Order the whole lot by name.
shakes_sql += """
ORDER BY title
"""
return shake.Shake.object_query(shakes_sql, *params)

_has_multiple_shakes = None
def has_multiple_shakes(self):
Expand Down Expand Up @@ -630,17 +644,21 @@ def following(self, page=None):
"""
This needs to be refactored, but it would be so slow if we
were grabbing user objects for each user on 1,000 users.
Sorting by title puts all user shakes first (because title is null)
with the rest of the group shakes following in correct order (title
is whatever), then by name to "tie-break" the user shakes.
"""
select = """
SELECT user.id as user_id, user.name as user_name, user.profile_image as user_image,
shake.name as shake_name, shake.type as shake_type , shake.image as shake_image,
shake.id as shake_id
SELECT user.id as user_id, user.name as user_name, user.profile_image as user_image,
shake.name as shake_name, shake.type as shake_type, shake.image as shake_image,
shake.id as shake_id, shake.title
FROM subscription, user, shake
WHERE subscription.user_id = %s
AND subscription.shake_id = shake.id
AND user.id = shake.user_id
AND shake.deleted = 0
AND subscription.deleted = 0
WHERE subscription.user_id = %s
AND subscription.shake_id = shake.id
AND user.id = shake.user_id
AND shake.deleted = 0
AND subscription.deleted = 0
ORDER BY shake.title, shake.name
""" % self.id

if page > 0:
Expand Down
25 changes: 13 additions & 12 deletions test/functional/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,18 +627,19 @@ def test_query_usershakes_resource(self):
self.assertTrue('created_at' in user_shake)
self.assertTrue('updated_at' in user_shake)

self.assertEqual(group_shake['id'], 3)
self.assertEqual(group_shake['type'], 'group')
self.assertEqual(group_shake['name'], 'Group Shake')
self.assertEqual(group_shake['owner'], {'name': 'user2', 'id': 2, 'profile_image_url': "https://mltshp-cdn.com/static/images/default-icon-venti.svg"})
self.assertEqual(group_shake['thumbnail_url'], 'https://mltshp-cdn.com/static/images/default-icon-venti.svg')
self.assertEqual(group_shake['url'], 'https://mltshp.com/groupshake')
self.assertEqual(group_shake['description'], 'This is a group shake.')
self.assertTrue('created_at' in group_shake)
self.assertTrue('updated_at' in group_shake)

self.assertEqual(group_shake_2['id'], 4)
self.assertEqual(group_shake_2['owner'], {'name': 'admin', 'id': 1, 'profile_image_url': "https://mltshp-cdn.com/static/images/default-icon-venti.svg"})
# Group shakes now sorted alphabetically - https://github.com/MLTSHP/mltshp/issues/460
self.assertEqual(group_shake['id'], 4)
self.assertEqual(group_shake['owner'], {'name': 'admin', 'id': 1, 'profile_image_url': "https://mltshp-cdn.com/static/images/default-icon-venti.svg"})

self.assertEqual(group_shake_2['id'], 3)
self.assertEqual(group_shake_2['type'], 'group')
self.assertEqual(group_shake_2['name'], 'Group Shake')
self.assertEqual(group_shake_2['owner'], {'name': 'user2', 'id': 2, 'profile_image_url': "https://mltshp-cdn.com/static/images/default-icon-venti.svg"})
self.assertEqual(group_shake_2['thumbnail_url'], 'https://mltshp-cdn.com/static/images/default-icon-venti.svg')
self.assertEqual(group_shake_2['url'], 'https://mltshp.com/groupshake')
self.assertEqual(group_shake_2['description'], 'This is a group shake.')
self.assertTrue('created_at' in group_shake_2)
self.assertTrue('updated_at' in group_shake_2)

def test_query_friend_shake(self):
request = signed_request(self.access_token, self.get_url('/api/friends'))
Expand Down