Admins can now change the ownership of a namespace

Fixes #750

Signed-off-by: Miquel Sabaté Solà <[email protected]>

app/assets/javascripts/teams.coffee

@@ -1,5 +1,4 @@
 $(document).on "page:change", ->
-
   set_typeahead = (url) ->
     $('.remote .typeahead').typeahead 'destroy'
     bloodhound = new Bloodhound(
@@ -15,6 +14,9 @@ $(document).on "page:change", ->
       displayKey: 'name',
       source: bloodhound.ttAdapter()
 
+  $('#edit_namespace').on 'click', (event) ->
+    set_typeahead('/teams/typeahead/%QUERY')
+
   $('#add_team_user_btn').on 'click', (event) ->
     $('#team_user_user').val('')
     $('#team_user_role').val('viewer')
@@ -53,7 +55,6 @@ $(document).on "page:change", ->
       open_close_icon(el)
       $('.description').toggle()
       $('#change_description_namespace_' + event.currentTarget.value).toggle()
-      $('#namespace_description').focus()
   )
 
   $('#add_namespace_btn').unbind('click').on 'click', (event) ->

app/controllers/concerns/change_name_description.rb

@@ -2,29 +2,28 @@ module ChangeNameDescription
   extend ActiveSupport::Concern
 
   # Update the attributes name or description of a ActiveRecord-object.
-  def change_name_description(object, symbol)
+  def change_name_description(object, symbol, params)
     authorize object
 
-    new_name = params.require(symbol).permit(:name)
-    new_description = params.require(symbol).permit(:description)
+    new_name = params[:name]
+    new_description = params[:description]
     old_description = object.description
     old_name = object.name
 
-    unless old_description == new_description["description"]
-      object.update(new_description)
-      create_activity(object, symbol, old_description, new_description)
+    if !new_description.nil? && old_description != new_description
+      object.update(description: new_description)
+      create_activity(object, symbol, "description", old_description, new_description)
     end
 
-    return if old_name == new_name["name"] || new_name.empty?
-    object.update(new_name)
-    create_activity(object, symbol, old_name, new_name)
+    return if old_name == new_name || new_name.blank?
+    object.update(name: new_name)
+    create_activity(object, symbol, "name", old_name, new_name)
   end
 
   private
 
   # Create a PublicActivity for updating an attribute.
-  def create_activity(object, symbol, old_value, new_value)
-    ctx, new_value = new_value.first
+  def create_activity(object, symbol, ctx, old_value, new_value)
     object.create_activity "change_#{symbol}_#{ctx}".to_sym,
                                   owner:      current_user,
                                   recipient:  object,

app/controllers/namespaces_controller.rb

@@ -48,7 +48,19 @@ def create
   # PATCH/PUT /namespace/1
   # PATCH/PUT /namespace/1.json
   def update
-    change_name_description(@namespace, :namespace)
+    p = params.require(:namespace).permit(:name, :description, :team)
+    change_name_description(@namespace, :namespace, p)
+
+    # Update the team if needed/authorized.
+    return if p[:team] == @namespace.team.name
+    authorize @namespace, :change_team?
+
+    @team = Team.find_by(name: p[:team])
+    if @team.nil?
+      @namespace.errors[:team_id] << "'#{p[:team]}' unknown."
+    else
+      @namespace.update_attributes(team: @team)
+    end
   end
 
   # GET /namespace/typeahead/%QUERY

app/controllers/teams_controller.rb

@@ -38,7 +38,8 @@ def create
   # PATCH/PUT /teams/1
   # PATCH/PUT /teams/1.json
   def update
-    change_name_description(@team, :team)
+    p = params.require(:team).permit(:name, :description)
+    change_name_description(@team, :team, p)
   end
 
   # GET /teams/1/typeahead/%QUERY
@@ -52,6 +53,16 @@ def typeahead
     end
   end
 
+  # GET /teams/typeahead/%QUERY
+  def all_with_query
+    query = "#{params[:query]}%"
+    teams = policy_scope(Team).where("name LIKE ?", query).pluck(:name)
+    matches = teams.map { |t| { name: t } }
+    respond_to do |format|
+      format.json { render json: matches.to_json }
+    end
+  end
+
   private
 
   def set_team

