This is a rather large change which might have consequences in several places. From my tests it works and hopefully I did not miss to many places. ---
Finally I prefer send the patch up for review.
I managed to get the frontend running pretty quickly, that was very nice ! :) However, I did not manage to get the tests working easily (I know I could have dig into the config files to find there the path are hardcoded and adjust them, but I thought that not being able to run the tests while developing was more a bug and thus my hack around was not the correct way to approach this).
Thus, I tested my changes against the UI (thus a rather limited test) but I did not run the unit-tests.
By the way, do you know we have a jenkins instance running in the infrastructure, would you be interested in hooking up copr's git in it?
The second patch contains changes I made to the unit-tests to adjust the URLs but as I said, I did not run the tests and therefore there might be tests that are now failing.
If you are fine with these changes, I will submit patches for the API and adjust the CLI.
Looking forward for your feedbacks,
Cheers, Pierre
.../coprs/templates/coprs/_coprs_forms.html | 10 ++++- coprs_frontend/coprs/templates/coprs/add.html | 2 +- .../coprs/templates/coprs/detail/overview.html | 7 +++- coprs_frontend/coprs/templates/coprs/show.html | 6 ++- .../coprs/views/backend_ns/backend_general.py | 4 ++ .../coprs/views/coprs_ns/coprs_builds.py | 16 +++++--- .../coprs/views/coprs_ns/coprs_general.py | 48 ++++++++++++++-------- coprs_frontend/coprs/views/misc.py | 2 + 8 files changed, 65 insertions(+), 30 deletions(-)
diff --git a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html index dd13b40..a43d307 100644 --- a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html +++ b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html @@ -1,8 +1,14 @@ {% from "_helpers.html" import render_field %}
-{% macro copr_form(form, view, copr = None) %} +{% macro copr_form(form, view, copr = None, username = None) %} {# if using for updating, we need to pass name to url_for, but otherwise we need to pass nothing #} - <form action="{% if copr %}{{ url_for(view, username = copr.owner.name, coprname = copr.name) }}{% else %}{{ url_for(view) }}{% endif %}" method=post class=add-entry> + <form action="{% if copr %} + {{ url_for(view, username = copr.owner.name, coprname = copr.name) }} + {% elif username %} + {{ url_for(view, username = username) }} + {% else %} + {{ url_for(view) }} + {% endif %}" method=post class=add-entry> <dl> {{ form.csrf_token }} {{ render_field(form.id, hidden = True) }} diff --git a/coprs_frontend/coprs/templates/coprs/add.html b/coprs_frontend/coprs/templates/coprs/add.html index d8a351b..9b97814 100644 --- a/coprs_frontend/coprs/templates/coprs/add.html +++ b/coprs_frontend/coprs/templates/coprs/add.html @@ -6,6 +6,6 @@ {% block body %}
<h1>Add a new Copr</h1> - {{ copr_form(form, view = 'coprs_ns.copr_new') }} + {{ copr_form(form, view = 'coprs_ns.copr_new', username=g.user.name) }}
{% endblock %} diff --git a/coprs_frontend/coprs/templates/coprs/detail/overview.html b/coprs_frontend/coprs/templates/coprs/detail/overview.html index d924378..fd09f71 100644 --- a/coprs_frontend/coprs/templates/coprs/detail/overview.html +++ b/coprs_frontend/coprs/templates/coprs/detail/overview.html @@ -11,7 +11,10 @@ <div class="shift-right">{{ copr.instructions_or_not_filled}}</div> <h2>Yum repo</h2> <div class="shift-right"> - <a href="{{ url_for('coprs_ns.generate_repo_file', reponame=copr.owner.name+'-'+copr.name) }}"> + <a href="{{ url_for( + 'coprs_ns.generate_repo_file', + username=copr.owner.name, + coprname=copr.name) }}"> {{ copr.owner.name }}-{{ copr.name }}.repo</a></div> <h2>Active Releases</h2> <table class="releases"> @@ -51,7 +54,7 @@ {% for repo in copr.repos_list %} <li><a href="{{ repo }}">{{ repo }}</a></li> {% endfor %} - </ul> + </ul> {% endif %}
<hr> diff --git a/coprs_frontend/coprs/templates/coprs/show.html b/coprs_frontend/coprs/templates/coprs/show.html index 3df2771..e766e48 100644 --- a/coprs_frontend/coprs/templates/coprs/show.html +++ b/coprs_frontend/coprs/templates/coprs/show.html @@ -15,7 +15,11 @@ {% endif %} <div class="coprs-list-{% if g.user %}thin{% else %}thick{% endif %}"> {% if g.user %} - <div class="add-copr">+ <a href={{ url_for('coprs_ns.copr_add') }}>add a new Copr</a></div> + <div class="add-copr">+ + <a href={{ url_for('coprs_ns.copr_add', username=g.user.name) }}> + add a new Copr + </a> + </div> {% endif %} {% if fulltext %} <div class="search-results">Displaying results for search "{{ fulltext }}"</div> diff --git a/coprs_frontend/coprs/views/backend_ns/backend_general.py b/coprs_frontend/coprs/views/backend_ns/backend_general.py index dfdc97d..7d46e2e 100644 --- a/coprs_frontend/coprs/views/backend_ns/backend_general.py +++ b/coprs_frontend/coprs/views/backend_ns/backend_general.py @@ -9,6 +9,7 @@ from coprs.logic import builds_logic from coprs.views import misc from coprs.views.backend_ns import backend_ns
+ @backend_ns.route('/waiting_builds/') def waiting_builds(): query = builds_logic.BuildsLogic.get_waiting_builds(None) @@ -19,6 +20,7 @@ def waiting_builds(): '__included_ids__': False}, '__included_ids__': False}) for build in builds]})
+ @backend_ns.route('/update_builds/', methods = ['POST', 'PUT']) @misc.backend_authenticated def update_builds(): @@ -42,12 +44,14 @@ def update_builds():
return flask.jsonify({'updated_builds_ids': list(existing.keys()), 'non_existing_builds_ids': non_existing_ids})
+ @backend_ns.route('/waiting_actions/') def waiting_actions(): actions = models.Action.query.filter(models.Action.result==helpers.BackendResultEnum('waiting')).all()
return flask.jsonify({'actions': [action.to_dict(options={'__columns_except__': ['result', 'message', 'ended_on']}) for action in actions]})
+ # TODO: this is very similar to update_builds, we should pull out the common functionality into a single function @backend_ns.route('/update_actions/', methods=['POST', 'PUT']) @misc.backend_authenticated diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py index a6871cd..c8cd0d7 100644 --- a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py +++ b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py @@ -15,8 +15,9 @@ from coprs.views.misc import login_required, page_not_found from coprs.views.coprs_ns import coprs_ns from coprs.views.coprs_ns import coprs_general
-@coprs_ns.route('/detail/<username>/<coprname>/builds/', defaults={'page': 1}) -@coprs_ns.route('/detail/<username>/<coprname>/builds/int:page/') + +@coprs_ns.route('/<username>/<coprname>/builds/', defaults={'page': 1}) +@coprs_ns.route('/<username>/<coprname>/builds/int:page/') def copr_builds(username, coprname, page=1): copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
@@ -29,7 +30,7 @@ def copr_builds(username, coprname, page=1): return flask.render_template('coprs/detail/builds.html', copr=copr, builds=paginator.sliced_query, paginator=paginator)
-@coprs_ns.route('/detail/<username>/<coprname>/add_build/') +@coprs_ns.route('/<username>/<coprname>/add_build/') @login_required def copr_add_build(username, coprname, form=None): copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first() @@ -42,7 +43,8 @@ def copr_add_build(username, coprname, form=None):
return flask.render_template('coprs/detail/add_build.html', copr=copr, form=form)
-@coprs_ns.route('/detail/<username>/<coprname>/new_build/', methods = ["POST"]) + +@coprs_ns.route('/<username>/<coprname>/new_build/', methods = ["POST"]) @login_required def copr_new_build(username, coprname): form = forms.BuildForm() @@ -69,7 +71,8 @@ def copr_new_build(username, coprname): else: return copr_add_build(username=username, coprname=coprname, form=form)
-@coprs_ns.route('/detail/<username>/<coprname>/cancel_build/int:build_id/', methods = ['POST']) + +@coprs_ns.route('/<username>/<coprname>/cancel_build/int:build_id/', methods = ['POST']) @login_required def copr_cancel_build(username, coprname, build_id): # only the user who ran the build can cancel it @@ -86,7 +89,8 @@ def copr_cancel_build(username, coprname, build_id):
return flask.redirect(flask.url_for('coprs_ns.copr_builds', username = username, coprname = coprname))
-@coprs_ns.route('/detail/<username>/<coprname>/repeat_build/int:build_id/', methods = ['POST']) + +@coprs_ns.route('/<username>/<coprname>/repeat_build/int:build_id/', methods = ['POST']) @login_required def copr_repeat_build(username, coprname, build_id): build = builds_logic.BuildsLogic.get(flask.g.user, build_id).first() diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py index 34f62a1..143ee6c 100644 --- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py +++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py @@ -16,6 +16,7 @@ from coprs.views.coprs_ns import coprs_ns from coprs.logic import builds_logic from coprs.logic import coprs_logic
+ @coprs_ns.route('/', defaults = {'page': 1}) @coprs_ns.route('/int:page/') def coprs_show(page=1): @@ -26,8 +27,8 @@ def coprs_show(page=1): return flask.render_template('coprs/show.html', coprs=coprs, paginator=paginator)
-@coprs_ns.route('/owned/<username>/', defaults = {'page': 1}) -@coprs_ns.route('/owned/<username>/int:page/') +@coprs_ns.route('/<username>/', defaults = {'page': 1}) +@coprs_ns.route('/<username>/int:page/') def coprs_by_owner(username=None, page=1): query = coprs_logic.CoprsLogic.get_multiple(flask.g.user, user_relation='owned', @@ -39,8 +40,8 @@ def coprs_by_owner(username=None, page=1): return flask.render_template('coprs/show.html', coprs=coprs, paginator=paginator)
-@coprs_ns.route('/allowed/<username>/', defaults = {'page': 1}) -@coprs_ns.route('/allowed/<username>/int:page/') +@coprs_ns.route('/<username>/allowed/', defaults = {'page': 1}) +@coprs_ns.route('/<username>/allowed/int:page/') def coprs_by_allowed(username=None, page=1): query = coprs_logic.CoprsLogic.get_multiple(flask.g.user, user_relation='allowed', @@ -51,6 +52,7 @@ def coprs_by_allowed(username=None, page=1): coprs = paginator.sliced_query return flask.render_template('coprs/show.html', coprs=coprs, paginator=paginator)
+ @coprs_ns.route('/fulltext/', defaults = {'page': 1}) @coprs_ns.route('/fulltext/int:page/') def coprs_fulltext_search(page=1): @@ -64,17 +66,18 @@ def coprs_fulltext_search(page=1): paginator=paginator, fulltext=fulltext)
-@coprs_ns.route('/add/') + +@coprs_ns.route('/<username>/add/') @login_required -def copr_add(): +def copr_add(username): form = forms.CoprFormFactory.create_form_cls()()
return flask.render_template('coprs/add.html', form = form)
-@coprs_ns.route('/new/', methods=['POST']) +@coprs_ns.route('/<username>/new/', methods=['POST']) @login_required -def copr_new(): +def copr_new(username): """ Receive information from the user on how to create its new copr and create it accordingly. """ @@ -101,7 +104,8 @@ def copr_new(): else: return flask.render_template('coprs/add.html', form = form)
-@coprs_ns.route('/detail/<username>/<coprname>/') + +@coprs_ns.route('/<username>/<coprname>/') def copr_detail(username, coprname): query = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname, with_mock_chroots=True) form = forms.CoprLegalFlagForm() @@ -114,7 +118,8 @@ def copr_detail(username, coprname): copr=copr, form=form)
-@coprs_ns.route('/detail/<username>/<coprname>/permissions/') + +@coprs_ns.route('/<username>/<coprname>/permissions/') def copr_permissions(username, coprname): query = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname) copr = query.first() @@ -145,7 +150,8 @@ def copr_permissions(username, coprname): permissions = permissions, current_user_permissions = user_perm)
-@coprs_ns.route('/detail/<username>/<coprname>/edit/') + +@coprs_ns.route('/<username>/<coprname>/edit/') @login_required def copr_edit(username, coprname, form=None): query = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname) @@ -161,7 +167,7 @@ def copr_edit(username, coprname, form=None): form=form)
-@coprs_ns.route('/detail/<username>/<coprname>/update/', methods = ['POST']) +@coprs_ns.route('/<username>/<coprname>/update/', methods = ['POST']) @login_required def copr_update(username, coprname): form = forms.CoprFormFactory.create_form_cls()() @@ -189,7 +195,7 @@ def copr_update(username, coprname): return copr_edit(username, coprname, form)
-@coprs_ns.route('/detail/<username>/<coprname>/permissions_applier_change/', methods = ['POST']) +@coprs_ns.route('/<username>/<coprname>/permissions_applier_change/', methods = ['POST']) @login_required def copr_permissions_applier_change(username, coprname): copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first() @@ -210,7 +216,8 @@ def copr_permissions_applier_change(username, coprname):
return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name))
-@coprs_ns.route('/detail/<username>/<coprname>/update_permissions/', methods = ['POST']) + +@coprs_ns.route('/<username>/<coprname>/update_permissions/', methods = ['POST']) @login_required def copr_update_permissions(username, coprname): query = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname) @@ -238,7 +245,8 @@ def copr_update_permissions(username, coprname):
return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name))
-@coprs_ns.route('/detail/<username>/<coprname>/delete/', methods=['GET', 'POST']) + +@coprs_ns.route('/<username>/<coprname>/delete/', methods=['GET', 'POST']) @login_required def copr_delete(username, coprname): form = forms.CoprDeleteForm() @@ -258,7 +266,8 @@ def copr_delete(username, coprname): else: return flask.render_template('coprs/detail/delete.html', form=form, copr=copr)
-@coprs_ns.route('/detail/<username>/<coprname>/legal_flag/', methods=['POST']) + +@coprs_ns.route('/<username>/<coprname>/legal_flag/', methods=['POST']) @login_required def copr_legal_flag(username, coprname): form = forms.CoprLegalFlagForm() @@ -273,14 +282,17 @@ def copr_legal_flag(username, coprname): flask.flash('Admin was noticed about your report and will investigate the copr shortly.') return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname))
-@coprs_ns.route('/repo/<reponame>.repo') -def generate_repo_file(reponame): + +@coprs_ns.route('/<username>/<coprname>/repo/') +def generate_repo_file(username, coprname): ''' Generate repo file for a given repo name. Reponame = username-coprname ''' # This solution is used because flask splits off the last part after a # dash, therefore user-re-po resolves to user-re/po instead of user/re-po # FAS usernames may not contain dashes, so this construction is safe.
+ reponame = "%s-%s" % (username, coprname) + if '-' not in reponame: return page_not_found('Bad repository name: {0}. Must be username-coprname'.format(reponame))
diff --git a/coprs_frontend/coprs/views/misc.py b/coprs_frontend/coprs/views/misc.py index ea79640..9863564 100644 --- a/coprs_frontend/coprs/views/misc.py +++ b/coprs_frontend/coprs/views/misc.py @@ -79,6 +79,7 @@ def logout(): flask.flash(u'You were signed out') return flask.redirect(oid.get_next_url())
+ def api_login_required(f): @functools.wraps(f) def decorated_function(*args, **kwargs): @@ -129,6 +130,7 @@ def login_required(role=helpers.RoleEnum('user')): else: return view_wrapper
+ # backend authentication def backend_authenticated(f): @functools.wraps(f)
--- .../test_views/test_coprs_ns/test_coprs_general.py | 80 +++++++++++----------- 1 file changed, 40 insertions(+), 40 deletions(-)
diff --git a/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py index 00551dd..ca3d25f 100644 --- a/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py +++ b/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py @@ -19,13 +19,13 @@ class TestCoprsOwned(CoprsTestCase): @TransactionDecorator('u3') def test_owned_none(self, f_users, f_coprs, f_db): self.db.session.add(self.u3) - r = self.test_client.get('/coprs/owned/{0}/'.format(self.u3.name)) + r = self.test_client.get('/coprs/{0}/'.format(self.u3.name)) assert 'No coprs...' in r.data
@TransactionDecorator('u1') def test_owned_one(self, f_users, f_coprs, f_db): self.db.session.add(self.u1) - r = self.test_client.get('/coprs/owned/{0}/'.format(self.u1.name)) + r = self.test_client.get('/coprs/{0}/'.format(self.u1.name)) assert r.data.count('<div class="copr">') == 1
class TestCoprsAllowed(CoprsTestCase): @@ -33,19 +33,19 @@ class TestCoprsAllowed(CoprsTestCase): @TransactionDecorator('u3') def test_allowed_none(self, f_users, f_coprs, f_copr_permissions, f_db): self.db.session.add(self.u3) - r = self.test_client.get('/coprs/allowed/{0}/'.format(self.u3.name)) + r = self.test_client.get('/coprs/{0}/allowed/'.format(self.u3.name)) assert 'No coprs...' in r.data
@TransactionDecorator('u2') def test_allowed_one(self, f_users, f_coprs, f_copr_permissions, f_db): self.db.session.add(self.u1) - r = self.test_client.get('/coprs/allowed/{0}/'.format(self.u1.name)) + r = self.test_client.get('/coprs/{0}/allowed/'.format(self.u2.name)) assert r.data.count('<div class="copr">') == 1
@TransactionDecorator('u1') def test_allowed_one_but_asked_for_one_more(self, f_users, f_coprs, f_copr_permissions, f_db): self.db.session.add(self.u1) - r = self.test_client.get('/coprs/allowed/{0}/'.format(self.u1.name)) + r = self.test_client.get('/coprs/{0}/allowed/'.format(self.u1.name)) assert r.data.count('<div class="copr">') == 1
class TestCoprNew(CoprsTestCase): @@ -53,7 +53,7 @@ class TestCoprNew(CoprsTestCase):
@TransactionDecorator('u1') def test_copr_new_normal(self, f_users, f_mock_chroots, f_db): - r = self.test_client.post('/coprs/new/', data = {'name': 'foo', 'fedora-rawhide-i386': 'y', 'arches': ['i386']}, follow_redirects = True) + r = self.test_client.post('/coprs/{0}/new/'.format(self.u1.name), data = {'name': 'foo', 'fedora-rawhide-i386': 'y', 'arches': ['i386']}, follow_redirects = True) assert self.models.Copr.query.filter(self.models.Copr.name == 'foo').first() assert self.success_string in r.data
@@ -67,7 +67,7 @@ class TestCoprNew(CoprsTestCase): def test_receiver(sender, **kwargs): signals_received.append(kwargs['copr']) copr_created.connect(test_receiver) - r = self.test_client.post('/coprs/new/', data={'name': 'foo', 'fedora-rawhide-i386': 'y', 'arches': ['i386']}, follow_redirects=True) + r = self.test_client.post('/coprs/{0}/new/'.format(self.u1.name)', data={'name': 'foo', 'fedora-rawhide-i386': 'y', 'arches': ['i386']}, follow_redirects=True) assert len(signals_received) == 1 assert signals_received[0].name == 'foo'
@@ -78,7 +78,7 @@ class TestCoprNew(CoprsTestCase): foocoprs = len(self.models.Copr.query.filter(self.models.Copr.name == self.c1.name).all()) assert foocoprs > 0
- r = self.test_client.post('/coprs/new/', data = {'name': self.c1.name, 'fedora-rawhide-i386': 'y'}, follow_redirects = True) + r = self.test_client.post('/coprs/{0}/new/'.format(self.u3.name), data = {'name': self.c1.name, 'fedora-rawhide-i386': 'y'}, follow_redirects = True) self.db.session.add(self.c1) assert len(self.models.Copr.query.filter(self.models.Copr.name == self.c1.name).all()) == foocoprs + 1 assert self.success_string in r.data @@ -89,14 +89,14 @@ class TestCoprNew(CoprsTestCase): foocoprs = len(self.models.Copr.query.filter(self.models.Copr.name == self.c1.name).all()) assert foocoprs > 0
- r = self.test_client.post('/coprs/new/', data = {'name': self.c1.name, 'fedora-rawhide-i386': 'y'}, follow_redirects = True) + r = self.test_client.post('/coprs/{0}/new/'.format(self.u1.name), data = {'name': self.c1.name, 'fedora-rawhide-i386': 'y'}, follow_redirects = True) self.db.session.add(self.c1) assert len(self.models.Copr.query.filter(self.models.Copr.name == self.c1.name).all()) == foocoprs assert "You already have copr named" in r.data
@TransactionDecorator('u1') def test_copr_new_with_initial_pkgs(self, f_users, f_mock_chroots, f_db): - r = self.test_client.post('/coprs/new/', data = {'name': 'foo', 'fedora-rawhide-i386': 'y', 'initial_pkgs': ['http://f', 'http://b%27%5D%7D, follow_redirects = True) + r = self.test_client.post('/coprs/{0}/new/'.format(self.u1.name), data = {'name': 'foo', 'fedora-rawhide-i386': 'y', 'initial_pkgs': ['http://f', 'http://b%27%5D%7D, follow_redirects = True) copr = self.models.Copr.query.filter(self.models.Copr.name == 'foo').first() assert copr assert self.success_string in r.data @@ -113,7 +113,7 @@ class TestCoprNew(CoprsTestCase): self.db.session.commit()
self.db.session.add(self.c1) - r = self.test_client.post('/coprs/new/', data = {'name': self.c1.name, 'fedora-rawhide-i386': 'y', 'arches': ['i386']}, follow_redirects = True) + r = self.test_client.post('/coprs/{0}/new/'.format(self.u1.name), data = {'name': self.c1.name, 'fedora-rawhide-i386': 'y', 'arches': ['i386']}, follow_redirects = True) self.db.session.add_all([self.c1, self.u1]) assert len(self.models.Copr.query.filter(self.models.Copr.name==self.c1.name).\ filter(self.models.Copr.owner==self.u1).\ @@ -123,24 +123,24 @@ class TestCoprNew(CoprsTestCase):
class TestCoprDetail(CoprsTestCase): def test_copr_detail_not_found(self): - r = self.tc.get('/coprs/detail/foo/bar/') + r = self.tc.get('/coprs/foo/bar/') assert r.status_code == 404
def test_copr_detail_normal(self, f_users, f_coprs, f_db): - r = self.tc.get('/coprs/detail/{0}/{1}/'.format(self.u1.name, self.c1.name)) + r = self.tc.get('/coprs/{0}/{1}/'.format(self.u1.name, self.c1.name)) assert r.status_code == 200 assert self.c1.name in r.data
def test_copr_detail_contains_builds(self, f_users, f_coprs, f_builds, f_db): - r = self.tc.get('/coprs/detail/{0}/{1}/builds/'.format(self.u1.name, self.c1.name)) + r = self.tc.get('/coprs/{0}/{1}/builds/'.format(self.u1.name, self.c1.name)) assert r.data.count('<tr class="build') == 2
def test_copr_detail_anonymous_doesnt_contain_permissions_table_when_no_permissions(self, f_users, f_coprs, f_copr_permissions, f_db): - r = self.tc.get('/coprs/detail/{0}/{1}/permissions/'.format(self.u1.name, self.c1.name)) + r = self.tc.get('/coprs/{0}/{1}/permissions/'.format(self.u1.name, self.c1.name)) assert '<table class="permissions"' not in r.data
def test_copr_detail_contains_permissions_table(self, f_users, f_coprs, f_copr_permissions, f_db): - r = self.tc.get('/coprs/detail/{0}/{1}/permissions/'.format(self.u2.name, self.c3.name)) + r = self.tc.get('/coprs/{0}/{1}/permissions/'.format(self.u2.name, self.c3.name)) assert '<table class="permissions-table"' in r.data assert '<td>{0}'.format(self.u3.name) in r.data assert '<td>{0}'.format(self.u1.name) in r.data @@ -148,39 +148,39 @@ class TestCoprDetail(CoprsTestCase): @TransactionDecorator('u1') def test_copr_detail_allows_asking_for_permissions(self, f_users, f_coprs, f_copr_permissions, f_db): self.db.session.add_all([self.u2, self.c2]) - r = self.test_client.get('/coprs/detail/{0}/{1}/permissions/'.format(self.u2.name, self.c2.name)) + r = self.test_client.get('/coprs/{0}/{1}/permissions/'.format(self.u2.name, self.c2.name)) # u1 is approved builder, check for that assert '/permissions_applier_change/' in r.data
@TransactionDecorator('u2') def test_copr_detail_doesnt_allow_owner_to_ask_for_permissions(self, f_users, f_coprs, f_db): self.db.session.add_all([self.u2, self.c2]) - r = self.test_client.get('/coprs/detail/{0}/{1}/permissions/'.format(self.u2.name, self.c2.name)) + r = self.test_client.get('/coprs/{0}/{1}/permissions/'.format(self.u2.name, self.c2.name)) assert '/permissions_applier_change/' not in r.data
@TransactionDecorator('u2') def test_detail_has_correct_permissions_form(self, f_users, f_coprs, f_copr_permissions, f_db): self.db.session.add_all([self.u2, self.c3]) - r = self.test_client.get('/coprs/detail/{0}/{1}/permissions/'.format(self.u2.name, self.c3.name)) + r = self.test_client.get('/coprs/{0}/{1}/permissions/'.format(self.u2.name, self.c3.name))
assert r.data.count('nothing') == 2 assert '<select id="copr_builder_1" name="copr_builder_1">' in r.data assert '<select id="copr_admin_1" name="copr_admin_1">' in r.data
def test_copr_detail_doesnt_show_cancel_build_for_anonymous(self, f_users, f_coprs, f_builds, f_db): - r = self.tc.get('/coprs/detail/{0}/{1}/'.format(self.u2.name, self.c2.name)) + r = self.tc.get('/coprs/{0}/{1}/'.format(self.u2.name, self.c2.name)) assert '/cancel_build/' not in r.data
@TransactionDecorator('u1') def test_copr_detail_doesnt_allow_non_submitter_to_cancel_build(self, f_users, f_coprs, f_builds, f_db): self.db.session.add_all([self.u2, self.c2]) - r = self.test_client.get('/coprs/detail/{0}/{1}/builds/'.format(self.u2.name, self.c2.name)) + r = self.test_client.get('/coprs/{0}/{1}/builds/'.format(self.u2.name, self.c2.name)) assert '/cancel_build/' not in r.data
@TransactionDecorator('u2') def test_copr_detail_allows_submitter_to_cancel_build(self, f_users, f_coprs, f_builds, f_db): self.db.session.add_all([self.u2, self.c2]) - r = self.test_client.get('/coprs/detail/{0}/{1}/builds/'.format(self.u2.name, self.c2.name)) + r = self.test_client.get('/coprs/{0}/{1}/builds/'.format(self.u2.name, self.c2.name)) assert '/cancel_build/' in r.data
@@ -189,7 +189,7 @@ class TestCoprEdit(CoprsTestCase): @TransactionDecorator('u1') def test_edit_prefills_id(self, f_users, f_coprs, f_db): self.db.session.add_all([self.u1, self.c1]) - r = self.test_client.get('/coprs/detail/{0}/{1}/edit/'.format(self.u1.name, self.c1.name)) + r = self.test_client.get('/coprs/{0}/{1}/edit/'.format(self.u1.name, self.c1.name)) # TODO: use some kind of html parsing library to look for the hidden input, this ties us # to the precise format of the tag assert '<input hidden id="id" name="id" type="hidden" value="{0}">'.format(self.c1.id) in r.data @@ -200,7 +200,7 @@ class TestCoprUpdate(CoprsTestCase): @TransactionDecorator('u1') def test_update_no_changes(self, f_users, f_coprs, f_mock_chroots, f_db): self.db.session.add_all([self.u1, self.c1]) - r = self.test_client.post('/coprs/detail/{0}/{1}/update/'.format(self.u1.name, self.c1.name), + r = self.test_client.post('/coprs/{0}/{1}/update/'.format(self.u1.name, self.c1.name), data = {'name': self.c1.name, 'fedora-18-x86_64': 'y', 'id': self.c1.id}, follow_redirects = True) assert 'Copr was updated successfully' in r.data @@ -208,7 +208,7 @@ class TestCoprUpdate(CoprsTestCase): @TransactionDecorator('u1') def test_copr_admin_can_update(self, f_users, f_coprs, f_copr_permissions, f_mock_chroots, f_db): self.db.session.add_all([self.u2, self.c3]) - r = self.test_client.post('/coprs/detail/{0}/{1}/update/'.format(self.u2.name, self.c3.name), + r = self.test_client.post('/coprs/{0}/{1}/update/'.format(self.u2.name, self.c3.name), data = {'name': self.c3.name, 'fedora-rawhide-i386': 'y', 'id': self.c3.id}, follow_redirects = True) assert 'Copr was updated successfully' in r.data @@ -216,7 +216,7 @@ class TestCoprUpdate(CoprsTestCase): @TransactionDecorator('u1') def test_update_multiple_chroots(self, f_users, f_coprs, f_copr_permissions, f_mock_chroots, f_db): self.db.session.add_all([self.u1, self.c1, self.mc1, self.mc2, self.mc3]) - r = self.test_client.post('/coprs/detail/{0}/{1}/update/'.format(self.u1.name, self.c1.name), + r = self.test_client.post('/coprs/{0}/{1}/update/'.format(self.u1.name, self.c1.name), data = {'name': self.c1.name, self.mc2.chroot_name: 'y', self.mc3.chroot_name: 'y', 'id': self.c1.id}, follow_redirects = True) assert 'Copr was updated successfully' in r.data @@ -238,7 +238,7 @@ class TestCoprUpdate(CoprsTestCase): cc.mock_chroot = self.mc1 self.c2.copr_chroots.append(cc)
- r = self.test_client.post('/coprs/detail/{0}/{1}/update/'.format(self.u2.name, self.c2.name), + r = self.test_client.post('/coprs/{0}/{1}/update/'.format(self.u2.name, self.c2.name), data = {'name': self.c2.name, self.mc1.chroot_name: 'y', 'id': self.c2.id}, follow_redirects = True) assert 'Copr was updated successfully' in r.data @@ -253,7 +253,7 @@ class TestCoprApplyForPermissions(CoprsTestCase): @TransactionDecorator('u2') def test_apply(self, f_users, f_coprs, f_db): self.db.session.add_all([self.u1, self.u2, self.c1]) - r = self.test_client.post('/coprs/detail/{0}/{1}/permissions_applier_change/'.format(self.u1.name, self.c1.name), + r = self.test_client.post('/coprs/{0}/{1}/permissions_applier_change/'.format(self.u1.name, self.c1.name), data = {'copr_builder': '1'}, follow_redirects = True) assert 'Successfuly updated' in r.data @@ -269,7 +269,7 @@ class TestCoprApplyForPermissions(CoprsTestCase): @TransactionDecorator('u1') def test_apply_doesnt_lower_other_values_from_admin_to_request(self, f_users, f_coprs, f_copr_permissions, f_db): self.db.session.add_all([self.u1, self.u2, self.cp1, self.c2]) - r = self.test_client.post('/coprs/detail/{0}/{1}/permissions_applier_change/'.format(self.u2.name, self.c2.name), + r = self.test_client.post('/coprs/{0}/{1}/permissions_applier_change/'.format(self.u2.name, self.c2.name), data = {'copr_builder': 1, 'copr_admin': '1'}, follow_redirects = True) assert 'Successfuly updated' in r.data @@ -286,7 +286,7 @@ class TestCoprUpdatePermissions(CoprsTestCase): @TransactionDecorator('u2') def test_cancel_permission(self, f_users, f_coprs, f_copr_permissions, f_db): self.db.session.add_all([self.u2, self.c2]) - r = self.test_client.post('/coprs/detail/{0}/{1}/update_permissions/'.format(self.u2.name, self.c2.name), + r = self.test_client.post('/coprs/{0}/{1}/update_permissions/'.format(self.u2.name, self.c2.name), data = {'copr_builder_1': '0'}, follow_redirects = True)
@@ -298,7 +298,7 @@ class TestCoprUpdatePermissions(CoprsTestCase): @TransactionDecorator('u2') def test_update_more_permissions(self, f_users, f_coprs, f_copr_permissions, f_db): self.db.session.add_all([self.u2, self.c3]) - r = self.test_client.post('/coprs/detail/{0}/{1}/update_permissions/'.format(self.u2.name, self.c3.name), + r = self.test_client.post('/coprs/{0}/{1}/update_permissions/'.format(self.u2.name, self.c3.name), data = {'copr_builder_1': '2', 'copr_admin_1': '1', 'copr_admin_3': '2'}, follow_redirects = True) self.db.session.add_all([self.c3, self.u1, self.u3]) @@ -317,7 +317,7 @@ class TestCoprUpdatePermissions(CoprsTestCase): @TransactionDecorator('u1') def test_copr_admin_can_update_permissions(self, f_users, f_coprs, f_copr_permissions, f_db): self.db.session.add_all([self.u2, self.c3]) - r = self.test_client.post('/coprs/detail/{0}/{1}/update_permissions/'.format(self.u2.name, self.c3.name), + r = self.test_client.post('/coprs/{0}/{1}/update_permissions/'.format(self.u2.name, self.c3.name), data = {'copr_builder_1': '2', 'copr_admin_3': '2'}, follow_redirects = True) assert 'Copr permissions were updated' in r.data @@ -332,7 +332,7 @@ class TestCoprUpdatePermissions(CoprsTestCase): # the admin is the first one and therefore this fails if # the view doesn't reorder the permissions flexmock(self.models.Copr, copr_permissions=[self.cp3, self.cp2]) - r = self.test_client.post('/coprs/detail/{0}/{1}/update_permissions/'.format(self.u2.name, self.c3.name), + r = self.test_client.post('/coprs/{0}/{1}/update_permissions/'.format(self.u2.name, self.c3.name), data = {'copr_admin_1': '1', 'copr_admin_3': '1'}, follow_redirects = True) self.db.session.add_all([self.u1, self.c3]) @@ -348,7 +348,7 @@ class TestCoprDelete(CoprsTestCase): @TransactionDecorator('u1') def test_delete(self, f_users, f_coprs, f_db): self.db.session.add_all([self.u1, self.c1]) - r = self.test_client.post('/coprs/detail/{0}/{1}/delete/'.format(self.u1.name, self.c1.name), + r = self.test_client.post('/coprs/{0}/{1}/delete/'.format(self.u1.name, self.c1.name), data = {'verify': 'yes'}, follow_redirects = True) assert 'Copr was deleted successfully' in r.data @@ -359,7 +359,7 @@ class TestCoprDelete(CoprsTestCase): @TransactionDecorator('u1') def test_copr_delete_does_not_delete_if_verify_filled_wrongly(self, f_users, f_coprs, f_db): self.db.session.add_all([self.u1, self.c1]) - r = self.test_client.post('/coprs/detail/{0}/{1}/delete/'.format(self.u1.name, self.c1.name), + r = self.test_client.post('/coprs/{0}/{1}/delete/'.format(self.u1.name, self.c1.name), data = {'verify': 'no'}, follow_redirects = True) assert 'Copr was deleted successfully' not in r.data @@ -369,7 +369,7 @@ class TestCoprDelete(CoprsTestCase): @TransactionDecorator('u2') def test_non_owner_cant_delete(self, f_users, f_coprs, f_db): self.db.session.add_all([self.u1, self.u2, self.c1]) - r = self.test_client.post('/coprs/detail/{0}/{1}/delete/'.format(self.u1.name, self.c1.name), + r = self.test_client.post('/coprs/{0}/{1}/delete/'.format(self.u1.name, self.c1.name), data = {'verify': 'yes'}, follow_redirects = True) self.db.session.add_all([self.c1]) @@ -388,22 +388,22 @@ class TestCoprRepoGeneration(CoprsTestCase): self.db.session.add_all([self.b5, self.b6, self.b7])
def test_fail_on_missing_dash(self): - r = self.tc.get('/coprs/repo/reponamewithoutdash.repo') + r = self.tc.get('/coprs/reponamewithoutdash/repo/') assert r.status_code == 404 assert 'Bad repository name' in r.data
def test_fail_on_nonexistent_copr(self): - r = self.tc.get('/coprs/repo/bogus-nonexistent-repo.repo') + r = self.tc.get('/coprs/bogus-nonexistent-repo/repo/') assert r.status_code == 404 assert 'bogus/nonexistent-repo does not exist' in r.data
def test_fail_on_no_finished_builds(self, f_users, f_coprs, f_db): r = self.tc.get( - '/coprs/repo/{0}-{1}.repo'.format(self.u1.name, self.c1.name)) + '/coprs/{0}/{1}/repo/'.format(self.u1.name, self.c1.name)) assert r.status_code == 404 assert 'Repository not initialized' in r.data
def test_works_on_older_builds(self, f_users, f_coprs, f_custom_builds, f_db): - r = self.tc.get('/coprs/repo/{0}-{1}.repo'.format(self.u1.name, self.c1.name)) + r = self.tc.get('/coprs/{0}/{1}/repo/'.format(self.u1.name, self.c1.name)) assert r.status_code == 200 assert 'baseurl=foo://bar.baz' in r.data
On 09/14/2013 01:09 PM, Pierre-Yves Chibon wrote:
Looking forward for your feedbacks,
When I try to build package: tito build --test --rpm I get: Bytecompiling .py files below /home/msuchy/rpmbuild/BUILDROOT/copr-1.1-1.git.62.93c8b68.fc19.x86_64/usr/lib/python2.7/ using /usr/bin/python2.7 Compiling /home/msuchy/rpmbuild/BUILDROOT/copr-1.1-1.git.62.93c8b68.fc19.x86_64/usr/share/copr/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py ... File "/usr/share/copr/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py", line 70 r = self.test_client.post('/coprs/{0}/new/'.format(self.u1.name)', data={'name': 'foo', 'fedora-rawhide-i386': 'y', 'arches': ['i386']}, follow_redirects=True) ^ SyntaxError: invalid syntax
You forgot one apostrophe after format().
When I fixed that and installed it, it seems good to me. Fix that one problem and commit.
On Mon, Sep 16, 2013 at 09:18:38AM +0200, Miroslav Suchý wrote:
On 09/14/2013 01:09 PM, Pierre-Yves Chibon wrote:
Looking forward for your feedbacks,
When I try to build package: tito build --test --rpm I get: Bytecompiling .py files below /home/msuchy/rpmbuild/BUILDROOT/copr-1.1-1.git.62.93c8b68.fc19.x86_64/usr/lib/python2.7/ using /usr/bin/python2.7 Compiling /home/msuchy/rpmbuild/BUILDROOT/copr-1.1-1.git.62.93c8b68.fc19.x86_64/usr/share/copr/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py ... File "/usr/share/copr/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_general.py", line 70 r = self.test_client.post('/coprs/{0}/new/'.format(self.u1.name)', data={'name': 'foo', 'fedora-rawhide-i386': 'y', 'arches': ['i386']}, follow_redirects=True) ^ SyntaxError: invalid syntax
You forgot one apostrophe after format().
When I fixed that and installed it, it seems good to me. Fix that one problem and commit.
Thanks for reviewing, I fixed the quote typo and pushed.
Pierre
On 09/14/2013 01:09 PM, Pierre-Yves Chibon wrote:
This is a rather large change which might have consequences in several places. From my tests it works and hopefully I did not miss to many places.
Pierre, it seems that it does not behave correctly when I name the copr (as project) "copr"
i.e. when I submit form from: http://copr-fe-dev.cloud.fedoraproject.org/coprs/msuchy/foo/edit/ then everything is correct, but when I submit form from: http://copr-fe-dev.cloud.fedoraproject.org/coprs/msuchy/copr/edit/ I will get logged out. It behave this way for all forms under "copr" copr.
Can you please investigate it?
Mirek
copr-devel@lists.fedorahosted.org
-
Miroslav Suchý -
Pierre-Yves Chibon