[PATCH 1/2] Update the URL to be based upon a /user/copr/<action> structure

Pierre-Yves Chibon pingou at pingoured.fr
Sat Sep 14 11:09:06 UTC 2013


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
 
- at coprs_ns.route('/detail/<username>/<coprname>/builds/', defaults={'page': 1})
- at coprs_ns.route('/detail/<username>/<coprname>/builds/<int:page>/')
+
+ at coprs_ns.route('/<username>/<coprname>/builds/', defaults={'page': 1})
+ at 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)
 
 
- at coprs_ns.route('/detail/<username>/<coprname>/add_build/')
+ at 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)
 
- at coprs_ns.route('/detail/<username>/<coprname>/new_build/', methods = ["POST"])
+
+ at 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)
 
- at coprs_ns.route('/detail/<username>/<coprname>/cancel_build/<int:build_id>/', methods = ['POST'])
+
+ at 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))
 
- at coprs_ns.route('/detail/<username>/<coprname>/repeat_build/<int:build_id>/', methods = ['POST'])
+
+ at 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)
 
 
- at coprs_ns.route('/owned/<username>/', defaults = {'page': 1})
- at coprs_ns.route('/owned/<username>/<int:page>/')
+ at coprs_ns.route('/<username>/', defaults = {'page': 1})
+ at 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)
 
 
- at coprs_ns.route('/allowed/<username>/', defaults = {'page': 1})
- at coprs_ns.route('/allowed/<username>/<int:page>/')
+ at coprs_ns.route('/<username>/allowed/', defaults = {'page': 1})
+ at 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)
 
- at coprs_ns.route('/add/')
+
+ at 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)
 
 
- at coprs_ns.route('/new/', methods=['POST'])
+ at 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)
 
- at coprs_ns.route('/detail/<username>/<coprname>/')
+
+ at 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)
 
- at coprs_ns.route('/detail/<username>/<coprname>/permissions/')
+
+ at 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)
 
- at coprs_ns.route('/detail/<username>/<coprname>/edit/')
+
+ at 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)
 
 
- at coprs_ns.route('/detail/<username>/<coprname>/update/', methods = ['POST'])
+ at 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)
 
 
- at coprs_ns.route('/detail/<username>/<coprname>/permissions_applier_change/', methods = ['POST'])
+ at 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))
 
- at coprs_ns.route('/detail/<username>/<coprname>/update_permissions/', methods = ['POST'])
+
+ at 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))
 
- at coprs_ns.route('/detail/<username>/<coprname>/delete/', methods=['GET', 'POST'])
+
+ at 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)
 
- at coprs_ns.route('/detail/<username>/<coprname>/legal_flag/', methods=['POST'])
+
+ at 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))
 
- at coprs_ns.route('/repo/<reponame>.repo')
-def generate_repo_file(reponame):
+
+ at 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)
-- 
1.8.3.1



More information about the copr-devel mailing list