From 3bcaeea2ca929c2319129274b2c5a32a7d26487f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 08:30:22 +0000 Subject: [PATCH 1/4] Fix the buttons within a tab to be white as they should be --- diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index 3ac7fab..deb0388 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -874,6 +874,10 @@ span.CONFLICTS { color: white!important; } +.tabs .ui-widget-content .button{ + color: white!important; +} + .tabs .ui-state-hover a:hover{ color: white; } From 2bbe0763c1117b5fed2ceb5cb9d75a7e55e78b36 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 08:33:33 +0000 Subject: [PATCH 2/4] Project wide pep8 clean up --- diff --git a/pagure/forms.py b/pagure/forms.py index 98825ba..40b6260 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -158,6 +158,7 @@ class AddPullRequestCommentForm(wtf.Form): [wtforms.validators.Required()] ) + class AddPullRequestFlagForm(wtf.Form): ''' Form to add a flag to a pull-request. ''' username = wtforms.TextField( diff --git a/pagure/hooks/files/fedmsg_hook.py b/pagure/hooks/files/fedmsg_hook.py index 60b9352..f837fa4 100755 --- a/pagure/hooks/files/fedmsg_hook.py +++ b/pagure/hooks/files/fedmsg_hook.py @@ -94,7 +94,7 @@ for line in sys.stdin.readlines(): path=abspath, username=username, repo=project.to_json(public=True) - if not isinstance(project, basestring) else project, + if not isinstance(project, basestring) else project, branch=refname, agent=os.getlogin(), ) diff --git a/pagure/hooks/files/pagure_hook_tickets.py b/pagure/hooks/files/pagure_hook_tickets.py index 7a7d320..240b4fe 100755 --- a/pagure/hooks/files/pagure_hook_tickets.py +++ b/pagure/hooks/files/pagure_hook_tickets.py @@ -56,7 +56,6 @@ def run_as_post_receive_hook(): print ' -- Ref name' print refname - if set(newrev) == set(['0']): print "Deleting a reference/branch, so we won't run the "\ "pagure hook" diff --git a/pagure/lib/model.py b/pagure/lib/model.py index b9a35d0..ec24e4c 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -346,7 +346,7 @@ class Project(BASE): 'name': self.name, 'description': self.description, 'parent': self.parent.to_json(public=public, api=api) - if self.parent else None, + if self.parent else None, 'date_created': self.date_created.strftime('%s'), 'user': self.user.to_json(public=public), } @@ -749,7 +749,7 @@ class PullRequest(BASE): 'date_created': self.date_created.strftime('%s'), 'user': self.user.to_json(public=public), 'assignee': self.assignee.to_json(public=public) - if self.assignee else None, + if self.assignee else None, 'status': self.status, 'commit_start': self.commit_start, 'commit_stop': self.commit_stop, diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py index fe6e79f..8fa2c44 100644 --- a/pagure/ui/filters.py +++ b/pagure/ui/filters.py @@ -302,7 +302,6 @@ def no_js(content): return content - @APP.template_filter('toRGB') def int_to_rgb(percent): """ Template filter converting a given percentage to a css RGB value. diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 3f25ac1..8b18e00 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -596,10 +596,12 @@ def fork_project(repo, username=None): methods=('GET', 'POST')) @APP.route('//diff/..', methods=('GET', 'POST')) -@APP.route('/fork///diff/../', - methods=('GET', 'POST')) -@APP.route('/fork///diff/..', - methods=('GET', 'POST')) +@APP.route( + '/fork///diff/../', + methods=('GET', 'POST')) +@APP.route( + '/fork///diff/..', + methods=('GET', 'POST')) @cla_required def new_request_pull(repo, branch_to, branch_from, username=None): """ Request pulling the changes from the fork into the project. From b1fc61ec5e9bdbf237a402656910856c079d36cb Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 08:56:15 +0000 Subject: [PATCH 3/4] Project wide pylint clean up and bug fixes --- diff --git a/pagure/__init__.py b/pagure/__init__.py index bc256e5..c89e6e0 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -157,7 +157,7 @@ def generate_gitolite_acls(): raise pagure.exceptions.PagureException( 'Non-supported gitolite version "%s"' % gitolite_version ) - proc = subprocess.Popen( + subprocess.Popen( cmd, shell=True, stdout=subprocess.PIPE, diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index 146920c..49b4d83 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -59,7 +59,6 @@ def check_api_acls(acls, optional=False): flask.g.user = None token = None token_str = None - apt_login = None if 'Authorization' in flask.request.headers: authorization = flask.request.headers['Authorization'] if 'token' in authorization: @@ -270,8 +269,8 @@ def api_project_tags(repo, username=None): if pattern is not None and not pattern.endswith('*'): pattern += '*' - project = pagure.lib.get_project(SESSION, repo, username) - if not project: + project_obj = pagure.lib.get_project(SESSION, repo, username) + if not project_obj: output = {'output': 'notok', 'error': 'Project not found'} jsonout = flask.jsonify(output) jsonout.status_code = 404 @@ -282,7 +281,7 @@ def api_project_tags(repo, username=None): 'tags': [ tag.tag for tag in pagure.lib.get_tags_of_project( - SESSION, project, pattern=pattern) + SESSION, project_obj, pattern=pattern) ] } ) diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 61aae66..5c81001 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -15,10 +15,8 @@ from sqlalchemy.exc import SQLAlchemyError import pagure import pagure.exceptions import pagure.lib -from pagure import APP, SESSION, is_repo_admin, authenticated -from pagure.api import ( - API, api_method, api_login_required, api_login_optional, APIERROR -) +from pagure import APP, SESSION, is_repo_admin +from pagure.api import API, api_method, api_login_required, APIERROR @API.route('//pull-requests') @@ -102,7 +100,6 @@ def api_pull_request_views(repo, username=None): """ repo = pagure.lib.get_project(SESSION, repo, user=username) - output = {} if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) @@ -209,7 +206,6 @@ def api_pull_request_view(repo, requestid, username=None): """ repo = pagure.lib.get_project(SESSION, repo, user=username) - output = {} if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) diff --git a/pagure/api/issue.py b/pagure/api/issue.py index ec50716..0253e1f 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -114,6 +114,7 @@ def api_new_issue(repo, username=None): output['message'] = 'Issue created' except SQLAlchemyError, err: # pragma: no cover SESSION.rollback() + APP.logger.exception(err) raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: @@ -250,7 +251,6 @@ def api_view_issues(repo, username=None): """ repo = pagure.lib.get_project(SESSION, repo, user=username) - output = {} if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) @@ -359,7 +359,6 @@ def api_view_issue(repo, issueid, username=None): """ repo = pagure.lib.get_project(SESSION, repo, user=username) - output = {} if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) @@ -542,6 +541,7 @@ def api_comment_issue(repo, issueid, username=None): output['message'] = message except SQLAlchemyError, err: # pragma: no cover SESSION.rollback() + APP.logger.exception(err) raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) else: diff --git a/pagure/api/project.py b/pagure/api/project.py index 5a3ea11..9d03a12 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -13,10 +13,8 @@ import flask import pagure import pagure.exceptions import pagure.lib -from pagure import APP, SESSION, is_repo_admin, authenticated -from pagure.api import ( - API, api_method, api_login_required, api_login_optional, APIERROR -) +from pagure import SESSION +from pagure.api import API, api_method, APIERROR @API.route('//git/tags') @@ -45,7 +43,6 @@ def api_git_tags(repo, username=None): """ repo = pagure.lib.get_project(SESSION, repo, user=username) - output = {} if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) diff --git a/pagure/api/user.py b/pagure/api/user.py index d7d1eba..d6841be 100644 --- a/pagure/api/user.py +++ b/pagure/api/user.py @@ -13,10 +13,8 @@ import flask import pagure import pagure.exceptions import pagure.lib -from pagure import APP, SESSION, is_repo_admin, authenticated -from pagure.api import ( - API, api_method, api_login_required, api_login_optional, APIERROR -) +from pagure import APP, SESSION +from pagure.api import API, api_method, APIERROR @API.route('/user/') diff --git a/pagure/hooks/__init__.py b/pagure/hooks/__init__.py index 411565d..f004f25 100644 --- a/pagure/hooks/__init__.py +++ b/pagure/hooks/__init__.py @@ -16,24 +16,24 @@ from pagure import APP, get_repo_path class RequiredIf(wtforms.validators.Required): - """ Wtforms validator setting a field as required if another field - has a value. - """ - - def __init__(self, fields, *args, **kwargs): - if isinstance(fields, basestring): - fields = [fields] - self.fields = fields - super(RequiredIf, self).__init__(*args, **kwargs) - - def __call__(self, form, field): - for fieldname in self.fields: - nfield = form._fields.get(fieldname) - if nfield is None: - raise Exception( - 'no field named "%s" in form' % fieldname) - if bool(nfield.data): - super(RequiredIf, self).__call__(form, field) + """ Wtforms validator setting a field as required if another field + has a value. + """ + + def __init__(self, fields, *args, **kwargs): + if isinstance(fields, basestring): + fields = [fields] + self.fields = fields + super(RequiredIf, self).__init__(*args, **kwargs) + + def __call__(self, form, field): + for fieldname in self.fields: + nfield = form._fields.get(fieldname) + if nfield is None: + raise Exception( + 'no field named "%s" in form' % fieldname) + if bool(nfield.data): + super(RequiredIf, self).__call__(form, field) class BaseHook(object): diff --git a/pagure/hooks/fedmsg.py b/pagure/hooks/fedmsg.py index be6f133..75fa30e 100644 --- a/pagure/hooks/fedmsg.py +++ b/pagure/hooks/fedmsg.py @@ -9,18 +9,16 @@ """ import os -import shutil import sqlalchemy as sa -import pygit2 import wtforms from flask.ext import wtf from sqlalchemy.orm import relation from sqlalchemy.orm import backref -from pagure.hooks import BaseHook, RequiredIf +from pagure.hooks import BaseHook from pagure.lib.model import BASE, Project -from pagure import SESSION, APP, get_repo_path +from pagure import get_repo_path class FedmsgTable(BASE): diff --git a/pagure/hooks/irc.py b/pagure/hooks/irc.py index e976a01..a1f88ff 100644 --- a/pagure/hooks/irc.py +++ b/pagure/hooks/irc.py @@ -19,7 +19,7 @@ from sqlalchemy.orm import backref from pagure.hooks import BaseHook, RequiredIf from pagure.lib.model import BASE, Project -from pagure import SESSION, APP, get_repo_path +from pagure import get_repo_path class IrcTable(BASE): diff --git a/pagure/hooks/mail.py b/pagure/hooks/mail.py index 90cdfca..ff0053c 100644 --- a/pagure/hooks/mail.py +++ b/pagure/hooks/mail.py @@ -9,7 +9,6 @@ """ import os -import shutil import sqlalchemy as sa import pygit2 @@ -20,7 +19,7 @@ from sqlalchemy.orm import backref from pagure.hooks import BaseHook, RequiredIf from pagure.lib.model import BASE, Project -from pagure import SESSION, APP, get_repo_path +from pagure import get_repo_path class MailTable(BASE): diff --git a/pagure/hooks/pagure_hook.py b/pagure/hooks/pagure_hook.py index 04eb686..2f8898c 100644 --- a/pagure/hooks/pagure_hook.py +++ b/pagure/hooks/pagure_hook.py @@ -9,7 +9,6 @@ """ import os -import shutil import sqlalchemy as sa import pygit2 @@ -20,7 +19,7 @@ from sqlalchemy.orm import backref from pagure.hooks import BaseHook from pagure.lib.model import BASE, Project -from pagure import SESSION, APP, get_repo_path +from pagure import APP, get_repo_path class PagureTable(BASE): @@ -88,7 +87,7 @@ class PagureHook(BaseHook): for repopath in repopaths: # Init the git repo in case - repo_obj = pygit2.Repository(repopath) + pygit2.Repository(repopath) # Install the hook itself hook_path = os.path.join( diff --git a/pagure/hooks/pagure_request_hook.py b/pagure/hooks/pagure_request_hook.py index 83421b7..b900e39 100644 --- a/pagure/hooks/pagure_request_hook.py +++ b/pagure/hooks/pagure_request_hook.py @@ -9,7 +9,6 @@ """ import os -import shutil import flask import sqlalchemy as sa @@ -21,7 +20,7 @@ from sqlalchemy.orm import backref from pagure.hooks import BaseHook from pagure.lib.model import BASE, Project -from pagure import SESSION, APP +from pagure import APP class PagureRequestsTable(BASE): @@ -107,7 +106,7 @@ class PagureRequestHook(BaseHook): hook_files = os.path.join( os.path.dirname(os.path.realpath(__file__)), 'files') - repo_obj = pygit2.Repository(repopath) + pygit2.Repository(repopath) # Install the hook itself hook_path = os.path.join( diff --git a/pagure/hooks/pagure_ticket_hook.py b/pagure/hooks/pagure_ticket_hook.py index bfad88c..ad604ec 100644 --- a/pagure/hooks/pagure_ticket_hook.py +++ b/pagure/hooks/pagure_ticket_hook.py @@ -9,7 +9,6 @@ """ import os -import shutil import flask import sqlalchemy as sa @@ -21,7 +20,7 @@ from sqlalchemy.orm import backref from pagure.hooks import BaseHook from pagure.lib.model import BASE, Project -from pagure import SESSION, APP +from pagure import APP class PagureTicketsTable(BASE): @@ -106,7 +105,7 @@ class PagureTicketHook(BaseHook): hook_files = os.path.join( os.path.dirname(os.path.realpath(__file__)), 'files') - repo_obj = pygit2.Repository(repopath) + pygit2.Repository(repopath) # Install the hook itself hook_path = os.path.join( diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index a02b77c..658ff5d 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -287,7 +287,8 @@ def add_issue_assignee(session, issue, assignee, user, ticketfolder): return 'Issue assigned' -def add_pull_request_assignee(session, request, assignee, user, requestfolder): +def add_pull_request_assignee( + session, request, assignee, user, requestfolder): ''' Add an assignee to a request, in other words, assigned an issue. ''' __get_user(session, assignee) user_obj = __get_user(session, user) diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 156972d..e55b826 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -20,6 +20,8 @@ import tempfile import pygit2 import werkzeug +from sqlalchemy.exc import SQLAlchemyError + import pagure import pagure.exceptions import pagure.lib @@ -220,7 +222,6 @@ def clean_git(obj, repo, repofolder, objtype='ticket'): index = new_repo.index # Are we adding files - added = False if not os.path.exists(file_path): shutil.rmtree(newpath) return @@ -229,7 +230,6 @@ def clean_git(obj, repo, repofolder, objtype='ticket'): os.unlink(file_path) diff = new_repo.diff() - files = [patch.new_file_path for patch in diff] # Add the changes to the index index.remove(obj.uid) @@ -522,7 +522,7 @@ def update_request_from_git( if assignee: pagure.lib.add_pull_request_assignee( session, request, assignee.username, - user=user.user, ticketfolder=None) + user=user.user, requestfolder=None) for comment in json_data['comments']: user = get_user_from_json(session, comment) @@ -682,7 +682,9 @@ def update_file_in_git(repo, branch, filename, content, message, user): files = [patch.new_file_path for patch in diff] # Add the changes to the index + added = False for filename in files: + added = True index.add(filename) # If not change, return @@ -727,6 +729,7 @@ def update_file_in_git(repo, branch, filename, content, message, user): def read_output(cmd, abspath, input=None, keepends=False, **kw): + """ Read the output from the given command to run """ if input: stdin = subprocess.PIPE else: @@ -893,7 +896,7 @@ def merge_pull_request( session.commit() except SQLAlchemyError as err: # pragma: no cover session.rollback() - APP.logger.exception(err) + pagure.APP.logger.exception(err) shutil.rmtree(newpath) raise pagure.exceptions.PagureException( 'Could not close this pull-request') @@ -960,7 +963,7 @@ def merge_pull_request( session.commit() except SQLAlchemyError as err: # pragma: no cover session.rollback() - APP.logger.exception(err) + pagure.APP.logger.exception(err) shutil.rmtree(newpath) raise pagure.exceptions.PagureException( 'Could not update this pull-request in the database') diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index 9330e29..b3b8739 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -202,9 +202,10 @@ def send_email(text, subject, to_mail, # envelope header. msg['To'] = mailto salt = pagure.APP.config.get('SALT_EMAIL') - m = hashlib.sha512('<%s>%s%s' % (mail_id, salt, mailto)) + mhash = hashlib.sha512('<%s>%s%s' % (mail_id, salt, mailto)) msg['Reply-To'] = 'reply+%s@%s' % ( - m.hexdigest(), pagure.APP.config['DOMAIN_EMAIL_NOTIFICATIONS']) + mhash.hexdigest(), + pagure.APP.config['DOMAIN_EMAIL_NOTIFICATIONS']) try: smtp.sendmail( from_email, diff --git a/pagure/ui/app.py b/pagure/ui/app.py index c53b7ef..573e56b 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -9,10 +9,8 @@ """ import flask -import os from math import ceil -import pygit2 from sqlalchemy.exc import SQLAlchemyError import pagure.exceptions @@ -60,8 +58,6 @@ def index(): user_forks = None total_page_repos = None total_page_forks = None - repos_obj = None - forks_obj = None username = None user_repos_length = None user_forks_length = None diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 8b18e00..cb81346 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -10,8 +10,6 @@ import flask import os -import shutil -import tempfile import pygit2 from sqlalchemy.exc import SQLAlchemyError diff --git a/pagure/ui/groups.py b/pagure/ui/groups.py index c19d244..09c27c0 100644 --- a/pagure/ui/groups.py +++ b/pagure/ui/groups.py @@ -9,7 +9,6 @@ """ import flask -import os from sqlalchemy.exc import SQLAlchemyError @@ -88,11 +87,11 @@ def view_group(group): pagure.SESSION.rollback() flask.flash( 'Could not add user `%s` to group `%s`.' % ( - user.user, group.group_name), + username, group.group_name), 'error') pagure.APP.logger.debug( 'Could not add user `%s` to group `%s`.' % ( - user.user, group.group_name)) + username, group.group_name)) pagure.APP.logger.exception(err) member = False diff --git a/pagure/ui/login.py b/pagure/ui/login.py index 0167c02..dee3a07 100644 --- a/pagure/ui/login.py +++ b/pagure/ui/login.py @@ -22,7 +22,6 @@ import pagure.lib.login import pagure.lib.model as model import pagure.lib.notify from pagure import APP, SESSION -from pagure.ui.admin import admin_required # pylint: disable=E1101 From 7cc2d4f5b9b7e8f6a1d882d2695fbc51ea220894 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 05 2015 09:23:02 +0000 Subject: [PATCH 4/4] Do not send emails in the tests --- diff --git a/tests/test_progit_flask_api_fork.py b/tests/test_progit_flask_api_fork.py index 96fc3bf..4f1423d 100644 --- a/tests/test_progit_flask_api_fork.py +++ b/tests/test_progit_flask_api_fork.py @@ -44,8 +44,11 @@ class PagureFlaskApiForktests(tests.Modeltests): self.app = pagure.APP.test_client() - def test_api_pull_request_views(self): + @patch('pagure.lib.notify.send_email') + def test_api_pull_request_views(self, send_email): """ Test the api_pull_request_views method of the flask api. """ + send_email.return_value = True + tests.create_projects(self.session) tests.create_tokens(self.session) tests.create_acls(self.session) @@ -152,8 +155,11 @@ class PagureFlaskApiForktests(tests.Modeltests): data2['requests'][0]['uid'] = '1431414800' self.assertDictEqual(data, data2) - def test_api_pull_request_view(self): + @patch('pagure.lib.notify.send_email') + def test_api_pull_request_view(self, send_email): """ Test the api_pull_request_view method of the flask api. """ + send_email.return_value = True + tests.create_projects(self.session) tests.create_tokens(self.session) tests.create_acls(self.session) @@ -264,8 +270,11 @@ class PagureFlaskApiForktests(tests.Modeltests): data2['date_created'] = '1431414800' self.assertDictEqual(data, data2) - def test_api_pull_request_close(self): + @patch('pagure.lib.notify.send_email') + def test_api_pull_request_close(self, send_email): """ Test the api_pull_request_close method of the flask api. """ + send_email.return_value = True + tests.create_projects(self.session) tests.create_tokens(self.session) tests.create_acls(self.session) @@ -372,10 +381,12 @@ class PagureFlaskApiForktests(tests.Modeltests): {"message": "Pull-request closed!"} ) + @patch('pagure.lib.notify.send_email') @patch('pagure.lib.git.merge_pull_request') - def test_api_pull_request_merge(self, mpr): + def test_api_pull_request_merge(self, mpr, send_email): """ Test the api_pull_request_merge method of the flask api. """ mpr.return_value = 'Changes merged!' + send_email.return_value = True tests.create_projects(self.session) tests.create_tokens(self.session)