diff options
45 files changed, 1249 insertions, 397 deletions
diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 994707422bb..ee3ca246374 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -54,12 +54,15 @@ .mr-widget-pipeline-graph { display: inline-block; vertical-align: middle; - margin-right: 4px; .stage-cell .stage-container { margin: 3px 3px 3px 0; } + .stage-container:last-child { + margin-right: 0; + } + .dropdown-menu { margin-top: 11px; } diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 086dd528579..8fc7a5eec9b 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -209,9 +209,11 @@ } .stage-cell { - @media (min-width: $screen-md-min) { - min-width: 148px; - margin-right: -4px; + &.table-section { + @media (min-width: $screen-md-min) { + min-width: 148px; + margin-right: -4px; + } } .mini-pipeline-graph-dropdown-toggle svg { diff --git a/app/controllers/dashboard/groups_controller.rb b/app/controllers/dashboard/groups_controller.rb index 8057a0b455c..7ed18fb481c 100644 --- a/app/controllers/dashboard/groups_controller.rb +++ b/app/controllers/dashboard/groups_controller.rb @@ -1,6 +1,6 @@ class Dashboard::GroupsController < Dashboard::ApplicationController def index - @sort = params[:sort] || 'id_desc' + @sort = params[:sort] || 'created_desc' @groups = if params[:parent_id] && Group.supports_nested_groups? diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index fc30d008dea..27f4dedffd3 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -256,23 +256,22 @@ module Issuable participants(user).include?(user) end - def to_hook_data(user) - hook_data = { - object_kind: self.class.name.underscore, - user: user.hook_attrs, - project: project.hook_attrs, - object_attributes: hook_attrs, - labels: labels.map(&:hook_attrs), - # DEPRECATED - repository: project.hook_attrs.slice(:name, :url, :description, :homepage) - } - if self.is_a?(Issue) - hook_data[:assignees] = assignees.map(&:hook_attrs) if assignees.any? - else - hook_data[:assignee] = assignee.hook_attrs if assignee + def to_hook_data(user, old_labels: [], old_assignees: []) + changes = previous_changes + + if old_labels != labels + changes[:labels] = [old_labels.map(&:hook_attrs), labels.map(&:hook_attrs)] + end + + if old_assignees != assignees + if self.is_a?(Issue) + changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)] + else + changes[:assignee] = [old_assignees&.first&.hook_attrs, assignee&.hook_attrs] + end end - hook_data + Gitlab::HookData::IssuableBuilder.new(self).build(user: user, changes: changes) end def labels_array diff --git a/app/models/issue.rb b/app/models/issue.rb index 155c5d972b7..36e4108b9d6 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -74,20 +74,6 @@ class Issue < ActiveRecord::Base end end - def hook_attrs - assignee_ids = self.assignee_ids - - attrs = { - total_time_spent: total_time_spent, - human_total_time_spent: human_total_time_spent, - human_time_estimate: human_time_estimate, - assignee_ids: assignee_ids, - assignee_id: assignee_ids.first # This key is deprecated - } - - attributes.merge!(attrs) - end - def self.reference_prefix '#' end @@ -131,6 +117,10 @@ class Issue < ActiveRecord::Base "id DESC") end + def hook_attrs + Gitlab::HookData::IssueBuilder.new(self).build + end + # Returns a Hash of attributes to be used for Twitter card metadata def card_attributes { diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 292122f779e..75e9bdaaa45 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -179,6 +179,10 @@ class MergeRequest < ActiveRecord::Base work_in_progress?(title) ? title : "WIP: #{title}" end + def hook_attrs + Gitlab::HookData::MergeRequestBuilder.new(self).build + end + # Returns a Hash of attributes to be used for Twitter card metadata def card_attributes { @@ -587,24 +591,6 @@ class MergeRequest < ActiveRecord::Base !discussions_to_be_resolved? end - def hook_attrs - attrs = { - source: source_project.try(:hook_attrs), - target: target_project.hook_attrs, - last_commit: nil, - work_in_progress: work_in_progress?, - total_time_spent: total_time_spent, - human_total_time_spent: human_total_time_spent, - human_time_estimate: human_time_estimate - } - - if diff_head_commit - attrs[:last_commit] = diff_head_commit.hook_attrs - end - - attributes.merge!(attrs) - end - def for_fork? target_project != source_project end diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb index 298569cb7a6..6e311806be1 100644 --- a/app/models/sent_notification.rb +++ b/app/models/sent_notification.rb @@ -53,13 +53,17 @@ class SentNotification < ActiveRecord::Base end def unsubscribable? - !for_commit? + !(for_commit? || for_snippet?) end def for_commit? noteable_type == "Commit" end + def for_snippet? + noteable_type.end_with?('Snippet') + end + def noteable if for_commit? project.commit(commit_id) rescue nil diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index f83ece7098f..d61a342ebad 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -255,7 +255,7 @@ class IssuableBaseService < BaseService invalidate_cache_counts(issuable, users: affected_assignees.compact) after_update(issuable) issuable.create_new_cross_references!(current_user) - execute_hooks(issuable, 'update') + execute_hooks(issuable, 'update', old_labels: old_labels, old_assignees: old_assignees) issuable.update_project_counter_caches if update_project_counters end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 4c198fc96ea..735257c4779 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -1,10 +1,10 @@ module Issues class BaseService < ::IssuableBaseService - def hook_data(issue, action) - issue_data = issue.to_hook_data(current_user) - issue_url = Gitlab::UrlBuilder.build(issue) - issue_data[:object_attributes].merge!(url: issue_url, action: action) - issue_data + def hook_data(issue, action, old_labels: [], old_assignees: []) + hook_data = issue.to_hook_data(current_user, old_labels: old_labels, old_assignees: old_assignees) + hook_data[:object_attributes][:action] = action + + hook_data end def reopen_service @@ -22,8 +22,8 @@ module Issues issue, issue.project, current_user, old_assignees) end - def execute_hooks(issue, action = 'open') - issue_data = hook_data(issue, action) + def execute_hooks(issue, action = 'open', old_labels: [], old_assignees: []) + issue_data = hook_data(issue, action, old_labels: old_labels, old_assignees: old_assignees) hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks issue.project.execute_hooks(issue_data, hooks_scope) issue.project.execute_services(issue_data, hooks_scope) diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index b4ca3966505..e0339ddf9bb 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -34,7 +34,7 @@ module Issues if issue.assignees != old_assignees create_assignee_note(issue, old_assignees) notification_service.reassigned_issue(issue, current_user, old_assignees) - todo_service.reassigned_issue(issue, current_user) + todo_service.reassigned_issue(issue, current_user, old_assignees) end if issue.previous_changes.include?('confidential') diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 35ccff26262..112606a82d7 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -18,19 +18,19 @@ module MergeRequests super if changed_title end - def hook_data(merge_request, action, oldrev = nil) - hook_data = merge_request.to_hook_data(current_user) - hook_data[:object_attributes][:url] = Gitlab::UrlBuilder.build(merge_request) + def hook_data(merge_request, action, old_rev: nil, old_labels: [], old_assignees: []) + hook_data = merge_request.to_hook_data(current_user, old_labels: old_labels, old_assignees: old_assignees) hook_data[:object_attributes][:action] = action - if oldrev && !Gitlab::Git.blank_ref?(oldrev) - hook_data[:object_attributes][:oldrev] = oldrev + if old_rev && !Gitlab::Git.blank_ref?(old_rev) + hook_data[:object_attributes][:oldrev] = old_rev end + hook_data end - def execute_hooks(merge_request, action = 'open', oldrev = nil) + def execute_hooks(merge_request, action = 'open', old_rev: nil, old_labels: [], old_assignees: []) if merge_request.project - merge_data = hook_data(merge_request, action, oldrev) + merge_data = hook_data(merge_request, action, old_rev: old_rev, old_labels: old_labels, old_assignees: old_assignees) merge_request.project.execute_hooks(merge_data, :merge_request_hooks) merge_request.project.execute_services(merge_data, :merge_request_hooks) end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index bc4a13cf4bc..fc100580c4f 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -166,7 +166,7 @@ module MergeRequests # Call merge request webhook with update branches def execute_mr_web_hooks merge_requests_for_source_branch.each do |merge_request| - execute_hooks(merge_request, 'update', @oldrev) + execute_hooks(merge_request, 'update', old_rev: @oldrev) end end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 6ee96d6a0f8..b6125cafa83 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -43,8 +43,8 @@ class TodoService # # * create a pending todo for new assignee if issue is assigned # - def reassigned_issue(issue, current_user) - create_assignment_todo(issue, current_user) + def reassigned_issue(issue, current_user, old_assignees = []) + create_assignment_todo(issue, current_user, old_assignees) end # When create a merge request we should: @@ -254,10 +254,11 @@ class TodoService create_mention_todos(project, target, author, note, skip_users) end - def create_assignment_todo(issuable, author) + def create_assignment_todo(issuable, author, old_assignees = []) if issuable.assignees.any? + assignees = issuable.assignees - old_assignees attributes = attributes_for_todo(issuable.project, issuable, author, Todo::ASSIGNED) - create_todos(issuable.assignees, attributes) + create_todos(assignees, attributes) end end diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 53ebdd6d2ff..9a763887b30 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -19,8 +19,7 @@ - create_mr = event.new_ref? && create_mr_button?(project.default_branch, event.ref_name, project) && event.authored_by?(current_user) - if event.commits_count > 1 %li.commits-stat - - if event.commits_count > 2 - %span ... and #{event.commits_count - 2} more commits. + %span ... and #{pluralize(event.commits_count - 1, 'more commit')}. - if event.md_ref? - from = event.commit_from diff --git a/changelogs/unreleased/23888-fix-unsubscription-link-for-snippet-notification.yml b/changelogs/unreleased/23888-fix-unsubscription-link-for-snippet-notification.yml new file mode 100644 index 00000000000..36bed037160 --- /dev/null +++ b/changelogs/unreleased/23888-fix-unsubscription-link-for-snippet-notification.yml @@ -0,0 +1,5 @@ +--- +title: Don't show an "Unsubscribe" link in snippet comment notifications +merge_request: 14764 +author: +type: fixed diff --git a/changelogs/unreleased/34284-add-changes-to-issuable-webhook-data.yml b/changelogs/unreleased/34284-add-changes-to-issuable-webhook-data.yml new file mode 100644 index 00000000000..816e1f83111 --- /dev/null +++ b/changelogs/unreleased/34284-add-changes-to-issuable-webhook-data.yml @@ -0,0 +1,5 @@ +--- +title: Include the changes in issuable webhook payloads +merge_request: 14308 +author: +type: added diff --git a/changelogs/unreleased/37483-activity-log-show-wrong-number-of-commits-per-push.yml b/changelogs/unreleased/37483-activity-log-show-wrong-number-of-commits-per-push.yml new file mode 100644 index 00000000000..225ab9acc44 --- /dev/null +++ b/changelogs/unreleased/37483-activity-log-show-wrong-number-of-commits-per-push.yml @@ -0,0 +1,5 @@ +--- +title: Fix the number representing the amount of commits related to a push event +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/38534-minigraph.yml b/changelogs/unreleased/38534-minigraph.yml new file mode 100644 index 00000000000..eed240eac2d --- /dev/null +++ b/changelogs/unreleased/38534-minigraph.yml @@ -0,0 +1,5 @@ +--- +title: Fixes mini pipeline graph in commit view +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/group-sort-dropdown-blank.yml b/changelogs/unreleased/group-sort-dropdown-blank.yml new file mode 100644 index 00000000000..dd16892be4d --- /dev/null +++ b/changelogs/unreleased/group-sort-dropdown-blank.yml @@ -0,0 +1,5 @@ +--- +title: Fixed group sort dropdown defaulting to empty +merge_request: +author: +type: fixed diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 47eb0b34f66..7abc600a680 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -205,7 +205,7 @@ X-Gitlab-Event: Issue Hook "username": "root", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon" }, - "project":{ + "project": { "name":"Gitlab Test", "description":"Aut reprehenderit ut est.", "web_url":"http://example.com/gitlabhq/gitlab-test", @@ -221,7 +221,7 @@ X-Gitlab-Event: Issue Hook "ssh_url":"git@example.com:gitlabhq/gitlab-test.git", "http_url":"http://example.com/gitlabhq/gitlab-test.git" }, - "repository":{ + "repository": { "name": "Gitlab Test", "url": "http://example.com/gitlabhq/gitlab-test.git", "description": "Aut reprehenderit ut est.", @@ -266,7 +266,37 @@ X-Gitlab-Event: Issue Hook "description": "API related issues", "type": "ProjectLabel", "group_id": 41 - }] + }], + "changes": { + "updated_by_id": [null, 1], + "updated_at": ["2017-09-15 16:50:55 UTC", "2017-09-15 16:52:00 UTC"], + "labels": { + "previous": [{ + "id": 206, + "title": "API", + "color": "#ffffff", + "project_id": 14, + "created_at": "2013-12-03T17:15:43Z", + "updated_at": "2013-12-03T17:15:43Z", + "template": false, + "description": "API related issues", + "type": "ProjectLabel", + "group_id": 41 + }], + "current": [{ + "id": 205, + "title": "Platform", + "color": "#123123", + "project_id": 14, + "created_at": "2013-12-03T17:15:43Z", + "updated_at": "2013-12-03T17:15:43Z", + "template": false, + "description": "Platform related issues", + "type": "ProjectLabel", + "group_id": 41 + }] + } + } } ``` @@ -661,6 +691,28 @@ X-Gitlab-Event: Merge Request Hook "username": "root", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon" }, + "project": { + "name":"Gitlab Test", + "description":"Aut reprehenderit ut est.", + "web_url":"http://example.com/gitlabhq/gitlab-test", + "avatar_url":null, + "git_ssh_url":"git@example.com:gitlabhq/gitlab-test.git", + "git_http_url":"http://example.com/gitlabhq/gitlab-test.git", + "namespace":"GitlabHQ", + "visibility_level":20, + "path_with_namespace":"gitlabhq/gitlab-test", + "default_branch":"master", + "homepage":"http://example.com/gitlabhq/gitlab-test", + "url":"http://example.com/gitlabhq/gitlab-test.git", + "ssh_url":"git@example.com:gitlabhq/gitlab-test.git", + "http_url":"http://example.com/gitlabhq/gitlab-test.git" + }, + "repository": { + "name": "Gitlab Test", + "url": "http://example.com/gitlabhq/gitlab-test.git", + "description": "Aut reprehenderit ut est.", + "homepage": "http://example.com/gitlabhq/gitlab-test" + }, "object_attributes": { "id": 99, "target_branch": "master", @@ -679,7 +731,7 @@ X-Gitlab-Event: Merge Request Hook "target_project_id": 14, "iid": 1, "description": "", - "source":{ + "source": { "name":"Awesome Project", "description":"Aut reprehenderit ut est.", "web_url":"http://example.com/awesome_space/awesome_project", @@ -729,6 +781,48 @@ X-Gitlab-Event: Merge Request Hook "username": "user1", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon" } + }, + "labels": [{ + "id": 206, + "title": "API", + "color": "#ffffff", + "project_id": 14, + "created_at": "2013-12-03T17:15:43Z", + "updated_at": "2013-12-03T17:15:43Z", + "template": false, + "description": "API related issues", + "type": "ProjectLabel", + "group_id": 41 + }], + "changes": { + "updated_by_id": [null, 1], + "updated_at": ["2017-09-15 16:50:55 UTC", "2017-09-15 16:52:00 UTC"], + "labels": { + "previous": [{ + "id": 206, + "title": "API", + "color": "#ffffff", + "project_id": 14, + "created_at": "2013-12-03T17:15:43Z", + "updated_at": "2013-12-03T17:15:43Z", + "template": false, + "description": "API related issues", + "type": "ProjectLabel", + "group_id": 41 + }], + "current": [{ + "id": 205, + "title": "Platform", + "color": "#123123", + "project_id": 14, + "created_at": "2013-12-03T17:15:43Z", + "updated_at": "2013-12-03T17:15:43Z", + "template": false, + "description": "Platform related issues", + "type": "ProjectLabel", + "group_id": 41 + }] + } } } ``` @@ -1015,7 +1109,7 @@ X-Gitlab-Event: Build Hook ## Testing webhooks -You can trigger the webhook manually. Sample data from the project will be used.Sample data will take from the project. +You can trigger the webhook manually. Sample data from the project will be used.Sample data will take from the project. > For example: for triggering `Push Events` your project should have at least one commit.  diff --git a/lib/gitlab/hook_data/issuable_builder.rb b/lib/gitlab/hook_data/issuable_builder.rb new file mode 100644 index 00000000000..4febb0ab430 --- /dev/null +++ b/lib/gitlab/hook_data/issuable_builder.rb @@ -0,0 +1,56 @@ +module Gitlab + module HookData + class IssuableBuilder + CHANGES_KEYS = %i[previous current].freeze + + attr_accessor :issuable + + def initialize(issuable) + @issuable = issuable + end + + def build(user: nil, changes: {}) + hook_data = { + object_kind: issuable.class.name.underscore, + user: user.hook_attrs, + project: issuable.project.hook_attrs, + object_attributes: issuable.hook_attrs, + labels: issuable.labels.map(&:hook_attrs), + changes: final_changes(changes.slice(*safe_keys)), + # DEPRECATED + repository: issuable.project.hook_attrs.slice(:name, :url, :description, :homepage) + } + + if issuable.is_a?(Issue) + hook_data[:assignees] = issuable.assignees.map(&:hook_attrs) if issuable.assignees.any? + else + hook_data[:assignee] = issuable.assignee.hook_attrs if issuable.assignee + end + + hook_data + end + + def safe_keys + issuable_builder::SAFE_HOOK_ATTRIBUTES + issuable_builder::SAFE_HOOK_RELATIONS + end + + private + + def issuable_builder + case issuable + when Issue + Gitlab::HookData::IssueBuilder + when MergeRequest + Gitlab::HookData::MergeRequestBuilder + end + end + + def final_changes(changes_hash) + changes_hash.reduce({}) do |hash, (key, changes_array)| + hash[key] = Hash[CHANGES_KEYS.zip(changes_array)] + hash + end + end + end + end +end diff --git a/lib/gitlab/hook_data/issue_builder.rb b/lib/gitlab/hook_data/issue_builder.rb new file mode 100644 index 00000000000..de9cab80a02 --- /dev/null +++ b/lib/gitlab/hook_data/issue_builder.rb @@ -0,0 +1,55 @@ +module Gitlab + module HookData + class IssueBuilder + SAFE_HOOK_ATTRIBUTES = %i[ + assignee_id + author_id + branch_name + closed_at + confidential + created_at + deleted_at + description + due_date + id + iid + last_edited_at + last_edited_by_id + milestone_id + moved_to_id + project_id + relative_position + state + time_estimate + title + updated_at + updated_by_id + ].freeze + + SAFE_HOOK_RELATIONS = %i[ + assignees + labels + ].freeze + + attr_accessor :issue + + def initialize(issue) + @issue = issue + end + + def build + attrs = { + url: Gitlab::UrlBuilder.build(issue), + total_time_spent: issue.total_time_spent, + human_total_time_spent: issue.human_total_time_spent, + human_time_estimate: issue.human_time_estimate, + assignee_ids: issue.assignee_ids, + assignee_id: issue.assignee_ids.first # This key is deprecated + } + + issue.attributes.with_indifferent_access.slice(*SAFE_HOOK_ATTRIBUTES) + .merge!(attrs) + end + end + end +end diff --git a/lib/gitlab/hook_data/merge_request_builder.rb b/lib/gitlab/hook_data/merge_request_builder.rb new file mode 100644 index 00000000000..eaef19c9d04 --- /dev/null +++ b/lib/gitlab/hook_data/merge_request_builder.rb @@ -0,0 +1,62 @@ +module Gitlab + module HookData + class MergeRequestBuilder + SAFE_HOOK_ATTRIBUTES = %i[ + assignee_id + author_id + created_at + deleted_at + description + head_pipeline_id + id + iid + last_edited_at + last_edited_by_id + merge_commit_sha + merge_error + merge_params + merge_status + merge_user_id + merge_when_pipeline_succeeds + milestone_id + ref_fetched + source_branch + source_project_id + state + target_branch + target_project_id + time_estimate + title + updated_at + updated_by_id + ].freeze + + SAFE_HOOK_RELATIONS = %i[ + assignee + labels + ].freeze + + attr_accessor :merge_request + + def initialize(merge_request) + @merge_request = merge_request + end + + def build + attrs = { + url: Gitlab::UrlBuilder.build(merge_request), + source: merge_request.source_project.try(:hook_attrs), + target: merge_request.target_project.hook_attrs, + last_commit: merge_request.diff_head_commit&.hook_attrs, + work_in_progress: merge_request.work_in_progress?, + total_time_spent: merge_request.total_time_spent, + human_total_time_spent: merge_request.human_total_time_spent, + human_time_estimate: merge_request.human_time_estimate + } + + merge_request.attributes.with_indifferent_access.slice(*SAFE_HOOK_ATTRIBUTES) + .merge!(attrs) + end + end + end +end @@ -31,9 +31,17 @@ module QA # GitLab instance scenarios. # module Gitlab + module Group + autoload :Create, 'qa/scenario/gitlab/group/create' + end + module Project autoload :Create, 'qa/scenario/gitlab/project/create' end + + module Sandbox + autoload :Prepare, 'qa/scenario/gitlab/sandbox/prepare' + end end end @@ -55,6 +63,7 @@ module QA end module Group + autoload :New, 'qa/page/group/new' autoload :Show, 'qa/page/group/show' end diff --git a/qa/qa/page/dashboard/groups.rb b/qa/qa/page/dashboard/groups.rb index 3690f40dcfe..083d2e1ab16 100644 --- a/qa/qa/page/dashboard/groups.rb +++ b/qa/qa/page/dashboard/groups.rb @@ -2,19 +2,22 @@ module QA module Page module Dashboard class Groups < Page::Base - def prepare_test_namespace - if page.has_content?(Runtime::Namespace.name) - return click_link(Runtime::Namespace.name) - end + def filter_by_name(name) + fill_in 'Filter by name...', with: name + end - click_on 'New group' + def has_group?(name) + filter_by_name(name) + + page.has_link?(name) + end - fill_in 'group_path', with: Runtime::Namespace.name - fill_in 'group_description', - with: "QA test run at #{Runtime::Namespace.time}" - choose 'Private' + def go_to_group(name) + click_link name + end - click_button 'Create group' + def go_to_new_group + click_on 'New group' end end end diff --git a/qa/qa/page/group/new.rb b/qa/qa/page/group/new.rb new file mode 100644 index 00000000000..cb743a7bf11 --- /dev/null +++ b/qa/qa/page/group/new.rb @@ -0,0 +1,23 @@ +module QA + module Page + module Group + class New < Page::Base + def set_path(path) + fill_in 'group_path', with: path + end + + def set_description(description) + fill_in 'group_description', with: description + end + + def set_visibility(visibility) + choose visibility + end + + def create + click_button 'Create group' + end + end + end + end +end diff --git a/qa/qa/page/group/show.rb b/qa/qa/page/group/show.rb index 296c311d7c6..6987c1f8f85 100644 --- a/qa/qa/page/group/show.rb +++ b/qa/qa/page/group/show.rb @@ -2,8 +2,24 @@ module QA module Page module Group class Show < Page::Base + def go_to_subgroups + click_link 'Subgroups' + end + + def go_to_subgroup(name) + click_link name + end + + def has_subgroup?(name) + page.has_link?(name) + end + + def go_to_new_subgroup + click_on 'New Subgroup' + end + def go_to_new_project - click_link 'New Project' + click_on 'New Project' end end end diff --git a/qa/qa/runtime/namespace.rb b/qa/qa/runtime/namespace.rb index e4910b63a14..996286430b9 100644 --- a/qa/qa/runtime/namespace.rb +++ b/qa/qa/runtime/namespace.rb @@ -10,6 +10,10 @@ module QA def name 'qa_test_' + time.strftime('%d_%m_%Y_%H-%M-%S') end + + def sandbox_name + 'gitlab-qa-sandbox' + end end end end diff --git a/qa/qa/scenario/gitlab/group/create.rb b/qa/qa/scenario/gitlab/group/create.rb new file mode 100644 index 00000000000..8e6c7c7ad80 --- /dev/null +++ b/qa/qa/scenario/gitlab/group/create.rb @@ -0,0 +1,27 @@ +require 'securerandom' + +module QA + module Scenario + module Gitlab + module Group + class Create < Scenario::Template + attr_writer :path, :description + + def initialize + @path = Runtime::Namespace.name + @description = "QA test run at #{Runtime::Namespace.time}" + end + + def perform + Page::Group::New.perform do |group| + group.set_path(@path) + group.set_description(@description) + group.set_visibility('Private') + group.create + end + end + end + end + end + end +end diff --git a/qa/qa/scenario/gitlab/project/create.rb b/qa/qa/scenario/gitlab/project/create.rb index b860701c304..7b614bfdd94 100644 --- a/qa/qa/scenario/gitlab/project/create.rb +++ b/qa/qa/scenario/gitlab/project/create.rb @@ -12,9 +12,23 @@ module QA end def perform - Page::Main::Menu.act { go_to_groups } - Page::Dashboard::Groups.act { prepare_test_namespace } - Page::Group::Show.act { go_to_new_project } + Scenario::Gitlab::Sandbox::Prepare.perform + + Page::Group::Show.perform do |page| + page.go_to_subgroups + + if page.has_subgroup?(Runtime::Namespace.name) + page.go_to_subgroup(Runtime::Namespace.name) + else + page.go_to_new_subgroup + + Scenario::Gitlab::Group::Create.perform do |group| + group.path = Runtime::Namespace.name + end + end + + page.go_to_new_project + end Page::Project::New.perform do |page| page.choose_test_namespace diff --git a/qa/qa/scenario/gitlab/sandbox/prepare.rb b/qa/qa/scenario/gitlab/sandbox/prepare.rb new file mode 100644 index 00000000000..990de456e20 --- /dev/null +++ b/qa/qa/scenario/gitlab/sandbox/prepare.rb @@ -0,0 +1,28 @@ +module QA + module Scenario + module Gitlab + module Sandbox + # Ensure we're in our sandbox namespace, either by navigating to it or + # by creating it if it doesn't yet exist + class Prepare < Scenario::Template + def perform + Page::Main::Menu.act { go_to_groups } + + Page::Dashboard::Groups.perform do |page| + if page.has_group?(Runtime::Namespace.sandbox_name) + page.go_to_group(Runtime::Namespace.sandbox_name) + else + page.go_to_new_group + + Scenario::Gitlab::Group::Create.perform do |group| + group.path = Runtime::Namespace.sandbox_name + group.description = 'QA sandbox' + end + end + end + end + end + end + end + end +end diff --git a/spec/features/dashboard/group_spec.rb b/spec/features/dashboard/group_spec.rb index 970173c7aaf..1213f8c32eb 100644 --- a/spec/features/dashboard/group_spec.rb +++ b/spec/features/dashboard/group_spec.rb @@ -5,6 +5,12 @@ RSpec.describe 'Dashboard Group' do sign_in(create(:user)) end + it 'defaults sort dropdown to last created' do + visit dashboard_groups_path + + expect(page).to have_button('Last created') + end + it 'creates new group', :js do visit dashboard_groups_path find('.btn-new').trigger('click') diff --git a/spec/lib/gitlab/hook_data/issuable_builder_spec.rb b/spec/lib/gitlab/hook_data/issuable_builder_spec.rb new file mode 100644 index 00000000000..30da56bec16 --- /dev/null +++ b/spec/lib/gitlab/hook_data/issuable_builder_spec.rb @@ -0,0 +1,105 @@ +require 'spec_helper' + +describe Gitlab::HookData::IssuableBuilder do + set(:user) { create(:user) } + + # This shared example requires a `builder` and `user` variable + shared_examples 'issuable hook data' do |kind| + let(:data) { builder.build(user: user) } + + include_examples 'project hook data' do + let(:project) { builder.issuable.project } + end + include_examples 'deprecated repository hook data' + + context "with a #{kind}" do + it 'contains issuable data' do + expect(data[:object_kind]).to eq(kind) + expect(data[:user]).to eq(user.hook_attrs) + expect(data[:project]).to eq(builder.issuable.project.hook_attrs) + expect(data[:object_attributes]).to eq(builder.issuable.hook_attrs) + expect(data[:changes]).to eq({}) + expect(data[:repository]).to eq(builder.issuable.project.hook_attrs.slice(:name, :url, :description, :homepage)) + end + + it 'does not contain certain keys' do + expect(data).not_to have_key(:assignees) + expect(data).not_to have_key(:assignee) + end + + describe 'changes are given' do + let(:changes) do + { + cached_markdown_version: %w[foo bar], + description: ['A description', 'A cool description'], + description_html: %w[foo bar], + in_progress_merge_commit_sha: %w[foo bar], + lock_version: %w[foo bar], + merge_jid: %w[foo bar], + title: ['A title', 'Hello World'], + title_html: %w[foo bar], + labels: [ + [{ id: 1, title: 'foo' }], + [{ id: 1, title: 'foo' }, { id: 2, title: 'bar' }] + ] + } + end + let(:data) { builder.build(user: user, changes: changes) } + + it 'populates the :changes hash' do + expect(data[:changes]).to match(hash_including({ + title: { previous: 'A title', current: 'Hello World' }, + description: { previous: 'A description', current: 'A cool description' }, + labels: { + previous: [{ id: 1, title: 'foo' }], + current: [{ id: 1, title: 'foo' }, { id: 2, title: 'bar' }] + } + })) + end + + it 'does not contain certain keys' do + expect(data[:changes]).not_to have_key('cached_markdown_version') + expect(data[:changes]).not_to have_key('description_html') + expect(data[:changes]).not_to have_key('lock_version') + expect(data[:changes]).not_to have_key('title_html') + expect(data[:changes]).not_to have_key('in_progress_merge_commit_sha') + expect(data[:changes]).not_to have_key('merge_jid') + end + end + end + end + + describe '#build' do + it_behaves_like 'issuable hook data', 'issue' do + let(:issuable) { create(:issue, description: 'A description') } + let(:builder) { described_class.new(issuable) } + end + + it_behaves_like 'issuable hook data', 'merge_request' do + let(:issuable) { create(:merge_request, description: 'A description') } + let(:builder) { described_class.new(issuable) } + end + + context 'issue is assigned' do + let(:issue) { create(:issue, assignees: [user]) } + let(:data) { described_class.new(issue).build(user: user) } + + it 'returns correct hook data' do + expect(data[:object_attributes]['assignee_id']).to eq(user.id) + expect(data[:assignees].first).to eq(user.hook_attrs) + expect(data).not_to have_key(:assignee) + end + end + + context 'merge_request is assigned' do + let(:merge_request) { create(:merge_request, assignee: user) } + let(:data) { described_class.new(merge_request).build(user: user) } + + it 'returns correct hook data' do + expect(data[:object_attributes]['assignee_id']).to eq(user.id) + expect(data[:assignee]).to eq(user.hook_attrs) + expect(data).not_to have_key(:assignees) + end + end + end +end diff --git a/spec/lib/gitlab/hook_data/issue_builder_spec.rb b/spec/lib/gitlab/hook_data/issue_builder_spec.rb new file mode 100644 index 00000000000..6c529cdd051 --- /dev/null +++ b/spec/lib/gitlab/hook_data/issue_builder_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe Gitlab::HookData::IssueBuilder do + set(:issue) { create(:issue) } + let(:builder) { described_class.new(issue) } + + describe '#build' do + let(:data) { builder.build } + + it 'includes safe attribute' do + %w[ + assignee_id + author_id + branch_name + closed_at + confidential + created_at + deleted_at + description + due_date + id + iid + last_edited_at + last_edited_by_id + milestone_id + moved_to_id + project_id + relative_position + state + time_estimate + title + updated_at + updated_by_id + ].each do |key| + expect(data).to include(key) + end + end + + it 'includes additional attrs' do + expect(data).to include(:total_time_spent) + expect(data).to include(:human_time_estimate) + expect(data).to include(:human_total_time_spent) + expect(data).to include(:assignee_ids) + end + end +end diff --git a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb new file mode 100644 index 00000000000..92bf87bbad4 --- /dev/null +++ b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe Gitlab::HookData::MergeRequestBuilder do + set(:merge_request) { create(:merge_request) } + let(:builder) { described_class.new(merge_request) } + + describe '#build' do + let(:data) { builder.build } + + it 'includes safe attribute' do + %w[ + assignee_id + author_id + created_at + deleted_at + description + head_pipeline_id + id + iid + last_edited_at + last_edited_by_id + merge_commit_sha + merge_error + merge_params + merge_status + merge_user_id + merge_when_pipeline_succeeds + milestone_id + ref_fetched + source_branch + source_project_id + state + target_branch + target_project_id + time_estimate + title + updated_at + updated_by_id + ].each do |key| + expect(data).to include(key) + end + end + + %i[source target].each do |key| + describe "#{key} key" do + include_examples 'project hook data', project_key: key do + let(:project) { merge_request.public_send("#{key}_project") } + end + end + end + + it 'includes additional attrs' do + expect(data).to include(:source) + expect(data).to include(:target) + expect(data).to include(:last_commit) + expect(data).to include(:work_in_progress) + expect(data).to include(:total_time_spent) + expect(data).to include(:human_time_estimate) + expect(data).to include(:human_total_time_spent) + end + end +end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 932e2fd8c95..c832cee965b 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -28,319 +28,334 @@ describe Notify do end def have_referable_subject(referable, reply: false) - prefix = referable.project.name if referable.project - prefix = "Re: #{prefix}" if reply + prefix = referable.project ? "#{referable.project.name} | " : '' + prefix.prepend('Re: ') if reply suffix = "#{referable.title} (#{referable.to_reference})" - have_subject [prefix, suffix].compact.join(' | ') + have_subject [prefix, suffix].compact.join end context 'for a project' do - describe 'items that are assignable, the email' do - let(:previous_assignee) { create(:user, name: 'Previous Assignee') } + shared_examples 'an assignee email' do + it 'is sent to the assignee as the author' do + sender = subject.header[:from].addrs.first + + aggregate_failures do + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + expect(subject).to deliver_to(assignee.email) + end + end + end - shared_examples 'an assignee email' do - it 'is sent to the assignee as the author' do - sender = subject.header[:from].addrs.first + context 'for issues' do + describe 'that are new' do + subject { described_class.new_issue_email(issue.assignees.first.id, issue.id) } + it_behaves_like 'an assignee email' + it_behaves_like 'an email starting a new thread with reply-by-email enabled' do + let(:model) { issue } + end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' + + it 'has the correct subject and body' do aggregate_failures do - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - expect(subject).to deliver_to(assignee.email) + is_expected.to have_referable_subject(issue) + is_expected.to have_body_text(project_issue_path(project, issue)) end end - end - context 'for issues' do - describe 'that are new' do - subject { described_class.new_issue_email(issue.assignees.first.id, issue.id) } + it 'contains the description' do + is_expected.to have_html_escaped_body_text issue.description + end - it_behaves_like 'an assignee email' - it_behaves_like 'an email starting a new thread with reply-by-email enabled' do - let(:model) { issue } - end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'an unsubscribeable thread' - - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(issue) - is_expected.to have_body_text(project_issue_path(project, issue)) - end + context 'when enabled email_author_in_body' do + before do + stub_application_setting(email_author_in_body: true) end - it 'contains the description' do - is_expected.to have_html_escaped_body_text issue.description + it 'contains a link to note author' do + is_expected.to have_html_escaped_body_text(issue.author_name) + is_expected.to have_body_text 'created an issue:' end + end + end - context 'when enabled email_author_in_body' do - before do - stub_application_setting(email_author_in_body: true) - end + describe 'that are reassigned' do + let(:previous_assignee) { create(:user, name: 'Previous Assignee') } + subject { described_class.reassigned_issue_email(recipient.id, issue.id, [previous_assignee.id], current_user.id) } - it 'contains a link to note author' do - is_expected.to have_html_escaped_body_text(issue.author_name) - is_expected.to have_body_text 'created an issue:' - end - end + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' - describe 'that have been reassigned' do - subject { described_class.reassigned_issue_email(recipient.id, issue.id, [previous_assignee.id], current_user.id) } + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { issue } + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_html_escaped_body_text(previous_assignee.name) + is_expected.to have_html_escaped_body_text(assignee.name) + is_expected.to have_body_text(project_issue_path(project, issue)) end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'an unsubscribeable thread' + end + end - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + describe 'that have been relabeled' do + subject { described_class.relabeled_issue_email(recipient.id, issue.id, %w[foo bar baz], current_user.id) } - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_html_escaped_body_text(previous_assignee.name) - is_expected.to have_html_escaped_body_text(assignee.name) - is_expected.to have_body_text(project_issue_path(project, issue)) - end - end + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'an email with a labels subscriptions link in its footer' - describe 'that have been relabeled' do - subject { described_class.relabeled_issue_email(recipient.id, issue.id, %w[foo bar baz], current_user.id) } + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { issue } + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text('foo, bar, and baz') + is_expected.to have_body_text(project_issue_path(project, issue)) end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'a user cannot unsubscribe through footer link' - it_behaves_like 'an email with a labels subscriptions link in its footer' + end - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) + context 'with a preferred language' do + before do + Gitlab::I18n.locale = :es end - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_body_text('foo, bar, and baz') - is_expected.to have_body_text(project_issue_path(project, issue)) - end + after do + Gitlab::I18n.use_default_locale end - context 'with a preferred language' do - before do - Gitlab::I18n.locale = :es - end - - after do - Gitlab::I18n.use_default_locale - end - - it 'always generates the email using the default language' do - is_expected.to have_body_text('foo, bar, and baz') - end + it 'always generates the email using the default language' do + is_expected.to have_body_text('foo, bar, and baz') end end + end - describe 'status changed' do - let(:status) { 'closed' } - subject { described_class.issue_status_changed_email(recipient.id, issue.id, status, current_user.id) } + describe 'status changed' do + let(:status) { 'closed' } + subject { described_class.issue_status_changed_email(recipient.id, issue.id, status, current_user.id) } - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { issue } - end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { issue } + end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_body_text(status) - is_expected.to have_html_escaped_body_text(current_user.name) - is_expected.to have_body_text(project_issue_path project, issue) - end + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text(status) + is_expected.to have_html_escaped_body_text(current_user.name) + is_expected.to have_body_text(project_issue_path project, issue) end end + end - describe 'moved to another project' do - let(:new_issue) { create(:issue) } - subject { described_class.issue_moved_email(recipient, issue, new_issue, current_user) } + describe 'moved to another project' do + let(:new_issue) { create(:issue) } + subject { described_class.issue_moved_email(recipient, issue, new_issue, current_user) } - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { issue } - end - it_behaves_like 'it should show Gmail Actions View Issue link' - it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { issue } + end + it_behaves_like 'it should show Gmail Actions View Issue link' + it_behaves_like 'an unsubscribeable thread' - it 'contains description about action taken' do - is_expected.to have_body_text 'Issue was moved to another project' - end + it 'contains description about action taken' do + is_expected.to have_body_text 'Issue was moved to another project' + end - it 'has the correct subject and body' do - new_issue_url = project_issue_path(new_issue.project, new_issue) + it 'has the correct subject and body' do + new_issue_url = project_issue_path(new_issue.project, new_issue) - aggregate_failures do - is_expected.to have_referable_subject(issue, reply: true) - is_expected.to have_body_text(new_issue_url) - is_expected.to have_body_text(project_issue_path(project, issue)) - end + aggregate_failures do + is_expected.to have_referable_subject(issue, reply: true) + is_expected.to have_body_text(new_issue_url) + is_expected.to have_body_text(project_issue_path(project, issue)) end end end + end - context 'for merge requests' do - describe 'that are new' do - subject { described_class.new_merge_request_email(merge_request.assignee_id, merge_request.id) } + context 'for merge requests' do + describe 'that are new' do + subject { described_class.new_merge_request_email(merge_request.assignee_id, merge_request.id) } + + it_behaves_like 'an assignee email' + it_behaves_like 'an email starting a new thread with reply-by-email enabled' do + let(:model) { merge_request } + end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' - it_behaves_like 'an assignee email' - it_behaves_like 'an email starting a new thread with reply-by-email enabled' do - let(:model) { merge_request } + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request) + is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + is_expected.to have_body_text(merge_request.source_branch) + is_expected.to have_body_text(merge_request.target_branch) end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like 'an unsubscribeable thread' - - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(merge_request) - is_expected.to have_body_text(project_merge_request_path(project, merge_request)) - is_expected.to have_body_text(merge_request.source_branch) - is_expected.to have_body_text(merge_request.target_branch) - end + end + + it 'contains the description' do + is_expected.to have_html_escaped_body_text merge_request.description + end + + context 'when enabled email_author_in_body' do + before do + stub_application_setting(email_author_in_body: true) end - it 'contains the description' do - is_expected.to have_html_escaped_body_text merge_request.description + it 'contains a link to note author' do + is_expected.to have_html_escaped_body_text merge_request.author_name + is_expected.to have_body_text 'created a merge request:' end + end + end - context 'when enabled email_author_in_body' do - before do - stub_application_setting(email_author_in_body: true) - end + describe 'that are reassigned' do + let(:previous_assignee) { create(:user, name: 'Previous Assignee') } + subject { described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) } - it 'contains a link to note author' do - is_expected.to have_html_escaped_body_text merge_request.author_name - is_expected.to have_body_text 'created a merge request:' - end - end + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like "an unsubscribeable thread" - describe 'that are reassigned' do - subject { described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) } + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { merge_request } + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_html_escaped_body_text(previous_assignee.name) + is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + is_expected.to have_html_escaped_body_text(assignee.name) end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like "an unsubscribeable thread" + end + end - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + describe 'that have been relabeled' do + subject { described_class.relabeled_merge_request_email(recipient.id, merge_request.id, %w[foo bar baz], current_user.id) } - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_html_escaped_body_text(previous_assignee.name) - is_expected.to have_body_text(project_merge_request_path(project, merge_request)) - is_expected.to have_html_escaped_body_text(assignee.name) - end - end + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'a user cannot unsubscribe through footer link' + it_behaves_like 'an email with a labels subscriptions link in its footer' - describe 'that have been relabeled' do - subject { described_class.relabeled_merge_request_email(recipient.id, merge_request.id, %w[foo bar baz], current_user.id) } + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { merge_request } - end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like 'a user cannot unsubscribe through footer link' - it_behaves_like 'an email with a labels subscriptions link in its footer' + it 'has the correct subject and body' do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_body_text('foo, bar, and baz') + is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + end + end - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + describe 'status changed' do + let(:status) { 'reopened' } + subject { described_class.merge_request_status_email(recipient.id, merge_request.id, status, current_user.id) } - it 'has the correct subject and body' do + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } + end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' + + it 'is sent as the author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(current_user.name) + expect(sender.address).to eq(gitlab_sender) + end + + it 'has the correct subject and body' do + aggregate_failures do is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_body_text('foo, bar, and baz') + is_expected.to have_body_text(status) + is_expected.to have_html_escaped_body_text(current_user.name) is_expected.to have_body_text(project_merge_request_path(project, merge_request)) end end + end - describe 'status changed' do - let(:status) { 'reopened' } - subject { described_class.merge_request_status_email(recipient.id, merge_request.id, status, current_user.id) } + describe 'that are merged' do + let(:merge_author) { create(:user) } + subject { described_class.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) } - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { merge_request } - end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like 'an unsubscribeable thread' + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } + end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' - it 'is sent as the author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(current_user.name) - expect(sender.address).to eq(gitlab_sender) - end + it 'is sent as the merge author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(merge_author.name) + expect(sender.address).to eq(gitlab_sender) + end - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_body_text(status) - is_expected.to have_html_escaped_body_text(current_user.name) - is_expected.to have_body_text(project_merge_request_path(project, merge_request)) - end + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_body_text('merged') + is_expected.to have_body_text(project_merge_request_path(project, merge_request)) end end + end + end - describe 'that are merged' do - let(:merge_author) { create(:user) } - subject { described_class.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) } + context 'for snippet notes' do + let(:project_snippet) { create(:project_snippet, project: project) } + let(:project_snippet_note) { create(:note_on_project_snippet, project: project, noteable: project_snippet) } - it_behaves_like 'a multiple recipients email' - it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - let(:model) { merge_request } - end - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like 'an unsubscribeable thread' + subject { described_class.note_snippet_email(project_snippet_note.author_id, project_snippet_note.id) } - it 'is sent as the merge author' do - sender = subject.header[:from].addrs[0] - expect(sender.display_name).to eq(merge_author.name) - expect(sender.address).to eq(gitlab_sender) - end + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { project_snippet } + end + it_behaves_like 'a user cannot unsubscribe through footer link' - it 'has the correct subject and body' do - aggregate_failures do - is_expected.to have_referable_subject(merge_request, reply: true) - is_expected.to have_body_text('merged') - is_expected.to have_body_text(project_merge_request_path(project, merge_request)) - end - end - end + it 'has the correct subject and body' do + is_expected.to have_referable_subject(project_snippet, reply: true) + is_expected.to have_html_escaped_body_text project_snippet_note.note end end @@ -1239,4 +1254,18 @@ describe Notify do end end end + + context 'for personal snippet notes' do + let(:personal_snippet) { create(:personal_snippet) } + let(:personal_snippet_note) { create(:note_on_personal_snippet, noteable: personal_snippet) } + + subject { described_class.note_personal_snippet_email(personal_snippet_note.author_id, personal_snippet_note.id) } + + it_behaves_like 'a user cannot unsubscribe through footer link' + + it 'has the correct subject and body' do + is_expected.to have_referable_subject(personal_snippet, reply: true) + is_expected.to have_html_escaped_body_text personal_snippet_note.note + end + end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index fb5fb7daaab..ba57301a3c9 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Issuable do let(:issuable_class) { Issue } - let(:issue) { create(:issue) } + let(:issue) { create(:issue, title: 'An issue', description: 'A description') } let(:user) { create(:user) } describe "Associations" do @@ -264,55 +264,75 @@ describe Issuable do end end - describe "#to_hook_data" do - let(:data) { issue.to_hook_data(user) } - let(:project) { issue.project } - - it "returns correct hook data" do - expect(data[:object_kind]).to eq("issue") - expect(data[:user]).to eq(user.hook_attrs) - expect(data[:object_attributes]).to eq(issue.hook_attrs) - expect(data).not_to have_key(:assignee) - end + describe '#to_hook_data' do + context 'labels are updated' do + let(:labels) { create_list(:label, 2) } - context "issue is assigned" do before do - issue.assignees << user + issue.update(labels: [labels[1]]) end - it "returns correct hook data" do - expect(data[:assignees].first).to eq(user.hook_attrs) + it 'delegates to Gitlab::HookData::IssuableBuilder#build' do + builder = double + + expect(Gitlab::HookData::IssuableBuilder) + .to receive(:new).with(issue).and_return(builder) + expect(builder).to receive(:build).with( + user: user, + changes: hash_including( + 'labels' => [[labels[0].hook_attrs], [labels[1].hook_attrs]] + )) + + issue.to_hook_data(user, old_labels: [labels[0]]) end end - context "merge_request is assigned" do - let(:merge_request) { create(:merge_request) } - let(:data) { merge_request.to_hook_data(user) } + context 'issue is assigned' do + let(:user2) { create(:user) } before do - merge_request.update_attribute(:assignee, user) + issue.assignees << user << user2 end - it "returns correct hook data" do - expect(data[:object_attributes]['assignee_id']).to eq(user.id) - expect(data[:assignee]).to eq(user.hook_attrs) + it 'delegates to Gitlab::HookData::IssuableBuilder#build' do + builder = double + + expect(Gitlab::HookData::IssuableBuilder) + .to receive(:new).with(issue).and_return(builder) + expect(builder).to receive(:build).with( + user: user, + changes: hash_including( + 'assignees' => [[user.hook_attrs], [user.hook_attrs, user2.hook_attrs]] + )) + + issue.to_hook_data(user, old_assignees: [user]) end end - context 'issue has labels' do - let(:labels) { [create(:label), create(:label)] } + context 'merge_request is assigned' do + let(:merge_request) { create(:merge_request) } + let(:user2) { create(:user) } before do - issue.update_attribute(:labels, labels) + merge_request.update(assignee: user) + merge_request.update(assignee: user2) end - it 'includes labels in the hook data' do - expect(data[:labels]).to eq(labels.map(&:hook_attrs)) + it 'delegates to Gitlab::HookData::IssuableBuilder#build' do + builder = double + + expect(Gitlab::HookData::IssuableBuilder) + .to receive(:new).with(merge_request).and_return(builder) + expect(builder).to receive(:build).with( + user: user, + changes: hash_including( + 'assignee_id' => [user.id, user2.id], + 'assignee' => [user.hook_attrs, user2.hook_attrs] + )) + + merge_request.to_hook_data(user, old_assignees: [user]) end end - - include_examples 'project hook data' - include_examples 'deprecated repository hook data' end describe '#labels_array' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index e547da0cfbe..bb5033c1628 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -700,18 +700,14 @@ describe Issue do end describe '#hook_attrs' do - let(:attrs_hash) { subject.hook_attrs } + it 'delegates to Gitlab::HookData::IssueBuilder#build' do + builder = double - it 'includes time tracking attrs' do - expect(attrs_hash).to include(:total_time_spent) - expect(attrs_hash).to include(:human_time_estimate) - expect(attrs_hash).to include(:human_total_time_spent) - expect(attrs_hash).to include('time_estimate') - end + expect(Gitlab::HookData::IssueBuilder) + .to receive(:new).with(subject).and_return(builder) + expect(builder).to receive(:build) - it 'includes assignee_ids and deprecated assignee_id' do - expect(attrs_hash).to include(:assignee_id) - expect(attrs_hash).to include(:assignee_ids) + subject.hook_attrs end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 950af653c80..17c9f15b021 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -660,27 +660,15 @@ describe MergeRequest do end end - describe "#hook_attrs" do - let(:attrs_hash) { subject.hook_attrs } - - [:source, :target].each do |key| - describe "#{key} key" do - include_examples 'project hook data', project_key: key do - let(:data) { attrs_hash } - let(:project) { subject.send("#{key}_project") } - end - end - end + describe '#hook_attrs' do + it 'delegates to Gitlab::HookData::MergeRequestBuilder#build' do + builder = double + + expect(Gitlab::HookData::MergeRequestBuilder) + .to receive(:new).with(subject).and_return(builder) + expect(builder).to receive(:build) - it "has all the required keys" do - expect(attrs_hash).to include(:source) - expect(attrs_hash).to include(:target) - expect(attrs_hash).to include(:last_commit) - expect(attrs_hash).to include(:work_in_progress) - expect(attrs_hash).to include(:total_time_spent) - expect(attrs_hash).to include(:human_time_estimate) - expect(attrs_hash).to include(:human_total_time_spent) - expect(attrs_hash).to include('time_estimate') + subject.hook_attrs end end diff --git a/spec/models/sent_notification_spec.rb b/spec/models/sent_notification_spec.rb index 8f05deb8b15..5ec04b99957 100644 --- a/spec/models/sent_notification_spec.rb +++ b/spec/models/sent_notification_spec.rb @@ -1,6 +1,9 @@ require 'spec_helper' describe SentNotification do + set(:user) { create(:user) } + set(:project) { create(:project) } + describe 'validation' do describe 'note validity' do context "when the project doesn't match the noteable's project" do @@ -34,7 +37,6 @@ describe SentNotification do end describe '.record' do - let(:user) { create(:user) } let(:issue) { create(:issue) } it 'creates a new SentNotification' do @@ -43,7 +45,6 @@ describe SentNotification do end describe '.record_note' do - let(:user) { create(:user) } let(:note) { create(:diff_note_on_merge_request) } it 'creates a new SentNotification' do @@ -51,6 +52,123 @@ describe SentNotification do end end + describe '#unsubscribable?' do + shared_examples 'an unsubscribable notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for #{noteable_type}" do + it { expect(subject).to be_unsubscribable } + end + end + + shared_examples 'a non-unsubscribable notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for a #{noteable_type}" do + it { expect(subject).not_to be_unsubscribable } + end + end + + it_behaves_like 'an unsubscribable notification', 'issue' do + let(:noteable) { create(:issue, project: project) } + end + + it_behaves_like 'an unsubscribable notification', 'merge request' do + let(:noteable) { create(:merge_request, source_project: project) } + end + + it_behaves_like 'a non-unsubscribable notification', 'commit' do + let(:project) { create(:project, :repository) } + let(:noteable) { project.commit } + end + + it_behaves_like 'a non-unsubscribable notification', 'personal snippet' do + let(:noteable) { create(:personal_snippet, project: project) } + end + + it_behaves_like 'a non-unsubscribable notification', 'project snippet' do + let(:noteable) { create(:project_snippet, project: project) } + end + end + + describe '#for_commit?' do + shared_examples 'a commit notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for #{noteable_type}" do + it { expect(subject).to be_for_commit } + end + end + + shared_examples 'a non-commit notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for a #{noteable_type}" do + it { expect(subject).not_to be_for_commit } + end + end + + it_behaves_like 'a non-commit notification', 'issue' do + let(:noteable) { create(:issue, project: project) } + end + + it_behaves_like 'a non-commit notification', 'merge request' do + let(:noteable) { create(:merge_request, source_project: project) } + end + + it_behaves_like 'a commit notification', 'commit' do + let(:project) { create(:project, :repository) } + let(:noteable) { project.commit } + end + + it_behaves_like 'a non-commit notification', 'personal snippet' do + let(:noteable) { create(:personal_snippet, project: project) } + end + + it_behaves_like 'a non-commit notification', 'project snippet' do + let(:noteable) { create(:project_snippet, project: project) } + end + end + + describe '#for_snippet?' do + shared_examples 'a snippet notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for #{noteable_type}" do + it { expect(subject).to be_for_snippet } + end + end + + shared_examples 'a non-snippet notification' do |noteable_type| + subject { described_class.record(noteable, user.id) } + + context "for a #{noteable_type}" do + it { expect(subject).not_to be_for_snippet } + end + end + + it_behaves_like 'a non-snippet notification', 'issue' do + let(:noteable) { create(:issue, project: project) } + end + + it_behaves_like 'a non-snippet notification', 'merge request' do + let(:noteable) { create(:merge_request, source_project: project) } + end + + it_behaves_like 'a non-snippet notification', 'commit' do + let(:project) { create(:project, :repository) } + let(:noteable) { project.commit } + end + + it_behaves_like 'a snippet notification', 'personal snippet' do + let(:noteable) { create(:personal_snippet, project: project) } + end + + it_behaves_like 'a snippet notification', 'project snippet' do + let(:noteable) { create(:project_snippet, project: project) } + end + end + describe '#create_reply' do context 'for issue' do let(:issue) { create(:issue) } diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index bdbe0a353fb..f07b81e842a 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -267,6 +267,30 @@ describe Issues::UpdateService, :mailer do end end + context 'when a new assignee added' do + subject { update_issue(assignees: issue.assignees + [user2]) } + + it 'creates only 1 new todo' do + expect { subject }.to change { Todo.count }.by(1) + end + + it 'creates a todo for new assignee' do + subject + + attributes = { + project: project, + author: user, + user: user2, + target_id: issue.id, + target_type: issue.class.name, + action: Todo::ASSIGNED, + state: :pending + } + + expect(Todo.where(attributes).count).to eq(1) + end + end + context 'when the milestone change' do it 'marks todos as done' do update_issue(milestone: create(:milestone)) diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 62dbe362ec8..a2c05761f6b 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -61,7 +61,7 @@ describe MergeRequests::RefreshService do it 'executes hooks with update action' do expect(refresh_service).to have_received(:execute_hooks) - .with(@merge_request, 'update', @oldrev) + .with(@merge_request, 'update', old_rev: @oldrev) expect(@merge_request.notes).not_to be_empty expect(@merge_request).to be_open @@ -87,7 +87,7 @@ describe MergeRequests::RefreshService do it 'executes hooks with update action' do expect(refresh_service).to have_received(:execute_hooks) - .with(@merge_request, 'update', @oldrev) + .with(@merge_request, 'update', old_rev: @oldrev) expect(@merge_request.notes).not_to be_empty expect(@merge_request).to be_open @@ -182,7 +182,7 @@ describe MergeRequests::RefreshService do it 'executes hooks with update action' do expect(refresh_service).to have_received(:execute_hooks) - .with(@fork_merge_request, 'update', @oldrev) + .with(@fork_merge_request, 'update', old_rev: @oldrev) expect(@merge_request.notes).to be_empty expect(@merge_request).to be_open @@ -264,7 +264,7 @@ describe MergeRequests::RefreshService do it 'refreshes the merge request' do expect(refresh_service).to receive(:execute_hooks) - .with(@fork_merge_request, 'update', Gitlab::Git::BLANK_SHA) + .with(@fork_merge_request, 'update', old_rev: Gitlab::Git::BLANK_SHA) allow_any_instance_of(Repository).to receive(:merge_base).and_return(@oldrev) refresh_service.execute(Gitlab::Git::BLANK_SHA, @newrev, 'refs/heads/master') diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index b11a1b31f32..7257c359a7e 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -78,8 +78,9 @@ describe MergeRequests::UpdateService, :mailer do end it 'executes hooks with update action' do - expect(service).to have_received(:execute_hooks) - .with(@merge_request, 'update') + expect(service) + .to have_received(:execute_hooks) + .with(@merge_request, 'update', old_labels: [], old_assignees: [user3]) end it 'sends email to user2 about assign of new merge request and email to user3 about merge request unassignment' do diff --git a/spec/support/shared_examples/models/issuable_hook_data_shared_examples.rb b/spec/support/shared_examples/models/issuable_hook_data_shared_examples.rb new file mode 100644 index 00000000000..a4762b68858 --- /dev/null +++ b/spec/support/shared_examples/models/issuable_hook_data_shared_examples.rb @@ -0,0 +1,57 @@ +# This shared example requires a `builder` and `user` variable +shared_examples 'issuable hook data' do |kind| + let(:data) { builder.build(user: user) } + + include_examples 'project hook data' do + let(:project) { builder.issuable.project } + end + include_examples 'deprecated repository hook data' + + context "with a #{kind}" do + it 'contains issuable data' do + expect(data[:object_kind]).to eq(kind) + expect(data[:user]).to eq(user.hook_attrs) + expect(data[:project]).to eq(builder.issuable.project.hook_attrs) + expect(data[:object_attributes]).to eq(builder.issuable.hook_attrs) + expect(data[:changes]).to eq({}) + expect(data[:repository]).to eq(builder.issuable.project.hook_attrs.slice(:name, :url, :description, :homepage)) + end + + it 'does not contain certain keys' do + expect(data).not_to have_key(:assignees) + expect(data).not_to have_key(:assignee) + end + + describe 'changes are given' do + let(:changes) do + { + cached_markdown_version: %w[foo bar], + description: ['A description', 'A cool description'], + description_html: %w[foo bar], + in_progress_merge_commit_sha: %w[foo bar], + lock_version: %w[foo bar], + merge_jid: %w[foo bar], + title: ['A title', 'Hello World'], + title_html: %w[foo bar] + } + end + let(:data) { builder.build(user: user, changes: changes) } + + it 'populates the :changes hash' do + expect(data[:changes]).to match(hash_including({ + title: { previous: 'A title', current: 'Hello World' }, + description: { previous: 'A description', current: 'A cool description' } + })) + end + + it 'does not contain certain keys' do + expect(data[:changes]).not_to have_key('cached_markdown_version') + expect(data[:changes]).not_to have_key('description_html') + expect(data[:changes]).not_to have_key('lock_version') + expect(data[:changes]).not_to have_key('title_html') + expect(data[:changes]).not_to have_key('in_progress_merge_commit_sha') + expect(data[:changes]).not_to have_key('merge_jid') + end + end + end +end diff --git a/spec/support/project_hook_data_shared_example.rb b/spec/support/shared_examples/models/project_hook_data_shared_examples.rb index 1eb405d4be8..f0264878811 100644 --- a/spec/support/project_hook_data_shared_example.rb +++ b/spec/support/shared_examples/models/project_hook_data_shared_examples.rb @@ -1,4 +1,4 @@ -RSpec.shared_examples 'project hook data with deprecateds' do |project_key: :project| +shared_examples 'project hook data with deprecateds' do |project_key: :project| it 'contains project data' do expect(data[project_key][:name]).to eq(project.name) expect(data[project_key][:description]).to eq(project.description) @@ -17,7 +17,7 @@ RSpec.shared_examples 'project hook data with deprecateds' do |project_key: :pro end end -RSpec.shared_examples 'project hook data' do |project_key: :project| +shared_examples 'project hook data' do |project_key: :project| it 'contains project data' do expect(data[project_key][:name]).to eq(project.name) expect(data[project_key][:description]).to eq(project.description) @@ -32,7 +32,7 @@ RSpec.shared_examples 'project hook data' do |project_key: :project| end end -RSpec.shared_examples 'deprecated repository hook data' do |project_key: :project| +shared_examples 'deprecated repository hook data' do it 'contains deprecated repository data' do expect(data[:repository][:name]).to eq(project.name) expect(data[:repository][:description]).to eq(project.description) |