Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Cannot remove user that owns a project. #1906

Merged
merged 1 commit into from Nov 19, 2012

Conversation

Projects
None yet
3 participants
Owner

dosire commented Nov 2, 2012

You get an error if you delete a user and then try to delete a project that user owns. To prevent this you should not be able to delete a user that owns project. This PR ensures that.

Also see https://github.com/dosire/gitlabhq/issues/55

@dzaporozhets dzaporozhets commented on an outdated diff Nov 3, 2012

app/controllers/admin/users_controller.rb
@@ -98,6 +98,7 @@ def update
def destroy
@admin_user = User.find(params[:id])
+ redirect_to admin_users_url, notice: 'User is a project owner, cannot be removed.' and return if @admin_user.my_own_projects.count > 0
@dzaporozhets

dzaporozhets Nov 3, 2012

Owner
  1. maybe admin_users_path
  2. too long string with condition at the end. Please make it more readable

@ghost ghost assigned dzaporozhets Nov 3, 2012

Owner

dosire commented Nov 5, 2012

  1. maybe admin_users_path
  2. too long string with condition at the end. Please make it more readable

=>

@randx Thank you for the above feedback. Both points are addressed.

Owner

dzaporozhets commented Nov 7, 2012

@dosire thank you. Can you please also squash all this commits or something.
8 commits with merges for 3 lines of code is not ok :)

@maxlazio @dosire maxlazio Cannot remove user that owns a project.
Cannot remove user that owns a project.

Make lines shorter, set alert, use path instead of url.
19e8473
Owner

dosire commented Nov 16, 2012

@randx I removed all the merge commits, sorry about that.

@dzaporozhets dzaporozhets added a commit that referenced this pull request Nov 19, 2012

@dzaporozhets dzaporozhets Merge pull request #1906 from dosire/user_cannot_be_removed_if_projec…
…t_owner

Cannot remove user that owns a project.
4eef112

@dzaporozhets dzaporozhets merged commit 4eef112 into gitlabhq:master Nov 19, 2012

1 check failed

default The Travis build failed
Details
Owner

dosire commented Nov 20, 2012

Thank you for the merge @randx

@DouweM DouweM added a commit that referenced this pull request Mar 8, 2015

@DouweM DouweM Add list of changed files to EmailsOnPush.
See #1906.
7b34c9d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment