Commit 64a05074 authored by Igor Drozdov's avatar Igor Drozdov

Remove merge_ref_head_comments FF

The feature flag has been enabled for a while without issues
We can remove it now
parent 8a5e9d54
import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils';
import { __ } from '~/locale';
import {
MATCH_LINE_TYPE,
......@@ -23,21 +22,8 @@ export const isMatchLine = type => type === MATCH_LINE_TYPE;
export const isMetaLine = type =>
[OLD_NO_NEW_LINE_TYPE, NEW_NO_NEW_LINE_TYPE, EMPTY_CELL_TYPE].includes(type);
export const shouldRenderCommentButton = (
isLoggedIn,
isCommentButtonRendered,
featureMergeRefHeadComments = false,
) => {
if (!isCommentButtonRendered) {
return false;
}
if (isLoggedIn) {
const isDiffHead = parseBoolean(getParameterByName('diff_head'));
return !isDiffHead || featureMergeRefHeadComments;
}
return false;
export const shouldRenderCommentButton = (isLoggedIn, isCommentButtonRendered) => {
return isCommentButtonRendered && isLoggedIn;
};
export const hasDiscussions = line => line?.discussions?.length > 0;
......
......@@ -81,11 +81,7 @@ export default {
return utils.addCommentTooltip(this.line);
},
shouldRenderCommentButton() {
return utils.shouldRenderCommentButton(
this.isLoggedIn,
true,
gon.features?.mergeRefHeadComments,
);
return utils.shouldRenderCommentButton(this.isLoggedIn, true);
},
shouldShowCommentButton() {
return utils.shouldShowCommentButton(
......
......@@ -102,11 +102,7 @@ export default {
return utils.addCommentTooltip(this.line.right);
},
shouldRenderCommentButton() {
return utils.shouldRenderCommentButton(
this.isLoggedIn,
this.isCommentButtonRendered,
gon.features?.mergeRefHeadComments,
);
return utils.shouldRenderCommentButton(this.isLoggedIn, this.isCommentButtonRendered);
},
shouldShowCommentButtonLeft() {
return utils.shouldShowCommentButton(
......
......@@ -173,7 +173,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
end
def update_diff_discussion_positions!
return unless Feature.enabled?(:merge_ref_head_comments, @merge_request.target_project, default_enabled: true)
return unless Feature.enabled?(:merge_red_head_comments_position_on_demand, @merge_request.target_project, default_enabled: true)
return if @merge_request.has_any_diff_note_positions?
......
......@@ -29,7 +29,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
before_action only: [:show] do
push_frontend_experiment(:suggest_pipeline)
push_frontend_feature_flag(:widget_visibility_polling, @project, default_enabled: true)
push_frontend_feature_flag(:merge_ref_head_comments, @project, default_enabled: true)
push_frontend_feature_flag(:mr_commit_neighbor_nav, @project, default_enabled: true)
push_frontend_feature_flag(:multiline_comments, @project, default_enabled: true)
push_frontend_feature_flag(:file_identifier_hash)
......
......@@ -69,9 +69,6 @@ class DiscussionEntity < Grape::Entity
end
def display_merge_ref_discussions?(discussion)
return unless discussion.diff_discussion?
return if discussion.legacy_diff_discussion?
Feature.enabled?(:merge_ref_head_comments, discussion.project, default_enabled: true)
discussion.diff_discussion? && !discussion.legacy_diff_discussion?
end
end
......@@ -125,8 +125,6 @@ module MergeRequests
end
def update_diff_discussion_positions!
return if Feature.disabled?(:merge_ref_head_comments, merge_request.target_project, default_enabled: true)
Discussions::CaptureDiffNotePositionsService.new(merge_request).execute
end
......
......@@ -70,7 +70,7 @@ module Notes
Gitlab::Tracking.event('Notes::CreateService', 'execute', tracking_data_for(note))
end
if Feature.enabled?(:merge_ref_head_comments, project, default_enabled: true) && note.for_merge_request? && note.diff_note? && note.start_of_discussion?
if note.for_merge_request? && note.diff_note? && note.start_of_discussion?
Discussions::CaptureDiffNotePositionService.new(note.noteable, note.diff_file&.paths).execute(note.discussion)
end
end
......
---
name: merge_ref_head_comments
introduced_by_url:
rollout_issue_url:
group:
type: development
default_enabled: true
import { shallowMount } from '@vue/test-utils';
import { TEST_HOST } from 'helpers/test_constants';
import { createStore } from '~/mr_notes/stores';
import InlineDiffTableRow from '~/diffs/components/inline_diff_table_row.vue';
import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue';
......@@ -28,13 +27,6 @@ describe('InlineDiffTableRow', () => {
});
};
const setWindowLocation = value => {
Object.defineProperty(window, 'location', {
writable: true,
value,
});
};
beforeEach(() => {
store = createStore();
store.state.notes.userData = TEST_USER;
......@@ -122,22 +114,15 @@ describe('InlineDiffTableRow', () => {
const findNoteButton = () => wrapper.find({ ref: 'addDiffNoteButton' });
it.each`
userData | query | mergeRefHeadComments | expectation
${TEST_USER} | ${'diff_head=false'} | ${false} | ${true}
${TEST_USER} | ${'diff_head=true'} | ${true} | ${true}
${TEST_USER} | ${'diff_head=true'} | ${false} | ${false}
${null} | ${''} | ${true} | ${false}
`(
'exists is $expectation - with userData ($userData) query ($query)',
({ userData, query, mergeRefHeadComments, expectation }) => {
store.state.notes.userData = userData;
gon.features = { mergeRefHeadComments };
setWindowLocation({ href: `${TEST_HOST}?${query}` });
createComponent({}, store);
expect(findNoteButton().exists()).toBe(expectation);
},
);
userData | expectation
${TEST_USER} | ${true}
${null} | ${false}
`('exists is $expectation - with userData ($userData)', ({ userData, expectation }) => {
store.state.notes.userData = userData;
createComponent({}, store);
expect(findNoteButton().exists()).toBe(expectation);
});
it.each`
isHover | line | expectation
......
import Vue from 'vue';
import { shallowMount } from '@vue/test-utils';
import { createComponentWithStore } from 'helpers/vue_mount_component_helper';
import { TEST_HOST } from 'helpers/test_constants';
import { createStore } from '~/mr_notes/stores';
import ParallelDiffTableRow from '~/diffs/components/parallel_diff_table_row.vue';
import diffFileMockData from '../mock_data/diff_file';
......@@ -186,13 +185,6 @@ describe('ParallelDiffTableRow', () => {
});
};
const setWindowLocation = value => {
Object.defineProperty(window, 'location', {
writable: true,
value,
});
};
beforeEach(() => {
// eslint-disable-next-line prefer-destructuring
thisLine = diffFileMockData.parallel_diff_lines[2];
......@@ -228,19 +220,15 @@ describe('ParallelDiffTableRow', () => {
const findNoteButton = () => wrapper.find({ ref: 'addDiffNoteButtonLeft' });
it.each`
hover | line | userData | query | mergeRefHeadComments | expectation
${true} | ${{}} | ${TEST_USER} | ${'diff_head=false'} | ${false} | ${true}
${true} | ${{ line: { left: null } }} | ${TEST_USER} | ${'diff_head=false'} | ${false} | ${false}
${true} | ${{}} | ${TEST_USER} | ${'diff_head=true'} | ${true} | ${true}
${true} | ${{}} | ${TEST_USER} | ${'diff_head=true'} | ${false} | ${false}
${true} | ${{}} | ${null} | ${''} | ${true} | ${false}
${false} | ${{}} | ${TEST_USER} | ${'diff_head=false'} | ${false} | ${false}
hover | line | userData | expectation
${true} | ${{}} | ${TEST_USER} | ${true}
${true} | ${{ line: { left: null } }} | ${TEST_USER} | ${false}
${true} | ${{}} | ${null} | ${false}
${false} | ${{}} | ${TEST_USER} | ${false}
`(
'exists is $expectation - with userData ($userData) query ($query)',
async ({ hover, line, userData, query, mergeRefHeadComments, expectation }) => {
'exists is $expectation - with userData ($userData)',
async ({ hover, line, userData, expectation }) => {
store.state.notes.userData = userData;
gon.features = { mergeRefHeadComments };
setWindowLocation({ href: `${TEST_HOST}?${query}` });
createComponent(line, store);
if (hover) await wrapper.find('.line_holder').trigger('mouseover');
......
......@@ -79,13 +79,5 @@ RSpec.describe DiscussionEntity do
:active
)
end
context 'diff_head_compare feature is disabled' do
it 'does not expose positions and line_codes attributes' do
stub_feature_flags(merge_ref_head_comments: false)
expect(subject.keys).not_to include(:positions, :line_codes)
end
end
end
end
......@@ -41,16 +41,6 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
subject
end
context 'when merge_ref_head_comments is disabled' do
it 'does not update diff discussion positions' do
stub_feature_flags(merge_ref_head_comments: false)
expect(Discussions::CaptureDiffNotePositionsService).not_to receive(:new)
subject
end
end
it 'updates the merge ref' do
expect { subject }.to change(merge_request, :merge_ref_head).from(nil)
end
......
......@@ -163,14 +163,6 @@ RSpec.describe Notes::CreateService do
expect(note.note_diff_file).to be_present
expect(note.diff_note_positions).to be_present
end
it 'does not create diff positions merge_ref_head_comments is disabled' do
stub_feature_flags(merge_ref_head_comments: false)
expect(Discussions::CaptureDiffNotePositionService).not_to receive(:new)
described_class.new(project_with_repo, user, new_opts).execute
end
end
context 'when DiffNote is a reply' do
......
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