#2253 Add the possibility to give a project to someone else
Merged by pingou. Opened by pingou.
give_repo  into  master

Download 2253.patch
no initial comment

rebased

shouldn't it be something in 400s? It's not the problem with the server but, the user somehow sent a username which is not correct

Shouldn't this be something like "owner"? That seems to be the term used when displaying a project's ACLs.

I agree it should be a 404 or similar. I think pingou here is assuming that the UI found the user to give the repo to in the UI (the autofill text box), and that if this fails, it's a server error.

Since the user can circumvent the UI, this should be a 404 or similar.

I tend to like flash messages that are more descriptive. In this case, something like "The project ownership has been updated" or 'The project owner has been transferred to "{0}"'.format(new_username).

I would prefer the user not be exposed the SQLAlchemy error here. In my opinion, things like this should be logged and a generic error be returned to the user.

Specifying here that you mean the user is a Pagure admin and not a project admin would be helpful.

@pingou the code looks good. I just left a few minor comments. I tried it out locally and everything worked for me except the autofill functionality when typing for a user to give the project to. Please make sure this works properly for you on your dev instance. I assume it was an issue on my end since the user I created to transfer the project to didn't actually exist in FAS (just manual creation in the database).

Since this is supposed to be also used in dist-git, I tried to avoid that wording.

Sure, good catch :)

Sure thing

I'll adjust the docstring

2 new commits added

  • Improve docstring in the tests to better explain what is tested
  • Adjust information returned to the user, error code and messages

+1 as long as you verified the autofill/autosuggestion form works on your instance (I explained this in my previous comment).

You'll need to update these lines for the tests to pass.

1 new commit added

  • Fix the unit-tests

I confirm that auto-suggest works for me locally, I am surprised it doesn't for you actually.

Maybe we can tackle this on IRC if you have some time.

Pretty please pagure-ci rebuild

Pretty please pagure-ci rebuild

1 new commit added

  • Drop un-used variable

Ok, so now that jenkins is finally happy, let's merge

Thanks for the reviews!

Pull-Request has been merged by pingou

Metadata