diff --git a/app/assets/javascripts/boards/models/issue.js b/app/assets/javascripts/boards/models/issue.js index 10f85c1d676..81edd95bf2b 100644 --- a/app/assets/javascripts/boards/models/issue.js +++ b/app/assets/javascripts/boards/models/issue.js @@ -20,6 +20,7 @@ class ListIssue { this.isFetching = { subscriptions: true, }; + this.isLoading = {}; this.sidebarInfoEndpoint = obj.issue_sidebar_endpoint; this.toggleSubscriptionEndpoint = obj.toggle_subscription_endpoint; @@ -86,6 +87,10 @@ class ListIssue { this.isFetching[key] = value; } + setLoadingState(key, value) { + this.isLoading[key] = value; + } + update (url) { const data = { issue: { diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 4e7a6e54f90..7ca783d3af6 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -514,10 +514,11 @@ GitLabDropdown = (function() { const dropdownToggle = this.dropdown.find('.dropdown-menu-toggle'); const hasFilterBulkUpdate = dropdownToggle.hasClass('js-filter-bulk-update'); + const shouldRefreshOnOpen = dropdownToggle.hasClass('js-gl-dropdown-refresh-on-open'); const hasMultiSelect = dropdownToggle.hasClass('js-multiselect'); // Makes indeterminate items effective - if (this.fullData && hasFilterBulkUpdate) { + if (this.fullData && (shouldRefreshOnOpen || hasFilterBulkUpdate)) { this.parseData(this.fullData); } diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index a41548bd694..fa7f6825d7e 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -15,7 +15,7 @@ import Cookies from 'js-cookie'; Sidebar.prototype.removeListeners = function () { this.sidebar.off('click', '.sidebar-collapsed-icon'); - $('.dropdown').off('hidden.gl.dropdown'); + this.sidebar.off('hidden.gl.dropdown'); $('.dropdown').off('loading.gl.dropdown'); $('.dropdown').off('loaded.gl.dropdown'); $(document).off('click', '.js-sidebar-toggle'); @@ -25,7 +25,7 @@ import Cookies from 'js-cookie'; const $document = $(document); this.sidebar.on('click', '.sidebar-collapsed-icon', this, this.sidebarCollapseClicked); - $('.dropdown').on('hidden.gl.dropdown', this, this.onSidebarDropdownHidden); + this.sidebar.on('hidden.gl.dropdown', this, this.onSidebarDropdownHidden); $('.dropdown').on('loading.gl.dropdown', this.sidebarDropdownLoading); $('.dropdown').on('loaded.gl.dropdown', this.sidebarDropdownLoaded); @@ -180,7 +180,7 @@ import Cookies from 'js-cookie'; var $block, sidebar; sidebar = e.data; e.preventDefault(); - $block = $(this).closest('.block'); + $block = $(e.target).closest('.block'); return sidebar.sidebarDropdownHidden($block); }; diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js new file mode 100644 index 00000000000..4032f156b15 --- /dev/null +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -0,0 +1,104 @@ +import Vue from 'vue'; +import SidebarTimeTracking from './components/time_tracking/sidebar_time_tracking'; +import SidebarAssignees from './components/assignees/sidebar_assignees'; +import ConfidentialIssueSidebar from './components/confidential/confidential_issue_sidebar.vue'; +import SidebarMoveIssue from './lib/sidebar_move_issue'; +import LockIssueSidebar from './components/lock/lock_issue_sidebar.vue'; +import sidebarParticipants from './components/participants/sidebar_participants.vue'; +import sidebarSubscriptions from './components/subscriptions/sidebar_subscriptions.vue'; +import Translate from '../vue_shared/translate'; + +Vue.use(Translate); + +function mountConfidentialComponent(mediator) { + const el = document.getElementById('js-confidential-entry-point'); + + if (!el) return; + + const dataNode = document.getElementById('js-confidential-issue-data'); + const initialData = JSON.parse(dataNode.innerHTML); + + const ConfidentialComp = Vue.extend(ConfidentialIssueSidebar); + + new ConfidentialComp({ + propsData: { + isConfidential: initialData.is_confidential, + isEditable: initialData.is_editable, + service: mediator.service, + }, + }).$mount(el); +} + +function mountLockComponent(mediator) { + const el = document.getElementById('js-lock-entry-point'); + + if (!el) return; + + const dataNode = document.getElementById('js-lock-issue-data'); + const initialData = JSON.parse(dataNode.innerHTML); + + const LockComp = Vue.extend(LockIssueSidebar); + + new LockComp({ + propsData: { + isLocked: initialData.is_locked, + isEditable: initialData.is_editable, + mediator, + issuableType: gl.utils.isInIssuePage() ? 'issue' : 'merge_request', + }, + }).$mount(el); +} + +function mountParticipantsComponent() { + const el = document.querySelector('.js-sidebar-participants-entry-point'); + + if (!el) return; + + // eslint-disable-next-line no-new + new Vue({ + el, + components: { + sidebarParticipants, + }, + render: createElement => createElement('sidebar-participants', {}), + }); +} + +function mountSubscriptionsComponent() { + const el = document.querySelector('.js-sidebar-subscriptions-entry-point'); + + if (!el) return; + + // eslint-disable-next-line no-new + new Vue({ + el, + components: { + sidebarSubscriptions, + }, + render: createElement => createElement('sidebar-subscriptions', {}), + }); +} + +function mount(mediator) { + const sidebarAssigneesEl = document.getElementById('js-vue-sidebar-assignees'); + // Only create the sidebarAssignees vue app if it is found in the DOM + // We currently do not use sidebarAssignees for the MR page + if (sidebarAssigneesEl) { + new Vue(SidebarAssignees).$mount(sidebarAssigneesEl); + } + + mountConfidentialComponent(mediator); + mountLockComponent(mediator); + mountParticipantsComponent(); + mountSubscriptionsComponent(); + + new SidebarMoveIssue( + mediator, + $('.js-move-issue'), + $('.js-move-issue-confirmation-button'), + ).init(); + + new Vue(SidebarTimeTracking).$mount('#issuable-time-tracker'); +} + +export default mount; diff --git a/app/assets/javascripts/sidebar/sidebar_bundle.js b/app/assets/javascripts/sidebar/sidebar_bundle.js index 2650bb725d4..f78287e504b 100644 --- a/app/assets/javascripts/sidebar/sidebar_bundle.js +++ b/app/assets/javascripts/sidebar/sidebar_bundle.js @@ -1,110 +1,12 @@ -import Vue from 'vue'; -import SidebarTimeTracking from './components/time_tracking/sidebar_time_tracking'; -import SidebarAssignees from './components/assignees/sidebar_assignees'; -import ConfidentialIssueSidebar from './components/confidential/confidential_issue_sidebar.vue'; -import SidebarMoveIssue from './lib/sidebar_move_issue'; -import LockIssueSidebar from './components/lock/lock_issue_sidebar.vue'; -import sidebarParticipants from './components/participants/sidebar_participants.vue'; -import sidebarSubscriptions from './components/subscriptions/sidebar_subscriptions.vue'; -import Translate from '../vue_shared/translate'; - import Mediator from './sidebar_mediator'; - -Vue.use(Translate); - -function mountConfidentialComponent(mediator) { - const el = document.getElementById('js-confidential-entry-point'); - - if (!el) return; - - const dataNode = document.getElementById('js-confidential-issue-data'); - const initialData = JSON.parse(dataNode.innerHTML); - - const ConfidentialComp = Vue.extend(ConfidentialIssueSidebar); - - new ConfidentialComp({ - propsData: { - isConfidential: initialData.is_confidential, - isEditable: initialData.is_editable, - service: mediator.service, - }, - }).$mount(el); -} - -function mountLockComponent(mediator) { - const el = document.getElementById('js-lock-entry-point'); - - if (!el) return; - - const dataNode = document.getElementById('js-lock-issue-data'); - const initialData = JSON.parse(dataNode.innerHTML); - - const LockComp = Vue.extend(LockIssueSidebar); - - new LockComp({ - propsData: { - isLocked: initialData.is_locked, - isEditable: initialData.is_editable, - mediator, - issuableType: gl.utils.isInIssuePage() ? 'issue' : 'merge_request', - }, - }).$mount(el); -} - -function mountParticipantsComponent() { - const el = document.querySelector('.js-sidebar-participants-entry-point'); - - if (!el) return; - - // eslint-disable-next-line no-new - new Vue({ - el, - components: { - sidebarParticipants, - }, - render: createElement => createElement('sidebar-participants', {}), - }); -} - -function mountSubscriptionsComponent() { - const el = document.querySelector('.js-sidebar-subscriptions-entry-point'); - - if (!el) return; - - // eslint-disable-next-line no-new - new Vue({ - el, - components: { - sidebarSubscriptions, - }, - render: createElement => createElement('sidebar-subscriptions', {}), - }); -} +import mountSidebar from './mount_sidebar'; function domContentLoaded() { const sidebarOptions = JSON.parse(document.querySelector('.js-sidebar-options').innerHTML); const mediator = new Mediator(sidebarOptions); mediator.fetch(); - const sidebarAssigneesEl = document.getElementById('js-vue-sidebar-assignees'); - // Only create the sidebarAssignees vue app if it is found in the DOM - // We currently do not use sidebarAssignees for the MR page - if (sidebarAssigneesEl) { - new Vue(SidebarAssignees).$mount(sidebarAssigneesEl); - } - - mountConfidentialComponent(mediator); - mountLockComponent(mediator); - mountParticipantsComponent(); - mountSubscriptionsComponent(); - - new SidebarMoveIssue( - mediator, - $('.js-move-issue'), - $('.js-move-issue-confirmation-button'), - ).init(); - - new Vue(SidebarTimeTracking).$mount('#issuable-time-tracker'); + mountSidebar(mediator); } document.addEventListener('DOMContentLoaded', domContentLoaded); diff --git a/app/assets/javascripts/sidebar/sidebar_mediator.js b/app/assets/javascripts/sidebar/sidebar_mediator.js index 2bda5a47791..d4c07a188b3 100644 --- a/app/assets/javascripts/sidebar/sidebar_mediator.js +++ b/app/assets/javascripts/sidebar/sidebar_mediator.js @@ -5,19 +5,23 @@ import Store from './stores/sidebar_store'; export default class SidebarMediator { constructor(options) { if (!SidebarMediator.singleton) { - this.store = new Store(options); - this.service = new Service({ - endpoint: options.endpoint, - toggleSubscriptionEndpoint: options.toggleSubscriptionEndpoint, - moveIssueEndpoint: options.moveIssueEndpoint, - projectsAutocompleteEndpoint: options.projectsAutocompleteEndpoint, - }); - SidebarMediator.singleton = this; + this.initSingleton(options); } return SidebarMediator.singleton; } + initSingleton(options) { + this.store = new Store(options); + this.service = new Service({ + endpoint: options.endpoint, + toggleSubscriptionEndpoint: options.toggleSubscriptionEndpoint, + moveIssueEndpoint: options.moveIssueEndpoint, + projectsAutocompleteEndpoint: options.projectsAutocompleteEndpoint, + }); + SidebarMediator.singleton = this; + } + assignYourself() { this.store.addAssignee(this.store.currentUser); } @@ -35,17 +39,21 @@ export default class SidebarMediator { } fetch() { - this.service.get() + return this.service.get() .then(response => response.json()) .then((data) => { - this.store.setAssigneeData(data); - this.store.setTimeTrackingData(data); - this.store.setParticipantsData(data); - this.store.setSubscriptionsData(data); + this.processFetchedData(data); }) .catch(() => new Flash('Error occurred when fetching sidebar data')); } + processFetchedData(data) { + this.store.setAssigneeData(data); + this.store.setTimeTrackingData(data); + this.store.setParticipantsData(data); + this.store.setSubscriptionsData(data); + } + toggleSubscription() { this.store.setFetchingState('subscriptions', true); return this.service.toggleSubscription() diff --git a/app/assets/javascripts/sidebar/stores/sidebar_store.js b/app/assets/javascripts/sidebar/stores/sidebar_store.js index 3150221b685..73eb25e2333 100644 --- a/app/assets/javascripts/sidebar/stores/sidebar_store.js +++ b/app/assets/javascripts/sidebar/stores/sidebar_store.js @@ -15,6 +15,7 @@ export default class SidebarStore { participants: true, subscriptions: true, }; + this.isLoading = {}; this.autocompleteProjects = []; this.moveToProjectId = 0; this.isLockDialogOpen = false; @@ -55,6 +56,10 @@ export default class SidebarStore { this.isFetching[key] = value; } + setLoadingState(key, value) { + this.isLoading[key] = value; + } + addAssignee(assignee) { if (!this.findAssignee(assignee)) { this.assignees.push(assignee); diff --git a/app/models/namespace.rb b/app/models/namespace.rb index fa76729a702..901dbf2ba69 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -139,7 +139,17 @@ class Namespace < ActiveRecord::Base def find_fork_of(project) return nil unless project.fork_network - project.fork_network.find_forks_in(projects).first + if RequestStore.active? + forks_in_namespace = RequestStore.fetch("namespaces:#{id}:forked_projects") do + Hash.new do |found_forks, project| + found_forks[project] = project.fork_network.find_forks_in(projects).first + end + end + + forks_in_namespace[project] + else + project.fork_network.find_forks_in(projects).first + end end def lfs_enabled? diff --git a/app/views/devise/passwords/edit.html.haml b/app/views/devise/passwords/edit.html.haml index eb0e6701627..35dafb3e980 100644 --- a/app/views/devise/passwords/edit.html.haml +++ b/app/views/devise/passwords/edit.html.haml @@ -1,7 +1,7 @@ = render 'devise/shared/tab_single', tab_title:'Change your password' .login-box .login-body - = form_for(resource, as: resource_name, url: password_path(resource_name), html: { method: :put, class: 'gl-show-field-errors' }) do |f| + = form_for(resource, as: resource_name, url: password_path(:user), html: { method: :put, class: 'gl-show-field-errors' }) do |f| .devise-errors = devise_error_messages! = f.hidden_field :reset_password_token @@ -17,5 +17,5 @@ .clearfix.prepend-top-20 %p %span.light Didn't receive a confirmation email? - = link_to "Request a new one", new_confirmation_path(resource_name) + = link_to "Request a new one", new_confirmation_path(:user) = render 'devise/shared/sign_in_link' diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml index 4095f30c369..41462f503cb 100644 --- a/app/views/devise/sessions/_new_base.html.haml +++ b/app/views/devise/sessions/_new_base.html.haml @@ -11,6 +11,6 @@ = f.check_box :remember_me, class: 'remember-me-checkbox' %span Remember me .pull-right.forgot-password - = link_to "Forgot your password?", new_password_path(resource_name) + = link_to "Forgot your password?", new_password_path(:user) .submit-container.move-submit-down = f.submit "Sign in", class: "btn btn-save" diff --git a/app/views/devise/shared/_links.erb b/app/views/devise/shared/_links.erb index 6e1cc244f26..cb934434c28 100644 --- a/app/views/devise/shared/_links.erb +++ b/app/views/devise/shared/_links.erb @@ -1,19 +1,19 @@ <%- if controller_name != 'sessions' %> - <%= link_to "Sign in", new_session_path(resource_name), class: "btn" %>
+ <%= link_to "Sign in", new_session_path(:user, redirect_to_referer: 'yes'), class: "btn" %>
<% end -%> <%- if devise_mapping.registerable? && controller_name != 'registrations' && allow_signup? %> - <%= link_to "Sign up", new_registration_path(resource_name) %>
+ <%= link_to "Sign up", new_registration_path(:user) %>
<% end -%> <%- if devise_mapping.recoverable? && controller_name != 'passwords' %> -<%= link_to "Forgot your password?", new_password_path(resource_name), class: "btn" %>
+<%= link_to "Forgot your password?", new_password_path(:user), class: "btn" %>
<% end -%> <%- if devise_mapping.confirmable? && controller_name != 'confirmations' %> - <%= link_to "Didn't receive confirmation instructions?", new_confirmation_path(resource_name) %>
+ <%= link_to "Didn't receive confirmation instructions?", new_confirmation_path(:user) %>
<% end -%> <%- if devise_mapping.lockable? && resource_class.unlock_strategy_enabled?(:email) && controller_name != 'unlocks' %> - <%= link_to "Didn't receive unlock instructions?", new_unlock_path(resource_name) %>
+ <%= link_to "Didn't receive unlock instructions?", new_unlock_path(:user) %>
<% end -%> diff --git a/app/views/devise/shared/_sign_in_link.html.haml b/app/views/devise/shared/_sign_in_link.html.haml index 289bf40f3de..77ef103cc47 100644 --- a/app/views/devise/shared/_sign_in_link.html.haml +++ b/app/views/devise/shared/_sign_in_link.html.haml @@ -1,4 +1,4 @@ %p %span.light Already have login and password? - = link_to "Sign in", new_session_path(resource_name) + = link_to "Sign in", new_session_path(:user, redirect_to_referer: 'yes') diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml index 298604dee8c..2554b2688bb 100644 --- a/app/views/devise/shared/_signup_box.html.haml +++ b/app/views/devise/shared/_signup_box.html.haml @@ -31,4 +31,4 @@ %p %span.light Didn't receive a confirmation email? = succeed '.' do - = link_to "Request a new one", new_confirmation_path(resource_name) + = link_to "Request a new one", new_confirmation_path(:user) diff --git a/app/views/errors/omniauth_error.html.haml b/app/views/errors/omniauth_error.html.haml index 20b7fa471a0..a2a4c75daad 100644 --- a/app/views/errors/omniauth_error.html.haml +++ b/app/views/errors/omniauth_error.html.haml @@ -9,7 +9,7 @@ %p Try logging in using your username or email. If you have forgotten your password, try recovering it = link_to "Sign in", new_session_path(:user), class: 'btn primary' - = link_to "Recover password", new_password_path(resource_name), class: 'btn secondary' + = link_to "Recover password", new_password_path(:user), class: 'btn secondary' %hr %p.light If none of the options work, try contacting a GitLab administrator. diff --git a/app/views/shared/issuable/_nav.html.haml b/app/views/shared/issuable/_nav.html.haml index 3f03cc7a275..6d8a4668cec 100644 --- a/app/views/shared/issuable/_nav.html.haml +++ b/app/views/shared/issuable/_nav.html.haml @@ -1,6 +1,5 @@ - type = local_assigns.fetch(:type, :issues) - page_context_word = type.to_s.humanize(capitalize: false) -- issuables = @issues || @merge_requests %ul.nav-links.issues-state-filters %li{ class: active_when(params[:state] == 'opened') }> @@ -20,6 +19,4 @@ = link_to page_filter_path(state: 'closed', label: true), id: 'state-closed', title: 'Filter by issues that are currently closed.', data: { state: 'closed' } do #{issuables_state_counter_text(type, :closed)} - %li{ class: active_when(params[:state] == 'all') }> - = link_to page_filter_path(state: 'all', label: true), id: 'state-all', title: "Show all #{page_context_word}.", data: { state: 'all' } do - #{issuables_state_counter_text(type, :all)} + = render 'shared/issuable/nav_links/all', page_context_word: page_context_word, counter: issuables_state_counter_text(type, :all) diff --git a/app/views/shared/issuable/nav_links/_all.html.haml b/app/views/shared/issuable/nav_links/_all.html.haml new file mode 100644 index 00000000000..d7ad7090a45 --- /dev/null +++ b/app/views/shared/issuable/nav_links/_all.html.haml @@ -0,0 +1,6 @@ +- page_context_word = local_assigns.fetch(:page_context_word) +- counter = local_assigns.fetch(:counter) + +%li{ class: active_when(params[:state] == 'all') }> + = link_to page_filter_path(state: 'all', label: true), id: 'state-all', title: "Show all #{page_context_word}.", data: { state: 'all' } do + #{counter} diff --git a/changelogs/unreleased/39367-fix-new-email-session-path.yml b/changelogs/unreleased/39367-fix-new-email-session-path.yml new file mode 100644 index 00000000000..73485d9d1a9 --- /dev/null +++ b/changelogs/unreleased/39367-fix-new-email-session-path.yml @@ -0,0 +1,5 @@ +--- +title: Confirming email with invalid token should no longer generate an error +merge_request: 15726 +author: +type: fixed diff --git a/changelogs/unreleased/bvl-limit-fork-queries-on-project-show.yml b/changelogs/unreleased/bvl-limit-fork-queries-on-project-show.yml new file mode 100644 index 00000000000..299d9bf6b9c --- /dev/null +++ b/changelogs/unreleased/bvl-limit-fork-queries-on-project-show.yml @@ -0,0 +1,5 @@ +--- +title: Reduce requests for project forks on show page of projects that have forks +merge_request: 15663 +author: +type: performance diff --git a/doc/development/writing_documentation.md b/doc/development/writing_documentation.md index 68ba3dd2da3..b6def7ef541 100644 --- a/doc/development/writing_documentation.md +++ b/doc/development/writing_documentation.md @@ -152,12 +152,23 @@ CE and EE. ## Previewing the changes live If you want to preview the doc changes of your merge request live, you can use -the manual `review-docs-deploy` job in your merge request. +the manual `review-docs-deploy` job in your merge request. You will need at +least Master permissions to be able to run it and is currently enabled for the +following projects: + +- https://gitlab.com/gitlab-org/gitlab-ce +- https://gitlab.com/gitlab-org/gitlab-ee + +NOTE: **Note:** +You will need to push a branch to those repositories, it doesn't work for forks. TIP: **Tip:** If your branch contains only documentation changes, you can use [special branch names](#testing) to avoid long running pipelines. +In the mini pipeline graph, you should see an `>>` icon. Clicking on it will +reveal the `review-docs-deploy` job. Hit the play button for the job to start. + ![Manual trigger a docs build](img/manual_build_docs.png) This job will: diff --git a/lib/gitlab/git/conflict/file.rb b/lib/gitlab/git/conflict/file.rb index fc1595f1faf..b2a625e08fa 100644 --- a/lib/gitlab/git/conflict/file.rb +++ b/lib/gitlab/git/conflict/file.rb @@ -2,7 +2,7 @@ module Gitlab module Git module Conflict class File - attr_reader :content, :their_path, :our_path, :our_mode, :repository + attr_reader :content, :their_path, :our_path, :our_mode, :repository, :commit_oid def initialize(repository, commit_oid, conflict, content) @repository = repository diff --git a/lib/gitlab/git/conflict/resolver.rb b/lib/gitlab/git/conflict/resolver.rb index df509c5f4ce..de8cce41b6d 100644 --- a/lib/gitlab/git/conflict/resolver.rb +++ b/lib/gitlab/git/conflict/resolver.rb @@ -75,7 +75,7 @@ module Gitlab resolved_lines = file.resolve_lines(params[:sections]) new_file = resolved_lines.map { |line| line[:full_line] }.join("\n") - new_file << "\n" if file.our_blob.data.ends_with?("\n") + new_file << "\n" if file.our_blob.data.end_with?("\n") elsif params[:content] new_file = file.resolve_content(params[:content]) end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index f2bbaed64a7..eab04bcac65 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -18,6 +18,8 @@ module Gitlab GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE ].freeze SEARCH_CONTEXT_LINES = 3 + REBASE_WORKTREE_PREFIX = 'rebase'.freeze + SQUASH_WORKTREE_PREFIX = 'squash'.freeze NoRepository = Class.new(StandardError) InvalidBlobName = Class.new(StandardError) @@ -1070,13 +1072,8 @@ module Gitlab raise ArgumentError, "invalid ref_path #{ref_path.inspect}" if ref_path.include?(' ') raise ArgumentError, "invalid ref #{ref.inspect}" if ref.include?("\x00") - command = [Gitlab.config.git.bin_path] + %w[update-ref --stdin -z] input = "update #{ref_path}\x00#{ref}\x00\x00" - output, status = circuit_breaker.perform do - popen(command, path) { |stdin| stdin.write(input) } - end - - raise GitError, output unless status.zero? + run_git!(%w[update-ref --stdin -z]) { |stdin| stdin.write(input) } end def fetch_ref(source_repository, source_ref:, target_ref:) @@ -1098,14 +1095,22 @@ module Gitlab end # Refactoring aid; allows us to copy code from app/models/repository.rb - def run_git(args, env: {}, nice: false) + def run_git(args, chdir: path, env: {}, nice: false, &block) cmd = [Gitlab.config.git.bin_path, *args] cmd.unshift("nice") if nice circuit_breaker.perform do - popen(cmd, path, env) + popen(cmd, chdir, env, &block) end end + def run_git!(args, chdir: path, env: {}, nice: false, &block) + output, status = run_git(args, chdir: chdir, env: env, nice: nice, &block) + + raise GitError, output unless status.zero? + + output + end + # Refactoring aid; allows us to copy code from app/models/repository.rb def run_git_with_timeout(args, timeout, env: {}) circuit_breaker.perform do @@ -1175,6 +1180,64 @@ module Gitlab raise GitError.new("Could not fsck repository:\n#{output}") unless status.zero? end + def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:) + rebase_path = worktree_path(REBASE_WORKTREE_PREFIX, rebase_id) + env = git_env_for_user(user) + + with_worktree(rebase_path, branch, env: env) do + run_git!( + %W(pull --rebase #{remote_repository.path} #{remote_branch}), + chdir: rebase_path, env: env + ) + + rebase_sha = run_git!(%w(rev-parse HEAD), chdir: rebase_path, env: env).strip + + Gitlab::Git::OperationService.new(user, self) + .update_branch(branch, rebase_sha, branch_sha) + + rebase_sha + end + end + + def rebase_in_progress?(rebase_id) + fresh_worktree?(worktree_path(REBASE_WORKTREE_PREFIX, rebase_id)) + end + + def squash(user, squash_id, branch:, start_sha:, end_sha:, author:, message:) + squash_path = worktree_path(SQUASH_WORKTREE_PREFIX, squash_id) + env = git_env_for_user(user).merge( + 'GIT_AUTHOR_NAME' => author.name, + 'GIT_AUTHOR_EMAIL' => author.email + ) + diff_range = "#{start_sha}...#{end_sha}" + diff_files = run_git!( + %W(diff --name-only --diff-filter=a --binary #{diff_range}) + ).chomp + + with_worktree(squash_path, branch, sparse_checkout_files: diff_files, env: env) do + # Apply diff of the `diff_range` to the worktree + diff = run_git!(%W(diff --binary #{diff_range})) + run_git!(%w(apply --index), chdir: squash_path, env: env) do |stdin| + stdin.write(diff) + end + + # Commit the `diff_range` diff + run_git!(%W(commit --no-verify --message #{message}), chdir: squash_path, env: env) + + # Return the squash sha. May print a warning for ambiguous refs, but + # we can ignore that with `--quiet` and just take the SHA, if present. + # HEAD here always refers to the current HEAD commit, even if there is + # another ref called HEAD. + run_git!( + %w(rev-parse --quiet --verify HEAD), chdir: squash_path, env: env + ).chomp + end + end + + def squash_in_progress?(squash_id) + fresh_worktree?(worktree_path(SQUASH_WORKTREE_PREFIX, squash_id)) + end + def gitaly_repository Gitlab::GitalyClient::Util.repository(@storage, @relative_path, @gl_repository) end @@ -1211,6 +1274,57 @@ module Gitlab private + def fresh_worktree?(path) + File.exist?(path) && !clean_stuck_worktree(path) + end + + def with_worktree(worktree_path, branch, sparse_checkout_files: nil, env:) + base_args = %w(worktree add --detach) + + # Note that we _don't_ want to test for `.present?` here: If the caller + # passes an non nil empty value it means it still wants sparse checkout + # but just isn't interested in any file, perhaps because it wants to + # checkout files in by a changeset but that changeset only adds files. + if sparse_checkout_files + # Create worktree without checking out + run_git!(base_args + ['--no-checkout', worktree_path], env: env) + worktree_git_path = run_git!(%w(rev-parse --git-dir), chdir: worktree_path) + + configure_sparse_checkout(worktree_git_path, sparse_checkout_files) + + # After sparse checkout configuration, checkout `branch` in worktree + run_git!(%W(checkout --detach #{branch}), chdir: worktree_path, env: env) + else + # Create worktree and checkout `branch` in it + run_git!(base_args + [worktree_path, branch], env: env) + end + + yield + ensure + FileUtils.rm_rf(worktree_path) if File.exist?(worktree_path) + FileUtils.rm_rf(worktree_git_path) if worktree_git_path && File.exist?(worktree_git_path) + end + + def clean_stuck_worktree(path) + return false unless File.mtime(path) < 15.minutes.ago + + FileUtils.rm_rf(path) + true + end + + # Adding a worktree means checking out the repository. For large repos, + # this can be very expensive, so set up sparse checkout for the worktree + # to only check out the files we're interested in. + def configure_sparse_checkout(worktree_git_path, files) + run_git!(%w(config core.sparseCheckout true)) + + return if files.empty? + + worktree_info_path = File.join(worktree_git_path, 'info') + FileUtils.mkdir_p(worktree_info_path) + File.write(File.join(worktree_info_path, 'sparse-checkout'), files) + end + def rugged_fetch_source_branch(source_repository, source_branch, local_ref) with_repo_branch_commit(source_repository, source_branch) do |commit| if commit @@ -1222,6 +1336,24 @@ module Gitlab end end + def worktree_path(prefix, id) + id = id.to_s + raise ArgumentError, "worktree id can't be empty" unless id.present? + raise ArgumentError, "worktree id can't contain slashes " if id.include?("/") + + File.join(path, 'gitlab-worktree', "#{prefix}-#{id}") + end + + def git_env_for_user(user) + { + 'GIT_COMMITTER_NAME' => user.name, + 'GIT_COMMITTER_EMAIL' => user.email, + 'GL_ID' => Gitlab::GlId.gl_id(user), + 'GL_PROTOCOL' => Gitlab::Git::Hook::GL_PROTOCOL, + 'GL_REPOSITORY' => gl_repository + } + end + # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'. def branches_filter(filter: nil, sort_by: nil) # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37464 diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index e7ab714c550..e4b2bbb7c51 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -261,6 +261,27 @@ describe ProjectsController do expect(response).to redirect_to(namespace_project_path) end end + + context 'when the project is forked and has a repository', :request_store do + let(:public_project) { create(:project, :public, :repository) } + let(:other_user) { create(:user) } + + render_views + + before do + # View the project as a user that does not have any rights + sign_in(other_user) + + fork_project(public_project) + end + + it 'does not increase the number of queries when the project is forked' do + expected_query = /#{public_project.fork_network.find_forks_in(other_user.namespace).to_sql}/ + + expect { get(:show, namespace_id: public_project.namespace, id: public_project) } + .not_to exceed_query_limit(1).for_query(expected_query) + end + end end describe "#update" do diff --git a/spec/javascripts/boards/issue_spec.js b/spec/javascripts/boards/issue_spec.js index ccde657789a..10b88878c2a 100644 --- a/spec/javascripts/boards/issue_spec.js +++ b/spec/javascripts/boards/issue_spec.js @@ -146,6 +146,12 @@ describe('Issue model', () => { expect(issue.isFetching.subscriptions).toBe(false); }); + it('sets loading state', () => { + issue.setLoadingState('foo', true); + + expect(issue.isLoading.foo).toBe(true); + }); + describe('update', () => { it('passes assignee ids when there are assignees', (done) => { spyOn(Vue.http, 'patch').and.callFake((url, data) => { diff --git a/spec/javascripts/sidebar/mock_data.js b/spec/javascripts/sidebar/mock_data.js index 0682b463043..3b094d20838 100644 --- a/spec/javascripts/sidebar/mock_data.js +++ b/spec/javascripts/sidebar/mock_data.js @@ -1,6 +1,6 @@ /* eslint-disable quote-props*/ -const sidebarMockData = { +const RESPONSE_MAP = { 'GET': { '/gitlab-org/gitlab-shell/issues/5.json': { id: 45, @@ -66,6 +66,65 @@ const sidebarMockData = { }, labels: [], }, + '/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar': { + assignees: [ + { + name: 'User 0', + username: 'user0', + id: 22, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/user0', + }, + { + name: 'Marguerite Bartell', + username: 'tajuana', + id: 18, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/tajuana', + }, + { + name: 'Laureen Ritchie', + username: 'michaele.will', + id: 16, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/michaele.will', + }, + ], + human_time_estimate: null, + human_total_time_spent: null, + participants: [ + { + name: 'User 0', + username: 'user0', + id: 22, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/user0', + }, + { + name: 'Marguerite Bartell', + username: 'tajuana', + id: 18, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/tajuana', + }, + { + name: 'Laureen Ritchie', + username: 'michaele.will', + id: 16, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/michaele.will', + }, + ], + subscribed: true, + time_estimate: 0, + total_time_spent: 0, + }, '/autocomplete/projects?project_id=15': [ { 'id': 0, @@ -113,9 +172,10 @@ const sidebarMockData = { }, }; -export default { +const mockData = { + responseMap: RESPONSE_MAP, mediator: { - endpoint: '/gitlab-org/gitlab-shell/issues/5.json', + endpoint: '/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar', toggleSubscriptionEndpoint: '/gitlab-org/gitlab-shell/issues/5/toggle_subscription', moveIssueEndpoint: '/gitlab-org/gitlab-shell/issues/5/move', projectsAutocompleteEndpoint: '/autocomplete/projects?project_id=15', @@ -141,12 +201,14 @@ export default { name: 'Administrator', username: 'root', }, - - sidebarMockInterceptor(request, next) { - const body = sidebarMockData[request.method.toUpperCase()][request.url]; - - next(request.respondWith(JSON.stringify(body), { - status: 200, - })); - }, }; + +mockData.sidebarMockInterceptor = function (request, next) { + const body = this.responseMap[request.method.toUpperCase()][request.url]; + + next(request.respondWith(JSON.stringify(body), { + status: 200, + })); +}.bind(mockData); + +export default mockData; diff --git a/spec/javascripts/sidebar/sidebar_mediator_spec.js b/spec/javascripts/sidebar/sidebar_mediator_spec.js index 7deb1fd2118..14c34d5a78c 100644 --- a/spec/javascripts/sidebar/sidebar_mediator_spec.js +++ b/spec/javascripts/sidebar/sidebar_mediator_spec.js @@ -33,10 +33,29 @@ describe('Sidebar mediator', () => { .catch(done.fail); }); - it('fetches the data', () => { - spyOn(this.mediator.service, 'get').and.callThrough(); - this.mediator.fetch(); - expect(this.mediator.service.get).toHaveBeenCalled(); + it('fetches the data', (done) => { + const mockData = Mock.responseMap.GET['/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar']; + spyOn(this.mediator, 'processFetchedData').and.callThrough(); + + this.mediator.fetch() + .then(() => { + expect(this.mediator.processFetchedData).toHaveBeenCalledWith(mockData); + }) + .then(done) + .catch(done.fail); + }); + + it('processes fetched data', () => { + const mockData = Mock.responseMap.GET['/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar']; + this.mediator.processFetchedData(mockData); + + expect(this.mediator.store.assignees).toEqual(mockData.assignees); + expect(this.mediator.store.humanTimeEstimate).toEqual(mockData.human_time_estimate); + expect(this.mediator.store.humanTotalTimeSpent).toEqual(mockData.human_total_time_spent); + expect(this.mediator.store.participants).toEqual(mockData.participants); + expect(this.mediator.store.subscribed).toEqual(mockData.subscribed); + expect(this.mediator.store.timeEstimate).toEqual(mockData.time_estimate); + expect(this.mediator.store.totalTimeSpent).toEqual(mockData.total_time_spent); }); it('sets moveToProjectId', () => { diff --git a/spec/javascripts/sidebar/sidebar_store_spec.js b/spec/javascripts/sidebar/sidebar_store_spec.js index 51dee64fb93..ea4eae1e23f 100644 --- a/spec/javascripts/sidebar/sidebar_store_spec.js +++ b/spec/javascripts/sidebar/sidebar_store_spec.js @@ -120,6 +120,12 @@ describe('Sidebar store', () => { expect(this.store.isFetching.participants).toEqual(false); }); + it('sets loading state', () => { + this.store.setLoadingState('assignees', true); + + expect(this.store.isLoading.assignees).toEqual(true); + }); + it('set time tracking data', () => { this.store.setTimeTrackingData(Mock.time); expect(this.store.timeEstimate).toEqual(Mock.time.time_estimate); diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 90b768f595e..3817f20bfe7 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -531,7 +531,7 @@ describe Namespace do end end - describe '#has_forks_of?' do + describe '#find_fork_of?' do let(:project) { create(:project, :public) } let!(:forked_project) { fork_project(project, namespace.owner, namespace: namespace) } @@ -550,5 +550,13 @@ describe Namespace do expect(other_namespace.find_fork_of(project)).to eq(other_fork) end + + context 'with request store enabled', :request_store do + it 'only queries once' do + expect(project.fork_network).to receive(:find_forks_in).once.and_call_original + + 2.times { namespace.find_fork_of(project) } + end + end end end diff --git a/spec/support/query_recorder.rb b/spec/support/query_recorder.rb index 369775db462..8cf8f45a8b2 100644 --- a/spec/support/query_recorder.rb +++ b/spec/support/query_recorder.rb @@ -41,7 +41,8 @@ RSpec::Matchers.define :exceed_query_limit do |expected| supports_block_expectations match do |block| - query_count(&block) > expected_count + threshold + @subject_block = block + actual_count > expected_count + threshold end failure_message_when_negated do |actual| @@ -55,6 +56,11 @@ RSpec::Matchers.define :exceed_query_limit do |expected| self end + def for_query(query) + @query = query + self + end + def threshold @threshold.to_i end @@ -68,12 +74,15 @@ RSpec::Matchers.define :exceed_query_limit do |expected| end def actual_count - @recorder.count + @actual_count ||= if @query + recorder.log.select { |recorded| recorded =~ @query }.size + else + recorder.count + end end - def query_count(&block) - @recorder = ActiveRecord::QueryRecorder.new(&block) - @recorder.count + def recorder + @recorder ||= ActiveRecord::QueryRecorder.new(&@subject_block) end def count_queries(queries)