Add cr remarksfix-name-vs-path-problem-for-cycle-analytics

Apply suggestion to changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml

Add cr remarks

12 files changed, 17 insertions, 18 deletions

diff --git a/app/serializers/analytics_issue_entity.rb b/app/serializers/analytics_issue_entity.rb
index cd96ce84c93..307ce14a921 100644
--- a/app/serializers/analytics_issue_entity.rb
+++ b/app/serializers/analytics_issue_entity.rb
@@ -26,6 +26,6 @@ class AnalyticsIssueEntity < Grape::Entity
   private
 
   def url_to(route, object)
-    public_send("#{route}_url", object[:path], object[:project_path], object[:iid].to_s) # rubocop:disable GitlabSecurity/PublicSend
+    public_send("#{route}_url", object[:namespace_path], object[:project_path], object[:iid].to_s) # rubocop:disable GitlabSecurity/PublicSend
   end
 end
diff --git a/changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml b/changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml
index 2f4631a5b1c..7d171c2cf5b 100644
--- a/changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml
+++ b/changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml
@@ -1,5 +1,5 @@
 ---
-title: Fix bug with passing project name instead of project path into url what was resulting with error
+title: Fix broken issue links and possible 500 error on cycle analytics page when project name and path are different
 merge_request: 31471
 author:
 type: fixed
diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb
index 474cf197758..459bb5177b5 100644
--- a/lib/gitlab/cycle_analytics/base_query.rb
+++ b/lib/gitlab/cycle_analytics/base_query.rb
@@ -20,6 +20,7 @@ module Gitlab
           .join(routes_table).on(projects_table[:namespace_id].eq(routes_table[:source_id]))
           .project(issue_table[:project_id].as("project_id"))
           .project(projects_table[:path].as("project_path"))
+          .project(routes_table[:path].as("namespace_path"))
 
         query = limit_query(query, project_ids)
 
diff --git a/lib/gitlab/cycle_analytics/code_event_fetcher.rb b/lib/gitlab/cycle_analytics/code_event_fetcher.rb
index fb9441ca18e..fcc282bf7a6 100644
--- a/lib/gitlab/cycle_analytics/code_event_fetcher.rb
+++ b/lib/gitlab/cycle_analytics/code_event_fetcher.rb
@@ -11,8 +11,7 @@ module Gitlab
                         mr_table[:id],
                         mr_table[:created_at],
                         mr_table[:state],
-                        mr_table[:author_id],
-                        routes_table[:path]]
+                        mr_table[:author_id]]
         @order = mr_table[:created_at]
 
         super(*args)
diff --git a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb
index 13eb38a883c..6914cf24c19 100644
--- a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb
+++ b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb
@@ -10,8 +10,7 @@ module Gitlab
                         issue_table[:iid],
                         issue_table[:id],
                         issue_table[:created_at],
-                        issue_table[:author_id],
-                        routes_table[:path]]
+                        issue_table[:author_id]]
 
         super(*args)
       end
diff --git a/lib/gitlab/cycle_analytics/issue_helper.rb b/lib/gitlab/cycle_analytics/issue_helper.rb
index 0d43098a74d..295eca5edca 100644
--- a/lib/gitlab/cycle_analytics/issue_helper.rb
+++ b/lib/gitlab/cycle_analytics/issue_helper.rb
@@ -9,6 +9,7 @@ module Gitlab
           .join(routes_table).on(projects_table[:namespace_id].eq(routes_table[:source_id]))
           .project(issue_table[:project_id].as("project_id"))
           .project(projects_table[:path].as("project_path"))
+          .project(routes_table[:path].as("namespace_path"))
 
         query = limit_query(query, project_ids)
 
diff --git a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb
index 415c610d74c..bad02e00a13 100644
--- a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb
+++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb
@@ -10,8 +10,7 @@ module Gitlab
                         issue_table[:iid],
                         issue_table[:id],
                         issue_table[:created_at],
-                        issue_table[:author_id],
-                        routes_table[:path]]
+                        issue_table[:author_id]]
 
         super(*args)
       end
diff --git a/lib/gitlab/cycle_analytics/plan_helper.rb b/lib/gitlab/cycle_analytics/plan_helper.rb
index 027573805a1..a63ae58ad21 100644
--- a/lib/gitlab/cycle_analytics/plan_helper.rb
+++ b/lib/gitlab/cycle_analytics/plan_helper.rb
@@ -9,14 +9,15 @@ module Gitlab
           .join(routes_table).on(projects_table[:namespace_id].eq(routes_table[:source_id]))
           .project(issue_table[:project_id].as("project_id"))
           .project(projects_table[:path].as("project_path"))
+          .project(routes_table[:path].as("namespace_path"))
           .where(issue_table[:project_id].in(project_ids))
           .where(routes_table[:source_type].eq('Namespace'))
-        query = add_conditions_to_query(query)
+        query = limit_query(query)
 
         query
       end
 
-      def add_conditions_to_query(query)
+      def limit_query(query)
         query.where(issue_table[:created_at].gteq(options[:from]))
           .where(issue_metrics_table[:first_added_to_board_at].not_eq(nil).or(issue_metrics_table[:first_associated_with_milestone_at].not_eq(nil)))
           .where(issue_metrics_table[:first_mentioned_in_commit_at].not_eq(nil))
diff --git a/lib/gitlab/cycle_analytics/review_event_fetcher.rb b/lib/gitlab/cycle_analytics/review_event_fetcher.rb
index 95b565cb31d..4b5d79097b7 100644
--- a/lib/gitlab/cycle_analytics/review_event_fetcher.rb
+++ b/lib/gitlab/cycle_analytics/review_event_fetcher.rb
@@ -11,8 +11,7 @@ module Gitlab
                         mr_table[:id],
                         mr_table[:created_at],
                         mr_table[:state],
-                        mr_table[:author_id],
-                        routes_table[:path]]
+                        mr_table[:author_id]]
 
         super(*args)
       end
diff --git a/spec/serializers/analytics_issue_entity_spec.rb b/spec/serializers/analytics_issue_entity_spec.rb
index 8061ee1fd68..c5b03bdd8c1 100644
--- a/spec/serializers/analytics_issue_entity_spec.rb
+++ b/spec/serializers/analytics_issue_entity_spec.rb
@@ -11,11 +11,11 @@ describe AnalyticsIssueEntity do
       created_at: "2016-11-12 15:04:02.948604",
       author: user,
       project_path: project.path,
-      path: project.namespace
+      namespace_path: project.namespace.route.path
     }
   end
 
-  let(:project) { create(:project) }
+  let(:project) { create(:project, name: 'my project') }
   let(:request) { EntityRequest.new(entity: :merge_request) }
 
   let(:entity) do
diff --git a/spec/serializers/analytics_issue_serializer_spec.rb b/spec/serializers/analytics_issue_serializer_spec.rb
index 2144d46d6a1..9cb2ce13d12 100644
--- a/spec/serializers/analytics_issue_serializer_spec.rb
+++ b/spec/serializers/analytics_issue_serializer_spec.rb
@@ -8,7 +8,7 @@ describe AnalyticsIssueSerializer do
   end
 
   let(:user) { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, name: 'my project') }
   let(:resource) do
     {
       total_time: "172802.724419",
@@ -18,7 +18,7 @@ describe AnalyticsIssueSerializer do
       created_at: "2016-11-12 15:04:02.948604",
       author: user,
       project_path: project.path,
-      path: project.namespace
+      namespace_path: project.namespace.route.path
     }
   end
 
diff --git a/spec/serializers/analytics_merge_request_serializer_spec.rb b/spec/serializers/analytics_merge_request_serializer_spec.rb
index e7e4f67ce5c..a864051b2a3 100644
--- a/spec/serializers/analytics_merge_request_serializer_spec.rb
+++ b/spec/serializers/analytics_merge_request_serializer_spec.rb
@@ -8,7 +8,7 @@ describe AnalyticsMergeRequestSerializer do
   end
 
   let(:user) { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, name: 'my project') }
   let(:resource) do
     {
       total_time: "172802.724419",
@@ -19,7 +19,7 @@ describe AnalyticsMergeRequestSerializer do
       created_at: "2016-11-12 15:04:02.948604",
       author: user,
       project_path: project.path,
-      path: project.namespace
+      namespace_path: project.namespace.route.path
     }
   end