This PR adds a token ACL called "modify_project", which can be used to modify the ownership of a project on a new API endpoint.
I had trouble running tests that required celery, so I couldn't test the entire test suite, but the new tests I added all pass.
celery
2 new commits added
I've seen people not liking the terminology around owner, maybe we should reconsider it. I don't have a one-word suggestion though :/
principle_maintainer or main_admin are what comes to my mind but :/
principle_maintainer
main_admin
@pingou, since owner is used elsewhere in the API, let's keep it as is. For the Pagure-over-dist-git deployment, perhaps we can have a patch on the UI to change the terminology.
owner
I think owner is only used in the settings page, so that's an easy one to fix.
I'm bringing this question now because I consider it still possible to change, the longer we wait the harder it will be to do.
@pingou I'd prefer not to change it in the API, but if you feel strongly about it, I can change it here and create a PR that changes the terminology in other use cases in the API. With that said, I have had trouble running the unit tests in Vagrant on the project as a whole since the rearchitecture. That would make it rather difficult to verify the renames.
@pingou I realized that I should be using the PATCH instead of PUT here, so I made that change.
To replace owner how about any of: steward , guardian , coordinator ?
If it's like "give project" from UI, then the condition should be : https://pagure.io/pagure/blob/master/f/pagure/ui/repo.py#_2584
Otherwise, the admins will be able to change the "main admin" via API and not via UI :p
So I want admins to be able to use this API, just not change the ownership.
You can see that I further restrict it below when the user tries to change the ownership:
if flask.g.fas_user != project.user: raise pagure.exceptions.APIError(401, error_code=APIERROR.ENOTOWNER)
oooo... so, this endpoint is not just about "giving project".
For now it can only be used to give a project, but I wrote it in a way so that we can expand it later on.
github has terms "owner" which can be multiple persons who can modify settings of projects and "collaborators" who can only have access to pushing code... So I would prefer to support multiple owners (and call them "admin"s) and everything else stays same.... Just my 5 cents about naming.
OK, :+1: to changing the language in the API but... can we make that a part of a separate change?
something_else
As for putting pagure over dist-git and concerns in the Fedora community about language, @mprahl and I can (try to) make sure that the client tooling calls this "point of contact" wherever users see it, even if under the hood the API calls call it "owner".
I filed a separate #2327 to take that up longer-term.
Is the language of the owner field the only item currently in debate for this PR?
In IRC in #pagure, @pingou suggested changing owner only in this new write API.. but keeping it in all of the other existing read APIs.
#pagure
read
rebased
@pingou: I renamed owner to main_admin in this API. Please let me know if that works for you.
Can't we rely on tests.create_tokens ?
No try/except on sqlalchemyerror?
Couple of comments otherwise looks good, thanks! :)
@pingou, thanks for the review. This should be ready for another round!
I'm getting a couple of test failures, I'll fix them then we can merge :)
Pull-Request has been merged by pingou
This PR adds a token ACL called "modify_project", which can be used to modify the ownership of a project on a new API endpoint.
I had trouble running tests that required
celery, so I couldn't test the entire test suite, but the new tests I added all pass.