From 6c5e9480130308abf9458f1ac58e25695c5ce2ea Mon Sep 17 00:00:00 2001 From: Gosia Ksionek Date: Mon, 5 Aug 2019 21:15:00 +0000 Subject: Fix error on project name Add project path to sql query to build proper path --- app/serializers/analytics_issue_entity.rb | 2 +- .../fix-name-vs-path-problem-for-cycle-analytics.yml | 5 +++++ lib/gitlab/cycle_analytics/base_query.rb | 13 ++++++++++--- lib/gitlab/cycle_analytics/code_event_fetcher.rb | 4 +--- lib/gitlab/cycle_analytics/issue_event_fetcher.rb | 4 +--- lib/gitlab/cycle_analytics/issue_helper.rb | 13 ++++++++++--- lib/gitlab/cycle_analytics/plan_event_fetcher.rb | 4 +--- lib/gitlab/cycle_analytics/plan_helper.rb | 6 ++++-- lib/gitlab/cycle_analytics/production_event_fetcher.rb | 1 - lib/gitlab/cycle_analytics/review_event_fetcher.rb | 4 +--- spec/serializers/analytics_issue_entity_spec.rb | 6 +++--- spec/serializers/analytics_issue_serializer_spec.rb | 6 +++--- spec/serializers/analytics_merge_request_serializer_spec.rb | 6 +++--- 13 files changed, 43 insertions(+), 31 deletions(-) create mode 100644 changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml diff --git a/app/serializers/analytics_issue_entity.rb b/app/serializers/analytics_issue_entity.rb index 29d4a6ae1d0..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[:name], 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 new file mode 100644 index 00000000000..7d171c2cf5b --- /dev/null +++ b/changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml @@ -0,0 +1,5 @@ +--- +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 9c98c0bfbf2..459bb5177b5 100644 --- a/lib/gitlab/cycle_analytics/base_query.rb +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -19,9 +19,10 @@ module Gitlab .join(projects_table).on(issue_table[:project_id].eq(projects_table[:id])) .join(routes_table).on(projects_table[:namespace_id].eq(routes_table[:source_id])) .project(issue_table[:project_id].as("project_id")) - .where(issue_table[:project_id].in(project_ids)) - .where(routes_table[:source_type].eq('Namespace')) - .where(issue_table[:created_at].gteq(options[:from])) + .project(projects_table[:path].as("project_path")) + .project(routes_table[:path].as("namespace_path")) + + query = limit_query(query, project_ids) # Load merge_requests @@ -30,6 +31,12 @@ module Gitlab query end + def limit_query(query, project_ids) + query.where(issue_table[:project_id].in(project_ids)) + .where(routes_table[:source_type].eq('Namespace')) + .where(issue_table[:created_at].gteq(options[:from])) + end + def load_merge_requests(query) query.join(mr_table, Arel::Nodes::OuterJoin) .on(mr_table[:id].eq(mr_closing_issues_table[:merge_request_id])) diff --git a/lib/gitlab/cycle_analytics/code_event_fetcher.rb b/lib/gitlab/cycle_analytics/code_event_fetcher.rb index 1e4e9b9e02c..fcc282bf7a6 100644 --- a/lib/gitlab/cycle_analytics/code_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/code_event_fetcher.rb @@ -11,9 +11,7 @@ module Gitlab mr_table[:id], mr_table[:created_at], mr_table[:state], - mr_table[:author_id], - projects_table[:name], - 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 2d03e425a6a..6914cf24c19 100644 --- a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb @@ -10,9 +10,7 @@ module Gitlab issue_table[:iid], issue_table[:id], issue_table[:created_at], - issue_table[:author_id], - projects_table[:name], - 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 0fc4f1dd41a..295eca5edca 100644 --- a/lib/gitlab/cycle_analytics/issue_helper.rb +++ b/lib/gitlab/cycle_analytics/issue_helper.rb @@ -8,12 +8,19 @@ module Gitlab .join(projects_table).on(issue_table[:project_id].eq(projects_table[:id])) .join(routes_table).on(projects_table[:namespace_id].eq(routes_table[:source_id])) .project(issue_table[:project_id].as("project_id")) - .where(issue_table[:project_id].in(project_ids)) + .project(projects_table[:path].as("project_path")) + .project(routes_table[:path].as("namespace_path")) + + query = limit_query(query, project_ids) + + query + end + + def limit_query(query, project_ids) + query.where(issue_table[:project_id].in(project_ids)) .where(routes_table[:source_type].eq('Namespace')) .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))) - - query end end end diff --git a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb index 77cc358daa9..bad02e00a13 100644 --- a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb @@ -10,9 +10,7 @@ module Gitlab issue_table[:iid], issue_table[:id], issue_table[:created_at], - issue_table[:author_id], - projects_table[:name], - 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 c3f742503a9..a63ae58ad21 100644 --- a/lib/gitlab/cycle_analytics/plan_helper.rb +++ b/lib/gitlab/cycle_analytics/plan_helper.rb @@ -8,14 +8,16 @@ module Gitlab .join(projects_table).on(issue_table[:project_id].eq(projects_table[:id])) .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/production_event_fetcher.rb b/lib/gitlab/cycle_analytics/production_event_fetcher.rb index 404b2460814..8843ab2bcb9 100644 --- a/lib/gitlab/cycle_analytics/production_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/production_event_fetcher.rb @@ -11,7 +11,6 @@ module Gitlab issue_table[:id], issue_table[:created_at], issue_table[:author_id], - projects_table[:name], routes_table[:path]] super(*args) diff --git a/lib/gitlab/cycle_analytics/review_event_fetcher.rb b/lib/gitlab/cycle_analytics/review_event_fetcher.rb index 6acd12517fa..4b5d79097b7 100644 --- a/lib/gitlab/cycle_analytics/review_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/review_event_fetcher.rb @@ -11,9 +11,7 @@ module Gitlab mr_table[:id], mr_table[:created_at], mr_table[:state], - mr_table[:author_id], - projects_table[:name], - 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 dd5e43a4b62..c5b03bdd8c1 100644 --- a/spec/serializers/analytics_issue_entity_spec.rb +++ b/spec/serializers/analytics_issue_entity_spec.rb @@ -10,12 +10,12 @@ describe AnalyticsIssueEntity do id: "1", created_at: "2016-11-12 15:04:02.948604", author: user, - name: project.name, - path: project.namespace + project_path: project.path, + 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 c9ffe1c5dad..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", @@ -17,8 +17,8 @@ describe AnalyticsIssueSerializer do id: "1", created_at: "2016-11-12 15:04:02.948604", author: user, - name: project.name, - path: project.namespace + project_path: project.path, + 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 123d7d795ce..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", @@ -18,8 +18,8 @@ describe AnalyticsMergeRequestSerializer do state: 'open', created_at: "2016-11-12 15:04:02.948604", author: user, - name: project.name, - path: project.namespace + project_path: project.path, + namespace_path: project.namespace.route.path } end -- cgit v1.2.1