diff options
138 files changed, 1375 insertions, 967 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dcdf520ee6b..5a66e04247e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,25 +27,26 @@ _This notice should stay as the first item in the CONTRIBUTING.md file._ - [Helping others](#helping-others) - [I want to contribute!](#i-want-to-contribute) - [Workflow labels](#workflow-labels) - - [Type labels (~"feature proposal", ~bug, ~customer, etc.)](#type-labels-feature-proposal-bug-customer-etc) - - [Subject labels (~wiki, ~"container registry", ~ldap, ~api, etc.)](#subject-labels-wiki-container-registry-ldap-api-etc) - - [Team labels (~"CI/CD", ~Discussion, ~Quality, ~Platform, etc.)](#team-labels-cicd-discussion-quality-platform-etc) - - [Milestone labels (~Deliverable, ~Stretch, ~"Next Patch Release")](#milestone-labels-deliverable-stretch-next-patch-release) - - [Priority labels (~P1, ~P2, ~P3 , ~P4)](#bug-priority-labels-p1-p2-p3-p4) - - [Severity labels (~S1, ~S2, ~S3 , ~S4)](#bug-severity-labels-s1-s2-s3-s4) - - [Label for community contributors (~"Accepting Merge Requests")](#label-for-community-contributors-accepting-merge-requests) -- [Implement design & UI elements](#implement-design--ui-elements) + - [Type labels](#type-labels) + - [Subject labels](#subject-labels) + - [Team labels](#team-labels) + - [Milestone labels](#milestone-labels) + - [Bug Priority labels](#bug-priority-labels) + - [Bug Severity labels](#bug-severity-labels) + - [Severity impact guidance](#severity-impact-guidance) + - [Label for community contributors](#label-for-community-contributors) +- [Implement design & UI elements](#implement-design-ui-elements) - [Issue tracker](#issue-tracker) - - [Issue triaging](#issue-triaging) - - [Feature proposals](#feature-proposals) - - [Issue tracker guidelines](#issue-tracker-guidelines) - - [Issue weight](#issue-weight) - - [Regression issues](#regression-issues) - - [Technical and UX debt](#technical-and-ux-debt) - - [Stewardship](#stewardship) + - [Issue triaging](#issue-triaging) + - [Feature proposals](#feature-proposals) + - [Issue tracker guidelines](#issue-tracker-guidelines) + - [Issue weight](#issue-weight) + - [Regression issues](#regression-issues) + - [Technical and UX debt](#technical-and-ux-debt) + - [Stewardship](#stewardship) - [Merge requests](#merge-requests) - - [Merge request guidelines](#merge-request-guidelines) - - [Contribution acceptance criteria](#contribution-acceptance-criteria) + - [Merge request guidelines](#merge-request-guidelines) + - [Contribution acceptance criteria](#contribution-acceptance-criteria) - [Definition of done](#definition-of-done) - [Style guides](#style-guides) - [Code of conduct](#code-of-conduct) @@ -145,7 +146,7 @@ labels, you can _always_ add the team and type, and often also the subject. [milestones-page]: https://gitlab.com/gitlab-org/gitlab-ce/milestones [labels-page]: https://gitlab.com/gitlab-org/gitlab-ce/labels -### Type labels (~"feature proposal", ~bug, ~customer, etc.) +### Type labels Type labels are very important. They define what kind of issue this is. Every issue should have one or more. @@ -161,28 +162,41 @@ already reserved for subject labels). The descriptions on the [labels page][labels-page] explain what falls under each type label. -### Subject labels (~wiki, ~"container registry", ~ldap, ~api, etc.) +### Subject labels Subject labels are labels that define what area or feature of GitLab this issue hits. They are not always necessary, but very convenient. +Examples of subject labels are ~wiki, ~ldap, ~api, +~issues, ~"merge requests", ~labels, and ~"container registry". + If you are an expert in a particular area, it makes it easier to find issues to work on. You can also subscribe to those labels to receive an email each time an issue is labeled with a subject label corresponding to your expertise. -Examples of subject labels are ~wiki, ~"container registry", ~ldap, ~api, -~issues, ~"merge requests", ~labels, and ~"container registry". - Subject labels are always all-lowercase. -### Team labels (~"CI/CD", ~Discussion, ~Quality, ~Platform, etc.) +### Team labels Team labels specify what team is responsible for this issue. Assigning a team label makes sure issues get the attention of the appropriate people. -The current team labels are ~Distribution, ~"CI/CD", ~Discussion, ~Documentation, ~Quality, -~Geo, ~Gitaly, ~Monitoring, ~Platform, ~Release, ~"Security Products", ~"Configuration", and ~"UX". +The current team labels are: + +- ~Configuration +- ~"CI/CD" +- ~Discussion +- ~Distribution +- ~Documentation +- ~Geo +- ~Gitaly +- ~Monitoring +- ~Platform +- ~Quality +- ~Release +- ~"Security Products" +- ~UX The descriptions on the [labels page][labels-page] explain what falls under the responsibility of each team. @@ -193,7 +207,7 @@ indicate if an issue needs backend work, frontend work, or both. Team labels are always capitalized so that they show up as the first label for any issue. -### Milestone labels (~Deliverable, ~Stretch, ~"Next Patch Release") +### Milestone labels Milestone labels help us clearly communicate expectations of the work for the release. There are three levels of Milestone labels: @@ -211,9 +225,9 @@ Each issue scheduled for the current milestone should be labeled ~Deliverable or ~"Stretch". Any open issue for a previous milestone should be labeled ~"Next Patch Release", or otherwise rescheduled to a different milestone. -### Bug Priority labels (~P1, ~P2, ~P3, ~P4) +### Bug Priority labels -Bug Priority labels help us define the time a ~bug fix should be completed. Priority determines how quickly the defect turnaround time must be. +Bug Priority labels help us define the time a ~bug fix should be completed. Priority determines how quickly the defect turnaround time must be. If there are multiple defects, the priority decides which defect has to be fixed immediately versus later. This label documents the planned timeline & urgency which is used to measure against our actual SLA on delivering ~bug fixes. @@ -224,7 +238,7 @@ This label documents the planned timeline & urgency which is used to measure aga | ~P3 | Medium Priority | Within the next 3 releases (approx one quarter) | | | ~P4 | Low Priority | Anything outside the next 3 releases (approx beyond one quarter) | The issue is prominent but does not impact user workflow and a workaround is documented | -### Bug Severity labels (~S1, ~S2, ~S3, ~S4) +### Bug Severity labels Severity labels help us clearly communicate the impact of a ~bug on users. @@ -240,11 +254,11 @@ Severity labels help us clearly communicate the impact of a ~bug on users. | Label | Security Impact | Availability / Performance Impact | |-------|---------------------------------------------------------------------|--------------------------------------------------------------| | ~S1 | >50% users impacted (possible company extinction level event) | | -| ~S2 | Many users or multiple paid customers impacted (but not apocalyptic)| The issue is (almost) guaranteed to occur in the near future | +| ~S2 | Many users or multiple paid customers impacted (but not apocalyptic)| The issue is (almost) guaranteed to occur in the near future | | ~S3 | A few users or a single paid customer impacted | The issue is likely to occur in the near future | | ~S4 | No paid users/customer impacted, or expected impact within 30 days | The issue _may_ occur but it's not likely | -### Label for community contributors (~"Accepting Merge Requests") +### Label for community contributors Issues that are beneficial to our users, 'nice to haves', that we currently do not have the capacity for or want to give the priority to, are labeled as @@ -300,14 +314,14 @@ For guidance on UX implementation at GitLab, please refer to our [Design System] The UX team uses labels to manage their workflow. -The ~"UX" label on an issue is a signal to the UX team that it will need UX attention. +The ~"UX" label on an issue is a signal to the UX team that it will need UX attention. To better understand the priority by which UX tackles issues, see the [UX section](https://about.gitlab.com/handbook/engineering/ux) of the handbook. Once an issue has been worked on and is ready for development, a UXer removes the ~"UX" label and applies the ~"UX ready" label to that issue. -The UX team has a special type label called ~"design artifact". This label indicates that the final output +The UX team has a special type label called ~"design artifact". This label indicates that the final output for an issue is a UX solution/design. The solution will be developed by frontend and/or backend in a subsequent milestone. -Any issue labeled ~"design artifact" should not also be labeled ~"frontend" or ~"backend" since no development is +Any issue labeled ~"design artifact" should not also be labeled ~"frontend" or ~"backend" since no development is needed until the solution has been decided. ~"design artifact" issues are like any other issue and should contain a milestone label, ~"Deliverable" or ~"Stretch", when scheduled in the current milestone. diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 952e27df29d..223717f1818 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -299,7 +299,7 @@ GEM flowdock (~> 0.7) gitlab-grit (>= 2.4.1) multi_json - gitlab-gollum-lib (4.2.7.2) + gitlab-gollum-lib (4.2.7.4) gemojione (~> 3.2) github-markup (~> 1.6) gollum-grit_adapter (~> 1.0) @@ -307,7 +307,7 @@ GEM rouge (~> 3.1) sanitize (~> 2.1) stringex (~> 2.6) - gitlab-gollum-rugged_adapter (0.4.4) + gitlab-gollum-rugged_adapter (0.4.4.1) mime-types (>= 1.15) rugged (~> 0.25) gitlab-grit (2.8.2) diff --git a/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue b/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue index 2254271c679..d376a004e84 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue @@ -38,14 +38,17 @@ export default { return this.modifiedFilesLength ? 'multi-file-modified' : ''; }, additionsTooltip() { - return sprintf(n__('1 %{type} addition', '%d %{type} additions', this.addedFilesLength), { + return sprintf(n__('1 %{type} addition', '%{count} %{type} additions', this.addedFilesLength), { type: this.title.toLowerCase(), + count: this.addedFilesLength, }); }, modifiedTooltip() { return sprintf( - n__('1 %{type} modification', '%d %{type} modifications', this.modifiedFilesLength), - { type: this.title.toLowerCase() }, + n__('1 %{type} modification', '%{count} %{type} modifications', this.modifiedFilesLength), { + type: this.title.toLowerCase(), + count: this.modifiedFilesLength, + }, ); }, titleTooltip() { diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index d269c45203a..334279137d8 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -16,10 +16,10 @@ export default class MilestoneSelect { typeof currentProject === 'string' ? JSON.parse(currentProject) : currentProject; } - this.init(els, options); + MilestoneSelect.init(els, options); } - init(els, options) { + static init(els, options) { let $els = $(els); if (!els) { @@ -224,7 +224,6 @@ export default class MilestoneSelect { $selectBox.hide(); $value.css('display', ''); if (data.milestone != null) { - data.milestone.full_path = this.currentProject.full_path; data.milestone.remaining = timeFor(data.milestone.due_date); data.milestone.name = data.milestone.title; $value.html(milestoneLinkTemplate(data.milestone)); diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 713f93456b1..9225a6b1a7c 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -170,7 +170,6 @@ export default { :author="author" :created-at="note.created_at" :note-id="note.id" - action-text="commented" /> <note-actions :author-id="author.id" diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue index 985f44dee97..1a444c04a1d 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue @@ -173,7 +173,7 @@ </a> <clipboard-button :title="__('Copy commit SHA to clipboard')" - :text="mr.shortMergeCommitSha" + :text="mr.mergeCommitSha" css-class="btn-default btn-transparent btn-clipboard js-mr-merged-copy-sha" /> </p> diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 134aaacf9d2..c881cd496d1 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -26,6 +26,7 @@ export default class MergeRequestStore { this.mergeStatus = data.merge_status; this.commitMessage = data.merge_commit_message; this.shortMergeCommitSha = data.short_merge_commit_sha; + this.mergeCommitSha = data.merge_commit_sha; this.commitMessageWithDescription = data.merge_commit_message_with_description; this.commitsCount = data.commits_count; this.divergedCommitsCount = data.diverged_commits_count; diff --git a/app/assets/stylesheets/bootstrap_migration.scss b/app/assets/stylesheets/bootstrap_migration.scss index d92b6f9fe09..e6303ad4642 100644 --- a/app/assets/stylesheets/bootstrap_migration.scss +++ b/app/assets/stylesheets/bootstrap_migration.scss @@ -89,6 +89,11 @@ a { color: $gl-link-color; } +a:not(.btn):focus, +a:not(.btn):active { + text-decoration: underline; +} + hr { overflow: hidden; } @@ -173,7 +178,10 @@ table { display: none; } -.badge { +// Add to .label so that old system notes that are saved to the db +// will still receive the correct styling +.badge, +.label { padding: 4px 5px; font-size: 12px; font-style: normal; @@ -208,6 +216,15 @@ table { &:not(:last-of-type) { border-bottom: 1px solid $well-inner-border; } + + p, + ol, + ul, + .form-group { + &:last-of-type { + margin-bottom: 0; + } + } } .badge.badge-gray { diff --git a/app/assets/stylesheets/framework/contextual_sidebar.scss b/app/assets/stylesheets/framework/contextual_sidebar.scss index 9cbaaa5dc8d..cccd1a6d942 100644 --- a/app/assets/stylesheets/framework/contextual_sidebar.scss +++ b/app/assets/stylesheets/framework/contextual_sidebar.scss @@ -193,6 +193,7 @@ &:focus { background: $link-active-background; color: $gl-text-color; + text-decoration: none; } } diff --git a/app/assets/stylesheets/framework/flash.scss b/app/assets/stylesheets/framework/flash.scss index 52c3f18a682..a6e324036ae 100644 --- a/app/assets/stylesheets/framework/flash.scss +++ b/app/assets/stylesheets/framework/flash.scss @@ -10,6 +10,20 @@ @extend .alert; background-color: $blue-500; margin: 0; + + &.flash-notice-persistent { + background-color: $blue-100; + color: $gl-text-color; + + a { + color: $gl-link-color; + + &:hover { + color: $gl-link-hover-color; + text-decoration: none; + } + } + } } .flash-warning { diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 1cbb8b7053a..f30f296d41f 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -244,10 +244,11 @@ $tooltip-font-size: 12px; /* * Padding */ -$gl-padding-24: 24px; -$gl-padding: 16px; -$gl-padding-8: 8px; $gl-padding-4: 4px; +$gl-padding-8: 8px; +$gl-padding: 16px; +$gl-padding-24: 24px; +$gl-padding-32: 32px; $gl-col-padding: 15px; $gl-input-padding: 10px; $gl-vert-padding: 6px; diff --git a/app/assets/stylesheets/pages/detail_page.scss b/app/assets/stylesheets/pages/detail_page.scss index 7b36bcb3c7d..2e007c52592 100644 --- a/app/assets/stylesheets/pages/detail_page.scss +++ b/app/assets/stylesheets/pages/detail_page.scss @@ -23,7 +23,8 @@ position: relative; line-height: 35px; display: flex; - flex-grow: 1; + flex: 1 1; + min-width: 0; @include media-breakpoint-up(sm) { padding-left: 0; diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index b42c232fd91..f9fd9f1ab8b 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -698,6 +698,8 @@ font-size: 14px; line-height: 24px; align-self: center; + overflow: hidden; + text-overflow: ellipsis; } .js-issuable-selector-wrap { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 99fe4a578be..596d3aa171c 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -46,6 +46,7 @@ .btn { font-size: $gl-font-size; + max-height: 26px; &[disabled] { opacity: 0.3; diff --git a/app/assets/stylesheets/pages/profiles/preferences.scss b/app/assets/stylesheets/pages/profiles/preferences.scss index babe81cb0f7..a353f301d07 100644 --- a/app/assets/stylesheets/pages/profiles/preferences.scss +++ b/app/assets/stylesheets/pages/profiles/preferences.scss @@ -16,7 +16,7 @@ .application-theme { label { - margin: 0 $gl-padding $gl-padding 0; + margin: 0 $gl-padding-32 $gl-padding 0; text-align: center; } @@ -24,7 +24,7 @@ font-size: 0; height: 48px; border-radius: 4px; - min-width: 135px; + min-width: 112px; margin-bottom: $gl-padding-8; &.ui-indigo { @@ -75,7 +75,8 @@ .syntax-theme { label { - margin-right: 20px; + margin-right: $gl-padding-32; + margin-bottom: $gl-padding; text-align: center; .preview { @@ -84,7 +85,6 @@ img { border-radius: 4px; - max-width: 100%; } } diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 4abb145067a..2f28031b9c8 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -127,13 +127,6 @@ color: $gl-danger; } -.service-settings { - input[type="radio"], - input[type="checkbox"] { - margin-top: 10px; - } -} - .integration-settings-form { .card.card-body, .info-well { @@ -296,7 +289,8 @@ } .btn-clipboard { - margin-left: 5px; + background-color: $white-light; + border: 1px solid $theme-gray-200; } .deploy-token-help-block { diff --git a/app/controllers/concerns/issues_action.rb b/app/controllers/concerns/issues_action.rb index b6eb7d292fc..9d58656773d 100644 --- a/app/controllers/concerns/issues_action.rb +++ b/app/controllers/concerns/issues_action.rb @@ -1,6 +1,7 @@ module IssuesAction extend ActiveSupport::Concern include IssuableCollections + include IssuesCalendar # rubocop:disable Gitlab/ModuleWithInstanceVariables def issues @@ -17,18 +18,9 @@ module IssuesAction end # rubocop:enable Gitlab/ModuleWithInstanceVariables - # rubocop:disable Gitlab/ModuleWithInstanceVariables def issues_calendar - @issues = issuables_collection - .non_archived - .with_due_date - .limit(100) - - respond_to do |format| - format.ics { response.headers['Content-Disposition'] = 'inline' } - end + render_issues_calendar(issuables_collection) end - # rubocop:enable Gitlab/ModuleWithInstanceVariables private diff --git a/app/controllers/concerns/issues_calendar.rb b/app/controllers/concerns/issues_calendar.rb new file mode 100644 index 00000000000..671a204621d --- /dev/null +++ b/app/controllers/concerns/issues_calendar.rb @@ -0,0 +1,24 @@ +module IssuesCalendar + extend ActiveSupport::Concern + + # rubocop:disable Gitlab/ModuleWithInstanceVariables + def render_issues_calendar(issuables) + @issues = issuables + .non_archived + .with_due_date + .limit(100) + + respond_to do |format| + format.ics do + # NOTE: with text/calendar as Content-Type, the browser always downloads + # the content as a file (even ignoring the Content-Disposition + # header). We want to display the content inline when accessed + # from GitLab, similarly to the RSS feed. + if request.referer&.start_with?(::Settings.gitlab.base_url) + response.headers['Content-Type'] = 'text/plain' + end + end + end + end + # rubocop:enable Gitlab/ModuleWithInstanceVariables +end diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index 170bca8b56f..16374146ae4 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -1,9 +1,15 @@ module UploadsActions + extend ActiveSupport::Concern + include Gitlab::Utils::StrongMemoize include SendFileUpload UPLOAD_MOUNTS = %w(avatar attachment file logo header_logo favicon).freeze + included do + prepend_before_action :set_html_format, only: :show + end + def create link_to_file = UploadService.new(model, params[:file], uploader_class).execute @@ -41,6 +47,13 @@ module UploadsActions private + # Explicitly set the format. + # Otherwise rails 5 will set it from a file extension. + # See https://github.com/rails/rails/commit/84e8accd6fb83031e4c27e44925d7596655285f7#diff-2b8f2fbb113b55ca8e16001c393da8f1 + def set_html_format + request.format = :html + end + def uploader_class raise NotImplementedError end diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index abc283d7aa9..6484a713f8e 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -7,6 +7,7 @@ class Projects::ArtifactsController < Projects::ApplicationController before_action :authorize_read_build! before_action :authorize_update_build!, only: [:keep] before_action :extract_ref_name_and_path + before_action :set_request_format, only: [:file] before_action :validate_artifacts! before_action :entry, only: [:file] @@ -101,4 +102,12 @@ class Projects::ArtifactsController < Projects::ApplicationController render_404 unless @entry.exists? end + + def set_request_format + request.format = :html if set_request_format? + end + + def set_request_format? + request.format != :json + end end diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 7c65f1f5dfe..a8c0a68fc17 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -7,6 +7,7 @@ class Projects::BlobController < Projects::ApplicationController prepend_before_action :authenticate_user!, only: [:edit] + before_action :set_request_format, only: [:edit, :show, :update] before_action :require_non_empty_project, except: [:new, :create] before_action :authorize_download_code! @@ -188,6 +189,18 @@ class Projects::BlobController < Projects::ApplicationController .last_for_path(@repository, @ref, @path).sha end + # In Rails 4.2 if params[:format] is empty, Rails set it to :html + # But since Rails 5.0 the framework now looks for an extension. + # E.g. for `blob/master/CHANGELOG.md` in Rails 4 the format would be `:html`, but in Rails 5 on it'd be `:md` + # This before_action explicitly sets the `:html` format for all requests unless `:format` is set by a client e.g. by JS for XHR requests. + def set_request_format + request.format = :html if set_request_format? + end + + def set_request_format? + params[:id].present? && params[:format].blank? && request.format != "json" + end + def show_html environment_params = @repository.branch_exists?(@ref) ? { ref: @ref } : { commit: @commit } @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index b7b36f770f5..cd7250b10fc 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -31,7 +31,10 @@ class Projects::BranchesController < Projects::ApplicationController end end - render + # https://gitlab.com/gitlab-org/gitlab-ce/issues/48097 + Gitlab::GitalyClient.allow_n_plus_1_calls do + render + end end format.json do branches = BranchesFinder.new(@repository, params).execute diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 35c36c725e2..7c897b2d86c 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -4,6 +4,7 @@ class Projects::IssuesController < Projects::ApplicationController include IssuableActions include ToggleAwardEmoji include IssuableCollections + include IssuesCalendar include SpammableActions prepend_before_action :authenticate_user!, only: [:new] @@ -40,14 +41,7 @@ class Projects::IssuesController < Projects::ApplicationController end def calendar - @issues = @issuables - .non_archived - .with_due_date - .limit(100) - - respond_to do |format| - format.ics { response.headers['Content-Disposition'] = 'inline' } - end + render_issues_calendar(@issuables) end def new diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index b452bfd7e6f..38918b3cd52 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -115,7 +115,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end format.json do - render json: @merge_request.to_json(include: { milestone: {}, assignee: { only: [:name, :username], methods: [:avatar_url] }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) + render json: serializer.represent(@merge_request, serializer: 'basic') end end rescue ActiveRecord::StaleObjectError diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index d42284868c7..9f501ea55fb 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -157,7 +157,7 @@ module IssuablesHelper output = "" output << "Opened #{time_ago_with_tooltip(issuable.created_at)} by ".html_safe output << content_tag(:strong) do - author_output = link_to_member(project, issuable.author, size: 24, mobile_classes: "d-none d-sm-inline-block", tooltip: true) + author_output = link_to_member(project, issuable.author, size: 24, mobile_classes: "d-none d-sm-inline", tooltip: true) author_output << link_to_member(project, issuable.author, size: 24, by_username: true, avatar: false, mobile_classes: "d-block d-sm-none") end diff --git a/app/models/project.rb b/app/models/project.rb index e5fa1c4db7b..0d777515536 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -25,6 +25,7 @@ class Project < ActiveRecord::Base include FastDestroyAll::Helpers include WithUploads include BatchDestroyDependentAssociations + extend Gitlab::Cache::RequestCache extend Gitlab::ConfigHelper @@ -2013,6 +2014,11 @@ class Project < ActiveRecord::Base @gitlab_deploy_token ||= deploy_tokens.gitlab_deploy_token end + def any_lfs_file_locks? + lfs_file_locks.any? + end + request_cache(:any_lfs_file_locks?) { self.id } + private def storage diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index ae0debbd3ac..a60b4c7fd0d 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -155,6 +155,7 @@ class ChatNotificationService < Service end def notify_for_ref?(data) + return true if data[:object_kind] == 'tag_push' return true if data.dig(:object_attributes, :tag) return true unless notify_only_default_branch? diff --git a/app/serializers/merge_request_basic_entity.rb b/app/serializers/merge_request_basic_entity.rb index e4aec977f01..1c06691026d 100644 --- a/app/serializers/merge_request_basic_entity.rb +++ b/app/serializers/merge_request_basic_entity.rb @@ -5,4 +5,8 @@ class MergeRequestBasicEntity < IssuableSidebarEntity expose :state expose :source_branch_exists?, as: :source_branch_exists expose :rebase_in_progress?, as: :rebase_in_progress + expose :milestone, using: API::Entities::Milestone + expose :labels, using: LabelEntity + expose :assignee, using: API::Entities::UserBasic + expose :task_status, :task_status_short end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 925775aea0b..9bdbb2c0d99 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -25,14 +25,12 @@ module Ci valid = true - if Feature.enabled?('ci_job_request_with_tags_matcher') - # pick builds that does not have other tags than runner's one - builds = builds.matches_tag_ids(runner.tags.ids) + # pick builds that does not have other tags than runner's one + builds = builds.matches_tag_ids(runner.tags.ids) - # pick builds that have at least one tag - unless runner.run_untagged? - builds = builds.with_any_tags - end + # pick builds that have at least one tag + unless runner.run_untagged? + builds = builds.with_any_tags end builds.find do |build| diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 3e38a8a12d4..aa60661f7f2 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -11,7 +11,7 @@ module Projects order: { due_date: :asc, title: :asc } } - finder_params[:group_ids] = @project.group.self_and_ancestors.select(:id) if @project.group + finder_params[:group_ids] = @project.group.self_and_ancestors_ids if @project.group MilestonesFinder.new(finder_params).execute.select([:iid, :title]) end diff --git a/app/views/email_rejection_mailer/rejection.html.haml b/app/views/email_rejection_mailer/rejection.html.haml index 7f7d841fe21..c4ae7befe4e 100644 --- a/app/views/email_rejection_mailer/rejection.html.haml +++ b/app/views/email_rejection_mailer/rejection.html.haml @@ -2,3 +2,4 @@ Unfortunately, your email message to GitLab could not be processed. = markdown @reason += render_if_exists 'shared/additional_email_text' diff --git a/app/views/email_rejection_mailer/rejection.text.haml b/app/views/email_rejection_mailer/rejection.text.haml index af518b5b583..0e13b2a6473 100644 --- a/app/views/email_rejection_mailer/rejection.text.haml +++ b/app/views/email_rejection_mailer/rejection.text.haml @@ -1,3 +1,4 @@ Unfortunately, your email message to GitLab could not be processed. \ = @reason += render_if_exists 'shared/additional_email_text' diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 0a9adc6f243..dd6a84e503d 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -9,6 +9,8 @@ %p Assignee: #{@merge_request.assignee_name} += render_if_exists 'notify/merge_request_approvers', merge_request: @merge_request + - if @merge_request.description %div = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author) diff --git a/app/views/notify/new_merge_request_email.text.erb b/app/views/notify/new_merge_request_email.text.erb index 7d98400e6fe..d5b8f8d764f 100644 --- a/app/views/notify/new_merge_request_email.text.erb +++ b/app/views/notify/new_merge_request_email.text.erb @@ -5,6 +5,6 @@ New Merge Request <%= @merge_request.to_reference %> <%= merge_path_description(@merge_request, 'to') %> Author: <%= @merge_request.author_name %> Assignee: <%= @merge_request.assignee_name %> +<%= render_if_exists 'notify/merge_request_approvers', merge_request: @merge_request %> <%= @merge_request.description %> - diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml index 1b150ec3e5c..0a0b3ce1d6f 100644 --- a/app/views/projects/blob/_header.html.haml +++ b/app/views/projects/blob/_header.html.haml @@ -11,6 +11,7 @@ = view_on_environment_button(@commit.sha, @path, @environment) if @environment .btn-group{ role: "group" }< + = render_if_exists 'projects/blob/header_file_locks_link' = edit_blob_button = ide_edit_button - if current_user @@ -18,3 +19,4 @@ = delete_blob_link = render 'projects/fork_suggestion' += render_if_exists 'projects/blob/header_file_locks', project: @project, path: @path diff --git a/app/views/projects/clusters/_sidebar.html.haml b/app/views/projects/clusters/_sidebar.html.haml index 73cd7c50922..3d10348212f 100644 --- a/app/views/projects/clusters/_sidebar.html.haml +++ b/app/views/projects/clusters/_sidebar.html.haml @@ -1,7 +1,9 @@ +- clusters_help_url = help_page_path('user/project/clusters/index.md') +- help_link_start = "<a href=\"%{url}\" target=\"_blank\" rel=\"noopener noreferrer\">".html_safe +- help_link_end = '</a>'.html_safe %h4.prepend-top-0 = s_('ClusterIntegration|Kubernetes cluster integration') %p = s_('ClusterIntegration|With a Kubernetes cluster associated to this project, you can use review apps, deploy your applications, run your pipelines, and much more in an easy way.') %p - - link = link_to(_('Kubernetes'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer') - = s_('ClusterIntegration|Learn more about %{link_to_documentation}').html_safe % { link_to_documentation: link } + = s_('ClusterIntegration|Learn more about %{help_link_start}Kubernetes%{help_link_end}.').html_safe % { help_link_start: help_link_start % { url: clusters_help_url }, help_link_end: help_link_end } diff --git a/app/views/projects/deploy_tokens/_index.html.haml b/app/views/projects/deploy_tokens/_index.html.haml index 50e5950ced4..725720d2222 100644 --- a/app/views/projects/deploy_tokens/_index.html.haml +++ b/app/views/projects/deploy_tokens/_index.html.haml @@ -10,9 +10,8 @@ .settings-content - if @new_deploy_token.persisted? = render 'projects/deploy_tokens/new_deploy_token', deploy_token: @new_deploy_token - - else - %h5.prepend-top-0 - = s_('DeployTokens|Add a deploy token') - = render 'projects/deploy_tokens/form', project: @project, token: @new_deploy_token, presenter: @deploy_tokens - %hr + %h5.prepend-top-0 + = s_('DeployTokens|Add a deploy token') + = render 'projects/deploy_tokens/form', project: @project, token: @new_deploy_token, presenter: @deploy_tokens + %hr = render 'projects/deploy_tokens/table', project: @project, active_tokens: @deploy_tokens diff --git a/app/views/projects/deploy_tokens/_new_deploy_token.html.haml b/app/views/projects/deploy_tokens/_new_deploy_token.html.haml index 1e715681e59..5dd9ffba074 100644 --- a/app/views/projects/deploy_tokens/_new_deploy_token.html.haml +++ b/app/views/projects/deploy_tokens/_new_deploy_token.html.haml @@ -1,14 +1,18 @@ -.created-deploy-token-container - %h5.prepend-top-0 - = s_('DeployTokens|Your New Deploy Token') +.created-deploy-token-container.info-well + .well-segment + %h5.prepend-top-0 + = s_('DeployTokens|Your New Deploy Token') - .form-group - = text_field_tag 'deploy-token-user', deploy_token.username, readonly: true, class: 'deploy-token-field form-control js-select-on-focus' - = clipboard_button(text: deploy_token.username, title: s_('DeployTokens|Copy username to clipboard'), placement: 'left') - %span.deploy-token-help-block.prepend-top-5.text-success= s_("DeployTokens|Use this username as a login.") + .form-group + .input-group + = text_field_tag 'deploy-token-user', deploy_token.username, readonly: true, class: 'deploy-token-field form-control js-select-on-focus' + .input-group-append + = clipboard_button(text: deploy_token.username, title: s_('DeployTokens|Copy username to clipboard'), placement: 'left') + %span.deploy-token-help-block.prepend-top-5.text-success= s_("DeployTokens|Use this username as a login.") - .form-group - = text_field_tag 'deploy-token', deploy_token.token, readonly: true, class: 'deploy-token-field form-control js-select-on-focus' - = clipboard_button(text: deploy_token.token, title: s_('DeployTokens|Copy deploy token to clipboard'), placement: 'left') - %span.deploy-token-help-block.prepend-top-5.text-danger= s_("DeployTokens|Use this token as a password. Make sure you save it - you won't be able to access it again.") -%hr + .form-group + .input-group + = text_field_tag 'deploy-token', deploy_token.token, readonly: true, class: 'deploy-token-field form-control js-select-on-focus' + .input-group-append + = clipboard_button(text: deploy_token.token, title: s_('DeployTokens|Copy deploy token to clipboard'), placement: 'left') + %span.deploy-token-help-block.prepend-top-5.text-danger= s_("DeployTokens|Use this token as a password. Make sure you save it - you won't be able to access it again.") diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index b478fbbb15e..f7b04c436a6 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -69,6 +69,8 @@ .wiki = markdown_field(@milestone, :description) + = render_if_exists 'shared/milestones/burndown', milestone: @milestone, project: @project + - if can?(current_user, :read_issue, @project) && @milestone.total_items_count(current_user).zero? .alert.alert-success.prepend-top-default %span Assign some issues to this milestone. diff --git a/app/views/projects/refs/logs_tree.js.haml b/app/views/projects/refs/logs_tree.js.haml index d07bb661615..506bf54b3f8 100644 --- a/app/views/projects/refs/logs_tree.js.haml +++ b/app/views/projects/refs/logs_tree.js.haml @@ -8,6 +8,8 @@ row.find("td.tree-time-ago").html('#{escape_javascript time_ago_with_tooltip(commit.committed_date)}'); row.find("td.tree-commit").html('#{escape_javascript render("projects/tree/tree_commit_column", commit: commit)}'); + = render_if_exists 'projects/refs/logs_tree_lock_label', lock_label: content_data[:lock_label] + - if @more_log_url :plain if($('#tree-slider').length) { diff --git a/app/views/projects/services/_index.html.haml b/app/views/projects/services/_index.html.haml index acbab8b85c9..16e48814578 100644 --- a/app/views/projects/services/_index.html.haml +++ b/app/views/projects/services/_index.html.haml @@ -8,7 +8,7 @@ %colgroup %col %col - %col.d-none.d-sm-block + %col %col{ width: "120" } %thead %tr diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index ca6e3602f05..526330f4e50 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -36,8 +36,6 @@ = note.author.to_reference %span.note-headline-light %span.note-headline-meta - - unless note.system - commented - if note.system %span.system-note-message = markdown_field(note, :note) diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index db48bb7e8b8..dbb215f1964 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -8,28 +8,12 @@ class RepositoryForkWorker target_project_id = args.shift target_project = Project.find(target_project_id) - # By v10.8, we should've drained the queue of all jobs using the old arguments. - # We can remove the else clause if we're no longer logging the message in that clause. - # See https://gitlab.com/gitlab-org/gitaly/issues/1110 - if args.empty? - source_project = target_project.forked_from_project - unless source_project - return target_project.mark_import_as_failed('Source project cannot be found.') - end - - fork_repository(target_project, source_project.repository_storage, source_project.disk_path) - else - Rails.logger.info("Project #{target_project.id} is being forked using old-style arguments.") - - source_repository_storage_path, source_disk_path = *args - - source_repository_storage_name = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab.config.repositories.storages.find do |_, info| - info.legacy_disk_path == source_repository_storage_path - end&.first || raise("no shard found for path '#{source_repository_storage_path}'") - end - fork_repository(target_project, source_repository_storage_name, source_disk_path) + source_project = target_project.forked_from_project + unless source_project + return target_project.mark_import_as_failed('Source project cannot be found.') end + + fork_repository(target_project, source_project.repository_storage, source_project.disk_path) end private diff --git a/changelogs/unreleased/45487-slack-tag-push-notifs.yml b/changelogs/unreleased/45487-slack-tag-push-notifs.yml new file mode 100644 index 00000000000..647000bd97c --- /dev/null +++ b/changelogs/unreleased/45487-slack-tag-push-notifs.yml @@ -0,0 +1,5 @@ +--- +title: Fix chat service tag notifications not sending when only default branch enabled +merge_request: 19864 +author: +type: fixed diff --git a/changelogs/unreleased/46429-creating-a-deploy-token-doesn-t-bring-back-to-the-creation-page.yml b/changelogs/unreleased/46429-creating-a-deploy-token-doesn-t-bring-back-to-the-creation-page.yml new file mode 100644 index 00000000000..b564fb0174f --- /dev/null +++ b/changelogs/unreleased/46429-creating-a-deploy-token-doesn-t-bring-back-to-the-creation-page.yml @@ -0,0 +1,5 @@ +--- +title: Allows you to create another deploy token dimmediately after creating one +merge_request: 19639 +author: +type: changed diff --git a/changelogs/unreleased/46861-issuable-title-with-longer-username.yml b/changelogs/unreleased/46861-issuable-title-with-longer-username.yml new file mode 100644 index 00000000000..9df6879deb6 --- /dev/null +++ b/changelogs/unreleased/46861-issuable-title-with-longer-username.yml @@ -0,0 +1,5 @@ +--- +title: Fix CSS for buttons not to be hidden on issues/MR title +merge_request: 19176 +author: Takuya Noguchi +type: fixed diff --git a/changelogs/unreleased/47672-set_inline_content_type_for_ics.yml b/changelogs/unreleased/47672-set_inline_content_type_for_ics.yml new file mode 100644 index 00000000000..4bc883d41bd --- /dev/null +++ b/changelogs/unreleased/47672-set_inline_content_type_for_ics.yml @@ -0,0 +1,5 @@ +--- +title: Render calendar feed inline when accessed from GitLab +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/48050-add-full-commit-sha.yml b/changelogs/unreleased/48050-add-full-commit-sha.yml new file mode 100644 index 00000000000..30376fe35e0 --- /dev/null +++ b/changelogs/unreleased/48050-add-full-commit-sha.yml @@ -0,0 +1,5 @@ +--- +title: Uses long sha version of the merged commit in MR widget copy to clipboard button +merge_request: 19955 +author: +type: other diff --git a/changelogs/unreleased/blackst0ne-rails5-expected-search-search-seed_project-got-nil.yml b/changelogs/unreleased/blackst0ne-rails5-expected-search-search-seed_project-got-nil.yml new file mode 100644 index 00000000000..e7bb2703b03 --- /dev/null +++ b/changelogs/unreleased/blackst0ne-rails5-expected-search-search-seed_project-got-nil.yml @@ -0,0 +1,5 @@ +--- +title: "[Rails5] Fix sessions_controller_spec" +merge_request: 19936 +author: "@blackst0ne" +type: fixed diff --git a/changelogs/unreleased/blackst0ne-rails5-expected-the-response-to-have-status-code-ok-but-it-was-404.yml b/changelogs/unreleased/blackst0ne-rails5-expected-the-response-to-have-status-code-ok-but-it-was-404.yml new file mode 100644 index 00000000000..fad15de2dd5 --- /dev/null +++ b/changelogs/unreleased/blackst0ne-rails5-expected-the-response-to-have-status-code-ok-but-it-was-404.yml @@ -0,0 +1,5 @@ +--- +title: "[Rails5] Set request.format for artifacts_controller" +merge_request: 19937 +author: "@blackst0ne" +type: fixed diff --git a/changelogs/unreleased/blackst0ne-rails5-fix-blob-requests-format.yml b/changelogs/unreleased/blackst0ne-rails5-fix-blob-requests-format.yml new file mode 100644 index 00000000000..a83aa03606a --- /dev/null +++ b/changelogs/unreleased/blackst0ne-rails5-fix-blob-requests-format.yml @@ -0,0 +1,5 @@ +--- +title: "[Rails5] Explicitly set request.format for blob_controller" +merge_request: 19876 +author: "@blackst0ne" +type: fixed diff --git a/changelogs/unreleased/blackst0ne-rails5-fix-pipeline-schedules-controller-spec.yml b/changelogs/unreleased/blackst0ne-rails5-fix-pipeline-schedules-controller-spec.yml new file mode 100644 index 00000000000..7a2b19ad681 --- /dev/null +++ b/changelogs/unreleased/blackst0ne-rails5-fix-pipeline-schedules-controller-spec.yml @@ -0,0 +1,5 @@ +--- +title: "[Rails5] Fix pipeline_schedules_controller_spec" +merge_request: 19919 +author: "@blackst0ne" +type: fixed diff --git a/changelogs/unreleased/blackst0ne-rails5-invalid-single-table-inheritance-type-group-is-not-a-subclass-of-namespace.yml b/changelogs/unreleased/blackst0ne-rails5-invalid-single-table-inheritance-type-group-is-not-a-subclass-of-namespace.yml new file mode 100644 index 00000000000..92e6ce35941 --- /dev/null +++ b/changelogs/unreleased/blackst0ne-rails5-invalid-single-table-inheritance-type-group-is-not-a-subclass-of-namespace.yml @@ -0,0 +1,6 @@ +--- +title: "[Rails5] Invalid single-table inheritance type: Group is not a subclass of + Namespace" +merge_request: 19918 +author: "@blackst0ne" +type: fixed diff --git a/changelogs/unreleased/dm-blockquote-trailing-whitespace.yml b/changelogs/unreleased/dm-blockquote-trailing-whitespace.yml new file mode 100644 index 00000000000..98ecdde4f4c --- /dev/null +++ b/changelogs/unreleased/dm-blockquote-trailing-whitespace.yml @@ -0,0 +1,5 @@ +--- +title: Allow trailing whitespace on blockquote fence lines +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/dm-branch-api-can-push.yml b/changelogs/unreleased/dm-branch-api-can-push.yml new file mode 100644 index 00000000000..3be8962089b --- /dev/null +++ b/changelogs/unreleased/dm-branch-api-can-push.yml @@ -0,0 +1,5 @@ +--- +title: Expose whether current user can push into a branch on branches API +merge_request: +author: +type: added diff --git a/changelogs/unreleased/fix-groups-api-ordering.yml b/changelogs/unreleased/fix-groups-api-ordering.yml new file mode 100644 index 00000000000..3a6a7f84356 --- /dev/null +++ b/changelogs/unreleased/fix-groups-api-ordering.yml @@ -0,0 +1,4 @@ +title: Fixed pagination of groups API +merge_request: 19665 +author: Marko, Peter +type: added diff --git a/changelogs/unreleased/more-group-api-sorting-options.yml b/changelogs/unreleased/more-group-api-sorting-options.yml new file mode 100644 index 00000000000..b29f76a65a9 --- /dev/null +++ b/changelogs/unreleased/more-group-api-sorting-options.yml @@ -0,0 +1,5 @@ +--- +title: Added id sorting option to GET groups and subgroups API +merge_request: 19665 +author: Marko, Peter +type: added diff --git a/changelogs/unreleased/optimise-paused-runners.yml b/changelogs/unreleased/optimise-paused-runners.yml new file mode 100644 index 00000000000..13097e507d3 --- /dev/null +++ b/changelogs/unreleased/optimise-paused-runners.yml @@ -0,0 +1,5 @@ +--- +title: Optimise paused runners to reduce amount of used requests +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/rails5-fix-46276.yml b/changelogs/unreleased/rails5-fix-46276.yml new file mode 100644 index 00000000000..cdca91a755d --- /dev/null +++ b/changelogs/unreleased/rails5-fix-46276.yml @@ -0,0 +1,5 @@ +--- +title: Rails5 fix format in uploads actions +merge_request: 19907 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/rails5-fix-47960.yml b/changelogs/unreleased/rails5-fix-47960.yml new file mode 100644 index 00000000000..2229511ccd6 --- /dev/null +++ b/changelogs/unreleased/rails5-fix-47960.yml @@ -0,0 +1,5 @@ +--- +title: Rails5 fix update_attribute usage not causing a save +merge_request: 19881 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/rails5-fix-48009.yml b/changelogs/unreleased/rails5-fix-48009.yml new file mode 100644 index 00000000000..7ade9ef2b7d --- /dev/null +++ b/changelogs/unreleased/rails5-fix-48009.yml @@ -0,0 +1,5 @@ +--- +title: Rails5 update Gemfile.rails5.lock +merge_request: 19921 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/rails5-fix-48012.yml b/changelogs/unreleased/rails5-fix-48012.yml new file mode 100644 index 00000000000..953ccbd8af7 --- /dev/null +++ b/changelogs/unreleased/rails5-fix-48012.yml @@ -0,0 +1,6 @@ +--- +title: Rails5 fix passing Group objects array into for_projects_and_groups milestone + scope +merge_request: 19920 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/rails5-fix-db-check.yml b/changelogs/unreleased/rails5-fix-db-check.yml new file mode 100644 index 00000000000..ccac4619ea7 --- /dev/null +++ b/changelogs/unreleased/rails5-fix-db-check.yml @@ -0,0 +1,5 @@ +--- +title: Rails5 fix connection execute return integer instead of string +merge_request: 19901 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/remove-ci_job_request_with_tags_matcher.yml b/changelogs/unreleased/remove-ci_job_request_with_tags_matcher.yml new file mode 100644 index 00000000000..b86512445d5 --- /dev/null +++ b/changelogs/unreleased/remove-ci_job_request_with_tags_matcher.yml @@ -0,0 +1,5 @@ +--- +title: Remove the ci_job_request_with_tags_matcher +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/sh-optimize-locks-check-ce.yml b/changelogs/unreleased/sh-optimize-locks-check-ce.yml new file mode 100644 index 00000000000..933ec9b79bf --- /dev/null +++ b/changelogs/unreleased/sh-optimize-locks-check-ce.yml @@ -0,0 +1,5 @@ +--- +title: Eliminate N+1 queries in LFS file locks checks during a push +merge_request: +author: +type: performance diff --git a/config/routes.rb b/config/routes.rb index 52726f94753..e0a9139b1b4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -11,6 +11,12 @@ Rails.application.routes.draw do post :toggle_award_emoji, on: :member end + favicon_redirect = redirect do |_params, _request| + ActionController::Base.helpers.asset_url(Gitlab::Favicon.main) + end + get 'favicon.png', to: favicon_redirect + get 'favicon.ico', to: favicon_redirect + draw :sherlock draw :development draw :ci diff --git a/doc/administration/auth/how_to_configure_ldap_gitlab_ce/index.md b/doc/administration/auth/how_to_configure_ldap_gitlab_ce/index.md index aa5e9513290..621d4f77d5e 100644 --- a/doc/administration/auth/how_to_configure_ldap_gitlab_ce/index.md +++ b/doc/administration/auth/how_to_configure_ldap_gitlab_ce/index.md @@ -107,7 +107,7 @@ Global Admins GitLab.org/GitLab INT/Global Groups/Global Admins ## GitLab LDAP configuration -The initial configuration of LDAP in GitLab requires changes to the `gitlab.rb` configuration file. Below is an example of a complete configuration using an Active Directory. +The initial configuration of LDAP in GitLab requires changes to the `gitlab.rb` configuration file (`/etc/gitlab/gitlab.rb`). Below is an example of a complete configuration using an Active Directory. The two Active Directory specific values are `active_directory: true` and `uid: 'sAMAccountName'`. `sAMAccountName` is an attribute returned by Active Directory used for GitLab usernames. See the example output from `ldapsearch` for a full list of attributes a "person" object (user) has in **AD** - [`ldapsearch` example](#using-ldapsearch-unix) diff --git a/doc/api/branches.md b/doc/api/branches.md index 01bb30c3859..bfb21608d28 100644 --- a/doc/api/branches.md +++ b/doc/api/branches.md @@ -29,6 +29,7 @@ Example response: "protected": true, "developers_can_push": false, "developers_can_merge": false, + "can_push": true, "commit": { "author_email": "john@example.com", "author_name": "John Smith", @@ -76,6 +77,7 @@ Example response: "protected": true, "developers_can_push": false, "developers_can_merge": false, + "can_push": true, "commit": { "author_email": "john@example.com", "author_name": "John Smith", @@ -140,7 +142,8 @@ Example response: "merged": false, "protected": true, "developers_can_push": true, - "developers_can_merge": true + "developers_can_merge": true, + "can_push": true } ``` @@ -188,7 +191,8 @@ Example response: "merged": false, "protected": false, "developers_can_push": false, - "developers_can_merge": false + "developers_can_merge": false, + "can_push": true } ``` @@ -231,7 +235,8 @@ Example response: "merged": false, "protected": false, "developers_can_push": false, - "developers_can_merge": false + "developers_can_merge": false, + "can_push": true } ``` diff --git a/doc/api/groups.md b/doc/api/groups.md index 96842ef330f..a48905f2f15 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -12,7 +12,7 @@ Parameters: | `skip_groups` | array of integers | no | Skip the group IDs passed | | `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for admin) | | `search` | string | no | Return the list of authorized groups matching the search criteria | -| `order_by` | string | no | Order groups by `name` or `path`. Default is `name` | +| `order_by` | string | no | Order groups by `name`, `path` or `id`. Default is `name` | | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` | | `statistics` | boolean | no | Include group statistics (admins only) | | `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | @@ -96,7 +96,7 @@ Parameters: | `skip_groups` | array of integers | no | Skip the group IDs passed | | `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for admin) | | `search` | string | no | Return the list of authorized groups matching the search criteria | -| `order_by` | string | no | Order groups by `name` or `path`. Default is `name` | +| `order_by` | string | no | Order groups by `name`, `path` or `id`. Default is `name` | | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` | | `statistics` | boolean | no | Include group statistics (admins only) | | `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 8e13ceb9257..26dcf67fe23 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -88,18 +88,18 @@ The example below simply moves all files from the root of the project to the `public/` directory. The `.public` workaround is so `cp` doesn't also copy `public/` to itself in an infinite loop: -``` +```yaml pages: stage: deploy script: - - mkdir .public - - cp -r * .public - - mv .public public + - mkdir .public + - cp -r * .public + - mv .public public artifacts: paths: - - public + - public only: - - master + - master ``` Read more on [GitLab Pages user documentation](../../user/project/pages/index.md). @@ -131,15 +131,15 @@ if you set it per-job: ```yaml before_script: -- global before script + - global before script job: before_script: - - execute this instead of global before script + - execute this instead of global before script script: - - my command + - my command after_script: - - execute this after my script + - execute this after my script ``` ## `stages` @@ -409,18 +409,18 @@ fails, it will not stop the next stage from running, since it's marked with job1: stage: test script: - - execute_script_that_will_fail + - execute_script_that_will_fail allow_failure: true job2: stage: test script: - - execute_script_that_will_succeed + - execute_script_that_will_succeed job3: stage: deploy script: - - deploy_to_staging + - deploy_to_staging ``` ## `when` @@ -442,38 +442,38 @@ For example: ```yaml stages: -- build -- cleanup_build -- test -- deploy -- cleanup + - build + - cleanup_build + - test + - deploy + - cleanup build_job: stage: build script: - - make build + - make build cleanup_build_job: stage: cleanup_build script: - - cleanup build when failed + - cleanup build when failed when: on_failure test_job: stage: test script: - - make test + - make test deploy_job: stage: deploy script: - - make deploy + - make deploy when: manual cleanup_job: stage: cleanup script: - - cleanup after jobs + - cleanup after jobs when: always ``` @@ -734,8 +734,8 @@ rspec: script: test cache: paths: - - binaries/*.apk - - .config + - binaries/*.apk + - .config ``` Locally defined cache overrides globally defined options. The following `rspec` @@ -744,14 +744,14 @@ job will cache only `binaries/`: ```yaml cache: paths: - - my/files + - my/files rspec: script: test cache: key: rspec paths: - - binaries/ + - binaries/ ``` Note that since cache is shared between jobs, if you're using different @@ -786,7 +786,7 @@ For example, to enable per-branch caching: cache: key: "$CI_COMMIT_REF_SLUG" paths: - - binaries/ + - binaries/ ``` If you use **Windows Batch** to run your shell scripts you need to replace @@ -796,7 +796,7 @@ If you use **Windows Batch** to run your shell scripts you need to replace cache: key: "%CI_COMMIT_REF_SLUG%" paths: - - binaries/ + - binaries/ ``` ### `cache:untracked` @@ -819,7 +819,7 @@ rspec: cache: untracked: true paths: - - binaries/ + - binaries/ ``` ### `cache:policy` @@ -897,8 +897,8 @@ Send all files in `binaries` and `.config`: ```yaml artifacts: paths: - - binaries/ - - .config + - binaries/ + - .config ``` To disable artifact passing, define the job with empty [dependencies](#dependencies): @@ -927,7 +927,7 @@ release-job: - mvn package -U artifacts: paths: - - target/*.war + - target/*.war only: - tags ``` @@ -949,7 +949,7 @@ job: artifacts: name: "$CI_JOB_NAME" paths: - - binaries/ + - binaries/ ``` To create an archive with a name of the current branch or tag including only @@ -960,7 +960,7 @@ job: artifacts: name: "$CI_COMMIT_REF_NAME" paths: - - binaries/ + - binaries/ ``` To create an archive with a name of the current job and the current branch or @@ -971,7 +971,7 @@ job: artifacts: name: "$CI_JOB_NAME-$CI_COMMIT_REF_NAME" paths: - - binaries/ + - binaries/ ``` To create an archive with a name of the current [stage](#stages) and branch name: @@ -981,7 +981,7 @@ job: artifacts: name: "$CI_JOB_STAGE-$CI_COMMIT_REF_NAME" paths: - - binaries/ + - binaries/ ``` --- @@ -994,7 +994,7 @@ job: artifacts: name: "%CI_JOB_STAGE%-%CI_COMMIT_REF_NAME%" paths: - - binaries/ + - binaries/ ``` If you use **Windows PowerShell** to run your shell scripts you need to replace @@ -1005,7 +1005,7 @@ job: artifacts: name: "$env:CI_JOB_STAGE-$env:CI_COMMIT_REF_NAME" paths: - - binaries/ + - binaries/ ``` ### `artifacts:untracked` @@ -1030,7 +1030,7 @@ Send all Git untracked files and files in `binaries`: artifacts: untracked: true paths: - - binaries/ + - binaries/ ``` ### `artifacts:when` @@ -1120,26 +1120,26 @@ build:osx: script: make build:osx artifacts: paths: - - binaries/ + - binaries/ build:linux: stage: build script: make build:linux artifacts: paths: - - binaries/ + - binaries/ test:osx: stage: test script: make test:osx dependencies: - - build:osx + - build:osx test:linux: stage: test script: make test:linux dependencies: - - build:linux + - build:linux deploy: stage: deploy @@ -1406,6 +1406,43 @@ variables: You can set it globally or per-job in the [`variables`](#variables) section. +### Custom build directories + +> [Introduced][gitlab-runner-876] in Gitlab Runner 11.1 + +NOTE: **Note:** +This can only be used when `custom_build_dir` is set to true in the [Runner's +configuration](https://docs.gitlab.com/runner/configuration/advanced-configuration.html). + +By default, GitLab Runner clones the repository in the `/builds` directory, +but sometimes your project might require to have the code in a specific +directory, like the GO projects for example. In that case, you can specify +the `CI_PROJECT_DIR` variable to tell the Runner in which directory to clone +the repository: + +```yml +image: golang:1.10-alpine3.7 + +variables: + CI_PROJECT_DIR: /go/src/gitlab.com/namespace/project-name + +stages: + - test + +dir: + stage: test + script: + - pwd # /go/src/gitlab.com/namespace/project-name +``` + +The following executors may use this feature only when +[concurrent](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-global-section) +is set to `1`: + +- `shell` +- `ssh` +- `docker`, `docker+machine` when the job's working directory is mounted as a host volume. + ## Special YAML features It's possible to use special YAML features like anchors (`&`), aliases (`*`) @@ -1599,5 +1636,6 @@ CI with various languages. [ce-7983]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7983 [ce-7447]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7447 [ce-12909]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12909 +[gitlab-runner-876]: https://gitlab.com/gitlab-org/gitlab-runner/merge_requests/876 [schedules]: ../../user/project/pipelines/schedules.md [variables-expressions]: ../variables/README.md#variables-expressions diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index 3edde3de83d..82e8fbdb93e 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -168,7 +168,7 @@ want their accounts to be upgraded to full internal accounts. >**Note:** The following information only applies for installations from source. -GitLab uses [Omniauth](http://www.omniauth.org/) for authentication and already ships +GitLab uses [Omniauth](https://github.com/omniauth/omniauth) for authentication and already ships with a few providers pre-installed (e.g. LDAP, GitHub, Twitter). But sometimes that is not enough and you need to integrate with other authentication solutions. For these cases you can use the Omniauth provider. diff --git a/doc/topics/git/index.md b/doc/topics/git/index.md index 2ca2bf743fb..7707d56764e 100644 --- a/doc/topics/git/index.md +++ b/doc/topics/git/index.md @@ -17,7 +17,7 @@ We've gathered some resources to help you to get the best from Git with GitLab. - [How to install Git](how_to_install_git/index.md) - [Start using Git on the command line](../../gitlab-basics/start-using-git.md) - [Command Line basic commands](../../gitlab-basics/command-line-commands.md) -- [GitLab Git Cheat Sheet (download)](https://gitlab.com/gitlab-com/marketing/raw/master/design/print/git-cheatsheet/print-pdf/git-cheatsheet.pdf) +- [GitLab Git Cheat Sheet (download)](https://about.gitlab.com/images/press/git-cheat-sheet.pdf) - Commits - [Revert a commit](../../user/project/merge_requests/revert_changes.md#reverting-a-commit) - [Cherry-picking a commit](../../user/project/merge_requests/cherry_pick_changes.md#cherry-picking-a-commit) diff --git a/doc/university/high-availability/aws/README.md b/doc/university/high-availability/aws/README.md index f340164b882..dc045961ed7 100644 --- a/doc/university/high-availability/aws/README.md +++ b/doc/university/high-availability/aws/README.md @@ -2,6 +2,10 @@ comments: false --- +DANGER: This guide exists for reference of how an AWS deployment could work. +We are currently seeing very slow EFS access performance which causes GitLab to +be 5-10x slower than using NFS or Local disk. We _do not_ recommend follow this +guide at this time. # High Availability on AWS diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 48bb2e543c1..58a483bb3b2 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -156,7 +156,7 @@ added directly to your configured cluster. Those applications are needed for | [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/) | 10.2+ | Ingress can provide load balancing, SSL termination, and name-based virtual hosting. It acts as a web proxy for your applications and is useful if you want to use [Auto DevOps] or deploy your own web apps. | | [Prometheus](https://prometheus.io/docs/introduction/overview/) | 10.4+ | Prometheus is an open-source monitoring and alerting system useful to supervise your deployed applications | | [GitLab Runner](https://docs.gitlab.com/runner/) | 10.6+ | GitLab Runner is the open source project that is used to run your jobs and send the results back to GitLab. It is used in conjunction with [GitLab CI/CD](https://about.gitlab.com/features/gitlab-ci-cd/), the open-source continuous integration service included with GitLab that coordinates the jobs. When installing the GitLab Runner via the applications, it will run in **privileged mode** by default. Make sure you read the [security implications](#security-implications) before doing so. | -| [JupyterHub](http://jupyter.org/) | 11.0+ | The Jupyter Notebook is an open-source web application that allows you to create and share documents that contain live code, equations, visualizations and narrative text. | +| [JupyterHub](http://jupyter.org/) | 11.0+ | [JupyterHub](https://jupyterhub.readthedocs.io/en/stable/) is a multi-user service for managing notebooks across a team. [Jupyter Notebooks](https://jupyter-notebook.readthedocs.io/en/latest/) provide a web-based interactive programming environment used for data analysis, visualization, and machine learning. **Note**: Authentication will be enabled for any user of the GitLab server via OAuth2. HTTPS will be supported in a future release. | ## Getting the external IP address diff --git a/doc/user/project/repository/index.md b/doc/user/project/repository/index.md index 376f4e3cbe4..bda293bd00e 100644 --- a/doc/user/project/repository/index.md +++ b/doc/user/project/repository/index.md @@ -176,4 +176,12 @@ Lock your files to prevent any conflicting changes. You can access your repos via [repository API](../../../api/repositories.md). +## Clone in Apple Xcode + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/45820) in GitLab 11.0 + +Projects that contain a `.xcodeproj` or `.xcworkspace` directory can now be cloned +in Xcode using the new **Open in Xcode** button, located next to the Git URL +used for cloning your project. The button is only shown on macOS. + [jupyter]: https://jupyter.org diff --git a/doc/user/reserved_names.md b/doc/user/reserved_names.md index 6c1378560ef..918daee5d9f 100644 --- a/doc/user/reserved_names.md +++ b/doc/user/reserved_names.md @@ -58,6 +58,7 @@ Currently the following names are reserved as top level groups: - dashboard - deploy.html - explore +- favicon.ico - favicon.png - groups - header_logo_dark.png diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 1cc8fcb8408..bb48a86fe9e 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -349,6 +349,10 @@ module API expose :developers_can_merge do |repo_branch, options| options[:project].protected_branches.developers_can?(:merge, repo_branch.name) end + + expose :can_push do |repo_branch, options| + Gitlab::UserAccess.new(options[:current_user], project: options[:project]).can_push_to_branch?(repo_branch.name) + end end class TreeObject < Grape::Entity diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 03b6b30a0d8..c7f41aba854 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -32,7 +32,7 @@ module API optional :all_available, type: Boolean, desc: 'Show all group that you have access to' optional :search, type: String, desc: 'Search for a specific group' optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user' - optional :order_by, type: String, values: %w[name path], default: 'name', desc: 'Order by name or path' + optional :order_by, type: String, values: %w[name path id], default: 'name', desc: 'Order by name, path or id' optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)' use :pagination end @@ -46,7 +46,9 @@ module API groups = GroupsFinder.new(current_user, find_params).execute groups = groups.search(params[:search]) if params[:search].present? groups = groups.where.not(id: params[:skip_groups]) if params[:skip_groups].present? - groups = groups.reorder(params[:order_by] => params[:sort]) + order_options = { params[:order_by] => params[:sort] } + order_options["id"] ||= "asc" + groups = groups.reorder(order_options) groups end diff --git a/lib/api/runner.rb b/lib/api/runner.rb index dc102259ca8..96a02914faa 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -84,7 +84,11 @@ module API end post '/request' do authenticate_runner! - no_content! unless current_runner.active? + + unless current_runner.active? + header 'X-GitLab-Last-Update', current_runner.ensure_runner_queue_value + break no_content! + end if current_runner.runner_queue_value_latest?(params[:last_update]) header 'X-GitLab-Last-Update', params[:last_update] diff --git a/lib/banzai/filter/blockquote_fence_filter.rb b/lib/banzai/filter/blockquote_fence_filter.rb index d2c4b1e4d76..fbfcd72c916 100644 --- a/lib/banzai/filter/blockquote_fence_filter.rb +++ b/lib/banzai/filter/blockquote_fence_filter.rb @@ -10,7 +10,7 @@ module Banzai ^``` .+? - \n```$ + \n```\ *$ ) | (?<html> @@ -19,9 +19,9 @@ module Banzai # Anything, including `>>>` blocks which are ignored by this filter # </tag> - ^<[^>]+?>\n + ^<[^>]+?>\ *\n .+? - \n<\/[^>]+?>$ + \n<\/[^>]+?>\ *$ ) | (?: @@ -30,14 +30,14 @@ module Banzai # Anything, including code and HTML blocks # >>> - ^>>>\n + ^>>>\ *\n (?<quote> (?: # Any character that doesn't introduce a code or HTML block (?! ^``` | - ^<[^>]+?>\n + ^<[^>]+?>\ *\n ) . | @@ -48,7 +48,7 @@ module Banzai \g<html> )+? ) - \n>>>$ + \n>>>\ *$ ) }mx.freeze diff --git a/lib/gitlab/checks/commit_check.rb b/lib/gitlab/checks/commit_check.rb index 43a52b493bb..22310e313ac 100644 --- a/lib/gitlab/checks/commit_check.rb +++ b/lib/gitlab/checks/commit_check.rb @@ -37,7 +37,7 @@ module Gitlab def validate_lfs_file_locks? strong_memoize(:validate_lfs_file_locks) do - project.lfs_enabled? && project.lfs_file_locks.any? && newrev && oldrev + project.lfs_enabled? && newrev && oldrev && project.any_lfs_file_locks? end end diff --git a/lib/gitlab/checks/force_push.rb b/lib/gitlab/checks/force_push.rb index c9c3050cfc2..87af4a90572 100644 --- a/lib/gitlab/checks/force_push.rb +++ b/lib/gitlab/checks/force_push.rb @@ -7,18 +7,10 @@ module Gitlab # Created or deleted branch return false if Gitlab::Git.blank_ref?(oldrev) || Gitlab::Git.blank_ref?(newrev) - GitalyClient.migrate(:force_push) do |is_enabled| - if is_enabled - !project - .repository - .gitaly_commit_client - .ancestor?(oldrev, newrev) - else - Gitlab::Git::RevList.new( - project.repository.raw, oldrev: oldrev, newrev: newrev - ).missed_ref.present? - end - end + !project + .repository + .gitaly_commit_client + .ancestor?(oldrev, newrev) end end end diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb index 62d4d0a92a6..26ae6966746 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/migration_classes.rb @@ -37,6 +37,7 @@ module Gitlab class Namespace < ActiveRecord::Base include MigrationClasses::Routable self.table_name = 'namespaces' + self.inheritance_column = :_type_disabled belongs_to :parent, class_name: "#{MigrationClasses.name}::Namespace" has_one :route, as: :source diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index eb5d6318dcb..7ce90ce170f 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -472,13 +472,21 @@ module Gitlab end def count_commits(options) - count_commits_options = process_count_commits_options(options) + options = process_count_commits_options(options.dup) - gitaly_migrate(:count_commits, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - count_commits_by_gitaly(count_commits_options) + wrapped_gitaly_errors do + if options[:left_right] + from = options[:from] + to = options[:to] + + right_count = gitaly_commit_client + .commit_count("#{from}..#{to}", options) + left_count = gitaly_commit_client + .commit_count("#{to}..#{from}", options) + + [left_count, right_count] else - count_commits_by_shelling_out(count_commits_options) + gitaly_commit_client.commit_count(options[:ref], options) end end end @@ -676,15 +684,9 @@ module Gitlab end # Return total commits count accessible from passed ref - # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/330 def commit_count(ref) - gitaly_migrate(:commit_count, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_commit_client.commit_count(ref) - else - rugged_commit_count(ref) - end + wrapped_gitaly_errors do + gitaly_commit_client.commit_count(ref) end end @@ -984,21 +986,7 @@ module Gitlab def info_attributes return @info_attributes if @info_attributes - content = - gitaly_migrate(:get_info_attributes, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_repository_client.info_attributes - else - attributes_path = File.join(File.expand_path(path), 'info', 'attributes') - - if File.exist?(attributes_path) - File.read(attributes_path) - else - "" - end - end - end - + content = gitaly_repository_client.info_attributes @info_attributes = AttributesParser.new(content) end @@ -1054,18 +1042,8 @@ module Gitlab end def license_short_name - gitaly_migrate(:license_short_name, - status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_repository_client.license_short_name - else - begin - # The licensee gem creates a Rugged object from the path: - # https://github.com/benbalter/licensee/blob/v8.7.0/lib/licensee/projects/git_project.rb - Licensee.license(path).try(:key) - rescue Rugged::Error - end - end + wrapped_gitaly_errors do + gitaly_repository_client.license_short_name end end @@ -1252,12 +1230,8 @@ module Gitlab end def rebase_in_progress?(rebase_id) - gitaly_migrate(:rebase_in_progress) do |is_enabled| - if is_enabled - gitaly_repository_client.rebase_in_progress?(rebase_id) - else - fresh_worktree?(worktree_path(REBASE_WORKTREE_PREFIX, rebase_id)) - end + wrapped_gitaly_errors do + gitaly_repository_client.rebase_in_progress?(rebase_id) end end @@ -1273,12 +1247,8 @@ module Gitlab end def squash_in_progress?(squash_id) - gitaly_migrate(:squash_in_progress, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_repository_client.squash_in_progress?(squash_id) - else - fresh_worktree?(worktree_path(SQUASH_WORKTREE_PREFIX, squash_id)) - end + wrapped_gitaly_errors do + gitaly_repository_client.squash_in_progress?(squash_id) end end @@ -1514,10 +1484,6 @@ module Gitlab run_git!(args, lazy_block: block) end - def missed_ref(oldrev, newrev) - run_git!(['rev-list', '--max-count=1', oldrev, "^#{newrev}"]) - end - def with_worktree(worktree_path, branch, sparse_checkout_files: nil, env:) base_args = %w(worktree add --detach) @@ -1621,21 +1587,6 @@ module Gitlab end end - # This function is duplicated in Gitaly-Go, don't change it! - # https://gitlab.com/gitlab-org/gitaly/merge_requests/698 - def fresh_worktree?(path) - File.exist?(path) && !clean_stuck_worktree(path) - end - - # This function is duplicated in Gitaly-Go, don't change it! - # https://gitlab.com/gitlab-org/gitaly/merge_requests/698 - 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. @@ -1902,71 +1853,6 @@ module Gitlab gitaly_repository_client.repository_size end - def count_commits_by_gitaly(options) - if options[:left_right] - from = options[:from] - to = options[:to] - - right_count = gitaly_commit_client - .commit_count("#{from}..#{to}", options) - left_count = gitaly_commit_client - .commit_count("#{to}..#{from}", options) - - [left_count, right_count] - else - gitaly_commit_client.commit_count(options[:ref], options) - end - end - - def count_commits_by_shelling_out(options) - cmd = count_commits_shelling_command(options) - - raw_output, _status = run_git(cmd) - - process_count_commits_raw_output(raw_output, options) - end - - def count_commits_shelling_command(options) - cmd = %w[rev-list] - cmd << "--after=#{options[:after].iso8601}" if options[:after] - cmd << "--before=#{options[:before].iso8601}" if options[:before] - cmd << "--max-count=#{options[:max_count]}" if options[:max_count] - cmd << "--left-right" if options[:left_right] - cmd << '--count' - - cmd << if options[:all] - '--all' - elsif options[:ref] - options[:ref] - else - raise ArgumentError, "Please specify a valid ref or set the 'all' attribute to true" - end - - cmd += %W[-- #{options[:path]}] if options[:path].present? - cmd - end - - def process_count_commits_raw_output(raw_output, options) - if options[:left_right] - result = raw_output.scan(/\d+/).map(&:to_i) - - if result.sum != options[:max_count] - result - else # Reaching max count, right is not accurate - right_option = - process_count_commits_options(options - .except(:left_right, :from, :to) - .merge(ref: options[:to])) - - right = count_commits_by_shelling_out(right_option) - - [result.first, right] # left should be accurate in the first call - end - else - raw_output.to_i - end - end - def gitaly_ls_files(ref) gitaly_commit_client.ls_files(ref) end @@ -2393,16 +2279,6 @@ module Gitlab nil end - def rugged_commit_count(ref) - walker = Rugged::Walker.new(rugged) - walker.sorting(Rugged::SORT_TOPO | Rugged::SORT_REVERSE) - oid = rugged.rev_parse_oid(ref) - walker.push(oid) - walker.count - rescue Rugged::ReferenceError - 0 - end - def rev_list_param(spec) spec == :all ? ['--all'] : spec end diff --git a/lib/gitlab/git/rev_list.rb b/lib/gitlab/git/rev_list.rb index 4e661eceffb..5fdad077eea 100644 --- a/lib/gitlab/git/rev_list.rb +++ b/lib/gitlab/git/rev_list.rb @@ -1,5 +1,3 @@ -# Gitaly note: JV: will probably be migrated indirectly by migrating the call sites. - module Gitlab module Git class RevList @@ -45,13 +43,6 @@ module Gitlab &lazy_block) end - # This methods returns an array of missed references - # - # Should become obsolete after https://gitlab.com/gitlab-org/gitaly/issues/348. - def missed_ref - repository.missed_ref(oldrev, newrev).split("\n") - end - private def execute(args) diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index 1ab8c4e0229..8ee46b59830 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -27,63 +27,38 @@ module Gitlab end def write_page(name, format, content, commit_details) - @repository.gitaly_migrate(:wiki_write_page) do |is_enabled| - if is_enabled - gitaly_write_page(name, format, content, commit_details) - else - gollum_write_page(name, format, content, commit_details) - end + @repository.wrapped_gitaly_errors do + gitaly_write_page(name, format, content, commit_details) end end def delete_page(page_path, commit_details) - @repository.gitaly_migrate(:wiki_delete_page) do |is_enabled| - if is_enabled - gitaly_delete_page(page_path, commit_details) - else - gollum_delete_page(page_path, commit_details) - end + @repository.wrapped_gitaly_errors do + gitaly_delete_page(page_path, commit_details) end end def update_page(page_path, title, format, content, commit_details) - @repository.gitaly_migrate(:wiki_update_page) do |is_enabled| - if is_enabled - gitaly_update_page(page_path, title, format, content, commit_details) - else - gollum_update_page(page_path, title, format, content, commit_details) - end + @repository.wrapped_gitaly_errors do + gitaly_update_page(page_path, title, format, content, commit_details) end end def pages(limit: nil) - @repository.gitaly_migrate(:wiki_get_all_pages) do |is_enabled| - if is_enabled - gitaly_get_all_pages - else - gollum_get_all_pages(limit: limit) - end + @repository.wrapped_gitaly_errors do + gitaly_get_all_pages end end def page(title:, version: nil, dir: nil) - @repository.gitaly_migrate(:wiki_find_page, - status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_find_page(title: title, version: version, dir: dir) - else - gollum_find_page(title: title, version: version, dir: dir) - end + @repository.wrapped_gitaly_errors do + gitaly_find_page(title: title, version: version, dir: dir) end end def file(name, version) - @repository.gitaly_migrate(:wiki_find_file) do |is_enabled| - if is_enabled - gitaly_find_file(name, version) - else - gollum_find_file(name, version) - end + @repository.wrapped_gitaly_errors do + gitaly_find_file(name, version) end end @@ -92,24 +67,15 @@ module Gitlab # :per_page - The number of items per page. # :limit - Total number of items to return. def page_versions(page_path, options = {}) - @repository.gitaly_migrate(:wiki_page_versions) do |is_enabled| - if is_enabled - versions = gitaly_wiki_client.page_versions(page_path, options) - - # Gitaly uses gollum-lib to get the versions. Gollum defaults to 20 - # per page, but also fetches 20 if `limit` or `per_page` < 20. - # Slicing returns an array with the expected number of items. - slice_bound = options[:limit] || options[:per_page] || Gollum::Page.per_page - versions[0..slice_bound] - else - current_page = gollum_page_by_path(page_path) - - commits_from_page(current_page, options).map do |gitlab_git_commit| - gollum_page = gollum_wiki.page(current_page.title, gitlab_git_commit.id) - Gitlab::Git::WikiPageVersion.new(gitlab_git_commit, gollum_page&.format) - end - end + versions = @repository.wrapped_gitaly_errors do + gitaly_wiki_client.page_versions(page_path, options) end + + # Gitaly uses gollum-lib to get the versions. Gollum defaults to 20 + # per page, but also fetches 20 if `limit` or `per_page` < 20. + # Slicing returns an array with the expected number of items. + slice_bound = options[:limit] || options[:per_page] || Gollum::Page.per_page + versions[0..slice_bound] end def count_page_versions(page_path) @@ -131,46 +97,13 @@ module Gitlab def page_formatted_data(title:, dir: nil, version: nil) version = version&.id - @repository.gitaly_migrate(:wiki_page_formatted_data, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_wiki_client.get_formatted_data(title: title, dir: dir, version: version) - else - # We don't use #page because if wiki_find_page feature is enabled, we would - # get a page without formatted_data. - gollum_find_page(title: title, dir: dir, version: version)&.formatted_data - end + @repository.wrapped_gitaly_errors do + gitaly_wiki_client.get_formatted_data(title: title, dir: dir, version: version) end end - def gollum_wiki - @gollum_wiki ||= Gollum::Wiki.new(@repository.path) - end - private - # options: - # :page - The Integer page number. - # :per_page - The number of items per page. - # :limit - Total number of items to return. - def commits_from_page(gollum_page, options = {}) - unless options[:limit] - options[:offset] = ([1, options.delete(:page).to_i].max - 1) * Gollum::Page.per_page - options[:limit] = (options.delete(:per_page) || Gollum::Page.per_page).to_i - end - - @repository.log(ref: gollum_page.last_version.id, - path: gollum_page.path, - limit: options[:limit], - offset: options[:offset]) - end - - def gollum_page_by_path(page_path) - page_name = Gollum::Page.canonicalize_filename(page_path) - page_dir = File.split(page_path).first - - gollum_wiki.paged(page_name, page_dir) - end - def new_page(gollum_page) Gitlab::Git::WikiPage.new(gollum_page, new_version(gollum_page, gollum_page.version.id)) end @@ -199,65 +132,6 @@ module Gitlab @gitaly_wiki_client ||= Gitlab::GitalyClient::WikiService.new(@repository) end - def gollum_write_page(name, format, content, commit_details) - assert_type!(format, Symbol) - assert_type!(commit_details, CommitDetails) - - with_committer_with_hooks(commit_details) do |committer| - filename = File.basename(name) - dir = (tmp_dir = File.dirname(name)) == '.' ? '' : tmp_dir - - gollum_wiki.write_page(filename, format, content, { committer: committer }, dir) - end - rescue Gollum::DuplicatePageError => e - raise Gitlab::Git::Wiki::DuplicatePageError, e.message - end - - def gollum_delete_page(page_path, commit_details) - assert_type!(commit_details, CommitDetails) - - with_committer_with_hooks(commit_details) do |committer| - gollum_wiki.delete_page(gollum_page_by_path(page_path), committer: committer) - end - end - - def gollum_update_page(page_path, title, format, content, commit_details) - assert_type!(format, Symbol) - assert_type!(commit_details, CommitDetails) - - with_committer_with_hooks(commit_details) do |committer| - page = gollum_page_by_path(page_path) - # Instead of performing two renames if the title has changed, - # the update_page will only update the format and content and - # the rename_page will do anything related to moving/renaming - gollum_wiki.update_page(page, page.name, format, content, committer: committer) - gollum_wiki.rename_page(page, title, committer: committer) - end - end - - def gollum_find_page(title:, version: nil, dir: nil) - if version - version = Gitlab::Git::Commit.find(@repository, version).id - end - - gollum_page = gollum_wiki.page(title, version, dir) - return unless gollum_page - - new_page(gollum_page) - end - - def gollum_find_file(name, version) - version ||= self.class.default_ref - gollum_file = gollum_wiki.file(name, version) - return unless gollum_file - - Gitlab::Git::WikiFile.new(gollum_file) - end - - def gollum_get_all_pages(limit: nil) - gollum_wiki.pages(limit: limit).map { |gollum_page| new_page(gollum_page) } - end - def gitaly_write_page(name, format, content, commit_details) gitaly_wiki_client.write_page(name, format, content, commit_details) end diff --git a/lib/gitlab/health_checks/db_check.rb b/lib/gitlab/health_checks/db_check.rb index e27e16ddaf6..08495c0a59e 100644 --- a/lib/gitlab/health_checks/db_check.rb +++ b/lib/gitlab/health_checks/db_check.rb @@ -17,7 +17,7 @@ module Gitlab def check catch_timeout 10.seconds do if Gitlab::Database.postgresql? - ActiveRecord::Base.connection.execute('SELECT 1 as ping')&.first&.[]('ping') + ActiveRecord::Base.connection.execute('SELECT 1 as ping')&.first&.[]('ping')&.to_s else ActiveRecord::Base.connection.execute('SELECT 1 as ping')&.first&.first&.to_s end diff --git a/lib/gitlab/i18n.rb b/lib/gitlab/i18n.rb index 3772ef11c7f..343487bc361 100644 --- a/lib/gitlab/i18n.rb +++ b/lib/gitlab/i18n.rb @@ -21,7 +21,8 @@ module Gitlab 'nl_NL' => 'Nederlands', 'tr_TR' => 'Türkçe', 'id_ID' => 'Bahasa Indonesia', - 'fil_PH' => 'Filipino' + 'fil_PH' => 'Filipino', + 'pl_PL' => 'Polski' }.freeze def available_locales diff --git a/lib/gitlab/i18n/metadata_entry.rb b/lib/gitlab/i18n/metadata_entry.rb index 35d57459a3d..36fc1bcdcb7 100644 --- a/lib/gitlab/i18n/metadata_entry.rb +++ b/lib/gitlab/i18n/metadata_entry.rb @@ -3,16 +3,25 @@ module Gitlab class MetadataEntry attr_reader :entry_data + # Avoid testing too many plurals if `nplurals` was incorrectly set. + # Based on info on https://www.gnu.org/software/gettext/manual/html_node/Plural-forms.html + # which mentions special cases for numbers ending in 2 digits + MAX_FORMS_TO_TEST = 101 + def initialize(entry_data) @entry_data = entry_data end - def expected_plurals + def expected_forms return nil unless plural_information plural_information['nplurals'].to_i end + def forms_to_test + @forms_to_test ||= [expected_forms, MAX_FORMS_TO_TEST].compact.min + end + private def plural_information diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb index 7d3ff8c7f58..d8e7269a2c2 100644 --- a/lib/gitlab/i18n/po_linter.rb +++ b/lib/gitlab/i18n/po_linter.rb @@ -1,6 +1,8 @@ module Gitlab module I18n class PoLinter + include Gitlab::Utils::StrongMemoize + attr_reader :po_path, :translation_entries, :metadata_entry, :locale VARIABLE_REGEX = /%{\w*}|%[a-z]/.freeze @@ -34,7 +36,7 @@ module Gitlab end @translation_entries = entries.map do |entry_data| - Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_plurals) + Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_forms) end nil @@ -48,7 +50,7 @@ module Gitlab translation_entries.each do |entry| errors_for_entry = validate_entry(entry) - errors[join_message(entry.msgid)] = errors_for_entry if errors_for_entry.any? + errors[entry.msgid] = errors_for_entry if errors_for_entry.any? end errors @@ -62,6 +64,7 @@ module Gitlab validate_newlines(errors, entry) validate_number_of_plurals(errors, entry) validate_unescaped_chars(errors, entry) + validate_translation(errors, entry) errors end @@ -81,35 +84,39 @@ module Gitlab end def validate_number_of_plurals(errors, entry) - return unless metadata_entry&.expected_plurals + return unless metadata_entry&.expected_forms return unless entry.translated? - if entry.has_plural? && entry.all_translations.size != metadata_entry.expected_plurals - errors << "should have #{metadata_entry.expected_plurals} "\ - "#{'translations'.pluralize(metadata_entry.expected_plurals)}" + if entry.has_plural? && entry.all_translations.size != metadata_entry.expected_forms + errors << "should have #{metadata_entry.expected_forms} "\ + "#{'translations'.pluralize(metadata_entry.expected_forms)}" end end def validate_newlines(errors, entry) - if entry.msgid_contains_newlines? + if entry.msgid_has_multiple_lines? errors << 'is defined over multiple lines, this breaks some tooling.' end - if entry.plural_id_contains_newlines? + if entry.plural_id_has_multiple_lines? errors << 'plural is defined over multiple lines, this breaks some tooling.' end - if entry.translations_contain_newlines? + if entry.translations_have_multiple_lines? errors << 'has translations defined over multiple lines, this breaks some tooling.' end end def validate_variables(errors, entry) if entry.has_singular_translation? + validate_variables_in_message(errors, entry.msgid, entry.msgid) + validate_variables_in_message(errors, entry.msgid, entry.singular_translation) end if entry.has_plural? + validate_variables_in_message(errors, entry.plural_id, entry.plural_id) + entry.plural_translations.each do |translation| validate_variables_in_message(errors, entry.plural_id, translation) end @@ -117,41 +124,98 @@ module Gitlab end def validate_variables_in_message(errors, message_id, message_translation) - message_id = join_message(message_id) required_variables = message_id.scan(VARIABLE_REGEX) validate_unnamed_variables(errors, required_variables) - validate_translation(errors, message_id, required_variables) validate_variable_usage(errors, message_translation, required_variables) end - def validate_translation(errors, message_id, used_variables) + def validate_translation(errors, entry) + Gitlab::I18n.with_locale(locale) do + if entry.has_plural? + translate_plural(entry) + else + translate_singular(entry) + end + end + + # `sprintf` could raise an `ArgumentError` when invalid passing something + # other than a Hash when using named variables + # + # `sprintf` could raise `TypeError` when passing a wrong type when using + # unnamed variables + # + # FastGettext::Translation could raise `RuntimeError` (raised as a string), + # or as subclassess `NoTextDomainConfigured` & `InvalidFormat` + # + # `FastGettext::Translation` could raise `ArgumentError` as subclassess + # `InvalidEncoding`, `IllegalSequence` & `InvalidCharacter` + rescue ArgumentError, TypeError, RuntimeError => e + errors << "Failure translating to #{locale}: #{e.message}" + end + + def translate_singular(entry) + used_variables = entry.msgid.scan(VARIABLE_REGEX) variables = fill_in_variables(used_variables) - begin - Gitlab::I18n.with_locale(locale) do - translated = if message_id.include?('|') - FastGettext::Translation.s_(message_id) - else - FastGettext::Translation._(message_id) - end + translation = if entry.msgid.include?('|') + FastGettext::Translation.s_(entry.msgid) + else + FastGettext::Translation._(entry.msgid) + end - translated % variables + translation % variables if used_variables.any? + end + + def translate_plural(entry) + used_variables = entry.plural_id.scan(VARIABLE_REGEX) + variables = fill_in_variables(used_variables) + + numbers_covering_all_plurals.map do |number| + translation = FastGettext::Translation.n_(entry.msgid, entry.plural_id, number) + + translation % variables if used_variables.any? + end + end + + def numbers_covering_all_plurals + @numbers_covering_all_plurals ||= calculate_numbers_covering_all_plurals + end + + def calculate_numbers_covering_all_plurals + required_numbers = [] + discovered_indexes = [] + counter = 0 + + while discovered_indexes.size < metadata_entry.forms_to_test && counter < Gitlab::I18n::MetadataEntry::MAX_FORMS_TO_TEST + index_for_count = index_for_pluralization(counter) + + unless discovered_indexes.include?(index_for_count) + discovered_indexes << index_for_count + required_numbers << counter end - # `sprintf` could raise an `ArgumentError` when invalid passing something - # other than a Hash when using named variables - # - # `sprintf` could raise `TypeError` when passing a wrong type when using - # unnamed variables - # - # FastGettext::Translation could raise `RuntimeError` (raised as a string), - # or as subclassess `NoTextDomainConfigured` & `InvalidFormat` - # - # `FastGettext::Translation` could raise `ArgumentError` as subclassess - # `InvalidEncoding`, `IllegalSequence` & `InvalidCharacter` - rescue ArgumentError, TypeError, RuntimeError => e - errors << "Failure translating to #{locale} with #{variables}: #{e.message}" + counter += 1 + end + + required_numbers + end + + def index_for_pluralization(counter) + # This calls the C function that defines the pluralization rule, it can + # return a boolean (`false` represents 0, `true` represents 1) or an integer + # that specifies the plural form to be used for the given number + pluralization_result = Gitlab::I18n.with_locale(locale) do + FastGettext.pluralisation_rule.call(counter) + end + + case pluralization_result + when false + 0 + when true + 1 + else + pluralization_result end end @@ -172,14 +236,18 @@ module Gitlab end def validate_unnamed_variables(errors, variables) - if variables.size > 1 && variables.any? { |variable_name| unnamed_variable?(variable_name) } + unnamed_variables, named_variables = variables.partition { |name| unnamed_variable?(name) } + + if unnamed_variables.any? && named_variables.any? + errors << 'is combining named variables with unnamed variables' + end + + if unnamed_variables.size > 1 errors << 'is combining multiple unnamed variables' end end def validate_variable_usage(errors, translation, required_variables) - translation = join_message(translation) - # We don't need to validate when the message is empty. # In this case we fall back to the default, which has all the the # required variables. @@ -205,10 +273,6 @@ module Gitlab def validate_flags(errors, entry) errors << "is marked #{entry.flag}" if entry.flag end - - def join_message(message) - Array(message).join - end end end end diff --git a/lib/gitlab/i18n/translation_entry.rb b/lib/gitlab/i18n/translation_entry.rb index e6c95afca7e..54adb98f42d 100644 --- a/lib/gitlab/i18n/translation_entry.rb +++ b/lib/gitlab/i18n/translation_entry.rb @@ -11,11 +11,11 @@ module Gitlab end def msgid - entry_data[:msgid] + @msgid ||= Array(entry_data[:msgid]).join end def plural_id - entry_data[:msgid_plural] + @plural_id ||= Array(entry_data[:msgid_plural]).join end def has_plural? @@ -23,12 +23,11 @@ module Gitlab end def singular_translation - all_translations.first if has_singular_translation? + all_translations.first.to_s if has_singular_translation? end def all_translations - @all_translations ||= entry_data.fetch_values(*translation_keys) - .reject(&:empty?) + @all_translations ||= translation_entries.map { |translation| Array(translation).join } end def translated? @@ -54,16 +53,16 @@ module Gitlab nplurals > 1 || !has_plural? end - def msgid_contains_newlines? - msgid.is_a?(Array) + def msgid_has_multiple_lines? + entry_data[:msgid].is_a?(Array) end - def plural_id_contains_newlines? - plural_id.is_a?(Array) + def plural_id_has_multiple_lines? + entry_data[:msgid_plural].is_a?(Array) end - def translations_contain_newlines? - all_translations.any? { |translation| translation.is_a?(Array) } + def translations_have_multiple_lines? + translation_entries.any? { |translation| translation.is_a?(Array) } end def msgid_contains_unescaped_chars? @@ -84,6 +83,11 @@ module Gitlab private + def translation_entries + @translation_entries ||= entry_data.fetch_values(*translation_keys) + .reject(&:empty?) + end + def translation_keys @translation_keys ||= entry_data.keys.select { |key| key.to_s =~ /\Amsgstr(\[\d+\])?\z/ } end diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index e5191f5c7f9..61653044433 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -30,6 +30,7 @@ module Gitlab dashboard deploy.html explore + favicon.ico favicon.png files groups diff --git a/lib/gitlab/profiler.rb b/lib/gitlab/profiler.rb index 18540e64d4c..ecff6ab5d5e 100644 --- a/lib/gitlab/profiler.rb +++ b/lib/gitlab/profiler.rb @@ -11,6 +11,7 @@ module Gitlab lib/gitlab/etag_caching/ lib/gitlab/metrics/ lib/gitlab/middleware/ + ee/lib/gitlab/middleware/ lib/gitlab/performance_bar/ lib/gitlab/request_profiler/ lib/gitlab/profiler.rb @@ -98,11 +99,7 @@ module Gitlab super - backtrace = Rails.backtrace_cleaner.clean(caller) - - backtrace.each do |caller_line| - next if caller_line.match(Regexp.union(IGNORE_BACKTRACES)) - + Gitlab::Profiler.clean_backtrace(caller).each do |caller_line| stripped_caller_line = caller_line.sub("#{Rails.root}/", '') super(" ↳ #{stripped_caller_line}") @@ -112,6 +109,12 @@ module Gitlab end end + def self.clean_backtrace(backtrace) + Array(Rails.backtrace_cleaner.clean(backtrace)).reject do |line| + line.match(Regexp.union(IGNORE_BACKTRACES)) + end + end + def self.with_custom_logger(logger) original_colorize_logging = ActiveSupport::LogSubscriber.colorize_logging original_activerecord_logger = ActiveRecord::Base.logger diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 4b8aae4f5a2..5cedd9e84c2 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -1,5 +1,4 @@ -# Gitaly note: JV: two sets of straightforward RPC's. 1 Hard RPC: fork_repository. -# SSH key operations are not part of Gitaly so will never be migrated. +# Gitaly note: SSH key operations are not part of Gitaly so will never be migrated. require 'securerandom' @@ -153,8 +152,6 @@ module Gitlab # # Ex. # mv_repository("/path/to/storage", "gitlab/gitlab-ci", "randx/gitlab-ci-new") - # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873 def mv_repository(storage, path, new_path) return false if path.empty? || new_path.empty? @@ -169,19 +166,11 @@ module Gitlab # # Ex. # fork_repository("nfs-file06", "gitlab/gitlab-ci", "nfs-file07", "new-namespace/gitlab-ci") - # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/817 def fork_repository(forked_from_storage, forked_from_disk_path, forked_to_storage, forked_to_disk_path) forked_from_relative_path = "#{forked_from_disk_path}.git" fork_args = [forked_to_storage, "#{forked_to_disk_path}.git"] - gitaly_migrate(:fork_repository, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - GitalyGitlabProjects.new(forked_from_storage, forked_from_relative_path).fork_repository(*fork_args) - else - gitlab_projects(forked_from_storage, forked_from_relative_path).fork_repository(*fork_args) - end - end + GitalyGitlabProjects.new(forked_from_storage, forked_from_relative_path).fork_repository(*fork_args) end # Removes a repository from file system, using rm_diretory which is an alias @@ -193,8 +182,6 @@ module Gitlab # # Ex. # remove_repository("/path/to/storage", "gitlab/gitlab-ci") - # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873 def remove_repository(storage, name) return false if name.empty? diff --git a/lib/tasks/gettext.rake b/lib/tasks/gettext.rake index 247d7be7d78..d52c419d66b 100644 --- a/lib/tasks/gettext.rake +++ b/lib/tasks/gettext.rake @@ -50,6 +50,32 @@ namespace :gettext do end end + task :updated_check do + # Removing all pre-translated files speeds up `gettext:find` as the + # files don't need to be merged. + `rm locale/*/gitlab.po` + + # `gettext:find` writes touches to temp files to `stderr` which would cause + # `static-analysis` to report failures. We can ignore these + silence_stream(STDERR) { Rake::Task['gettext:find'].invoke } + + changed_files = `git diff --name-only`.lines.map(&:strip) + + # reset the locale folder for potential next tasks + `git checkout -- locale` + + if changed_files.include?('locale/gitlab.pot') + raise <<~MSG + Newly translated strings found, please add them to `gitlab.pot` by running: + + bundle exec rake gettext:find; git checkout -- locale/*/gitlab.po; + + Then commit and push the resulting changes to `locale/gitlab.pot`. + + MSG + end + end + def report_errors_for_file(file, errors_for_file) puts "Errors in `#{file}`:" diff --git a/lib/tasks/lint.rake b/lib/tasks/lint.rake index 8b86a5c72a5..b5a9cddaacb 100644 --- a/lib/tasks/lint.rake +++ b/lib/tasks/lint.rake @@ -27,6 +27,7 @@ unless Rails.env.production? scss_lint flay gettext:lint + gettext:updated_check lint:static_verification ].each do |task| pid = Process.fork do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e8a5a15f410..db5c183bac3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8,8 +8,8 @@ msgid "" msgstr "" "Project-Id-Version: gitlab 1.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2018-06-12 18:57+1000\n" -"PO-Revision-Date: 2018-06-12 18:57+1000\n" +"POT-Creation-Date: 2018-06-13 14:05+0200\n" +"PO-Revision-Date: 2018-06-13 14:05+0200\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" "Language: \n" @@ -133,12 +133,12 @@ msgid "- show less" msgstr "" msgid "1 %{type} addition" -msgid_plural "%d %{type} additions" +msgid_plural "%{count} %{type} additions" msgstr[0] "" msgstr[1] "" msgid "1 %{type} modification" -msgid_plural "%d %{type} modifications" +msgid_plural "%{count} %{type} modifications" msgstr[0] "" msgstr[1] "" @@ -2252,10 +2252,10 @@ msgstr "" msgid "Gitaly" msgstr "" -msgid "Gitaly|Address" +msgid "Gitaly Servers" msgstr "" -msgid "Gitaly Servers" +msgid "Gitaly|Address" msgstr "" msgid "Go Back" @@ -2419,6 +2419,15 @@ msgstr "" msgid "If your HTTP repository is not publicly accessible, add authentication information to the URL: <code>https://username:password@gitlab.company.com/group/project.git</code>." msgstr "" +msgid "ImageDiffViewer|2-up" +msgstr "" + +msgid "ImageDiffViewer|Onion skin" +msgstr "" + +msgid "ImageDiffViewer|Swipe" +msgstr "" + msgid "Import" msgstr "" diff --git a/public/favicon.png b/public/favicon.png Binary files differdeleted file mode 100644 index 845e0ec34a5..00000000000 --- a/public/favicon.png +++ /dev/null diff --git a/qa/qa/page/main/login.rb b/qa/qa/page/main/login.rb index 596205fe540..26c99efc53d 100644 --- a/qa/qa/page/main/login.rb +++ b/qa/qa/page/main/login.rb @@ -26,53 +26,58 @@ module QA end def initialize + # The login page is usually the entry point for all the scenarios so + # we need to wait for the instance to start. That said, in some cases + # we are already logged-in so we check both cases here. wait(max: 500) do - page.has_css?('.application') + page.has_css?('.login-page') || + Page::Menu::Main.act { has_personal_area? } end end - def set_initial_password_if_present - if page.has_content?('Change your password') - fill_in :user_password, with: Runtime::User.password - fill_in :user_password_confirmation, with: Runtime::User.password - click_button 'Change your password' + def sign_in_using_credentials + # Don't try to log-in if we're already logged-in + return if Page::Menu::Main.act { has_personal_area? } + + using_wait_time 0 do + set_initial_password_if_present + + if Runtime::User.ldap_user? + sign_in_using_ldap_credentials + else + sign_in_using_gitlab_credentials + end end end - def sign_in_using_credentials - if Runtime::User.ldap_user? - sign_in_using_ldap_credentials - else - sign_in_using_gitlab_credentials - end + def self.path + '/users/sign_in' end - def sign_in_using_ldap_credentials - using_wait_time 0 do - set_initial_password_if_present + private - click_link 'LDAP' + def sign_in_using_ldap_credentials + click_link 'LDAP' - fill_in :username, with: Runtime::User.ldap_username - fill_in :password, with: Runtime::User.ldap_password - click_button 'Sign in' - end + fill_in :username, with: Runtime::User.ldap_username + fill_in :password, with: Runtime::User.ldap_password + click_button 'Sign in' end def sign_in_using_gitlab_credentials - using_wait_time 0 do - set_initial_password_if_present - - click_link 'Standard' if page.has_content?('LDAP') + click_link 'Standard' if page.has_content?('LDAP') - fill_in :user_login, with: Runtime::User.name - fill_in :user_password, with: Runtime::User.password - click_button 'Sign in' - end + fill_in :user_login, with: Runtime::User.name + fill_in :user_password, with: Runtime::User.password + click_button 'Sign in' end - def self.path - '/users/sign_in' + def set_initial_password_if_present + return unless page.has_content?('Change your password') + + fill_in :user_password, with: Runtime::User.password + fill_in :user_password_confirmation, with: Runtime::User.password + click_button 'Change your password' end end end diff --git a/qa/qa/page/menu/main.rb b/qa/qa/page/menu/main.rb index 644fedecc90..fda9c45c091 100644 --- a/qa/qa/page/menu/main.rb +++ b/qa/qa/page/menu/main.rb @@ -55,7 +55,8 @@ module QA end def has_personal_area? - page.has_selector?('.qa-user-avatar') + # No need to wait, either we're logged-in, or not. + using_wait_time(0) { page.has_selector?('.qa-user-avatar') } end private diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb index 011843baffc..812833cc86b 100644 --- a/spec/controllers/projects/imports_controller_spec.rb +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -29,7 +29,7 @@ describe Projects::ImportsController do context 'when import is in progress' do before do - project.update_attribute(:import_status, :started) + project.update_attributes(import_status: :started) end it 'renders template' do @@ -47,7 +47,7 @@ describe Projects::ImportsController do context 'when import failed' do before do - project.update_attribute(:import_status, :failed) + project.update_attributes(import_status: :failed) end it 'redirects to new_namespace_project_import_path' do @@ -59,7 +59,7 @@ describe Projects::ImportsController do context 'when import finished' do before do - project.update_attribute(:import_status, :finished) + project.update_attributes(import_status: :finished) end context 'when project is a fork' do @@ -108,7 +108,7 @@ describe Projects::ImportsController do context 'when import never happened' do before do - project.update_attribute(:import_status, :none) + project.update_attributes(import_status: :none) end it 'redirects to namespace_project_path' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 22858de0475..a412e74581d 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -234,7 +234,7 @@ describe Projects::MergeRequestsController do body = JSON.parse(response.body) expect(body['assignee'].keys) - .to match_array(%w(name username avatar_url)) + .to match_array(%w(name username avatar_url id state web_url)) end end diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index 3506305f755..4cdaa54e0bc 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -310,9 +310,19 @@ describe Projects::PipelineSchedulesController do end def go - put :update, namespace_id: project.namespace.to_param, - project_id: project, id: pipeline_schedule, - schedule: schedule + if Gitlab.rails5? + put :update, params: { namespace_id: project.namespace.to_param, + project_id: project, + id: pipeline_schedule, + schedule: schedule }, + as: :html + + else + put :update, namespace_id: project.namespace.to_param, + project_id: project, + id: pipeline_schedule, + schedule: schedule + end end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 555b186fe31..2b61e0d4a85 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -257,15 +257,15 @@ describe SessionsController do end end - describe '#new' do + describe "#new" do before do set_devise_mapping(context: @request) end - it 'redirects correctly for referer on same host with params' do - search_path = '/search?search=seed_project' - allow(controller.request).to receive(:referer) - .and_return('http://%{host}%{path}' % { host: 'test.host', path: search_path }) + it "redirects correctly for referer on same host with params" do + host = "test.host" + search_path = "/search?search=seed_project" + request.headers[:HTTP_REFERER] = "http://#{host}#{search_path}" get(:new, redirect_to_referer: :yes) diff --git a/spec/features/dashboard/groups_list_spec.rb b/spec/features/dashboard/groups_list_spec.rb index ed47f7ed390..29280bd6e06 100644 --- a/spec/features/dashboard/groups_list_spec.rb +++ b/spec/features/dashboard/groups_list_spec.rb @@ -65,7 +65,11 @@ feature 'Dashboard Groups page', :js do fill_in 'filter', with: group.name wait_for_requests + expect(page).to have_content(group.name) + expect(page).not_to have_content(nested_group.parent.name) + fill_in 'filter', with: '' + page.find('[name="filter"]').send_keys(:enter) wait_for_requests expect(page).to have_content(group.name) diff --git a/spec/features/explore/groups_list_spec.rb b/spec/features/explore/groups_list_spec.rb index 801a33979ff..ad02b454aee 100644 --- a/spec/features/explore/groups_list_spec.rb +++ b/spec/features/explore/groups_list_spec.rb @@ -35,7 +35,11 @@ describe 'Explore Groups page', :js do fill_in 'filter', with: group.name wait_for_requests + expect(page).to have_content(group.full_name) + expect(page).not_to have_content(public_group.full_name) + fill_in 'filter', with: "" + page.find('[name="filter"]').send_keys(:enter) wait_for_requests expect(page).to have_content(group.full_name) diff --git a/spec/features/ics/dashboard_issues_spec.rb b/spec/features/ics/dashboard_issues_spec.rb index 5d6cd44ad1c..90d02f7e40f 100644 --- a/spec/features/ics/dashboard_issues_spec.rb +++ b/spec/features/ics/dashboard_issues_spec.rb @@ -11,13 +11,25 @@ describe 'Dashboard Issues Calendar Feed' do end context 'when authenticated' do - it 'renders calendar feed' do - sign_in user - visit issues_dashboard_path(:ics) + context 'with no referer' do + it 'renders calendar feed' do + sign_in user + visit issues_dashboard_path(:ics) - expect(response_headers['Content-Type']).to have_content('text/calendar') - expect(response_headers['Content-Disposition']).to have_content('inline') - expect(body).to have_text('BEGIN:VCALENDAR') + expect(response_headers['Content-Type']).to have_content('text/calendar') + expect(body).to have_text('BEGIN:VCALENDAR') + end + end + + context 'with GitLab as the referer' do + it 'renders calendar feed as text/plain' do + sign_in user + page.driver.header('Referer', issues_dashboard_url(host: Settings.gitlab.base_url)) + visit issues_dashboard_path(:ics) + + expect(response_headers['Content-Type']).to have_content('text/plain') + expect(body).to have_text('BEGIN:VCALENDAR') + end end end @@ -28,7 +40,6 @@ describe 'Dashboard Issues Calendar Feed' do visit issues_dashboard_path(:ics, private_token: personal_access_token.token) expect(response_headers['Content-Type']).to have_content('text/calendar') - expect(response_headers['Content-Disposition']).to have_content('inline') expect(body).to have_text('BEGIN:VCALENDAR') end end @@ -38,7 +49,6 @@ describe 'Dashboard Issues Calendar Feed' do visit issues_dashboard_path(:ics, feed_token: user.feed_token) expect(response_headers['Content-Type']).to have_content('text/calendar') - expect(response_headers['Content-Disposition']).to have_content('inline') expect(body).to have_text('BEGIN:VCALENDAR') end end diff --git a/spec/features/ics/group_issues_spec.rb b/spec/features/ics/group_issues_spec.rb index 0a049be2ffe..24de5b4b7c6 100644 --- a/spec/features/ics/group_issues_spec.rb +++ b/spec/features/ics/group_issues_spec.rb @@ -13,13 +13,25 @@ describe 'Group Issues Calendar Feed' do end context 'when authenticated' do - it 'renders calendar feed' do - sign_in user - visit issues_group_path(group, :ics) + context 'with no referer' do + it 'renders calendar feed' do + sign_in user + visit issues_group_path(group, :ics) - expect(response_headers['Content-Type']).to have_content('text/calendar') - expect(response_headers['Content-Disposition']).to have_content('inline') - expect(body).to have_text('BEGIN:VCALENDAR') + expect(response_headers['Content-Type']).to have_content('text/calendar') + expect(body).to have_text('BEGIN:VCALENDAR') + end + end + + context 'with GitLab as the referer' do + it 'renders calendar feed as text/plain' do + sign_in user + page.driver.header('Referer', issues_group_url(group, host: Settings.gitlab.base_url)) + visit issues_group_path(group, :ics) + + expect(response_headers['Content-Type']).to have_content('text/plain') + expect(body).to have_text('BEGIN:VCALENDAR') + end end end @@ -30,7 +42,6 @@ describe 'Group Issues Calendar Feed' do visit issues_group_path(group, :ics, private_token: personal_access_token.token) expect(response_headers['Content-Type']).to have_content('text/calendar') - expect(response_headers['Content-Disposition']).to have_content('inline') expect(body).to have_text('BEGIN:VCALENDAR') end end @@ -40,7 +51,6 @@ describe 'Group Issues Calendar Feed' do visit issues_group_path(group, :ics, feed_token: user.feed_token) expect(response_headers['Content-Type']).to have_content('text/calendar') - expect(response_headers['Content-Disposition']).to have_content('inline') expect(body).to have_text('BEGIN:VCALENDAR') end end diff --git a/spec/features/ics/project_issues_spec.rb b/spec/features/ics/project_issues_spec.rb index b99e9607f1d..2ca3d52a5be 100644 --- a/spec/features/ics/project_issues_spec.rb +++ b/spec/features/ics/project_issues_spec.rb @@ -12,13 +12,25 @@ describe 'Project Issues Calendar Feed' do end context 'when authenticated' do - it 'renders calendar feed' do - sign_in user - visit project_issues_path(project, :ics) + context 'with no referer' do + it 'renders calendar feed' do + sign_in user + visit project_issues_path(project, :ics) - expect(response_headers['Content-Type']).to have_content('text/calendar') - expect(response_headers['Content-Disposition']).to have_content('inline') - expect(body).to have_text('BEGIN:VCALENDAR') + expect(response_headers['Content-Type']).to have_content('text/calendar') + expect(body).to have_text('BEGIN:VCALENDAR') + end + end + + context 'with GitLab as the referer' do + it 'renders calendar feed as text/plain' do + sign_in user + page.driver.header('Referer', project_issues_url(project, host: Settings.gitlab.base_url)) + visit project_issues_path(project, :ics) + + expect(response_headers['Content-Type']).to have_content('text/plain') + expect(body).to have_text('BEGIN:VCALENDAR') + end end end @@ -29,7 +41,6 @@ describe 'Project Issues Calendar Feed' do visit project_issues_path(project, :ics, private_token: personal_access_token.token) expect(response_headers['Content-Type']).to have_content('text/calendar') - expect(response_headers['Content-Disposition']).to have_content('inline') expect(body).to have_text('BEGIN:VCALENDAR') end end @@ -39,7 +50,6 @@ describe 'Project Issues Calendar Feed' do visit project_issues_path(project, :ics, feed_token: user.feed_token) expect(response_headers['Content-Type']).to have_content('text/calendar') - expect(response_headers['Content-Disposition']).to have_content('inline') expect(body).to have_text('BEGIN:VCALENDAR') end end diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json index f7bc137c90c..cf257ac00de 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_basic.json +++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json @@ -14,7 +14,21 @@ "subscribed": { "type": ["boolean", "null"] }, "participants": { "type": "array" }, "allow_collaboration": { "type": "boolean"}, - "allow_maintainer_to_push": { "type": "boolean"} + "allow_maintainer_to_push": { "type": "boolean"}, + "assignee": { + "oneOf": [ + { "type": "null" }, + { "$ref": "user.json" } + ] + }, + "milestone": { + "type": [ "object", "null" ] + }, + "labels": { + "type": [ "array", "null" ] + }, + "task_status": { "type": "string" }, + "task_status_short": { "type": "string" } }, "additionalProperties": false } diff --git a/spec/fixtures/api/schemas/public_api/v4/branch.json b/spec/fixtures/api/schemas/public_api/v4/branch.json index a3581178974..a8891680d06 100644 --- a/spec/fixtures/api/schemas/public_api/v4/branch.json +++ b/spec/fixtures/api/schemas/public_api/v4/branch.json @@ -14,7 +14,8 @@ "merged": { "type": "boolean" }, "protected": { "type": "boolean" }, "developers_can_push": { "type": "boolean" }, - "developers_can_merge": { "type": "boolean" } + "developers_can_merge": { "type": "boolean" }, + "can_push": { "type": "boolean" } }, "additionalProperties": false } diff --git a/spec/javascripts/notes/components/noteable_note_spec.js b/spec/javascripts/notes/components/noteable_note_spec.js index cfd037633e9..2ffdec7314d 100644 --- a/spec/javascripts/notes/components/noteable_note_spec.js +++ b/spec/javascripts/notes/components/noteable_note_spec.js @@ -32,7 +32,6 @@ describe('issue_note', () => { it('should render note header content', () => { expect(vm.$el.querySelector('.note-header .note-header-author-name').textContent.trim()).toEqual(note.author.name); - expect(vm.$el.querySelector('.note-header .note-headline-meta').textContent.trim()).toContain('commented'); }); it('should render note actions', () => { diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js index 3e2fd71b5b8..efa5c878678 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_merged_spec.js @@ -39,7 +39,8 @@ describe('MRWidgetMerged', () => { readableClosedAt: '', }, updatedAt: 'mergedUpdatedAt', - shortMergeCommitSha: 'asdf1234', + shortMergeCommitSha: '958c0475', + mergeCommitSha: '958c047516e182dfc52317f721f696e8a1ee85ed', mergeCommitPath: 'http://localhost:3000/root/nautilus/commit/f7ce827c314c9340b075657fd61c789fb01cf74d', sourceBranch: 'bar', targetBranch, @@ -153,7 +154,7 @@ describe('MRWidgetMerged', () => { it('shows button to copy commit SHA to clipboard', () => { expect(selectors.copyMergeShaButton).toExist(); - expect(selectors.copyMergeShaButton.getAttribute('data-clipboard-text')).toBe(vm.mr.shortMergeCommitSha); + expect(selectors.copyMergeShaButton.getAttribute('data-clipboard-text')).toBe(vm.mr.mergeCommitSha); }); it('shows merge commit SHA link', () => { diff --git a/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb b/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb index 8224dc5a6b9..b645e49bd43 100644 --- a/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb +++ b/spec/lib/banzai/filter/blockquote_fence_filter_spec.rb @@ -11,4 +11,8 @@ describe Banzai::Filter::BlockquoteFenceFilter do expect(output).to eq(expected) end + + it 'allows trailing whitespace on blockquote fence lines' do + expect(filter(">>> \ntest\n>>> ")).to eq("> test") + end end diff --git a/spec/lib/gitlab/auth/user_auth_finders_spec.rb b/spec/lib/gitlab/auth/user_auth_finders_spec.rb index 136646bd4ee..454ad1589b9 100644 --- a/spec/lib/gitlab/auth/user_auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/user_auth_finders_spec.rb @@ -99,7 +99,7 @@ describe Gitlab::Auth::UserAuthFinders do context 'when the request format is empty' do it 'the method call does not modify the original value' do - env['action_dispatch.request.formats'] = nil + env.delete('action_dispatch.request.formats') find_user_from_feed_token diff --git a/spec/lib/gitlab/checks/force_push_spec.rb b/spec/lib/gitlab/checks/force_push_spec.rb index a65012d2314..0e0788ce974 100644 --- a/spec/lib/gitlab/checks/force_push_spec.rb +++ b/spec/lib/gitlab/checks/force_push_spec.rb @@ -1,21 +1,17 @@ require 'spec_helper' describe Gitlab::Checks::ForcePush do - let(:project) { create(:project, :repository) } - let(:repository) { project.repository.raw } + set(:project) { create(:project, :repository) } - context "exit code checking", :skip_gitaly_mock do - it "does not raise a runtime error if the `popen` call to git returns a zero exit code" do - allow(repository).to receive(:popen).and_return(['normal output', 0]) + describe '.force_push?' do + it 'returns false if the repo is empty' do + allow(project).to receive(:empty_repo?).and_return(true) - expect { described_class.force_push?(project, 'oldrev', 'newrev') }.not_to raise_error + expect(described_class.force_push?(project, 'HEAD', 'HEAD~')).to be(false) end - it "raises a GitError error if the `popen` call to git returns a non-zero exit code" do - allow(repository).to receive(:popen).and_return(['error', 1]) - - expect { described_class.force_push?(project, 'oldrev', 'newrev') } - .to raise_error(Gitlab::Git::Repository::GitError) + it 'checks if old rev is an anchestor' do + expect(described_class.force_push?(project, 'HEAD', 'HEAD~')).to be(true) end end end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index bc5a5e43103..2e204da307d 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -49,7 +49,7 @@ describe Gitlab::Ci::Config do describe '.new' do it 'raises error' do expect { config }.to raise_error( - Gitlab::Ci::Config::Loader::FormatError, + ::Gitlab::Ci::Config::Loader::FormatError, /Invalid configuration format/ ) end diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index c5a4d9b4778..284aed91e29 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Chain::Populate do - set(:project) { create(:project) } + set(:project) { create(:project, :repository) } set(:user) { create(:user) } let(:pipeline) do @@ -174,7 +174,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do end let(:pipeline) do - build(:ci_pipeline, ref: 'master', config: config) + build(:ci_pipeline, ref: 'master', project: project, config: config) end it_behaves_like 'a correct pipeline' diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index c53294d091c..a8dc5356413 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Chain::Validate::Config do - set(:project) { create(:project) } + set(:project) { create(:project, :repository) } set(:user) { create(:user) } let(:command) do diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index ecb16daec96..fa5327c26f0 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' module Gitlab module Ci - describe YamlProcessor, :lib do + describe YamlProcessor do subject { described_class.new(config) } describe 'our current .gitlab-ci.yml' do diff --git a/spec/lib/gitlab/git/committer_with_hooks_spec.rb b/spec/lib/gitlab/git/committer_with_hooks_spec.rb index 267056b96e6..2100690f873 100644 --- a/spec/lib/gitlab/git/committer_with_hooks_spec.rb +++ b/spec/lib/gitlab/git/committer_with_hooks_spec.rb @@ -1,154 +1,156 @@ require 'spec_helper' describe Gitlab::Git::CommitterWithHooks, seed_helper: true do - shared_examples 'calling wiki hooks' do - let(:project) { create(:project) } - let(:user) { project.owner } - let(:project_wiki) { ProjectWiki.new(project, user) } - let(:wiki) { project_wiki.wiki } - let(:options) do - { - id: user.id, - username: user.username, - name: user.name, - email: user.email, - message: 'commit message' - } - end - - subject { described_class.new(wiki, options) } + # TODO https://gitlab.com/gitlab-org/gitaly/issues/1234 + skip 'needs to be moved to gitaly-ruby test suite' do + shared_examples 'calling wiki hooks' do + let(:project) { create(:project) } + let(:user) { project.owner } + let(:project_wiki) { ProjectWiki.new(project, user) } + let(:wiki) { project_wiki.wiki } + let(:options) do + { + id: user.id, + username: user.username, + name: user.name, + email: user.email, + message: 'commit message' + } + end - before do - project_wiki.create_page('home', 'test content') - end + subject { described_class.new(wiki, options) } - shared_examples 'failing pre-receive hook' do before do - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([false, '']) - expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('update') - expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive') + project_wiki.create_page('home', 'test content') end - it 'raises exception' do - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - end + shared_examples 'failing pre-receive hook' do + before do + expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([false, '']) + expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('update') + expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive') + end - it 'does not create a new commit inside the repository' do - current_rev = find_current_rev + it 'raises exception' do + expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) + end - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) + it 'does not create a new commit inside the repository' do + current_rev = find_current_rev - expect(current_rev).to eq find_current_rev - end - end + expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - shared_examples 'failing update hook' do - before do - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, '']) - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([false, '']) - expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive') + expect(current_rev).to eq find_current_rev + end end - it 'raises exception' do - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - end + shared_examples 'failing update hook' do + before do + expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, '']) + expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([false, '']) + expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive') + end - it 'does not create a new commit inside the repository' do - current_rev = find_current_rev + it 'raises exception' do + expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) + end - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) + it 'does not create a new commit inside the repository' do + current_rev = find_current_rev - expect(current_rev).to eq find_current_rev - end - end + expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - shared_examples 'failing post-receive hook' do - before do - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, '']) - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([true, '']) - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('post-receive').and_return([false, '']) + expect(current_rev).to eq find_current_rev + end end - it 'does not raise exception' do - expect { subject.commit }.not_to raise_error - end + shared_examples 'failing post-receive hook' do + before do + expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, '']) + expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([true, '']) + expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('post-receive').and_return([false, '']) + end + + it 'does not raise exception' do + expect { subject.commit }.not_to raise_error + end - it 'creates the commit' do - current_rev = find_current_rev + it 'creates the commit' do + current_rev = find_current_rev - subject.commit + subject.commit - expect(current_rev).not_to eq find_current_rev + expect(current_rev).not_to eq find_current_rev + end end - end - shared_examples 'when hooks call succceeds' do - let(:hook) { double(:hook) } + shared_examples 'when hooks call succceeds' do + let(:hook) { double(:hook) } - it 'calls the three hooks' do - expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - expect(hook).to receive(:trigger).exactly(3).times.and_return([true, nil]) + it 'calls the three hooks' do + expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) + expect(hook).to receive(:trigger).exactly(3).times.and_return([true, nil]) - subject.commit - end + subject.commit + end - it 'creates the commit' do - current_rev = find_current_rev + it 'creates the commit' do + current_rev = find_current_rev - subject.commit + subject.commit - expect(current_rev).not_to eq find_current_rev + expect(current_rev).not_to eq find_current_rev + end end - end - context 'when creating a page' do - before do - project_wiki.create_page('index', 'test content') + context 'when creating a page' do + before do + project_wiki.create_page('index', 'test content') + end + + it_behaves_like 'failing pre-receive hook' + it_behaves_like 'failing update hook' + it_behaves_like 'failing post-receive hook' + it_behaves_like 'when hooks call succceeds' end - it_behaves_like 'failing pre-receive hook' - it_behaves_like 'failing update hook' - it_behaves_like 'failing post-receive hook' - it_behaves_like 'when hooks call succceeds' - end + context 'when updating a page' do + before do + project_wiki.update_page(find_page('home'), content: 'some other content', format: :markdown) + end - context 'when updating a page' do - before do - project_wiki.update_page(find_page('home'), content: 'some other content', format: :markdown) + it_behaves_like 'failing pre-receive hook' + it_behaves_like 'failing update hook' + it_behaves_like 'failing post-receive hook' + it_behaves_like 'when hooks call succceeds' end - it_behaves_like 'failing pre-receive hook' - it_behaves_like 'failing update hook' - it_behaves_like 'failing post-receive hook' - it_behaves_like 'when hooks call succceeds' - end + context 'when deleting a page' do + before do + project_wiki.delete_page(find_page('home')) + end - context 'when deleting a page' do - before do - project_wiki.delete_page(find_page('home')) + it_behaves_like 'failing pre-receive hook' + it_behaves_like 'failing update hook' + it_behaves_like 'failing post-receive hook' + it_behaves_like 'when hooks call succceeds' end - it_behaves_like 'failing pre-receive hook' - it_behaves_like 'failing update hook' - it_behaves_like 'failing post-receive hook' - it_behaves_like 'when hooks call succceeds' - end + def find_current_rev + wiki.gollum_wiki.repo.commits.first&.sha + end - def find_current_rev - wiki.gollum_wiki.repo.commits.first&.sha + def find_page(name) + wiki.page(title: name) + end end - def find_page(name) - wiki.page(title: name) + context 'when Gitaly is enabled' do + it_behaves_like 'calling wiki hooks' end - end - - # TODO: Uncomment once Gitaly updates the ruby vendor code - # context 'when Gitaly is enabled' do - # it_behaves_like 'calling wiki hooks' - # end - context 'when Gitaly is disabled', :skip_gitaly_mock do - it_behaves_like 'calling wiki hooks' + context 'when Gitaly is disabled', :disable_gitaly do + it_behaves_like 'calling wiki hooks' + end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 5bae99101e6..595482f76d5 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1114,7 +1114,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#count_commits' do - shared_examples 'extended commit counting' do + describe 'extended commit counting' do context 'with after timestamp' do it 'returns the number of commits after timestamp' do options = { ref: 'master', after: Time.iso8601('2013-03-03T20:15:01+00:00') } @@ -1199,14 +1199,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end end - - context 'when Gitaly count_commits feature is enabled' do - it_behaves_like 'extended commit counting' - end - - context 'when Gitaly count_commits feature is disabled', :disable_gitaly do - it_behaves_like 'extended commit counting' - end end describe '#autocrlf' do @@ -1688,70 +1680,52 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#languages' do - shared_examples 'languages' do - it 'returns exactly the expected results' do - languages = repository.languages('4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6') - expected_languages = [ - { value: 66.63, label: "Ruby", color: "#701516", highlight: "#701516" }, - { value: 22.96, label: "JavaScript", color: "#f1e05a", highlight: "#f1e05a" }, - { value: 7.9, label: "HTML", color: "#e34c26", highlight: "#e34c26" }, - { value: 2.51, label: "CoffeeScript", color: "#244776", highlight: "#244776" } - ] + it 'returns exactly the expected results' do + languages = repository.languages('4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6') + expected_languages = [ + { value: 66.63, label: "Ruby", color: "#701516", highlight: "#701516" }, + { value: 22.96, label: "JavaScript", color: "#f1e05a", highlight: "#f1e05a" }, + { value: 7.9, label: "HTML", color: "#e34c26", highlight: "#e34c26" }, + { value: 2.51, label: "CoffeeScript", color: "#244776", highlight: "#244776" } + ] - expect(languages.size).to eq(expected_languages.size) + expect(languages.size).to eq(expected_languages.size) - expected_languages.size.times do |i| - a = expected_languages[i] - b = languages[i] + expected_languages.size.times do |i| + a = expected_languages[i] + b = languages[i] - expect(a.keys.sort).to eq(b.keys.sort) - expect(a[:value]).to be_within(0.1).of(b[:value]) + expect(a.keys.sort).to eq(b.keys.sort) + expect(a[:value]).to be_within(0.1).of(b[:value]) - non_float_keys = a.keys - [:value] - expect(a.values_at(*non_float_keys)).to eq(b.values_at(*non_float_keys)) - end - end - - it "uses the repository's HEAD when no ref is passed" do - lang = repository.languages.first - - expect(lang[:label]).to eq('Ruby') + non_float_keys = a.keys - [:value] + expect(a.values_at(*non_float_keys)).to eq(b.values_at(*non_float_keys)) end end - it_behaves_like 'languages' + it "uses the repository's HEAD when no ref is passed" do + lang = repository.languages.first - context 'with rugged', :skip_gitaly_mock do - it_behaves_like 'languages' + expect(lang[:label]).to eq('Ruby') end end describe '#license_short_name' do - shared_examples 'acquiring the Licensee license key' do - subject { repository.license_short_name } - - context 'when no license file can be found' do - let(:project) { create(:project, :repository) } - let(:repository) { project.repository.raw_repository } + subject { repository.license_short_name } - before do - project.repository.delete_file(project.owner, 'LICENSE', message: 'remove license', branch_name: 'master') - end - - it { is_expected.to be_nil } - end + context 'when no license file can be found' do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository.raw_repository } - context 'when an mit license is found' do - it { is_expected.to eq('mit') } + before do + project.repository.delete_file(project.owner, 'LICENSE', message: 'remove license', branch_name: 'master') end - end - context 'when gitaly is enabled' do - it_behaves_like 'acquiring the Licensee license key' + it { is_expected.to be_nil } end - context 'when gitaly is disabled', :disable_gitaly do - it_behaves_like 'acquiring the Licensee license key' + context 'when an mit license is found' do + it { is_expected.to eq('mit') } end end diff --git a/spec/lib/gitlab/git/rev_list_spec.rb b/spec/lib/gitlab/git/rev_list_spec.rb index 95dc47e2a00..b752c3e8341 100644 --- a/spec/lib/gitlab/git/rev_list_spec.rb +++ b/spec/lib/gitlab/git/rev_list_spec.rb @@ -93,14 +93,4 @@ describe Gitlab::Git::RevList do expect { |b| rev_list.all_objects(&b) }.to yield_with_args(%w[sha1 sha2]) end end - - context "#missed_ref" do - let(:rev_list) { described_class.new(repository, oldrev: 'oldrev', newrev: 'newrev') } - - it 'calls out to `popen`' do - stub_popen_rev_list('--max-count=1', 'oldrev', '^newrev', with_lazy_block: false, output: "sha1\nsha2") - - expect(rev_list.missed_ref).to eq(%w[sha1 sha2]) - end - end end diff --git a/spec/lib/gitlab/git/wiki_spec.rb b/spec/lib/gitlab/git/wiki_spec.rb index 35b06b14620..b63658e1b3b 100644 --- a/spec/lib/gitlab/git/wiki_spec.rb +++ b/spec/lib/gitlab/git/wiki_spec.rb @@ -6,9 +6,7 @@ describe Gitlab::Git::Wiki do let(:project_wiki) { ProjectWiki.new(project, user) } subject { project_wiki.wiki } - # Remove skip_gitaly_mock flag when gitaly_find_page when - # https://gitlab.com/gitlab-org/gitlab-ce/issues/42039 is solved - describe '#page', :skip_gitaly_mock do + describe '#page' do before do create_page('page1', 'content') create_page('foo/page1', 'content foo/page1') @@ -25,7 +23,7 @@ describe Gitlab::Git::Wiki do end end - describe '#delete_page', :skip_gitaly_mock do + describe '#delete_page' do after do destroy_page('page1') end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 0d5f6a0b576..ff32025253a 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -934,6 +934,22 @@ describe Gitlab::GitAccess do expect(project.repository).to receive(:clean_stale_repository_files).and_call_original expect { push_access_check }.not_to raise_error end + + it 'avoids N+1 queries', :request_store do + # Run this once to establish a baseline. Cached queries should get + # cached, so that when we introduce another change we shouldn't see + # additional queries. + access.check('git-receive-pack', changes) + + control_count = ActiveRecord::QueryRecorder.new do + access.check('git-receive-pack', changes) + end + + changes = ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'] + + # There is still an N+1 query with protected branches + expect { access.check('git-receive-pack', changes) }.not_to exceed_query_limit(control_count).with_threshold(1) + end end end diff --git a/spec/lib/gitlab/i18n/metadata_entry_spec.rb b/spec/lib/gitlab/i18n/metadata_entry_spec.rb index ab71d6454a9..a399517cc04 100644 --- a/spec/lib/gitlab/i18n/metadata_entry_spec.rb +++ b/spec/lib/gitlab/i18n/metadata_entry_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::I18n::MetadataEntry do - describe '#expected_plurals' do + describe '#expected_forms' do it 'returns the number of plurals' do data = { msgid: "", @@ -22,7 +22,7 @@ describe Gitlab::I18n::MetadataEntry do } entry = described_class.new(data) - expect(entry.expected_plurals).to eq(2) + expect(entry.expected_forms).to eq(2) end it 'returns 0 for the POT-metadata' do @@ -45,7 +45,7 @@ describe Gitlab::I18n::MetadataEntry do } entry = described_class.new(data) - expect(entry.expected_plurals).to eq(0) + expect(entry.expected_forms).to eq(0) end end end diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb index 3a962ba7f22..3dbc23d2aaf 100644 --- a/spec/lib/gitlab/i18n/po_linter_spec.rb +++ b/spec/lib/gitlab/i18n/po_linter_spec.rb @@ -1,10 +1,31 @@ require 'spec_helper' require 'simple_po_parser' +# Disabling this cop to allow for multi-language examples in comments +# rubocop:disable Style/AsciiComments describe Gitlab::I18n::PoLinter do let(:linter) { described_class.new(po_path) } let(:po_path) { 'spec/fixtures/valid.po' } + def fake_translation(msgid:, translation:, plural_id: nil, plurals: []) + data = { msgid: msgid, msgid_plural: plural_id } + + if plural_id + [translation, *plurals].each_with_index do |plural, index| + allow(FastGettext::Translation).to receive(:n_).with(msgid, plural_id, index).and_return(plural) + data.merge!("msgstr[#{index}]" => plural) + end + else + allow(FastGettext::Translation).to receive(:_).with(msgid).and_return(translation) + data[:msgstr] = translation + end + + Gitlab::I18n::TranslationEntry.new( + data, + plurals.size + 1 + ) + end + describe '#errors' do it 'only calls validation once' do expect(linter).to receive(:validate_po).once.and_call_original @@ -155,9 +176,8 @@ describe Gitlab::I18n::PoLinter do describe '#validate_entries' do it 'keeps track of errors for entries' do - fake_invalid_entry = Gitlab::I18n::TranslationEntry.new( - { msgid: "Hello %{world}", msgstr: "Bonjour %{monde}" }, 2 - ) + fake_invalid_entry = fake_translation(msgid: "Hello %{world}", + translation: "Bonjour %{monde}") allow(linter).to receive(:translation_entries) { [fake_invalid_entry] } expect(linter).to receive(:validate_entry) @@ -177,6 +197,7 @@ describe Gitlab::I18n::PoLinter do expect(linter).to receive(:validate_newlines).with([], fake_entry) expect(linter).to receive(:validate_number_of_plurals).with([], fake_entry) expect(linter).to receive(:validate_unescaped_chars).with([], fake_entry) + expect(linter).to receive(:validate_translation).with([], fake_entry) linter.validate_entry(fake_entry) end @@ -185,7 +206,7 @@ describe Gitlab::I18n::PoLinter do describe '#validate_number_of_plurals' do it 'validates when there are an incorrect number of translations' do fake_metadata = double - allow(fake_metadata).to receive(:expected_plurals).and_return(2) + allow(fake_metadata).to receive(:expected_forms).and_return(2) allow(linter).to receive(:metadata_entry).and_return(fake_metadata) fake_entry = Gitlab::I18n::TranslationEntry.new( @@ -201,13 +222,16 @@ describe Gitlab::I18n::PoLinter do end describe '#validate_variables' do - it 'validates both signular and plural in a pluralized string when the entry has a singular' do - pluralized_entry = Gitlab::I18n::TranslationEntry.new( - { msgid: 'Hello %{world}', - msgid_plural: 'Hello all %{world}', - 'msgstr[0]' => 'Bonjour %{world}', - 'msgstr[1]' => 'Bonjour tous %{world}' }, - 2 + before do + allow(linter).to receive(:validate_variables_in_message).and_call_original + end + + it 'validates both singular and plural in a pluralized string when the entry has a singular' do + pluralized_entry = fake_translation( + msgid: 'Hello %{world}', + translation: 'Bonjour %{world}', + plural_id: 'Hello all %{world}', + plurals: ['Bonjour tous %{world}'] ) expect(linter).to receive(:validate_variables_in_message) @@ -221,11 +245,10 @@ describe Gitlab::I18n::PoLinter do end it 'only validates plural when there is no separate singular' do - pluralized_entry = Gitlab::I18n::TranslationEntry.new( - { msgid: 'Hello %{world}', - msgid_plural: 'Hello all %{world}', - 'msgstr[0]' => 'Bonjour %{world}' }, - 1 + pluralized_entry = fake_translation( + msgid: 'Hello %{world}', + translation: 'Bonjour %{world}', + plural_id: 'Hello all %{world}' ) expect(linter).to receive(:validate_variables_in_message) @@ -235,37 +258,65 @@ describe Gitlab::I18n::PoLinter do end it 'validates the message variables' do - entry = Gitlab::I18n::TranslationEntry.new( - { msgid: 'Hello', msgstr: 'Bonjour' }, - 2 - ) + entry = fake_translation(msgid: 'Hello', translation: 'Bonjour') expect(linter).to receive(:validate_variables_in_message) .with([], 'Hello', 'Bonjour') linter.validate_variables([], entry) end + + it 'validates variable usage in message ids' do + entry = fake_translation( + msgid: 'Hello %{world}', + translation: 'Bonjour %{world}', + plural_id: 'Hello all %{world}', + plurals: ['Bonjour tous %{world}'] + ) + + expect(linter).to receive(:validate_variables_in_message) + .with([], 'Hello %{world}', 'Hello %{world}') + .and_call_original + expect(linter).to receive(:validate_variables_in_message) + .with([], 'Hello all %{world}', 'Hello all %{world}') + .and_call_original + + linter.validate_variables([], entry) + end end describe '#validate_variables_in_message' do it 'detects when a variables are used incorrectly' do errors = [] - expected_errors = ['<hello %{world} %d> is missing: [%{hello}]', - '<hello %{world} %d> is using unknown variables: [%{world}]', - 'is combining multiple unnamed variables'] + expected_errors = ['<%d hello %{world} %s> is missing: [%{hello}]', + '<%d hello %{world} %s> is using unknown variables: [%{world}]', + 'is combining multiple unnamed variables', + 'is combining named variables with unnamed variables'] - linter.validate_variables_in_message(errors, '%{hello} world %d', 'hello %{world} %d') + linter.validate_variables_in_message(errors, '%d %{hello} world %s', '%d hello %{world} %s') expect(errors).to include(*expected_errors) end + + it 'does not allow combining 1 `%d` unnamed variable with named variables' do + errors = [] + + linter.validate_variables_in_message(errors, + '%{type} detected %d vulnerability', + '%{type} detecteerde %d kwetsbaarheid') + + expect(errors).not_to be_empty + end end describe '#validate_translation' do + let(:entry) { fake_translation(msgid: 'Hello %{world}', translation: 'Bonjour %{world}') } + it 'succeeds with valid variables' do errors = [] - linter.validate_translation(errors, 'Hello %{world}', ['%{world}']) + linter.validate_translation(errors, entry) expect(errors).to be_empty end @@ -275,43 +326,80 @@ describe Gitlab::I18n::PoLinter do expect(FastGettext::Translation).to receive(:_) { raise 'broken' } - linter.validate_translation(errors, 'Hello', []) + linter.validate_translation(errors, entry) - expect(errors).to include('Failure translating to en with []: broken') + expect(errors).to include('Failure translating to en: broken') end it 'adds an error message when translating fails when translating with context' do + entry = fake_translation(msgid: 'Tests|Hello', translation: 'broken') errors = [] expect(FastGettext::Translation).to receive(:s_) { raise 'broken' } - linter.validate_translation(errors, 'Tests|Hello', []) + linter.validate_translation(errors, entry) - expect(errors).to include('Failure translating to en with []: broken') + expect(errors).to include('Failure translating to en: broken') end it "adds an error when trying to translate with incorrect variables when using unnamed variables" do + entry = fake_translation(msgid: 'Hello %s', translation: 'Hello %d') errors = [] - linter.validate_translation(errors, 'Hello %d', ['%s']) + linter.validate_translation(errors, entry) - expect(errors.first).to start_with("Failure translating to en with") + expect(errors.first).to start_with("Failure translating to en") end it "adds an error when trying to translate with named variables when unnamed variables are expected" do + entry = fake_translation(msgid: 'Hello %s', translation: 'Hello %{thing}') errors = [] - linter.validate_translation(errors, 'Hello %d', ['%{world}']) + linter.validate_translation(errors, entry) - expect(errors.first).to start_with("Failure translating to en with") + expect(errors.first).to start_with("Failure translating to en") end - it 'adds an error when translated with incorrect variables using named variables' do - errors = [] + it 'tests translation for all given forms' do + # Fake a language that has 3 forms to translate + fake_metadata = double + allow(fake_metadata).to receive(:forms_to_test).and_return(3) + allow(linter).to receive(:metadata_entry).and_return(fake_metadata) + entry = fake_translation( + msgid: '%d exception', + translation: '%d uitzondering', + plural_id: '%d exceptions', + plurals: ['%d uitzonderingen', '%d uitzonderingetjes'] + ) + + # Make each count use a different index + allow(linter).to receive(:index_for_pluralization).with(0).and_return(0) + allow(linter).to receive(:index_for_pluralization).with(1).and_return(1) + allow(linter).to receive(:index_for_pluralization).with(2).and_return(2) + + expect(FastGettext::Translation).to receive(:n_).with('%d exception', '%d exceptions', 0).and_call_original + expect(FastGettext::Translation).to receive(:n_).with('%d exception', '%d exceptions', 1).and_call_original + expect(FastGettext::Translation).to receive(:n_).with('%d exception', '%d exceptions', 2).and_call_original + + linter.validate_translation([], entry) + end + end + + describe '#numbers_covering_all_plurals' do + it 'can correctly find all required numbers to translate to Polish' do + # Polish used as an example with 3 different forms: + # 0, all plurals except the ones ending in 2,3,4: Kotów + # 1: Kot + # 2-3-4: Koty + # So translating with [0, 1, 2] will give us all different posibilities + fake_metadata = double + allow(fake_metadata).to receive(:forms_to_test).and_return(4) + allow(linter).to receive(:metadata_entry).and_return(fake_metadata) + allow(linter).to receive(:locale).and_return('pl_PL') - linter.validate_translation(errors, 'Hello %{thing}', ['%d']) + numbers = linter.numbers_covering_all_plurals - expect(errors.first).to start_with("Failure translating to en with") + expect(numbers).to contain_exactly(0, 1, 2) end end @@ -336,3 +424,4 @@ describe Gitlab::I18n::PoLinter do end end end +# rubocop:enable Style/AsciiComments diff --git a/spec/lib/gitlab/i18n/translation_entry_spec.rb b/spec/lib/gitlab/i18n/translation_entry_spec.rb index f68bc8feff9..b301e6ea443 100644 --- a/spec/lib/gitlab/i18n/translation_entry_spec.rb +++ b/spec/lib/gitlab/i18n/translation_entry_spec.rb @@ -109,7 +109,7 @@ describe Gitlab::I18n::TranslationEntry do data = { msgid: %w(hello world) } entry = described_class.new(data, 2) - expect(entry.msgid_contains_newlines?).to be_truthy + expect(entry.msgid_has_multiple_lines?).to be_truthy end end @@ -118,7 +118,7 @@ describe Gitlab::I18n::TranslationEntry do data = { msgid_plural: %w(hello world) } entry = described_class.new(data, 2) - expect(entry.plural_id_contains_newlines?).to be_truthy + expect(entry.plural_id_has_multiple_lines?).to be_truthy end end @@ -127,7 +127,7 @@ describe Gitlab::I18n::TranslationEntry do data = { msgstr: %w(hello world) } entry = described_class.new(data, 2) - expect(entry.translations_contain_newlines?).to be_truthy + expect(entry.translations_have_multiple_lines?).to be_truthy end end diff --git a/spec/lib/gitlab/profiler_spec.rb b/spec/lib/gitlab/profiler_spec.rb index 548eb28fe4d..4059188fba1 100644 --- a/spec/lib/gitlab/profiler_spec.rb +++ b/spec/lib/gitlab/profiler_spec.rb @@ -135,6 +135,51 @@ describe Gitlab::Profiler do end end + describe '.clean_backtrace' do + it 'uses the Rails backtrace cleaner' do + backtrace = [] + + expect(Rails.backtrace_cleaner).to receive(:clean).with(backtrace) + + described_class.clean_backtrace(backtrace) + end + + it 'removes lines from IGNORE_BACKTRACES' do + backtrace = [ + "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'", + "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'", + "lib/gitlab/gitaly_client.rb:280:in `block in migrate'", + "lib/gitlab/metrics/influx_db.rb:103:in `measure'", + "lib/gitlab/gitaly_client.rb:278:in `migrate'", + "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'", + "lib/gitlab/git/commit.rb:66:in `find'", + "app/models/repository.rb:1047:in `find_commit'", + "lib/gitlab/metrics/instrumentation.rb:159:in `block in find_commit'", + "lib/gitlab/metrics/method_call.rb:36:in `measure'", + "lib/gitlab/metrics/instrumentation.rb:159:in `find_commit'", + "app/models/repository.rb:113:in `commit'", + "lib/gitlab/i18n.rb:50:in `with_locale'", + "lib/gitlab/middleware/multipart.rb:95:in `call'", + "lib/gitlab/request_profiler/middleware.rb:14:in `call'", + "ee/lib/gitlab/database/load_balancing/rack_middleware.rb:37:in `call'", + "ee/lib/gitlab/jira/middleware.rb:15:in `call'" + ] + + expect(described_class.clean_backtrace(backtrace)) + .to eq([ + "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'", + "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'", + "lib/gitlab/gitaly_client.rb:280:in `block in migrate'", + "lib/gitlab/gitaly_client.rb:278:in `migrate'", + "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'", + "lib/gitlab/git/commit.rb:66:in `find'", + "app/models/repository.rb:1047:in `find_commit'", + "app/models/repository.rb:113:in `commit'", + "ee/lib/gitlab/jira/middleware.rb:15:in `call'" + ]) + end + end + describe '.with_custom_logger' do context 'when the logger is set' do it 'uses the replacement logger for the duration of the block' do diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 155e1663298..c435f988cdd 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -498,34 +498,18 @@ describe Gitlab::Shell do ) end - context 'with gitaly' do - it 'returns true when the command succeeds' do - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository) - .with(repository.raw_repository) { :gitaly_response_object } - - is_expected.to be_truthy - end - - it 'return false when the command fails' do - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository) - .with(repository.raw_repository) { raise GRPC::BadStatus, 'bla' } + it 'returns true when the command succeeds' do + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository) + .with(repository.raw_repository) { :gitaly_response_object } - is_expected.to be_falsy - end + is_expected.to be_truthy end - context 'without gitaly', :disable_gitaly do - it 'returns true when the command succeeds' do - expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { true } - - is_expected.to be_truthy - end - - it 'return false when the command fails' do - expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { false } + it 'return false when the command fails' do + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository) + .with(repository.raw_repository) { raise GRPC::BadStatus, 'bla' } - is_expected.to be_falsy - end + is_expected.to be_falsy end end @@ -665,7 +649,7 @@ describe Gitlab::Shell do subject do gitlab_shell.fetch_remote(repository.raw_repository, remote_name, - forced: true, no_tags: true, ssh_auth: ssh_auth) + forced: true, no_tags: true, ssh_auth: ssh_auth) end it 'passes the correct params to the gitaly service' do diff --git a/spec/migrations/remove_soft_removed_objects_spec.rb b/spec/migrations/remove_soft_removed_objects_spec.rb index fb70c284f5e..d0bde98b80e 100644 --- a/spec/migrations/remove_soft_removed_objects_spec.rb +++ b/spec/migrations/remove_soft_removed_objects_spec.rb @@ -3,6 +3,18 @@ require Rails.root.join('db', 'post_migrate', '20171207150343_remove_soft_remove describe RemoveSoftRemovedObjects, :migration do describe '#up' do + let!(:groups) do + table(:namespaces).tap do |t| + t.inheritance_column = nil + end + end + + let!(:routes) do + table(:routes).tap do |t| + t.inheritance_column = nil + end + end + it 'removes various soft removed objects' do 5.times do create_with_deleted_at(:issue) @@ -28,19 +40,20 @@ describe RemoveSoftRemovedObjects, :migration do it 'removes routes of soft removed personal namespaces' do namespace = create_with_deleted_at(:namespace) - group = create(:group) # rubocop:disable RSpec/FactoriesInMigrationSpecs + group = groups.create!(name: 'group', path: 'group_path', type: 'Group') + routes.create!(source_id: group.id, source_type: 'Group', name: 'group', path: 'group_path') - expect(Route.where(source: namespace).exists?).to eq(true) - expect(Route.where(source: group).exists?).to eq(true) + expect(routes.where(source_id: namespace.id).exists?).to eq(true) + expect(routes.where(source_id: group.id).exists?).to eq(true) run_migration - expect(Route.where(source: namespace).exists?).to eq(false) - expect(Route.where(source: group).exists?).to eq(true) + expect(routes.where(source_id: namespace.id).exists?).to eq(false) + expect(routes.where(source_id: group.id).exists?).to eq(true) end it 'schedules the removal of soft removed groups' do - group = create_with_deleted_at(:group) + group = create_deleted_group admin = create(:user, admin: true) # rubocop:disable RSpec/FactoriesInMigrationSpecs expect_any_instance_of(GroupDestroyWorker) @@ -51,7 +64,7 @@ describe RemoveSoftRemovedObjects, :migration do end it 'does not remove soft removed groups when no admin user could be found' do - create_with_deleted_at(:group) + create_deleted_group expect_any_instance_of(GroupDestroyWorker) .not_to receive(:perform) @@ -74,4 +87,13 @@ describe RemoveSoftRemovedObjects, :migration do row end + + def create_deleted_group + group = groups.create!(name: 'group', path: 'group_path', type: 'Group') + routes.create!(source_id: group.id, source_type: 'Group', name: 'group', path: 'group_path') + + groups.where(id: group.id).update_all(deleted_at: 1.year.ago) + + group + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index bc9cce6b0c3..a2f8fac2f38 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2339,6 +2339,22 @@ describe Project do end end + describe '#any_lfs_file_locks?', :request_store do + set(:project) { create(:project) } + + it 'returns false when there are no LFS file locks' do + expect(project.any_lfs_file_locks?).to be_falsey + end + + it 'returns a cached true when there are LFS file locks' do + create(:lfs_file_lock, project: project) + + expect(project.lfs_file_locks).to receive(:any?).once.and_call_original + + 2.times { expect(project.any_lfs_file_locks?).to be_truthy } + end + end + describe '#protected_for?' do let(:project) { create(:project) } diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 7d923932309..da23fdd7dca 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -138,10 +138,15 @@ describe API::Groups do context "when using sorting" do let(:group3) { create(:group, name: "a#{group1.name}", path: "z#{group1.path}") } + let(:group4) { create(:group, name: "same-name", path: "y#{group1.path}") } + let(:group5) { create(:group, name: "same-name") } let(:response_groups) { json_response.map { |group| group['name'] } } + let(:response_groups_ids) { json_response.map { |group| group['id'] } } before do group3.add_owner(user1) + group4.add_owner(user1) + group5.add_owner(user1) end it "sorts by name ascending by default" do @@ -150,7 +155,7 @@ describe API::Groups do expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(response_groups).to eq([group3.name, group1.name]) + expect(response_groups).to eq(Group.visible_to_user(user1).order(:name).pluck(:name)) end it "sorts in descending order when passed" do @@ -159,16 +164,52 @@ describe API::Groups do expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(response_groups).to eq([group1.name, group3.name]) + expect(response_groups).to eq(Group.visible_to_user(user1).order(name: :desc).pluck(:name)) end - it "sorts by the order_by param" do + it "sorts by path in order_by param" do get api("/groups", user1), order_by: "path" expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(response_groups).to eq([group1.name, group3.name]) + expect(response_groups).to eq(Group.visible_to_user(user1).order(:path).pluck(:name)) + end + + it "sorts by id in the order_by param" do + get api("/groups", user1), order_by: "id" + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(response_groups).to eq(Group.visible_to_user(user1).order(:id).pluck(:name)) + end + + it "sorts also by descending id with pagination fix" do + get api("/groups", user1), order_by: "id", sort: "desc" + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(response_groups).to eq(Group.visible_to_user(user1).order(id: :desc).pluck(:name)) + end + + it "sorts identical keys by id for good pagination" do + get api("/groups", user1), search: "same-name", order_by: "name" + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(response_groups_ids).to eq(Group.select { |group| group['name'] == 'same-name' }.map { |group| group['id'] }.sort) + end + + it "sorts descending identical keys by id for good pagination" do + get api("/groups", user1), search: "same-name", order_by: "name", sort: "desc" + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(response_groups_ids).to eq(Group.select { |group| group['name'] == 'same-name' }.map { |group| group['id'] }.sort) end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 16e6f19773f..e7639599874 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -351,11 +351,13 @@ describe API::Runner, :clean_gitlab_redis_shared_state do context 'when valid token is provided' do context 'when Runner is not active' do let(:runner) { create(:ci_runner, :inactive) } + let(:update_value) { runner.ensure_runner_queue_value } it 'returns 204 error' do request_job - expect(response).to have_gitlab_http_status 204 + expect(response).to have_gitlab_http_status(204) + expect(response.header['X-GitLab-Last-Update']).to eq(update_value) end end diff --git a/spec/support/helpers/features/notes_helpers.rb b/spec/support/helpers/features/notes_helpers.rb index 1a1d5853a7a..2b9f8b30c60 100644 --- a/spec/support/helpers/features/notes_helpers.rb +++ b/spec/support/helpers/features/notes_helpers.rb @@ -13,7 +13,7 @@ module Spec module Features module NotesHelpers def add_note(text) - Sidekiq::Testing.fake! do + perform_enqueued_jobs do page.within(".js-main-target-form") do fill_in("note[note]", with: text) find(".js-comment-submit-button").click diff --git a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb index 2228e872926..7c34c7b4977 100644 --- a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb @@ -245,6 +245,70 @@ RSpec.shared_examples 'slack or mattermost notifications' do end end + describe 'Push events' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository, creator: user) } + + before do + allow(chat_service).to receive_messages( + project: project, + service_hook: true, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url) + end + + context 'only notify for the default branch' do + context 'when enabled' do + before do + chat_service.notify_only_default_branch = true + end + + it 'does not notify push events if they are not for the default branch' do + ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test" + push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, []) + + chat_service.execute(push_sample_data) + + expect(WebMock).not_to have_requested(:post, webhook_url) + end + + it 'notifies about push events for the default branch' do + push_sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) + + chat_service.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + + it 'still notifies about pushed tags' do + ref = "#{Gitlab::Git::TAG_REF_PREFIX}test" + push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, []) + + chat_service.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when disabled' do + before do + chat_service.notify_only_default_branch = false + end + + it 'notifies about all push events' do + ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test" + push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, []) + + chat_service.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + end + end + describe "Note events" do let(:user) { create(:user) } let(:project) { create(:project, :repository, creator: user) } @@ -394,23 +458,6 @@ RSpec.shared_examples 'slack or mattermost notifications' do expect(result).to be_falsy end - - it 'does not notify push events if they are not for the default branch' do - ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test" - push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, []) - - chat_service.execute(push_sample_data) - - expect(WebMock).not_to have_requested(:post, webhook_url) - end - - it 'notifies about push events for the default branch' do - push_sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) - - chat_service.execute(push_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end end context 'when disabled' do diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 9e3b99b3502..2106959e23c 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -13,7 +13,7 @@ describe 'Every Sidekiq worker' do file_worker_queues = Gitlab::SidekiqConfig.worker_queues.to_set worker_queues = Gitlab::SidekiqConfig.workers.map(&:queue).to_set - worker_queues << ActionMailer::DeliveryJob.queue_name + worker_queues << ActionMailer::DeliveryJob.new.queue_name worker_queues << 'default' missing_from_file = worker_queues - file_worker_queues diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 5d83397e8df..ac8716ecfb1 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -92,16 +92,6 @@ describe RepositoryForkWorker do end it_behaves_like 'RepositoryForkWorker performing' - - it 'logs a message about forking with old-style arguments' do - allow(subject).to receive(:gitlab_shell).and_return(shell) - expect(shell).to receive(:fork_repository) { true } - - allow(Rails.logger).to receive(:info).with(anything) # To compensate for other logs - expect(Rails.logger).to receive(:info).with("Project #{fork_project.id} is being forked using old-style arguments.") - - perform! - end end end end |
