Wouldn't it be better to do /<repo>/token/create or something? createapitoken is a pretty poorly named endpoint...

Thanks, yes you are right so here is an update.

You could use a wtforms form here to do the validation for you (disable the csrf protection and dynamically load the list of ACLs), there are some examples of this in the code already (let me know if you don't find one)

Let's specify that it's acls=None here :)

We should use a list here, saves us the troubles of splitting the content on comas

You may want to use form.getlist('acl') here ;-)

@fbo if you would have time to update this one it would be nice, this way we could get it into the 5.3-beta I hope to cut tomorrow

Sure, I'll travel tomorrow morning but I'll be able to finish it (and the other PR (hope)) in the train.

I don't think we want the full list here, more likely the USER_ACLS list only

descriptions are not unique, so depending on the ordering you may get an expired token here. Should we make add_token_to_user return the token id instead?

Any idea why the CI fails this way ? is it related to the patch ?

Looks like at least one test is failing (genuinely)

It returns three failing test files:

15:50:35 FAILED test: py-test_pagure_repospanner
15:50:35 FAILED test: py-test_pagure_flask_ui_repo
15:50:35 FAILED test: py-test_pagure_flask_ui_app

The first one currently fails on all tests/PRs (I hope the new way to run the tests will help with this), the other two are more concerning me :(

A local run with the container reports:
Failed tests:
FAILED test: py3-test_style
FAILED test: py3-test_pagure_flask_ui_repo
FAILED test: py3-test_pagure_flask_ui_app

Ok so then let me know if the patch needs more edit or if it ready to be merged :)

91 failed :( should I rebase the branch on master ?

The UI endpoint seems to be //token/new, should we align the API with the UI?

Ok, I've had to apply the following two patches to get this PR to pass tests:

Black seems still unhappy: https://ci.centos.org/job/pagure-pr/2212/artifact/pagure/results_f29-rpms-py3/py3-test_style/view/

This is the change black does locally:

diff --git a/ pagure/api/project.py b/ pagure/api/project.py
index 20c6e8af..e947a306 100644
--- a/ pagure/api/project.py    
+++ b/ pagure/api/project.py    
@@ -2138,9 +2138,7 @@ def api_modify_project_options(repo, username=None, namespace=None):
 @API.route("/<repo>/token/new", methods=["POST"])
 @API.route("/<namespace>/<repo>/token/new", methods=["POST"])
 @API.route("/fork/<username>/<repo>/token/new", methods=["POST"])
-@API.route(
-    "/fork/<username>/<namespace>/<repo>/token/new", methods=["POST"]
-)
+@API.route("/fork/<username>/<namespace>/<repo>/token/new", methods=["POST"])
 @api_login_required(acls=["modify_project"])
 @api_method
 def api_project_create_api_token(repo, namespace=None, username=None):

Thanks I'll fix that.

Curious, I've run the full test suite locally and black has not complain.

The failed test is repospanner which fails for everyone, everything pass, so let's get this in :)

\o/