From f019f524d7a8afe267febd588ddfda0c276e47e1 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 1/16] Star a project Signed-off-by: Vivek Anand --- diff --git a/alembic/versions/c34f4b09ef18_star_a_project.py b/alembic/versions/c34f4b09ef18_star_a_project.py new file mode 100644 index 0000000..dcaecd1 --- /dev/null +++ b/alembic/versions/c34f4b09ef18_star_a_project.py @@ -0,0 +1,51 @@ +"""star_a_project + +Revision ID: c34f4b09ef18 +Revises: 27a79ff0fb41 +Create Date: 2017-07-07 00:08:18.257075 + +""" + +# revision identifiers, used by Alembic. +revision = 'c34f4b09ef18' +down_revision = '27a79ff0fb41' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ''' Add a new table to store data about who starred which project ''' + op.create_table( + 'stargazers', + sa.MetaData(), + sa.Column('id', sa.Integer, primary_key=True), + sa.Column( + 'project_id', + sa.Integer, + sa.ForeignKey( + 'projects.id', onupdate='CASCADE', ondelete='CASCADE'), + nullable=False + ), + sa.Column( + 'user_id', + sa.Integer, + sa.ForeignKey('users.id', onupdate='CASCADE', ondelete='CASCADE'), + nullable=False, + ) + ) + + op.create_unique_constraint( + constraint_name='stargazers_project_id_user_id_key', + table_name='stargazers', + columns=['project_id', 'user_id'] + ) + + +def downgrade(): + ''' Remove the stargazers table from the database ''' + op.drop_constraint( + constraint_name='stargazers_project_id_user_id_key', + table_name='stargazers' + ) + op.drop_table('stargazers') diff --git a/pagure/__init__.py b/pagure/__init__.py index 0da5514..c58df7f 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -500,6 +500,9 @@ def set_variables(): flask.g.repo_forked = pagure.get_authorized_project( SESSION, repo, user=flask.g.fas_user.username, namespace=namespace) + flask.g.repo_starred = pagure.lib.has_starred( + SESSION, flask.g.repo, user=flask.g.fas_user.username, + ) if not flask.g.repo \ and APP.config.get('OLD_VIEW_COMMIT_ENABLED', False) \ diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index db46b11..8e3d347 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -4359,3 +4359,101 @@ def get_pagination_metadata(flask_request, page, per_page, total): 'first': first_page, 'last': last_page } + + +def update_star_project(session, repo, star, user): + ''' Unset or set the star status depending on the star value. + + :arg session: the session to use to connect to the database. + :arg repo: a model.Project object representing the project to star/unstar + :arg user: string representing the user + :return: string containg 'starred' or 'unstarred' + ''' + + if not all([repo, user, star]): + return + user_obj = get_user(session, user) + if star == '1': + msg = _star_project( + session, + repo=repo, + user=user_obj, + ) + return msg + msg = _unstar_project( + session, + repo=repo, + user=user_obj, + ) + return msg + + +def _star_project(session, repo, user): + ''' Star a project + + :arg session: Session object to connect to db with + :arg repo: model.Project object representing the repo to star + :arg user: model.User object who is starring this repo + ''' + + if not all([repo, user]): + return + stargazer_obj = model.Star( + project_id=repo.id, + user_id=user.id, + ) + session.add(stargazer_obj) + return 'You liked this project!' + + +def _unstar_project(session, repo, user): + ''' Unstar a project + :arg session: Session object to connect to db with + :arg repo: model.Project object representing the repo to star + :arg user: model.User object who is unstarring this repo + ''' + + if not all([repo, user]): + return + # First find the stargazer_obj object + stargazer_obj = _get_stargazer_obj(session, repo, user) + session.delete(stargazer_obj) + return 'You didn\'t like this project :(' + + +def _get_stargazer_obj(session, repo, user): + ''' Query the db to find stargazer object with given repo and user + :arg session: Session object to connect to db with + :arg repo: model.Project object representing the repo to star + :arg user: model.User object who is starring this repo + ''' + + if not all([repo, user]): + return + stargazer_obj = session.query( + model.Star, + ).filter( + model.Star.project_id == repo.id, + ).filter( + model.Star.user_id == user.id, + ) + + return stargazer_obj.first() + + +def has_starred(session, repo, user): + ''' Check if a given user has starred a particular project + + :arg session: The session object to query the db with + :arg repo: model.Project object for which the star is checked + :arg user: The username of the user in question + :type user: str + ''' + + if not all([repo, user]): + return + user_obj = get_user(session, user) + stargazer_obj = _get_stargazer_obj(session, repo, user_obj) + if isinstance(stargazer_obj, model.Star): + return True + return False diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 15aacc0..15bfce2 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -2097,6 +2097,43 @@ class ProjectGroup(BASE): __table_args__ = (sa.UniqueConstraint('project_id', 'group_id'),) +class Star(BASE): + """ Stores users association with the all the projects which + they have starred + + Table -- star + """ + + __tablename__ = 'stargazers' + __table_args__ = ( + sa.UniqueConstraint('project_id', 'user_id'), + ) + + id = sa.Column(sa.Integer, primary_key=True) + project_id = sa.Column( + sa.Integer, + sa.ForeignKey('projects.id', onupdate='CASCADE'), + nullable=False) + user_id = sa.Column( + sa.Integer, + sa.ForeignKey('users.id', onupdate='CASCADE'), + nullable=False, + index=True + ) + user = relation( + 'User', foreign_keys=[user_id], remote_side=[User.id], + backref=backref( + 'stars', cascade="delete, delete-orphan" + ), + ) + project = relation( + 'Project', foreign_keys=[project_id], remote_side=[Project.id], + backref=backref( + 'stargazers', cascade="delete, delete-orphan", + ), + ) + + class Watcher(BASE): """ Stores the user of a projects. diff --git a/pagure/templates/master.html b/pagure/templates/master.html index 24ea2cd..085c9a2 100644 --- a/pagure/templates/master.html +++ b/pagure/templates/master.html @@ -75,6 +75,11 @@ url_for('view_user_requests', username=g.fas_user.username) }}">My Pull Requests + My Stars + + Log Out diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index e109c7a..80c5e07 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -50,6 +50,46 @@ class="btn btn-success btn-sm">New Issue {% endif %}
+ {% if not g.repo_starred %} +
+ {{ forkbuttonform.csrf_token }} +
+ + {{repo.stargazers|length}} + {% else %} +
+ {{ forkbuttonform.csrf_token }} +
+ + {{repo.stargazers|length}} + {% endif %} + {% if not repo.is_fork %} {% if g.repo_forked %} +
+