app/policies/namespace_policy.rb

@@ -39,6 +39,12 @@ def toggle_public?
     !namespace.global? && (user.admin? || namespace.team.owners.exists?(user.id))
   end
 
+  # Only owners and admins can change the team ownership.
+  def change_team?
+    raise Pundit::NotAuthorizedError, "must be logged in" unless user
+    user.admin? || namespace.team.owners.exists?(user.id)
+  end
+
   class Scope
     attr_reader :user, :scope
 

app/views/namespaces/show.html.slim

@@ -1,14 +1,13 @@
 .panel.panel-default
   .panel-heading
-
     h5
       strong
         '#{@namespace.clean_name}
       ' namespace
       - if can_manage_namespace?(@namespace)
         .pull-right
           button.btn.btn-link.btn-xs.btn-edit-role[
-            value="#{@namespace.id}" class="button_namespace_description"]
+            value="#{@namespace.id}" id="edit_namespace" class="button_namespace_description"]
             i.fa.fa-pencil.fa-lg
             | Edit namespace
       small
@@ -28,11 +27,18 @@
         = markdown(@namespace.description)
     .collapse id="change_description_namespace_#{@namespace.id}"
       = form_for @namespace, remote: true, html: {role: 'form'} do |f|
-        = f.text_area(:description, class: 'form-control fixed-size', placeholder: html_escape(@namespace.description))
-        br
-          = button_tag(type: 'submit', class: 'btn btn-primary pull-right') do
-            i.fa.fa-check
-            | Save
+        .form-group
+          = f.label :team, "Team", {class: 'control-label col-md-2'}
+          .remote
+            = f.text_field(:team, value: "#{@namespace.team.name}", class: 'form-control typeahead', required: true, placeholder: "Name of the team")
+        .form-group
+          = f.label :description, {class: 'control-label col-md-2'}
+          = f.text_area(:description, class: 'form-control fixed-size', placeholder: html_escape(@namespace.description))
+        .form-group
+          br
+            = button_tag(type: 'submit', class: 'btn btn-primary pull-right') do
+              i.fa.fa-check
+              | Save
 
         .errors
   .panel-footer
@@ -44,7 +50,7 @@
       strong
         = @namespace.clean_name
     - unless @namespace.global?
-      h6.label.label-info
+      h6.label.label-info#team-label
         | <span>Belongs to: </span>
         = link_to "#{@namespace.team.name}", @namespace.team
   .panel-body

app/views/namespaces/update.js.erb

@@ -10,6 +10,8 @@
   <% else %>
     $('.description').html("<%= escape_javascript(markdown(@namespace.description)) %>");
   <% end %>
+
+  $("#team-label a").replaceWith("<%= escape_javascript(link_to(@team.name, @team)) %>")
 <% else %>
   $('#alert p').html("<%= escape_javascript(@namespace.errors.full_messages.join('<br/>')) %>");
   $('#alert').fadeIn();

app/views/teams/show.html.slim

@@ -6,7 +6,7 @@
           .input-group
             = f.text_field(:name,
                            class: 'form-control',
-                           placeholer: html_escape(@team.name),
+                           placeholder: html_escape(@team.name),
                            required: true,
                            input_html: { tabindex: 1 })
             .input-group-btn

config/routes.rb

@@ -5,6 +5,8 @@
       get "typeahead/:query" => "teams#typeahead", :defaults => { format: "json" }
     end
   end
+  get "/teams/typeahead/:query" => "teams#all_with_query", :defaults => { format: "json" }
+
   resources :team_users, only: [:create, :destroy, :update]
   resources :namespaces, only: [:create, :index, :show, :update] do
     put "toggle_public", on: :member

spec/controllers/namespaces_controller_spec.rb

@@ -81,7 +81,6 @@
 
       expect(response.status).to eq 401
     end
-
   end
 
   describe "POST #create" do
@@ -221,6 +220,36 @@
       patch :update, id: namespace.id, namespace: { description: "new description" }, format: "js"
       expect(response.status).to eq(200)
     end
