Skip to content

Like/dislike feature #115

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
63 changes: 63 additions & 0 deletions firefly/models/like.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# coding: utf-8

from __future__ import absolute_import
from datetime import datetime

from firefly.ext import db
from .user import User


class Likes(object):
def __init__(self, product_type):
self.product_type = product_type
self._instance = None

def __get__(self, instance, owner):
self._instance = instance
return self

def __len__(self):
return Like.objects(
product_id=self.product_id,
product_type=self.product_type
).count()

def productidgetter(self, func):
return func

@property
def product_id(self):
return str(self.productidgetter(self._instance))
Copy link
Member

Choose a reason for hiding this comment

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

这里等价于 return str(self._instance) 应该不是你预期的结果吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mozillazg 不是呀 这里是要通过 productidgetter 来拿到 product_id

Copy link
Contributor

Choose a reason for hiding this comment

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

@killpanda 其实我也没看懂. 在productidgetter 里面 直接返回了参数 不就是 @mozillazg 说的意思么

其次 productidgetter这个方法名字 不是应该用下划线分词的么 比如 product_id_getter


def add(self, user_id):
user = User.objects(id=user_id).first()
if user:
return Like.objects.create(
product_id=self.product_id,
product_type=self.product_type,
user=user
)

def delete(self, user_id):
user = User.objects(id=user_id).first()
if user:
like = Like.objects.filter(
product_id=self.product_id,
product_type=self.product_type,
user=user
).first()
if like:
like.delete()
Copy link
Member

Choose a reason for hiding this comment

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

直接 .filter(...).delete() 可以省去判断。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mozillazg 好的,谢谢



class Like(db.Document):
id = db.SequenceField(primary_key=True)
created_at = db.DateTimeField(default=datetime.utcnow, required=True)
product_type = db.StringField(required=True)
product_id = db.StringField(required=True,
unique_with=['user', 'product_type'])
user = db.ReferenceField(User)

meta = {
'indexes': ['product_type', 'product_id']
}
16 changes: 16 additions & 0 deletions firefly/models/topic.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from firefly.views.utils import timesince
from firefly.models.consts import CATEGORY_COLORS
from .user import User
from .like import Likes

__all__ = ["Category", "Post", "Video", "Image", "Comment"]

Expand Down Expand Up @@ -58,6 +59,12 @@ class Post(db.Document):
comments = db.ListField(db.ReferenceField('Comment'))
category = db.ReferenceField(Category)

likes = Likes('Post')

@likes.productidgetter
Copy link
Member

Choose a reason for hiding this comment

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

这个好像没什么作用?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里是为了让 Like 取到 product_id @mozillazg

Copy link
Contributor

Choose a reason for hiding this comment

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

我也没看懂 这里用装饰器的优点是什么

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我是觉得这里用装饰器会比较灵活,因为 product_id 的获取方法完全是由宿主实例决定的。当然现在这种情况也可以去掉装饰器,从 Likesself._instance.id 里面拿到。

Copy link
Contributor

Choose a reason for hiding this comment

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

那这样呢:

def product_id(self):
    return self.likes._instance.id

Copy link
Contributor

Choose a reason for hiding this comment

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

设计一个项目 其实 一个大的角度是代码可读性和可维护性. 这段实现一眼看去 真的不好懂..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这样目前可以呀,可是假如未来有个对象需要加 likes 的功能但没有 id 属性怎么办...这样的情况需要考虑么?

Copy link
Member

Choose a reason for hiding this comment

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

问题主要出在 productidgetter 方法中并没有保存注册的方法。
@killpanda 应该是想要类似这样的效果吧:

class Likes():
   def __init__(self):
       self.product_id_func = None

   def productidgetter(self, func):
        self.product_id_func = func
        return func

   @property
   def product_id(self):
        return self.product_id_func(self._instance)

class Post():
   @likes.productidgetter
   def product_id(self):
         return self.id

def product_id(self):
return self.id

def url(self):
return url_for('post.detail', id=self.id)

Expand All @@ -84,14 +91,17 @@ def recent_activity_time(self):


class Video(Post):
likes = Likes('Video')
Copy link
Contributor

Choose a reason for hiding this comment

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

我在想 可不可以

likes = db.ListField(db.ReferenceField('Likes')) 

在基类里面 用引用的方式

但是这个性能是不是就有问题了

Copy link
Contributor

Choose a reason for hiding this comment

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

要不然 把

@property
def likes(self):
   return  Like.objects(
            product_id=self.id,
            product_type=self.__class__.__name__
     )

放在一个基类里面 这些Topic/Post等等都继承.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

如果是这个用法,那 view 里面怎么添加或者去掉某个 like 呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@property
def likes(self):
   return  Like.objects(
            product_id=self.id,
            product_type=self.__class__.__name__
     )

# View: add Like

Like.create(product_id=post.id, product_type=post.__class__.__name__, user=current_user)

那么 view 层用起来会比较麻烦吧?

@dongweiming

Copy link
Contributor

Choose a reason for hiding this comment

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

还是借用Like

比如

like = Like(product_id=1, product_type ='Post',
                    user=User.objects.get_or_404(id=author_id))
like.save()

但是觉得product_type应该有个关联 或者常量的列表

product_id 其实就是对应的xx.id. 我觉得可以使用int 而不是str

Copy link
Contributor

Choose a reason for hiding this comment

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

