namespaces: unify admin vision with regular page

We got rid of the admin namespaces page and unified the vision of the admin on the regular namespace page. So, if the user is an admin, they will be seeing all of the namespaces and will be able to manage all of them. If the user is a regular one, everything stays the same. API is also affected by this change. One improvement we did was to separate the other namespaces the user has access to without being through membership in a different panel. This will help the user understand better how they have access to those namespaces. Signed-off-by: Vítor Avelino <[email protected]>
SUSE · Apr 16, 2018 · 3e56107 · 3e56107
1 parent dfc5357
commit 3e56107
Show file tree

Hide file tree

Showing 18 changed files with 190 additions and 214 deletions.
diff --git a/app/assets/javascripts/modules/namespaces/pages/index.vue b/app/assets/javascripts/modules/namespaces/pages/index.vue
@@ -1,5 +1,7 @@
 <template>
   <div class="namespaces-index-page">
+    <new-namespace-form :state="state" form-state="newFormVisible"></new-namespace-form>
+
     <namespaces-panel :namespaces="specialNamespaces" :namespaces-path="namespacesPath" :webhooks-path="webhooksPath" prefix="sns_">
       <h5 slot="name">
         <a data-placement="right"
@@ -13,16 +15,18 @@
         </a>
         Special namespaces
       </h5>
-    </namespaces-panel>
-
-    <new-namespace-form :state="state" form-state="newFormVisible"></new-namespace-form>
-
-    <namespaces-panel :namespaces="normalNamespaces" :namespaces-path="namespacesPath" :webhooks-path="webhooksPath" :table-sortable="true">
-      <h5 slot="name">Namespaces you have access to</h5>
       <div slot="actions" v-if="canCreateNamespace">
         <toggle-link text="Create new namespace" :state="state" state-key="newFormVisible" class="toggle-link-new-namespace"></toggle-link>
       </div>
     </namespaces-panel>
+
+    <namespaces-panel :namespaces="normalNamespaces" :namespaces-path="namespacesPath" :webhooks-path="webhooksPath" :table-sortable="true" class="member-namespaces-panel">
+      <h5 slot="name">Namespaces you have access to through membership</h5>
+    </namespaces-panel>
+
+    <namespaces-panel :namespaces="otherNamespaces" :namespaces-path="namespacesPath" :webhooks-path="webhooksPath" prefix="ons_" :table-sortable="true" v-if="otherNamespaces.length">
+      <h5 slot="name">Other namespaces</h5>
+    </namespaces-panel>
   </div>
 </template>
 
@@ -54,6 +58,9 @@
       canCreateNamespace: {
         type: Boolean,
       },
