diff options
74 files changed, 798 insertions, 267 deletions
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/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..0d8e867f41d 100644 --- a/app/assets/stylesheets/bootstrap_migration.scss +++ b/app/assets/stylesheets/bootstrap_migration.scss @@ -173,7 +173,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 +211,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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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 |