@killpanda 但是你使用封装的 Likes创建删除的时候也得这样用吧 只不过加了些接口. 可是参数必选的一个都省不了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

class Likes(object):
    def __init__(self):
        self.product_id = None
        self.product_type = None

    def __get__(self, instance, owner):
        self.product_id = str(instance.id)
        self.product_type = owner.__name__
        return self

    def __len__(self):
        return Like.objects(
            product_id=self.product_id,
            product_type=self.product_type
        ).count()

    def add(self, user_id):
        user = User.objects(id=user_id).first()
        if user:
            return Like.objects.create(
                product_id=self.product_id,
                product_type=self.product_type,
                user=user
            )

    def delete(self, user_id):
        user = User.objects(id=user_id).first()
        if user:
            like = Like.objects.filter(
                product_id=self.product_id,
                product_type=self.product_type,
                user=user
            ).first()
            if like:
                like.delete()

如果是这样呢?我当初的构想是如果 Like 封装的足够好,能尽量做成即插即用,就能让每个需要喜欢功能的地方调用起来简便一些。

embed_code = db.StringField(required=True)


class Image(Post):
likes = Likes('Image')
image_url = db.StringField(required=True, max_length=255)


class Quote(Post):
likes = Likes('Quote')
content = db.StringField(required=True)
author = db.ReferenceField(User)

Expand All @@ -103,6 +113,12 @@ class Comment(db.Document):
author = db.ReferenceField(User)
ref_id = db.IntField(default=0)

likes = Likes('Comment')

@likes.productidgetter
def product_id(self):
return str(self.id)

@property
def post_type(self):
return self.__class__.__name__
Expand Down
2 changes: 2 additions & 0 deletions firefly/views/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from .category import CategoryApi, CategoryListApi
from .comment import ReplyApi
from .user import FollowUserApi, BlockUserApi
from .topic import LikePostApi

bp = Blueprint('api', __name__, url_prefix='/api')
api = Api(bp)
Expand All @@ -14,3 +15,4 @@
api.add_resource(FollowUserApi, '/users/<id>/follow')
api.add_resource(BlockUserApi, '/users/<id>/block')
api.add_resource(ReplyApi, '/posts/<int:id>/replies')
api.add_resource(LikePostApi, '/posts/<int:id>/like', endpoint='like')
26 changes: 26 additions & 0 deletions firefly/views/api/topic.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# coding: utf-8

from __future__ import absolute_import

from flask_restful import Resource
from flask_security import login_required
from flask_login import current_user

from firefly.models.topic import Post


class LikePostApi(Resource):

method_decorators = [login_required]

def put(self, id):
post = Post.objects.get_or_404(id=id)
if post:
Copy link
Member

Choose a reason for hiding this comment

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

get_or_404 已经判断过了不需要再次判断。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mozillazg 好的,谢谢

Copy link
Contributor

Choose a reason for hiding this comment

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

404 会直接raise

post.likes.add(current_user.id)
return '', 201
Copy link
Member

Choose a reason for hiding this comment

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

body 为空的话,状态码一般是 204。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

通常 201 是代表修改成功吧?204 No Content 是否有些奇怪? @mozillazg

Copy link
Contributor

Choose a reason for hiding this comment

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

@killpanda @mozillazg +1 你可以看看状态码的说明 比如 https://zh.wikipedia.org/wiki/HTTP%E7%8A%B6%E6%80%81%E7%A0%81

如果只处理 但是返回空的body 正规的做法是204. 而且201一般都是POST

Copy link
Contributor

Choose a reason for hiding this comment

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

202也可以接受

Copy link
Contributor Author

Choose a reason for hiding this comment

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

那我跟 api/user.py 统一吧,PUT 202 DELETE 204

Copy link
Contributor

Choose a reason for hiding this comment

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

我又翻到一篇 https://developer.yahoo.com/social/rest_api_guide/http-response-codes.html

其实不同方法的状态码还是可以不一样的 关键是用途.


def delete(self, id):
post = Post.objects.get_or_404(id=id)
if post:
post.likes.delete(current_user.id)
return '', 201
Copy link
Contributor

Choose a reason for hiding this comment

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

delete 应该也是204

60 changes: 60 additions & 0 deletions tests/test_like.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# coding: utf-8

from __future__ import absolute_import
import pytest

from flask import url_for
from flask_login import current_user

from firefly.models.user import User
from firefly.models.topic import Category, Post


@pytest.mark.usefixtures('client_class')
class TestLike:

users = []

def setup(self):
c = Category.objects.create(
name='python', description='描述', _slug='python-slug'
)
Post.objects.create(
title='标题test', content='内容test', category=c
)

self.users = []
for x in range(3):
self.users.append(
User.create_user(
username='user' + str(x),
password='password123',
email='user' + str(x) + '@firefly.dev'
)
)

def login(self, email):
form = {
'email': email,
'password': 'password123'
}
self.client.post(
url_for('home.login'), data=form,
follow_redirects=True
)
assert current_user.is_authenticated()

def test_like(self):
post = Post.objects.first()
assert len(post.likes) == 0

for user in self.users:
self.login(user.email)
url = url_for('api.like', id=post.id)
rv = self.client.put(url)
assert rv.status_code == 201
assert len(post.likes) == len(self.users)

rv = self.client.delete(url)
assert rv.status_code == 201
assert len(post.likes) == 2