diff options
76 files changed, 1982 insertions, 728 deletions
diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 090ea9dad19..1aa5e414fd3 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -6.0.3 +6.0.4 @@ -126,6 +126,7 @@ gem 'html-pipeline', '~> 1.11.0' gem 'deckar01-task_list', '2.0.0' gem 'gitlab-markup', '~> 1.6.2' gem 'redcarpet', '~> 3.4' +gem 'commonmarker', '~> 0.17' gem 'RedCloth', '~> 4.3.2' gem 'rdoc', '~> 4.2' gem 'org-ruby', '~> 0.9.12' diff --git a/Gemfile.lock b/Gemfile.lock index 010d4f7b56a..bbdc48f964d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -131,6 +131,8 @@ GEM coercible (1.0.0) descendants_tracker (~> 0.0.1) colorize (0.7.7) + commonmarker (0.17.8) + ruby-enum (~> 0.5) concord (0.1.5) adamantium (~> 0.2.0) equalizer (~> 0.0.9) @@ -797,6 +799,8 @@ GEM rubocop (>= 0.51) rubocop-rspec (1.22.1) rubocop (>= 0.52.1) + ruby-enum (0.7.2) + i18n ruby-fogbugz (0.2.1) crack (~> 0.4) ruby-prof (0.16.2) @@ -1019,6 +1023,7 @@ DEPENDENCIES charlock_holmes (~> 0.7.5) chronic (~> 0.10.2) chronic_duration (~> 0.10.6) + commonmarker (~> 0.17) concurrent-ruby (~> 1.0.5) connection_pool (~> 2.0) creole (~> 0.5.0) diff --git a/app/assets/javascripts/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue index 27136c7289f..f8dcdf3f60a 100644 --- a/app/assets/javascripts/clusters/components/applications.vue +++ b/app/assets/javascripts/clusters/components/applications.vue @@ -186,7 +186,7 @@ <clipboard-button :text="ingressExternalIp" :title="s__('ClusterIntegration|Copy Ingress IP Address to clipboard')" - css-class="btn btn-default js-clipboard-btn" + class="js-clipboard-btn" /> </span> </div> diff --git a/app/assets/javascripts/notes/components/diff_file_header.vue b/app/assets/javascripts/notes/components/diff_file_header.vue index fe5baa3537f..3bcde17f07c 100644 --- a/app/assets/javascripts/notes/components/diff_file_header.vue +++ b/app/assets/javascripts/notes/components/diff_file_header.vue @@ -35,6 +35,7 @@ <clipboard-button title="Copy file path to clipboard" :text="diffFile.submoduleLink" + css-class="btn-default btn-transparent btn-clipboard" /> </span> </div> @@ -79,6 +80,7 @@ <clipboard-button title="Copy file path to clipboard" :text="diffFile.filePath" + css-class="btn-default btn-transparent btn-clipboard" /> <small diff --git a/app/assets/javascripts/registry/components/collapsible_container.vue b/app/assets/javascripts/registry/components/collapsible_container.vue index b4906ba4ee5..a03180e80e6 100644 --- a/app/assets/javascripts/registry/components/collapsible_container.vue +++ b/app/assets/javascripts/registry/components/collapsible_container.vue @@ -86,6 +86,7 @@ v-if="repo.location" :text="clipboardText" :title="repo.location" + css-class="btn-default btn-transparent btn-clipboard" /> <div class="controls hidden-xs pull-right"> diff --git a/app/assets/javascripts/registry/components/table_registry.vue b/app/assets/javascripts/registry/components/table_registry.vue index bef850eddc0..ee4eb3581f3 100644 --- a/app/assets/javascripts/registry/components/table_registry.vue +++ b/app/assets/javascripts/registry/components/table_registry.vue @@ -90,6 +90,7 @@ v-if="item.location" :title="item.location" :text="clipboardText(item.location)" + css-class="btn-default btn-transparent btn-clipboard" /> </td> <td> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue index 18a3787857d..3d886e7d628 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue @@ -67,6 +67,7 @@ <clipboard-button :text="branchNameClipboardData" :title="__('Copy branch name to clipboard')" + css-class="btn-default btn-transparent btn-clipboard" /> {{ s__("mrWidget|into") }} diff --git a/app/assets/javascripts/vue_shared/components/clipboard_button.vue b/app/assets/javascripts/vue_shared/components/clipboard_button.vue index 3b6c2da1664..cab126a7eca 100644 --- a/app/assets/javascripts/vue_shared/components/clipboard_button.vue +++ b/app/assets/javascripts/vue_shared/components/clipboard_button.vue @@ -31,7 +31,7 @@ cssClass: { type: String, required: false, - default: 'btn btn-default btn-transparent btn-clipboard', + default: 'btn-default', }, }, }; @@ -40,6 +40,7 @@ <template> <button type="button" + class="btn" :class="cssClass" :title="title" :data-clipboard-text="text" diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index a1af125547c..54e7d81de6a 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -187,7 +187,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo begin @merge_request.environments_for(current_user).map do |environment| project = environment.project - deployment = environment.first_deployment_for(@merge_request.diff_head_commit) + deployment = environment.first_deployment_for(@merge_request.diff_head_sha) stop_url = if environment.stop_action? && can?(current_user, :create_deployment, environment) diff --git a/app/models/concerns/deployment_platform.rb b/app/models/concerns/deployment_platform.rb index 89d0474a596..faa94204e33 100644 --- a/app/models/concerns/deployment_platform.rb +++ b/app/models/concerns/deployment_platform.rb @@ -1,5 +1,6 @@ module DeploymentPlatform - def deployment_platform + # EE would override this and utilize the extra argument + def deployment_platform(environment: nil) @deployment_platform ||= find_cluster_platform_kubernetes || find_kubernetes_service_integration || diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 4560bc23193..5a566f3ac02 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -223,6 +223,10 @@ module Issuable def to_ability_name model_name.singular end + + def parent_class + ::Project + end end def today? diff --git a/app/models/environment.rb b/app/models/environment.rb index 24d4f1d8761..2b0a88ac5b4 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -99,8 +99,8 @@ class Environment < ActiveRecord::Base folder_name == "production" end - def first_deployment_for(commit) - ref = project.repository.ref_name_for_sha(ref_path, commit.sha) + def first_deployment_for(commit_sha) + ref = project.repository.ref_name_for_sha(ref_path, commit_sha) return nil unless ref @@ -225,7 +225,7 @@ class Environment < ActiveRecord::Base end def deployment_platform - project.deployment_platform + project.deployment_platform(environment: self) end private diff --git a/app/models/event.rb b/app/models/event.rb index be0fc7efa9a..17a198d52c7 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -65,6 +65,7 @@ class Event < ActiveRecord::Base # Callbacks after_create :reset_project_activity after_create :set_last_repository_updated_at, if: :push? + after_create :track_user_interacted_projects # Scopes scope :recent, -> { reorder(id: :desc) } @@ -389,4 +390,11 @@ class Event < ActiveRecord::Base Project.unscoped.where(id: project_id) .update_all(last_repository_updated_at: created_at) end + + def track_user_interacted_projects + # Note the call to .available? is due to earlier migrations + # that would otherwise conflict with the call to .track + # (because the table does not exist yet). + UserInteractedProject.track(self) if UserInteractedProject.available? + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5bec68ce4f6..9a7e66a9cbb 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -375,15 +375,27 @@ class MergeRequest < ActiveRecord::Base end def diff_start_sha - diff_start_commit.try(:sha) + if persisted? + merge_request_diff.start_commit_sha + else + target_branch_head.try(:sha) + end end def diff_base_sha - diff_base_commit.try(:sha) + if persisted? + merge_request_diff.base_commit_sha + else + branch_merge_base_commit.try(:sha) + end end def diff_head_sha - diff_head_commit.try(:sha) + if persisted? + merge_request_diff.head_commit_sha + else + source_branch_head.try(:sha) + end end # When importing a pull request from GitHub, the old and new branches may no @@ -646,7 +658,7 @@ class MergeRequest < ActiveRecord::Base !ProtectedBranch.protected?(source_project, source_branch) && !source_project.root_ref?(source_branch) && Ability.allowed?(current_user, :push_code, source_project) && - diff_head_commit == source_branch_head + diff_head_sha == source_branch_head.try(:sha) end def should_remove_source_branch? diff --git a/app/models/note.rb b/app/models/note.rb index d7a67ec277c..787a80f0196 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -81,7 +81,7 @@ class Note < ActiveRecord::Base validates :author, presence: true validates :discussion_id, presence: true, format: { with: /\A\h{40}\z/ } - validate unless: [:for_commit?, :importing?, :for_personal_snippet?] do |note| + validate unless: [:for_commit?, :importing?, :skip_project_check?] do |note| unless note.noteable.try(:project) == note.project errors.add(:project, 'does not match noteable project') end @@ -228,7 +228,7 @@ class Note < ActiveRecord::Base end def skip_project_check? - for_personal_snippet? + !for_project_noteable? end def commit @@ -308,6 +308,11 @@ class Note < ActiveRecord::Base self.noteable.supports_discussions? && !part_of_discussion? end + def can_create_todo? + # Skip system notes, and notes on project snippet + !system? && !for_snippet? + end + def discussion_class(noteable = nil) # When commit notes are rendered on an MR's Discussion page, they are # displayed in one discussion instead of individually. diff --git a/app/models/snippet.rb b/app/models/snippet.rb index a58c208279e..644120453cf 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -168,5 +168,9 @@ class Snippet < ActiveRecord::Base def search_code(query) fuzzy_search(query, [:content]) end + + def parent_class + ::Project + end end end diff --git a/app/models/user_interacted_project.rb b/app/models/user_interacted_project.rb new file mode 100644 index 00000000000..dd55a6acb79 --- /dev/null +++ b/app/models/user_interacted_project.rb @@ -0,0 +1,59 @@ +class UserInteractedProject < ActiveRecord::Base + belongs_to :user + belongs_to :project + + validates :project_id, presence: true + validates :user_id, presence: true + + CACHE_EXPIRY_TIME = 1.day + + # Schema version required for this model + REQUIRED_SCHEMA_VERSION = 20180223120443 + + class << self + def track(event) + # For events without a project, we simply don't care. + # An example of this is the creation of a snippet (which + # is not related to any project). + return unless event.project_id + + attributes = { + project_id: event.project_id, + user_id: event.author_id + } + + cached_exists?(attributes) do + transaction(requires_new: true) do + begin + where(attributes).select(1).first || create!(attributes) + true # not caching the whole record here for now + rescue ActiveRecord::RecordNotUnique + # Note, above queries are not atomic and prone + # to race conditions (similar like #find_or_create!). + # In the case where we hit this, the record we want + # already exists - shortcut and return. + true + end + end + end + end + + # Check if we can safely call .track (table exists) + def available? + @available_flag ||= ActiveRecord::Migrator.current_version >= REQUIRED_SCHEMA_VERSION # rubocop:disable Gitlab/PredicateMemoization + end + + # Flushes cached information about schema + def reset_column_information + @available_flag = nil + super + end + + private + + def cached_exists?(project_id:, user_id:, &block) + cache_key = "user_interacted_projects:#{project_id}:#{user_id}" + Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRY_TIME, &block) + end + end +end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 4e8ef320af2..a3ebec0efc6 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -38,7 +38,7 @@ class MergeRequestWidgetEntity < IssuableEntity # Diff sha's expose :diff_head_sha do |merge_request| - merge_request.diff_head_sha if merge_request.diff_head_commit + merge_request.diff_head_sha.presence end expose :merge_commit_message diff --git a/app/services/notes/build_service.rb b/app/services/notes/build_service.rb index abf25bb778b..77e7b8a5ea7 100644 --- a/app/services/notes/build_service.rb +++ b/app/services/notes/build_service.rb @@ -26,14 +26,19 @@ module Notes if project project.notes.find_discussion(discussion_id) else - # only PersonalSnippets can have discussions without project association discussion = Note.find_discussion(discussion_id) noteable = discussion.noteable - return nil unless noteable.is_a?(PersonalSnippet) && can?(current_user, :comment_personal_snippet, noteable) + return nil unless noteable_without_project?(noteable) discussion end end + + def noteable_without_project?(noteable) + return true if noteable.is_a?(PersonalSnippet) && can?(current_user, :comment_personal_snippet, noteable) + + false + end end end diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb index 6a10e172483..ad3dcc5010b 100644 --- a/app/services/notes/post_process_service.rb +++ b/app/services/notes/post_process_service.rb @@ -11,7 +11,7 @@ module Notes unless @note.system? EventCreateService.new.leave_note(@note, @note.author) - return if @note.for_personal_snippet? + return unless @note.for_project_noteable? @note.create_cross_references! execute_note_hooks diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 6835b14648b..e4be953e810 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -280,7 +280,7 @@ module NotificationRecipientService add_participants(note.author) add_mentions(note.author, target: note) - unless note.for_personal_snippet? + if note.for_project_noteable? # Merge project watchers add_project_watchers diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index c2ca404b179..ffd48e842c2 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -241,8 +241,7 @@ class TodoService end def handle_note(note, author, skip_users = []) - # Skip system notes, and notes on project snippet - return if note.system? || note.for_snippet? + return unless note.can_create_todo? project = note.project target = note.noteable diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 68788134b8e..81d7db04a3c 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -657,9 +657,11 @@ .checkbox = f.label :version_check_enabled do = f.check_box :version_check_enabled - Version check enabled + Enable version check .help-block - Let GitLab inform you when an update is available. + GitLab will inform you if a new version is available. + = link_to 'Learn more', help_page_path("user/admin_area/settings/usage_statistics", anchor: "version-check") + about what information is shared with GitLab Inc. .form-group .col-sm-offset-2.col-sm-10 - can_be_configured = @application_setting.usage_ping_can_be_configured? diff --git a/app/views/admin/hooks/_form.html.haml b/app/views/admin/hooks/_form.html.haml index d8f96ed5b0d..a6324a97fd5 100644 --- a/app/views/admin/hooks/_form.html.haml +++ b/app/views/admin/hooks/_form.html.haml @@ -1,21 +1,20 @@ = form_errors(hook) .form-group - = form.label :url, 'URL', class: 'control-label' - .col-sm-10 - = form.text_field :url, class: 'form-control' + = form.label :url, 'URL', class: 'label-light' + = form.text_field :url, class: 'form-control' .form-group - = form.label :token, 'Secret Token', class: 'control-label' - .col-sm-10 - = form.text_field :token, class: 'form-control' - %p.help-block - Use this token to validate received payloads + = form.label :token, 'Secret Token', class: 'label-light' + = form.text_field :token, class: 'form-control' + %p.help-block + Use this token to validate received payloads .form-group - = form.label :url, 'Trigger', class: 'control-label' - .col-sm-10.prepend-top-10 - %div - System hook will be triggered on set of events like creating project - or adding ssh key. But you can also enable extra triggers like Push events. + = form.label :url, 'Trigger', class: 'label-light' + %ul.list-unstyled + %li + .help-block + System hook will be triggered on set of events like creating project + or adding ssh key. But you can also enable extra triggers like Push events. .prepend-top-default = form.check_box :repository_update_events, class: 'pull-left' @@ -24,21 +23,21 @@ %strong Repository update events %p.light This URL will be triggered when repository is updated - %div + %li = form.check_box :push_events, class: 'pull-left' .prepend-left-20 = form.label :push_events, class: 'list-label' do %strong Push events %p.light This URL will be triggered for each branch updated to the repository - %div + %li = form.check_box :tag_push_events, class: 'pull-left' .prepend-left-20 = form.label :tag_push_events, class: 'list-label' do %strong Tag push events %p.light This URL will be triggered when a new tag is pushed to the repository - %div + %li = form.check_box :merge_requests_events, class: 'pull-left' .prepend-left-20 = form.label :merge_requests_events, class: 'list-label' do @@ -46,9 +45,8 @@ %p.light This URL will be triggered when a merge request is created/updated/merged .form-group - = form.label :enable_ssl_verification, 'SSL verification', class: 'control-label checkbox' - .col-sm-10 - .checkbox - = form.label :enable_ssl_verification do - = form.check_box :enable_ssl_verification - %strong Enable SSL verification + = form.label :enable_ssl_verification, 'SSL verification', class: 'label-light checkbox' + .checkbox + = form.label :enable_ssl_verification do + = form.check_box :enable_ssl_verification + %strong Enable SSL verification diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml index bc02d9969d6..d9e2ce5e74c 100644 --- a/app/views/admin/hooks/index.html.haml +++ b/app/views/admin/hooks/index.html.haml @@ -1,33 +1,35 @@ - page_title 'System Hooks' -%h3.page-title - System hooks +.row.prepend-top-default + .col-lg-4 + %h4.prepend-top-0 + = page_title + %p + #{link_to 'System hooks ', help_page_path('system_hooks/system_hooks'), class: 'vlink'} can be + used for binding events when GitLab creates a User or Project. -%p.light - #{link_to 'System hooks ', help_page_path('system_hooks/system_hooks'), class: 'vlink'} can be - used for binding events when GitLab creates a User or Project. + .col-lg-8.append-bottom-default + = form_for @hook, as: :hook, url: admin_hooks_path do |f| + = render partial: 'form', locals: { form: f, hook: @hook } + = f.submit 'Add system hook', class: 'btn btn-create' -%hr + %hr -= form_for @hook, as: :hook, url: admin_hooks_path, html: { class: 'form-horizontal' } do |f| - = render partial: 'form', locals: { form: f, hook: @hook } - .form-actions - = f.submit 'Add system hook', class: 'btn btn-create' -%hr + - if @hooks.any? + .panel.panel-default + .panel-heading + System hooks (#{@hooks.count}) + %ul.content-list + - @hooks.each do |hook| + %li + .controls + = render 'shared/web_hooks/test_button', triggers: SystemHook.triggers, hook: hook, button_class: 'btn-sm' + = link_to 'Edit', edit_admin_hook_path(hook), class: 'btn btn-sm' + = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm' + .monospace= hook.url + %div + - SystemHook.triggers.each_value do |event| + - if hook.public_send(event) + %span.label.label-gray= event.to_s.titleize + %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'} -- if @hooks.any? - .panel.panel-default - .panel-heading - System hooks (#{@hooks.count}) - %ul.content-list - - @hooks.each do |hook| - %li - .controls - = render 'shared/web_hooks/test_button', triggers: SystemHook.triggers, hook: hook, button_class: 'btn-sm' - = link_to 'Edit', edit_admin_hook_path(hook), class: 'btn btn-sm' - = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm' - .monospace= hook.url - %div - - SystemHook.triggers.each_value do |event| - - if hook.public_send(event) - %span.label.label-gray= event.to_s.titleize - %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'} += render 'shared/plugins/index' diff --git a/app/views/shared/plugins/_index.html.haml b/app/views/shared/plugins/_index.html.haml new file mode 100644 index 00000000000..fc643c3ecc2 --- /dev/null +++ b/app/views/shared/plugins/_index.html.haml @@ -0,0 +1,23 @@ +- plugins = Gitlab::Plugin.files + +.row.prepend-top-default + .col-lg-4 + %h4.prepend-top-0 + Plugins + %p + #{link_to 'Plugins', help_page_path('administration/plugins')} are similar to + system hooks but are executed as files instead of sending data to a URL. + + .col-lg-8.append-bottom-default + - if plugins.any? + .panel.panel-default + .panel-heading + Plugins (#{plugins.count}) + %ul.content-list + - plugins.each do |file| + %li + .monospace + = File.basename(file) + - else + %p.light-well.text-center + No plugins found. diff --git a/changelogs/unreleased/43460-track-projects-a-user-interacted-with.yml b/changelogs/unreleased/43460-track-projects-a-user-interacted-with.yml new file mode 100644 index 00000000000..99b6ac76a3e --- /dev/null +++ b/changelogs/unreleased/43460-track-projects-a-user-interacted-with.yml @@ -0,0 +1,5 @@ +--- +title: Keep track of projects a user interacted with. +merge_request: 17327 +author: +type: other diff --git a/changelogs/unreleased/discussions-api.yml b/changelogs/unreleased/discussions-api.yml new file mode 100644 index 00000000000..110df3aa414 --- /dev/null +++ b/changelogs/unreleased/discussions-api.yml @@ -0,0 +1,5 @@ +--- +title: Add discussions API for Issues and Snippets +merge_request: +author: +type: added diff --git a/changelogs/unreleased/dz-plugins-project-integrations.yml b/changelogs/unreleased/dz-plugins-project-integrations.yml new file mode 100644 index 00000000000..9dbe82f9af8 --- /dev/null +++ b/changelogs/unreleased/dz-plugins-project-integrations.yml @@ -0,0 +1,5 @@ +--- +title: Add plugins list to the system hooks page +merge_request: 17518 +author: +type: added diff --git a/changelogs/unreleased/feature--43691-count-diff-note-calendar-activity.yml b/changelogs/unreleased/feature--43691-count-diff-note-calendar-activity.yml index 768686aeda8..d8020592897 100644 --- a/changelogs/unreleased/feature--43691-count-diff-note-calendar-activity.yml +++ b/changelogs/unreleased/feature--43691-count-diff-note-calendar-activity.yml @@ -1,5 +1,5 @@ --- -title: Count comments on diffs as contributions for the contributions calendar +title: Count comments on diffs and discussions as contributions for the contributions calendar merge_request: 17418 author: Riccardo Padovani type: fixed diff --git a/changelogs/unreleased/mr-commit-optimization.yml b/changelogs/unreleased/mr-commit-optimization.yml new file mode 100644 index 00000000000..522d8951b18 --- /dev/null +++ b/changelogs/unreleased/mr-commit-optimization.yml @@ -0,0 +1,5 @@ +--- +title: Use persisted/memoized value for MRs shas instead of doing git lookups +merge_request: 17555 +author: +type: performance diff --git a/changelogs/unreleased/replace_redcarpet_with_cmark.yml b/changelogs/unreleased/replace_redcarpet_with_cmark.yml new file mode 100644 index 00000000000..7ce848b0bbd --- /dev/null +++ b/changelogs/unreleased/replace_redcarpet_with_cmark.yml @@ -0,0 +1,5 @@ +--- +title: Add CommonMark markdown engine (experimental) +merge_request: 14835 +author: blackst0ne +type: added diff --git a/changelogs/unreleased/zj-move-opt-out-ruby-endpoints.yml b/changelogs/unreleased/zj-move-opt-out-ruby-endpoints.yml new file mode 100644 index 00000000000..0ddb42bc80a --- /dev/null +++ b/changelogs/unreleased/zj-move-opt-out-ruby-endpoints.yml @@ -0,0 +1,5 @@ +--- +title: Move Ruby endpoints to OPT_OUT +merge_request: +author: +type: other diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb index 45b39b2a38d..7cdf49159b4 100644 --- a/config/initializers/8_metrics.rb +++ b/config/initializers/8_metrics.rb @@ -94,6 +94,7 @@ def instrument_classes(instrumentation) instrumentation.instrument_instance_methods(RepositoryCheck::SingleRepositoryWorker) + instrumentation.instrument_instance_methods(Rouge::Plugins::CommonMark) instrumentation.instrument_instance_methods(Rouge::Plugins::Redcarpet) instrumentation.instrument_instance_methods(Rouge::Formatters::HTMLGitlab) diff --git a/db/migrate/20180223120443_create_user_interacted_projects_table.rb b/db/migrate/20180223120443_create_user_interacted_projects_table.rb new file mode 100644 index 00000000000..20749940b1e --- /dev/null +++ b/db/migrate/20180223120443_create_user_interacted_projects_table.rb @@ -0,0 +1,18 @@ +class CreateUserInteractedProjectsTable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + create_table :user_interacted_projects, id: false do |t| + t.references :user, null: false + t.references :project, null: false + end + end + + def down + drop_table :user_interacted_projects + end +end diff --git a/db/migrate/20180227182112_add_group_id_to_boards.rb b/db/migrate/20180227182112_add_group_id_to_boards_ce.rb index 8e5460d44c9..f54dd8d7687 100644 --- a/db/migrate/20180227182112_add_group_id_to_boards.rb +++ b/db/migrate/20180227182112_add_group_id_to_boards_ce.rb @@ -1,4 +1,4 @@ -class AddGroupIdToBoards < ActiveRecord::Migration +class AddGroupIdToBoardsCe < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb b/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb new file mode 100644 index 00000000000..5e729b1aa53 --- /dev/null +++ b/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb @@ -0,0 +1,124 @@ +class BuildUserInteractedProjectsTable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + if Gitlab::Database.postgresql? + PostgresStrategy.new + else + MysqlStrategy.new + end.up + + unless index_exists?(:user_interacted_projects, [:project_id, :user_id]) + add_concurrent_index :user_interacted_projects, [:project_id, :user_id], unique: true + end + + unless foreign_key_exists?(:user_interacted_projects, :user_id) + add_concurrent_foreign_key :user_interacted_projects, :users, column: :user_id, on_delete: :cascade + end + + unless foreign_key_exists?(:user_interacted_projects, :project_id) + add_concurrent_foreign_key :user_interacted_projects, :projects, column: :project_id, on_delete: :cascade + end + end + + def down + execute "TRUNCATE user_interacted_projects" + + if foreign_key_exists?(:user_interacted_projects, :user_id) + remove_foreign_key :user_interacted_projects, :users + end + + if foreign_key_exists?(:user_interacted_projects, :project_id) + remove_foreign_key :user_interacted_projects, :projects + end + + if index_exists_by_name?(:user_interacted_projects, 'index_user_interacted_projects_on_project_id_and_user_id') + remove_concurrent_index_by_name :user_interacted_projects, 'index_user_interacted_projects_on_project_id_and_user_id' + end + end + + private + + # Rails' index_exists? doesn't work when you only give it a table and index + # name. As such we have to use some extra code to check if an index exists for + # a given name. + def index_exists_by_name?(table, index) + indexes_for_table[table].include?(index) + end + + def indexes_for_table + @indexes_for_table ||= Hash.new do |hash, table_name| + hash[table_name] = indexes(table_name).map(&:name) + end + end + + def foreign_key_exists?(table, column) + foreign_keys(table).any? do |key| + key.options[:column] == column.to_s + end + end + + class PostgresStrategy < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + BATCH_SIZE = 100_000 + SLEEP_TIME = 5 + + def up + with_index(:events, [:author_id, :project_id], name: 'events_user_interactions_temp', where: 'project_id IS NOT NULL') do + iteration = 0 + records = 0 + begin + Rails.logger.info "Building user_interacted_projects table, batch ##{iteration}" + result = execute <<~SQL + INSERT INTO user_interacted_projects (user_id, project_id) + SELECT e.user_id, e.project_id + FROM (SELECT DISTINCT author_id AS user_id, project_id FROM events WHERE project_id IS NOT NULL) AS e + LEFT JOIN user_interacted_projects ucp USING (user_id, project_id) + WHERE ucp.user_id IS NULL + LIMIT #{BATCH_SIZE} + SQL + iteration += 1 + records += result.cmd_tuples + Rails.logger.info "Building user_interacted_projects table, batch ##{iteration} complete, created #{records} overall" + Kernel.sleep(SLEEP_TIME) if result.cmd_tuples > 0 + rescue ActiveRecord::InvalidForeignKey => e + Rails.logger.info "Retry on InvalidForeignKey: #{e}" + retry + end while result.cmd_tuples > 0 + end + + execute "ANALYZE user_interacted_projects" + + end + + private + + def with_index(*args) + add_concurrent_index(*args) unless index_exists?(*args) + yield + ensure + remove_concurrent_index(*args) if index_exists?(*args) + end + end + + class MysqlStrategy < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + def up + execute <<~SQL + INSERT INTO user_interacted_projects (user_id, project_id) + SELECT e.user_id, e.project_id + FROM (SELECT DISTINCT author_id AS user_id, project_id FROM events WHERE project_id IS NOT NULL) AS e + LEFT JOIN user_interacted_projects ucp USING (user_id, project_id) + WHERE ucp.user_id IS NULL + SQL + end + end + +end diff --git a/db/schema.rb b/db/schema.rb index d49bf022d0b..387b15f8f30 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1843,6 +1843,13 @@ ActiveRecord::Schema.define(version: 20180307012445) do add_index "user_custom_attributes", ["key", "value"], name: "index_user_custom_attributes_on_key_and_value", using: :btree add_index "user_custom_attributes", ["user_id", "key"], name: "index_user_custom_attributes_on_user_id_and_key", unique: true, using: :btree + create_table "user_interacted_projects", id: false, force: :cascade do |t| + t.integer "user_id", null: false + t.integer "project_id", null: false + end + + add_index "user_interacted_projects", ["project_id", "user_id"], name: "index_user_interacted_projects_on_project_id_and_user_id", unique: true, using: :btree + create_table "user_synced_attributes_metadata", force: :cascade do |t| t.boolean "name_synced", default: false t.boolean "email_synced", default: false @@ -2115,6 +2122,8 @@ ActiveRecord::Schema.define(version: 20180307012445) do add_foreign_key "u2f_registrations", "users" add_foreign_key "user_callouts", "users", on_delete: :cascade add_foreign_key "user_custom_attributes", "users", on_delete: :cascade + add_foreign_key "user_interacted_projects", "projects", name: "fk_722ceba4f7", on_delete: :cascade + add_foreign_key "user_interacted_projects", "users", name: "fk_0894651f08", on_delete: :cascade add_foreign_key "user_synced_attributes_metadata", "users", on_delete: :cascade add_foreign_key "users_star_projects", "projects", name: "fk_22cd27ddfc", on_delete: :cascade add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade diff --git a/doc/administration/monitoring/index.md b/doc/administration/monitoring/index.md index b6320aba83e..d18dddf09c0 100644 --- a/doc/administration/monitoring/index.md +++ b/doc/administration/monitoring/index.md @@ -7,3 +7,4 @@ Explore our features to monitor your GitLab instance: - [GitHub imports](github_imports.md): Monitor the health and progress of GitLab's GitHub importer with various Prometheus metrics. - [Monitoring uptime](../../user/admin_area/monitoring/health_check.md): Check the server status using the health check endpoint. - [IP whitelists](ip_whitelist.md): Configure GitLab for monitoring endpoints that provide health check information when probed. +- [nginx_status](https://docs.gitlab.com/omnibus/settings/nginx.html#enabling-disabling-nginx_status): Monitor your Nginx server status diff --git a/doc/api/README.md b/doc/api/README.md index b67500a9b9e..f40fd587c0a 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -37,6 +37,7 @@ following locations: - [Group milestones](group_milestones.md) - [Namespaces](namespaces.md) - [Notes](notes.md) (comments) +- [Threaded comments](discussions.md) - [Notification settings](notification_settings.md) - [Open source license templates](templates/licenses.md) - [Pages Domains](pages_domains.md) diff --git a/doc/api/discussions.md b/doc/api/discussions.md new file mode 100644 index 00000000000..07837d7d4c3 --- /dev/null +++ b/doc/api/discussions.md @@ -0,0 +1,411 @@ +# Discussions API + +Discussions are set of related notes on snippets, issues or epics. + +## Issues + +### List project issue discussions + +Gets a list of all discussions for a single issue. + +``` +GET /projects/:id/issues/:issue_iid/discussions +``` + +| Attribute | Type | Required | Description | +| ------------------- | ---------------- | ---------- | ------------ | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | + +```json +[ + { + "id": "6a9c1750b37d513a43987b574953fceb50b03ce7", + "individual_note": false, + "notes": [ + { + "id": 1126, + "type": "DiscussionNote", + "body": "discussion text", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-03T21:54:39.668Z", + "updated_at": "2018-03-03T21:54:39.668Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Issue", + "noteable_iid": null + }, + { + "id": 1129, + "type": "DiscussionNote", + "body": "reply to the discussion", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T13:38:02.127Z", + "updated_at": "2018-03-04T13:38:02.127Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Issue", + "noteable_iid": null + } + ] + }, + { + "id": "87805b7c09016a7058e91bdbe7b29d1f284a39e6", + "individual_note": true, + "notes": [ + { + "id": 1128, + "type": null, + "body": "a single comment", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T09:17:22.520Z", + "updated_at": "2018-03-04T09:17:22.520Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Issue", + "noteable_iid": null + } + ] + } +] +``` + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions +``` + +### Get single issue discussion + +Returns a single discussion for a specific project issue + +``` +GET /projects/:id/issues/:issue_iid/discussions/:discussion_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `discussion_id` | integer | yes | The ID of a discussion | + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7 +``` + +### Create new issue discussion + +Creates a new discussion to a single project issue. This is similar to creating +a note but but another comments (replies) can be added to it later. + +``` +POST /projects/:id/issues/:issue_iid/discussions +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `body` | string | yes | The content of a discussion | +| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions?body=comment +``` + +### Add note to existing issue discussion + +Adds a new note to the discussion. + +``` +POST /projects/:id/issues/:issue_iid/discussions/:discussion_id/notes +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | +| `body` | string | yes | The content of a discussion | +| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment +``` + +### Modify existing issue discussion note + +Modify existing discussion note of an issue. + +``` +PUT /projects/:id/issues/:issue_iid/discussions/:discussion_id/notes/:note_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | +| `body` | string | yes | The content of a discussion | + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes/1108?body=comment +``` + +### Delete an issue discussion note + +Deletes an existing discussion note of an issue. + +``` +DELETE /projects/:id/issues/:issue_iid/discussions/:discussion_id/notes/:note_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions/636 +``` + +## Snippets + +### List project snippet discussions + +Gets a list of all discussions for a single snippet. + +``` +GET /projects/:id/snippets/:snippet_id/discussions +``` + +| Attribute | Type | Required | Description | +| ------------------- | ---------------- | ---------- | ------------| +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | + +```json +[ + { + "id": "6a9c1750b37d513a43987b574953fceb50b03ce7", + "individual_note": false, + "notes": [ + { + "id": 1126, + "type": "DiscussionNote", + "body": "discussion text", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-03T21:54:39.668Z", + "updated_at": "2018-03-03T21:54:39.668Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Snippet", + "noteable_id": null + }, + { + "id": 1129, + "type": "DiscussionNote", + "body": "reply to the discussion", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T13:38:02.127Z", + "updated_at": "2018-03-04T13:38:02.127Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Snippet", + "noteable_id": null + } + ] + }, + { + "id": "87805b7c09016a7058e91bdbe7b29d1f284a39e6", + "individual_note": true, + "notes": [ + { + "id": 1128, + "type": null, + "body": "a single comment", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T09:17:22.520Z", + "updated_at": "2018-03-04T09:17:22.520Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Snippet", + "noteable_id": null + } + ] + } +] +``` + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions +``` + +### Get single snippet discussion + +Returns a single discussion for a specific project snippet + +``` +GET /projects/:id/snippets/:snippet_id/discussions/:discussion_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `discussion_id` | integer | yes | The ID of a discussion | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7 +``` + +### Create new snippet discussion + +Creates a new discussion to a single project snippet. This is similar to creating +a note but but another comments (replies) can be added to it later. + +``` +POST /projects/:id/snippets/:snippet_id/discussions +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `body` | string | yes | The content of a discussion | +| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions?body=comment +``` + +### Add note to existing snippet discussion + +Adds a new note to the discussion. + +``` +POST /projects/:id/snippets/:snippet_id/discussions/:discussion_id/notes +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | +| `body` | string | yes | The content of a discussion | +| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment +``` + +### Modify existing snippet discussion note + +Modify existing discussion note of an snippet. + +``` +PUT /projects/:id/snippets/:snippet_id/discussions/:discussion_id/notes/:note_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | +| `body` | string | yes | The content of a discussion | + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes/1108?body=comment +``` + +### Delete an snippet discussion note + +Deletes an existing discussion note of an snippet. + +``` +DELETE /projects/:id/snippets/:snippet_id/discussions/:discussion_id/notes/:note_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/636 +``` diff --git a/doc/api/notes.md b/doc/api/notes.md index 1b68bd99ce2..aa38d22845c 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -15,7 +15,7 @@ GET /projects/:id/issues/:issue_iid/notes?sort=asc&order_by=updated_at | Attribute | Type | Required | Description | | ------------------- | ---------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | `issue_iid` | integer | yes | The IID of an issue | `sort` | string | no | Return issue notes sorted in `asc` or `desc` order. Default is `desc` | `order_by` | string | no | Return issue notes ordered by `created_at` or `updated_at` fields. Default is `created_at` @@ -63,6 +63,10 @@ GET /projects/:id/issues/:issue_iid/notes?sort=asc&order_by=updated_at ] ``` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/notes +``` + ### Get single issue note Returns a single note for a specific project issue @@ -73,14 +77,17 @@ GET /projects/:id/issues/:issue_iid/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `issue_iid` (required) - The IID of a project issue - `note_id` (required) - The ID of an issue note +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/notes/1 +``` + ### Create new issue note -Creates a new note to a single project issue. If you create a note where the body -only contains an Award Emoji, you'll receive this object back. +Creates a new note to a single project issue. ``` POST /projects/:id/issues/:issue_iid/notes @@ -88,11 +95,15 @@ POST /projects/:id/issues/:issue_iid/notes Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `issue_id` (required) - The IID of an issue - `body` (required) - The content of a note - `created_at` (optional) - Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/notes?body=note +``` + ### Modify existing issue note Modify existing note of an issue. @@ -103,11 +114,15 @@ PUT /projects/:id/issues/:issue_iid/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `issue_iid` (required) - The IID of an issue - `note_id` (required) - The ID of a note - `body` (required) - The content of a note +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/notes?body=note +``` + ### Delete an issue note Deletes an existing note of an issue. @@ -120,7 +135,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | | `issue_iid` | integer | yes | The IID of an issue | | `note_id` | integer | yes | The ID of a note | @@ -141,11 +156,15 @@ GET /projects/:id/snippets/:snippet_id/notes?sort=asc&order_by=updated_at | Attribute | Type | Required | Description | | ------------------- | ---------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | `snippet_id` | integer | yes | The ID of a project snippet | `sort` | string | no | Return snippet notes sorted in `asc` or `desc` order. Default is `desc` | `order_by` | string | no | Return snippet notes ordered by `created_at` or `updated_at` fields. Default is `created_at` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/notes +``` + ### Get single snippet note Returns a single note for a given snippet. @@ -156,7 +175,7 @@ GET /projects/:id/snippets/:snippet_id/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `snippet_id` (required) - The ID of a project snippet - `note_id` (required) - The ID of a snippet note @@ -179,6 +198,10 @@ Parameters: } ``` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/notes/11 +``` + ### Create new snippet note Creates a new note for a single snippet. Snippet notes are comments users can post to a snippet. @@ -190,10 +213,14 @@ POST /projects/:id/snippets/:snippet_id/notes Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `snippet_id` (required) - The ID of a snippet - `body` (required) - The content of a note +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippet/11/notes?body=note +``` + ### Modify existing snippet note Modify existing note of a snippet. @@ -204,11 +231,15 @@ PUT /projects/:id/snippets/:snippet_id/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `snippet_id` (required) - The ID of a snippet - `note_id` (required) - The ID of a note - `body` (required) - The content of a note +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/notes?body=note +``` + ### Delete a snippet note Deletes an existing note of a snippet. @@ -221,7 +252,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | | `snippet_id` | integer | yes | The ID of a snippet | | `note_id` | integer | yes | The ID of a note | @@ -242,11 +273,15 @@ GET /projects/:id/merge_requests/:merge_request_iid/notes?sort=asc&order_by=upda | Attribute | Type | Required | Description | | ------------------- | ---------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | `merge_request_iid` | integer | yes | The IID of a project merge request | `sort` | string | no | Return merge request notes sorted in `asc` or `desc` order. Default is `desc` | `order_by` | string | no | Return merge request notes ordered by `created_at` or `updated_at` fields. Default is `created_at` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/notes +``` + ### Get single merge request note Returns a single note for a given merge request. @@ -257,7 +292,7 @@ GET /projects/:id/merge_requests/:merge_request_iid/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `merge_request_iid` (required) - The IID of a project merge request - `note_id` (required) - The ID of a merge request note @@ -283,6 +318,10 @@ Parameters: } ``` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/notes/1 +``` + ### Create new merge request note Creates a new note for a single merge request. @@ -295,7 +334,7 @@ POST /projects/:id/merge_requests/:merge_request_iid/notes Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `merge_request_iid` (required) - The IID of a merge request - `body` (required) - The content of a note @@ -309,11 +348,15 @@ PUT /projects/:id/merge_requests/:merge_request_iid/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `merge_request_iid` (required) - The IID of a merge request - `note_id` (required) - The ID of a note - `body` (required) - The content of a note +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/notes?body=note +``` + ### Delete a merge request note Deletes an existing note of a merge request. @@ -326,7 +369,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | | `merge_request_iid` | integer | yes | The IID of a merge request | | `note_id` | integer | yes | The ID of a note | diff --git a/doc/user/admin_area/settings/img/update-available.png b/doc/user/admin_area/settings/img/update-available.png Binary files differnew file mode 100644 index 00000000000..0dafdad618e --- /dev/null +++ b/doc/user/admin_area/settings/img/update-available.png diff --git a/doc/user/admin_area/settings/usage_statistics.md b/doc/user/admin_area/settings/usage_statistics.md index d874688cc29..381efdf5d67 100644 --- a/doc/user/admin_area/settings/usage_statistics.md +++ b/doc/user/admin_area/settings/usage_statistics.md @@ -8,20 +8,26 @@ under **Admin area > Settings > Usage statistics**. ## Version check -GitLab can inform you when an update is available and the importance of it. +If enabled, version check will inform you if a new version is available and the +importance of it through a status. This is shown on the help page (i.e. `/help`) +for all signed in users, and on the admin pages. The statuses are: -No information other than the GitLab version and the instance's hostname (through the HTTP referer) -are collected. +* Green: You are running the latest version of GitLab. +* Orange: An updated version of GitLab is available. +* Red: The version of GitLab you are running is vulnerable. You should install + the latest version with security fixes as soon as possible. -In the **Overview** tab you can see if your GitLab version is up to date. There -are three cases: 1) you are up to date (green), 2) there is an update available -(yellow) and 3) your version is vulnerable and a security fix is released (red). + -In any case, you will see a message informing you of the state and the -importance of the update. +GitLab Inc. collects your instance's version and hostname (through the HTTP +referer) as part of the version check. No other information is collected. -If enabled, the version status will also be shown in the help page (`/help`) -for all signed in users. +This information is used, among other things, to identify to which versions +patches will need to be back ported, making sure active GitLab instances remain +secure. + +If you disable version check, this information will not be collected. Enable or +disable the version check at **Admin area > Settings > Usage statistics**. ## Usage ping diff --git a/lib/api/api.rb b/lib/api/api.rb index 5e93c129bc8..62ffebeacb0 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -136,6 +136,7 @@ module API mount ::API::MergeRequests mount ::API::Namespaces mount ::API::Notes + mount ::API::Discussions mount ::API::NotificationSettings mount ::API::PagesDomains mount ::API::Pipelines diff --git a/lib/api/discussions.rb b/lib/api/discussions.rb new file mode 100644 index 00000000000..6abd575b6ad --- /dev/null +++ b/lib/api/discussions.rb @@ -0,0 +1,195 @@ +module API + class Discussions < Grape::API + include PaginationParams + helpers ::API::Helpers::NotesHelpers + + before { authenticate! } + + NOTEABLE_TYPES = [Issue, Snippet].freeze + + NOTEABLE_TYPES.each do |noteable_type| + parent_type = noteable_type.parent_class.to_s.underscore + noteables_str = noteable_type.to_s.underscore.pluralize + + params do + requires :id, type: String, desc: "The ID of a #{parent_type}" + end + resource parent_type.pluralize.to_sym, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do + desc "Get a list of #{noteable_type.to_s.downcase} discussions" do + success Entities::Discussion + end + params do + requires :noteable_id, type: Integer, desc: 'The ID of the noteable' + use :pagination + end + get ":id/#{noteables_str}/:noteable_id/discussions" do + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + + return not_found!("Discussions") unless can?(current_user, noteable_read_ability_name(noteable), noteable) + + notes = noteable.notes + .inc_relations_for_view + .includes(:noteable) + .fresh + + notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) } + discussions = Kaminari.paginate_array(Discussion.build_collection(notes, noteable)) + + present paginate(discussions), with: Entities::Discussion + end + + desc "Get a single #{noteable_type.to_s.downcase} discussion" do + success Entities::Discussion + end + params do + requires :discussion_id, type: String, desc: 'The ID of a discussion' + requires :noteable_id, type: Integer, desc: 'The ID of the noteable' + end + get ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id" do + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + notes = readable_discussion_notes(noteable, params[:discussion_id]) + + if notes.empty? || !can?(current_user, noteable_read_ability_name(noteable), noteable) + return not_found!("Discussion") + end + + discussion = Discussion.build(notes, noteable) + + present discussion, with: Entities::Discussion + end + + desc "Create a new #{noteable_type.to_s.downcase} discussion" do + success Entities::Discussion + end + params do + requires :noteable_id, type: Integer, desc: 'The ID of the noteable' + requires :body, type: String, desc: 'The content of a note' + optional :created_at, type: String, desc: 'The creation date of the note' + end + post ":id/#{noteables_str}/:noteable_id/discussions" do + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + + opts = { + note: params[:body], + created_at: params[:created_at], + type: 'DiscussionNote', + noteable_type: noteables_str.classify, + noteable_id: noteable.id + } + + note = create_note(noteable, opts) + + if note.valid? + present note.discussion, with: Entities::Discussion + else + bad_request!("Note #{note.errors.messages}") + end + end + + desc "Get comments in a single #{noteable_type.to_s.downcase} discussion" do + success Entities::Discussion + end + params do + requires :discussion_id, type: String, desc: 'The ID of a discussion' + requires :noteable_id, type: Integer, desc: 'The ID of the noteable' + end + get ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes" do + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + notes = readable_discussion_notes(noteable, params[:discussion_id]) + + if notes.empty? || !can?(current_user, noteable_read_ability_name(noteable), noteable) + return not_found!("Notes") + end + + present notes, with: Entities::Note + end + + desc "Add a comment to a #{noteable_type.to_s.downcase} discussion" do + success Entities::Note + end + params do + requires :noteable_id, type: Integer, desc: 'The ID of the noteable' + requires :discussion_id, type: String, desc: 'The ID of a discussion' + requires :body, type: String, desc: 'The content of a note' + optional :created_at, type: String, desc: 'The creation date of the note' + end + post ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes" do + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + notes = readable_discussion_notes(noteable, params[:discussion_id]) + + return not_found!("Discussion") if notes.empty? + return bad_request!("Discussion is an individual note.") unless notes.first.part_of_discussion? + + opts = { + note: params[:body], + type: 'DiscussionNote', + in_reply_to_discussion_id: params[:discussion_id], + created_at: params[:created_at] + } + note = create_note(noteable, opts) + + if note.valid? + present note, with: Entities::Note + else + bad_request!("Note #{note.errors.messages}") + end + end + + desc "Get a comment in a #{noteable_type.to_s.downcase} discussion" do + success Entities::Note + end + params do + requires :noteable_id, type: Integer, desc: 'The ID of the noteable' + requires :discussion_id, type: String, desc: 'The ID of a discussion' + requires :note_id, type: Integer, desc: 'The ID of a note' + end + get ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes/:note_id" do + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + + get_note(noteable, params[:note_id]) + end + + desc "Edit a comment in a #{noteable_type.to_s.downcase} discussion" do + success Entities::Note + end + params do + requires :noteable_id, type: Integer, desc: 'The ID of the noteable' + requires :discussion_id, type: String, desc: 'The ID of a discussion' + requires :note_id, type: Integer, desc: 'The ID of a note' + requires :body, type: String, desc: 'The content of a note' + end + put ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes/:note_id" do + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + + update_note(noteable, params[:note_id]) + end + + desc "Delete a comment in a #{noteable_type.to_s.downcase} discussion" do + success Entities::Note + end + params do + requires :noteable_id, type: Integer, desc: 'The ID of the noteable' + requires :discussion_id, type: String, desc: 'The ID of a discussion' + requires :note_id, type: Integer, desc: 'The ID of a note' + end + delete ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes/:note_id" do + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + + delete_note(noteable, params[:note_id]) + end + end + end + + helpers do + def readable_discussion_notes(noteable, discussion_id) + notes = noteable.notes + .where(discussion_id: discussion_id) + .inc_relations_for_view + .includes(:noteable) + .fresh + + notes.reject { |n| n.cross_reference_not_visible_for?(current_user) } + end + end + end +end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index f39906270d8..4555184095c 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -644,6 +644,7 @@ module API NOTEABLE_TYPES_WITH_IID = %w(Issue MergeRequest).freeze expose :id + expose :type expose :note, as: :body expose :attachment_identifier, as: :attachment expose :author, using: Entities::UserBasic @@ -655,6 +656,12 @@ module API expose(:noteable_iid) { |note| note.noteable.iid if NOTEABLE_TYPES_WITH_IID.include?(note.noteable_type) } end + class Discussion < Grape::Entity + expose :id + expose :individual_note?, as: :individual_note + expose :notes, using: Entities::Note + end + class AwardEmoji < Grape::Entity expose :id expose :name diff --git a/lib/api/helpers/notes_helpers.rb b/lib/api/helpers/notes_helpers.rb new file mode 100644 index 00000000000..cd91df1ecd8 --- /dev/null +++ b/lib/api/helpers/notes_helpers.rb @@ -0,0 +1,76 @@ +module API + module Helpers + module NotesHelpers + def update_note(noteable, note_id) + note = noteable.notes.find(params[:note_id]) + + authorize! :admin_note, note + + opts = { + note: params[:body] + } + parent = noteable_parent(noteable) + project = parent if parent.is_a?(Project) + + note = ::Notes::UpdateService.new(project, current_user, opts).execute(note) + + if note.valid? + present note, with: Entities::Note + else + bad_request!("Failed to save note #{note.errors.messages}") + end + end + + def delete_note(noteable, note_id) + note = noteable.notes.find(note_id) + + authorize! :admin_note, note + + parent = noteable_parent(noteable) + project = parent if parent.is_a?(Project) + destroy_conditionally!(note) do |note| + ::Notes::DestroyService.new(project, current_user).execute(note) + end + end + + def get_note(noteable, note_id) + note = noteable.notes.with_metadata.find(params[:note_id]) + can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user) + + if can_read_note + present note, with: Entities::Note + else + not_found!("Note") + end + end + + def noteable_read_ability_name(noteable) + "read_#{noteable.class.to_s.underscore}".to_sym + end + + def find_noteable(parent, noteables_str, noteable_id) + public_send("find_#{parent}_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend + end + + def noteable_parent(noteable) + public_send("user_#{noteable.class.parent_class.to_s.underscore}") # rubocop:disable GitlabSecurity/PublicSend + end + + def create_note(noteable, opts) + noteables_str = noteable.model_name.to_s.underscore.pluralize + + return not_found!(noteables_str) unless can?(current_user, noteable_read_ability_name(noteable), noteable) + + authorize! :create_note, noteable + + parent = noteable_parent(noteable) + if opts[:created_at] + opts.delete(:created_at) unless current_user.admin? || parent.owner == current_user + end + + project = parent if parent.is_a?(Project) + ::Notes::CreateService.new(project, current_user, opts).execute + end + end + end +end diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 3588dc85c9e..69f1df6b341 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -1,19 +1,23 @@ module API class Notes < Grape::API include PaginationParams + helpers ::API::Helpers::NotesHelpers before { authenticate! } NOTEABLE_TYPES = [Issue, MergeRequest, Snippet].freeze - params do - requires :id, type: String, desc: 'The ID of a project' - end - resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do - NOTEABLE_TYPES.each do |noteable_type| + NOTEABLE_TYPES.each do |noteable_type| + parent_type = noteable_type.parent_class.to_s.underscore + noteables_str = noteable_type.to_s.underscore.pluralize + + params do + requires :id, type: String, desc: "The ID of a #{parent_type}" + end + resource parent_type.pluralize.to_sym, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do noteables_str = noteable_type.to_s.underscore.pluralize - desc 'Get a list of project +noteable+ notes' do + desc "Get a list of #{noteable_type.to_s.downcase} notes" do success Entities::Note end params do @@ -25,7 +29,7 @@ module API use :pagination end get ":id/#{noteables_str}/:noteable_id/notes" do - noteable = find_project_noteable(noteables_str, params[:noteable_id]) + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) if can?(current_user, noteable_read_ability_name(noteable), noteable) # We exclude notes that are cross-references and that cannot be viewed @@ -46,7 +50,7 @@ module API end end - desc 'Get a single +noteable+ note' do + desc "Get a single #{noteable_type.to_s.downcase} note" do success Entities::Note end params do @@ -54,18 +58,11 @@ module API requires :noteable_id, type: Integer, desc: 'The ID of the noteable' end get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do - noteable = find_project_noteable(noteables_str, params[:noteable_id]) - note = noteable.notes.with_metadata.find(params[:note_id]) - can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user) - - if can_read_note - present note, with: Entities::Note - else - not_found!("Note") - end + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + get_note(noteable, params[:note_id]) end - desc 'Create a new +noteable+ note' do + desc "Create a new #{noteable_type.to_s.downcase} note" do success Entities::Note end params do @@ -74,34 +71,25 @@ module API optional :created_at, type: String, desc: 'The creation date of the note' end post ":id/#{noteables_str}/:noteable_id/notes" do - noteable = find_project_noteable(noteables_str, params[:noteable_id]) + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) opts = { note: params[:body], noteable_type: noteables_str.classify, - noteable_id: noteable.id + noteable_id: noteable.id, + created_at: params[:created_at] } - if can?(current_user, noteable_read_ability_name(noteable), noteable) - authorize! :create_note, noteable + note = create_note(noteable, opts) - if params[:created_at] && (current_user.admin? || user_project.owner == current_user) - opts[:created_at] = params[:created_at] - end - - note = ::Notes::CreateService.new(user_project, current_user, opts).execute - - if note.valid? - present note, with: Entities.const_get(note.class.name) - else - not_found!("Note #{note.errors.messages}") - end + if note.valid? + present note, with: Entities.const_get(note.class.name) else - not_found!("Note") + bad_request!("Note #{note.errors.messages}") end end - desc 'Update an existing +noteable+ note' do + desc "Update an existing #{noteable_type.to_s.downcase} note" do success Entities::Note end params do @@ -110,24 +98,12 @@ module API requires :body, type: String, desc: 'The content of a note' end put ":id/#{noteables_str}/:noteable_id/notes/:note_id" do - note = user_project.notes.find(params[:note_id]) + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) - authorize! :admin_note, note - - opts = { - note: params[:body] - } - - note = ::Notes::UpdateService.new(user_project, current_user, opts).execute(note) - - if note.valid? - present note, with: Entities::Note - else - render_api_error!("Failed to save note #{note.errors.messages}", 400) - end + update_note(noteable, params[:note_id]) end - desc 'Delete a +noteable+ note' do + desc "Delete a #{noteable_type.to_s.downcase} note" do success Entities::Note end params do @@ -135,25 +111,11 @@ module API requires :note_id, type: Integer, desc: 'The ID of a note' end delete ":id/#{noteables_str}/:noteable_id/notes/:note_id" do - note = user_project.notes.find(params[:note_id]) - - authorize! :admin_note, note + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) - destroy_conditionally!(note) do |note| - ::Notes::DestroyService.new(user_project, current_user).execute(note) - end + delete_note(noteable, params[:note_id]) end end end - - helpers do - def find_project_noteable(noteables_str, noteable_id) - public_send("find_project_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend - end - - def noteable_read_ability_name(noteable) - "read_#{noteable.class.to_s.underscore}".to_sym - end - end end end diff --git a/lib/banzai/filter/markdown_engines/common_mark.rb b/lib/banzai/filter/markdown_engines/common_mark.rb new file mode 100644 index 00000000000..bc9597df894 --- /dev/null +++ b/lib/banzai/filter/markdown_engines/common_mark.rb @@ -0,0 +1,45 @@ +# `CommonMark` markdown engine for GitLab's Banzai markdown filter. +# This module is used in Banzai::Filter::MarkdownFilter. +# Used gem is `commonmarker` which is a ruby wrapper for libcmark (CommonMark parser) +# including GitHub's GFM extensions. +# Homepage: https://github.com/gjtorikian/commonmarker + +module Banzai + module Filter + module MarkdownEngines + class CommonMark + EXTENSIONS = [ + :autolink, # provides support for automatically converting URLs to anchor tags. + :strikethrough, # provides support for strikethroughs. + :table, # provides support for tables. + :tagfilter # strips out several "unsafe" HTML tags from being used: https://github.github.com/gfm/#disallowed-raw-html-extension- + ].freeze + + PARSE_OPTIONS = [ + :FOOTNOTES, # parse footnotes. + :STRIKETHROUGH_DOUBLE_TILDE, # parse strikethroughs by double tildes (as redcarpet does). + :VALIDATE_UTF8 # replace illegal sequences with the replacement character U+FFFD. + ].freeze + + # The `:GITHUB_PRE_LANG` option is not used intentionally because + # it renders a fence block with language as `<pre lang="LANG"><code>some code\n</code></pre>` + # while GitLab's syntax is `<pre><code lang="LANG">some code\n</code></pre>`. + # If in the future the syntax is about to be made GitHub-compatible, please, add `:GITHUB_PRE_LANG` render option below + # and remove `code_block` method from `lib/banzai/renderer/common_mark/html.rb`. + RENDER_OPTIONS = [ + :DEFAULT # default rendering system. Nothing special. + ].freeze + + def initialize + @renderer = Banzai::Renderer::CommonMark::HTML.new(options: RENDER_OPTIONS) + end + + def render(text) + doc = CommonMarker.render_doc(text, PARSE_OPTIONS, EXTENSIONS) + + @renderer.render(doc) + end + end + end + end +end diff --git a/lib/banzai/filter/markdown_engines/redcarpet.rb b/lib/banzai/filter/markdown_engines/redcarpet.rb new file mode 100644 index 00000000000..ac99941fefa --- /dev/null +++ b/lib/banzai/filter/markdown_engines/redcarpet.rb @@ -0,0 +1,32 @@ +# `Redcarpet` markdown engine for GitLab's Banzai markdown filter. +# This module is used in Banzai::Filter::MarkdownFilter. +# Used gem is `redcarpet` which is a ruby library for markdown processing. +# Homepage: https://github.com/vmg/redcarpet + +module Banzai + module Filter + module MarkdownEngines + class Redcarpet + OPTIONS = { + fenced_code_blocks: true, + footnotes: true, + lax_spacing: true, + no_intra_emphasis: true, + space_after_headers: true, + strikethrough: true, + superscript: true, + tables: true + }.freeze + + def initialize + html_renderer = Banzai::Renderer::Redcarpet::HTML.new + @renderer = ::Redcarpet::Markdown.new(html_renderer, OPTIONS) + end + + def render(text) + @renderer.render(text) + end + end + end + end +end diff --git a/lib/banzai/filter/markdown_filter.rb b/lib/banzai/filter/markdown_filter.rb index 9cac303e645..c1e2b680240 100644 --- a/lib/banzai/filter/markdown_filter.rb +++ b/lib/banzai/filter/markdown_filter.rb @@ -1,34 +1,31 @@ module Banzai module Filter class MarkdownFilter < HTML::Pipeline::TextFilter - # https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use - REDCARPET_OPTIONS = { - fenced_code_blocks: true, - footnotes: true, - lax_spacing: true, - no_intra_emphasis: true, - space_after_headers: true, - strikethrough: true, - superscript: true, - tables: true - }.freeze - def initialize(text, context = nil, result = nil) - super text, context, result - @text = @text.delete "\r" + super(text, context, result) + + @renderer = renderer(context[:markdown_engine]).new + @text = @text.delete("\r") end def call - html = self.class.renderer.render(@text) - html.rstrip! - html + @renderer.render(@text).rstrip + end + + private + + DEFAULT_ENGINE = :redcarpet + + def engine(engine_from_context) + engine_from_context ||= DEFAULT_ENGINE + + engine_from_context.to_s.classify end - def self.renderer - Thread.current[:banzai_markdown_renderer] ||= begin - renderer = Banzai::Renderer::HTML.new - Redcarpet::Markdown.new(renderer, REDCARPET_OPTIONS) - end + def renderer(engine_from_context) + "Banzai::Filter::MarkdownEngines::#{engine(engine_from_context)}".constantize + rescue NameError + raise NameError, "`#{engine_from_context}` is unknown markdown engine" end end end diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 0ac7e231b5b..6dbf0d68fe8 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -1,3 +1,4 @@ +require 'rouge/plugins/common_mark' require 'rouge/plugins/redcarpet' module Banzai diff --git a/lib/banzai/renderer/common_mark/html.rb b/lib/banzai/renderer/common_mark/html.rb new file mode 100644 index 00000000000..c7a54629f31 --- /dev/null +++ b/lib/banzai/renderer/common_mark/html.rb @@ -0,0 +1,21 @@ +module Banzai + module Renderer + module CommonMark + class HTML < CommonMarker::HtmlRenderer + def code_block(node) + block do + code = node.string_content + lang = node.fence_info + lang_attr = lang.present? ? %Q{ lang="#{lang}"} : '' + result = + "<pre>" \ + "<code#{lang_attr}>#{html_escape(code)}</code>" \ + "</pre>" + + out(result) + end + end + end + end + end +end diff --git a/lib/banzai/renderer/html.rb b/lib/banzai/renderer/html.rb deleted file mode 100644 index 252caa35947..00000000000 --- a/lib/banzai/renderer/html.rb +++ /dev/null @@ -1,13 +0,0 @@ -module Banzai - module Renderer - class HTML < Redcarpet::Render::HTML - def block_code(code, lang) - lang_attr = lang ? %Q{ lang="#{lang}"} : '' - - "\n<pre>" \ - "<code#{lang_attr}>#{html_escape(code)}</code>" \ - "</pre>" - end - end - end -end diff --git a/lib/banzai/renderer/redcarpet/html.rb b/lib/banzai/renderer/redcarpet/html.rb new file mode 100644 index 00000000000..94df5d8b1e1 --- /dev/null +++ b/lib/banzai/renderer/redcarpet/html.rb @@ -0,0 +1,15 @@ +module Banzai + module Renderer + module Redcarpet + class HTML < ::Redcarpet::Render::HTML + def block_code(code, lang) + lang_attr = lang ? %Q{ lang="#{lang}"} : '' + + "\n<pre>" \ + "<code#{lang_attr}>#{html_escape(code)}</code>" \ + "</pre>" + end + end + end + end +end diff --git a/lib/gitlab/contributions_calendar.rb b/lib/gitlab/contributions_calendar.rb index 02d3763514e..d7369060cc5 100644 --- a/lib/gitlab/contributions_calendar.rb +++ b/lib/gitlab/contributions_calendar.rb @@ -23,7 +23,7 @@ module Gitlab mr_events = event_counts(date_from, :merge_requests) .having(action: [Event::MERGED, Event::CREATED, Event::CLOSED], target_type: "MergeRequest") note_events = event_counts(date_from, :merge_requests) - .having(action: [Event::COMMENTED], target_type: %w(Note DiffNote)) + .having(action: [Event::COMMENTED]) union = Gitlab::SQL::Union.new([repo_events, issue_events, mr_events, note_events]) events = Event.find_by_sql(union.to_sql).map(&:attributes) diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 594b6a9cbc5..93037ed8d90 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -347,7 +347,7 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/324 def to_diff - Gitlab::GitalyClient.migrate(:commit_patch) do |is_enabled| + Gitlab::GitalyClient.migrate(:commit_patch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| if is_enabled @repository.gitaly_commit_client.patch(id) else diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 21c79a7a550..1445b7924dc 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -228,7 +228,7 @@ module Gitlab end def has_local_branches? - gitaly_migrate(:has_local_branches) do |is_enabled| + gitaly_migrate(:has_local_branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| if is_enabled gitaly_repository_client.has_local_branches? else @@ -715,7 +715,7 @@ module Gitlab end def add_branch(branch_name, user:, target:) - gitaly_migrate(:operation_user_create_branch) do |is_enabled| + gitaly_migrate(:operation_user_create_branch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| if is_enabled gitaly_add_branch(branch_name, user, target) else @@ -725,7 +725,7 @@ module Gitlab end def add_tag(tag_name, user:, target:, message: nil) - gitaly_migrate(:operation_user_add_tag) do |is_enabled| + gitaly_migrate(:operation_user_add_tag, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| if is_enabled gitaly_add_tag(tag_name, user: user, target: target, message: message) else @@ -735,7 +735,7 @@ module Gitlab end def rm_branch(branch_name, user:) - gitaly_migrate(:operation_user_delete_branch) do |is_enabled| + gitaly_migrate(:operation_user_delete_branch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| if is_enabled gitaly_operations_client.user_delete_branch(branch_name, user) else @@ -810,7 +810,7 @@ module Gitlab end def revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:) - gitaly_migrate(:revert) do |is_enabled| + gitaly_migrate(:revert, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| args = { user: user, commit: commit, @@ -876,7 +876,7 @@ module Gitlab # Delete the specified branch from the repository def delete_branch(branch_name) - gitaly_migrate(:delete_branch) do |is_enabled| + gitaly_migrate(:delete_branch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| if is_enabled gitaly_ref_client.delete_branch(branch_name) else @@ -903,7 +903,7 @@ module Gitlab # create_branch("feature") # create_branch("other-feature", "master") def create_branch(ref, start_point = "HEAD") - gitaly_migrate(:create_branch) do |is_enabled| + gitaly_migrate(:create_branch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| if is_enabled gitaly_ref_client.create_branch(ref, start_point) else @@ -1010,7 +1010,7 @@ module Gitlab end def languages(ref = nil) - Gitlab::GitalyClient.migrate(:commit_languages) do |is_enabled| + gitaly_migrate(:commit_languages, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| if is_enabled gitaly_commit_client.languages(ref) else diff --git a/lib/rouge/plugins/common_mark.rb b/lib/rouge/plugins/common_mark.rb new file mode 100644 index 00000000000..8f9de061124 --- /dev/null +++ b/lib/rouge/plugins/common_mark.rb @@ -0,0 +1,20 @@ +# A rouge plugin for CommonMark markdown engine. +# Used to highlight code generated by CommonMark. + +module Rouge + module Plugins + module CommonMark + def code_block(code, language) + lexer = Lexer.find_fancy(language, code) || Lexers::PlainText + + formatter = rouge_formatter(lexer) + formatter.format(lexer.lex(code)) + end + + # override this method for custom formatting behavior + def rouge_formatter(lexer) + Formatters::HTMLLegacy.new(css_class: "highlight #{lexer.tag}") + end + end + end +end diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 3f4e408b3a6..857333f222d 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -16,6 +16,8 @@ FactoryBot.define do factory :note_on_personal_snippet, traits: [:on_personal_snippet] factory :system_note, traits: [:system] + factory :discussion_note, class: DiscussionNote + factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: DiscussionNote do association :project, :repository @@ -31,6 +33,8 @@ FactoryBot.define do factory :discussion_note_on_personal_snippet, traits: [:on_personal_snippet], class: DiscussionNote + factory :discussion_note_on_snippet, traits: [:on_snippet], class: DiscussionNote + factory :legacy_diff_note_on_commit, traits: [:on_commit, :legacy_diff_note], class: LegacyDiffNote factory :legacy_diff_note_on_merge_request, traits: [:on_merge_request, :legacy_diff_note], class: LegacyDiffNote do @@ -96,6 +100,10 @@ FactoryBot.define do noteable { create(:issue, project: project) } end + trait :on_snippet do + noteable { create(:snippet, project: project) } + end + trait :on_merge_request do noteable { create(:merge_request, source_project: project) } end diff --git a/spec/features/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb index f266f2ecc54..25ed3bdc88e 100644 --- a/spec/features/admin/admin_hooks_spec.rb +++ b/spec/features/admin/admin_hooks_spec.rb @@ -24,6 +24,16 @@ describe 'Admin::Hooks' do visit admin_hooks_path expect(page).to have_content(system_hook.url) end + + it 'renders plugins list as well' do + allow(Gitlab::Plugin).to receive(:files).and_return(['foo.rb', 'bar.clj']) + + visit admin_hooks_path + + expect(page).to have_content('Plugins') + expect(page).to have_content('foo.rb') + expect(page).to have_content('bar.clj') + end end describe 'New Hook' do diff --git a/spec/fixtures/api/schemas/public_api/v4/notes.json b/spec/fixtures/api/schemas/public_api/v4/notes.json index 6525f7c2c80..4c4ca3b582f 100644 --- a/spec/fixtures/api/schemas/public_api/v4/notes.json +++ b/spec/fixtures/api/schemas/public_api/v4/notes.json @@ -4,6 +4,7 @@ "type": "object", "properties" : { "id": { "type": "integer" }, + "type": { "type": ["string", "null"] }, "body": { "type": "string" }, "attachment": { "type": ["string", "null"] }, "author": { diff --git a/spec/javascripts/vue_shared/components/clipboard_button_spec.js b/spec/javascripts/vue_shared/components/clipboard_button_spec.js index d0fc10d69ea..f598b1afa74 100644 --- a/spec/javascripts/vue_shared/components/clipboard_button_spec.js +++ b/spec/javascripts/vue_shared/components/clipboard_button_spec.js @@ -10,6 +10,7 @@ describe('clipboard button', () => { vm = mountComponent(Component, { text: 'copy me', title: 'Copy this value into Clipboard!', + cssClass: 'btn-danger', }); }); @@ -28,4 +29,8 @@ describe('clipboard button', () => { expect(vm.$el.getAttribute('data-placement')).toEqual('top'); expect(vm.$el.getAttribute('data-container')).toEqual(null); }); + + it('should render provided classname', () => { + expect(vm.$el.classList).toContain('btn-danger'); + }); }); diff --git a/spec/lib/gitlab/contributions_calendar_spec.rb b/spec/lib/gitlab/contributions_calendar_spec.rb index 167876ca158..2c63f3b0455 100644 --- a/spec/lib/gitlab/contributions_calendar_spec.rb +++ b/spec/lib/gitlab/contributions_calendar_spec.rb @@ -77,6 +77,13 @@ describe Gitlab::ContributionsCalendar do expect(calendar(contributor).activity_dates[today]).to eq(1) end + it "counts the discussions on merge requests and issues" do + create_event(public_project, today, 0, Event::COMMENTED, :discussion_note_on_merge_request) + create_event(public_project, today, 2, Event::COMMENTED, :discussion_note_on_issue) + + expect(calendar(contributor).activity_dates[today]).to eq(2) + end + context "when events fall under different dates depending on the time zone" do before do create_event(public_project, today, 1) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index ceb570ac777..412eca4a56b 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -142,15 +142,15 @@ describe Environment do let(:commit) { project.commit.parent } it 'returns deployment id for the environment' do - expect(environment.first_deployment_for(commit)).to eq deployment1 + expect(environment.first_deployment_for(commit.id)).to eq deployment1 end it 'return nil when no deployment is found' do - expect(environment.first_deployment_for(head_commit)).to eq nil + expect(environment.first_deployment_for(head_commit.id)).to eq nil end it 'returns a UTF-8 ref' do - expect(environment.first_deployment_for(commit).ref).to be_utf8 + expect(environment.first_deployment_for(commit.id).ref).to be_utf8 end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 67f49348acb..8ea92410022 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -49,6 +49,22 @@ describe Event do end end end + + describe 'after_create :track_user_interacted_projects' do + let(:event) { build(:push_event, project: project, author: project.owner) } + + it 'passes event to UserInteractedProject.track' do + expect(UserInteractedProject).to receive(:available?).and_return(true) + expect(UserInteractedProject).to receive(:track).with(event) + event.save + end + + it 'does not call UserInteractedProject.track if its not yet available' do + expect(UserInteractedProject).to receive(:available?).and_return(false) + expect(UserInteractedProject).not_to receive(:track) + event.save + end + end end describe "Push event" do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 38653e18306..579069ffa14 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1004,7 +1004,7 @@ describe Repository do end end - context 'with Gitaly disabled', :skip_gitaly_mock do + context 'with Gitaly disabled', :disable_gitaly do context 'when pre hooks were successful' do it 'runs without errors' do hook = double(trigger: [true, nil]) @@ -1896,7 +1896,7 @@ describe Repository do it_behaves_like 'adding tag' end - context 'when Gitaly operation_user_add_tag feature is disabled', :skip_gitaly_mock do + context 'when Gitaly operation_user_add_tag feature is disabled', :disable_gitaly do it_behaves_like 'adding tag' it 'passes commit SHA to pre-receive and update hooks and tag SHA to post-receive hook' do @@ -1955,7 +1955,7 @@ describe Repository do end end - context 'with gitaly disabled', :skip_gitaly_mock do + context 'with gitaly disabled', :disable_gitaly do it_behaves_like "user deleting a branch" let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature diff --git a/spec/models/user_interacted_project_spec.rb b/spec/models/user_interacted_project_spec.rb new file mode 100644 index 00000000000..cb4bb3372d4 --- /dev/null +++ b/spec/models/user_interacted_project_spec.rb @@ -0,0 +1,60 @@ +require 'spec_helper' + +describe UserInteractedProject do + describe '.track' do + subject { described_class.track(event) } + let(:event) { build(:event) } + + Event::ACTIONS.each do |action| + context "for all actions (event types)" do + let(:event) { build(:event, action: action) } + it 'creates a record' do + expect { subject }.to change { described_class.count }.from(0).to(1) + end + end + end + + it 'sets project accordingly' do + subject + expect(described_class.first.project).to eq(event.project) + end + + it 'sets user accordingly' do + subject + expect(described_class.first.user).to eq(event.author) + end + + it 'only creates a record once per user/project' do + expect do + subject + described_class.track(event) + end.to change { described_class.count }.from(0).to(1) + end + + describe 'with an event without a project' do + let(:event) { build(:event, project: nil) } + + it 'ignores the event' do + expect { subject }.not_to change { described_class.count } + end + end + end + + describe '.available?' do + before do + described_class.instance_variable_set('@available_flag', nil) + end + + it 'checks schema version and properly caches positive result' do + expect(ActiveRecord::Migrator).to receive(:current_version).and_return(described_class::REQUIRED_SCHEMA_VERSION - 1 - rand(1000)) + expect(described_class.available?).to be_falsey + expect(ActiveRecord::Migrator).to receive(:current_version).and_return(described_class::REQUIRED_SCHEMA_VERSION + rand(1000)) + expect(described_class.available?).to be_truthy + expect(ActiveRecord::Migrator).not_to receive(:current_version) + expect(described_class.available?).to be_truthy # cached response + end + end + + it { is_expected.to validate_presence_of(:project_id) } + it { is_expected.to validate_presence_of(:user_id) } +end diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb new file mode 100644 index 00000000000..4a44b219a67 --- /dev/null +++ b/spec/requests/api/discussions_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe API::Discussions do + let(:user) { create(:user) } + let!(:project) { create(:project, :public, namespace: user.namespace) } + let(:private_user) { create(:user) } + + before do + project.add_reporter(user) + end + + context "when noteable is an Issue" do + let!(:issue) { create(:issue, project: project, author: user) } + let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) } + + it_behaves_like "discussions API", 'projects', 'issues', 'iid' do + let(:parent) { project } + let(:noteable) { issue } + let(:note) { issue_note } + end + end + + context "when noteable is a Snippet" do + let!(:snippet) { create(:project_snippet, project: project, author: user) } + let!(:snippet_note) { create(:discussion_note_on_snippet, noteable: snippet, project: project, author: user) } + + it_behaves_like "discussions API", 'projects', 'snippets', 'id' do + let(:parent) { project } + let(:noteable) { snippet } + let(:note) { snippet_note } + end + end +end diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 981c9c27325..dd568c24c72 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -3,117 +3,86 @@ require 'spec_helper' describe API::Notes do let(:user) { create(:user) } let!(:project) { create(:project, :public, namespace: user.namespace) } - let!(:issue) { create(:issue, project: project, author: user) } - let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } - let!(:snippet) { create(:project_snippet, project: project, author: user) } - let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) } - let!(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) } - let!(:snippet_note) { create(:note, noteable: snippet, project: project, author: user) } - - # For testing the cross-reference of a private issue in a public issue let(:private_user) { create(:user) } - let(:private_project) do - create(:project, namespace: private_user.namespace) - .tap { |p| p.add_master(private_user) } - end - let(:private_issue) { create(:issue, project: private_project) } - - let(:ext_proj) { create(:project, :public) } - let(:ext_issue) { create(:issue, project: ext_proj) } - - let!(:cross_reference_note) do - create :note, - noteable: ext_issue, project: ext_proj, - note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", - system: true - end before do project.add_reporter(user) end - describe "GET /projects/:id/noteable/:noteable_id/notes" do - context "when noteable is an Issue" do - context 'sorting' do - before do - create_list(:note, 3, noteable: issue, project: project, author: user) - end - - it 'sorts by created_at in descending order by default' do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes", user) - - response_dates = json_response.map { |noteable| noteable['created_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort.reverse) - end - - it 'sorts by ascending order when requested' do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes?sort=asc", user) - - response_dates = json_response.map { |noteable| noteable['created_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort) - end - - it 'sorts by updated_at in descending order when requested' do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes?order_by=updated_at", user) - - response_dates = json_response.map { |noteable| noteable['updated_at'] } + context "when noteable is an Issue" do + let!(:issue) { create(:issue, project: project, author: user) } + let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) } - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort.reverse) - end + it_behaves_like "noteable API", 'projects', 'issues', 'iid' do + let(:parent) { project } + let(:noteable) { issue } + let(:note) { issue_note } + end - it 'sorts by updated_at in ascending order when requested' do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes??order_by=updated_at&sort=asc", user) + context 'when user does not have access to create noteable' do + let(:private_issue) { create(:issue, project: create(:project, :private)) } - response_dates = json_response.map { |noteable| noteable['updated_at'] } + ## + # We are posting to project user has access to, but we use issue id + # from a different project, see #15577 + # + before do + post api("/projects/#{private_issue.project.id}/issues/#{private_issue.iid}/notes", user), + body: 'Hi!' + end - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort) - end + it 'responds with resource not found error' do + expect(response.status).to eq 404 end - it "returns an array of issue notes" do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes", user) + it 'does not create new note' do + expect(private_issue.notes.reload).to be_empty + end + end - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.first['body']).to eq(issue_note.note) + context "when referencing other project" do + # For testing the cross-reference of a private issue in a public project + let(:private_project) do + create(:project, namespace: private_user.namespace) + .tap { |p| p.add_master(private_user) } end + let(:private_issue) { create(:issue, project: private_project) } - it "returns a 404 error when issue id not found" do - get api("/projects/#{project.id}/issues/12345/notes", user) + let(:ext_proj) { create(:project, :public) } + let(:ext_issue) { create(:issue, project: ext_proj) } - expect(response).to have_gitlab_http_status(404) + let!(:cross_reference_note) do + create :note, + noteable: ext_issue, project: ext_proj, + note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", + system: true end - context "and current user cannot view the notes" do - it "returns an empty array" do - get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", user) - - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response).to be_empty - end + describe "GET /projects/:id/noteable/:noteable_id/notes" do + context "current user cannot view the notes" do + it "returns an empty array" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", user) - context "and issue is confidential" do - before do - ext_issue.update_attributes(confidential: true) + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response).to be_empty end - it "returns 404" do - get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", user) + context "issue is confidential" do + before do + ext_issue.update_attributes(confidential: true) + end - expect(response).to have_gitlab_http_status(404) + it "returns 404" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", user) + + expect(response).to have_gitlab_http_status(404) + end end end - context "and current user can view the note" do + context "current user can view the note" do it "returns an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", private_user) @@ -124,172 +93,29 @@ describe API::Notes do end end end - end - - context "when noteable is a Snippet" do - context 'sorting' do - before do - create_list(:note, 3, noteable: snippet, project: project, author: user) - end - - it 'sorts by created_at in descending order by default' do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user) - - response_dates = json_response.map { |noteable| noteable['created_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort.reverse) - end - - it 'sorts by ascending order when requested' do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes?sort=asc", user) - - response_dates = json_response.map { |noteable| noteable['created_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort) - end - - it 'sorts by updated_at in descending order when requested' do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes?order_by=updated_at", user) - - response_dates = json_response.map { |noteable| noteable['updated_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort.reverse) - end - it 'sorts by updated_at in ascending order when requested' do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes??order_by=updated_at&sort=asc", user) + describe "GET /projects/:id/noteable/:noteable_id/notes/:note_id" do + context "current user cannot view the notes" do + it "returns a 404 error" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes/#{cross_reference_note.id}", user) - response_dates = json_response.map { |noteable| noteable['updated_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort) - end - end - it "returns an array of snippet notes" do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user) - - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.first['body']).to eq(snippet_note.note) - end - - it "returns a 404 error when snippet id not found" do - get api("/projects/#{project.id}/snippets/42/notes", user) - - expect(response).to have_gitlab_http_status(404) - end - - it "returns 404 when not authorized" do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", private_user) - - expect(response).to have_gitlab_http_status(404) - end - end - - context "when noteable is a Merge Request" do - context 'sorting' do - before do - create_list(:note, 3, noteable: merge_request, project: project, author: user) - end - - it 'sorts by created_at in descending order by default' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes", user) - - response_dates = json_response.map { |noteable| noteable['created_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort.reverse) - end - - it 'sorts by ascending order when requested' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes?sort=asc", user) - - response_dates = json_response.map { |noteable| noteable['created_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort) - end - - it 'sorts by updated_at in descending order when requested' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes?order_by=updated_at", user) - - response_dates = json_response.map { |noteable| noteable['updated_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort.reverse) - end - - it 'sorts by updated_at in ascending order when requested' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes??order_by=updated_at&sort=asc", user) - - response_dates = json_response.map { |noteable| noteable['updated_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort) - end - end - it "returns an array of merge_requests notes" do - get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes", user) - - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.first['body']).to eq(merge_request_note.note) - end - - it "returns a 404 error if merge request id not found" do - get api("/projects/#{project.id}/merge_requests/4444/notes", user) - - expect(response).to have_gitlab_http_status(404) - end - - it "returns 404 when not authorized" do - get api("/projects/#{project.id}/merge_requests/4444/notes", private_user) - - expect(response).to have_gitlab_http_status(404) - end - end - end - - describe "GET /projects/:id/noteable/:noteable_id/notes/:note_id" do - context "when noteable is an Issue" do - it "returns an issue note by id" do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response['body']).to eq(issue_note.note) - end - - it "returns a 404 error if issue note not found" do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - - context "and current user cannot view the note" do - it "returns a 404 error" do - get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes/#{cross_reference_note.id}", user) - - expect(response).to have_gitlab_http_status(404) - end - - context "when issue is confidential" do - before do - issue.update_attributes(confidential: true) + expect(response).to have_gitlab_http_status(404) end - it "returns 404" do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", private_user) + context "when issue is confidential" do + before do + issue.update_attributes(confidential: true) + end - expect(response).to have_gitlab_http_status(404) + it "returns 404" do + get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", private_user) + + expect(response).to have_gitlab_http_status(404) + end end end - context "and current user can view the note" do + context "current user can view the note" do it "returns an issue note by id" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes/#{cross_reference_note.id}", private_user) @@ -299,132 +125,27 @@ describe API::Notes do end end end - - context "when noteable is a Snippet" do - it "returns a snippet note by id" do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/#{snippet_note.id}", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response['body']).to eq(snippet_note.note) - end - - it "returns a 404 error if snippet note not found" do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - end end - describe "POST /projects/:id/noteable/:noteable_id/notes" do - context "when noteable is an Issue" do - it "creates a new issue note" do - post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), body: 'hi!' - - expect(response).to have_gitlab_http_status(201) - expect(json_response['body']).to eq('hi!') - expect(json_response['author']['username']).to eq(user.username) - end - - it "returns a 400 bad request error if body not given" do - post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user) - - expect(response).to have_gitlab_http_status(400) - end - - it "returns a 401 unauthorized error if user not authenticated" do - post api("/projects/#{project.id}/issues/#{issue.iid}/notes"), body: 'hi!' - - expect(response).to have_gitlab_http_status(401) - end - - context 'when an admin or owner makes the request' do - it 'accepts the creation date to be set' do - creation_time = 2.weeks.ago - post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), - body: 'hi!', created_at: creation_time - - expect(response).to have_gitlab_http_status(201) - expect(json_response['body']).to eq('hi!') - expect(json_response['author']['username']).to eq(user.username) - expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) - end - end - - context 'when the user is posting an award emoji on an issue created by someone else' do - let(:issue2) { create(:issue, project: project) } - - it 'creates a new issue note' do - post api("/projects/#{project.id}/issues/#{issue2.iid}/notes", user), body: ':+1:' - - expect(response).to have_gitlab_http_status(201) - expect(json_response['body']).to eq(':+1:') - end - end - - context 'when the user is posting an award emoji on his/her own issue' do - it 'creates a new issue note' do - post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), body: ':+1:' - - expect(response).to have_gitlab_http_status(201) - expect(json_response['body']).to eq(':+1:') - end - end - end - - context "when noteable is a Snippet" do - it "creates a new snippet note" do - post api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user), body: 'hi!' + context "when noteable is a Snippet" do + let!(:snippet) { create(:project_snippet, project: project, author: user) } + let!(:snippet_note) { create(:note, noteable: snippet, project: project, author: user) } - expect(response).to have_gitlab_http_status(201) - expect(json_response['body']).to eq('hi!') - expect(json_response['author']['username']).to eq(user.username) - end - - it "returns a 400 bad request error if body not given" do - post api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user) - - expect(response).to have_gitlab_http_status(400) - end - - it "returns a 401 unauthorized error if user not authenticated" do - post api("/projects/#{project.id}/snippets/#{snippet.id}/notes"), body: 'hi!' - - expect(response).to have_gitlab_http_status(401) - end + it_behaves_like "noteable API", 'projects', 'snippets', 'id' do + let(:parent) { project } + let(:noteable) { snippet } + let(:note) { snippet_note } end + end - context 'when user does not have access to read the noteable' do - it 'responds with 404' do - project = create(:project, :private) { |p| p.add_guest(user) } - issue = create(:issue, :confidential, project: project) - - post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), - body: 'Foo' - - expect(response).to have_gitlab_http_status(404) - end - end - - context 'when user does not have access to create noteable' do - let(:private_issue) { create(:issue, project: create(:project, :private)) } - - ## - # We are posting to project user has access to, but we use issue id - # from a different project, see #15577 - # - before do - post api("/projects/#{private_issue.project.id}/issues/#{private_issue.iid}/notes", user), - body: 'Hi!' - end - - it 'responds with resource not found error' do - expect(response.status).to eq 404 - end + context "when noteable is a Merge Request" do + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } + let!(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) } - it 'does not create new note' do - expect(private_issue.notes.reload).to be_empty - end + it_behaves_like "noteable API", 'projects', 'merge_requests', 'iid' do + let(:parent) { project } + let(:noteable) { merge_request } + let(:note) { merge_request_note } end context 'when the merge request discussion is locked' do @@ -461,145 +182,4 @@ describe API::Notes do end end end - - describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do - it "creates an activity event when an issue note is created" do - expect(Event).to receive(:create!) - - post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), body: 'hi!' - end - end - - describe 'PUT /projects/:id/noteable/:noteable_id/notes/:note_id' do - context 'when noteable is an Issue' do - it 'returns modified note' do - put api("/projects/#{project.id}/issues/#{issue.iid}/"\ - "notes/#{issue_note.id}", user), body: 'Hello!' - - expect(response).to have_gitlab_http_status(200) - expect(json_response['body']).to eq('Hello!') - end - - it 'returns a 404 error when note id not found' do - put api("/projects/#{project.id}/issues/#{issue.iid}/notes/12345", user), - body: 'Hello!' - - expect(response).to have_gitlab_http_status(404) - end - - it 'returns a 400 bad request error if body not given' do - put api("/projects/#{project.id}/issues/#{issue.iid}/"\ - "notes/#{issue_note.id}", user) - - expect(response).to have_gitlab_http_status(400) - end - end - - context 'when noteable is a Snippet' do - it 'returns modified note' do - put api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/#{snippet_note.id}", user), body: 'Hello!' - - expect(response).to have_gitlab_http_status(200) - expect(json_response['body']).to eq('Hello!') - end - - it 'returns a 404 error when note id not found' do - put api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/12345", user), body: "Hello!" - - expect(response).to have_gitlab_http_status(404) - end - end - - context 'when noteable is a Merge Request' do - it 'returns modified note' do - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/"\ - "notes/#{merge_request_note.id}", user), body: 'Hello!' - - expect(response).to have_gitlab_http_status(200) - expect(json_response['body']).to eq('Hello!') - end - - it 'returns a 404 error when note id not found' do - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/"\ - "notes/12345", user), body: "Hello!" - - expect(response).to have_gitlab_http_status(404) - end - end - end - - describe 'DELETE /projects/:id/noteable/:noteable_id/notes/:note_id' do - context 'when noteable is an Issue' do - it 'deletes a note' do - delete api("/projects/#{project.id}/issues/#{issue.iid}/"\ - "notes/#{issue_note.id}", user) - - expect(response).to have_gitlab_http_status(204) - # Check if note is really deleted - delete api("/projects/#{project.id}/issues/#{issue.iid}/"\ - "notes/#{issue_note.id}", user) - expect(response).to have_gitlab_http_status(404) - end - - it 'returns a 404 error when note id not found' do - delete api("/projects/#{project.id}/issues/#{issue.iid}/notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - - it_behaves_like '412 response' do - let(:request) { api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", user) } - end - end - - context 'when noteable is a Snippet' do - it 'deletes a note' do - delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/#{snippet_note.id}", user) - - expect(response).to have_gitlab_http_status(204) - # Check if note is really deleted - delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/#{snippet_note.id}", user) - expect(response).to have_gitlab_http_status(404) - end - - it 'returns a 404 error when note id not found' do - delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - - it_behaves_like '412 response' do - let(:request) { api("/projects/#{project.id}/snippets/#{snippet.id}/notes/#{snippet_note.id}", user) } - end - end - - context 'when noteable is a Merge Request' do - it 'deletes a note' do - delete api("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.iid}/notes/#{merge_request_note.id}", user) - - expect(response).to have_gitlab_http_status(204) - # Check if note is really deleted - delete api("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.iid}/notes/#{merge_request_note.id}", user) - expect(response).to have_gitlab_http_status(404) - end - - it 'returns a 404 error when note id not found' do - delete api("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.iid}/notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - - it_behaves_like '412 response' do - let(:request) { api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes/#{merge_request_note.id}", user) } - end - end - end end diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index 80a271ba7fb..d2072198d83 100644 --- a/spec/serializers/merge_request_widget_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -147,9 +147,9 @@ describe MergeRequestWidgetEntity do allow(resource).to receive(:diff_head_sha) { 'sha' } end - context 'when no diff head commit' do + context 'when diff head commit is empty' do it 'returns nil' do - allow(resource).to receive(:diff_head_commit) { nil } + allow(resource).to receive(:diff_head_sha) { '' } expect(subject[:diff_head_sha]).to be_nil end @@ -157,8 +157,6 @@ describe MergeRequestWidgetEntity do context 'when diff head commit present' do it 'returns diff head commit short id' do - allow(resource).to receive(:diff_head_commit) { double } - expect(subject[:diff_head_sha]).to eq('sha') end end diff --git a/spec/support/matchers/match_ids.rb b/spec/support/matchers/match_ids.rb new file mode 100644 index 00000000000..d8424405b96 --- /dev/null +++ b/spec/support/matchers/match_ids.rb @@ -0,0 +1,24 @@ +RSpec::Matchers.define :match_ids do |*expected| + match do |actual| + actual_ids = map_ids(actual) + expected_ids = map_ids(expected) + + expect(actual_ids).to match_array(expected_ids) + end + + description do + 'matches elements by ids' + end + + def map_ids(elements) + elements = elements.flatten if elements.respond_to?(:flatten) + + if elements.respond_to?(:map) + elements.map(&:id) + elsif elements.respond_to?(:id) + [elements.id] + else + raise ArgumentError, "could not map elements to ids: #{elements}" + end + end +end diff --git a/spec/support/shared_examples/requests/api/discussions.rb b/spec/support/shared_examples/requests/api/discussions.rb new file mode 100644 index 00000000000..b6aeb30d69c --- /dev/null +++ b/spec/support/shared_examples/requests/api/discussions.rb @@ -0,0 +1,169 @@ +shared_examples 'discussions API' do |parent_type, noteable_type, id_name| + describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do + it "returns an array of discussions" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first['id']).to eq(note.discussion_id) + end + + it "returns a 404 error when noteable id not found" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/12345/discussions", user) + + expect(response).to have_gitlab_http_status(404) + end + + it "returns 404 when not authorized" do + parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", private_user) + + expect(response).to have_gitlab_http_status(404) + end + end + + describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do + it "returns a discussion by id" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{note.discussion_id}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['id']).to eq(note.discussion_id) + expect(json_response['notes'].first['body']).to eq(note.note) + end + + it "returns a 404 error if discussion not found" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/12345", user) + + expect(response).to have_gitlab_http_status(404) + end + end + + describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do + it "creates a new note" do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!' + + expect(response).to have_gitlab_http_status(201) + expect(json_response['notes'].first['body']).to eq('hi!') + expect(json_response['notes'].first['author']['username']).to eq(user.username) + end + + it "returns a 400 bad request error if body not given" do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user) + + expect(response).to have_gitlab_http_status(400) + end + + it "returns a 401 unauthorized error if user not authenticated" do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions"), body: 'hi!' + + expect(response).to have_gitlab_http_status(401) + end + + context 'when an admin or owner makes the request' do + it 'accepts the creation date to be set' do + creation_time = 2.weeks.ago + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), + body: 'hi!', created_at: creation_time + + expect(response).to have_gitlab_http_status(201) + expect(json_response['notes'].first['body']).to eq('hi!') + expect(json_response['notes'].first['author']['username']).to eq(user.username) + expect(Time.parse(json_response['notes'].first['created_at'])).to be_like_time(creation_time) + end + end + + context 'when user does not have access to read the discussion' do + before do + parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it 'responds with 404' do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", private_user), + body: 'Foo' + + expect(response).to have_gitlab_http_status(404) + end + end + end + + describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do + it 'adds a new note to the discussion' do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes", user), body: 'Hello!' + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq('Hello!') + expect(json_response['type']).to eq('DiscussionNote') + end + + it 'returns a 400 bad request error if body not given' do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes", user) + + expect(response).to have_gitlab_http_status(400) + end + + it "returns a 400 bad request error if discussion is individual note" do + note.update_attribute(:type, nil) + + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes", user), body: 'hi!' + + expect(response).to have_gitlab_http_status(400) + end + end + + describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes/:note_id" do + it 'returns modified note' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/#{note.id}", user), body: 'Hello!' + + expect(response).to have_gitlab_http_status(200) + expect(json_response['body']).to eq('Hello!') + end + + it 'returns a 404 error when note id not found' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/12345", user), + body: 'Hello!' + + expect(response).to have_gitlab_http_status(404) + end + + it 'returns a 400 bad request error if body not given' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/#{note.id}", user) + + expect(response).to have_gitlab_http_status(400) + end + end + + describe "DELETE /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes/:note_id" do + it 'deletes a note' do + delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/#{note.id}", user) + + expect(response).to have_gitlab_http_status(204) + # Check if note is really deleted + delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/#{note.id}", user) + expect(response).to have_gitlab_http_status(404) + end + + it 'returns a 404 error when note id not found' do + delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/12345", user) + + expect(response).to have_gitlab_http_status(404) + end + + it_behaves_like '412 response' do + let(:request) do + api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/#{note.id}", user) + end + end + end +end diff --git a/spec/support/shared_examples/requests/api/notes.rb b/spec/support/shared_examples/requests/api/notes.rb new file mode 100644 index 00000000000..79b2196660c --- /dev/null +++ b/spec/support/shared_examples/requests/api/notes.rb @@ -0,0 +1,206 @@ +shared_examples 'noteable API' do |parent_type, noteable_type, id_name| + describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes" do + context 'sorting' do + before do + params = { noteable: noteable, author: user } + params[:project] = parent if parent.is_a?(Project) + + create_list(:note, 3, params) + end + + it 'sorts by created_at in descending order by default' do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user) + + response_dates = json_response.map { |note| note['created_at'] } + + expect(json_response.length).to eq(4) + expect(response_dates).to eq(response_dates.sort.reverse) + end + + it 'sorts by ascending order when requested' do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?sort=asc", user) + + response_dates = json_response.map { |note| note['created_at'] } + + expect(json_response.length).to eq(4) + expect(response_dates).to eq(response_dates.sort) + end + + it 'sorts by updated_at in descending order when requested' do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?order_by=updated_at", user) + + response_dates = json_response.map { |note| note['updated_at'] } + + expect(json_response.length).to eq(4) + expect(response_dates).to eq(response_dates.sort.reverse) + end + + it 'sorts by updated_at in ascending order when requested' do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?order_by=updated_at&sort=asc", user) + + response_dates = json_response.map { |note| note['updated_at'] } + + expect(json_response.length).to eq(4) + expect(response_dates).to eq(response_dates.sort) + end + end + + it "returns an array of notes" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first['body']).to eq(note.note) + end + + it "returns a 404 error when noteable id not found" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/12345/notes", user) + + expect(response).to have_gitlab_http_status(404) + end + + it "returns 404 when not authorized" do + parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", private_user) + + expect(response).to have_gitlab_http_status(404) + end + end + + describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes/:note_id" do + it "returns a note by id" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/#{note.id}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['body']).to eq(note.note) + end + + it "returns a 404 error if note not found" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/12345", user) + + expect(response).to have_gitlab_http_status(404) + end + end + + describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes" do + it "creates a new note" do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), body: 'hi!' + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq('hi!') + expect(json_response['author']['username']).to eq(user.username) + end + + it "returns a 400 bad request error if body not given" do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user) + + expect(response).to have_gitlab_http_status(400) + end + + it "returns a 401 unauthorized error if user not authenticated" do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes"), body: 'hi!' + + expect(response).to have_gitlab_http_status(401) + end + + it "creates an activity event when a note is created" do + expect(Event).to receive(:create!) + + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), body: 'hi!' + end + + context 'when an admin or owner makes the request' do + it 'accepts the creation date to be set' do + creation_time = 2.weeks.ago + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), + body: 'hi!', created_at: creation_time + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq('hi!') + expect(json_response['author']['username']).to eq(user.username) + expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time) + end + end + + context 'when the user is posting an award emoji on a noteable created by someone else' do + it 'creates a new note' do + parent.add_developer(private_user) + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", private_user), body: ':+1:' + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq(':+1:') + end + end + + context 'when the user is posting an award emoji on his/her own noteable' do + it 'creates a new note' do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), body: ':+1:' + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq(':+1:') + end + end + + context 'when user does not have access to read the noteable' do + before do + parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it 'responds with 404' do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", private_user), + body: 'Foo' + + expect(response).to have_gitlab_http_status(404) + end + end + end + + describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes/:note_id" do + it 'returns modified note' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "notes/#{note.id}", user), body: 'Hello!' + + expect(response).to have_gitlab_http_status(200) + expect(json_response['body']).to eq('Hello!') + end + + it 'returns a 404 error when note id not found' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/12345", user), + body: 'Hello!' + + expect(response).to have_gitlab_http_status(404) + end + + it 'returns a 400 bad request error if body not given' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "notes/#{note.id}", user) + + expect(response).to have_gitlab_http_status(400) + end + end + + describe "DELETE /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes/:note_id" do + it 'deletes a note' do + delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "notes/#{note.id}", user) + + expect(response).to have_gitlab_http_status(204) + # Check if note is really deleted + delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "notes/#{note.id}", user) + expect(response).to have_gitlab_http_status(404) + end + + it 'returns a 404 error when note id not found' do + delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/12345", user) + + expect(response).to have_gitlab_http_status(404) + end + + it_behaves_like '412 response' do + let(:request) { api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/#{note.id}", user) } + end + end +end |
