summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
authorOswaldo Ferreira <oswaldo@gitlab.com>2018-12-04 20:17:10 -0200
committerStan Hu <stanhu@gmail.com>2018-12-06 17:07:49 -0800
commit26b94bcc7d510482eea253b02dcc4ff22c48d68a (patch)
tree68a0f8dddb009b03af5de530fe4feb94a289dc6d /spec
parent9a78524f465488b2b5daa3fbdb2ec7c71c9641a6 (diff)
downloadgitlab-ce-26b94bcc7d510482eea253b02dcc4ff22c48d68a.tar.gz
Remove unused data from discussions endpoint
We don't need a series of attributes to render diff files on discussions.json request. Therefore this MR removes lots of unnecessary attributes from the request, mainly the highlighted diff lines, which are pretty expensive.
Diffstat (limited to 'spec')
-rw-r--r--spec/javascripts/diffs/mock_data/diff_discussions.js2
-rw-r--r--spec/serializers/diff_file_entity_spec.rb33
-rw-r--r--spec/serializers/discussion_diff_file_entity_spec.rb39
-rw-r--r--spec/serializers/discussion_entity_spec.rb8
-rw-r--r--spec/support/shared_examples/serializers/diff_file_entity_examples.rb46
5 files changed, 85 insertions, 43 deletions
diff --git a/spec/javascripts/diffs/mock_data/diff_discussions.js b/spec/javascripts/diffs/mock_data/diff_discussions.js
index 5ffe5a366ba..44313caba29 100644
--- a/spec/javascripts/diffs/mock_data/diff_discussions.js
+++ b/spec/javascripts/diffs/mock_data/diff_discussions.js
@@ -489,8 +489,6 @@ export default {
diff_discussion: true,
truncated_diff_lines:
'<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="1">\n1\n</td>\n<td class="line_content new noteable_line"><span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n</td>\n</tr>\n<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="2">\n2\n</td>\n<td class="line_content new noteable_line"><span id="LC2" class="line" lang="plaintext"></span>\n</td>\n</tr>\n',
- image_diff_html:
- '<div class="image js-replaced-image" data="">\n<div class="two-up view">\n<div class="wrap">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n<div class="wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{&quot;base_sha&quot;:&quot;e63f41fe459e62e1228fcef60d7189127aeba95a&quot;,&quot;start_sha&quot;:&quot;d9eaefe5a676b820c57ff18cf5b68316025f7962&quot;,&quot;head_sha&quot;:&quot;c48ee0d1bf3b30453f5b32250ce03134beaa6d13&quot;,&quot;old_path&quot;:&quot;CHANGELOG&quot;,&quot;new_path&quot;:&quot;CHANGELOG&quot;,&quot;position_type&quot;:&quot;text&quot;,&quot;old_line&quot;:null,&quot;new_line&quot;:2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n</div>\n<div class="swipe view hide">\n<div class="swipe-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="swipe-wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{&quot;base_sha&quot;:&quot;e63f41fe459e62e1228fcef60d7189127aeba95a&quot;,&quot;start_sha&quot;:&quot;d9eaefe5a676b820c57ff18cf5b68316025f7962&quot;,&quot;head_sha&quot;:&quot;c48ee0d1bf3b30453f5b32250ce03134beaa6d13&quot;,&quot;old_path&quot;:&quot;CHANGELOG&quot;,&quot;new_path&quot;:&quot;CHANGELOG&quot;,&quot;position_type&quot;:&quot;text&quot;,&quot;old_line&quot;:null,&quot;new_line&quot;:2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n</div>\n<span class="swipe-bar">\n<span class="top-handle"></span>\n<span class="bottom-handle"></span>\n</span>\n</div>\n</div>\n<div class="onion-skin view hide">\n<div class="onion-skin-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{&quot;base_sha&quot;:&quot;e63f41fe459e62e1228fcef60d7189127aeba95a&quot;,&quot;start_sha&quot;:&quot;d9eaefe5a676b820c57ff18cf5b68316025f7962&quot;,&quot;head_sha&quot;:&quot;c48ee0d1bf3b30453f5b32250ce03134beaa6d13&quot;,&quot;old_path&quot;:&quot;CHANGELOG&quot;,&quot;new_path&quot;:&quot;CHANGELOG&quot;,&quot;position_type&quot;:&quot;text&quot;,&quot;old_line&quot;:null,&quot;new_line&quot;:2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<div class="controls">\n<div class="transparent"></div>\n<div class="drag-track">\n<div class="dragger" style="left: 0px;"></div>\n</div>\n<div class="opaque"></div>\n</div>\n</div>\n</div>\n</div>\n<div class="view-modes hide">\n<ul class="view-modes-menu">\n<li class="two-up" data-mode="two-up">2-up</li>\n<li class="swipe" data-mode="swipe">Swipe</li>\n<li class="onion-skin" data-mode="onion-skin">Onion skin</li>\n</ul>\n</div>\n',
};
export const imageDiffDiscussions = [
diff --git a/spec/serializers/diff_file_entity_spec.rb b/spec/serializers/diff_file_entity_spec.rb
index 7497b8f27bd..073c13c2cbb 100644
--- a/spec/serializers/diff_file_entity_spec.rb
+++ b/spec/serializers/diff_file_entity_spec.rb
@@ -13,39 +13,6 @@ describe DiffFileEntity do
subject { entity.as_json }
- shared_examples 'diff file entity' do
- it 'exposes correct attributes' do
- expect(subject).to include(
- :submodule, :submodule_link, :submodule_tree_url, :file_path,
- :deleted_file, :old_path, :new_path, :mode_changed,
- :a_mode, :b_mode, :text, :old_path_html,
- :new_path_html, :highlighted_diff_lines, :parallel_diff_lines,
- :blob, :file_hash, :added_lines, :removed_lines, :diff_refs, :content_sha,
- :stored_externally, :external_storage, :too_large, :collapsed, :new_file,
- :context_lines_path
- )
- end
-
- it 'includes viewer' do
- expect(subject[:viewer].with_indifferent_access)
- .to match_schema('entities/diff_viewer')
- end
-
- # Converted diff files from GitHub import does not contain blob file
- # and content sha.
- context 'when diff file does not have a blob and content sha' do
- it 'exposes some attributes as nil' do
- allow(diff_file).to receive(:content_sha).and_return(nil)
- allow(diff_file).to receive(:blob).and_return(nil)
-
- expect(subject[:context_lines_path]).to be_nil
- expect(subject[:view_path]).to be_nil
- expect(subject[:highlighted_diff_lines]).to be_nil
- expect(subject[:can_modify_blob]).to be_nil
- end
- end
- end
-
context 'when there is no merge request' do
it_behaves_like 'diff file entity'
end
diff --git a/spec/serializers/discussion_diff_file_entity_spec.rb b/spec/serializers/discussion_diff_file_entity_spec.rb
new file mode 100644
index 00000000000..101ac918a98
--- /dev/null
+++ b/spec/serializers/discussion_diff_file_entity_spec.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DiscussionDiffFileEntity do
+ include RepoHelpers
+
+ let(:project) { create(:project, :repository) }
+ let(:repository) { project.repository }
+ let(:commit) { project.commit(sample_commit.id) }
+ let(:diff_refs) { commit.diff_refs }
+ let(:diff) { commit.raw_diffs.first }
+ let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) }
+ let(:entity) { described_class.new(diff_file, request: {}) }
+
+ subject { entity.as_json }
+
+ context 'when there is no merge request' do
+ it_behaves_like 'diff file discussion entity'
+ end
+
+ context 'when there is a merge request' do
+ let(:user) { create(:user) }
+ let(:request) { EntityRequest.new(project: project, current_user: user) }
+ let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+ let(:entity) { described_class.new(diff_file, request: request, merge_request: merge_request) }
+
+ it_behaves_like 'diff file discussion entity'
+
+ it 'exposes additional attributes' do
+ expect(subject).to include(:edit_path)
+ end
+
+ it 'exposes no diff lines' do
+ expect(subject).not_to include(:highlighted_diff_lines,
+ :parallel_diff_lines)
+ end
+ end
+end
diff --git a/spec/serializers/discussion_entity_spec.rb b/spec/serializers/discussion_entity_spec.rb
index 0590304e832..138749b0fdf 100644
--- a/spec/serializers/discussion_entity_spec.rb
+++ b/spec/serializers/discussion_entity_spec.rb
@@ -74,13 +74,5 @@ describe DiscussionEntity do
:active
)
end
-
- context 'when diff file is a image' do
- it 'exposes image attributes' do
- allow(discussion).to receive(:on_image?).and_return(true)
-
- expect(subject.keys).to include(:image_diff_html)
- end
- end
end
end
diff --git a/spec/support/shared_examples/serializers/diff_file_entity_examples.rb b/spec/support/shared_examples/serializers/diff_file_entity_examples.rb
new file mode 100644
index 00000000000..b8065886c42
--- /dev/null
+++ b/spec/support/shared_examples/serializers/diff_file_entity_examples.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+shared_examples 'diff file base entity' do
+ it 'exposes essential attributes' do
+ expect(subject).to include(:content_sha, :submodule, :submodule_link,
+ :submodule_tree_url, :old_path_html,
+ :new_path_html, :blob, :can_modify_blob,
+ :file_hash, :file_path, :old_path, :new_path,
+ :collapsed, :text, :diff_refs, :stored_externally,
+ :external_storage, :renamed_file, :deleted_file,
+ :mode_changed, :a_mode, :b_mode, :new_file)
+ end
+
+ # Converted diff files from GitHub import does not contain blob file
+ # and content sha.
+ context 'when diff file does not have a blob and content sha' do
+ it 'exposes some attributes as nil' do
+ allow(diff_file).to receive(:content_sha).and_return(nil)
+ allow(diff_file).to receive(:blob).and_return(nil)
+
+ expect(subject[:context_lines_path]).to be_nil
+ expect(subject[:view_path]).to be_nil
+ expect(subject[:highlighted_diff_lines]).to be_nil
+ expect(subject[:can_modify_blob]).to be_nil
+ end
+ end
+end
+
+shared_examples 'diff file entity' do
+ it_behaves_like 'diff file base entity'
+
+ it 'exposes correct attributes' do
+ expect(subject).to include(:too_large, :added_lines, :removed_lines,
+ :context_lines_path, :highlighted_diff_lines,
+ :parallel_diff_lines)
+ end
+
+ it 'includes viewer' do
+ expect(subject[:viewer].with_indifferent_access)
+ .to match_schema('entities/diff_viewer')
+ end
+end
+
+shared_examples 'diff file discussion entity' do
+ it_behaves_like 'diff file base entity'
+end