Commit 08882689 authored by Niko Belokolodov's avatar Niko Belokolodov Committed by Nikolay Belokolodov

Alter masked_page_url to accept group and project

Refactoring masked_page_url  to accept arguments explicitly
instead of relying on controllers fields. This chage imrooves
readability and helps prevent bugs similar to
https://gitlab.com/gitlab-org/gitlab/-/issues/345417
parent 4fa116b0
...@@ -69,12 +69,10 @@ module Routing ...@@ -69,12 +69,10 @@ module Routing
end end
end end
def masked_page_url def masked_page_url(group:, project:)
return unless Feature.enabled?(:mask_page_urls, type: :ops) return unless Feature.enabled?(:mask_page_urls, type: :ops)
current_group = group if defined?(group) mask_helper = MaskHelper.new(request, group, project)
current_project = project if defined?(project)
mask_helper = MaskHelper.new(request, current_group, current_project)
mask_helper.mask_params mask_helper.mask_params
# We rescue all exception for time being till we test this helper extensively. # We rescue all exception for time being till we test this helper extensively.
......
- return unless Gitlab::Tracking.enabled? - return unless Gitlab::Tracking.enabled?
- namespace = @group || @project&.namespace
= javascript_tag do = javascript_tag do
:plain :plain
;(function(p,l,o,w,i,n,g){if(!p[i]){p.GlobalSnowplowNamespace=p.GlobalSnowplowNamespace||[]; ;(function(p,l,o,w,i,n,g){if(!p[i]){p.GlobalSnowplowNamespace=p.GlobalSnowplowNamespace||[];
...@@ -10,6 +12,6 @@ ...@@ -10,6 +12,6 @@
window.snowplowOptions = #{Gitlab::Tracking.options(@group).to_json} window.snowplowOptions = #{Gitlab::Tracking.options(@group).to_json}
gl = window.gl || {}; gl = window.gl || {};
gl.snowplowStandardContext = #{Gitlab::Tracking::StandardContext.new(namespace: @group || @project&.namespace, gl.snowplowStandardContext = #{Gitlab::Tracking::StandardContext.new(namespace: namespace,
project: @project, user: current_user).to_context.to_json.to_json} project: @project, user: current_user).to_context.to_json.to_json}
gl.snowplowPseudonymizedPageUrl = #{masked_page_url.to_json}; gl.snowplowPseudonymizedPageUrl = #{masked_page_url(group: namespace, project: @project).to_json};
...@@ -14,7 +14,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do ...@@ -14,7 +14,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do
shared_examples 'masked url' do shared_examples 'masked url' do
it 'generates masked page url' do it 'generates masked page url' do
expect(helper.masked_page_url).to eq(masked_url) expect(helper.masked_page_url(group: group, project: project)).to eq(masked_url)
end end
end end
......
...@@ -11,15 +11,15 @@ RSpec.describe ::Routing::PseudonymizationHelper do ...@@ -11,15 +11,15 @@ RSpec.describe ::Routing::PseudonymizationHelper do
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request, source_project: project) }
let(:subject) { helper.masked_page_url(group: group, project: project) }
before do before do
stub_feature_flags(mask_page_urls: true) stub_feature_flags(mask_page_urls: true)
allow(helper).to receive(:group).and_return(group)
allow(helper).to receive(:project).and_return(project)
end end
shared_examples 'masked url' do shared_examples 'masked url' do
it 'generates masked page url' do it 'generates masked page url' do
expect(helper.masked_page_url).to eq(masked_url) expect(subject).to eq(masked_url)
end end
end end
...@@ -72,6 +72,8 @@ RSpec.describe ::Routing::PseudonymizationHelper do ...@@ -72,6 +72,8 @@ RSpec.describe ::Routing::PseudonymizationHelper do
context 'with controller for groups with subgroups and project' do context 'with controller for groups with subgroups and project' do
let(:masked_url) { "http://localhost/namespace#{subgroup.id}/project#{subproject.id}"} let(:masked_url) { "http://localhost/namespace#{subgroup.id}/project#{subproject.id}"}
let(:group) { subgroup }
let(:project) { subproject }
let(:request) do let(:request) do
double(:Request, double(:Request,
path_parameters: { path_parameters: {
...@@ -86,8 +88,6 @@ RSpec.describe ::Routing::PseudonymizationHelper do ...@@ -86,8 +88,6 @@ RSpec.describe ::Routing::PseudonymizationHelper do
end end
before do before do
allow(helper).to receive(:group).and_return(subgroup)
allow(helper).to receive(:project).and_return(subproject)
allow(helper).to receive(:request).and_return(request) allow(helper).to receive(:request).and_return(request)
end end
...@@ -96,6 +96,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do ...@@ -96,6 +96,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do
context 'with controller for groups and subgroups' do context 'with controller for groups and subgroups' do
let(:masked_url) { "http://localhost/groups/namespace#{subgroup.id}/-/shared"} let(:masked_url) { "http://localhost/groups/namespace#{subgroup.id}/-/shared"}
let(:group) { subgroup }
let(:request) do let(:request) do
double(:Request, double(:Request,
path_parameters: { path_parameters: {
...@@ -109,7 +110,6 @@ RSpec.describe ::Routing::PseudonymizationHelper do ...@@ -109,7 +110,6 @@ RSpec.describe ::Routing::PseudonymizationHelper do
end end
before do before do
allow(helper).to receive(:group).and_return(subgroup)
allow(helper).to receive(:request).and_return(request) allow(helper).to receive(:request).and_return(request)
end end
...@@ -230,7 +230,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do ...@@ -230,7 +230,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do
end end
it 'masked_page_url' do it 'masked_page_url' do
expect(helper.masked_page_url).to eq(root_url) expect(subject).to eq(root_url)
end end
end end
end end
...@@ -262,7 +262,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do ...@@ -262,7 +262,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do
ActionController::RoutingError, ActionController::RoutingError,
url: '/dashboard/issues?assignee_username=root').and_call_original url: '/dashboard/issues?assignee_username=root').and_call_original
expect(helper.masked_page_url).to be_nil expect(subject).to be_nil
end end
end end
end end
...@@ -273,7 +273,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do ...@@ -273,7 +273,7 @@ RSpec.describe ::Routing::PseudonymizationHelper do
end end
it 'returns nil' do it 'returns nil' do
expect(helper.masked_page_url).to be_nil expect(subject).to be_nil
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment