Merge branch '29943-environment-folder' into 'security-9-5'

Do not use `location.pathname` when accessing environments folders

See merge request !2147

5 files changed, 19 insertions, 30 deletions

diff --git a/app/assets/javascripts/environments/components/environment.vue b/app/assets/javascripts/environments/components/environment.vue
index 91ed8c8467f..f54d573db6e 100644
--- a/app/assets/javascripts/environments/components/environment.vue
+++ b/app/assets/javascripts/environments/components/environment.vue
@@ -111,11 +111,11 @@ export default {
   },
 
   methods: {
-    toggleFolder(folder, folderUrl) {
+    toggleFolder(folder) {
       this.store.toggleFolder(folder);
 
       if (!folder.isOpen) {
-        this.fetchChildEnvironments(folder, folderUrl, true);
+        this.fetchChildEnvironments(folder, true);
       }
     },
 
@@ -143,10 +143,10 @@ export default {
         .catch(this.errorCallback);
     },
 
-    fetchChildEnvironments(folder, folderUrl, showLoader = false) {
+    fetchChildEnvironments(folder, showLoader = false) {
       this.store.updateEnvironmentProp(folder, 'isLoadingFolderContent', showLoader);
 
-      this.service.getFolderContent(folderUrl)
+      this.service.getFolderContent(folder.folder_path)
         .then(resp => resp.json())
         .then(response => this.store.setfolderContent(folder, response.environments))
         .then(() => this.store.updateEnvironmentProp(folder, 'isLoadingFolderContent', false))
@@ -173,12 +173,7 @@ export default {
       // We need to verify if any folder is open to also update it
       const openFolders = this.store.getOpenFolders();
       if (openFolders.length) {
-        openFolders.forEach((folder) => {
-          // TODO - Move this to the backend
-          const folderUrl = `${window.location.pathname}/folders/${folder.folderName}`;
-
-          return this.fetchChildEnvironments(folder, folderUrl);
-        });
+        openFolders.forEach(folder => this.fetchChildEnvironments(folder));
       }
     },
 
diff --git a/app/assets/javascripts/environments/components/environment_item.vue b/app/assets/javascripts/environments/components/environment_item.vue
index d8b1b2f1b92..6de01fa53d0 100644
--- a/app/assets/javascripts/environments/components/environment_item.vue
+++ b/app/assets/javascripts/environments/components/environment_item.vue
@@ -410,20 +410,11 @@ export default {
              this.hasStopAction ||
              this.canRetry;
     },
-
-    /**
-     * Constructs folder URL based on the current location and the folder id.
-     *
-     * @return {String}
-     */
-    folderUrl() {
-      return `${window.location.pathname}/folders/${this.model.folderName}`;
-    },
   },
 
   methods: {
     onClickFolder() {
-      eventHub.$emit('toggleFolder', this.model, this.folderUrl);
+      eventHub.$emit('toggleFolder', this.model);
     },
   },
 };
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 435eeaf0e2e..9b05f8b1cd5 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -82,12 +82,7 @@ class Environment < ActiveRecord::Base
   def set_environment_type
     names = name.split('/')
 
-    self.environment_type =
-      if names.many?
-        names.first
-      else
-        nil
-      end
+    self.environment_type = names.many? ? names.first : nil
   end
 
   def includes_commit?(commit)
@@ -101,7 +96,7 @@ class Environment < ActiveRecord::Base
   end
 
   def update_merge_request_metrics?
-    (environment_type || name) == "production"
+    folder_name == "production"
   end
 
   def first_deployment_for(commit)
@@ -223,6 +218,10 @@ class Environment < ActiveRecord::Base
       format: :json)
   end
 
+  def folder_name
+    self.environment_type || self.name
+  end
+
   private
 
   # Slugifying a name may remove the uniqueness guarantee afforded by it being
diff --git a/app/serializers/environment_entity.rb b/app/serializers/environment_entity.rb
index dcaccc3007d..ba0ae6ba8a0 100644
--- a/app/serializers/environment_entity.rb
+++ b/app/serializers/environment_entity.rb
@@ -26,5 +26,9 @@ class EnvironmentEntity < Grape::Entity
       terminal_project_environment_path(environment.project, environment)
   end
 
+  expose :folder_path do |environment|
+    folder_project_environments_path(environment.project, environment.folder_name)
+  end
+
   expose :created_at, :updated_at
 end
diff --git a/app/serializers/environment_serializer.rb b/app/serializers/environment_serializer.rb
index d0a60f134da..88842a9aa75 100644
--- a/app/serializers/environment_serializer.rb
+++ b/app/serializers/environment_serializer.rb
@@ -36,9 +36,9 @@ class EnvironmentSerializer < BaseSerializer
   private
 
   def itemize(resource)
-    items = resource.order('folder_name ASC')
+    items = resource.order('folder ASC')
       .group('COALESCE(environment_type, name)')
-      .select('COALESCE(environment_type, name) AS folder_name',
+      .select('COALESCE(environment_type, name) AS folder',
               'COUNT(*) AS size', 'MAX(id) AS last_id')
 
     # It makes a difference when you call `paginate` method, because
@@ -49,7 +49,7 @@ class EnvironmentSerializer < BaseSerializer
     environments = resource.where(id: items.map(&:last_id)).index_by(&:id)
 
     items.map do |item|
-      Item.new(item.folder_name, item.size, environments[item.last_id])
+      Item.new(item.folder, item.size, environments[item.last_id])
     end
   end
 end