From 19edeba8e3e8d091853ceed27f271cd67a636551 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Fri, 28 Apr 2017 16:05:00 +0200 Subject: [PATCH 01/21] Prevent people from creating branches if they don't have persmission to push --- ...-creating-branches-if-they-don-have-permission-to-push.yml | 4 ++++ lib/gitlab/user_access.rb | 4 +--- spec/lib/gitlab/user_access_spec.rb | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/28968-prevent-people-from-creating-branches-if-they-don-have-permission-to-push.yml diff --git a/changelogs/unreleased/28968-prevent-people-from-creating-branches-if-they-don-have-permission-to-push.yml b/changelogs/unreleased/28968-prevent-people-from-creating-branches-if-they-don-have-permission-to-push.yml new file mode 100644 index 00000000000..6612cfd8866 --- /dev/null +++ b/changelogs/unreleased/28968-prevent-people-from-creating-branches-if-they-don-have-permission-to-push.yml @@ -0,0 +1,4 @@ +--- +title: Prevent people from creating branches if they don't have persmission to push +merge_request: +author: diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 54728e5ff0e..e46ff313654 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -44,9 +44,7 @@ module Gitlab if ProtectedBranch.protected?(project, ref) return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user) - has_access = project.protected_branches.protected_ref_accessible_to?(ref, user, action: :push) - - has_access || !project.repository.branch_exists?(ref) && can_merge_to_branch?(ref) + project.protected_branches.protected_ref_accessible_to?(ref, user, action: :push) else user.can?(:push_code, project) end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 611cdbbc865..2b27ff66c09 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -87,10 +87,10 @@ describe Gitlab::UserAccess, lib: true do expect(access.can_push_to_branch?(branch.name)).to be_falsey end - it 'returns true if branch does not exist and user has permission to merge' do + it 'returns false if branch does not exist' do project.team << [user, :developer] - expect(access.can_push_to_branch?(not_existing_branch.name)).to be_truthy + expect(access.can_push_to_branch?(not_existing_branch.name)).to be_falsey end end From fe771b51fd80dd0926a31091b18a935a01fe1296 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Tue, 11 Apr 2017 21:23:00 +0200 Subject: [PATCH 02/21] Fix Gitaly::Commit#is_ancestor - Upgrade Gitaly-version --- GITALY_SERVER_VERSION | 2 +- app/models/repository.rb | 16 +++++++--------- lib/gitlab/git/repository.rb | 6 +++++- lib/gitlab/gitaly_client/commit.rb | 29 +++++++++++++++++------------ spec/models/repository_spec.rb | 24 +++++++++++------------- 5 files changed, 41 insertions(+), 36 deletions(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index a918a2aa18d..a3df0a6959e 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.6.0 +0.8.0 diff --git a/app/models/repository.rb b/app/models/repository.rb index d02aea49689..feabfa111fb 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -961,15 +961,13 @@ class Repository end def is_ancestor?(ancestor_id, descendant_id) - # NOTE: This feature is intentionally disabled until - # https://gitlab.com/gitlab-org/gitlab-ce/issues/30586 is resolved - # Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled| - # if is_enabled - # raw_repository.is_ancestor?(ancestor_id, descendant_id) - # else - merge_base_commit(ancestor_id, descendant_id) == ancestor_id - # end - # end + Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled| + if is_enabled + raw_repository.is_ancestor?(ancestor_id, descendant_id) + else + merge_base_commit(ancestor_id, descendant_id) == ancestor_id + end + end end def empty_repo? diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 452dba7971d..18eda0279f7 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -451,7 +451,7 @@ module Gitlab # Returns true is +from+ is direct ancestor to +to+, otherwise false def is_ancestor?(from, to) - Gitlab::GitalyClient::Commit.is_ancestor(self, from, to) + gitaly_commit_client.is_ancestor(from, to) end # Return an array of Diff objects that represent the diff @@ -1273,6 +1273,10 @@ module Gitlab @gitaly_ref_client ||= Gitlab::GitalyClient::Ref.new(self) end + def gitaly_commit_client + @gitaly_commit_client ||= Gitlab::GitalyClient::Commit.new(self) + end + # Returns the `Rugged` sorting type constant for a given # sort type key. Valid keys are `:none`, `:topo`, and `:date` def rugged_sort_type(key) diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb index b7f39f3ef0b..27db1e19bc1 100644 --- a/lib/gitlab/gitaly_client/commit.rb +++ b/lib/gitlab/gitaly_client/commit.rb @@ -5,6 +5,23 @@ module Gitlab # See http://stackoverflow.com/a/40884093/1856239 and https://github.com/git/git/blob/3ad8b5bf26362ac67c9020bf8c30eee54a84f56d/cache.h#L1011-L1012 EMPTY_TREE_ID = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'.freeze + attr_accessor :stub + + def initialize(repository) + @gitaly_repo = repository.gitaly_repository + @stub = Gitaly::Commit::Stub.new(nil, nil, channel_override: repository.gitaly_channel) + end + + def is_ancestor(ancestor_id, child_id) + request = Gitaly::CommitIsAncestorRequest.new( + repository: @gitaly_repo, + ancestor_id: ancestor_id, + child_id: child_id + ) + + @stub.commit_is_ancestor(request).value + end + class << self def diff_from_parent(commit, options = {}) repository = commit.project.repository @@ -20,18 +37,6 @@ module Gitlab Gitlab::Git::DiffCollection.new(stub.commit_diff(request), options) end - - def is_ancestor(repository, ancestor_id, child_id) - gitaly_repo = repository.gitaly_repository - stub = Gitaly::Commit::Stub.new(nil, nil, channel_override: repository.gitaly_channel) - request = Gitaly::CommitIsAncestorRequest.new( - repository: gitaly_repo, - ancestor_id: ancestor_id, - child_id: child_id - ) - - stub.commit_is_ancestor(request).value - end end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 98d0641443e..f6846cc1b2f 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1849,17 +1849,15 @@ describe Repository, models: true do end end - # TODO: Uncomment when feature is reenabled - # describe '#is_ancestor?' do - # context 'Gitaly is_ancestor feature enabled' do - # it 'asks Gitaly server if it\'s an ancestor' do - # commit = repository.commit - # allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true) - # expect(Gitlab::GitalyClient::Commit).to receive(:is_ancestor). - # with(repository.raw_repository, commit.id, commit.id).and_return(true) - # - # expect(repository.is_ancestor?(commit.id, commit.id)).to be true - # end - # end - # end + describe '#is_ancestor?' do + context 'Gitaly is_ancestor feature enabled' do + it "asks Gitaly server if it's an ancestor" do + commit = repository.commit + expect(repository.raw_repository).to receive(:is_ancestor?).and_call_original + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true) + + expect(repository.is_ancestor?(commit.id, commit.id)).to be true + end + end + end end From 9f3985c6133507ac721abbb4c19049656008ec68 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 11 Apr 2017 22:44:22 -0400 Subject: [PATCH 03/21] Move PDFLab into GitLab --- .gitignore | 1 + app/assets/javascripts/blob/pdf/index.js | 10 +-- app/assets/javascripts/pdf/assets/img/bg.gif | Bin 0 -> 58 bytes app/assets/javascripts/pdf/index.vue | 73 ++++++++++++++++++ app/assets/javascripts/pdf/page/index.vue | 68 ++++++++++++++++ config/webpack.config.js | 5 ++ package.json | 2 + .../projects/commit/cherry_pick_spec.rb | 6 +- .../projects/environments/environment_spec.rb | 2 +- .../projects/merge_request_button_spec.rb | 4 +- spec/javascripts/blob/pdf/index_spec.js | 6 +- spec/javascripts/blob/pdf/test.pdf | Bin 11956 -> 0 bytes spec/javascripts/fixtures/pdf.rb | 18 +++++ spec/javascripts/pdf/index_spec.js | 61 +++++++++++++++ spec/javascripts/pdf/page_spec.js | 57 ++++++++++++++ spec/support/test_env.rb | 3 +- yarn.lock | 26 ++++++- 17 files changed, 327 insertions(+), 15 deletions(-) create mode 100644 app/assets/javascripts/pdf/assets/img/bg.gif create mode 100644 app/assets/javascripts/pdf/index.vue create mode 100644 app/assets/javascripts/pdf/page/index.vue delete mode 100644 spec/javascripts/blob/pdf/test.pdf create mode 100644 spec/javascripts/fixtures/pdf.rb create mode 100644 spec/javascripts/pdf/index_spec.js create mode 100644 spec/javascripts/pdf/page_spec.js diff --git a/.gitignore b/.gitignore index 51b4d06b01b..7e515515693 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,7 @@ eslint-report.html /public/uploads.* /public/uploads/ /shared/artifacts/ +/spec/javascripts/fixtures/blob/pdf/ /rails_best_practices_output.html /tags /tmp/* diff --git a/app/assets/javascripts/blob/pdf/index.js b/app/assets/javascripts/blob/pdf/index.js index a74c2db9a61..7fc4cd7c9fd 100644 --- a/app/assets/javascripts/blob/pdf/index.js +++ b/app/assets/javascripts/blob/pdf/index.js @@ -1,11 +1,6 @@ /* eslint-disable no-new */ import Vue from 'vue'; -import PDFLab from 'vendor/pdflab'; -import workerSrc from 'vendor/pdf.worker'; - -Vue.use(PDFLab, { - workerSrc, -}); +import pdfLab from '../../pdf/index.vue'; export default () => { const el = document.getElementById('js-pdf-viewer'); @@ -20,6 +15,9 @@ export default () => { pdf: el.dataset.endpoint, }; }, + components: { + pdfLab, + }, methods: { onLoad() { this.loading = false; diff --git a/app/assets/javascripts/pdf/assets/img/bg.gif b/app/assets/javascripts/pdf/assets/img/bg.gif new file mode 100644 index 0000000000000000000000000000000000000000..c7e98e044f578204f9c1ebdefcb05f4b2baaea32 GIT binary patch literal 58 zcmZ?wbhEHbQxZ@ literal 0 HcmV?d00001 diff --git a/app/assets/javascripts/pdf/index.vue b/app/assets/javascripts/pdf/index.vue new file mode 100644 index 00000000000..4603859d7b0 --- /dev/null +++ b/app/assets/javascripts/pdf/index.vue @@ -0,0 +1,73 @@ + + + + + diff --git a/app/assets/javascripts/pdf/page/index.vue b/app/assets/javascripts/pdf/page/index.vue new file mode 100644 index 00000000000..7b74ee4eb2e --- /dev/null +++ b/app/assets/javascripts/pdf/page/index.vue @@ -0,0 +1,68 @@ + + + + + diff --git a/config/webpack.config.js b/config/webpack.config.js index cb0a57a3a41..0ec9e48845e 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -77,6 +77,11 @@ var config = { test: /\.svg$/, loader: 'raw-loader', }, + { + test: /\.gif$/, + loader: 'url-loader', + query: { mimetype: 'image/gif' }, + }, { test: /\.(worker\.js|pdf)$/, exclude: /node_modules/, diff --git a/package.json b/package.json index f8c151ebd81..9ed5e1a7475 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "jszip-utils": "^0.0.2", "marked": "^0.3.6", "mousetrap": "^1.4.6", + "pdfjs-dist": "^1.8.252", "pikaday": "^1.5.1", "prismjs": "^1.6.0", "raphael": "^2.2.7", @@ -46,6 +47,7 @@ "three-stl-loader": "^1.0.4", "timeago.js": "^2.0.5", "underscore": "^1.8.3", + "url-loader": "^0.5.8", "visibilityjs": "^1.2.4", "vue": "^2.2.6", "vue-loader": "^11.3.4", diff --git a/spec/features/projects/commit/cherry_pick_spec.rb b/spec/features/projects/commit/cherry_pick_spec.rb index 5d64d42fd61..fa67d390c47 100644 --- a/spec/features/projects/commit/cherry_pick_spec.rb +++ b/spec/features/projects/commit/cherry_pick_spec.rb @@ -74,8 +74,10 @@ describe 'Cherry-pick Commits' do wait_for_ajax - page.within('#modal-cherry-pick-commit .dropdown-menu .dropdown-content') do - click_link 'feature' + page.within('#modal-cherry-pick-commit .dropdown-menu') do + find('.dropdown-input input').set('feature') + wait_for_ajax + click_link "feature" end page.within('#modal-cherry-pick-commit') do diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb index acc3efe04e6..1e12f8542e2 100644 --- a/spec/features/projects/environments/environment_spec.rb +++ b/spec/features/projects/environments/environment_spec.rb @@ -200,7 +200,7 @@ feature 'Environment', :feature do end scenario 'user deletes the branch with running environment' do - visit namespace_project_branches_path(project.namespace, project) + visit namespace_project_branches_path(project.namespace, project, search: 'feature') remove_branch_with_hooks(project, user, 'feature') do page.within('.js-branch-feature') { find('a.btn-remove').click } diff --git a/spec/features/projects/merge_request_button_spec.rb b/spec/features/projects/merge_request_button_spec.rb index 05f3162f13c..1370ab1c521 100644 --- a/spec/features/projects/merge_request_button_spec.rb +++ b/spec/features/projects/merge_request_button_spec.rb @@ -85,8 +85,8 @@ feature 'Merge Request button', feature: true do context 'on branches page' do it_behaves_like 'Merge request button only shown when allowed' do let(:label) { 'Merge request' } - let(:url) { namespace_project_branches_path(project.namespace, project) } - let(:fork_url) { namespace_project_branches_path(forked_project.namespace, forked_project) } + let(:url) { namespace_project_branches_path(project.namespace, project, search: 'feature') } + let(:fork_url) { namespace_project_branches_path(forked_project.namespace, forked_project, search: 'feature') } end end diff --git a/spec/javascripts/blob/pdf/index_spec.js b/spec/javascripts/blob/pdf/index_spec.js index d3a4d04345b..bbeaf95e68d 100644 --- a/spec/javascripts/blob/pdf/index_spec.js +++ b/spec/javascripts/blob/pdf/index_spec.js @@ -1,5 +1,7 @@ +/* eslint-disable import/no-unresolved */ + import renderPDF from '~/blob/pdf'; -import testPDF from './test.pdf'; +import testPDF from '../../fixtures/blob/pdf/test.pdf'; describe('PDF renderer', () => { let viewer; @@ -59,7 +61,7 @@ describe('PDF renderer', () => { describe('error getting file', () => { beforeEach((done) => { - viewer.dataset.endpoint = 'invalid/endpoint'; + viewer.dataset.endpoint = 'invalid/path/to/file.pdf'; app = renderPDF(); checkLoaded(done); diff --git a/spec/javascripts/blob/pdf/test.pdf b/spec/javascripts/blob/pdf/test.pdf deleted file mode 100644 index eb3d147fde31fb904c6476acc41b61f20c9b5e2a..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 11956 zcmeHNdt6NG*Dn&M%Q-|Ox2>e2Y45quq(+x1m8Q~+N*9`Db~T!tnMqf<<+z6kg(#IH zlpG;RL?O4_r9+fbxrE%W@3UvRO!DKrpL6~^(`TB!*R!7Wto2>%dG>zRdd$3;E;zh3 zN#Cr#;qo^5hw!imgvZm#6xa(QkYQ(tLLwuuC!lix zokXI;u23LECQ~3hl^D!m=%ZpjCI#yK(btKQp<&9A@I-wbq~n99K*}pf+(o0&d(rTC z3jJF?JdsZAMWfKj-_i(pVlR0F3YqpTADKcxdeLaT$EDzjz4$0ZDgY4ssM$d!^wJr@ zpz7jN$hZO_D$~~iptAwq5Rn3V%h0(Pv_u;0?}LW16(R`8fqf(r1w6-3f7GGMy-R8k69_bR-aH&NLSi8SjGN$#g2i*4c&VNOzz*;R#F!3W-E? zKxlXxg+z5>G94*+Cp-i5oQ_<6}*fih8^gpa~*Kzk|(8!8=ULqs}*Ay>#yt_ZmQPH0HTeSSztV!4Kv ziSBT3&A|izM5H6_2R%#*)z+S+J^Iq3^YzQ)4{PmxAol;gdR&xVUX9MO;lqu_1&k%F zH~nh}MO%B{o`KU`MlI0jtMTZ#u0mf2#9N2_j>sco7;x1LO2WY?;&6*EUp1UH zJT-6{(>2(ZKN(m#s0TL`70Mw3Ca-(NAgBeIlT0FYl*9yrX$Dd71Q1GuV9-^DiWMCS zt8`P5Fp!)iVg+agQo)o1YWheNAR;mJfZw>iWci?SNtBF-Vor7eUBM5BgFpn;q~dV! z5TV0d4gn&7l>*Zg1c8LdMiqfDz!>JBF^XW=IYu#sjSU4POkpDs5nDz9FW^=pg#oK( zBPPGY>FlUb1r|LV;8!IprUw(1^JD@k7H8NR0}R0&=@h}0VKdH8z*j`b0|{iT>7V~m z3788g6e_?NuarlmVd$x7bP@?HJOFztt)m^g1It%1?8ud)n6@xSAVTGyXta++#1(t9 zagGuppTz-_P1d0_kpMf3!Kxq-hXd`!4q~~W>%PlqHGDA8|1Lh5%4+!FNwmKQ(BFkm z2@D1f_+qAa-gknc0;>}~8h}pOf^jP0#9FY`9=pR@5D4SJ0$K(3KX>5&M*-UM?gdm^9TNo`%gXqVkIw17<6j2N*@=&<+DJBY*fM zfOzhdiT!@-TFgC|+d7?zjezlCbaf{yALPoQ=;>r^-l(LikAgrXK*w}b|HWi?>Z{TV zyTh)qS=ZUp`Kz=Zgb>!=NmB(Dc8AIBIpWXnus-1Xzk}i*MFJM=9g*PC@gSzr@!xqA zcRxH`mnZqvifdgB=DqbB^3q_!exrM4IPJc-52VE0+Nc+}7`k24KS1af#LmujoAEs1 zb4Fg1a|%3Z^I6t`66;-gp3`Stx_tcJ6-n8=(owT!t#c~-nixyY{j;EM%*cS$@L7-K zOM}wk%O8@T(th2jJKHy2W;b%qyUQA1CppC%RVT>9?%ti{b>>zyb$$O2b0?SN-1}5! zRb@BCuqex9DEWXXHED0G?2Y8H*X8#47ZR@~xJ)kCwRVnO%j>?8aj)%%g*n_RP2{uU zGhRP!T60cuL1M0;raA1sxE(TS{v7_r>*nL(rO6R|bY{quodH|SZod9};!SXC$)UkZ znqEKt8e~m%#t^?T&UA&#x{Jx8+an$q4)cdgznsY0Lt~SYqw;-sXTi^; z%e+ns2wBx+D0>{aNZUt>Me|5IacmIM}9Q)AEjHdj^-=@qoUHg>xmsb92+M?>>?sPx$>nen@g8iz$lh=#7Udq9xYcRV zvbda##iHw*rWcByA_`OAV~|_pJRWmK#mmgr*2>4NmFteqxpSbjkN1o<+yTYUf@VCN z7?^(JY}WAW=i--`U#xNxO5f>^UD~H+JK4ZnU-!wxL!VBVyK@l382_uHHFVhVP;%jl zb<#Wb5`KYkqs}Y_b;_9cR$t@mFXvP1FqMiu>4VEUmDfIcK|-=!jl^wN-V~t5LT~ zswpq#RSaKSpBg&lu}gVn`K1$ECoI?x$7g4AX3n98p|ek(-{z9IR8M;1eb|$k?TPC) zoc3-npJKGY-e|blnUZK>b2|FU%qXMiVgH;#lLfzJ6h&UoO1}MDtYvG=F54T$10rk2 zPrLGf93i)UX|(_SCSwaj^Jb?-1Be9IMQ^MQ57d3QP2(W^;!kwUA)(*N@U%pngIU2~ zN7v(LmY3z*ovgB7VC-nkHfZOK@9PzzXr;MTn8$|NMV=f{_f~YI`bBMXh~Egip%Z45 z5Uz{}*u19oI>FQ+fOOM@ke@v5m)eufN3<(KtAE#|F0bIc31%8vxlc7pDfQ1-)^Fs3 zpcjK`a&o-OkJhIrS!7(iT}hvpJm$;;o$-Ts-h0PAa9sSXJc&xO^j|UP0BiQLM%)aW z5y!mE(w^uo(f@SE+ia%M*T&i%je6^6dM^puSU-TJzav1;ReSrPLl>+P?szVK<6E8l z{B8n7ic?s~Jh=D6AF#8Oth9dhwpQA^(0$va0*3}`;+KCGp#b>xg>y*mf9eWmx>8+T2Y*W%p;fD;y#DHPN^?3!Cc6+ohag^3S_1)t<;oA93+ui<_?7Cdt z)h+ff&IY6fX@B)zc+P2XRiV~3UY5@eo7J(Tf?pd~1(cK@dfs=7^^L5MgH%_|-Fc~1 z(SrOVW47#Zy}Q!RLtxyX;WGITs!iHG!?#mEZcZ&(xu&jFM-&%gX4&>^%FVBfSMkc% zI#aHc-mzeOwD>hLNP);Iiq3CG%g;CHOD$t54F%>!ciRGzChhO<8NXN<(y%;Kl()$= zaI;P}r`$@TMQ4FQ{x7jUP9{UXIL_Ew(E9A9=Dp7WZ4Xi-HEvlCiVZNlaOZf3B#^jCJw@?q|%9^vq!7*#x&M4NoQ+J*<5w*Bn%v-oB7~_x42t=>tK|gu*5LXTDig;1XA~ zK5xO?_5P#y(>8JLJ>Jk5->2eozg@qi}AtE}w&&e4i@U;5j|rO;k~F?H;> z^J-uA^g?s~siCF@9}eUbr##zqMqpZWDAm%c|6r48yYj@TsMQt5F0V2jVpPn~F<0%Z zYfG(>vGZGf9#}jGLf~MSejTp(wXks{F&uehzGVnLzqugzXiQ$(W#YEcZsynj?6>>A zI1Po(gKo{G?i%rjw_{Mv9?rAOCTea@(|NZyeP%NTy~OuDxz<*=0V%AFPSN7y+gq_{l7r};rZP{Wv%Wi8LLUsFQU0v>&+Al;b|Ikl8? zfsPa~X)8=?^|pP`-y*9XvFNev;KTGxk>*??dzjs>*=sz-WmE0vnZ8-;bvKinI9l^G z=^QfiwD<3}JLd)LtE;viu%Rhbr`=$B?jL#U6XViT%xg~n6<>H)cAA!ZwUs&j(%5+y zJZ4EsLQ+Q`)-@OAthr|-9qC$j^@aA0msDPk>qsl<<5~gl_`Iqm18QoA7?l#I9THVX z%_81!)M7VWd{{qQcDccX{AuoqpfR_$oXI9jnkMQuxu=)h7%f^jG_ra@m0#=U@e|yd zV{y7qiVdNxz)`z4*BwbsyI?!|BZI$j@Q$?5N+hXXr*GdelLwgR5;hB)*kLw`N0$?D zx@P%C`0Ds#XLHx>&QVnY$)y!DtMxPq{@%nfI);%!kCPfHtDkuKU)NdxL2FC#@xbWR zDNDk3+?g>V>ivWk(YaNJE2|Q5t@{ty=I&eha`~6pHd^*gj6UIb|Icfl%g%8JYKJx- z-?faUk$1%DZScYV%ifK1JjXWM9ZC;jEg3^-`~CTL$??PyhXQOCqVLeP{DxZBnXzT4m?Q3 zcb_+FO)-);xtD1c9>^TN{8^*_2ECMo{TYYt3J|;GsyjRKCl{};TWBUvc?x6nxI$$YKU5GNp@66eNUkX}RYVfVp2>Mwjs{1- ztK>RjnHL-x~jas5q4y0x*CJS0oU|S}0Y3Kt=&lrW7n;C$7|0 zH4p{lp#3|pabyxjm0#(ZG=Ww7R|j#p5afQk^9n?4$KBI#JjI$uq5jzT?%nX;gn&-_ zpAe9!DhTi-1SB01B1mZcLkNf<2ahF?egFXuPgVl~+imv%fkybB_G25r9~w>iJASA1 z^gBZR$437@{f=$jJ52WSW`U}Omp2r~70OXqBJH?PV%U|&gV_!pbT$h@tPw&terhqK zOmKlTH>i%NHnt?%_x80|oung9y+UV$On&!9H|EZ1HcN8He&AEDG+w;x+f&wtfgx%gtr45y9Gi+}f$`xog9#V>C`~%&m;S5fhkkz=m{IC%l$*AGdmrkhAAA zLz?s`C^&XCxp?P^65X}Y1Cx#wYh?9pc`zx^{{DeQhDYW+@oah7{`5(G=DSyBueKJI zd`|z^VrQ2<{%AgF=;R^A#(rqzy1V|@R&lN^ZNC|-GtoF_#udgh>Y>>|V<-``%zfL( z2AdwO)D%>=nx`L{&)OvuWz?`@>KjJOuNHLHKAbG*;4{K0i| z%PW1KA_rr6+a6AGq>8=$b!)Z-U3W+@&PW$a-#x7E^R;Sm2{$;Ew26JQkiPBy$@s5f z6a89V-V?67TXTcw7qi-{vc6E}II~E1EbG+-or|6?=KCaHoo_L3?8Bw?bDmXPu8TM@ zDe2kA!f9oA_Wgq`@6YGHjpQuyNS=JTFv5NE{q4n9;x*q7bAItLt69(Q#qhr#>WM#$ zeR~*GCbmXLo%zo4t2X~UqeOC$-Bu=x)v9J{;mU@Y-Jz#e(r}VQfd#f>>MK+D@KgaG zOA3Qf_feCjmQL?XHT$zXIs=wSCy-RjM`uByJ2q`6$AJ1nB&vWy(Qtv-&ZMoR%mfnf z?M(d0EQBR>LL&sOaWa%0=gHy4Me^u;lgSJn+ZdY|kyHfA6HtsuD3;sA*qLxaEfBQ< z*BBW#fs`zYNIMe;tO4?8c|%ST848iC5gd;QDuE=DHGvFt!&^b@C^6*4Q$S$31dk1@ zzG#Cd*%0X_3{aDTY-Ri~8y}`i$5`Ob&Ll#iklMg-bab?JG|^fj3y1M^Ivqv`FoA#r z95{KbSiz0KiRGp$A)RtMv+e?k7{cUoLqQ?L&cp;8sO!^V6j$0ckXUZ5G|-wS5y3HB zDU7#9;4aCaqb(rXIDz69$meq;5+TEZCzFJ76%eQ&u(**ZNa|bTp)io__mjx@wwR0_ z?A_xlB`NDUu}n~GU|{te9D)ZARSp1!gxDZ7973}}5S0)$&TnKWwL!llgGdM1s>@L0 z{6(I1VT`Q#av;HKo+1%!IwnKEp6c*{QtJ~F501@d8zCqv+L^@Q_-Gh6 zN~kblc>B0{I=Oj!II|%aA1@YEzhJAYmmkFOg8bY(0m;qTlgWl0JeiQ2r;EpQXHO?* z$lKWqboh}SJ=IX!(L>{ZElR0w+Po=WEPLg`^(O#so*t8cdtu~me{vT+lGO1~JCjXUHL zL%DcD7&R0};SmrZ7vGiZeQk;sg?&Mj#St9lgKd`k|4k5CKmi z@brVb9)G=pTQ%|i<^SE*{mX*Y#r-#kIvPK{{tvF7LZr^{A6)8a{Pg-ixPA(eI>Uc( zsiX1J>;K^TDMacFKY~lAdjk!Q{n?pBgYEHm(gA8`!IXO~@IH_2lD;{ur`(j8BZ#>r z>=31hkjN~s7j%#qRlVF>{BTQYi5*!}Zg{%*zTNNyb?$hAEG=R1a@0!&km$qqmMRrG zUbif;*DXu9=etsu;+=0)mLQ`Hwg492zQ3>jn{V&nm}2K~;*L|P5JBzWFi0g+$YgLf z6`Uzno~T8zv&r3uYgIHVf}P~UKHUd%sR#um^S-6^J~&Lp6Tj^Xa(+Z`eyC?(<=r>B e{6|xQ)XPUbap`pcnm`0jS3iji4t{r?MgBk1ynNsQ diff --git a/spec/javascripts/fixtures/pdf.rb b/spec/javascripts/fixtures/pdf.rb new file mode 100644 index 00000000000..6b2422a7986 --- /dev/null +++ b/spec/javascripts/fixtures/pdf.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe 'PDF file', '(JavaScript fixtures)', type: :controller do + include JavaScriptFixturesHelpers + + let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} + let(:project) { create(:project, namespace: namespace, path: 'pdf-project') } + + before(:all) do + clean_frontend_fixtures('blob/pdf/') + end + + it 'blob/pdf/test.pdf' do |example| + blob = project.repository.blob_at('e774ebd33', 'files/pdf/test.pdf') + + store_frontend_fixture(blob.data.force_encoding("utf-8"), example.description) + end +end diff --git a/spec/javascripts/pdf/index_spec.js b/spec/javascripts/pdf/index_spec.js new file mode 100644 index 00000000000..f661fae5fe2 --- /dev/null +++ b/spec/javascripts/pdf/index_spec.js @@ -0,0 +1,61 @@ +/* eslint-disable import/no-unresolved */ + +import Vue from 'vue'; +import { PDFJS } from 'pdfjs-dist'; +import workerSrc from 'vendor/pdf.worker'; + +import PDFLab from '~/pdf/index.vue'; +import pdf from '../fixtures/blob/pdf/test.pdf'; + +PDFJS.workerSrc = workerSrc; +const Component = Vue.extend(PDFLab); + +describe('PDF component', () => { + let vm; + + const checkLoaded = (done) => { + if (vm.loading) { + setTimeout(() => { + checkLoaded(done); + }, 100); + } else { + done(); + } + }; + + describe('without PDF data', () => { + beforeEach((done) => { + vm = new Component({ + propsData: { + pdf: '', + }, + }); + + vm.$mount(); + + checkLoaded(done); + }); + + it('does not render', () => { + expect(vm.$el.tagName).toBeUndefined(); + }); + }); + + describe('with PDF data', () => { + beforeEach((done) => { + vm = new Component({ + propsData: { + pdf, + }, + }); + + vm.$mount(); + + checkLoaded(done); + }); + + it('renders pdf component', () => { + expect(vm.$el.tagName).toBeDefined(); + }); + }); +}); diff --git a/spec/javascripts/pdf/page_spec.js b/spec/javascripts/pdf/page_spec.js new file mode 100644 index 00000000000..ac76ebbfbe6 --- /dev/null +++ b/spec/javascripts/pdf/page_spec.js @@ -0,0 +1,57 @@ +/* eslint-disable import/no-unresolved */ + +import Vue from 'vue'; +import pdfjsLib from 'pdfjs-dist'; +import workerSrc from 'vendor/pdf.worker'; + +import PageComponent from '~/pdf/page/index.vue'; +import testPDF from '../fixtures/blob/pdf/test.pdf'; + +const Component = Vue.extend(PageComponent); + +describe('Page component', () => { + let vm; + let testPage; + pdfjsLib.PDFJS.workerSrc = workerSrc; + + const checkRendered = (done) => { + if (vm.rendering) { + setTimeout(() => { + checkRendered(done); + }, 100); + } else { + done(); + } + }; + + beforeEach((done) => { + pdfjsLib.getDocument(testPDF) + .then(pdf => pdf.getPage(1)) + .then((page) => { + testPage = page; + done(); + }) + .catch((error) => { + console.error(error); + }); + }); + + describe('render', () => { + beforeEach((done) => { + vm = new Component({ + propsData: { + page: testPage, + number: 1, + }, + }); + + vm.$mount(); + + checkRendered(done); + }); + + it('renders first page', () => { + expect(vm.$el.tagName).toBeDefined(); + }); + }); +}); diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 5c8ee8d62f5..0b3c6169c9b 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -39,7 +39,8 @@ module TestEnv 'wip' => 'b9238ee', 'csv' => '3dd0896', 'v1.1.0' => 'b83d6e3', - 'add-ipython-files' => '6d85bb69' + 'add-ipython-files' => '6d85bb69', + 'add-pdf-file' => 'e774ebd3' }.freeze # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily diff --git a/yarn.lock b/yarn.lock index 8f38fb4a9a4..fdef0665d15 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3638,7 +3638,7 @@ mime-types@^2.1.12, mime-types@~2.1.11, mime-types@~2.1.13, mime-types@~2.1.7: dependencies: mime-db "~1.26.0" -mime@1.3.4, mime@^1.3.4: +mime@1.3.4, mime@1.3.x, mime@^1.3.4: version "1.3.4" resolved "https://registry.yarnpkg.com/mime/-/mime-1.3.4.tgz#115f9e3b6b3daf2959983cb38f149a2d40eb5d53" @@ -3710,6 +3710,10 @@ nested-error-stacks@^1.0.0: dependencies: inherits "~2.0.1" +node-ensure@^0.0.0: + version "0.0.0" + resolved "https://registry.yarnpkg.com/node-ensure/-/node-ensure-0.0.0.tgz#ecae764150de99861ec5c810fd5d096b183932a7" + node-libs-browser@^1.0.0: version "1.1.1" resolved "https://registry.yarnpkg.com/node-libs-browser/-/node-libs-browser-1.1.1.tgz#2a38243abedd7dffcd07a97c9aca5668975a6fea" @@ -4102,6 +4106,13 @@ pbkdf2@^3.0.3: dependencies: create-hmac "^1.1.2" +pdfjs-dist@^1.8.252: + version "1.8.252" + resolved "https://registry.yarnpkg.com/pdfjs-dist/-/pdfjs-dist-1.8.252.tgz#2477245695341f7fe096824dacf327bc324c0f52" + dependencies: + node-ensure "^0.0.0" + worker-loader "^0.8.0" + pend@~1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/pend/-/pend-1.2.0.tgz#7a57eb550a6783f9115331fcf4663d5c8e007a50" @@ -5538,6 +5549,13 @@ update-notifier@0.5.0: semver-diff "^2.0.0" string-length "^1.0.0" +url-loader@^0.5.8: + version "0.5.8" + resolved "https://registry.yarnpkg.com/url-loader/-/url-loader-0.5.8.tgz#b9183b1801e0f847718673673040bc9dc1c715c5" + dependencies: + loader-utils "^1.0.2" + mime "1.3.x" + url-parse@1.0.x: version "1.0.5" resolved "https://registry.yarnpkg.com/url-parse/-/url-parse-1.0.5.tgz#0854860422afdcfefeb6c965c662d4800169927b" @@ -5821,6 +5839,12 @@ wordwrap@~0.0.2: version "0.0.3" resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-0.0.3.tgz#a3d5da6cd5c0bc0008d37234bbaf1bed63059107" +worker-loader@^0.8.0: + version "0.8.0" + resolved "https://registry.yarnpkg.com/worker-loader/-/worker-loader-0.8.0.tgz#13582960dcd7d700dc829d3fd252a7561696167e" + dependencies: + loader-utils "^1.0.2" + wrap-ansi@^2.0.0: version "2.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-2.1.0.tgz#d8fc3d284dd05794fe84973caecdd1cf824fdd85" From bbdaf982e6e9b4f674a309df9b16fb9c85498b50 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 24 Apr 2017 12:14:09 -0500 Subject: [PATCH 04/21] Refactor the AddColumnWithDefault cop to use node matchers --- .../cop/migration/add_column_with_default.rb | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/add_column_with_default.rb index 54a920d4b49..e04af8fc096 100644 --- a/rubocop/cop/migration/add_column_with_default.rb +++ b/rubocop/cop/migration/add_column_with_default.rb @@ -8,26 +8,27 @@ module RuboCop class AddColumnWithDefault < RuboCop::Cop::Cop include MigrationHelpers + def_node_matcher :add_column_with_default?, <<~PATTERN + (send nil :add_column_with_default $...) + PATTERN + + def_node_matcher :defines_change?, <<~PATTERN + (def :change ...) + PATTERN + MSG = '`add_column_with_default` is not reversible so you must manually define ' \ 'the `up` and `down` methods in your migration class, using `remove_column` in `down`'.freeze def on_send(node) return unless in_migration?(node) - - name = node.children[1] - - return unless name == :add_column_with_default + return unless add_column_with_default?(node) node.each_ancestor(:def) do |def_node| - next unless method_name(def_node) == :change + next unless defines_change?(def_node) add_offense(def_node, :name) end end - - def method_name(node) - node.children.first - end end end end From 9c27c90b4a8a9ea179af9588a7dbd2037829905f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 24 Apr 2017 12:16:33 -0500 Subject: [PATCH 05/21] Rename AddColumnWithDefault to ReversibleAddColumnWithDefault We're going to add another cop that deals with another aspect of `add_column_with_default`, so we need to separate them. --- ..._with_default.rb => reversible_add_column_with_default.rb} | 2 +- rubocop/rubocop.rb | 2 +- ...ult_spec.rb => reversible_add_column_with_default_spec.rb} | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename rubocop/cop/migration/{add_column_with_default.rb => reversible_add_column_with_default.rb} (93%) rename spec/rubocop/cop/migration/{add_column_with_default_spec.rb => reversible_add_column_with_default_spec.rb} (87%) diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/reversible_add_column_with_default.rb similarity index 93% rename from rubocop/cop/migration/add_column_with_default.rb rename to rubocop/cop/migration/reversible_add_column_with_default.rb index e04af8fc096..f413f06f39b 100644 --- a/rubocop/cop/migration/add_column_with_default.rb +++ b/rubocop/cop/migration/reversible_add_column_with_default.rb @@ -5,7 +5,7 @@ module RuboCop module Migration # Cop that checks if `add_column_with_default` is used with `up`/`down` methods # and not `change`. - class AddColumnWithDefault < RuboCop::Cop::Cop + class ReversibleAddColumnWithDefault < RuboCop::Cop::Cop include MigrationHelpers def_node_matcher :add_column_with_default?, <<~PATTERN diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index d580aa6857a..cdc0fbfd6a8 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,9 +1,9 @@ require_relative 'cop/custom_error_class' require_relative 'cop/gem_fetcher' require_relative 'cop/migration/add_column' -require_relative 'cop/migration/add_column_with_default' require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' require_relative 'cop/migration/remove_concurrent_index' require_relative 'cop/migration/remove_index' +require_relative 'cop/migration/reversible_add_column_with_default' diff --git a/spec/rubocop/cop/migration/add_column_with_default_spec.rb b/spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb similarity index 87% rename from spec/rubocop/cop/migration/add_column_with_default_spec.rb rename to spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb index 6b9b6b19650..3723d635083 100644 --- a/spec/rubocop/cop/migration/add_column_with_default_spec.rb +++ b/spec/rubocop/cop/migration/reversible_add_column_with_default_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' require 'rubocop' require 'rubocop/rspec/support' -require_relative '../../../../rubocop/cop/migration/add_column_with_default' +require_relative '../../../../rubocop/cop/migration/reversible_add_column_with_default' -describe RuboCop::Cop::Migration::AddColumnWithDefault do +describe RuboCop::Cop::Migration::ReversibleAddColumnWithDefault do include CopHelper subject(:cop) { described_class.new } From b9fa17d87b534e588d80bb029c7f8cf9f9ff0c45 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 24 Apr 2017 12:48:35 -0500 Subject: [PATCH 06/21] Add AddColumnWithDefaultToLargeTable cop --- .../add_column_with_default_to_large_table.rb | 51 +++++++++++++++++++ rubocop/rubocop.rb | 1 + ...column_with_default_to_large_table_spec.rb | 44 ++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 rubocop/cop/migration/add_column_with_default_to_large_table.rb create mode 100644 spec/rubocop/cop/migration/add_column_with_default_to_large_table_spec.rb diff --git a/rubocop/cop/migration/add_column_with_default_to_large_table.rb b/rubocop/cop/migration/add_column_with_default_to_large_table.rb new file mode 100644 index 00000000000..2372e6b60ea --- /dev/null +++ b/rubocop/cop/migration/add_column_with_default_to_large_table.rb @@ -0,0 +1,51 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # This cop checks for `add_column_with_default` on a table that's been + # explicitly blacklisted because of its size. + # + # Even though this helper performs the update in batches to avoid + # downtime, using it with tables with millions of rows still causes a + # significant delay in the deploy process and is best avoided. + # + # See https://gitlab.com/gitlab-com/infrastructure/issues/1602 for more + # information. + class AddColumnWithDefaultToLargeTable < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = 'Using `add_column_with_default` on the `%s` table will take a ' \ + 'long time to complete, and should be avoided unless absolutely ' \ + 'necessary'.freeze + + LARGE_TABLES = %i[ + events + issues + merge_requests + namespaces + notes + projects + routes + users + ].freeze + + def_node_matcher :add_column_with_default?, <<~PATTERN + (send nil :add_column_with_default $(sym ...) ...) + PATTERN + + def on_send(node) + return unless in_migration?(node) + + matched = add_column_with_default?(node) + return unless matched + + table = matched.to_a.first + return unless LARGE_TABLES.include?(table) + + add_offense(node, :expression, format(MSG, table)) + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index cdc0fbfd6a8..4ff204f939e 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,6 +1,7 @@ require_relative 'cop/custom_error_class' require_relative 'cop/gem_fetcher' require_relative 'cop/migration/add_column' +require_relative 'cop/migration/add_column_with_default_to_large_table' require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' diff --git a/spec/rubocop/cop/migration/add_column_with_default_to_large_table_spec.rb b/spec/rubocop/cop/migration/add_column_with_default_to_large_table_spec.rb new file mode 100644 index 00000000000..07cb3fc4a2e --- /dev/null +++ b/spec/rubocop/cop/migration/add_column_with_default_to_large_table_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/add_column_with_default_to_large_table' + +describe RuboCop::Cop::Migration::AddColumnWithDefaultToLargeTable do + include CopHelper + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + described_class::LARGE_TABLES.each do |table| + it "registers an offense for the #{table} table" do + inspect_source(cop, "add_column_with_default :#{table}, :column, default: true") + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + end + + it 'registers no offense for non-blacklisted tables' do + inspect_source(cop, "add_column_with_default :table, :column, default: true") + + expect(cop.offenses).to be_empty + end + end + + context 'outside of migration' do + it 'registers no offense' do + table = described_class::LARGE_TABLES.sample + inspect_source(cop, "add_column_with_default :#{table}, :column, default: true") + + expect(cop.offenses).to be_empty + end + end +end From 30cc6b202a1ce18be880b101bc16df6bb1d2536f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 24 Apr 2017 12:54:09 -0500 Subject: [PATCH 07/21] Disable AddColumnWithDefaultToLargeTable cop for pre-existing migrations --- ...9122101_add_only_allow_merge_if_build_succeeds_to_projects.rb | 1 + db/migrate/20160608195742_add_repository_storage_to_projects.rb | 1 + .../20160715154212_add_request_access_enabled_to_projects.rb | 1 + .../20160715204316_add_request_access_enabled_to_groups.rb | 1 + .../20160831223750_remove_features_enabled_from_projects.rb | 1 + db/migrate/20160913162434_remove_projects_pushes_since_gc.rb | 1 + .../20170124193147_add_two_factor_columns_to_namespaces.rb | 1 + db/migrate/20170124193205_add_two_factor_columns_to_users.rb | 1 + ...1125302_add_printing_merge_request_link_enabled_to_project.rb | 1 + ...0170305180853_add_auto_cancel_pending_pipelines_to_project.rb | 1 + ...0170315174634_revert_add_notified_of_own_activity_to_users.rb | 1 + 11 files changed, 11 insertions(+) diff --git a/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb b/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb index 69d64ccd006..22bac46e25c 100644 --- a/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb +++ b/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class AddOnlyAllowMergeIfBuildSucceedsToProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160608195742_add_repository_storage_to_projects.rb b/db/migrate/20160608195742_add_repository_storage_to_projects.rb index c700d2b569d..0f3664c13ef 100644 --- a/db/migrate/20160608195742_add_repository_storage_to_projects.rb +++ b/db/migrate/20160608195742_add_repository_storage_to_projects.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class AddRepositoryStorageToProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160715154212_add_request_access_enabled_to_projects.rb b/db/migrate/20160715154212_add_request_access_enabled_to_projects.rb index bf0131c6d76..5dc26f8982a 100644 --- a/db/migrate/20160715154212_add_request_access_enabled_to_projects.rb +++ b/db/migrate/20160715154212_add_request_access_enabled_to_projects.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class AddRequestAccessEnabledToProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160715204316_add_request_access_enabled_to_groups.rb b/db/migrate/20160715204316_add_request_access_enabled_to_groups.rb index e7b14cd3ee2..4a317646788 100644 --- a/db/migrate/20160715204316_add_request_access_enabled_to_groups.rb +++ b/db/migrate/20160715204316_add_request_access_enabled_to_groups.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class AddRequestAccessEnabledToGroups < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160831223750_remove_features_enabled_from_projects.rb b/db/migrate/20160831223750_remove_features_enabled_from_projects.rb index a2c207b49ea..7414a28ac97 100644 --- a/db/migrate/20160831223750_remove_features_enabled_from_projects.rb +++ b/db/migrate/20160831223750_remove_features_enabled_from_projects.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class RemoveFeaturesEnabledFromProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb b/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb index 18ea9d43a43..0100e30a733 100644 --- a/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb +++ b/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class RemoveProjectsPushesSinceGc < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb b/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb index df5cddeb205..ae37da275fd 100644 --- a/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb +++ b/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class AddTwoFactorColumnsToNamespaces < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170124193205_add_two_factor_columns_to_users.rb b/db/migrate/20170124193205_add_two_factor_columns_to_users.rb index 1d1021fcbb3..8d4aefa4365 100644 --- a/db/migrate/20170124193205_add_two_factor_columns_to_users.rb +++ b/db/migrate/20170124193205_add_two_factor_columns_to_users.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class AddTwoFactorColumnsToUsers < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb b/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb index f54608ecceb..7ad01a04815 100644 --- a/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb +++ b/db/migrate/20170301125302_add_printing_merge_request_link_enabled_to_project.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class AddPrintingMergeRequestLinkEnabledToProject < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb b/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb index aa64f2dddca..f335e77fb5e 100644 --- a/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb +++ b/db/migrate/20170305180853_add_auto_cancel_pending_pipelines_to_project.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class AddAutoCancelPendingPipelinesToProject < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb b/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb index b39c0a3be0f..6c9fe19ca34 100644 --- a/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb +++ b/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/AddColumnWithDefaultToLargeTable class RevertAddNotifiedOfOwnActivityToUsers < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! From e2c3f44d7c7b24064f41a573c0b4e00a262aedcb Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Sun, 30 Apr 2017 11:22:23 +0000 Subject: [PATCH 08/21] Fixed transient failure related to dropdown animations --- spec/features/merge_requests/create_new_mr_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb index 16b09933bda..e1353ec71ed 100644 --- a/spec/features/merge_requests/create_new_mr_spec.rb +++ b/spec/features/merge_requests/create_new_mr_spec.rb @@ -20,6 +20,7 @@ feature 'Create New Merge Request', feature: true, js: true do expect(page).to have_content('Target branch') first('.js-source-branch').click + first('.dropdown-source-branch .dropdown-content') find('.dropdown-source-branch .dropdown-content a', match: :first).click expect(page).to have_content "b83d6e3" From f1c9081f6bedfe92e86a8ed3eabe07e1847e14e4 Mon Sep 17 00:00:00 2001 From: Tsvi Mostovicz Date: Sun, 30 Apr 2017 11:44:50 +0000 Subject: [PATCH 09/21] Fix typo --- doc/user/profile/account/delete_account.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/profile/account/delete_account.md b/doc/user/profile/account/delete_account.md index 505248536c8..b5d3b009044 100644 --- a/doc/user/profile/account/delete_account.md +++ b/doc/user/profile/account/delete_account.md @@ -1,7 +1,7 @@ # Deleting a User Account - As a user, you can delete your own account by navigating to **Settings** > **Account** and selecting **Delete account** -- As an admin, you can delete a user account by navigating to the **Admin Area**, selecting the **Users** tab, selecting a user, and clicking on **Remvoe user** +- As an admin, you can delete a user account by navigating to the **Admin Area**, selecting the **Users** tab, selecting a user, and clicking on **Remove user** ## Associated Records From 60ebd101d0fa3ddbfab86a676b81efba589f8393 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 13 Apr 2017 11:47:28 -0500 Subject: [PATCH 10/21] Use blob viewers for snippets --- app/assets/javascripts/blob/viewer/index.js | 4 +- app/assets/javascripts/dispatcher.js | 6 + app/assets/javascripts/line_highlighter.js | 6 +- app/controllers/concerns/renders_blob.rb | 4 + app/controllers/projects/blob_controller.rb | 2 +- .../projects/snippets_controller.rb | 21 ++- app/controllers/snippets_controller.rb | 13 ++ app/helpers/blob_helper.rb | 14 +- app/models/snippet.rb | 26 +--- app/models/snippet_blob.rb | 59 ++++++++ app/views/projects/blob/_header.html.haml | 2 +- .../projects/blob/viewers/_markup.html.haml | 3 +- app/views/projects/snippets/show.html.haml | 2 +- .../search/results/_snippet_blob.html.haml | 2 +- app/views/shared/snippets/_blob.html.haml | 29 ++-- app/views/snippets/show.html.haml | 2 +- features/project/snippets.feature | 1 + features/snippets/snippets.feature | 1 + features/steps/project/snippets.rb | 5 +- features/steps/snippets/snippets.rb | 4 +- .../features/projects/blobs/blob_show_spec.rb | 13 +- spec/features/projects/snippets/show_spec.rb | 132 ++++++++++++++++++ spec/features/snippets/create_snippet_spec.rb | 8 +- .../features/snippets/public_snippets_spec.rb | 3 +- spec/features/snippets/show_spec.rb | 126 +++++++++++++++++ spec/helpers/blob_helper_spec.rb | 1 + .../fixtures/line_highlighter.html.haml | 2 +- spec/models/snippet_blob_spec.rb | 47 +++++++ spec/models/snippet_spec.rb | 13 +- .../projects/blob/_viewer.html.haml_spec.rb | 1 + 30 files changed, 477 insertions(+), 75 deletions(-) create mode 100644 app/models/snippet_blob.rb create mode 100644 spec/features/projects/snippets/show_spec.rb create mode 100644 spec/features/snippets/show_spec.rb create mode 100644 spec/models/snippet_blob_spec.rb diff --git a/app/assets/javascripts/blob/viewer/index.js b/app/assets/javascripts/blob/viewer/index.js index 7efa8537298..07d67d49aa5 100644 --- a/app/assets/javascripts/blob/viewer/index.js +++ b/app/assets/javascripts/blob/viewer/index.js @@ -6,7 +6,7 @@ export default class BlobViewer { this.copySourceBtn = document.querySelector('.js-copy-blob-source-btn'); this.simpleViewer = document.querySelector('.blob-viewer[data-type="simple"]'); this.richViewer = document.querySelector('.blob-viewer[data-type="rich"]'); - this.$blobContentHolder = $('#blob-content-holder'); + this.$fileHolder = $('.file-holder'); let initialViewerName = document.querySelector('.blob-viewer:not(.hidden)').getAttribute('data-type'); @@ -82,7 +82,7 @@ export default class BlobViewer { viewer.setAttribute('data-loaded', 'true'); - this.$blobContentHolder.trigger('highlight:line'); + this.$fileHolder.trigger('highlight:line'); this.toggleCopyButtonState(); }); diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index d3d75c4bf4a..15fe87f21ea 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -356,6 +356,10 @@ const ShortcutsBlob = require('./shortcuts_blob'); case 'users:show': new UserCallout(); break; + case 'snippets:show': + new LineHighlighter(); + new BlobViewer(); + break; } switch (path.first()) { case 'sessions': @@ -434,6 +438,8 @@ const ShortcutsBlob = require('./shortcuts_blob'); shortcut_handler = new ShortcutsNavigation(); if (path[2] === 'show') { new ZenMode(); + new LineHighlighter(); + new BlobViewer(); } break; case 'labels': diff --git a/app/assets/javascripts/line_highlighter.js b/app/assets/javascripts/line_highlighter.js index a6f7bea99f5..3ac6dedf131 100644 --- a/app/assets/javascripts/line_highlighter.js +++ b/app/assets/javascripts/line_highlighter.js @@ -57,9 +57,9 @@ require('vendor/jquery.scrollTo'); } LineHighlighter.prototype.bindEvents = function() { - const $blobContentHolder = $('#blob-content-holder'); - $blobContentHolder.on('click', 'a[data-line-number]', this.clickHandler); - $blobContentHolder.on('highlight:line', this.highlightHash); + const $fileHolder = $('.file-holder'); + $fileHolder.on('click', 'a[data-line-number]', this.clickHandler); + $fileHolder.on('highlight:line', this.highlightHash); }; LineHighlighter.prototype.highlightHash = function() { diff --git a/app/controllers/concerns/renders_blob.rb b/app/controllers/concerns/renders_blob.rb index d478c3bb6ca..9faf68e6d97 100644 --- a/app/controllers/concerns/renders_blob.rb +++ b/app/controllers/concerns/renders_blob.rb @@ -14,4 +14,8 @@ module RendersBlob html: view_to_html_string("projects/blob/_viewer", viewer: viewer, load_asynchronously: false) } end + + def override_max_blob_size(blob) + blob.override_max_size! if params[:override_max_size] == 'true' + end end diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index be5822b2cd4..9489bbddfc4 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -35,7 +35,7 @@ class Projects::BlobController < Projects::ApplicationController end def show - @blob.override_max_size! if params[:override_max_size] == 'true' + override_max_blob_size(@blob) respond_to do |format| format.html do diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index 5c9e0d4d1a1..66f913f8f9d 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -3,6 +3,7 @@ class Projects::SnippetsController < Projects::ApplicationController include ToggleAwardEmoji include SpammableActions include SnippetsActions + include RendersBlob before_action :module_enabled before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji, :mark_as_spam] @@ -55,11 +56,23 @@ class Projects::SnippetsController < Projects::ApplicationController end def show - @note = @project.notes.new(noteable: @snippet) - @noteable = @snippet + blob = @snippet.blob + override_max_blob_size(blob) - @discussions = @snippet.discussions - @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes)) + respond_to do |format| + format.html do + @note = @project.notes.new(noteable: @snippet) + @noteable = @snippet + + @discussions = @snippet.discussions + @notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes)) + render 'show' + end + + format.json do + render_blob_json(blob) + end + end end def destroy diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 056910fad67..906833505d1 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -3,6 +3,7 @@ class SnippetsController < ApplicationController include SpammableActions include SnippetsActions include MarkdownPreview + include RendersBlob before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :download] @@ -60,6 +61,18 @@ class SnippetsController < ApplicationController end def show + blob = @snippet.blob + override_max_blob_size(blob) + + respond_to do |format| + format.html do + render 'show' + end + + format.json do + render_blob_json(blob) + end + end end def destroy diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index cc47654dc06..0b80747f001 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -119,7 +119,15 @@ module BlobHelper end def blob_raw_url - namespace_project_raw_path(@project.namespace, @project, @id) + if @snippet + if @snippet.project_id + raw_namespace_project_snippet_path(@project.namespace, @project, @snippet) + else + raw_snippet_path(@snippet) + end + elsif @blob + namespace_project_raw_path(@project.namespace, @project, @id) + end end # SVGs can contain malicious JavaScript; only include whitelisted @@ -212,8 +220,8 @@ module BlobHelper clipboard_button(target: ".blob-content[data-blob-id='#{blob.id}']", class: "btn btn-sm js-copy-blob-source-btn", title: "Copy source to clipboard") end - def open_raw_file_button(path) - link_to icon('file-code-o'), path, class: 'btn btn-sm has-tooltip', target: '_blank', rel: 'noopener noreferrer', title: 'Open raw', data: { container: 'body' } + def open_raw_blob_button + link_to icon('file-code-o'), blob_raw_url, class: 'btn btn-sm has-tooltip', target: '_blank', rel: 'noopener noreferrer', title: 'Open raw', data: { container: 'body' } end def blob_render_error_reason(viewer) diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 380835707e8..d8860718cb5 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -1,6 +1,5 @@ class Snippet < ActiveRecord::Base include Gitlab::VisibilityLevel - include Linguist::BlobHelper include CacheMarkdownField include Noteable include Participable @@ -87,47 +86,26 @@ class Snippet < ActiveRecord::Base ] end - def data - content + def blob + @blob ||= Blob.decorate(SnippetBlob.new(self), nil) end def hook_attrs attributes end - def size - 0 - end - def file_name super.to_s end - # alias for compatibility with blobs and highlighting - def path - file_name - end - - def name - file_name - end - def sanitized_file_name file_name.gsub(/[^a-zA-Z0-9_\-\.]+/, '') end - def mode - nil - end - def visibility_level_field :visibility_level end - def no_highlighting? - content.lines.count > 1000 - end - def notes_with_associations notes.includes(:author) end diff --git a/app/models/snippet_blob.rb b/app/models/snippet_blob.rb new file mode 100644 index 00000000000..d6cab74eb1a --- /dev/null +++ b/app/models/snippet_blob.rb @@ -0,0 +1,59 @@ +class SnippetBlob + include Linguist::BlobHelper + + attr_reader :snippet + + def initialize(snippet) + @snippet = snippet + end + + delegate :id, to: :snippet + + def name + snippet.file_name + end + + alias_method :path, :name + + def size + data.bytesize + end + + def data + snippet.content + end + + def rendered_markup + return unless Gitlab::MarkupHelper.gitlab_markdown?(name) + + Banzai.render_field(snippet, :content) + end + + def mode + nil + end + + def binary? + false + end + + def load_all_data!(repository) + # No-op + end + + def lfs_pointer? + false + end + + def lfs_oid + nil + end + + def lfs_size + nil + end + + def truncated? + false + end +end diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml index b89cd460455..638f8cef3bd 100644 --- a/app/views/projects/blob/_header.html.haml +++ b/app/views/projects/blob/_header.html.haml @@ -16,7 +16,7 @@ .btn-group{ role: "group" }< = copy_blob_source_button(blob) if !blame && blob.rendered_as_text?(ignore_errors: false) - = open_raw_file_button(namespace_project_raw_path(@project.namespace, @project, @id)) + = open_raw_blob_button = view_on_environment_button(@commit.sha, @path, @environment) if @environment .btn-group{ role: "group" }< diff --git a/app/views/projects/blob/viewers/_markup.html.haml b/app/views/projects/blob/viewers/_markup.html.haml index b9a998d96ff..230305b488d 100644 --- a/app/views/projects/blob/viewers/_markup.html.haml +++ b/app/views/projects/blob/viewers/_markup.html.haml @@ -1,3 +1,4 @@ - blob = viewer.blob +- rendered_markup = blob.rendered_markup if blob.respond_to?(:rendered_markup) .file-content.wiki - = markup(blob.name, blob.data) + = markup(blob.name, blob.data, rendered: rendered_markup) diff --git a/app/views/projects/snippets/show.html.haml b/app/views/projects/snippets/show.html.haml index 7c6be003d4c..7a175f63eeb 100644 --- a/app/views/projects/snippets/show.html.haml +++ b/app/views/projects/snippets/show.html.haml @@ -4,7 +4,7 @@ .project-snippets %article.file-holder.snippet-file-content - = render 'shared/snippets/blob', raw_path: raw_namespace_project_snippet_path(@project.namespace, @project, @snippet) + = render 'shared/snippets/blob' .row-content-block.top-block.content-component-block = render 'award_emoji/awards_block', awardable: @snippet, inline: true diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml index f2fe5742c12..c4a5131c1a7 100644 --- a/app/views/search/results/_snippet_blob.html.haml +++ b/app/views/search/results/_snippet_blob.html.haml @@ -39,7 +39,7 @@ .blob-content - snippet_chunks.each do |chunk| - unless chunk[:data].empty? - = highlight(snippet.file_name, chunk[:data], repository: nil, plain: snippet.no_highlighting?) + = highlight(snippet.file_name, chunk[:data], repository: nil, plain: snippet.blob.no_highlighting?) - else .file-content.code .nothing-here-block Empty file diff --git a/app/views/shared/snippets/_blob.html.haml b/app/views/shared/snippets/_blob.html.haml index 37c66ff2595..0c481120969 100644 --- a/app/views/shared/snippets/_blob.html.haml +++ b/app/views/shared/snippets/_blob.html.haml @@ -1,29 +1,24 @@ +- blob = @snippet.blob .js-file-title.file-title-flex-parent .file-header-content - = blob_icon @snippet.mode, @snippet.path + = blob_icon blob.mode, blob.path %strong.file-title-name - = @snippet.path + = blob.path - = copy_file_path_button(@snippet.path) + = copy_file_path_button(blob.path) + + %small + = number_to_human_size(blob.raw_size) .file-actions.hidden-xs + = render 'projects/blob/viewer_switcher', blob: blob + .btn-group{ role: "group" }< - = copy_blob_source_button(@snippet) - = open_raw_file_button(raw_path) + = copy_blob_source_button(blob) + = open_raw_blob_button - if defined?(download_path) && download_path = link_to icon('download'), download_path, class: "btn btn-sm has-tooltip", title: 'Download', data: { container: 'body' } -- if @snippet.content.empty? - .file-content.code - .nothing-here-block Empty file -- else - - if markup?(@snippet.file_name) - .file-content.wiki - - if gitlab_markdown?(@snippet.file_name) - = preserve(markdown_field(@snippet, :content)) - - else - = markup(@snippet.file_name, @snippet.content) - - else - = render 'shared/file_highlight', blob: @snippet += render 'projects/blob/content', blob: blob diff --git a/app/views/snippets/show.html.haml b/app/views/snippets/show.html.haml index e5711ca79c7..8a80013bbfd 100644 --- a/app/views/snippets/show.html.haml +++ b/app/views/snippets/show.html.haml @@ -3,7 +3,7 @@ = render 'shared/snippets/header' %article.file-holder.snippet-file-content - = render 'shared/snippets/blob', raw_path: raw_snippet_path(@snippet), download_path: download_snippet_path(@snippet) + = render 'shared/snippets/blob', download_path: download_snippet_path(@snippet) .row-content-block.top-block.content-component-block = render 'award_emoji/awards_block', awardable: @snippet, inline: true diff --git a/features/project/snippets.feature b/features/project/snippets.feature index 3c51ea56585..50bc4c93df3 100644 --- a/features/project/snippets.feature +++ b/features/project/snippets.feature @@ -11,6 +11,7 @@ Feature: Project Snippets Then I should see "Snippet one" in snippets And I should not see "Snippet two" in snippets + @javascript Scenario: I create new project snippet Given I click link "New snippet" And I submit new snippet "Snippet three" diff --git a/features/snippets/snippets.feature b/features/snippets/snippets.feature index e15d7c79342..1ad02780229 100644 --- a/features/snippets/snippets.feature +++ b/features/snippets/snippets.feature @@ -5,6 +5,7 @@ Feature: Snippets And I have public "Personal snippet one" snippet And I have private "Personal snippet private" snippet + @javascript Scenario: I create new snippet Given I visit new snippet page And I submit new snippet "Personal snippet three" diff --git a/features/steps/project/snippets.rb b/features/steps/project/snippets.rb index a3bebfa4b71..60febd20104 100644 --- a/features/steps/project/snippets.rb +++ b/features/steps/project/snippets.rb @@ -3,6 +3,7 @@ class Spinach::Features::ProjectSnippets < Spinach::FeatureSteps include SharedProject include SharedNote include SharedPaths + include WaitForAjax step 'project "Shop" have "Snippet one" snippet' do create(:project_snippet, @@ -55,9 +56,10 @@ class Spinach::Features::ProjectSnippets < Spinach::FeatureSteps fill_in "project_snippet_title", with: "Snippet three" fill_in "project_snippet_file_name", with: "my_snippet.rb" page.within('.file-editor') do - find(:xpath, "//input[@id='project_snippet_content']").set 'Content of snippet three' + find('.ace_editor').native.send_keys 'Content of snippet three' end click_button "Create snippet" + wait_for_ajax end step 'I should see snippet "Snippet three"' do @@ -79,6 +81,7 @@ class Spinach::Features::ProjectSnippets < Spinach::FeatureSteps fill_in "note_note", with: "Good snippet!" click_button "Comment" end + wait_for_ajax end step 'I should see comment "Good snippet!"' do diff --git a/features/steps/snippets/snippets.rb b/features/steps/snippets/snippets.rb index 19366b11071..0b3e942a4fd 100644 --- a/features/steps/snippets/snippets.rb +++ b/features/steps/snippets/snippets.rb @@ -3,6 +3,7 @@ class Spinach::Features::Snippets < Spinach::FeatureSteps include SharedPaths include SharedProject include SharedSnippet + include WaitForAjax step 'I click link "Personal snippet one"' do click_link "Personal snippet one" @@ -26,9 +27,10 @@ class Spinach::Features::Snippets < Spinach::FeatureSteps fill_in "personal_snippet_title", with: "Personal snippet three" fill_in "personal_snippet_file_name", with: "my_snippet.rb" page.within('.file-editor') do - find(:xpath, "//input[@id='personal_snippet_content']").set 'Content of snippet three' + find('.ace_editor').native.send_keys 'Content of snippet three' end click_button "Create snippet" + wait_for_ajax end step 'I submit new internal snippet' do diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index cc11cb7a55f..8613b850203 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -1,13 +1,10 @@ require 'spec_helper' feature 'File blob', :js, feature: true do - include TreeHelper - include WaitForAjax - let(:project) { create(:project, :public) } def visit_blob(path, fragment = nil) - visit namespace_project_blob_path(project.namespace, project, tree_join('master', path), anchor: fragment) + visit namespace_project_blob_path(project.namespace, project, File.join('master', path), anchor: fragment) end context 'Ruby file' do @@ -39,7 +36,7 @@ feature 'File blob', :js, feature: true do wait_for_ajax end - it 'displays the blob' do + it 'displays the blob using the rich viewer' do aggregate_failures do # hides the simple viewer expect(page).to have_selector('.blob-viewer[data-type="simple"]', visible: false) @@ -63,7 +60,7 @@ feature 'File blob', :js, feature: true do wait_for_ajax end - it 'displays the blob' do + it 'displays the blob using the simple viewer' do aggregate_failures do # hides the rich viewer expect(page).to have_selector('.blob-viewer[data-type="simple"]') @@ -84,7 +81,7 @@ feature 'File blob', :js, feature: true do wait_for_ajax end - it 'displays the blob' do + it 'displays the blob using the rich viewer' do aggregate_failures do # hides the simple viewer expect(page).to have_selector('.blob-viewer[data-type="simple"]', visible: false) @@ -105,7 +102,7 @@ feature 'File blob', :js, feature: true do wait_for_ajax end - it 'displays the blob' do + it 'displays the blob using the simple viewer' do aggregate_failures do # hides the rich viewer expect(page).to have_selector('.blob-viewer[data-type="simple"]') diff --git a/spec/features/projects/snippets/show_spec.rb b/spec/features/projects/snippets/show_spec.rb new file mode 100644 index 00000000000..7eb1210e307 --- /dev/null +++ b/spec/features/projects/snippets/show_spec.rb @@ -0,0 +1,132 @@ +require 'spec_helper' + +feature 'Project snippet', :js, feature: true do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:snippet) { create(:project_snippet, project: project, file_name: file_name, content: content) } + + before do + project.team << [user, :master] + login_as(user) + end + + context 'Ruby file' do + let(:file_name) { 'popen.rb' } + let(:content) { project.repository.blob_at('master', 'files/ruby/popen.rb').data } + + before do + visit namespace_project_snippet_path(project.namespace, project, snippet) + + wait_for_ajax + end + + it 'displays the blob' do + aggregate_failures do + # shows highlighted Ruby code + expect(page).to have_content("require 'fileutils'") + + # does not show a viewer switcher + expect(page).not_to have_selector('.js-blob-viewer-switcher') + + # shows an enabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + end + end + end + + context 'Markdown file' do + let(:file_name) { 'ruby-style-guide.md' } + let(:content) { project.repository.blob_at('master', 'files/markdown/ruby-style-guide.md').data } + + context 'visiting directly' do + before do + visit namespace_project_snippet_path(project.namespace, project, snippet) + + wait_for_ajax + end + + it 'displays the blob using the rich viewer' do + aggregate_failures do + # hides the simple viewer + expect(page).to have_selector('.blob-viewer[data-type="simple"]', visible: false) + expect(page).to have_selector('.blob-viewer[data-type="rich"]') + + # shows rendered Markdown + expect(page).to have_link("PEP-8") + + # shows a viewer switcher + expect(page).to have_selector('.js-blob-viewer-switcher') + + # shows a disabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn.disabled') + end + end + + context 'switching to the simple viewer' do + before do + find('.js-blob-viewer-switch-btn[data-viewer=simple]').click + + wait_for_ajax + end + + it 'displays the blob using the simple viewer' do + aggregate_failures do + # hides the rich viewer + expect(page).to have_selector('.blob-viewer[data-type="simple"]') + expect(page).to have_selector('.blob-viewer[data-type="rich"]', visible: false) + + # shows highlighted Markdown code + expect(page).to have_content("[PEP-8](http://www.python.org/dev/peps/pep-0008/)") + + # shows an enabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + end + end + + context 'switching to the rich viewer again' do + before do + find('.js-blob-viewer-switch-btn[data-viewer=rich]').click + + wait_for_ajax + end + + it 'displays the blob using the rich viewer' do + aggregate_failures do + # hides the simple viewer + expect(page).to have_selector('.blob-viewer[data-type="simple"]', visible: false) + expect(page).to have_selector('.blob-viewer[data-type="rich"]') + + # shows an enabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + end + end + end + end + end + + context 'visiting with a line number anchor' do + before do + visit namespace_project_snippet_path(project.namespace, project, snippet, anchor: 'L1') + + wait_for_ajax + end + + it 'displays the blob using the simple viewer' do + aggregate_failures do + # hides the rich viewer + expect(page).to have_selector('.blob-viewer[data-type="simple"]') + expect(page).to have_selector('.blob-viewer[data-type="rich"]', visible: false) + + # highlights the line in question + expect(page).to have_selector('#LC1.hll') + + # shows highlighted Markdown code + expect(page).to have_content("[PEP-8](http://www.python.org/dev/peps/pep-0008/)") + + # shows an enabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + end + end + end + end +end diff --git a/spec/features/snippets/create_snippet_spec.rb b/spec/features/snippets/create_snippet_spec.rb index 5470276bf06..9409c323288 100644 --- a/spec/features/snippets/create_snippet_spec.rb +++ b/spec/features/snippets/create_snippet_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -feature 'Create Snippet', feature: true do +feature 'Create Snippet', :js, feature: true do before do login_as :user visit new_snippet_path @@ -9,10 +9,11 @@ feature 'Create Snippet', feature: true do scenario 'Authenticated user creates a snippet' do fill_in 'personal_snippet_title', with: 'My Snippet Title' page.within('.file-editor') do - find(:xpath, "//input[@id='personal_snippet_content']").set 'Hello World!' + find('.ace_editor').native.send_keys 'Hello World!' end click_button 'Create snippet' + wait_for_ajax expect(page).to have_content('My Snippet Title') expect(page).to have_content('Hello World!') @@ -22,10 +23,11 @@ feature 'Create Snippet', feature: true do fill_in 'personal_snippet_title', with: 'My Snippet Title' page.within('.file-editor') do find(:xpath, "//input[@id='personal_snippet_file_name']").set 'snippet+file+name' - find(:xpath, "//input[@id='personal_snippet_content']").set 'Hello World!' + find('.ace_editor').native.send_keys 'Hello World!' end click_button 'Create snippet' + wait_for_ajax expect(page).to have_content('My Snippet Title') expect(page).to have_content('snippet+file+name') diff --git a/spec/features/snippets/public_snippets_spec.rb b/spec/features/snippets/public_snippets_spec.rb index 34300ccb940..2df483818c3 100644 --- a/spec/features/snippets/public_snippets_spec.rb +++ b/spec/features/snippets/public_snippets_spec.rb @@ -1,10 +1,11 @@ require 'rails_helper' -feature 'Public Snippets', feature: true do +feature 'Public Snippets', :js, feature: true do scenario 'Unauthenticated user should see public snippets' do public_snippet = create(:personal_snippet, :public) visit snippet_path(public_snippet) + wait_for_ajax expect(page).to have_content(public_snippet.content) end diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb new file mode 100644 index 00000000000..cebcba6a230 --- /dev/null +++ b/spec/features/snippets/show_spec.rb @@ -0,0 +1,126 @@ +require 'spec_helper' + +feature 'Snippet', :js, feature: true do + let(:project) { create(:project, :repository) } + let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content) } + + context 'Ruby file' do + let(:file_name) { 'popen.rb' } + let(:content) { project.repository.blob_at('master', 'files/ruby/popen.rb').data } + + before do + visit snippet_path(snippet) + + wait_for_ajax + end + + it 'displays the blob' do + aggregate_failures do + # shows highlighted Ruby code + expect(page).to have_content("require 'fileutils'") + + # does not show a viewer switcher + expect(page).not_to have_selector('.js-blob-viewer-switcher') + + # shows an enabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + end + end + end + + context 'Markdown file' do + let(:file_name) { 'ruby-style-guide.md' } + let(:content) { project.repository.blob_at('master', 'files/markdown/ruby-style-guide.md').data } + + context 'visiting directly' do + before do + visit snippet_path(snippet) + + wait_for_ajax + end + + it 'displays the blob using the rich viewer' do + aggregate_failures do + # hides the simple viewer + expect(page).to have_selector('.blob-viewer[data-type="simple"]', visible: false) + expect(page).to have_selector('.blob-viewer[data-type="rich"]') + + # shows rendered Markdown + expect(page).to have_link("PEP-8") + + # shows a viewer switcher + expect(page).to have_selector('.js-blob-viewer-switcher') + + # shows a disabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn.disabled') + end + end + + context 'switching to the simple viewer' do + before do + find('.js-blob-viewer-switch-btn[data-viewer=simple]').click + + wait_for_ajax + end + + it 'displays the blob using the simple viewer' do + aggregate_failures do + # hides the rich viewer + expect(page).to have_selector('.blob-viewer[data-type="simple"]') + expect(page).to have_selector('.blob-viewer[data-type="rich"]', visible: false) + + # shows highlighted Markdown code + expect(page).to have_content("[PEP-8](http://www.python.org/dev/peps/pep-0008/)") + + # shows an enabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + end + end + + context 'switching to the rich viewer again' do + before do + find('.js-blob-viewer-switch-btn[data-viewer=rich]').click + + wait_for_ajax + end + + it 'displays the blob using the rich viewer' do + aggregate_failures do + # hides the simple viewer + expect(page).to have_selector('.blob-viewer[data-type="simple"]', visible: false) + expect(page).to have_selector('.blob-viewer[data-type="rich"]') + + # shows an enabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + end + end + end + end + end + + context 'visiting with a line number anchor' do + before do + visit snippet_path(snippet, anchor: 'L1') + + wait_for_ajax + end + + it 'displays the blob using the simple viewer' do + aggregate_failures do + # hides the rich viewer + expect(page).to have_selector('.blob-viewer[data-type="simple"]') + expect(page).to have_selector('.blob-viewer[data-type="rich"]', visible: false) + + # highlights the line in question + expect(page).to have_selector('#LC1.hll') + + # shows highlighted Markdown code + expect(page).to have_content("[PEP-8](http://www.python.org/dev/peps/pep-0008/)") + + # shows an enabled copy button + expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + end + end + end + end +end diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index 379f62f73e1..075f1887d91 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -157,6 +157,7 @@ describe BlobHelper do describe '#blob_render_error_options' do before do assign(:project, project) + assign(:blob, blob) assign(:id, File.join('master', blob.path)) controller.params[:controller] = 'projects/blob' diff --git a/spec/javascripts/fixtures/line_highlighter.html.haml b/spec/javascripts/fixtures/line_highlighter.html.haml index 514877340e4..2782c50e298 100644 --- a/spec/javascripts/fixtures/line_highlighter.html.haml +++ b/spec/javascripts/fixtures/line_highlighter.html.haml @@ -1,4 +1,4 @@ -#blob-content-holder +.file-holder .file-content .line-numbers - 1.upto(25) do |i| diff --git a/spec/models/snippet_blob_spec.rb b/spec/models/snippet_blob_spec.rb new file mode 100644 index 00000000000..120b390586b --- /dev/null +++ b/spec/models/snippet_blob_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe SnippetBlob, models: true do + let(:snippet) { create(:snippet) } + + subject { described_class.new(snippet) } + + describe '#id' do + it 'returns the snippet ID' do + expect(subject.id).to eq(snippet.id) + end + end + + describe '#name' do + it 'returns the snippet file name' do + expect(subject.name).to eq(snippet.file_name) + end + end + + describe '#size' do + it 'returns the data size' do + expect(subject.size).to eq(subject.data.bytesize) + end + end + + describe '#data' do + it 'returns the snippet content' do + expect(subject.data).to eq(snippet.content) + end + end + + describe '#rendered_markup' do + context 'when the content is GFM' do + let(:snippet) { create(:snippet, file_name: 'file.md') } + + it 'returns the rendered GFM' do + expect(subject.rendered_markup).to eq(snippet.content_html) + end + end + + context 'when the content is not GFM' do + it 'returns nil' do + expect(subject.rendered_markup).to be_nil + end + end + end +end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 8095d01b69e..75b1fc7e216 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -5,7 +5,6 @@ describe Snippet, models: true do subject { described_class } it { is_expected.to include_module(Gitlab::VisibilityLevel) } - it { is_expected.to include_module(Linguist::BlobHelper) } it { is_expected.to include_module(Participable) } it { is_expected.to include_module(Referable) } it { is_expected.to include_module(Sortable) } @@ -241,4 +240,16 @@ describe Snippet, models: true do end end end + + describe '#blob' do + let(:snippet) { create(:snippet) } + + it 'returns a blob representing the snippet data' do + blob = snippet.blob + + expect(blob).to be_a(Blob) + expect(blob.path).to eq(snippet.file_name) + expect(blob.data).to eq(snippet.content) + end + end end diff --git a/spec/views/projects/blob/_viewer.html.haml_spec.rb b/spec/views/projects/blob/_viewer.html.haml_spec.rb index a4915264abe..501f90c5f9a 100644 --- a/spec/views/projects/blob/_viewer.html.haml_spec.rb +++ b/spec/views/projects/blob/_viewer.html.haml_spec.rb @@ -21,6 +21,7 @@ describe 'projects/blob/_viewer.html.haml', :view do before do assign(:project, project) + assign(:blob, blob) assign(:id, File.join('master', blob.path)) controller.params[:controller] = 'projects/blob' From 54040ce0662c71cfaee3cc12305b5ab021beafbb Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 13 Apr 2017 12:11:52 -0500 Subject: [PATCH 11/21] Show Raw button as Download for binary files --- app/helpers/blob_helper.rb | 14 +++++++++-- app/views/projects/blob/_header.html.haml | 4 ++-- app/views/shared/snippets/_blob.html.haml | 2 +- features/steps/project/source/browse_files.rb | 2 +- .../features/projects/blobs/blob_show_spec.rb | 24 +++++++++++++++++++ 5 files changed, 40 insertions(+), 6 deletions(-) diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 0b80747f001..377b080b3c6 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -217,11 +217,21 @@ module BlobHelper end def copy_blob_source_button(blob) + return unless blob.rendered_as_text?(ignore_errors: false) + clipboard_button(target: ".blob-content[data-blob-id='#{blob.id}']", class: "btn btn-sm js-copy-blob-source-btn", title: "Copy source to clipboard") end - def open_raw_blob_button - link_to icon('file-code-o'), blob_raw_url, class: 'btn btn-sm has-tooltip', target: '_blank', rel: 'noopener noreferrer', title: 'Open raw', data: { container: 'body' } + def open_raw_blob_button(blob) + if blob.raw_binary? + icon = icon('download') + title = 'Download' + else + icon = icon('file-code-o') + title = 'Open raw' + end + + link_to icon, blob_raw_url, class: 'btn btn-sm has-tooltip', target: '_blank', rel: 'noopener noreferrer', title: title, data: { container: 'body' } end def blob_render_error_reason(viewer) diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml index 638f8cef3bd..219dc14645b 100644 --- a/app/views/projects/blob/_header.html.haml +++ b/app/views/projects/blob/_header.html.haml @@ -15,8 +15,8 @@ = render 'projects/blob/viewer_switcher', blob: blob unless blame .btn-group{ role: "group" }< - = copy_blob_source_button(blob) if !blame && blob.rendered_as_text?(ignore_errors: false) - = open_raw_blob_button + = copy_blob_source_button(blob) unless blame + = open_raw_blob_button(blob) = view_on_environment_button(@commit.sha, @path, @environment) if @environment .btn-group{ role: "group" }< diff --git a/app/views/shared/snippets/_blob.html.haml b/app/views/shared/snippets/_blob.html.haml index 0c481120969..67d186e2874 100644 --- a/app/views/shared/snippets/_blob.html.haml +++ b/app/views/shared/snippets/_blob.html.haml @@ -16,7 +16,7 @@ .btn-group{ role: "group" }< = copy_blob_source_button(blob) - = open_raw_blob_button + = open_raw_blob_button(blob) - if defined?(download_path) && download_path = link_to icon('download'), download_path, class: "btn btn-sm has-tooltip", title: 'Download', data: { container: 'body' } diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index 36fe21a047c..ef09bddddd8 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -367,7 +367,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps step 'I should see buttons for allowed commands' do page.within '.content' do - expect(page).to have_link 'Open raw' + expect(page).to have_link 'Download' expect(page).to have_content 'History' expect(page).to have_content 'Permalink' expect(page).not_to have_content 'Edit' diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 8613b850203..6a6f8b4f4d5 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -24,6 +24,9 @@ feature 'File blob', :js, feature: true do # shows an enabled copy button expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + + # shows a raw button + expect(page).to have_link('Open raw') end end end @@ -50,6 +53,9 @@ feature 'File blob', :js, feature: true do # shows a disabled copy button expect(page).to have_selector('.js-copy-blob-source-btn.disabled') + + # shows a raw button + expect(page).to have_link('Open raw') end end @@ -160,6 +166,9 @@ feature 'File blob', :js, feature: true do # does not show a copy button expect(page).not_to have_selector('.js-copy-blob-source-btn') + + # shows a raw button + expect(page).to have_link('Open raw') end end @@ -203,6 +212,9 @@ feature 'File blob', :js, feature: true do # shows an enabled copy button expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + + # shows a raw button + expect(page).to have_link('Open raw') end end end @@ -237,6 +249,9 @@ feature 'File blob', :js, feature: true do # does not show a copy button expect(page).not_to have_selector('.js-copy-blob-source-btn') + + # shows a download button + expect(page).to have_link('Download') end end end @@ -262,6 +277,9 @@ feature 'File blob', :js, feature: true do # does not show a copy button expect(page).not_to have_selector('.js-copy-blob-source-btn') + + # shows a download button + expect(page).to have_link('Download') end end end @@ -283,6 +301,9 @@ feature 'File blob', :js, feature: true do # shows an enabled copy button expect(page).to have_selector('.js-copy-blob-source-btn:not(.disabled)') + + # shows a raw button + expect(page).to have_link('Open raw') end end end @@ -305,6 +326,9 @@ feature 'File blob', :js, feature: true do # does not show a copy button expect(page).not_to have_selector('.js-copy-blob-source-btn') + + # shows a download button + expect(page).to have_link('Download') end end end From 1686ce039c6823e7974ee5813d00f54e7067aa48 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 30 Apr 2017 14:22:52 -0500 Subject: [PATCH 12/21] Add changelog --- changelogs/unreleased/dm-blob-download-button.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/dm-blob-download-button.yml diff --git a/changelogs/unreleased/dm-blob-download-button.yml b/changelogs/unreleased/dm-blob-download-button.yml new file mode 100644 index 00000000000..bd31137b670 --- /dev/null +++ b/changelogs/unreleased/dm-blob-download-button.yml @@ -0,0 +1,4 @@ +--- +title: Show Raw button as Download for binary files +merge_request: +author: From 582a8c6bf4e71609a829483dab72af9fd9733457 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sun, 30 Apr 2017 14:24:05 -0500 Subject: [PATCH 13/21] Add changelog --- changelogs/unreleased/dm-snippet-blob-viewers.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/dm-snippet-blob-viewers.yml diff --git a/changelogs/unreleased/dm-snippet-blob-viewers.yml b/changelogs/unreleased/dm-snippet-blob-viewers.yml new file mode 100644 index 00000000000..f218095f401 --- /dev/null +++ b/changelogs/unreleased/dm-snippet-blob-viewers.yml @@ -0,0 +1,4 @@ +--- +title: Use blob viewers for snippets +merge_request: +author: From 6277bda61c511696f9d12fae4238b5214a722571 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Mon, 1 May 2017 12:06:40 +0000 Subject: [PATCH 14/21] Update CHANGELOG.md for 9.1.2 [ci skip] --- CHANGELOG.md | 11 +++++++++++ .../2246-uuid-is-nil-for-new-installation.yml | 4 ---- .../30645-show-pipeline-events-description.yml | 4 ---- .../unreleased/30973-fix-network-graph-ordering.yml | 4 ---- ...r-display-incorect-number-of-mr-when-minimized.yml | 4 ---- .../add_index_on_ci_runners_contacted_at.yml | 4 ---- .../unreleased/dm-fix-ghost-user-validation.yml | 4 ---- changelogs/unreleased/pages-0-4-1.yml | 4 ---- .../unreleased/zj-accept-default-branch-param.yml | 4 ---- 9 files changed, 11 insertions(+), 32 deletions(-) delete mode 100644 changelogs/unreleased/2246-uuid-is-nil-for-new-installation.yml delete mode 100644 changelogs/unreleased/30645-show-pipeline-events-description.yml delete mode 100644 changelogs/unreleased/30973-fix-network-graph-ordering.yml delete mode 100644 changelogs/unreleased/31292-milestone-sidebar-display-incorect-number-of-mr-when-minimized.yml delete mode 100644 changelogs/unreleased/add_index_on_ci_runners_contacted_at.yml delete mode 100644 changelogs/unreleased/dm-fix-ghost-user-validation.yml delete mode 100644 changelogs/unreleased/pages-0-4-1.yml delete mode 100644 changelogs/unreleased/zj-accept-default-branch-param.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index a47c43dd5d6..2686d778b09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 9.1.2 (2017-05-01) + +- Add index on ci_runners.contacted_at. !10876 (blackst0ne) +- Fix pipeline events description for Slack and Mattermost integration. !10908 +- Fixed milestone sidebar showing incorrect number of MRs when collapsed. !10933 +- Fix ordering of commits in the network graph. !10936 +- Ensure the chat notifications service properly saves the "Notify only default branch" setting. !10959 +- Lazily sets UUID in ApplicationSetting for new installations. +- Skip validation when creating internal (ghost, service desk) users. +- Use GitLab Pages v0.4.1. + ## 9.1.1 (2017-04-26) - Add a transaction around move_issues_to_ghost_user. !10465 diff --git a/changelogs/unreleased/2246-uuid-is-nil-for-new-installation.yml b/changelogs/unreleased/2246-uuid-is-nil-for-new-installation.yml deleted file mode 100644 index 70d35f06af4..00000000000 --- a/changelogs/unreleased/2246-uuid-is-nil-for-new-installation.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Lazily sets UUID in ApplicationSetting for new installations -merge_request: -author: diff --git a/changelogs/unreleased/30645-show-pipeline-events-description.yml b/changelogs/unreleased/30645-show-pipeline-events-description.yml deleted file mode 100644 index fb75dde1d86..00000000000 --- a/changelogs/unreleased/30645-show-pipeline-events-description.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Fix pipeline events description for Slack and Mattermost integration -merge_request: 10908 -author: diff --git a/changelogs/unreleased/30973-fix-network-graph-ordering.yml b/changelogs/unreleased/30973-fix-network-graph-ordering.yml deleted file mode 100644 index 420ec107842..00000000000 --- a/changelogs/unreleased/30973-fix-network-graph-ordering.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Fix ordering of commits in the network graph -merge_request: 10936 -author: diff --git a/changelogs/unreleased/31292-milestone-sidebar-display-incorect-number-of-mr-when-minimized.yml b/changelogs/unreleased/31292-milestone-sidebar-display-incorect-number-of-mr-when-minimized.yml deleted file mode 100644 index dee831c668b..00000000000 --- a/changelogs/unreleased/31292-milestone-sidebar-display-incorect-number-of-mr-when-minimized.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Fixed milestone sidebar showing incorrect number of MRs when collapsed -merge_request: 10933 -author: diff --git a/changelogs/unreleased/add_index_on_ci_runners_contacted_at.yml b/changelogs/unreleased/add_index_on_ci_runners_contacted_at.yml deleted file mode 100644 index 10c3206c2ff..00000000000 --- a/changelogs/unreleased/add_index_on_ci_runners_contacted_at.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Add index on ci_runners.contacted_at -merge_request: 10876 -author: blackst0ne diff --git a/changelogs/unreleased/dm-fix-ghost-user-validation.yml b/changelogs/unreleased/dm-fix-ghost-user-validation.yml deleted file mode 100644 index 4214786cb5a..00000000000 --- a/changelogs/unreleased/dm-fix-ghost-user-validation.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Skip validation when creating internal (ghost, service desk) users -merge_request: -author: diff --git a/changelogs/unreleased/pages-0-4-1.yml b/changelogs/unreleased/pages-0-4-1.yml deleted file mode 100644 index fbc78a36cae..00000000000 --- a/changelogs/unreleased/pages-0-4-1.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Use GitLab Pages v0.4.1 -merge_request: -author: diff --git a/changelogs/unreleased/zj-accept-default-branch-param.yml b/changelogs/unreleased/zj-accept-default-branch-param.yml deleted file mode 100644 index 8f6fa8a6386..00000000000 --- a/changelogs/unreleased/zj-accept-default-branch-param.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Ensure the chat notifications service properly saves the "Notify only default branch" setting -merge_request: 10959 -author: From 08b1380ff712f46a6005d366a9159a3e81674fb7 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 1 May 2017 14:25:04 +0100 Subject: [PATCH 15/21] Don't blow up when email has no References header If an email doesn't match our incoming email patterns on the To header, we fall back to the References header. If there was no References header, we'd raise an exception, when we'd be better off acting as if it was empty. --- ...ont-blow-up-when-email-has-no-references-header.yml | 5 +++++ lib/gitlab/email/receiver.rb | 2 ++ spec/lib/gitlab/email/receiver_spec.rb | 10 +++++++++- 3 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/dont-blow-up-when-email-has-no-references-header.yml diff --git a/changelogs/unreleased/dont-blow-up-when-email-has-no-references-header.yml b/changelogs/unreleased/dont-blow-up-when-email-has-no-references-header.yml new file mode 100644 index 00000000000..a4345b70744 --- /dev/null +++ b/changelogs/unreleased/dont-blow-up-when-email-has-no-references-header.yml @@ -0,0 +1,5 @@ +--- +title: Gracefully handle failures for incoming emails which do not match on the To + header, and have no References header +merge_request: +author: diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index 419d56a51e0..c270c0ea9ff 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -70,6 +70,8 @@ module Gitlab # Handle emails from clients which append with commas, # example clients are Microsoft exchange and iOS app Gitlab::IncomingEmail.scan_fallback_references(references) + when nil + [] end end diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index 2a86b427806..f127e45ae6a 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -7,9 +7,17 @@ describe Gitlab::Email::Receiver, lib: true do context "when we cannot find a capable handler" do let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "!!!") } - it "raises a UnknownIncomingEmail" do + it "raises an UnknownIncomingEmail error" do expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) end + + context "and the email contains no references header" do + let(:email_raw) { fixture_file("emails/auto_reply.eml").gsub(mail_key, "!!!") } + + it "raises an UnknownIncomingEmail error" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) + end + end end context "when the email is blank" do From 336d79fbf9715839d7f969f00d51632282b91fa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 26 Apr 2017 18:54:48 +0200 Subject: [PATCH 16/21] Group static-analysis jobs into a single job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .gitlab-ci.yml | 46 +++++++++++++++------------------------------ scripts/lint-doc.sh | 1 - 2 files changed, 15 insertions(+), 32 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ddc2c5f2542..114b22ef6c3 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -253,38 +253,40 @@ spinach mysql 9 10: *spinach-knapsack-mysql SETUP_DB: "false" USE_BUNDLE_INSTALL: "true" -.exec: &exec +.rake-exec: &rake-exec <<: *ruby-static-analysis <<: *dedicated-runner <<: *except-docs stage: test script: - - bundle exec $CI_JOB_NAME + - bundle exec rake $CI_JOB_NAME -rubocop: +static-analysis: <<: *ruby-static-analysis <<: *dedicated-runner <<: *except-docs stage: test script: + - bundle exec rake config_lint + - bundle exec rake flay + - bundle exec rake haml_lint + - bundle exec rake scss_lint + - bundle exec rake brakeman + - bundle exec license_finder + - scripts/lint-doc.sh + - yarn run eslint - bundle exec "rubocop --require rubocop-rspec" -rake haml_lint: *exec -rake scss_lint: *exec -rake config_lint: *exec -rake brakeman: *exec -rake flay: *exec -license_finder: *exec -rake downtime_check: - <<: *exec +downtime_check: + <<: *rake-exec except: - master - tags - /^[\d-]+-stable(-ee)?$/ - /^docs\/*/ -rake ee_compat_check: - <<: *exec +ee_compat_check: + <<: *rake-exec only: - branches@gitlab-org/gitlab-ce except: @@ -402,16 +404,6 @@ rake karma: paths: - coverage-javascript/ -docs:check:apilint: - image: "phusion/baseimage" - stage: test - <<: *dedicated-runner - cache: {} - dependencies: [] - before_script: [] - script: - - scripts/lint-doc.sh - docs:check:links: image: "registry.gitlab.com/gitlab-org/gitlab-build-images:nanoc-bootstrap-ruby-2.4-alpine" stage: test @@ -485,14 +477,6 @@ coverage: - coverage/index.html - coverage/assets/ -lint:javascript: - <<: *dedicated-runner - <<: *except-docs - stage: test - before_script: [] - script: - - yarn run eslint - lint:javascript:report: <<: *dedicated-runner <<: *except-docs diff --git a/scripts/lint-doc.sh b/scripts/lint-doc.sh index 62236ed539a..54c1ef3dfdd 100755 --- a/scripts/lint-doc.sh +++ b/scripts/lint-doc.sh @@ -21,4 +21,3 @@ fi echo "✔ Linting passed" exit 0 - From f2fc716c4f00caf4d0184caf19c930e2d37bb6c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 26 Apr 2017 18:55:12 +0200 Subject: [PATCH 17/21] Shorten and improve some job names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .gitlab-ci.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 114b22ef6c3..6b411c4df07 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -311,12 +311,12 @@ ee_compat_check: script: - bundle exec rake db:migrate:reset -rake pg db:migrate:reset: +db:migrate:reset pg: <<: *db-migrate-reset <<: *use-pg <<: *except-docs -rake mysql db:migrate:reset: +db:migrate:reset mysql: <<: *db-migrate-reset <<: *use-mysql <<: *except-docs @@ -328,12 +328,12 @@ rake mysql db:migrate:reset: - bundle exec rake db:rollback STEP=120 - bundle exec rake db:migrate -rake pg db:rollback: +db:rollback pg: <<: *db-rollback <<: *use-pg <<: *except-docs -rake mysql db:rollback: +db:rollback mysql: <<: *db-rollback <<: *use-mysql <<: *except-docs @@ -355,17 +355,17 @@ rake mysql db:rollback: paths: - log/development.log -rake pg db:seed_fu: +db:seed_fu pg: <<: *db-seed_fu <<: *use-pg <<: *except-docs -rake mysql db:seed_fu: +db:seed_fu mysql: <<: *db-seed_fu <<: *use-mysql <<: *except-docs -rake gitlab:assets:compile: +gitlab:assets:compile: stage: test <<: *dedicated-runner <<: *except-docs @@ -385,7 +385,7 @@ rake gitlab:assets:compile: paths: - webpack-report/ -rake karma: +karma: cache: paths: - vendor/ruby @@ -451,11 +451,11 @@ bundler:audit: - . scripts/prepare_build.sh - bundle exec rake db:migrate -migration pg paths: +migration path pg: <<: *migration-paths <<: *use-pg -migration mysql paths: +migration path mysql: <<: *migration-paths <<: *use-mysql @@ -532,8 +532,8 @@ pages: <<: *dedicated-runner dependencies: - coverage - - rake karma - - rake gitlab:assets:compile + - karma + - gitlab:assets:compile - lint:javascript:report script: - mv public/ .public/ From 9b3f728cca163e1e6c0a67b932bbd04f74896d87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 28 Apr 2017 17:31:18 +0200 Subject: [PATCH 18/21] Add scripts/static-analysis to run all the static analysers in one go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .gitlab-ci.yml | 10 +--------- scripts/static-analysis | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 9 deletions(-) create mode 100755 scripts/static-analysis diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6b411c4df07..dab1b220bfb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -267,15 +267,7 @@ static-analysis: <<: *except-docs stage: test script: - - bundle exec rake config_lint - - bundle exec rake flay - - bundle exec rake haml_lint - - bundle exec rake scss_lint - - bundle exec rake brakeman - - bundle exec license_finder - - scripts/lint-doc.sh - - yarn run eslint - - bundle exec "rubocop --require rubocop-rspec" + - scripts/static-analysis downtime_check: <<: *rake-exec diff --git a/scripts/static-analysis b/scripts/static-analysis new file mode 100755 index 00000000000..192d9d4c3ba --- /dev/null +++ b/scripts/static-analysis @@ -0,0 +1,40 @@ +#!/usr/bin/env ruby + +require ::File.expand_path('../lib/gitlab/popen', __dir__) + +tasks = [ + %w[bundle exec rake config_lint], + %w[bundle exec rake flay], + %w[bundle exec rake haml_lint], + %w[bundle exec rake scss_lint], + %w[bundle exec rake brakeman], + %w[bundle exec license_finder], + %w[scripts/lint-doc.sh], + %w[yarn run eslint], + %w[bundle exec rubocop --require rubocop-rspec] +] + +failed_tasks = tasks.reduce({}) do |failures, task| + output, status = Gitlab::Popen.popen(task) + + puts "Running: #{task.join(' ')}" + puts output + + failures[task.join(' ')] = output unless status.zero? + + failures +end + +if failed_tasks.empty? + puts 'All static analyses passed successfully.' +else + puts "\n===================================================\n\n" + puts "Some static analyses failed:" + + failed_tasks.each do |failed_task, output| + puts "\n**** #{failed_task} failed with the following error:\n\n" + puts output + end + + exit 1 +end From 13de8aeec8cce390b07c0ab31d5336cb54123ec7 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Mon, 1 May 2017 00:29:34 +0200 Subject: [PATCH 19/21] Add specs for Gitlab::RequestProfiler Closes #31513 --- .../admin/admin_requests_profiles_spec.rb | 69 +++++++++++++++++++ spec/lib/gitlab/request_profiler_spec.rb | 27 ++++++++ spec/requests/request_profiler_spec.rb | 44 ++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 spec/features/admin/admin_requests_profiles_spec.rb create mode 100644 spec/lib/gitlab/request_profiler_spec.rb create mode 100644 spec/requests/request_profiler_spec.rb diff --git a/spec/features/admin/admin_requests_profiles_spec.rb b/spec/features/admin/admin_requests_profiles_spec.rb new file mode 100644 index 00000000000..e8ecb70306b --- /dev/null +++ b/spec/features/admin/admin_requests_profiles_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe 'Admin::RequestsProfilesController', feature: true do + before do + FileUtils.mkdir_p(Gitlab::RequestProfiler::PROFILES_DIR) + login_as(:admin) + end + + after do + Gitlab::RequestProfiler.remove_all_profiles + end + + describe 'GET /admin/requests_profiles' do + it 'shows the current profile token' do + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + + visit admin_requests_profiles_path + + expect(page).to have_content("X-Profile-Token: #{Gitlab::RequestProfiler.profile_token}") + end + + it 'lists all available profiles' do + time1 = 1.hour.ago + time2 = 2.hours.ago + time3 = 3.hours.ago + profile1 = "|gitlab-org|gitlab-ce_#{time1.to_i}.html" + profile2 = "|gitlab-org|gitlab-ce_#{time2.to_i}.html" + profile3 = "|gitlab-com|infrastructure_#{time3.to_i}.html" + + FileUtils.touch("#{Gitlab::RequestProfiler::PROFILES_DIR}/#{profile1}") + FileUtils.touch("#{Gitlab::RequestProfiler::PROFILES_DIR}/#{profile2}") + FileUtils.touch("#{Gitlab::RequestProfiler::PROFILES_DIR}/#{profile3}") + + visit admin_requests_profiles_path + + within('.panel', text: '/gitlab-org/gitlab-ce') do + expect(page).to have_selector("a[href='#{admin_requests_profile_path(profile1)}']", text: time1.to_s(:long)) + expect(page).to have_selector("a[href='#{admin_requests_profile_path(profile2)}']", text: time2.to_s(:long)) + end + + within('.panel', text: '/gitlab-com/infrastructure') do + expect(page).to have_selector("a[href='#{admin_requests_profile_path(profile3)}']", text: time3.to_s(:long)) + end + end + end + + describe 'GET /admin/requests_profiles/:profile' do + context 'when a profile exists' do + it 'displays the content of the profile' do + content = 'This is a request profile' + profile = "|gitlab-org|gitlab-ce_#{Time.now.to_i}.html" + + File.write("#{Gitlab::RequestProfiler::PROFILES_DIR}/#{profile}", content) + + visit admin_requests_profile_path(profile) + + expect(page).to have_content(content) + end + end + + context 'when a profile does not exist' do + it 'shows an error message' do + visit admin_requests_profile_path('|non|existent_12345.html') + + expect(page).to have_content('Profile not found') + end + end + end +end diff --git a/spec/lib/gitlab/request_profiler_spec.rb b/spec/lib/gitlab/request_profiler_spec.rb new file mode 100644 index 00000000000..ae9c06ebb7d --- /dev/null +++ b/spec/lib/gitlab/request_profiler_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Gitlab::RequestProfiler, lib: true do + describe '.profile_token' do + it 'returns a token' do + expect(described_class.profile_token).to be_present + end + + it 'caches the token' do + expect(Rails.cache).to receive(:fetch).with('profile-token') + + described_class.profile_token + end + end + + describe '.remove_all_profiles' do + it 'removes Gitlab::RequestProfiler::PROFILES_DIR directory' do + dir = described_class::PROFILES_DIR + FileUtils.mkdir_p(dir) + + expect(Dir.exist?(dir)).to be true + + described_class.remove_all_profiles + expect(Dir.exist?(dir)).to be false + end + end +end diff --git a/spec/requests/request_profiler_spec.rb b/spec/requests/request_profiler_spec.rb new file mode 100644 index 00000000000..51fbfecec4b --- /dev/null +++ b/spec/requests/request_profiler_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe 'Request Profiler' do + let(:user) { create(:user) } + + shared_examples 'profiling a request' do + before do + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + allow(RubyProf::Profile).to receive(:profile) do |&blk| + blk.call + RubyProf::Profile.new + end + end + + it 'creates a profile of the request' do + project = create(:project, namespace: user.namespace) + time = Time.now + path = "/#{project.path_with_namespace}" + + Timecop.freeze(time) do + get path, nil, 'X-Profile-Token' => Gitlab::RequestProfiler.profile_token + end + + profile_path = "#{Gitlab.config.shared.path}/tmp/requests_profiles/#{path.tr('/', '|')}_#{time.to_i}.html" + expect(File.exist?(profile_path)).to be true + end + + after do + Gitlab::RequestProfiler.remove_all_profiles + end + end + + context "when user is logged-in" do + before do + login_as(user) + end + + include_examples 'profiling a request' + end + + context "when user is not logged-in" do + include_examples 'profiling a request' + end +end From d261f83626b9abde93f91d9850ae3e989348e12d Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Mon, 1 May 2017 20:15:16 +0000 Subject: [PATCH 20/21] Update Carrierwave and fog-core --- Gemfile | 4 ++-- Gemfile.lock | 22 +++++++++---------- .../expire_build_instance_artifacts_worker.rb | 2 +- config/initializers/carrierwave.rb | 2 ++ spec/factories/projects.rb | 4 ++++ spec/models/project_spec.rb | 15 +++++-------- ...re_build_instance_artifacts_worker_spec.rb | 6 +++-- 7 files changed, 29 insertions(+), 26 deletions(-) diff --git a/Gemfile b/Gemfile index 41c2dcfd76e..f54a1f500fd 100644 --- a/Gemfile +++ b/Gemfile @@ -85,14 +85,14 @@ gem 'kaminari', '~> 0.17.0' gem 'hamlit', '~> 2.6.1' # Files attachments -gem 'carrierwave', '~> 0.11.0' +gem 'carrierwave', '~> 1.0' # Drag and Drop UI gem 'dropzonejs-rails', '~> 0.7.1' # for backups gem 'fog-aws', '~> 0.9' -gem 'fog-core', '~> 1.40' +gem 'fog-core', '~> 1.44' gem 'fog-google', '~> 0.5' gem 'fog-local', '~> 0.3' gem 'fog-openstack', '~> 0.1' diff --git a/Gemfile.lock b/Gemfile.lock index 038d1f746b3..b822a325861 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -105,12 +105,10 @@ GEM capybara-screenshot (1.0.14) capybara (>= 1.0, < 3) launchy - carrierwave (0.11.2) - activemodel (>= 3.2.0) - activesupport (>= 3.2.0) - json (>= 1.7) + carrierwave (1.0.0) + activemodel (>= 4.0.0) + activesupport (>= 4.0.0) mime-types (>= 1.16) - mimemagic (>= 0.3.0) cause (0.1) charlock_holmes (0.7.3) chronic (0.10.2) @@ -184,7 +182,7 @@ GEM erubis (2.7.0) escape_utils (1.1.1) eventmachine (1.0.8) - excon (0.52.0) + excon (0.55.0) execjs (2.6.0) expression_parser (0.9.0) extlib (0.9.16) @@ -210,12 +208,12 @@ GEM flowdock (0.7.1) httparty (~> 0.7) multi_json - fog-aws (0.11.0) + fog-aws (0.13.0) fog-core (~> 1.38) fog-json (~> 1.0) fog-xml (~> 0.1) ipaddress (~> 0.8) - fog-core (1.42.0) + fog-core (1.44.1) builder excon (~> 0.49) formatador (~> 0.2) @@ -237,9 +235,9 @@ GEM fog-json (>= 1.0) fog-xml (>= 0.1) ipaddress (>= 0.8) - fog-xml (0.1.2) + fog-xml (0.1.3) fog-core - nokogiri (~> 1.5, >= 1.5.11) + nokogiri (>= 1.5.11, < 2.0.0) font-awesome-rails (4.7.0.1) railties (>= 3.2, < 5.1) foreman (0.78.0) @@ -871,7 +869,7 @@ DEPENDENCIES bundler-audit (~> 0.5.0) capybara (~> 2.6.2) capybara-screenshot (~> 1.0.0) - carrierwave (~> 0.11.0) + carrierwave (~> 1.0) charlock_holmes (~> 0.7.3) chronic (~> 0.10.2) chronic_duration (~> 0.10.6) @@ -896,7 +894,7 @@ DEPENDENCIES ffaker (~> 2.4) flay (~> 2.8.0) fog-aws (~> 0.9) - fog-core (~> 1.40) + fog-core (~> 1.44) fog-google (~> 0.5) fog-local (~> 0.3) fog-openstack (~> 0.1) diff --git a/app/workers/expire_build_instance_artifacts_worker.rb b/app/workers/expire_build_instance_artifacts_worker.rb index eb403c134d1..7b59e976492 100644 --- a/app/workers/expire_build_instance_artifacts_worker.rb +++ b/app/workers/expire_build_instance_artifacts_worker.rb @@ -8,7 +8,7 @@ class ExpireBuildInstanceArtifactsWorker .reorder(nil) .find_by(id: build_id) - return unless build.try(:project) + return unless build&.project && !build.project.pending_delete Rails.logger.info "Removing artifacts for build #{build.id}..." build.erase_artifacts! diff --git a/config/initializers/carrierwave.rb b/config/initializers/carrierwave.rb index 1933afcbfb1..cd7df44351a 100644 --- a/config/initializers/carrierwave.rb +++ b/config/initializers/carrierwave.rb @@ -6,6 +6,8 @@ if File.exist?(aws_file) AWS_CONFIG = YAML.load(File.read(aws_file))[Rails.env] CarrierWave.configure do |config| + config.fog_provider = 'fog/aws' + config.fog_credentials = { provider: 'AWS', # required aws_access_key_id: AWS_CONFIG['access_key_id'], # required diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 0db2fe04edd..3580752a805 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -32,6 +32,10 @@ FactoryGirl.define do request_access_enabled true end + trait :with_avatar do + avatar { File.open(Rails.root.join('spec/fixtures/dk.png')) } + end + trait :repository do # no-op... for now! end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 92d420337f9..d9244657953 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -781,17 +781,14 @@ describe Project, models: true do let(:project) { create(:empty_project) } - context 'When avatar file is uploaded' do - before do - project.update_columns(avatar: 'uploads/avatar.png') - allow(project.avatar).to receive(:present?) { true } - end + context 'when avatar file is uploaded' do + let(:project) { create(:empty_project, :with_avatar) } - let(:avatar_path) do - "/uploads/project/avatar/#{project.id}/uploads/avatar.png" - end + it 'creates a correct avatar path' do + avatar_path = "/uploads/project/avatar/#{project.id}/dk.png" - it { should eq "http://#{Gitlab.config.gitlab.host}#{avatar_path}" } + expect(project.avatar_url).to eq("http://#{Gitlab.config.gitlab.host}#{avatar_path}") + end end context 'When avatar file in git' do diff --git a/spec/workers/expire_build_instance_artifacts_worker_spec.rb b/spec/workers/expire_build_instance_artifacts_worker_spec.rb index d202b3de77e..1d8da68883b 100644 --- a/spec/workers/expire_build_instance_artifacts_worker_spec.rb +++ b/spec/workers/expire_build_instance_artifacts_worker_spec.rb @@ -34,12 +34,14 @@ describe ExpireBuildInstanceArtifactsWorker do context 'when associated project was removed' do let(:build) do create(:ci_build, :artifacts, artifacts_expiry) do |build| - build.project.delete + build.project.pending_delete = true end end it 'does not remove artifacts' do - expect(build.reload.artifacts_file.exists?).to be_truthy + expect do + build.reload.artifacts_file + end.not_to raise_error end end end From 5412020762b676fdfba5991b59ebec7adc59aeca Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Tue, 2 May 2017 08:05:43 +0000 Subject: [PATCH 21/21] Better Explore Groups view --- app/assets/javascripts/dispatcher.js | 12 ++ app/assets/javascripts/landing.js | 37 ++++ app/assets/stylesheets/framework/blocks.scss | 57 +++++++ app/assets/stylesheets/framework/common.scss | 5 + .../stylesheets/pages/cycle_analytics.scss | 44 +---- app/assets/stylesheets/pages/groups.scss | 23 +++ app/views/dashboard/_groups_head.html.haml | 6 +- app/views/explore/groups/index.html.haml | 9 + .../icons/_icon_explore_groups_splash.svg | 1 + spec/features/explore/groups_list_spec.rb | 28 ++- .../merge_requests/create_new_mr_spec.rb | 1 + spec/javascripts/landing_spec.js | 160 ++++++++++++++++++ 12 files changed, 335 insertions(+), 48 deletions(-) create mode 100644 app/assets/javascripts/landing.js create mode 100644 app/views/shared/icons/_icon_explore_groups_splash.svg create mode 100644 spec/javascripts/landing_spec.js diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 15fe87f21ea..0bdce52cc89 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -44,6 +44,7 @@ import GroupsList from './groups_list'; import ProjectsList from './projects_list'; import MiniPipelineGraph from './mini_pipeline_graph_dropdown'; import BlobLinePermalinkUpdater from './blob/blob_line_permalink_updater'; +import Landing from './landing'; import BlobForkSuggestion from './blob/blob_fork_suggestion'; import UserCallout from './user_callout'; import { ProtectedTagCreate, ProtectedTagEditList } from './protected_tags'; @@ -148,8 +149,19 @@ const ShortcutsBlob = require('./shortcuts_blob'); new ProjectsList(); break; case 'dashboard:groups:index': + new GroupsList(); + break; case 'explore:groups:index': new GroupsList(); + + const landingElement = document.querySelector('.js-explore-groups-landing'); + if (!landingElement) break; + const exploreGroupsLanding = new Landing( + landingElement, + landingElement.querySelector('.dismiss-button'), + 'explore_groups_landing_dismissed', + ); + exploreGroupsLanding.toggle(); break; case 'projects:milestones:new': case 'projects:milestones:edit': diff --git a/app/assets/javascripts/landing.js b/app/assets/javascripts/landing.js new file mode 100644 index 00000000000..8c0950ad5d5 --- /dev/null +++ b/app/assets/javascripts/landing.js @@ -0,0 +1,37 @@ +import Cookies from 'js-cookie'; + +class Landing { + constructor(landingElement, dismissButton, cookieName) { + this.landingElement = landingElement; + this.cookieName = cookieName; + this.dismissButton = dismissButton; + this.eventWrapper = {}; + } + + toggle() { + const isDismissed = this.isDismissed(); + + this.landingElement.classList.toggle('hidden', isDismissed); + if (!isDismissed) this.addEvents(); + } + + addEvents() { + this.eventWrapper.dismissLanding = this.dismissLanding.bind(this); + this.dismissButton.addEventListener('click', this.eventWrapper.dismissLanding); + } + + removeEvents() { + this.dismissButton.removeEventListener('click', this.eventWrapper.dismissLanding); + } + + dismissLanding() { + this.landingElement.classList.add('hidden'); + Cookies.set(this.cookieName, 'true', { expires: 365 }); + } + + isDismissed() { + return Cookies.get(this.cookieName) === 'true'; + } +} + +export default Landing; diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index f3e2a5db0a6..ac1fc0eb8ae 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -254,6 +254,63 @@ padding: 10px 0; } +.landing { + margin-bottom: $gl-padding; + overflow: hidden; + display: flex; + position: relative; + border: 1px solid $blue-300; + border-radius: $border-radius-default; + background-color: $blue-25; + justify-content: center; + + .dismiss-button { + position: absolute; + right: 6px; + top: 6px; + cursor: pointer; + color: $blue-300; + z-index: 1; + border: none; + background-color: transparent; + + &:hover, + &:focus { + border: none; + color: $blue-400; + } + } + + .svg-container { + align-self: center; + } + + .inner-content { + text-align: left; + white-space: nowrap; + + h4 { + color: $gl-text-color; + font-size: 17px; + } + + p { + color: $gl-text-color; + margin-bottom: $gl-padding; + } + } + + @media (max-width: $screen-sm-min) { + flex-direction: column; + + .inner-content { + white-space: normal; + padding: 0 28px; + text-align: center; + } + } +} + .empty-state { margin: 100px 0 0; diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 638c1403b25..1a6f36d032d 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -424,6 +424,11 @@ table { } } +.bordered-box { + border: 1px solid $border-color; + border-radius: $border-radius-default; +} + .str-truncated { &-60 { @include str-truncated(60%); diff --git a/app/assets/stylesheets/pages/cycle_analytics.scss b/app/assets/stylesheets/pages/cycle_analytics.scss index ad3dbc7ac48..403724cd68a 100644 --- a/app/assets/stylesheets/pages/cycle_analytics.scss +++ b/app/assets/stylesheets/pages/cycle_analytics.scss @@ -93,11 +93,6 @@ top: $gl-padding-top; } - .bordered-box { - border: 1px solid $border-color; - border-radius: $border-radius-default; - } - .content-list { li { padding: 18px $gl-padding $gl-padding; @@ -139,42 +134,9 @@ } } - .landing { - margin-bottom: $gl-padding; - overflow: hidden; - - .dismiss-icon { - position: absolute; - right: $cycle-analytics-box-padding; - cursor: pointer; - color: $cycle-analytics-dismiss-icon-color; - } - - .svg-container { - text-align: center; - - svg { - width: 136px; - height: 136px; - } - } - - .inner-content { - @media (max-width: $screen-xs-max) { - padding: 0 28px; - text-align: center; - } - - h4 { - color: $gl-text-color; - font-size: 17px; - } - - p { - color: $cycle-analytics-box-text-color; - margin-bottom: $gl-padding; - } - } + .landing svg { + width: 136px; + height: 136px; } .fa-spinner { diff --git a/app/assets/stylesheets/pages/groups.scss b/app/assets/stylesheets/pages/groups.scss index 73a5889867a..72d73b89a2a 100644 --- a/app/assets/stylesheets/pages/groups.scss +++ b/app/assets/stylesheets/pages/groups.scss @@ -88,3 +88,26 @@ color: $gl-text-color-secondary; margin-top: 10px; } + +.explore-groups.landing { + margin-top: 10px; + + .inner-content { + padding: 0; + + p { + margin: 7px 0 0; + max-width: 480px; + padding: 0 $gl-padding; + + @media (max-width: $screen-sm-min) { + margin: 0 auto; + } + } + } + + svg { + width: 62px; + height: 50px; + } +} diff --git a/app/views/dashboard/_groups_head.html.haml b/app/views/dashboard/_groups_head.html.haml index 0e848386ebb..4594c52b34b 100644 --- a/app/views/dashboard/_groups_head.html.haml +++ b/app/views/dashboard/_groups_head.html.haml @@ -2,10 +2,10 @@ %ul.nav-links = nav_link(page: dashboard_groups_path) do = link_to dashboard_groups_path, title: 'Your groups' do - Your Groups + Your groups = nav_link(page: explore_groups_path) do - = link_to explore_groups_path, title: 'Explore groups' do - Explore Groups + = link_to explore_groups_path, title: 'Explore public groups' do + Explore public groups .nav-controls = render 'shared/groups/search_form' = render 'shared/groups/dropdown' diff --git a/app/views/explore/groups/index.html.haml b/app/views/explore/groups/index.html.haml index bb2cd0d44c8..ffe07b217a7 100644 --- a/app/views/explore/groups/index.html.haml +++ b/app/views/explore/groups/index.html.haml @@ -7,6 +7,15 @@ = render 'explore/head' = render 'nav' +- if cookies[:explore_groups_landing_dismissed] != 'true' + .explore-groups.landing.content-block.js-explore-groups-landing.hidden + %button.dismiss-button{ type: 'button', 'aria-label' => 'Dismiss' }= icon('times') + .svg-container + = custom_icon('icon_explore_groups_splash') + .inner-content + %p Below you will find all the groups that are public. + %p You can easily contribute to them by requesting to join these groups. + - if @groups.present? = render 'groups' - else diff --git a/app/views/shared/icons/_icon_explore_groups_splash.svg b/app/views/shared/icons/_icon_explore_groups_splash.svg new file mode 100644 index 00000000000..79f17872739 --- /dev/null +++ b/app/views/shared/icons/_icon_explore_groups_splash.svg @@ -0,0 +1 @@ + diff --git a/spec/features/explore/groups_list_spec.rb b/spec/features/explore/groups_list_spec.rb index 8e5421a984b..9828cb179a7 100644 --- a/spec/features/explore/groups_list_spec.rb +++ b/spec/features/explore/groups_list_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Explore Groups page', js: true, feature: true do +describe 'Explore Groups page', :js, :feature do let!(:user) { create :user } let!(:group) { create(:group) } let!(:public_group) { create(:group, :public) } @@ -46,19 +46,39 @@ describe 'Explore Groups page', js: true, feature: true do it 'shows non-archived projects count' do # Initially project is not archived expect(find('.js-groups-list-holder .content-list li:first-child .stats span:first-child')).to have_text("1") - + # Archive project empty_project.archive! visit explore_groups_path # Check project count expect(find('.js-groups-list-holder .content-list li:first-child .stats span:first-child')).to have_text("0") - + # Unarchive project empty_project.unarchive! visit explore_groups_path # Check project count - expect(find('.js-groups-list-holder .content-list li:first-child .stats span:first-child')).to have_text("1") + expect(find('.js-groups-list-holder .content-list li:first-child .stats span:first-child')).to have_text("1") + end + + describe 'landing component' do + it 'should show a landing component' do + expect(page).to have_content('Below you will find all the groups that are public.') + end + + it 'should be dismissable' do + find('.dismiss-button').click + + expect(page).not_to have_content('Below you will find all the groups that are public.') + end + + it 'should persistently not show once dismissed' do + find('.dismiss-button').click + + visit explore_groups_path + + expect(page).not_to have_content('Below you will find all the groups that are public.') + end end end diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb index e1353ec71ed..f0fec625108 100644 --- a/spec/features/merge_requests/create_new_mr_spec.rb +++ b/spec/features/merge_requests/create_new_mr_spec.rb @@ -35,6 +35,7 @@ feature 'Create New Merge Request', feature: true, js: true do expect(page).to have_content('Target branch') first('.js-target-branch').click + first('.dropdown-target-branch .dropdown-content') first('.dropdown-target-branch .dropdown-content a', text: 'v1.1.0').click expect(page).to have_content "b83d6e3" diff --git a/spec/javascripts/landing_spec.js b/spec/javascripts/landing_spec.js new file mode 100644 index 00000000000..7916073190a --- /dev/null +++ b/spec/javascripts/landing_spec.js @@ -0,0 +1,160 @@ +import Landing from '~/landing'; +import Cookies from 'js-cookie'; + +describe('Landing', function () { + describe('class constructor', function () { + beforeEach(function () { + this.landingElement = {}; + this.dismissButton = {}; + this.cookieName = 'cookie_name'; + + this.landing = new Landing(this.landingElement, this.dismissButton, this.cookieName); + }); + + it('should set .landing', function () { + expect(this.landing.landingElement).toBe(this.landingElement); + }); + + it('should set .cookieName', function () { + expect(this.landing.cookieName).toBe(this.cookieName); + }); + + it('should set .dismissButton', function () { + expect(this.landing.dismissButton).toBe(this.dismissButton); + }); + + it('should set .eventWrapper', function () { + expect(this.landing.eventWrapper).toEqual({}); + }); + }); + + describe('toggle', function () { + beforeEach(function () { + this.isDismissed = false; + this.landingElement = { classList: jasmine.createSpyObj('classList', ['toggle']) }; + this.landing = { + isDismissed: () => {}, + addEvents: () => {}, + landingElement: this.landingElement, + }; + + spyOn(this.landing, 'isDismissed').and.returnValue(this.isDismissed); + spyOn(this.landing, 'addEvents'); + + Landing.prototype.toggle.call(this.landing); + }); + + it('should call .isDismissed', function () { + expect(this.landing.isDismissed).toHaveBeenCalled(); + }); + + it('should call .classList.toggle', function () { + expect(this.landingElement.classList.toggle).toHaveBeenCalledWith('hidden', this.isDismissed); + }); + + it('should call .addEvents', function () { + expect(this.landing.addEvents).toHaveBeenCalled(); + }); + + describe('if isDismissed is true', function () { + beforeEach(function () { + this.isDismissed = true; + this.landingElement = { classList: jasmine.createSpyObj('classList', ['toggle']) }; + this.landing = { + isDismissed: () => {}, + addEvents: () => {}, + landingElement: this.landingElement, + }; + + spyOn(this.landing, 'isDismissed').and.returnValue(this.isDismissed); + spyOn(this.landing, 'addEvents'); + + this.landing.isDismissed.calls.reset(); + + Landing.prototype.toggle.call(this.landing); + }); + + it('should not call .addEvents', function () { + expect(this.landing.addEvents).not.toHaveBeenCalled(); + }); + }); + }); + + describe('addEvents', function () { + beforeEach(function () { + this.dismissButton = jasmine.createSpyObj('dismissButton', ['addEventListener']); + this.eventWrapper = {}; + this.landing = { + eventWrapper: this.eventWrapper, + dismissButton: this.dismissButton, + dismissLanding: () => {}, + }; + + Landing.prototype.addEvents.call(this.landing); + }); + + it('should set .eventWrapper.dismissLanding', function () { + expect(this.eventWrapper.dismissLanding).toEqual(jasmine.any(Function)); + }); + + it('should call .addEventListener', function () { + expect(this.dismissButton.addEventListener).toHaveBeenCalledWith('click', this.eventWrapper.dismissLanding); + }); + }); + + describe('removeEvents', function () { + beforeEach(function () { + this.dismissButton = jasmine.createSpyObj('dismissButton', ['removeEventListener']); + this.eventWrapper = { dismissLanding: () => {} }; + this.landing = { + eventWrapper: this.eventWrapper, + dismissButton: this.dismissButton, + }; + + Landing.prototype.removeEvents.call(this.landing); + }); + + it('should call .removeEventListener', function () { + expect(this.dismissButton.removeEventListener).toHaveBeenCalledWith('click', this.eventWrapper.dismissLanding); + }); + }); + + describe('dismissLanding', function () { + beforeEach(function () { + this.landingElement = { classList: jasmine.createSpyObj('classList', ['add']) }; + this.cookieName = 'cookie_name'; + this.landing = { landingElement: this.landingElement, cookieName: this.cookieName }; + + spyOn(Cookies, 'set'); + + Landing.prototype.dismissLanding.call(this.landing); + }); + + it('should call .classList.add', function () { + expect(this.landingElement.classList.add).toHaveBeenCalledWith('hidden'); + }); + + it('should call Cookies.set', function () { + expect(Cookies.set).toHaveBeenCalledWith(this.cookieName, 'true', { expires: 365 }); + }); + }); + + describe('isDismissed', function () { + beforeEach(function () { + this.cookieName = 'cookie_name'; + this.landing = { cookieName: this.cookieName }; + + spyOn(Cookies, 'get').and.returnValue('true'); + + this.isDismissed = Landing.prototype.isDismissed.call(this.landing); + }); + + it('should call Cookies.get', function () { + expect(Cookies.get).toHaveBeenCalledWith(this.cookieName); + }); + + it('should return a boolean', function () { + expect(typeof this.isDismissed).toEqual('boolean'); + }); + }); +});