+      accessibleTeamsIds: {
+        type: Array,
+      },
     },
 
     components: {
@@ -65,31 +72,50 @@
     data() {
       return {
         state: NamespacesStore.state,
-        normalNamespaces: [],
-        specialNamespaces: [],
+        namespaces: [],
       };
     },
 
+    computed: {
+      otherNamespaces() {
+        // eslint-disable-next-line
+        return this.namespaces.filter((n) => {
+          return !n.global &&
+                 n.id !== this.userNamespaceId &&
+                 this.accessibleTeamsIds.indexOf(n.team_id) === -1;
+        });
+      },
+
+      normalNamespaces() {
+        // eslint-disable-next-line
+        return this.namespaces.filter((n) => {
+          return !n.global &&
+                 n.id !== this.userNamespaceId &&
+                 this.accessibleTeamsIds.indexOf(n.team_id) !== -1;
+        });
+      },
+
+      specialNamespaces() {
+        return this.namespaces.filter(n => n.global || n.id === this.userNamespaceId);
+      },
+    },
+
     methods: {
       onCreate(namespace) {
-        const currentNamespaces = this.normalNamespaces;
+        const currentNamespaces = this.namespaces;
         const namespaces = [
           ...currentNamespaces,
           namespace,
         ];
 
-        set(this, 'normalNamespaces', namespaces);
+        set(this, 'namespaces', namespaces);
       },
 
       loadData() {
         NamespacesService.all().then((response) => {
           const namespaces = response.data;
 
-          const normal = namespaces.filter(n => !n.global && n.id !== this.userNamespaceId);
-          const special = namespaces.filter(n => n.global || n.id === this.userNamespaceId);
-
-          set(this, 'normalNamespaces', normal);
-          set(this, 'specialNamespaces', special);
+          set(this, 'namespaces', namespaces);
           set(this.state, 'isLoading', false);
         });
       },

diff --git a/app/controllers/admin/namespaces_controller.rb b/app/controllers/admin/namespaces_controller.rb
diff --git a/app/controllers/namespaces_controller.rb b/app/controllers/namespaces_controller.rb
@@ -56,9 +56,14 @@ def update
 
   # GET /namespace/typeahead/%QUERY
   def typeahead
-    @query = params[:query]
-    valid_teams = TeamUser.get_valid_team_ids(current_user.id)
-    matches = Team.search_from_query(valid_teams, "#{@query}%").pluck(:name)
+    valid_teams_ids = if current_user.admin?
+                        Team.all_non_special.pluck(:id)
+                      else
+                        TeamUser.get_valid_team_ids(current_user.id)
+                      end
+
+    query = "#{params[:query]}%"
+    matches = Team.search_from_query(valid_teams_ids, query).pluck(:name)
     matches = matches.map { |team| { name: ActionController::Base.helpers.sanitize(team) } }
     respond_to do |format|
       format.json { render json: matches.to_json }

diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb
@@ -9,7 +9,7 @@ class TeamsController < ApplicationController
 
   # GET /teams
   def index
-    @teams = policy_scope(Team).page(params[:page])
+    @teams = policy_scope(Team)
     @teams_serialized = API::Entities::Teams.represent(
       @teams,
       current_user: current_user,

diff --git a/app/models/namespace.rb b/app/models/namespace.rb
@@ -29,10 +29,6 @@ class Namespace < ActiveRecord::Base
     attributes :name, :description
   end
 
-  scope :special_for, lambda { |user|
-    where("global = ? OR namespaces.id = ?", true, user.namespace_id)
-  }
-
   # This regexp is extracted from the reference package of Docker Distribution
   # and it matches a valid namespace name.
   NAME_REGEXP = /\A[a-z0-9]+(?:[._\\-][a-z0-9]+)*\Z/

diff --git a/app/policies/namespace_policy.rb b/app/policies/namespace_policy.rb
@@ -82,17 +82,23 @@ def initialize(user, scope)
     end
 
     def resolve
-      scope
-        .joins(team: [:team_users])
-        .where(
-          "(namespaces.visibility = :public OR namespaces.visibility = :protected " \
-          "OR team_users.user_id = :user_id) AND " \
-          "namespaces.global = :global AND namespaces.id != :namespace_id",
-          public: Namespace.visibilities[:visibility_public],
-          protected: Namespace.visibilities[:visibility_protected],
-          user_id: user.id, global: false, namespace_id: user.namespace_id
-        )
-        .distinct
+      if user.admin?
+        global = scope.where(global: true)
+        normal = scope.not_portus
+                      .where(global: false)
+                      .order(created_at: :asc)
+        global + normal
+      else
+        scope
+          .joins(team: [:team_users])
+          .where(
+            "namespaces.visibility = :public OR namespaces.visibility = :protected " \
+            "OR team_users.user_id = :user_id",
+            public:    Namespace.visibilities[:visibility_public],
+            protected: Namespace.visibilities[:visibility_protected],
+            user_id:   user.id
+          )
+      end
     end
   end
 

diff --git a/app/views/admin/dashboard/index.html.slim b/app/views/admin/dashboard/index.html.slim
@@ -47,7 +47,7 @@
   .col-md-3.col-xs-12.namespaces
     .panel-overview
       .panel-heading
-        = link_to 'Namespaces', admin_namespaces_path
+        = link_to 'Namespaces', namespaces_path
       .panel-body
         .row
           .col-xs-3.col-md-4

diff --git a/app/views/namespaces/index.html.erb b/app/views/namespaces/index.html.erb
@@ -1,6 +1,7 @@
 <namespaces-index-page
   :can-create-namespace="<%= can_create_namespace? %>"
   :user-namespace-id="<%= current_user.namespace_id %>"
+  :accessible-teams-ids="<%= current_user.teams.pluck(:id) %>"
   webhooks-path="webhooks"
   namespaces-path="<%= namespaces_path %>">
 </namespaces-index-page>

diff --git a/config/routes/admin.rb b/config/routes/admin.rb
@@ -4,7 +4,6 @@
   resources :activities, only: [:index]
   resources :dashboard, only: [:index]
   resources :registries, except: %i[show destroy]
-  resources :namespaces, only: [:index]
   resources :users do
     put "toggle_admin", on: :member
   end

diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
@@ -2,14 +2,12 @@
 
 require "portus/auth_from_token"
 require "api/helpers/errors"
-require "api/helpers/namespaces"
 
 module API
   module Helpers
     include ::Portus::AuthFromToken
 
     include Errors
-    include Namespaces
 
     # On success it will fill the @user instance variable with the currently
     # authenticated user for the API. Otherwise it will raise:

diff --git a/lib/api/helpers/namespaces.rb b/lib/api/helpers/namespaces.rb
diff --git a/lib/api/v1/namespaces.rb b/lib/api/v1/namespaces.rb
@@ -11,11 +11,12 @@ class Namespaces < Grape::API
           authorization!(force_admin: false)
         end
 
-        helpers ::API::Helpers::Namespaces
-
         desc "Returns a list of namespaces",
              tags:     ["namespaces"],
-             detail:   "This will expose all accessible namespaces",
+             detail:   "This will expose all accessible namespaces by the user via either team
+                        membership or visibility. Keep in mind that if the user is an admin, this will
+                        return all the global, personal and other namespaces created by all the
+                        users.",
              is_array: true,
              entity:   API::Entities::Namespaces,
              failure:  [
@@ -24,7 +25,7 @@ class Namespaces < Grape::API
              ]
 
         get do
-          present accessible_namespaces,
+          present policy_scope(Namespace),
                   with:         API::Entities::Namespaces,
                   current_user: current_user,
                   type:         current_type

diff --git a/spec/api/grape_api/v1/namespaces_spec.rb b/spec/api/grape_api/v1/namespaces_spec.rb
@@ -36,22 +36,29 @@
   end
 
   context "GET /api/v1/namespaces" do
-    it "returns an empty list" do
-      get "/api/v1/namespaces", nil, @admin_header
+    let!(:registry) { create(:registry) }
 
-      namespaces = JSON.parse(response.body)
-      expect(response).to have_http_status(:success)
-      expect(namespaces.length).to eq(0)
+    context "as admin" do
+      it "returns list of accessible namespaces" do
+        get "/api/v1/namespaces", nil, @admin_header
+
+        # global + personal + weird ones that I have no idea where it comes from
+        # so, magic number :(
+        namespaces = JSON.parse(response.body)
+        expect(response).to have_http_status(:success)
+        expect(namespaces.length).to eq(7)
+      end
     end
 
-    it "returns list of accessible namespaces" do
-      # global + personal + below
-      create(:namespace, visibility: public_visibility, team: team)
-      get "/api/v1/namespaces", nil, @admin_header
+    context "as regular user" do
+      it "returns list of accessible namespaces" do
+        get "/api/v1/namespaces", nil, @owner_header
 
-      namespaces = JSON.parse(response.body)
-      expect(response).to have_http_status(:success)
-      expect(namespaces.length).to eq(3)
+        # only personal one
+        namespaces = JSON.parse(response.body)
+        expect(response).to have_http_status(:success)
+        expect(namespaces.length).to eq(1)
+      end
     end
   end
 

diff --git a/spec/controllers/admin/namespaces_controller_spec.rb b/spec/controllers/admin/namespaces_controller_spec.rb