diff options
58 files changed, 1367 insertions, 218 deletions
@@ -219,7 +219,7 @@ gem 'oj', '~> 2.17.4' gem 'chronic', '~> 0.10.2' gem 'chronic_duration', '~> 0.10.6' -gem 'sassc-rails', '~> 1.3.0' +gem 'sass-rails', '~> 5.0.6' gem 'coffee-rails', '~> 4.1.0' gem 'uglifier', '~> 2.7.2' gem 'gitlab-turbolinks-classic', '~> 2.5', '>= 2.5.6' diff --git a/Gemfile.lock b/Gemfile.lock index 4d025683b20..230d1785aa3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -667,17 +667,12 @@ GEM sanitize (2.1.0) nokogiri (>= 1.4.4) sass (3.4.22) - sassc (1.11.1) - bundler - ffi (~> 1.9.6) - sass (>= 3.3.0) - sassc-rails (1.3.0) - railties (>= 4.0.0) - sass - sassc (~> 1.9) - sprockets (> 2.11) - sprockets-rails - tilt + sass-rails (5.0.6) + railties (>= 4.0.0, < 6) + sass (~> 3.1) + sprockets (>= 2.8, < 4.0) + sprockets-rails (>= 2.0, < 4.0) + tilt (>= 1.1, < 3) sawyer (0.8.1) addressable (>= 2.3.5, < 2.6) faraday (~> 0.8, < 1.0) @@ -976,7 +971,7 @@ DEPENDENCIES ruby-prof (~> 0.16.2) rugged (~> 0.24.0) sanitize (~> 2.0) - sassc-rails (~> 1.3.0) + sass-rails (~> 5.0.6) scss_lint (~> 0.47.0) seed-fu (~> 2.3.5) select2-rails (~> 3.5.9) diff --git a/app/assets/javascripts/gfm_auto_complete.js.es6 b/app/assets/javascripts/gfm_auto_complete.js.es6 index 6ca543c2b00..358c1e52178 100644 --- a/app/assets/javascripts/gfm_auto_complete.js.es6 +++ b/app/assets/javascripts/gfm_auto_complete.js.es6 @@ -48,8 +48,9 @@ }, DefaultOptions: { sorter: function(query, items, searchKey) { - this.setting.highlightFirst = query.length > 0; + this.setting.highlightFirst = this.setting.alwaysHighlightFirst || query.length > 0; if (gl.GfmAutoComplete.isLoading(items)) { + this.setting.highlightFirst = false; return items; } return $.fn.atwho["default"].callbacks.sorter(query, items, searchKey); diff --git a/app/assets/javascripts/project_new.js b/app/assets/javascripts/project_new.js index 7fc611d0dad..86d57d97eae 100644 --- a/app/assets/javascripts/project_new.js +++ b/app/assets/javascripts/project_new.js @@ -14,11 +14,21 @@ return $('.save-project-loader').show(); }; })(this)); + + this.initVisibilitySelect(); + this.toggleSettings(); this.toggleSettingsOnclick(); this.toggleRepoVisibility(); } + ProjectNew.prototype.initVisibilitySelect = function() { + const visibilityContainer = document.querySelector('.js-visibility-select'); + if (!visibilityContainer) return; + const visibilitySelect = new gl.VisibilitySelect(visibilityContainer); + visibilitySelect.init(); + }; + ProjectNew.prototype.toggleSettings = function() { var self = this; diff --git a/app/assets/javascripts/visibility_select.js.es6 b/app/assets/javascripts/visibility_select.js.es6 new file mode 100644 index 00000000000..f712d7ba930 --- /dev/null +++ b/app/assets/javascripts/visibility_select.js.es6 @@ -0,0 +1,27 @@ +(() => { + const gl = window.gl || (window.gl = {}); + + class VisibilitySelect { + constructor(container) { + if (!container) throw new Error('VisibilitySelect requires a container element as argument 1'); + this.container = container; + this.helpBlock = this.container.querySelector('.help-block'); + this.select = this.container.querySelector('select'); + } + + init() { + if (this.select) { + this.updateHelpText(); + this.select.addEventListener('change', this.updateHelpText.bind(this)); + } else { + this.helpBlock.textContent = this.container.querySelector('.js-locked').dataset.helpBlock; + } + } + + updateHelpText() { + this.helpBlock.textContent = this.select.querySelector('option:checked').dataset.description; + } + } + + gl.VisibilitySelect = VisibilitySelect; +})(); diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index e04a87a7327..bb6129158d9 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -324,7 +324,7 @@ &:focus { cursor: text; box-shadow: none; - border-color: $border-color; + border-color: lighten($dropdown-input-focus-border, 20%); color: $gray-darkest; background-color: $gray-light; } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 9455ba3b98a..131cf23299a 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -9,17 +9,17 @@ .new_project, .edit-project { - fieldset { - - &.features { + .sharing-and-permissions { + .header { + padding-top: $gl-vert-padding; + } - .label-light { - margin-bottom: 0; - } + .label-light { + margin-bottom: 0; + } - .help-block { - margin-top: 0; - } + .help-block { + margin-top: 0; } .form-group { @@ -905,10 +905,18 @@ pre.light-well { } } -.project-feature-nested { +.project-feature { + padding-top: 10px; + @media (min-width: $screen-sm-min) { padding-left: 45px; } + + &.nested { + @media (min-width: $screen-sm-min) { + padding-left: 90px; + } + } } .project-repo-select { diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index fbe391fc58c..9b45ed6b6af 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -94,7 +94,7 @@ class Projects::BuildsController < Projects::ApplicationController private def build - @build ||= project.builds.find_by!(id: params[:id]) + @build ||= project.builds.find_by!(id: params[:id]).present(user: current_user) end def build_path(build) diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 791ed88db30..bfc59bcc862 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -12,7 +12,6 @@ class Projects::CommitController < Projects::ApplicationController before_action :authorize_read_pipeline!, only: [:pipelines] before_action :commit before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines] - before_action :define_status_vars, only: [:show, :pipelines] before_action :define_note_vars, only: [:show, :diff_for_path] before_action :authorize_edit_tree!, only: [:revert, :cherry_pick] @@ -106,10 +105,6 @@ class Projects::CommitController < Projects::ApplicationController } end - def define_status_vars - @ci_pipelines = project.pipelines.where(sha: commit.sha) - end - def assign_change_commit_vars(mr_source_branch) @commit = project.commit(params[:id]) @target_branch = params[:target_branch] diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index c6c63918fa5..eb98204285d 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -409,4 +409,15 @@ module ProjectsHelper def project_issues(project) IssuesFinder.new(current_user, project_id: project.id).execute end + + def visibility_select_options(project, selected_level) + levels_options_array = Gitlab::VisibilityLevel.values.map do |level| + [ + visibility_level_label(level), + { data: { description: visibility_level_description(level, project) } }, + level + ] + end + options_for_select(levels_options_array, selected_level) + end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 48ffe40abc6..ce23c2d1088 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -2,6 +2,7 @@ module Ci class Build < CommitStatus include TokenAuthenticatable include AfterCommitQueue + include Presentable belongs_to :runner belongs_to :trigger_request diff --git a/app/models/concerns/presentable.rb b/app/models/concerns/presentable.rb new file mode 100644 index 00000000000..7b33b837004 --- /dev/null +++ b/app/models/concerns/presentable.rb @@ -0,0 +1,7 @@ +module Presentable + def present(**attributes) + Gitlab::View::Presenter::Factory + .new(self, attributes) + .fabricate! + end +end diff --git a/app/models/generic_commit_status.rb b/app/models/generic_commit_status.rb index fa54e3540d0..8867ba0d2ff 100644 --- a/app/models/generic_commit_status.rb +++ b/app/models/generic_commit_status.rb @@ -1,6 +1,10 @@ class GenericCommitStatus < CommitStatus before_validation :set_default_values + validates :target_url, addressable_url: true, + length: { maximum: 255 }, + allow_nil: true + # GitHub compatible API alias_attribute :context, :name @@ -12,4 +16,10 @@ class GenericCommitStatus < CommitStatus def tags [:external] end + + def detailed_status(current_user) + Gitlab::Ci::Status::External::Factory + .new(self, current_user) + .fabricate! + end end diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 118c100ca11..b9f1c29c32e 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -53,6 +53,10 @@ class BasePolicy def self.class_for(subject) return GlobalPolicy if subject.nil? + if subject.class.try(:presenter?) + subject = subject.subject + end + subject.class.ancestors.each do |klass| next unless klass.name diff --git a/app/presenters/README.md b/app/presenters/README.md new file mode 100644 index 00000000000..3edd63451e7 --- /dev/null +++ b/app/presenters/README.md @@ -0,0 +1,154 @@ +# Presenters + +This type of class is responsible for giving the view an object which defines +**view-related logic/data methods**. It is usually useful to extract such +methods from models to presenters. + +## When to use a presenter? + +### When your view is full of logic + +When your view is full of logic (`if`, `else`, `select` on arrays etc.), it's +time to create a presenter! + +### When your model has a lot of view-related logic/data methods + +When your model has a lot of view-related logic/data methods, you can easily +move them to a presenter. + +## Why are we using presenters instead of helpers? + +We don't use presenters to generate complex view output that would rely on helpers. + +Presenters should be used for: + +- Data and logic methods that can be pulled & combined into single methods from + view. This can include loops extracted from views too. A good example is + https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7073/diffs. +- Data and logic methods that can be pulled from models. +- Simple text output methods: it's ok if the method returns a string, but not a + whole DOM element for which we'd need HAML, a view context, helpers etc. + +## Why use presenters instead of model concerns? + +We should strive to follow the single-responsibility principle, and view-related +logic/data methods are definitely not the responsibility of models! + +Another reason is as follows: + +> Avoid using concerns and use presenters instead. Why? After all, concerns seem +to be a core part of Rails and can DRY up code when shared among multiple models. +Nonetheless, the main issue is that concerns don’t make the model object more +cohesive. The code is just better organized. In other words, there’s no real +change to the API of the model. + +– https://www.toptal.com/ruby-on-rails/decoupling-rails-components + +## Benefits + +By moving pure view-related logic/data methods from models & views to presenters, +we gain the following benefits: + +- rules are more explicit and centralized in the presenter => improves security +- testing is easier and faster as presenters are Plain Old Ruby Object (PORO) +- views are more readable and maintainable +- decreases number of CE -> EE merge conflicts since code is in separate files +- moves the conflicts from views (not always obvious) to presenters (a lot easier to resolve) + +## What not to do with presenters? + +- Don't use helpers in presenters. Presenters are not aware of the view context. +- Don't generate complex DOM elements, forms etc. with presenters. Presenters + can return simple data as texts, and URLs using URL helpers from + `Gitlab::Routing` but nothing much more fancy. + +## Implementation + +### Presenter definition + +Every presenter should inherit from `Gitlab::View::Presenter::Simple`, which +provides a `.presents` method which allows you to define an accessor for the +presented object. It also includes common helpers like `Gitlab::Routing` and +`Gitlab::Allowable`. + +```ruby +class LabelPresenter < Gitlab::View::Presenter::Simple + presents :label + + def text_color + label.color.to_s + end + + def to_partial_path + 'projects/labels/show' + end +end +``` + +In some cases, it can be more practical to transparently delegate all missing +method calls to the presented object, in these cases, you can make your +presenter inherit from `Gitlab::View::Presenter::Delegated`: + +```ruby +class LabelPresenter < Gitlab::View::Presenter::Delegated + presents :label + + def text_color + # color is delegated to label + color.to_s + end + + def to_partial_path + 'projects/labels/show' + end +end +``` + +### Presenter instantiation + +Instantiation must be done via the `Gitlab::View::Presenter::Factory` class which +detects the presenter based on the presented subject's class. + +```ruby +class Projects::LabelsController < Projects::ApplicationController + def edit + @label = Gitlab::View::Presenter::Factory + .new(@label, user: current_user) + .fabricate! + end +end +``` + +You can also include the `Presentable` concern in the model: + +```ruby +class Label + include Presentable +end +``` + +and then in the controller: + +```ruby +class Projects::LabelsController < Projects::ApplicationController + def edit + @label = @label.present(user: current_user) + end +end +``` + +### Presenter usage + +```ruby +%div{ class: @label.text_color } + = render partial: @label, label: @label +``` + +You can also present the model in the view: + +```ruby +- label = @label.present(current_user) + +%div{ class: label.text_color } + = render partial: label, label: label +``` diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb new file mode 100644 index 00000000000..ed72ed14d72 --- /dev/null +++ b/app/presenters/ci/build_presenter.rb @@ -0,0 +1,15 @@ +module Ci + class BuildPresenter < Gitlab::View::Presenter::Delegated + presents :build + + def erased_by_user? + # Build can be erased through API, therefore it does not have + # `erased_by` user assigned in that case. + erased? && erased_by + end + + def erased_by_name + erased_by.name if erased_by_user? + end + end +end diff --git a/app/views/projects/_visibility_select.html.haml b/app/views/projects/_visibility_select.html.haml new file mode 100644 index 00000000000..65fc0a36ca9 --- /dev/null +++ b/app/views/projects/_visibility_select.html.haml @@ -0,0 +1,7 @@ +- if can_change_visibility_level?(@project, current_user) + = form.select(model_method, visibility_select_options(@project, selected_level), {}, class: 'form-control visibility-select') +- else + .info.js-locked{ data: { help_block: visibility_level_description(@project.visibility_level, @project) } } + = visibility_level_icon(@project.visibility_level) + %strong + = visibility_level_label(@project.visibility_level) diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index 54724ef5cab..c613e473e4c 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -51,8 +51,10 @@ .prepend-top-default - if @build.erased? .erased.alert.alert-warning - - erased_by = "by #{link_to @build.erased_by.name, user_path(@build.erased_by)}" if @build.erased_by - Build has been erased #{erased_by.html_safe} #{time_ago_with_tooltip(@build.erased_at)} + - if @build.erased_by_user? + Build has been erased by #{link_to(@build.erased_by_name, user_path(@build.erased_by))} #{time_ago_with_tooltip(@build.erased_at)} + - else + Build has been erased #{time_ago_with_tooltip(@build.erased_at)} - else #js-build-scroll.scroll-controls .scroll-step diff --git a/app/views/projects/commit/_ci_menu.html.haml b/app/views/projects/commit/_ci_menu.html.haml index 13ab2253733..8aed88da38b 100644 --- a/app/views/projects/commit/_ci_menu.html.haml +++ b/app/views/projects/commit/_ci_menu.html.haml @@ -7,4 +7,4 @@ = nav_link(path: 'commit#pipelines') do = link_to pipelines_namespace_project_commit_path(@project.namespace, @project, @commit.id) do Pipelines - %span.badge= @ci_pipelines.count + %span.badge= @commit.pipelines.size diff --git a/app/views/projects/commit/pipelines.html.haml b/app/views/projects/commit/pipelines.html.haml index 8233e26e4e7..00e7cdd1729 100644 --- a/app/views/projects/commit/pipelines.html.haml +++ b/app/views/projects/commit/pipelines.html.haml @@ -3,4 +3,4 @@ = render "commit_box" = render "ci_menu" -= render "pipelines_list", pipelines: @ci_pipelines += render "pipelines_list", pipelines: @commit.pipelines.order(id: :desc) diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 38e7fc4279c..1d7fdf68cb3 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -21,76 +21,68 @@ .form-group = f.label :default_branch, "Default Branch", class: 'label-light' = f.select(:default_branch, @project.repository.branch_names, {}, {class: 'select2 select-wide'}) - .form-group.project-visibility-level-holder - = f.label :visibility_level, class: 'label-light' do - Visibility Level - = link_to "(?)", help_page_path("public_access/public_access") - - if can_change_visibility_level?(@project, current_user) - = render('shared/visibility_radios', model_method: :visibility_level, form: f, selected_level: @project.visibility_level, form_model: @project) - - else - .info - = visibility_level_icon(@project.visibility_level) - %strong - = visibility_level_label(@project.visibility_level) - .light= visibility_level_description(@project.visibility_level, @project) - - .form-group - = render 'shared/allow_request_access', form: f - .form-group = f.label :tag_list, "Tags", class: 'label-light' = f.text_field :tag_list, value: @project.tag_list.to_s, maxlength: 2000, class: "form-control" %p.help-block Separate tags with commas. %hr - %fieldset.features.append-bottom-0 + %fieldset.append-bottom-0 %h5.prepend-top-0 - Feature Visibility - - = f.fields_for :project_feature do |feature_fields| - .form_group.prepend-top-20 - .row - .col-md-9 - = feature_fields.label :repository_access_level, "Repository", class: 'label-light' - %span.help-block Push files to be stored in this project - .col-md-3.js-repo-access-level - = project_feature_access_select(:repository_access_level) - - .col-sm-12 - .row - .col-md-9.project-feature-nested - = feature_fields.label :merge_requests_access_level, "Merge requests", class: 'label-light' - %span.help-block Submit changes to be merged upstream - .col-md-3 - = project_feature_access_select(:merge_requests_access_level) + Sharing & Permissions + .form_group.prepend-top-20.sharing-and-permissions + .row.js-visibility-select + .col-md-9 + %label.label-light + = label_tag :project_visibility, 'Project Visibility', class: 'label-light' + = link_to "(?)", help_page_path("public_access/public_access") + %span.help-block + .col-md-3.visibility-select-container + = render('projects/visibility_select', model_method: :visibility_level, form: f, selected_level: @project.visibility_level) + = f.fields_for :project_feature do |feature_fields| + %fieldset.features + .row + .col-md-9.project-feature + = feature_fields.label :repository_access_level, "Repository", class: 'label-light' + %span.help-block View and edit files in this project + .col-md-3.js-repo-access-level + = project_feature_access_select(:repository_access_level) - .row - .col-md-9.project-feature-nested - = feature_fields.label :builds_access_level, "Builds", class: 'label-light' - %span.help-block Submit, test and deploy your changes before merge - .col-md-3 - = project_feature_access_select(:builds_access_level) + .row + .col-md-9.project-feature.nested + = feature_fields.label :merge_requests_access_level, "Merge requests", class: 'label-light' + %span.help-block Submit changes to be merged upstream + .col-md-3 + = project_feature_access_select(:merge_requests_access_level) - .row - .col-md-9 - = feature_fields.label :snippets_access_level, "Snippets", class: 'label-light' - %span.help-block Share code pastes with others out of Git repository - .col-md-3 - = project_feature_access_select(:snippets_access_level) + .row + .col-md-9.project-feature.nested + = feature_fields.label :builds_access_level, "Builds", class: 'label-light' + %span.help-block Submit, test and deploy your changes before merge + .col-md-3 + = project_feature_access_select(:builds_access_level) - .row - .col-md-9 - = feature_fields.label :issues_access_level, "Issues", class: 'label-light' - %span.help-block Lightweight issue tracking system for this project - .col-md-3 - = project_feature_access_select(:issues_access_level) + .row + .col-md-9.project-feature + = feature_fields.label :snippets_access_level, "Snippets", class: 'label-light' + %span.help-block Share code pastes with others out of Git repository + .col-md-3 + = project_feature_access_select(:snippets_access_level) - .row - .col-md-9 - = feature_fields.label :wiki_access_level, "Wiki", class: 'label-light' - %span.help-block Pages for project documentation - .col-md-3 - = project_feature_access_select(:wiki_access_level) + .row + .col-md-9.project-feature + = feature_fields.label :issues_access_level, "Issues", class: 'label-light' + %span.help-block Lightweight issue tracking system for this project + .col-md-3 + = project_feature_access_select(:issues_access_level) + .row + .col-md-9.project-feature + = feature_fields.label :wiki_access_level, "Wiki", class: 'label-light' + %span.help-block Pages for project documentation + .col-md-3 + = project_feature_access_select(:wiki_access_level) + .form-group + = render 'shared/allow_request_access', form: f - if Gitlab.config.lfs.enabled && current_user.admin? .row .col-md-9 diff --git a/app/views/projects/services/mattermost_slash_commands/_help.html.haml b/app/views/projects/services/mattermost_slash_commands/_help.html.haml index 63b797cd391..c1e576b42fc 100644 --- a/app/views/projects/services/mattermost_slash_commands/_help.html.haml +++ b/app/views/projects/services/mattermost_slash_commands/_help.html.haml @@ -8,8 +8,8 @@ by entering %code /<command_trigger_word> help - - unless enabled + - unless enabled || @service.template? = render 'projects/services/mattermost_slash_commands/detailed_help', subject: @service -- if enabled +- if enabled && !@service.template? = render 'projects/services/mattermost_slash_commands/installation_info', subject: @service diff --git a/app/views/projects/services/slack_slash_commands/_help.html.haml b/app/views/projects/services/slack_slash_commands/_help.html.haml index 6d7c2defe2b..04b9100acc6 100644 --- a/app/views/projects/services/slack_slash_commands/_help.html.haml +++ b/app/views/projects/services/slack_slash_commands/_help.html.haml @@ -1,4 +1,5 @@ -- run_actions_text = "Perform common operations on this project: #{@project.name_with_namespace}" +- pretty_name = defined?(@project) ? @project.name_with_namespace : "namespace / path" +- run_actions_text = "Perform common operations on this project: #{pretty_name}" .well This service allows GitLab users to perform common operations on this @@ -9,85 +10,86 @@ %code /<command> help %br %br - To setup this service: - %ul.list-unstyled - %li - 1. - = link_to 'Add a slash command', 'https://my.slack.com/services/new/slash-commands' - in your Slack team with these options: + - unless @service.template? + To setup this service: + %ul.list-unstyled + %li + 1. + = link_to 'Add a slash command', 'https://my.slack.com/services/new/slash-commands' + in your Slack team with these options: - %hr + %hr - .help-form - .form-group - = label_tag nil, 'Command', class: 'col-sm-2 col-xs-12 control-label' - .col-sm-10.col-xs-12.text-block - %p Fill in the word that works best for your team. - %p - Suggestions: - %code= 'gitlab' - %code= @project.path # Path contains no spaces, but dashes - %code= @project.path_with_namespace + .help-form + .form-group + = label_tag nil, 'Command', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.text-block + %p Fill in the word that works best for your team. + %p + Suggestions: + %code= 'gitlab' + %code= @project.path # Path contains no spaces, but dashes + %code= @project.path_with_namespace - .form-group - = label_tag :url, 'URL', class: 'col-sm-2 col-xs-12 control-label' - .col-sm-10.col-xs-12.input-group - = text_field_tag :url, service_trigger_url(subject), class: 'form-control input-sm', readonly: 'readonly' - .input-group-btn - = clipboard_button(clipboard_target: '#url') + .form-group + = label_tag :url, 'URL', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :url, service_trigger_url(subject), class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#url') - .form-group - = label_tag nil, 'Method', class: 'col-sm-2 col-xs-12 control-label' - .col-sm-10.col-xs-12.text-block POST + .form-group + = label_tag nil, 'Method', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.text-block POST - .form-group - = label_tag :customize_name, 'Customize name', class: 'col-sm-2 col-xs-12 control-label' - .col-sm-10.col-xs-12.input-group - = text_field_tag :customize_name, 'GitLab', class: 'form-control input-sm', readonly: 'readonly' - .input-group-btn - = clipboard_button(clipboard_target: '#customize_name') + .form-group + = label_tag :customize_name, 'Customize name', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :customize_name, 'GitLab', class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#customize_name') - .form-group - = label_tag nil, 'Customize icon', class: 'col-sm-2 col-xs-12 control-label' - .col-sm-10.col-xs-12.text-block - = image_tag(asset_url('slash-command-logo.png'), width: 36, height: 36) - = link_to('Download image', asset_url('gitlab_logo.png'), class: 'btn btn-sm', target: '_blank') + .form-group + = label_tag nil, 'Customize icon', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.text-block + = image_tag(asset_url('slash-command-logo.png'), width: 36, height: 36) + = link_to('Download image', asset_url('gitlab_logo.png'), class: 'btn btn-sm', target: '_blank') - .form-group - = label_tag nil, 'Autocomplete', class: 'col-sm-2 col-xs-12 control-label' - .col-sm-10.col-xs-12.text-block Show this command in the autocomplete list + .form-group + = label_tag nil, 'Autocomplete', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.text-block Show this command in the autocomplete list - .form-group - = label_tag :autocomplete_description, 'Autocomplete description', class: 'col-sm-2 col-xs-12 control-label' - .col-sm-10.col-xs-12.input-group - = text_field_tag :autocomplete_description, run_actions_text, class: 'form-control input-sm', readonly: 'readonly' - .input-group-btn - = clipboard_button(clipboard_target: '#autocomplete_description') + .form-group + = label_tag :autocomplete_description, 'Autocomplete description', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :autocomplete_description, run_actions_text, class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#autocomplete_description') - .form-group - = label_tag :autocomplete_usage_hint, 'Autocomplete usage hint', class: 'col-sm-2 col-xs-12 control-label' - .col-sm-10.col-xs-12.input-group - = text_field_tag :autocomplete_usage_hint, '[help]', class: 'form-control input-sm', readonly: 'readonly' - .input-group-btn - = clipboard_button(clipboard_target: '#autocomplete_usage_hint') + .form-group + = label_tag :autocomplete_usage_hint, 'Autocomplete usage hint', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :autocomplete_usage_hint, '[help]', class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#autocomplete_usage_hint') - .form-group - = label_tag :descriptive_label, 'Descriptive label', class: 'col-sm-2 col-xs-12 control-label' - .col-sm-10.col-xs-12.input-group - = text_field_tag :descriptive_label, 'Perform common operations on GitLab project', class: 'form-control input-sm', readonly: 'readonly' - .input-group-btn - = clipboard_button(clipboard_target: '#descriptive_label') + .form-group + = label_tag :descriptive_label, 'Descriptive label', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :descriptive_label, 'Perform common operations on GitLab project', class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#descriptive_label') - %hr + %hr - %ul.list-unstyled - %li - 2. Paste the - %strong Token - into the field below - %li - 3. Select the - %strong Active - checkbox, press - %strong Save changes - and start using GitLab inside Slack! + %ul.list-unstyled + %li + 2. Paste the + %strong Token + into the field below + %li + 3. Select the + %strong Active + checkbox, press + %strong Save changes + and start using GitLab inside Slack! diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml index 51c195ffbcd..28935c8b598 100644 --- a/app/views/shared/milestones/_issuable.html.haml +++ b/app/views/shared/milestones/_issuable.html.haml @@ -16,7 +16,7 @@ = link_to_gfm issuable.title, [project.namespace.becomes(Namespace), project, issuable], title: issuable.title .issuable-detail = link_to [project.namespace.becomes(Namespace), project, issuable] do - %span.issuable-number >= issuable.to_reference + %span.issuable-number= issuable.to_reference - issuable.labels.each do |label| = link_to polymorphic_path(base_url_args, { milestone_title: @milestone.title, label_name: label.title, state: 'all' }) do diff --git a/app/views/shared/milestones/_issuables.html.haml b/app/views/shared/milestones/_issuables.html.haml index c8fd45c4319..31eb07ca666 100644 --- a/app/views/shared/milestones/_issuables.html.haml +++ b/app/views/shared/milestones/_issuables.html.haml @@ -8,8 +8,7 @@ = title - if show_counter .right - = issuables.size - .pull-right= number_with_delimiter(issuables.size) + = number_with_delimiter(issuables.size) - class_prefix = dom_class(issuables).pluralize %ul{ class: "well-list #{class_prefix}-sortable-list", id: "#{class_prefix}-list-#{id}", "data-state" => id } diff --git a/changelogs/unreleased/24032-changed-visibility-level-to-public-but-project-is-not-public.yml b/changelogs/unreleased/24032-changed-visibility-level-to-public-but-project-is-not-public.yml new file mode 100644 index 00000000000..875106d7dd5 --- /dev/null +++ b/changelogs/unreleased/24032-changed-visibility-level-to-public-but-project-is-not-public.yml @@ -0,0 +1,4 @@ +--- +title: Updated project visibility settings UX +merge_request: 7645 +author: diff --git a/changelogs/unreleased/26117-sort-pipeline-for-commit.yml b/changelogs/unreleased/26117-sort-pipeline-for-commit.yml new file mode 100644 index 00000000000..b2f5294d380 --- /dev/null +++ b/changelogs/unreleased/26117-sort-pipeline-for-commit.yml @@ -0,0 +1,4 @@ +--- +title: Add sorting pipeline for a commit +merge_request: 8319 +author: Takuya Noguchi diff --git a/changelogs/unreleased/fix-external-status-badge-links.yml b/changelogs/unreleased/fix-external-status-badge-links.yml new file mode 100644 index 00000000000..2287a9b76c4 --- /dev/null +++ b/changelogs/unreleased/fix-external-status-badge-links.yml @@ -0,0 +1,4 @@ +--- +title: Link external build badge to its target URL +merge_request: 8611 +author: diff --git a/changelogs/unreleased/input-button-hover.yml b/changelogs/unreleased/input-button-hover.yml new file mode 100644 index 00000000000..cbb35adb769 --- /dev/null +++ b/changelogs/unreleased/input-button-hover.yml @@ -0,0 +1,4 @@ +--- +title: Add hover state to MR comment reply button +merge_request: +author: diff --git a/changelogs/unreleased/switch-to-sassc.yml b/changelogs/unreleased/switch-to-sassc.yml deleted file mode 100644 index 3e6c4baf6d9..00000000000 --- a/changelogs/unreleased/switch-to-sassc.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Switch to sassc-rails for faster stylesheet compilation -merge_request: 8556 -author: Richard Macklin diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 9e782ab977f..f79bd23dc90 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -297,16 +297,74 @@ For our currently-supported browsers, see our [requirements][requirements]. ## Gotchas -### Phantom.JS (used by Teaspoon & Rspec) chokes, returning vague JavaScript errors - -If you see very generic JavaScript errors (e.g. `jQuery is undefined`) being thrown in tests, but -can't reproduce them manually, you may have included `ES6`-style JavaScript in files that don't -have the `.js.es6` file extension. Either use ES5-friendly JavaScript or rename the file you're -working in (`git mv <file.js> <file.js.es6>`). - -Similar errors will be thrown if you're using -any of the [array methods introduced in ES6](http://www.2ality.com/2014/05/es6-array-methods.html) -whether or not you've updated the file extension. +### Spec errors due to use of ES6 features in `.js` files + +If you see very generic JavaScript errors (e.g. `jQuery is undefined`) being +thrown in Teaspoon, Spinach, or Rspec tests but can't reproduce them manually, +you may have included `ES6`-style JavaScript in files that don't have the +`.js.es6` file extension. Either use ES5-friendly JavaScript or rename the file +you're working in (`git mv <file.js> <file.js.es6>`). + +### Spec errors due to use of unsupported JavaScript + +Similar errors will be thrown if you're using JavaScript features not yet +supported by our test runner's version of webkit, whether or not you've updated +the file extension. Examples of unsupported JavaScript features are: + +- Array.from +- Array.find +- Array.first +- Object.assign +- Async functions +- Generators +- Array destructuring +- For Of +- Symbol/Symbol.iterator +- Spread + +Until these are polyfilled or transpiled appropriately, they should not be used. +Please update this list with additional unsupported features or when any of +these are made usable. + +### Spec errors due to JavaScript not enabled + +If, as a result of a change you've made, a feature now depends on JavaScript to +run correctly, you need to make sure a JavaScript web driver is enabled when +specs are run. If you don't you'll see vague error messages from the spec +runner, and an explosion of vague console errors in the HTML snapshot. + +To enable a JavaScript driver in an `rspec` test, add `js: true` to the +individual spec or the context block containing multiple specs that need +JavaScript enabled: + +```ruby + +# For one spec +it 'presents information about abuse report', js: true do + # assertions... +end + +describe "Admin::AbuseReports", js: true do + it 'presents information about abuse report' do + # assertions... + end + it 'shows buttons for adding to abuse report' do + # assertions... + end +end +``` +In Spinach, the JavaScript driver is enabled differently. In the `*.feature` +file for the failing spec, add the `@javascript` flag above the Scenario: +``` +@javascript +Scenario: Developer can approve merge request + Given I am a "Shop" developer + And I visit project "Shop" merge requests page + And merge request 'Bug NS-04' must be approved + And I click link "Bug NS-04" + When I click link "Approve" + Then I should see approved merge request "Bug NS-04" +``` diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index 4c933cef9b7..98a680d0dbe 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -41,6 +41,9 @@ that are in common for all providers that we need to consider. - `allow_single_sign_on` allows you to specify the providers you want to allow to automatically create an account. It defaults to `false`. If `false` users must be created manually or they will not be able to sign in via OmniAuth. +- `auto_link_ldap_user` can be used if you have [LDAP / ActiveDirectory](ldap.md) + integration enabled. It defaults to false. When enabled, users automatically + created through OmniAuth will be linked to their LDAP entry as well. - `block_auto_created_users` defaults to `true`. If `true` auto created users will be blocked by default and will have to be unblocked by an administrator before they are able to sign in. @@ -52,6 +55,10 @@ SAML, Shibboleth, Crowd or Google, or set it to `false` otherwise any user on the Internet will be able to successfully sign in to your GitLab without administrative approval. +>**Note:** +`auto_link_ldap_user` requires the `uid` of the user to be the same in both LDAP +and the OmniAuth provider. + To change these settings: * **For omnibus package** @@ -72,6 +79,7 @@ To change these settings: # using an array, e.g. ["saml", "twitter"], or as true/false to allow all providers or none. # User accounts will be created automatically when authentication was successful. gitlab_rails['omniauth_allow_single_sign_on'] = ['saml', 'twitter'] + gitlab_rails['omniauth_auto_link_ldap_user'] = true gitlab_rails['omniauth_block_auto_created_users'] = true ``` @@ -99,6 +107,8 @@ To change these settings: # User accounts will be created automatically when authentication was successful. allow_single_sign_on: ["saml", "twitter"] + auto_link_ldap_user: true + # Locks down those users until they have been cleared by the admin (default: true). block_auto_created_users: true ``` diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index 553de0345d5..7a6707a7dfb 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -97,7 +97,7 @@ module SharedProject step 'I should see project settings' do expect(current_path).to eq edit_namespace_project_path(@project.namespace, @project) expect(page).to have_content("Project name") - expect(page).to have_content("Feature Visibility") + expect(page).to have_content("Sharing & Permissions") end def current_project diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 4bbdf06a49c..b6e6820c3f4 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -78,6 +78,8 @@ module API description: params[:description] ) + render_validation_error!(status) if status.invalid? + begin case params[:state] when 'pending' diff --git a/lib/gitlab/ci/status/external/common.rb b/lib/gitlab/ci/status/external/common.rb new file mode 100644 index 00000000000..4969a350862 --- /dev/null +++ b/lib/gitlab/ci/status/external/common.rb @@ -0,0 +1,22 @@ +module Gitlab + module Ci + module Status + module External + module Common + def has_details? + subject.target_url.present? && + can?(user, :read_commit_status, subject) + end + + def details_path + subject.target_url + end + + def has_action? + false + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/external/factory.rb b/lib/gitlab/ci/status/external/factory.rb new file mode 100644 index 00000000000..07b15bd8d97 --- /dev/null +++ b/lib/gitlab/ci/status/external/factory.rb @@ -0,0 +1,13 @@ +module Gitlab + module Ci + module Status + module External + class Factory < Status::Factory + def self.common_helpers + Status::External::Common + end + end + end + end + end +end diff --git a/lib/gitlab/view/presenter/base.rb b/lib/gitlab/view/presenter/base.rb new file mode 100644 index 00000000000..83c8ba5c1cf --- /dev/null +++ b/lib/gitlab/view/presenter/base.rb @@ -0,0 +1,28 @@ +module Gitlab + module View + module Presenter + module Base + extend ActiveSupport::Concern + + include Gitlab::Routing + include Gitlab::Allowable + + attr_reader :subject + + def can?(user, action, overriden_subject = nil) + super(user, action, overriden_subject || subject) + end + + class_methods do + def presenter? + true + end + + def presents(name) + define_method(name) { subject } + end + end + end + end + end +end diff --git a/lib/gitlab/view/presenter/delegated.rb b/lib/gitlab/view/presenter/delegated.rb new file mode 100644 index 00000000000..f4d330c590e --- /dev/null +++ b/lib/gitlab/view/presenter/delegated.rb @@ -0,0 +1,19 @@ +module Gitlab + module View + module Presenter + class Delegated < SimpleDelegator + include Gitlab::View::Presenter::Base + + def initialize(subject, **attributes) + @subject = subject + + attributes.each do |key, value| + define_singleton_method(key) { value } + end + + super(subject) + end + end + end + end +end diff --git a/lib/gitlab/view/presenter/factory.rb b/lib/gitlab/view/presenter/factory.rb new file mode 100644 index 00000000000..d172d61e2c9 --- /dev/null +++ b/lib/gitlab/view/presenter/factory.rb @@ -0,0 +1,24 @@ +module Gitlab + module View + module Presenter + class Factory + def initialize(subject, **attributes) + @subject = subject + @attributes = attributes + end + + def fabricate! + presenter_class.new(subject, attributes) + end + + private + + attr_reader :subject, :attributes + + def presenter_class + "#{subject.class.name}Presenter".constantize + end + end + end + end +end diff --git a/lib/gitlab/view/presenter/simple.rb b/lib/gitlab/view/presenter/simple.rb new file mode 100644 index 00000000000..b7653a0f3cc --- /dev/null +++ b/lib/gitlab/view/presenter/simple.rb @@ -0,0 +1,17 @@ +module Gitlab + module View + module Presenter + class Simple + include Gitlab::View::Presenter::Base + + def initialize(subject, **attributes) + @subject = subject + + attributes.each do |key, value| + define_singleton_method(key) { value } + end + end + end + end + end +end diff --git a/spec/controllers/admin/services_controller_spec.rb b/spec/controllers/admin/services_controller_spec.rb new file mode 100644 index 00000000000..e5cdd52307e --- /dev/null +++ b/spec/controllers/admin/services_controller_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Admin::ServicesController do + let(:admin) { create(:admin) } + + before { sign_in(admin) } + + describe 'GET #edit' do + let!(:project) { create(:empty_project) } + + Service.available_services_names.each do |service_name| + context "#{service_name}" do + let!(:service) do + service_template = service_name.concat("_service").camelize.constantize + service_template.where(template: true).first_or_create + end + + it 'successfully displays the template' do + get :edit, id: service.id + + expect(response).to have_http_status(200) + end + end + end + end +end diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 646b097d74e..0fa06a38d2a 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -1,9 +1,10 @@ require 'spec_helper' describe Projects::CommitController do - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:commit) { project.commit("master") } + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:commit) { project.commit("master") } + let(:pipeline) { create(:ci_pipeline, project: project, commit: commit) } let(:master_pickable_sha) { '7d3b0f7cff5f37573aea97cebfd5692ea1689924' } let(:master_pickable_commit) { project.commit(master_pickable_sha) } @@ -309,4 +310,35 @@ describe Projects::CommitController do end end end + + describe 'GET pipelines' do + def get_pipelines(extra_params = {}) + params = { + namespace_id: project.namespace.to_param, + project_id: project.to_param + } + + get :pipelines, params.merge(extra_params) + end + + context 'when the commit exists' do + context 'when the commit has one or more pipelines' do + it 'shows pipelines' do + get_pipelines(id: commit.id) + + expect(response).to be_ok + end + end + end + + context 'when the commit does not exist' do + before do + get_pipelines(id: 'e7a412c8da9f6d0081a633a4a402dde1c4694ebd') + end + + it 'returns a 404' do + expect(response).to have_http_status(404) + end + end + end end diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb index 82c9bd0e6e6..31156fcf994 100644 --- a/spec/features/issues/gfm_autocomplete_spec.rb +++ b/spec/features/issues/gfm_autocomplete_spec.rb @@ -33,6 +33,45 @@ feature 'GFM autocomplete', feature: true, js: true do expect(page).not_to have_selector('.atwho-view') end + it 'doesnt select the first item for non-assignee dropdowns' do + page.within '.timeline-content-form' do + find('#note_note').native.send_keys('') + find('#note_note').native.send_keys(':') + end + + expect(page).to have_selector('.atwho-container') + + wait_for_ajax + + expect(find('#at-view-58')).not_to have_selector('.cur:first-of-type') + end + + it 'selects the first item for assignee dropdowns' do + page.within '.timeline-content-form' do + find('#note_note').native.send_keys('') + find('#note_note').native.send_keys('@') + end + + expect(page).to have_selector('.atwho-container') + + wait_for_ajax + + expect(find('#at-view-64')).to have_selector('.cur:first-of-type') + end + + it 'selects the first item for non-assignee dropdowns if a query is entered' do + page.within '.timeline-content-form' do + find('#note_note').native.send_keys('') + find('#note_note').native.send_keys(':1') + end + + expect(page).to have_selector('.atwho-container') + + wait_for_ajax + + expect(find('#at-view-58')).to have_selector('.cur:first-of-type') + end + context 'if a selected value has special characters' do it 'wraps the result in double quotes' do note = find('#note_note') diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 14e009daba8..e673ece37c3 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -11,18 +11,42 @@ describe 'Pipeline', :feature, :js do project.team << [user, :developer] end + shared_context 'pipeline builds' do + let!(:build_passed) do + create(:ci_build, :success, + pipeline: pipeline, stage: 'build', name: 'build') + end + + let!(:build_failed) do + create(:ci_build, :failed, + pipeline: pipeline, stage: 'test', name: 'test', commands: 'test') + end + + let!(:build_running) do + create(:ci_build, :running, + pipeline: pipeline, stage: 'deploy', name: 'deploy') + end + + let!(:build_manual) do + create(:ci_build, :manual, + pipeline: pipeline, stage: 'deploy', name: 'manual-build') + end + + let!(:build_external) do + create(:generic_commit_status, status: 'success', + pipeline: pipeline, + name: 'jenkins', + stage: 'external', + target_url: 'http://gitlab.com/status') + end + end + describe 'GET /:project/pipelines/:id' do + include_context 'pipeline builds' + let(:project) { create(:project) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) } - before do - @success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build') - @failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test') - @running = create(:ci_build, :running, pipeline: pipeline, stage: 'deploy', name: 'deploy') - @manual = create(:ci_build, :manual, pipeline: pipeline, stage: 'deploy', name: 'manual-build') - @external = create(:generic_commit_status, status: 'success', pipeline: pipeline, name: 'jenkins', stage: 'external') - end - before { visit namespace_project_pipeline_path(project.namespace, project, pipeline) } it 'shows the pipeline graph' do @@ -116,6 +140,7 @@ describe 'Pipeline', :feature, :js do it 'shows the success icon and the generic comit status build' do expect(page).to have_selector('.ci-status-icon-success') expect(page).to have_content('jenkins') + expect(page).to have_link('jenkins', href: 'http://gitlab.com/status') end end end @@ -157,26 +182,22 @@ describe 'Pipeline', :feature, :js do end describe 'GET /:project/pipelines/:id/builds' do + include_context 'pipeline builds' + let(:project) { create(:project) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) } before do - @success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build') - @failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test') - @running = create(:ci_build, :running, pipeline: pipeline, stage: 'deploy', name: 'deploy') - @manual = create(:ci_build, :manual, pipeline: pipeline, stage: 'deploy', name: 'manual-build') - @external = create(:generic_commit_status, status: 'success', pipeline: pipeline, name: 'jenkins', stage: 'external') + visit builds_namespace_project_pipeline_path(project.namespace, project, pipeline) end - before { visit builds_namespace_project_pipeline_path(project.namespace, project, pipeline)} - it 'shows a list of builds' do expect(page).to have_content('Test') - expect(page).to have_content(@success.id) + expect(page).to have_content(build_passed.id) expect(page).to have_content('Deploy') - expect(page).to have_content(@failed.id) - expect(page).to have_content(@running.id) - expect(page).to have_content(@external.id) + expect(page).to have_content(build_failed.id) + expect(page).to have_content(build_running.id) + expect(page).to have_content(build_external.id) expect(page).to have_content('Retry failed') expect(page).to have_content('Cancel running') expect(page).to have_link('Play') @@ -230,7 +251,7 @@ describe 'Pipeline', :feature, :js do end end - it { expect(@manual.reload).to be_pending } + it { expect(build_manual.reload).to be_pending } end end end diff --git a/spec/features/projects/settings/visibility_settings_spec.rb b/spec/features/projects/settings/visibility_settings_spec.rb new file mode 100644 index 00000000000..cef315ac9cd --- /dev/null +++ b/spec/features/projects/settings/visibility_settings_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +feature 'Visibility settings', feature: true, js: true do + let(:user) { create(:user) } + let(:project) { create(:project, namespace: user.namespace, visibility_level: 20) } + + context 'as owner' do + before do + login_as(user) + visit edit_namespace_project_path(project.namespace, project) + end + + scenario 'project visibility select is available' do + visibility_select_container = find('.js-visibility-select') + + expect(visibility_select_container.find('.visibility-select').value).to eq project.visibility_level.to_s + expect(visibility_select_container).to have_content 'The project can be cloned without any authentication.' + end + + scenario 'project visibility description updates on change' do + visibility_select_container = find('.js-visibility-select') + visibility_select = visibility_select_container.find('.visibility-select') + visibility_select.select('Private') + + expect(visibility_select.value).to eq '0' + expect(visibility_select_container).to have_content 'Project access must be granted explicitly to each user.' + end + end + + context 'as master' do + let(:master_user) { create(:user) } + + before do + project.team << [master_user, :master] + login_as(master_user) + visit edit_namespace_project_path(project.namespace, project) + end + + scenario 'project visibility is locked' do + visibility_select_container = find('.js-visibility-select') + + expect(visibility_select_container).not_to have_select '.visibility-select' + expect(visibility_select_container).to have_content 'Public' + expect(visibility_select_container).to have_content 'The project can be cloned without any authentication.' + end + end +end diff --git a/spec/javascripts/gfm_auto_complete_spec.js.es6 b/spec/javascripts/gfm_auto_complete_spec.js.es6 new file mode 100644 index 00000000000..6b48d82cb23 --- /dev/null +++ b/spec/javascripts/gfm_auto_complete_spec.js.es6 @@ -0,0 +1,65 @@ +//= require gfm_auto_complete +//= require jquery +//= require jquery.atwho + +const global = window.gl || (window.gl = {}); +const GfmAutoComplete = global.GfmAutoComplete; + +describe('GfmAutoComplete', function () { + describe('DefaultOptions.sorter', function () { + describe('assets loading', function () { + beforeEach(function () { + spyOn(GfmAutoComplete, 'isLoading').and.returnValue(true); + + this.atwhoInstance = { setting: {} }; + this.items = []; + + this.sorterValue = GfmAutoComplete.DefaultOptions.sorter + .call(this.atwhoInstance, '', this.items); + }); + + it('should disable highlightFirst', function () { + expect(this.atwhoInstance.setting.highlightFirst).toBe(false); + }); + + it('should return the passed unfiltered items', function () { + expect(this.sorterValue).toEqual(this.items); + }); + }); + + describe('assets finished loading', function () { + beforeEach(function () { + spyOn(GfmAutoComplete, 'isLoading').and.returnValue(false); + spyOn($.fn.atwho.default.callbacks, 'sorter'); + }); + + it('should enable highlightFirst if alwaysHighlightFirst is set', function () { + const atwhoInstance = { setting: { alwaysHighlightFirst: true } }; + + GfmAutoComplete.DefaultOptions.sorter.call(atwhoInstance); + + expect(atwhoInstance.setting.highlightFirst).toBe(true); + }); + + it('should enable highlightFirst if a query is present', function () { + const atwhoInstance = { setting: {} }; + + GfmAutoComplete.DefaultOptions.sorter.call(atwhoInstance, 'query'); + + expect(atwhoInstance.setting.highlightFirst).toBe(true); + }); + + it('should call the default atwho sorter', function () { + const atwhoInstance = { setting: {} }; + + const query = 'query'; + const items = []; + const searchKey = 'searchKey'; + + GfmAutoComplete.DefaultOptions.sorter.call(atwhoInstance, query, items, searchKey); + + expect($.fn.atwho.default.callbacks.sorter).toHaveBeenCalledWith(query, items, searchKey); + }); + }); + }); +}); diff --git a/spec/javascripts/visibility_select_spec.js.es6 b/spec/javascripts/visibility_select_spec.js.es6 new file mode 100644 index 00000000000..b21f6912e06 --- /dev/null +++ b/spec/javascripts/visibility_select_spec.js.es6 @@ -0,0 +1,100 @@ +/*= require visibility_select */ + +(() => { + const VisibilitySelect = gl.VisibilitySelect; + + describe('VisibilitySelect', function () { + const lockedElement = document.createElement('div'); + lockedElement.dataset.helpBlock = 'lockedHelpBlock'; + + const checkedElement = document.createElement('div'); + checkedElement.dataset.description = 'checkedDescription'; + + const mockElements = { + container: document.createElement('div'), + select: document.createElement('div'), + '.help-block': document.createElement('div'), + '.js-locked': lockedElement, + 'option:checked': checkedElement, + }; + + beforeEach(function () { + spyOn(Element.prototype, 'querySelector').and.callFake(selector => mockElements[selector]); + }); + + describe('#constructor', function () { + beforeEach(function () { + this.visibilitySelect = new VisibilitySelect(mockElements.container); + }); + + it('sets the container member', function () { + expect(this.visibilitySelect.container).toEqual(mockElements.container); + }); + + it('queries and sets the helpBlock member', function () { + expect(Element.prototype.querySelector).toHaveBeenCalledWith('.help-block'); + expect(this.visibilitySelect.helpBlock).toEqual(mockElements['.help-block']); + }); + + it('queries and sets the select member', function () { + expect(Element.prototype.querySelector).toHaveBeenCalledWith('select'); + expect(this.visibilitySelect.select).toEqual(mockElements.select); + }); + + describe('if there is no container element provided', function () { + it('throws an error', function () { + expect(() => new VisibilitySelect()).toThrowError('VisibilitySelect requires a container element as argument 1'); + }); + }); + }); + + describe('#init', function () { + describe('if there is a select', function () { + beforeEach(function () { + this.visibilitySelect = new VisibilitySelect(mockElements.container); + }); + + it('calls updateHelpText', function () { + spyOn(VisibilitySelect.prototype, 'updateHelpText'); + this.visibilitySelect.init(); + expect(this.visibilitySelect.updateHelpText).toHaveBeenCalled(); + }); + + it('adds a change event listener', function () { + spyOn(this.visibilitySelect.select, 'addEventListener'); + this.visibilitySelect.init(); + expect(this.visibilitySelect.select.addEventListener.calls.argsFor(0)).toContain('change'); + }); + }); + + describe('if there is no select', function () { + beforeEach(function () { + mockElements.select = undefined; + this.visibilitySelect = new VisibilitySelect(mockElements.container); + this.visibilitySelect.init(); + }); + + it('updates the helpBlock text to the locked `data-help-block` messaged', function () { + expect(this.visibilitySelect.helpBlock.textContent) + .toEqual(lockedElement.dataset.helpBlock); + }); + + afterEach(function () { + mockElements.select = document.createElement('div'); + }); + }); + }); + + describe('#updateHelpText', function () { + beforeEach(function () { + this.visibilitySelect = new VisibilitySelect(mockElements.container); + this.visibilitySelect.init(); + }); + + it('updates the helpBlock text to the selected options `data-description`', function () { + expect(this.visibilitySelect.helpBlock.textContent) + .toEqual(checkedElement.dataset.description); + }); + }); + }); +})(); diff --git a/spec/lib/gitlab/ci/status/external/common_spec.rb b/spec/lib/gitlab/ci/status/external/common_spec.rb new file mode 100644 index 00000000000..5a97d98b55f --- /dev/null +++ b/spec/lib/gitlab/ci/status/external/common_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::External::Common do + let(:user) { create(:user) } + let(:project) { external_status.project } + let(:external_target_url) { 'http://example.gitlab.com/status' } + + let(:external_status) do + create(:generic_commit_status, target_url: external_target_url) + end + + subject do + Gitlab::Ci::Status::Core + .new(external_status, user) + .extend(described_class) + end + + describe '#has_action?' do + it { is_expected.not_to have_action } + end + + describe '#has_details?' do + context 'when user has access to read commit status' do + before { project.team << [user, :developer] } + + it { is_expected.to have_details } + end + + context 'when user does not have access to read commit status' do + it { is_expected.not_to have_details } + end + end + + describe '#details_path' do + it 'links to the external target URL' do + expect(subject.details_path).to eq external_target_url + end + end +end diff --git a/spec/lib/gitlab/ci/status/external/factory_spec.rb b/spec/lib/gitlab/ci/status/external/factory_spec.rb new file mode 100644 index 00000000000..c96fd53e730 --- /dev/null +++ b/spec/lib/gitlab/ci/status/external/factory_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::External::Factory do + let(:user) { create(:user) } + let(:project) { resource.project } + let(:status) { factory.fabricate! } + let(:factory) { described_class.new(resource, user) } + let(:external_url) { 'http://gitlab.com/status' } + + before do + project.team << [user, :developer] + end + + context 'when external status has a simple core status' do + HasStatus::AVAILABLE_STATUSES.each do |simple_status| + context "when core status is #{simple_status}" do + let(:resource) do + create(:generic_commit_status, status: simple_status, + target_url: external_url) + end + + let(:expected_status) do + Gitlab::Ci::Status.const_get(simple_status.capitalize) + end + + it "fabricates a core status #{simple_status}" do + expect(status).to be_a expected_status + end + + it 'extends core status with common methods' do + expect(status).to have_details + expect(status).not_to have_action + expect(status.details_path).to eq external_url + end + end + end + end +end diff --git a/spec/lib/gitlab/view/presenter/base_spec.rb b/spec/lib/gitlab/view/presenter/base_spec.rb new file mode 100644 index 00000000000..f2c152cdcd4 --- /dev/null +++ b/spec/lib/gitlab/view/presenter/base_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe Gitlab::View::Presenter::Base do + let(:project) { double(:project) } + let(:presenter_class) do + Struct.new(:subject).include(described_class) + end + + describe '.presenter?' do + it 'returns true' do + presenter = presenter_class.new(project) + + expect(presenter.class).to be_presenter + end + end + + describe '.presents' do + it 'exposes #subject with the given keyword' do + presenter_class.presents(:foo) + presenter = presenter_class.new(project) + + expect(presenter.foo).to eq(project) + end + end + + describe '#can?' do + context 'user is not allowed' do + it 'returns false' do + presenter = presenter_class.new(build_stubbed(:empty_project)) + + expect(presenter.can?(nil, :read_project)).to be_falsy + end + end + + context 'user is allowed' do + it 'returns true' do + presenter = presenter_class.new(build_stubbed(:empty_project, :public)) + + expect(presenter.can?(nil, :read_project)).to be_truthy + end + end + + context 'subject is overriden' do + it 'returns true' do + presenter = presenter_class.new(build_stubbed(:empty_project, :public)) + + expect(presenter.can?(nil, :read_project, build_stubbed(:empty_project))).to be_falsy + end + end + end +end diff --git a/spec/lib/gitlab/view/presenter/delegated_spec.rb b/spec/lib/gitlab/view/presenter/delegated_spec.rb new file mode 100644 index 00000000000..888ab80cad5 --- /dev/null +++ b/spec/lib/gitlab/view/presenter/delegated_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::View::Presenter::Delegated do + let(:project) { double(:project, bar: 'baz') } + let(:presenter_class) do + Class.new(described_class) + end + + it 'includes Gitlab::View::Presenter::Base' do + expect(described_class).to include(Gitlab::View::Presenter::Base) + end + + describe '#initialize' do + it 'takes arbitrary key/values and exposes them' do + presenter = presenter_class.new(project, user: 'user', foo: 'bar') + + expect(presenter.user).to eq('user') + expect(presenter.foo).to eq('bar') + end + end + + describe 'delegation' do + it 'forwards missing methods to subject' do + presenter = presenter_class.new(project) + + expect(presenter.bar).to eq('baz') + end + end +end diff --git a/spec/lib/gitlab/view/presenter/factory_spec.rb b/spec/lib/gitlab/view/presenter/factory_spec.rb new file mode 100644 index 00000000000..55c5ecbf92f --- /dev/null +++ b/spec/lib/gitlab/view/presenter/factory_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Gitlab::View::Presenter::Factory do + let(:build) { Ci::Build.new } + + describe '#initialize' do + context 'without optional parameters' do + it 'takes a subject and optional params' do + presenter = described_class.new(build) + + expect { presenter }.not_to raise_error + end + end + + context 'with optional parameters' do + it 'takes a subject and optional params' do + presenter = described_class.new(build, user: 'user') + + expect { presenter }.not_to raise_error + end + end + end + + describe '#fabricate!' do + it 'exposes given params' do + presenter = described_class.new(build, user: 'user', foo: 'bar').fabricate! + + expect(presenter.user).to eq('user') + expect(presenter.foo).to eq('bar') + end + + it 'detects the presenter based on the given subject' do + presenter = described_class.new(build).fabricate! + + expect(presenter).to be_a(Ci::BuildPresenter) + end + end +end diff --git a/spec/lib/gitlab/view/presenter/simple_spec.rb b/spec/lib/gitlab/view/presenter/simple_spec.rb new file mode 100644 index 00000000000..b489bdf1981 --- /dev/null +++ b/spec/lib/gitlab/view/presenter/simple_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::View::Presenter::Simple do + let(:project) { double(:project) } + let(:presenter_class) do + Class.new(described_class) + end + + it 'includes Gitlab::View::Presenter::Base' do + expect(described_class).to include(Gitlab::View::Presenter::Base) + end + + describe '#initialize' do + it 'takes arbitrary key/values and exposes them' do + presenter = presenter_class.new(project, user: 'user', foo: 'bar') + + expect(presenter.user).to eq('user') + expect(presenter.foo).to eq('bar') + end + end + + describe 'delegation' do + it 'does not forward missing methods to subject' do + presenter = presenter_class.new(project) + + expect { presenter.foo }.to raise_error(NoMethodError) + end + end +end diff --git a/spec/models/concerns/presentable_spec.rb b/spec/models/concerns/presentable_spec.rb new file mode 100644 index 00000000000..941647a79fb --- /dev/null +++ b/spec/models/concerns/presentable_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe Presentable do + let(:build) { Ci::Build.new } + + describe '#present' do + it 'returns a presenter' do + expect(build.present).to be_a(Ci::BuildPresenter) + end + + it 'takes optional attributes' do + expect(build.present(foo: 'bar').foo).to eq('bar') + end + end +end diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index 6004bfdb7b7..f4c3e6d503f 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -1,10 +1,20 @@ require 'spec_helper' describe GenericCommitStatus, models: true do - let(:pipeline) { create(:ci_pipeline) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:external_url) { 'http://example.gitlab.com/status' } let(:generic_commit_status) do - create(:generic_commit_status, pipeline: pipeline) + create(:generic_commit_status, pipeline: pipeline, + target_url: external_url) + end + + describe 'validations' do + it { is_expected.to validate_length_of(:target_url).is_at_most(255) } + it { is_expected.to allow_value(nil).for(:target_url) } + it { is_expected.to allow_value('http://gitlab.com/s').for(:target_url) } + it { is_expected.not_to allow_value('javascript:alert(1)').for(:target_url) } end describe '#context' do @@ -22,10 +32,25 @@ describe GenericCommitStatus, models: true do describe '#detailed_status' do let(:user) { create(:user) } + let(:status) { generic_commit_status.detailed_status(user) } it 'returns detailed status object' do - expect(generic_commit_status.detailed_status(user)) - .to be_a Gitlab::Ci::Status::Success + expect(status).to be_a Gitlab::Ci::Status::Success + end + + context 'when user has ability to see datails' do + before { project.team << [user, :developer] } + + it 'details path points to an external URL' do + expect(status).to have_details + expect(status.details_path).to eq external_url + end + end + + context 'when user should not see details' do + it 'does not have details' do + expect(status).not_to have_details + end end end diff --git a/spec/policies/base_policy_spec.rb b/spec/policies/base_policy_spec.rb new file mode 100644 index 00000000000..63acc0b68cd --- /dev/null +++ b/spec/policies/base_policy_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe BasePolicy, models: true do + let(:build) { Ci::Build.new } + + describe '.class_for' do + it 'detects policy class based on the subject ancestors' do + expect(described_class.class_for(build)).to eq(Ci::BuildPolicy) + end + + it 'detects policy class for a presented subject' do + presentee = Ci::BuildPresenter.new(build) + + expect(described_class.class_for(presentee)).to eq(Ci::BuildPolicy) + end + end +end diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb new file mode 100644 index 00000000000..7a35da38b2b --- /dev/null +++ b/spec/presenters/ci/build_presenter_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' + +describe Ci::BuildPresenter do + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, pipeline: pipeline) } + + subject(:presenter) do + described_class.new(build) + end + + it 'inherits from Gitlab::View::Presenter::Delegated' do + expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated) + end + + describe '#initialize' do + it 'takes a build and optional params' do + expect { presenter }.not_to raise_error + end + + it 'exposes build' do + expect(presenter.build).to eq(build) + end + + it 'forwards missing methods to build' do + expect(presenter.ref).to eq('master') + end + end + + describe '#erased_by_user?' do + it 'takes a build and optional params' do + expect(presenter).not_to be_erased_by_user + end + end + + describe '#erased_by_name' do + context 'when build is not erased' do + before do + expect(presenter).to receive(:erased_by_user?).and_return(false) + end + + it 'returns nil' do + expect(presenter.erased_by_name).to be_nil + end + end + + context 'when build is erased' do + before do + expect(presenter).to receive(:erased_by_user?).and_return(true) + expect(build).to receive(:erased_by). + and_return(double(:user, name: 'John Doe')) + end + + it 'returns the name of the eraser' do + expect(presenter.erased_by_name).to eq('John Doe') + end + end + end + + describe 'quack like a Ci::Build permission-wise' do + context 'user is not allowed' do + let(:project) { build_stubbed(:empty_project, public_builds: false) } + + it 'returns false' do + expect(presenter.can?(nil, :read_build)).to be_falsy + end + end + + context 'user is allowed' do + let(:project) { build_stubbed(:empty_project, :public) } + + it 'returns true' do + expect(presenter.can?(nil, :read_build)).to be_truthy + end + end + end +end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 335efc4db6c..c1c7c0882de 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -152,8 +152,11 @@ describe API::CommitStatuses, api: true do context 'with all optional parameters' do before do - optional_params = { state: 'success', context: 'coverage', - ref: 'develop', target_url: 'url', description: 'test' } + optional_params = { state: 'success', + context: 'coverage', + ref: 'develop', + description: 'test', + target_url: 'http://gitlab.com/status' } post api(post_url, developer), optional_params end @@ -164,12 +167,12 @@ describe API::CommitStatuses, api: true do expect(json_response['status']).to eq('success') expect(json_response['name']).to eq('coverage') expect(json_response['ref']).to eq('develop') - expect(json_response['target_url']).to eq('url') expect(json_response['description']).to eq('test') + expect(json_response['target_url']).to eq('http://gitlab.com/status') end end - context 'invalid status' do + context 'when status is invalid' do before { post api(post_url, developer), state: 'invalid' } it 'does not create commit status' do @@ -177,7 +180,7 @@ describe API::CommitStatuses, api: true do end end - context 'request without state' do + context 'when request without a state made' do before { post api(post_url, developer) } it 'does not create commit status' do @@ -185,7 +188,7 @@ describe API::CommitStatuses, api: true do end end - context 'invalid commit' do + context 'when commit SHA is invalid' do let(:sha) { 'invalid_sha' } before { post api(post_url, developer), state: 'running' } @@ -193,6 +196,19 @@ describe API::CommitStatuses, api: true do expect(response).to have_http_status(404) end end + + context 'when target URL is an invalid address' do + before do + post api(post_url, developer), state: 'pending', + target_url: 'invalid url' + end + + it 'responds with bad request status and validation errors' do + expect(response).to have_http_status(400) + expect(json_response['message']['target_url']) + .to include 'must be a valid URL' + end + end end context 'reporter user' do |