From 68376f4b030004512e3e03e7c048fbfb640b845d Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Tue, 13 Aug 2024 01:43:17 +0400 Subject: [PATCH 1/2] Add Linked file feature Remove `pinned_file` feature flag Changelog: added - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/387246) in GitLab 16.9 [with a flag](../../administration/feature_flags.md) named `pinned_file`. Disabled by default. - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/433250#note_1751596958) in GitLab 16.9. - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/433250) in GitLab 17.4. Feature flag `pinned_file` removed. --- .../diffs/components/diff_row_utils.js | 4 +- .../projects/merge_requests_controller.rb | 5 +-- .../feature_flags/development/pinned_file.yml | 8 ---- .../diffs/components/diff_row_utils_spec.js | 45 +++++-------------- 4 files changed, 12 insertions(+), 50 deletions(-) delete mode 100644 config/feature_flags/development/pinned_file.yml diff --git a/app/assets/javascripts/diffs/components/diff_row_utils.js b/app/assets/javascripts/diffs/components/diff_row_utils.js index 6d2027b4961c8c..d6c64221ac751d 100644 --- a/app/assets/javascripts/diffs/components/diff_row_utils.js +++ b/app/assets/javascripts/diffs/components/diff_row_utils.js @@ -39,9 +39,7 @@ export const fileContentsId = (diffFile) => { const createDiffUrl = (diffFile) => { const url = new URL(window.location); - if (window?.gon?.features?.pinnedFile) { - url.searchParams.set('file', diffFile.file_hash); - } + url.searchParams.set('file', diffFile.file_hash); return url; }; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index fa3e4a8f31c2ab..a3aced96f93b30 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -44,7 +44,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:mr_pipelines_graphql, project) push_frontend_feature_flag(:ci_graphql_pipeline_mini_graph, project) push_frontend_feature_flag(:notifications_todos_buttons, current_user) - push_frontend_feature_flag(:pinned_file, project) push_frontend_feature_flag(:reviewer_assign_drawer, current_user) push_frontend_feature_flag(:async_merge_request_pipeline_creation, current_user) end @@ -458,9 +457,7 @@ def render_html_page @update_current_user_path = expose_path(api_v4_user_preferences_path) @endpoint_metadata_url = endpoint_metadata_url(@project, @merge_request) @endpoint_diff_batch_url = endpoint_diff_batch_url(@project, @merge_request) - if params[:file] && Feature.enabled?(:pinned_file, @project) - @linked_file_url = linked_file_url(@project, @merge_request) - end + @linked_file_url = linked_file_url(@project, @merge_request) if params[:file] if merge_request.diffs_batch_cache_with_max_age? @diffs_batch_cache_key = @merge_request.merge_head_diff&.patch_id_sha diff --git a/config/feature_flags/development/pinned_file.yml b/config/feature_flags/development/pinned_file.yml deleted file mode 100644 index 712a35fd790a4d..00000000000000 --- a/config/feature_flags/development/pinned_file.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: pinned_file -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/137544 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/433250 -milestone: '16.9' -type: development -group: group::code review -default_enabled: false diff --git a/spec/frontend/diffs/components/diff_row_utils_spec.js b/spec/frontend/diffs/components/diff_row_utils_spec.js index 3c9f22351ce490..0003867cc2243a 100644 --- a/spec/frontend/diffs/components/diff_row_utils_spec.js +++ b/spec/frontend/diffs/components/diff_row_utils_spec.js @@ -129,14 +129,6 @@ describe('diff_row_utils', () => { }); describe('linked file enabled', () => { - beforeEach(() => { - window.gon.features = { pinnedFile: true }; - }); - - afterEach(() => { - delete window.gon.features; - }); - it(`should return linked file URL`, () => { const diffFile = getDiffFileMock(); expect(utils.lineHref({ line_code: LINE_CODE }, diffFile)).toContain( @@ -147,37 +139,20 @@ describe('diff_row_utils', () => { }); describe('createFileUrl', () => { - describe('linked file enabled', () => { - beforeEach(() => { - window.gon.features = { pinnedFile: true }; - }); - - afterEach(() => { - delete window.gon.features; - }); - - it(`should return linked file URL`, () => { - const diffFile = getDiffFileMock(); - const url = utils.createFileUrl(diffFile); - expect(url.searchParams.get('file')).toBe(diffFile.file_hash); - expect(url.hash).toBe(`#diff-content-${diffFile.file_hash}`); - }); - - it(`removes existing linked file search param`, () => { - const newLocation = new URL(window.location); - newLocation.searchParams.append('file', 'foo'); - setWindowLocation(newLocation.toString()); - const diffFile = getDiffFileMock(); - const url = utils.createFileUrl(diffFile); - expect(url.searchParams.get('file')).toBe(diffFile.file_hash); - expect(url.hash).toBe(`#diff-content-${diffFile.file_hash}`); - }); + it(`should return linked file URL`, () => { + const diffFile = getDiffFileMock(); + const url = utils.createFileUrl(diffFile); + expect(url.searchParams.get('file')).toBe(diffFile.file_hash); + expect(url.hash).toBe(`#diff-content-${diffFile.file_hash}`); }); - it('should return file URL', () => { + it(`removes existing linked file search param`, () => { + const newLocation = new URL(window.location); + newLocation.searchParams.append('file', 'foo'); + setWindowLocation(newLocation.toString()); const diffFile = getDiffFileMock(); const url = utils.createFileUrl(diffFile); - expect(url.searchParams.get('file')).toBe(null); + expect(url.searchParams.get('file')).toBe(diffFile.file_hash); expect(url.hash).toBe(`#diff-content-${diffFile.file_hash}`); }); }); -- GitLab From aef301b922ad34493cfd7600546a4df5a020aa1d Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Fri, 23 Aug 2024 09:56:00 +0400 Subject: [PATCH 2/2] Update docs --- doc/user/project/merge_requests/changes.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/doc/user/project/merge_requests/changes.md b/doc/user/project/merge_requests/changes.md index 738c48c4f403c9..bd7c330c972ef9 100644 --- a/doc/user/project/merge_requests/changes.md +++ b/doc/user/project/merge_requests/changes.md @@ -81,10 +81,7 @@ DETAILS: **Offering:** GitLab.com > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/387246) in GitLab 16.9 [with a flag](../../../administration/feature_flags.md) named `pinned_file`. Disabled by default. - -FLAG: -The availability of this feature is controlled by a feature flag. -For more information, see the history. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/162503) in GitLab 17.4. Feature flag `pinned_file` removed. When you share a merge request link with a team member, you might want to show a specific file first in the list of changed files. To copy a merge request link that shows your desired file first: -- GitLab