From 3f1778140e5b459d46b4d24b16a4b8af71394354 Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Mon, 29 Oct 2018 13:27:47 +0200 Subject: Fix quick links button styles --- app/assets/stylesheets/framework/secondary_navigation_elements.scss | 4 ++-- app/presenters/project_presenter.rb | 2 +- app/views/projects/empty.html.haml | 2 +- app/views/projects/show.html.haml | 2 +- changelogs/unreleased/gt-fix-quick-links-button-styles.yml | 5 +++++ spec/presenters/project_presenter_spec.rb | 2 +- 6 files changed, 11 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/gt-fix-quick-links-button-styles.yml diff --git a/app/assets/stylesheets/framework/secondary_navigation_elements.scss b/app/assets/stylesheets/framework/secondary_navigation_elements.scss index f47dfe1b563..de9e7c37695 100644 --- a/app/assets/stylesheets/framework/secondary_navigation_elements.scss +++ b/app/assets/stylesheets/framework/secondary_navigation_elements.scss @@ -1,6 +1,6 @@ // For tabbed navigation links, scrolling tabs, etc. For all top/main navigation, // please check nav.scss -.nav-links { +.nav-links:not(.quick-links) { display: flex; padding: 0; margin: 0; @@ -106,7 +106,7 @@ display: inline-block; float: right; text-align: right; - padding: 11px 0; + padding: $gl-padding-8 0; margin-bottom: 0; > .btn, diff --git a/app/presenters/project_presenter.rb b/app/presenters/project_presenter.rb index 79cd3606aec..d61124fa787 100644 --- a/app/presenters/project_presenter.rb +++ b/app/presenters/project_presenter.rb @@ -176,7 +176,7 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated AnchorData.new(false, _('New file'), project_new_blob_path(project, default_branch || 'master'), - 'new') + 'success') end end diff --git a/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml index 75f35360e5e..936900a0087 100644 --- a/app/views/projects/empty.html.haml +++ b/app/views/projects/empty.html.haml @@ -36,7 +36,7 @@ .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller .fade-left= icon('angle-left') .fade-right= icon('angle-right') - .nav-links.scrolling-tabs + .nav-links.scrolling-tabs.quick-links = render 'stat_anchor_list', anchors: @project.empty_repo_statistics_anchors = render 'stat_anchor_list', anchors: @project.empty_repo_statistics_buttons diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index 283031b06da..f29ce4f5c06 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -22,7 +22,7 @@ .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller .fade-left= icon('angle-left') .fade-right= icon('angle-right') - .nav-links.scrolling-tabs + .nav-links.scrolling-tabs.quick-links = render 'stat_anchor_list', anchors: @project.statistics_anchors(show_auto_devops_callout: show_auto_devops_callout) = render 'stat_anchor_list', anchors: @project.statistics_buttons(show_auto_devops_callout: show_auto_devops_callout) diff --git a/changelogs/unreleased/gt-fix-quick-links-button-styles.yml b/changelogs/unreleased/gt-fix-quick-links-button-styles.yml new file mode 100644 index 00000000000..4c1150631f8 --- /dev/null +++ b/changelogs/unreleased/gt-fix-quick-links-button-styles.yml @@ -0,0 +1,5 @@ +--- +title: Fix quick links button styles +merge_request: 22657 +author: George Tsiolis +type: fixed diff --git a/spec/presenters/project_presenter_spec.rb b/spec/presenters/project_presenter_spec.rb index 3eb2f149311..7b0192fa9c8 100644 --- a/spec/presenters/project_presenter_spec.rb +++ b/spec/presenters/project_presenter_spec.rb @@ -239,7 +239,7 @@ describe ProjectPresenter do expect(presenter.new_file_anchor_data).to have_attributes(enabled: false, label: "New file", link: presenter.project_new_blob_path(project, 'master'), - class_modifier: 'new') + class_modifier: 'success') end it 'returns nil if user cannot push' do -- cgit v1.2.1 From d4ef4ad752bf05758351bd0b8761566e40ab0e8e Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 30 Oct 2018 16:41:49 -0700 Subject: Reduce SQL queries needed to load open merge requests The SQL queries and memory allocation in MergeRequests::RefreshService is dominated by queries for Project and Route loads. On staging, the absence of an inverse relationship caused Rails to make over 1100 extraneous SQL queries for the www-gitlab-com repository. Relates to https://gitlab.com/gitlab-org/gitlab-ce/issues/49703 --- app/models/project.rb | 2 +- .../sh-optimize-merge-request-project-lookup.yml | 5 +++++ lib/gitlab/import/merge_request_helpers.rb | 2 +- spec/models/merge_request_spec.rb | 14 ++++++++++++++ spec/models/project_spec.rb | 4 ++++ 5 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/sh-optimize-merge-request-project-lookup.yml diff --git a/app/models/project.rb b/app/models/project.rb index d593cbb223a..106cb92e0cc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -181,7 +181,7 @@ class Project < ActiveRecord::Base has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent # Merge Requests for target project should be removed with it - has_many :merge_requests, foreign_key: 'target_project_id' + has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project has_many :source_of_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest' has_many :issues has_many :labels, class_name: 'ProjectLabel' diff --git a/changelogs/unreleased/sh-optimize-merge-request-project-lookup.yml b/changelogs/unreleased/sh-optimize-merge-request-project-lookup.yml new file mode 100644 index 00000000000..241b89c4633 --- /dev/null +++ b/changelogs/unreleased/sh-optimize-merge-request-project-lookup.yml @@ -0,0 +1,5 @@ +--- +title: Reduce SQL queries needed to load open merge requests +merge_request: 22709 +author: +type: performance diff --git a/lib/gitlab/import/merge_request_helpers.rb b/lib/gitlab/import/merge_request_helpers.rb index 97dc1a987c4..9215067d973 100644 --- a/lib/gitlab/import/merge_request_helpers.rb +++ b/lib/gitlab/import/merge_request_helpers.rb @@ -22,7 +22,7 @@ module Gitlab # additional work that is strictly necessary. merge_request_id = insert_and_return_id(attributes, project.merge_requests) - merge_request = project.merge_requests.find(merge_request_id) + merge_request = project.merge_requests.reload.find(merge_request_id) # We use .insert_and_return_id which effectively disables all callbacks. # Trigger iid logic here to make sure we track internal id values consistently. diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c8943f2d86f..85a4ebac66c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -13,6 +13,20 @@ describe MergeRequest do it { is_expected.to belong_to(:merge_user).class_name("User") } it { is_expected.to belong_to(:assignee) } it { is_expected.to have_many(:merge_request_diffs) } + + context 'for forks' do + let!(:project) { create(:project) } + let!(:fork) { fork_project(project) } + let!(:merge_request) { create(:merge_request, target_project: project, source_project: fork) } + + it 'does not load another project due to inverse relationship' do + expect(project.merge_requests.first.target_project.object_id).to eq(project.object_id) + end + + it 'finds the associated merge request' do + expect(project.merge_requests.find(merge_request.id)).to eq(merge_request) + end + end end describe '#squash_in_progress?' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e66838edd1a..83b3f308ec3 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -88,6 +88,10 @@ describe Project do it { is_expected.to have_many(:project_deploy_tokens) } it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) } + it 'has an inverse relationship with merge requests' do + expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project) + end + context 'after initialized' do it "has a project_feature" do expect(described_class.new.project_feature).to be_present -- cgit v1.2.1 From 44089058d9ba9c3548e1f1ea772802a0924d38c8 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 31 Oct 2018 19:13:44 +0900 Subject: Drop gcp_clusters table --- .../20181031190559_drop_gcp_clusters_table.rb | 53 ++++++++++++++++++++++ db/schema.rb | 34 +------------- 2 files changed, 54 insertions(+), 33 deletions(-) create mode 100644 db/migrate/20181031190559_drop_gcp_clusters_table.rb diff --git a/db/migrate/20181031190559_drop_gcp_clusters_table.rb b/db/migrate/20181031190559_drop_gcp_clusters_table.rb new file mode 100644 index 00000000000..808d474b4fc --- /dev/null +++ b/db/migrate/20181031190559_drop_gcp_clusters_table.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +class DropGcpClustersTable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + drop_table :gcp_clusters + end + + def down + create_table :gcp_clusters do |t| + # Order columns by best align scheme + t.references :project, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade } + t.references :user, foreign_key: { on_delete: :nullify } + t.references :service, foreign_key: { on_delete: :nullify } + t.integer :status + t.integer :gcp_cluster_size, null: false + + # Timestamps + t.datetime_with_timezone :created_at, null: false + t.datetime_with_timezone :updated_at, null: false + + # Enable/disable + t.boolean :enabled, default: true + + # General + t.text :status_reason + + # k8s integration specific + t.string :project_namespace + + # Cluster details + t.string :endpoint + t.text :ca_cert + t.text :encrypted_kubernetes_token + t.string :encrypted_kubernetes_token_iv + t.string :username + t.text :encrypted_password + t.string :encrypted_password_iv + + # GKE + t.string :gcp_project_id, null: false + t.string :gcp_cluster_zone, null: false + t.string :gcp_cluster_name, null: false + t.string :gcp_machine_type + t.string :gcp_operation_id + t.text :encrypted_gcp_token + t.string :encrypted_gcp_token_iv + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 6b0bb33f969..22474916034 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20181017001059) do +ActiveRecord::Schema.define(version: 20181031190559) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -918,35 +918,6 @@ ActiveRecord::Schema.define(version: 20181017001059) do add_index "forked_project_links", ["forked_to_project_id"], name: "index_forked_project_links_on_forked_to_project_id", unique: true, using: :btree - create_table "gcp_clusters", force: :cascade do |t| - t.integer "project_id", null: false - t.integer "user_id" - t.integer "service_id" - t.integer "status" - t.integer "gcp_cluster_size", null: false - t.datetime_with_timezone "created_at", null: false - t.datetime_with_timezone "updated_at", null: false - t.boolean "enabled", default: true - t.text "status_reason" - t.string "project_namespace" - t.string "endpoint" - t.text "ca_cert" - t.text "encrypted_kubernetes_token" - t.string "encrypted_kubernetes_token_iv" - t.string "username" - t.text "encrypted_password" - t.string "encrypted_password_iv" - t.string "gcp_project_id", null: false - t.string "gcp_cluster_zone", null: false - t.string "gcp_cluster_name", null: false - t.string "gcp_machine_type" - t.string "gcp_operation_id" - t.text "encrypted_gcp_token" - t.string "encrypted_gcp_token_iv" - end - - add_index "gcp_clusters", ["project_id"], name: "index_gcp_clusters_on_project_id", unique: true, using: :btree - create_table "gpg_key_subkeys", force: :cascade do |t| t.integer "gpg_key_id", null: false t.binary "keyid" @@ -2412,9 +2383,6 @@ ActiveRecord::Schema.define(version: 20181017001059) do add_foreign_key "fork_network_members", "projects", on_delete: :cascade add_foreign_key "fork_networks", "projects", column: "root_project_id", name: "fk_e7b436b2b5", on_delete: :nullify add_foreign_key "forked_project_links", "projects", column: "forked_to_project_id", name: "fk_434510edb0", on_delete: :cascade - add_foreign_key "gcp_clusters", "projects", on_delete: :cascade - add_foreign_key "gcp_clusters", "services", on_delete: :nullify - add_foreign_key "gcp_clusters", "users", on_delete: :nullify add_foreign_key "gpg_key_subkeys", "gpg_keys", on_delete: :cascade add_foreign_key "gpg_keys", "users", on_delete: :cascade add_foreign_key "gpg_signatures", "gpg_key_subkeys", on_delete: :nullify -- cgit v1.2.1 From 319a8e8e7795dddca0cad718495d3d4c9ee67bfe Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 31 Oct 2018 19:14:56 +0900 Subject: Add changelog --- changelogs/unreleased/drop-gcp-cluster-table.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/drop-gcp-cluster-table.yml diff --git a/changelogs/unreleased/drop-gcp-cluster-table.yml b/changelogs/unreleased/drop-gcp-cluster-table.yml new file mode 100644 index 00000000000..15964ec2eaf --- /dev/null +++ b/changelogs/unreleased/drop-gcp-cluster-table.yml @@ -0,0 +1,5 @@ +--- +title: Drop gcp_clusters table +merge_request: 22713 +author: +type: other -- cgit v1.2.1 From 1d7737ae8615b31b9c0896ac43ced351159a1cf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= Date: Mon, 15 Oct 2018 10:36:07 +0200 Subject: Prepare Banzai to work with group issuables --- lib/banzai/filter/issuable_state_filter.rb | 11 ++++++-- lib/banzai/issuable_extractor.rb | 41 ++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/lib/banzai/filter/issuable_state_filter.rb b/lib/banzai/filter/issuable_state_filter.rb index d7fe012883d..8e2358694d4 100644 --- a/lib/banzai/filter/issuable_state_filter.rb +++ b/lib/banzai/filter/issuable_state_filter.rb @@ -18,7 +18,7 @@ module Banzai issuables = extractor.extract([doc]) issuables.each do |node, issuable| - next if !can_read_cross_project? && issuable.project != project + next if !can_read_cross_project? && cross_reference?(issuable) if VISIBLE_STATES.include?(issuable.state) && issuable_reference?(node.inner_html, issuable) node.content += " (#{issuable.state})" @@ -31,7 +31,14 @@ module Banzai private def issuable_reference?(text, issuable) - text == issuable.reference_link_text(project || group) + CGI.unescapeHTML(text) == issuable.reference_link_text(project || group) + end + + def cross_reference?(issuable) + return true if issuable.project != project + return true if issuable.respond_to?(:group) && issuable.group != group + + false end def can_read_cross_project? diff --git a/lib/banzai/issuable_extractor.rb b/lib/banzai/issuable_extractor.rb index 0a05d46db4c..341dbb74fe0 100644 --- a/lib/banzai/issuable_extractor.rb +++ b/lib/banzai/issuable_extractor.rb @@ -9,13 +9,11 @@ module Banzai # so we can avoid N+1 queries problem class IssuableExtractor - QUERY = %q( - descendant-or-self::a[contains(concat(" ", @class, " "), " gfm ")] - [@data-reference-type="issue" or @data-reference-type="merge_request"] - ).freeze - attr_reader :context + ISSUE_REFERENCE_TYPE = '@data-reference-type="issue"'.freeze + MERGE_REQUEST_REFERENCE_TYPE = '@data-reference-type="merge_request"'.freeze + # context - An instance of Banzai::RenderContext. def initialize(context) @context = context @@ -24,21 +22,38 @@ module Banzai # Returns Hash in the form { node => issuable_instance } def extract(documents) nodes = documents.flat_map do |document| - document.xpath(QUERY) + document.xpath(query) end - issue_parser = Banzai::ReferenceParser::IssueParser.new(context) + # The project or group for the issuable might be pending for deletion! + # Filter them out because we don't care about them. + issuables_for_nodes(nodes).select { |node, issuable| issuable.project || issuable.group } + end + + private - merge_request_parser = + def issuables_for_nodes(nodes) + parsers.each_with_object({}) do |parser, result| + result.merge!(parser.records_for_nodes(nodes)) + end + end + + def parsers + [ + Banzai::ReferenceParser::IssueParser.new(context), Banzai::ReferenceParser::MergeRequestParser.new(context) + ] + end - issuables_for_nodes = issue_parser.records_for_nodes(nodes).merge( - merge_request_parser.records_for_nodes(nodes) + def query + %Q( + descendant-or-self::a[contains(concat(" ", @class, " "), " gfm ")] + [#{reference_types.join(' or ')}] ) + end - # The project for the issue/MR might be pending for deletion! - # Filter them out because we don't care about them. - issuables_for_nodes.select { |node, issuable| issuable.project } + def reference_types + [ISSUE_REFERENCE_TYPE, MERGE_REQUEST_REFERENCE_TYPE] end end end -- cgit v1.2.1 From 4ceabef9d2bda6d72b01fb650c61c4ba1df94bbf Mon Sep 17 00:00:00 2001 From: Lukas Eipert Date: Wed, 31 Oct 2018 12:35:33 +0100 Subject: Rename @gitlab-org/gitlab-svgs to @gitlab/svgs --- app/assets/javascripts/clusters/components/applications.vue | 2 +- app/assets/javascripts/vue_shared/components/icon.vue | 2 +- app/helpers/icons_helper.rb | 2 +- config/application.rb | 2 +- config/dependency_decisions.yml | 2 +- doc/development/fe_guide/icons.md | 4 ++-- package.json | 2 +- vendor/licenses.csv | 2 +- yarn.lock | 10 +++++----- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue index 6e7b5eb5526..6d7f45a35d8 100644 --- a/app/assets/javascripts/clusters/components/applications.vue +++ b/app/assets/javascripts/clusters/components/applications.vue @@ -1,6 +1,6 @@