+
+    it "does not allow to change the team to viewers" do
+      sign_out user
+
+      sign_in viewer
+      patch :update, id: namespace.id, namespace: { team: team.name + "o" }, format: "js"
+      expect(response.status).to eq(401)
+    end
+
+    it "does not allow to change the team to contributors" do
+      sign_out user
+      sign_in contributor
+      patch :update, id: namespace.id, namespace: { team: team.name + "o" }, format: "js"
+      expect(response.status).to eq(401)
+    end
+
+    it "changes the team if needed" do
+      team2 = create(:team)
+      sign_in owner
+      patch :update, id: namespace.id, namespace: { team: team2.name }, format: "js"
+      expect(response.status).to eq(200)
+      expect(namespace.reload.team.id).to eq team2.id
+    end
+
+    it "does nothing if you try to change to a non-existing team" do
+      sign_in owner
+      patch :update, id: namespace.id, namespace: { team: "unknown" }, format: "js"
+      expect(response.status).to eq(200)
+      expect(namespace.reload.team.id).to eq team.id
+    end
   end
 
   describe "typeahead" do

spec/controllers/teams_controller_spec.rb

@@ -11,16 +11,18 @@
     { admin: "not valid" }
   end
 
-  let(:owner) { create(:user) }
+  # TODO: (mssola) re-factor this so the team is always created. I've had to
+  # modify some tests that are not obvious because of the lazyness of `let` vs
+  # `let!`.
+  let!(:owner) { create(:user) }
   let(:team) { create(:team, description: "short test description", owners: [owner]) }
-  let(:hidden_team) do
+  let!(:hidden_team) do
     create(:team, name: "portus_global_team_1",
            description: "short test description", owners: [owner],
            hidden: true)
   end
 
   describe "GET #show" do
-
     it "paginates team users" do
       sign_in owner
       get :show, id: team.id
@@ -61,7 +63,7 @@
       get :show, id: team.id
 
       expect(response.status).to eq 200
-      expect(TeamUser.count).to be 2
+      expect(TeamUser.count).to be 3
       expect(assigns(:team_users).count).to be 1
     end
   end
@@ -152,7 +154,7 @@
       expect(usernames[0]["name"]).to eq("user2")
     end
 
-    it "does not allow to search by contributers or viewers" do
+    it "does not allow to search by contributors or viewers" do
       disallowed_roles = ["viewer", "contributer"]
       disallowed_roles.each do |role|
         user = create(:user)
@@ -164,6 +166,31 @@
     end
   end
 
+  describe "#all_with_query" do
+    it "fetches all the teams available" do
+      sign_in owner
+
+      # At this point the `team` variable has not been instantiated on the DB
+      # yet, so the result will be empty (the global team is rightfully not
+      # picked).
+      get :all_with_query, query: "team", format: "json"
+      teams = JSON.parse(response.body)
+      expect(teams).to be_empty
+
+      # Thus forcing the creation of the team too.
+      TeamUser.create(
+        team: team,
+        user: create(:user),
+        role: TeamUser.roles["viewer"]
+      )
+
+      get :all_with_query, query: "team", format: "json"
+      teams = JSON.parse(response.body)
+      expect(teams.size).to eq(1)
+      expect(teams.first["name"]).to eq(team.name)
+    end
+  end
+
   describe "activity tracking" do
     before :each do
       sign_in owner

spec/features/namespaces_spec.rb

@@ -84,6 +84,7 @@
       # Check that it created a link to it and that it's accessible.
       click_link "valid-namespace"
       namespace = Namespace.find_by(name: "valid-namespace")
+      wait_until { current_path == namespace_path(namespace) }
       expect(current_path).to eq namespace_path(namespace)
     end
 
@@ -123,4 +124,19 @@
       expect(page).to have_content("Namespace '#{namespace.name}' is now public")
     end
   end
+
+  describe "#update" do
+    it "returns an error when trying to update the team to a non-existing one", js: true do
+      visit namespace_path(namespace.id)
+      find("#edit_namespace").click
+      wait_for_ajax
+
+      fill_in "Team", with: "unknown"
+      find("#change_description_namespace_#{namespace.id} .btn").click
+
+      wait_for_ajax
+      wait_for_effect_on("#alert")
+      expect(page).to have_content("Team 'unknown' unknown")
+    end
+  end
 end