+ Starred by {{ users | length}} users +

+
+ + + + + + + + {% for user in users %} + + + + {% endfor %} + +
Stargazers of {{ repo.fullname }}
+ + {{user.user}} +
+
+
+
+
+{% endblock %} diff --git a/pagure/templates/user_stars.html b/pagure/templates/user_stars.html new file mode 100644 index 0000000..aec3a29 --- /dev/null +++ b/pagure/templates/user_stars.html @@ -0,0 +1,50 @@ +{% extends "master.html" %} + +{% block title %}My Starred Projects{% endblock %} + +{% block content %} +
+
+

+ {{ repos | length }} Projects Starred by You +

+
+ + + + + + + + {% for repo in repos %} + + + + {% endfor %} + +
My Starred Projects
+ {% if repo.avatar_email %} + + {{repo.fullname}} +
+ {% else %} + +
+ {% endif %} +
+
+
+
+{% endblock %} diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 9e4f640..fae7d19 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -432,6 +432,26 @@ def view_user_issues(username): ) +@APP.route('/user//stars/') +@APP.route('/user//stars') +def view_user_stars(username): + """ + Shows the starred projects of the specified user. + + :param username: The username whose stars we have to retrieve + :type username: str + """ + + user = _get_user(username=username) + + return flask.render_template( + 'user_stars.html', + username=username, + user=user, + repos=[star.project for star in user.stars], + ) + + @APP.route('/new/', methods=('GET', 'POST')) @APP.route('/new', methods=('GET', 'POST')) @login_required diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index d09d3c3..e2d3acd 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -2263,6 +2263,53 @@ def view_project_activity(repo, namespace=None): ) +@APP.route('//stargazers/') +@APP.route('/fork///stargazers/') +@APP.route('///stargazers/') +@APP.route('/fork////stargazers/') +def view_stargazers(repo, username=None, namespace=None): + ''' View all the users who have starred the project ''' + + stargazers = flask.g.repo.stargazers + users = [star.user for star in stargazers] + return flask.render_template( + 'repo_stargazers.html', repo=flask.g.repo, users=users) + + +@APP.route('//star/', methods=["POST"]) +@APP.route('/fork///star/', methods=["POST"]) +@APP.route('///star/', methods=["POST"]) +@APP.route('/fork////star/', methods=["POST"]) +@login_required +def star_project(repo, star, username=None, namespace=None): + ''' Star a project ''' + + return_point = flask.url_for('index') + if pagure.is_safe_url(flask.request.referrer): + return_point = flask.request.referrer + + form = pagure.forms.ConfirmationForm() + if not form.validate_on_submit(): + flask.abort(400) + + if star not in ['0', '1']: + flask.abort(400) + + try: + msg = pagure.lib.update_star_project( + SESSION, + user=flask.g.fas_user.username, + repo=flask.g.repo, + star=star, + ) + SESSION.commit() + flask.flash(msg) + except SQLAlchemyError: + flask.flash('Could not star the project') + + return flask.redirect(return_point) + + @APP.route('//watch/settings/', methods=['POST']) @APP.route('///watch/settings/', methods=['POST']) @APP.route( From 3fd23d2c7620c449892fcf2e5bd257d04580bece Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 2/16] star project: Add titles to specify what actions buttons are meant for Signed-off-by: Vivek Anand --- diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index 80c5e07..7d2268e 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -60,7 +60,7 @@ star=1)}}"> {{ forkbuttonform.csrf_token }} -
{{repo.stargazers|length}} + )}}" class="btn btn-sm btn-primary" title="Star count. Click to see who all have starred">{{repo.stargazers|length}} {% else %}
{{ forkbuttonform.csrf_token }}
- {{repo.stargazers|length}} + )}}" class="btn btn-sm btn-primary" title="Star count. Click to see who all have starred">{{repo.stargazers|length}} {% endif %} {% if not repo.is_fork %} From 0b85e6d07b75527df854ca3de174b5213a85baa5 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 3/16] star repo: index project_id, add ondelete cascade in models Signed-off-by: Vivek Anand --- diff --git a/alembic/versions/c34f4b09ef18_star_a_project.py b/alembic/versions/c34f4b09ef18_star_a_project.py index dcaecd1..4f70daf 100644 --- a/alembic/versions/c34f4b09ef18_star_a_project.py +++ b/alembic/versions/c34f4b09ef18_star_a_project.py @@ -25,7 +25,8 @@ def upgrade(): sa.Integer, sa.ForeignKey( 'projects.id', onupdate='CASCADE', ondelete='CASCADE'), - nullable=False + nullable=False, + index=True, ), sa.Column( 'user_id', diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 15bfce2..71232f0 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -2112,13 +2112,14 @@ class Star(BASE): id = sa.Column(sa.Integer, primary_key=True) project_id = sa.Column( sa.Integer, - sa.ForeignKey('projects.id', onupdate='CASCADE'), - nullable=False) + sa.ForeignKey('projects.id', onupdate='CASCADE', ondelete='CASCADE'), + nullable=False, + index=True, + ) user_id = sa.Column( sa.Integer, - sa.ForeignKey('users.id', onupdate='CASCADE'), + sa.ForeignKey('users.id', onupdate='CASCADE', ondelete='CASCADE'), nullable=False, - index=True ) user = relation( 'User', foreign_keys=[user_id], remote_side=[User.id], From ed7323eac1225b03eaa9d26823a957a82fd4dbf3 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 4/16] star repo: name the unique constraint in model as well Signed-off-by: Vivek Anand --- diff --git a/alembic/versions/c34f4b09ef18_star_a_project.py b/alembic/versions/c34f4b09ef18_star_a_project.py index 4f70daf..d6c5272 100644 --- a/alembic/versions/c34f4b09ef18_star_a_project.py +++ b/alembic/versions/c34f4b09ef18_star_a_project.py @@ -37,7 +37,7 @@ def upgrade(): ) op.create_unique_constraint( - constraint_name='stargazers_project_id_user_id_key', + constraint_name='uq_stargazers_project_id_user_id_key', table_name='stargazers', columns=['project_id', 'user_id'] ) @@ -46,7 +46,7 @@ def upgrade(): def downgrade(): ''' Remove the stargazers table from the database ''' op.drop_constraint( - constraint_name='stargazers_project_id_user_id_key', + constraint_name='uq_stargazers_project_id_user_id_key', table_name='stargazers' ) op.drop_table('stargazers') diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 71232f0..fd4b130 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -2106,7 +2106,10 @@ class Star(BASE): __tablename__ = 'stargazers' __table_args__ = ( - sa.UniqueConstraint('project_id', 'user_id'), + sa.UniqueConstraint( + 'project_id', + 'user_id', + name='uq_stargazers_project_id_user_id_key'), ) id = sa.Column(sa.Integer, primary_key=True) From 5d97214c8d4e827158a9761295f3e17f8f5df2f0 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 5/16] star repo: use if/else construct in update_star function Signed-off-by: Vivek Anand --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 8e3d347..914506f 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -4379,12 +4379,12 @@ def update_star_project(session, repo, star, user): repo=repo, user=user_obj, ) - return msg - msg = _unstar_project( - session, - repo=repo, - user=user_obj, - ) + else: + msg = _unstar_project( + session, + repo=repo, + user=user_obj, + ) return msg From 1c674fe302251fd4cfa7fcdde28de0e11944c88d Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 6/16] star repo: remove titles, change return msgs Signed-off-by: Vivek Anand --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 914506f..0f53f64 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -4403,7 +4403,7 @@ def _star_project(session, repo, user): user_id=user.id, ) session.add(stargazer_obj) - return 'You liked this project!' + return 'You starred this project' def _unstar_project(session, repo, user): @@ -4418,7 +4418,7 @@ def _unstar_project(session, repo, user): # First find the stargazer_obj object stargazer_obj = _get_stargazer_obj(session, repo, user) session.delete(stargazer_obj) - return 'You didn\'t like this project :(' + return 'You unstarred this project' def _get_stargazer_obj(session, repo, user): diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index 7d2268e..80c5e07 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -60,7 +60,7 @@ star=1)}}"> {{ forkbuttonform.csrf_token }} - {{repo.stargazers|length}} + )}}" class="btn btn-sm btn-primary">{{repo.stargazers|length}} {% else %}
{{ forkbuttonform.csrf_token }}
- {{repo.stargazers|length}} + )}}" class="btn btn-sm btn-primary">{{repo.stargazers|length}} {% endif %} {% if not repo.is_fork %} From bd4dd9b56a0027e8fee37bf877a4dd6dd748a02d Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 7/16] star repo: update view - star_project docstring Signed-off-by: Vivek Anand --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index e2d3acd..ceb475e 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -2282,7 +2282,15 @@ def view_stargazers(repo, username=None, namespace=None): @APP.route('/fork////star/', methods=["POST"]) @login_required def star_project(repo, star, username=None, namespace=None): - ''' Star a project ''' + ''' Star or Unstar a project + + :arg repo: string representing the project which has to be starred or + unstarred. + :arg star: either '0' or '1' for unstar and star respectively + :arg username: string representing the user the fork of whose is being + starred or unstarred. + :arg namespace: namespace of the project if any + ''' return_point = flask.url_for('index') if pagure.is_safe_url(flask.request.referrer): From f37b5cd6442e68876599acf23fd6c64adcd6e429 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 8/16] star repo: check the object exists before deleting --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 0f53f64..7e4b196 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -4417,8 +4417,12 @@ def _unstar_project(session, repo, user): return # First find the stargazer_obj object stargazer_obj = _get_stargazer_obj(session, repo, user) - session.delete(stargazer_obj) - return 'You unstarred this project' + if isinstance(stargazer_obj, model.Star): + session.delete(stargazer_obj) + msg = 'You unstarred this project' + else: + msg = 'You never starred the project' + return msg def _get_stargazer_obj(session, repo, user): From 4c431239cc5a1b923d96cd975e3fc205e8ec7f03 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 9/16] star_project: update alembic down revision after date_modified in projects Signed-off-by: Vivek Anand --- diff --git a/alembic/versions/c34f4b09ef18_star_a_project.py b/alembic/versions/c34f4b09ef18_star_a_project.py index d6c5272..40607b3 100644 --- a/alembic/versions/c34f4b09ef18_star_a_project.py +++ b/alembic/versions/c34f4b09ef18_star_a_project.py @@ -1,14 +1,14 @@ """star_a_project Revision ID: c34f4b09ef18 -Revises: 27a79ff0fb41 +Revises: 8a5d68f74beb Create Date: 2017-07-07 00:08:18.257075 """ # revision identifiers, used by Alembic. revision = 'c34f4b09ef18' -down_revision = '27a79ff0fb41' +down_revision = '8a5d68f74beb' from alembic import op import sqlalchemy as sa From 327a4f7c27a060824906908118a3a19f0cc8493a Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 10/16] Unit test: add lib tests for star project Signed-off-by: Vivek Anand --- diff --git a/tests/test_pagure_lib_star_project.py b/tests/test_pagure_lib_star_project.py new file mode 100644 index 0000000..3246aa2 --- /dev/null +++ b/tests/test_pagure_lib_star_project.py @@ -0,0 +1,249 @@ +# coding=utf-8 +""" + (c) 2015-2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + Vivek Anand + +""" + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources + +import unittest +import sys +import os + +from mock import patch + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure +import pagure.lib +import tests + +class TestStarProjectLib(tests.SimplePagureTest): + ''' Test the star project feature of pagure ''' + + def setUp(self): + """ Set up the environnment for running each star project lib tests """ + super(TestStarProjectLib, self).setUp() + tests.create_projects(self.session) + + def test_update_star_project(self): + ''' Test the update_star_project endpoint of pagure.lib ''' + + self.repo_obj = pagure.lib._get_project(self.session, 'test') + # test with invalud Star object, should return None + msg = pagure.lib.update_star_project( + self.session, + self.repo_obj, + None, + 'pingou', + ) + self.session.commit() + self.assertEqual(msg, None) + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 0) + + # test starring the project + self.repo_obj = pagure.lib._get_project(self.session, 'test') + msg = pagure.lib.update_star_project( + self.session, + self.repo_obj, + '1', + 'pingou', + ) + + self.session.commit() + self.assertEqual(msg, 'You starred this project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 1) + + # test unstarring the project + self.repo_obj = pagure.lib._get_project(self.session, 'test') + msg = pagure.lib.update_star_project( + self.session, + self.repo_obj, + '0', + 'pingou', + ) + + self.session.commit() + self.assertEqual(msg, 'You unstarred this project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 0) + + def test_star_project(self): + ''' Test the _star_project endpoint of pagure.lib ''' + + # test with not all arguments present + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.repo_obj = pagure.lib._get_project(self.session, 'test') + msg = pagure.lib._star_project( + self.session, + self.repo_obj, + None + ) + self.session.commit() + self.assertEqual(msg, None) + + self.repo_obj = pagure.lib._get_project(self.session, 'test') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + msg = pagure.lib._star_project( + self.session, + self.repo_obj, + self.user_obj, + ) + self.session.commit() + self.assertEqual(msg, 'You starred this project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 1) + + def test_unstar_project(self): + ''' Test the _unstar_project endpoint of pagure.lib ''' + + # test with not all arguments present + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.repo_obj = pagure.lib._get_project(self.session, 'test') + msg = pagure.lib._unstar_project( + self.session, + self.repo_obj, + None + ) + self.session.commit() + self.assertEqual(msg, None) + + # the user hasn't starred the project before + self.repo_obj = pagure.lib._get_project(self.session, 'test') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + msg = pagure.lib._unstar_project( + self.session, + self.repo_obj, + self.user_obj, + ) + self.assertEqual(msg, 'You never starred the project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 0) + + # star it for testing + msg = pagure.lib._star_project( + self.session, + self.repo_obj, + self.user_obj, + ) + self.session.commit() + self.assertEqual(msg, 'You starred this project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 1) + + # the user starred and wishes to unstar + self.repo_obj = pagure.lib._get_project(self.session, 'test') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + msg = pagure.lib._unstar_project( + self.session, + self.repo_obj, + self.user_obj, + ) + self.session.commit() + self.assertEqual(msg, 'You unstarred this project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 0) + + def test_get_stargazer_obj(self): + ''' Test the _get_stargazer_obj test of pagure.lib ''' + + # star the project first + self.repo_obj = pagure.lib._get_project(self.session, 'test') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + msg = pagure.lib._star_project( + self.session, + self.repo_obj, + self.user_obj, + ) + self.session.commit() + self.assertEqual(msg, 'You starred this project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 1) + + # get the object now + self.repo_obj = pagure.lib._get_project(self.session, 'test') + star_obj = pagure.lib._get_stargazer_obj( + self.session, + self.repo_obj, + self.user_obj + ) + self.assertEqual(isinstance(star_obj, pagure.lib.model.Star), True) + + # unstar it and then try to get the object + self.repo_obj = pagure.lib._get_project(self.session, 'test') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + msg = pagure.lib._unstar_project( + self.session, + self.repo_obj, + self.user_obj, + ) + self.session.commit() + self.assertEqual(msg, 'You unstarred this project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 0) + + # we don't store if the user has unstarred, we delete the obj + # so, we should get anything back in the query + self.repo_obj = pagure.lib._get_project(self.session, 'test') + star_obj = pagure.lib._get_stargazer_obj( + self.session, + self.repo_obj, + self.user_obj + ) + self.assertEqual(star_obj is None, True) + + def test_has_starred(self): + ''' Test the has_starred endpoint of pagure.lib ''' + + # star the project + self.repo_obj = pagure.lib._get_project(self.session, 'test') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + msg = pagure.lib._star_project( + self.session, + self.repo_obj, + self.user_obj, + ) + self.session.commit() + self.assertEqual(msg, 'You starred this project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 1) + + has_starred = pagure.lib.has_starred( + self.session, + self.repo_obj, + 'pingou' + ) + self.assertEqual(has_starred is True, True) + + # unstar it and then test for has_starred + self.repo_obj = pagure.lib._get_project(self.session, 'test') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + msg = pagure.lib._unstar_project( + self.session, + self.repo_obj, + self.user_obj, + ) + self.session.commit() + self.assertEqual(msg, 'You unstarred this project') + self.user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(self.user_obj.stars), 0) + + # check now, it should return False + has_starred = pagure.lib.has_starred( + self.session, + self.repo_obj, + 'pingou' + ) + self.assertEqual(has_starred is False, True) + + +if __name__ == '__main__': + unittest.main(verbosity=2) From 08b6a4e3d315d1e9a5adb03b8dafa1df19fac7d7 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 11/16] Unit test: add ui tests for star project --- diff --git a/tests/test_pagure_flask_ui_star_project.py b/tests/test_pagure_flask_ui_star_project.py new file mode 100644 index 0000000..7b2d417 --- /dev/null +++ b/tests/test_pagure_flask_ui_star_project.py @@ -0,0 +1,268 @@ +# coding=utf-8 + +""" + (c) 2016 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources + +import datetime +import json +import unittest +import shutil +import sys +import tempfile +import os + +import pygit2 + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure +import pagure.lib +import tests + +class TestStarProjectUI(tests.SimplePagureTest): + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(TestStarProjectUI, self).setUp() + + pagure.APP.config['TESTING'] = True + pagure.SESSION = self.session + pagure.ui.SESSION = self.session + pagure.ui.app.SESSION = self.session + pagure.ui.filters.SESSION = self.session + pagure.ui.repo.SESSION = self.session + pagure.ui.issues.SESSION = self.session + + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) + + def _check_star_count(self, data, stars=1): + """ Check if the star count is correct or not """ + output = self.app.get( + '/test/', data=data, follow_redirects=True) + if stars == 1: + self.assertIn( + '1', + output.data + ) + elif stars == 0: + self.assertIn( + '0', + output.data + ) + + def test_star_project_no_project(self): + """ Test the star_project endpoint. """ + + # No such project + output = self.app.post('/test42/star/1') + self.assertEqual(output.status_code, 404) + + def test_star_project_no_csrf(self): + """ Test the star_project endpoint for the case when there + is no CSRF token given """ + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + + data = {} + output = self.app.post( + '/test/star/1', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 400) + + def test_star_project_invalid_star(self): + """ Test the star_project endpoint for invalid star """ + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + data = { + 'csrf_token': csrf_token, + } + output = self.app.post( + '/test/star/2', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 400) + self._check_star_count(data=data, stars=0) + + def test_star_project_valid_star(self): + """ Test the star_project endpoint for correct start """ + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + data = { + 'csrf_token': csrf_token, + } + + # try starring the project for pingou + output = self.app.post( + '/test/star/1', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '\n You starred ' + 'this project\n ', + output.data) + + # check home page of project for star count + self._check_star_count(data=data, stars=1) + + # try unstarring the project for pingou + output = self.app.post( + '/test/star/0', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '\n You unstarred ' + 'this project\n ', + output.data) + self._check_star_count(data=data, stars=0) + + def test_repo_stargazers(self): + """ Test the repo_stargazers endpoint of pagure.ui.repo """ + + # make pingou star the project + # first create pingou + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + data = { + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/test/star/1', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '\n You starred ' + 'this project\n ', + output.data) + self._check_star_count(data=data, stars=1) + + # now, test if pingou's name comes in repo stargazers + output = self.app.get( + '/test/stargazers/' + ) + self.assertIn( + 'Stargazers of test - Pagure', + output.data + ) + self.assertIn( + 'pingou\n ', + output.data + ) + + # make pingou unstar the project + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + data = { + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/test/star/0', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '\n You unstarred ' + 'this project\n ', + output.data) + self._check_star_count(data=data, stars=0) + + # now, test if pingou's name comes in repo stargazers + # it shouldn't because, he just unstarred + output = self.app.get( + '/test/stargazers/' + ) + self.assertIn( + 'Stargazers of test - Pagure', + output.data + ) + self.assertNotIn( + 'pingou\n ', + output.data + ) + + def test_user_stars(self): + """ Test the user_stars endpoint of pagure.ui.app """ + + # make pingou star the project + # first create pingou + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + data = { + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/test/star/1', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '\n You starred ' + 'this project\n ', + output.data) + self._check_star_count(data=data, stars=1) + + # now, test if the project 'test' comes in pingou's stars + output = self.app.get( + '/user/pingou/stars' + ) + self.assertIn( + 'My Starred Projects - Pagure', + output.data + ) + self.assertIn( + 'test\n', + output.data + ) + + # make pingou unstar the project + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + csrf_token = self.get_csrf() + data = { + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/test/star/0', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '\n You unstarred ' + 'this project\n ', + output.data) + self._check_star_count(data=data, stars=0) + + # now, test if test's name comes in pingou's stars + # it shouldn't because, he just unstarred + output = self.app.get( + '/user/pingou/stars/' + ) + self.assertIn( + 'My Starred Projects - Pagure', + output.data + ) + self.assertNotIn( + 'test\n', + output.data + ) + + +if __name__ == '__main__': + unittest.main(verbosity=2) From e28e4b195a8e7aaa5c3cfa207afd55e681f777e9 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 12/16] star repo: Check for url safety only when referrer is there While testing, the referrer is not there and pagure.is_safe_url return True in case the referrer is not there. Thus, it used to give 404s instead of returning to index Signed-off-by: Vivek Anand --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index ceb475e..2a2dcc2 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -2293,7 +2293,9 @@ def star_project(repo, star, username=None, namespace=None): ''' return_point = flask.url_for('index') - if pagure.is_safe_url(flask.request.referrer): + if ( + flask.request.referrer is not None and + pagure.is_safe_url(flask.request.referrer)): return_point = flask.request.referrer form = pagure.forms.ConfirmationForm() diff --git a/tests/test_pagure_flask_ui_star_project.py b/tests/test_pagure_flask_ui_star_project.py index 7b2d417..0b01bb2 100644 --- a/tests/test_pagure_flask_ui_star_project.py +++ b/tests/test_pagure_flask_ui_star_project.py @@ -5,22 +5,17 @@ Authors: Pierre-Yves Chibon + Vivek Anand """ __requires__ = ['SQLAlchemy >= 0.8'] import pkg_resources -import datetime -import json import unittest -import shutil import sys -import tempfile import os -import pygit2 - sys.path.insert(0, os.path.join(os.path.dirname( os.path.abspath(__file__)), '..')) @@ -28,6 +23,7 @@ import pagure import pagure.lib import tests + class TestStarProjectUI(tests.SimplePagureTest): def setUp(self): """ Set up the environnment, ran before every tests. """ @@ -50,15 +46,15 @@ class TestStarProjectUI(tests.SimplePagureTest): '/test/', data=data, follow_redirects=True) if stars == 1: self.assertIn( - '1', - output.data + '1', + output.data ) elif stars == 0: self.assertIn( - '0', - output.data + '0', + output.data ) def test_star_project_no_project(self): @@ -112,9 +108,10 @@ class TestStarProjectUI(tests.SimplePagureTest): '/test/star/1', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( - '\n You starred ' - 'this project\n ', - output.data) + '\n You starred ' + 'this project\n ', + output.data + ) # check home page of project for star count self._check_star_count(data=data, stars=1) @@ -124,9 +121,10 @@ class TestStarProjectUI(tests.SimplePagureTest): '/test/star/0', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( - '\n You unstarred ' - 'this project\n ', - output.data) + '\n You unstarred ' + 'this project\n ', + output.data + ) self._check_star_count(data=data, stars=0) def test_repo_stargazers(self): @@ -146,9 +144,10 @@ class TestStarProjectUI(tests.SimplePagureTest): '/test/star/1', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( - '\n You starred ' - 'this project\n ', - output.data) + '\n You starred ' + 'this project\n ', + output.data + ) self._check_star_count(data=data, stars=1) # now, test if pingou's name comes in repo stargazers @@ -156,12 +155,12 @@ class TestStarProjectUI(tests.SimplePagureTest): '/test/stargazers/' ) self.assertIn( - 'Stargazers of test - Pagure', - output.data + 'Stargazers of test - Pagure', + output.data ) self.assertIn( - 'pingou\n ', - output.data + 'pingou\n ', + output.data ) # make pingou unstar the project @@ -177,9 +176,10 @@ class TestStarProjectUI(tests.SimplePagureTest): '/test/star/0', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( - '\n You unstarred ' - 'this project\n ', - output.data) + '\n You unstarred ' + 'this project\n ', + output.data + ) self._check_star_count(data=data, stars=0) # now, test if pingou's name comes in repo stargazers @@ -188,12 +188,12 @@ class TestStarProjectUI(tests.SimplePagureTest): '/test/stargazers/' ) self.assertIn( - 'Stargazers of test - Pagure', - output.data + 'Stargazers of test - Pagure', + output.data ) self.assertNotIn( - 'pingou\n ', - output.data + 'pingou\n ', + output.data ) def test_user_stars(self): @@ -213,9 +213,10 @@ class TestStarProjectUI(tests.SimplePagureTest): '/test/star/1', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( - '\n You starred ' - 'this project\n ', - output.data) + '\n You starred ' + 'this project\n ', + output.data + ) self._check_star_count(data=data, stars=1) # now, test if the project 'test' comes in pingou's stars @@ -223,12 +224,12 @@ class TestStarProjectUI(tests.SimplePagureTest): '/user/pingou/stars' ) self.assertIn( - 'My Starred Projects - Pagure', - output.data + 'My Starred Projects - Pagure', + output.data ) self.assertIn( - 'test\n', - output.data + 'test\n', + output.data ) # make pingou unstar the project @@ -239,14 +240,14 @@ class TestStarProjectUI(tests.SimplePagureTest): data = { 'csrf_token': csrf_token, } - output = self.app.post( '/test/star/0', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( - '\n You unstarred ' - 'this project\n ', - output.data) + '\n You unstarred ' + 'this project\n ', + output.data + ) self._check_star_count(data=data, stars=0) # now, test if test's name comes in pingou's stars @@ -255,12 +256,12 @@ class TestStarProjectUI(tests.SimplePagureTest): '/user/pingou/stars/' ) self.assertIn( - 'My Starred Projects - Pagure', - output.data + 'My Starred Projects - Pagure', + output.data ) self.assertNotIn( - 'test\n', - output.data + 'test\n', + output.data ) diff --git a/tests/test_pagure_lib_star_project.py b/tests/test_pagure_lib_star_project.py index 3246aa2..ad0d323 100644 --- a/tests/test_pagure_lib_star_project.py +++ b/tests/test_pagure_lib_star_project.py @@ -15,8 +15,6 @@ import unittest import sys import os -from mock import patch - sys.path.insert(0, os.path.join(os.path.dirname( os.path.abspath(__file__)), '..')) @@ -24,6 +22,7 @@ import pagure import pagure.lib import tests + class TestStarProjectLib(tests.SimplePagureTest): ''' Test the star project feature of pagure ''' @@ -35,168 +34,168 @@ class TestStarProjectLib(tests.SimplePagureTest): def test_update_star_project(self): ''' Test the update_star_project endpoint of pagure.lib ''' - self.repo_obj = pagure.lib._get_project(self.session, 'test') + repo_obj = pagure.lib._get_project(self.session, 'test') # test with invalud Star object, should return None msg = pagure.lib.update_star_project( self.session, - self.repo_obj, + repo_obj, None, 'pingou', ) self.session.commit() self.assertEqual(msg, None) - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 0) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 0) # test starring the project - self.repo_obj = pagure.lib._get_project(self.session, 'test') + repo_obj = pagure.lib._get_project(self.session, 'test') msg = pagure.lib.update_star_project( self.session, - self.repo_obj, + repo_obj, '1', 'pingou', ) self.session.commit() self.assertEqual(msg, 'You starred this project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 1) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 1) # test unstarring the project - self.repo_obj = pagure.lib._get_project(self.session, 'test') + repo_obj = pagure.lib._get_project(self.session, 'test') msg = pagure.lib.update_star_project( self.session, - self.repo_obj, + repo_obj, '0', 'pingou', ) self.session.commit() self.assertEqual(msg, 'You unstarred this project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 0) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 0) def test_star_project(self): ''' Test the _star_project endpoint of pagure.lib ''' # test with not all arguments present - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.repo_obj = pagure.lib._get_project(self.session, 'test') + user_obj = pagure.lib.get_user(self.session, 'pingou') + repo_obj = pagure.lib._get_project(self.session, 'test') msg = pagure.lib._star_project( self.session, - self.repo_obj, + repo_obj, None ) self.session.commit() self.assertEqual(msg, None) - self.repo_obj = pagure.lib._get_project(self.session, 'test') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') + repo_obj = pagure.lib._get_project(self.session, 'test') + user_obj = pagure.lib.get_user(self.session, 'pingou') msg = pagure.lib._star_project( self.session, - self.repo_obj, - self.user_obj, + repo_obj, + user_obj, ) self.session.commit() self.assertEqual(msg, 'You starred this project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 1) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 1) def test_unstar_project(self): ''' Test the _unstar_project endpoint of pagure.lib ''' # test with not all arguments present - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.repo_obj = pagure.lib._get_project(self.session, 'test') + user_obj = pagure.lib.get_user(self.session, 'pingou') + repo_obj = pagure.lib._get_project(self.session, 'test') msg = pagure.lib._unstar_project( self.session, - self.repo_obj, + repo_obj, None ) self.session.commit() self.assertEqual(msg, None) # the user hasn't starred the project before - self.repo_obj = pagure.lib._get_project(self.session, 'test') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') + repo_obj = pagure.lib._get_project(self.session, 'test') + user_obj = pagure.lib.get_user(self.session, 'pingou') msg = pagure.lib._unstar_project( self.session, - self.repo_obj, - self.user_obj, + repo_obj, + user_obj, ) self.assertEqual(msg, 'You never starred the project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 0) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 0) # star it for testing msg = pagure.lib._star_project( self.session, - self.repo_obj, - self.user_obj, + repo_obj, + user_obj, ) self.session.commit() self.assertEqual(msg, 'You starred this project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 1) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 1) # the user starred and wishes to unstar - self.repo_obj = pagure.lib._get_project(self.session, 'test') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') + repo_obj = pagure.lib._get_project(self.session, 'test') + user_obj = pagure.lib.get_user(self.session, 'pingou') msg = pagure.lib._unstar_project( self.session, - self.repo_obj, - self.user_obj, + repo_obj, + user_obj, ) self.session.commit() self.assertEqual(msg, 'You unstarred this project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 0) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 0) def test_get_stargazer_obj(self): ''' Test the _get_stargazer_obj test of pagure.lib ''' # star the project first - self.repo_obj = pagure.lib._get_project(self.session, 'test') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') + repo_obj = pagure.lib._get_project(self.session, 'test') + user_obj = pagure.lib.get_user(self.session, 'pingou') msg = pagure.lib._star_project( self.session, - self.repo_obj, - self.user_obj, + repo_obj, + user_obj, ) self.session.commit() self.assertEqual(msg, 'You starred this project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 1) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 1) # get the object now - self.repo_obj = pagure.lib._get_project(self.session, 'test') + repo_obj = pagure.lib._get_project(self.session, 'test') star_obj = pagure.lib._get_stargazer_obj( self.session, - self.repo_obj, - self.user_obj + repo_obj, + user_obj ) self.assertEqual(isinstance(star_obj, pagure.lib.model.Star), True) # unstar it and then try to get the object - self.repo_obj = pagure.lib._get_project(self.session, 'test') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') + repo_obj = pagure.lib._get_project(self.session, 'test') + user_obj = pagure.lib.get_user(self.session, 'pingou') msg = pagure.lib._unstar_project( self.session, - self.repo_obj, - self.user_obj, + repo_obj, + user_obj, ) self.session.commit() self.assertEqual(msg, 'You unstarred this project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 0) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 0) # we don't store if the user has unstarred, we delete the obj # so, we should get anything back in the query - self.repo_obj = pagure.lib._get_project(self.session, 'test') + repo_obj = pagure.lib._get_project(self.session, 'test') star_obj = pagure.lib._get_stargazer_obj( self.session, - self.repo_obj, - self.user_obj + repo_obj, + user_obj ) self.assertEqual(star_obj is None, True) @@ -204,42 +203,42 @@ class TestStarProjectLib(tests.SimplePagureTest): ''' Test the has_starred endpoint of pagure.lib ''' # star the project - self.repo_obj = pagure.lib._get_project(self.session, 'test') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') + repo_obj = pagure.lib._get_project(self.session, 'test') + user_obj = pagure.lib.get_user(self.session, 'pingou') msg = pagure.lib._star_project( self.session, - self.repo_obj, - self.user_obj, + repo_obj, + user_obj, ) self.session.commit() self.assertEqual(msg, 'You starred this project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 1) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 1) has_starred = pagure.lib.has_starred( self.session, - self.repo_obj, + repo_obj, 'pingou' ) self.assertEqual(has_starred is True, True) # unstar it and then test for has_starred - self.repo_obj = pagure.lib._get_project(self.session, 'test') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') + repo_obj = pagure.lib._get_project(self.session, 'test') + user_obj = pagure.lib.get_user(self.session, 'pingou') msg = pagure.lib._unstar_project( self.session, - self.repo_obj, - self.user_obj, + repo_obj, + user_obj, ) self.session.commit() self.assertEqual(msg, 'You unstarred this project') - self.user_obj = pagure.lib.get_user(self.session, 'pingou') - self.assertEqual(len(self.user_obj.stars), 0) + user_obj = pagure.lib.get_user(self.session, 'pingou') + self.assertEqual(len(user_obj.stars), 0) # check now, it should return False has_starred = pagure.lib.has_starred( self.session, - self.repo_obj, + repo_obj, 'pingou' ) self.assertEqual(has_starred is False, True) From 7746b74e80f58bcddc9765520423a8d546e21ee2 Mon Sep 17 00:00:00 2001 From: Vivek Anand Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 13/16] star project: fix docstrings in lib and ui Signed-off-by: Vivek Anand --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 7e4b196..77b684a 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -4366,20 +4366,23 @@ def update_star_project(session, repo, star, user): :arg session: the session to use to connect to the database. :arg repo: a model.Project object representing the project to star/unstar + :arg star: '1' for starring and '0' for unstarring :arg user: string representing the user - :return: string containg 'starred' or 'unstarred' + :return: None or string containing 'You starred this project' or + 'You unstarred this project' ''' if not all([repo, user, star]): return user_obj = get_user(session, user) + msg = None if star == '1': msg = _star_project( session, repo=repo, user=user_obj, ) - else: + elif star == '0': msg = _unstar_project( session, repo=repo, @@ -4394,6 +4397,7 @@ def _star_project(session, repo, user): :arg session: Session object to connect to db with :arg repo: model.Project object representing the repo to star :arg user: model.User object who is starring this repo + :return: None or string containing 'You starred this project' ''' if not all([repo, user]): @@ -4409,8 +4413,10 @@ def _star_project(session, repo, user): def _unstar_project(session, repo, user): ''' Unstar a project :arg session: Session object to connect to db with - :arg repo: model.Project object representing the repo to star + :arg repo: model.Project object representing the repo to unstar :arg user: model.User object who is unstarring this repo + :return: None or string containing 'You unstarred this project' + or 'You never starred the project' ''' if not all([repo, user]): @@ -4428,8 +4434,9 @@ def _unstar_project(session, repo, user): def _get_stargazer_obj(session, repo, user): ''' Query the db to find stargazer object with given repo and user :arg session: Session object to connect to db with - :arg repo: model.Project object representing the repo to star - :arg user: model.User object who is starring this repo + :arg repo: model.Project object + :arg user: model.User object + :return: None or model.Star object ''' if not all([repo, user]): @@ -4451,7 +4458,7 @@ def has_starred(session, repo, user): :arg session: The session object to query the db with :arg repo: model.Project object for which the star is checked :arg user: The username of the user in question - :type user: str + :return: True if user has starred the project, False otherwise ''' if not all([repo, user]): diff --git a/pagure/ui/app.py b/pagure/ui/app.py index fae7d19..d1f6ae7 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -438,8 +438,7 @@ def view_user_stars(username): """ Shows the starred projects of the specified user. - :param username: The username whose stars we have to retrieve - :type username: str + :arg username: The username whose stars we have to retrieve """ user = _get_user(username=username) diff --git a/tests/test_pagure_flask_ui_star_project.py b/tests/test_pagure_flask_ui_star_project.py index 0b01bb2..5ef54b7 100644 --- a/tests/test_pagure_flask_ui_star_project.py +++ b/tests/test_pagure_flask_ui_star_project.py @@ -1,7 +1,7 @@ # coding=utf-8 """ - (c) 2016 - Copyright Red Hat Inc + (c) 2017 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon @@ -93,7 +93,7 @@ class TestStarProjectUI(tests.SimplePagureTest): self._check_star_count(data=data, stars=0) def test_star_project_valid_star(self): - """ Test the star_project endpoint for correct start """ + """ Test the star_project endpoint for correct star """ user = tests.FakeUser() user.username = 'pingou' From e48a84ade835567b9a4b3002ebf8cbb0114e1fdf Mon Sep 17 00:00:00 2001 From: Clement Verna Date: Aug 31 2017 13:50:47 +0000 Subject: [PATCH 14/16] star repo: UI fixes 1. repo_master: Give the star button it's own div 2. user_stars page: Show username instead of 'you'/'my' in title and header 3. user_stars page: fix size of project's data-glyph Signed-off-by: Clement Verna --- diff --git a/alembic/versions/c34f4b09ef18_star_a_project.py b/alembic/versions/c34f4b09ef18_star_a_project.py index 40607b3..e8bb7ec 100644 --- a/alembic/versions/c34f4b09ef18_star_a_project.py +++ b/alembic/versions/c34f4b09ef18_star_a_project.py @@ -18,7 +18,6 @@ def upgrade(): ''' Add a new table to store data about who starred which project ''' op.create_table( 'stargazers', - sa.MetaData(), sa.Column('id', sa.Integer, primary_key=True), sa.Column( 'project_id', diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index 80c5e07..26367e3 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -89,7 +89,9 @@ namespace=repo.namespace, )}}" class="btn btn-sm btn-primary">{{repo.stargazers|length}} {% endif %} + +
{% if not repo.is_fork %} {% if g.repo_forked %}

