summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Thomas <nick@gitlab.com>2019-04-03 13:54:05 +0000
committerNick Thomas <nick@gitlab.com>2019-04-03 13:54:05 +0000
commitdb12e729a7f0615ccb2fd8cd3209bddc3f7bfe39 (patch)
tree8cae9f6f1a184bcbc8c99c015d8390a5c0556ced
parentdd7e927b38814c54cdaa11e8dd09ce6067615509 (diff)
parentc17b7afa0298beb3290d46ad504f0f4dc4508d7a (diff)
downloadgitlab-ce-db12e729a7f0615ccb2fd8cd3209bddc3f7bfe39.tar.gz
Merge branch '58644-remove-reply_to_individual_notes-feature-flag' into 'master'
Remove reply_to_individual_notes feature flag Closes #58644 See merge request gitlab-org/gitlab-ce!26889
-rw-r--r--app/assets/javascripts/notes/components/note_actions.vue5
-rw-r--r--app/controllers/concerns/issuable_actions.rb3
-rw-r--r--app/models/individual_note_discussion.rb2
-rw-r--r--spec/features/merge_request/user_posts_notes_spec.rb13
-rw-r--r--spec/javascripts/notes/components/note_actions_spec.js82
-rw-r--r--spec/services/notes/build_service_spec.rb36
-rw-r--r--spec/services/notes/create_service_spec.rb41
7 files changed, 36 insertions, 146 deletions
diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue
index fc73726857d..aabb77f6a85 100644
--- a/app/assets/javascripts/notes/components/note_actions.vue
+++ b/app/assets/javascripts/notes/components/note_actions.vue
@@ -86,9 +86,6 @@ export default {
},
computed: {
...mapGetters(['getUserDataByProp']),
- showReplyButton() {
- return gon.features && gon.features.replyToIndividualNotes && this.showReply;
- },
shouldShowActionsDropdown() {
return this.currentUserId && (this.canEdit || this.canReportAsAbuse);
},
@@ -167,7 +164,7 @@ export default {
</a>
</div>
<reply-button
- v-if="showReplyButton"
+ v-if="showReply"
ref="replyButton"
class="js-reply-button"
@startReplying="$emit('startReplying')"
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index 05d88429cfe..8ef3b6502df 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -7,9 +7,6 @@ module IssuableActions
included do
before_action :authorize_destroy_issuable!, only: :destroy
before_action :authorize_admin_issuable!, only: :bulk_update
- before_action only: :show do
- push_frontend_feature_flag(:reply_to_individual_notes, default_enabled: true)
- end
end
def permitted_keys
diff --git a/app/models/individual_note_discussion.rb b/app/models/individual_note_discussion.rb
index 3b6b68a9c5f..d926e39f96e 100644
--- a/app/models/individual_note_discussion.rb
+++ b/app/models/individual_note_discussion.rb
@@ -14,7 +14,7 @@ class IndividualNoteDiscussion < Discussion
end
def can_convert_to_discussion?
- noteable.supports_replying_to_individual_notes? && Feature.enabled?(:reply_to_individual_notes, default_enabled: true)
+ noteable.supports_replying_to_individual_notes?
end
def convert_to_discussion!(save: false)
diff --git a/spec/features/merge_request/user_posts_notes_spec.rb b/spec/features/merge_request/user_posts_notes_spec.rb
index dc0862be6fc..e5770905dbd 100644
--- a/spec/features/merge_request/user_posts_notes_spec.rb
+++ b/spec/features/merge_request/user_posts_notes_spec.rb
@@ -67,18 +67,7 @@ describe 'Merge request > User posts notes', :js do
end
end
- describe 'when reply_to_individual_notes feature flag is disabled' do
- before do
- stub_feature_flags(reply_to_individual_notes: false)
- visit project_merge_request_path(project, merge_request)
- end
-
- it 'does not show a reply button' do
- expect(page).to have_no_selector('.js-reply-button')
- end
- end
-
- describe 'when reply_to_individual_notes feature flag is not set' do
+ describe 'reply button' do
before do
visit project_merge_request_path(project, merge_request)
end
diff --git a/spec/javascripts/notes/components/note_actions_spec.js b/spec/javascripts/notes/components/note_actions_spec.js
index d604e90b529..0cfcc994234 100644
--- a/spec/javascripts/notes/components/note_actions_spec.js
+++ b/spec/javascripts/notes/components/note_actions_spec.js
@@ -128,87 +128,33 @@ describe('noteActions', () => {
});
});
- describe('with feature flag replyToIndividualNotes enabled', () => {
+ describe('for showReply = true', () => {
beforeEach(() => {
- gon.features = {
- replyToIndividualNotes: true,
- };
- });
-
- afterEach(() => {
- gon.features = {};
- });
-
- describe('for showReply = true', () => {
- beforeEach(() => {
- wrapper = shallowMountNoteActions({
- ...props,
- showReply: true,
- });
- });
-
- it('shows a reply button', () => {
- const replyButton = wrapper.find({ ref: 'replyButton' });
-
- expect(replyButton.exists()).toBe(true);
+ wrapper = shallowMountNoteActions({
+ ...props,
+ showReply: true,
});
});
- describe('for showReply = false', () => {
- beforeEach(() => {
- wrapper = shallowMountNoteActions({
- ...props,
- showReply: false,
- });
- });
-
- it('does not show a reply button', () => {
- const replyButton = wrapper.find({ ref: 'replyButton' });
+ it('shows a reply button', () => {
+ const replyButton = wrapper.find({ ref: 'replyButton' });
- expect(replyButton.exists()).toBe(false);
- });
+ expect(replyButton.exists()).toBe(true);
});
});
- describe('with feature flag replyToIndividualNotes disabled', () => {
+ describe('for showReply = false', () => {
beforeEach(() => {
- gon.features = {
- replyToIndividualNotes: false,
- };
- });
-
- afterEach(() => {
- gon.features = {};
- });
-
- describe('for showReply = true', () => {
- beforeEach(() => {
- wrapper = shallowMountNoteActions({
- ...props,
- showReply: true,
- });
- });
-
- it('does not show a reply button', () => {
- const replyButton = wrapper.find({ ref: 'replyButton' });
-
- expect(replyButton.exists()).toBe(false);
+ wrapper = shallowMountNoteActions({
+ ...props,
+ showReply: false,
});
});
- describe('for showReply = false', () => {
- beforeEach(() => {
- wrapper = shallowMountNoteActions({
- ...props,
- showReply: false,
- });
- });
-
- it('does not show a reply button', () => {
- const replyButton = wrapper.find({ ref: 'replyButton' });
+ it('does not show a reply button', () => {
+ const replyButton = wrapper.find({ ref: 'replyButton' });
- expect(replyButton.exists()).toBe(false);
- });
+ expect(replyButton.exists()).toBe(false);
});
});
});
diff --git a/spec/services/notes/build_service_spec.rb b/spec/services/notes/build_service_spec.rb
index af4daff336b..96fff20f7fb 100644
--- a/spec/services/notes/build_service_spec.rb
+++ b/spec/services/notes/build_service_spec.rb
@@ -128,37 +128,19 @@ describe Notes::BuildService do
subject { described_class.new(project, author, note: 'Test', in_reply_to_discussion_id: note.discussion_id).execute }
- shared_examples 'an individual note reply' do
- it 'builds another individual note' do
- expect(subject).to be_valid
- expect(subject).to be_a(Note)
- expect(subject.discussion_id).not_to eq(note.discussion_id)
- end
+ it 'sets the note up to be in reply to that note' do
+ expect(subject).to be_valid
+ expect(subject).to be_a(DiscussionNote)
+ expect(subject.discussion_id).to eq(note.discussion_id)
end
- context 'when reply_to_individual_notes is disabled' do
- before do
- stub_feature_flags(reply_to_individual_notes: false)
- end
-
- it_behaves_like 'an individual note reply'
- end
+ context 'when noteable does not support replies' do
+ let(:note) { create(:note_on_commit) }
- context 'when reply_to_individual_notes is enabled' do
- before do
- stub_feature_flags(reply_to_individual_notes: true)
- end
-
- it 'sets the note up to be in reply to that note' do
+ it 'builds another individual note' do
expect(subject).to be_valid
- expect(subject).to be_a(DiscussionNote)
- expect(subject.discussion_id).to eq(note.discussion_id)
- end
-
- context 'when noteable does not support replies' do
- let(:note) { create(:note_on_commit) }
-
- it_behaves_like 'an individual note reply'
+ expect(subject).to be_a(Note)
+ expect(subject.discussion_id).not_to eq(note.discussion_id)
end
end
end
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 8d8e81173ff..bcbb8950910 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -298,41 +298,20 @@ describe Notes::CreateService do
subject { described_class.new(project, user, reply_opts).execute }
- context 'when reply_to_individual_notes is disabled' do
- before do
- stub_feature_flags(reply_to_individual_notes: false)
- end
-
- it 'creates an individual note' do
- expect(subject.type).to eq(nil)
- expect(subject.discussion_id).not_to eq(existing_note.discussion_id)
- end
-
- it 'does not convert existing note' do
- expect { subject }.not_to change { existing_note.reload.type }
- end
+ it 'creates a DiscussionNote in reply to existing note' do
+ expect(subject).to be_a(DiscussionNote)
+ expect(subject.discussion_id).to eq(existing_note.discussion_id)
end
- context 'when reply_to_individual_notes is enabled' do
- before do
- stub_feature_flags(reply_to_individual_notes: true)
- end
-
- it 'creates a DiscussionNote in reply to existing note' do
- expect(subject).to be_a(DiscussionNote)
- expect(subject.discussion_id).to eq(existing_note.discussion_id)
- end
-
- it 'converts existing note to DiscussionNote' do
- expect do
- existing_note
+ it 'converts existing note to DiscussionNote' do
+ expect do
+ existing_note
- Timecop.freeze(Time.now + 1.minute) { subject }
+ Timecop.freeze(Time.now + 1.minute) { subject }
- existing_note.reload
- end.to change { existing_note.type }.from(nil).to('DiscussionNote')
- .and change { existing_note.updated_at }
- end
+ existing_note.reload
+ end.to change { existing_note.type }.from(nil).to('DiscussionNote')
+ .and change { existing_note.updated_at }
end
end
end