- {{ repos | length }} Projects Starred by You + {{ repos | length }} Projects Starred by {{ username }}


- + @@ -29,14 +29,15 @@ )}}">{{repo.fullname}}
{% else %} -
{% endif %} From da67fd65f0d8e9e3ba5911a0e23bf2db59a3989e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 31 2017 15:48:35 +0000 Subject: [PATCH 15/16] Fix the tests for the changes in the star page --- diff --git a/tests/test_pagure_flask_ui_star_project.py b/tests/test_pagure_flask_ui_star_project.py index 5ef54b7..059422b 100644 --- a/tests/test_pagure_flask_ui_star_project.py +++ b/tests/test_pagure_flask_ui_star_project.py @@ -224,7 +224,7 @@ class TestStarProjectUI(tests.SimplePagureTest): '/user/pingou/stars' ) self.assertIn( - 'My Starred Projects - Pagure', + "pingou's starred Projects - Pagure", output.data ) self.assertIn( @@ -256,7 +256,7 @@ class TestStarProjectUI(tests.SimplePagureTest): '/user/pingou/stars/' ) self.assertIn( - 'My Starred Projects - Pagure', + "pingou's starred Projects - Pagure", output.data ) self.assertNotIn( From 7c1f78bb4a6d39060c17263ad9d73ea59795abdd Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 31 2017 15:49:02 +0000 Subject: [PATCH 16/16] Use search_user() instead of get_user() so it does not raise an exception Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 77b684a..6c1975c 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -4463,7 +4463,7 @@ def has_starred(session, repo, user): if not all([repo, user]): return - user_obj = get_user(session, user) + user_obj = search_user(session, username=user) stargazer_obj = _get_stargazer_obj(session, repo, user_obj) if isinstance(stargazer_obj, model.Star): return True
My Starred ProjectsStarred Projects