Commit b255cbf3 authored by Rémy Coutable's avatar Rémy Coutable Committed by Yorick Peterse

Merge branch 'slack_wiki_notifications' into 'master'

add slack notifications for wiki pages

## What does this MR do?

Lets the Slack service be configured to send notifications when wiki pages are created or edited.

## Are there points in the code the reviewer needs to double check?

I'm just starting to get familiar with the Gitlab codebase and I was unsure on how to get the wiki page url to pass it to the slack message, on whether or not I needed to refactor the create/update methods for wiki pages from the controller to a service (but seemed necessary to test it better), and if I needed to add a column to the web hooks table or if the services table would have been enough. Please let me know if I should change anything and I will improve the MR, thanks for checking :)

## Why was this MR needed?

Related to #563 and fixes #4233.



See merge request !2998
parent 413ff5b3
...@@ -82,6 +82,7 @@ v 8.7.0 (unreleased) ...@@ -82,6 +82,7 @@ v 8.7.0 (unreleased)
- Fix: Allow empty recipients list for builds emails service when pushed is added (Frank Groeneveld) - Fix: Allow empty recipients list for builds emails service when pushed is added (Frank Groeneveld)
- Improved markdown forms - Improved markdown forms
- Delete tags using Rugged for performance reasons (Robert Schilling) - Delete tags using Rugged for performance reasons (Robert Schilling)
- Add Slack notifications when Wiki is edited (Sebastian Klier)
- Diffs load at the correct point when linking from from number - Diffs load at the correct point when linking from from number
- Selected diff rows highlight - Selected diff rows highlight
- Fix emoji categories in the emoji picker - Fix emoji categories in the emoji picker
......
...@@ -6,7 +6,7 @@ class Projects::ServicesController < Projects::ApplicationController ...@@ -6,7 +6,7 @@ class Projects::ServicesController < Projects::ApplicationController
:description, :issues_url, :new_issue_url, :restrict_to_branch, :channel, :description, :issues_url, :new_issue_url, :restrict_to_branch, :channel,
:colorize_messages, :channels, :colorize_messages, :channels,
:push_events, :issues_events, :merge_requests_events, :tag_push_events, :push_events, :issues_events, :merge_requests_events, :tag_push_events,
:note_events, :build_events, :note_events, :build_events, :wiki_page_events,
:notify_only_broken_builds, :add_pusher, :notify_only_broken_builds, :add_pusher,
:send_from_committer_email, :disable_diffs, :external_wiki_url, :send_from_committer_email, :disable_diffs, :external_wiki_url,
:notify, :color, :notify, :color,
......
...@@ -44,7 +44,7 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -44,7 +44,7 @@ class Projects::WikisController < Projects::ApplicationController
return render('empty') unless can?(current_user, :create_wiki, @project) return render('empty') unless can?(current_user, :create_wiki, @project)
if @page.update(content, format, message) if @page = WikiPages::UpdateService.new(@project, current_user, wiki_params).execute(@page)
redirect_to( redirect_to(
namespace_project_wiki_path(@project.namespace, @project, @page), namespace_project_wiki_path(@project.namespace, @project, @page),
notice: 'Wiki was successfully updated.' notice: 'Wiki was successfully updated.'
...@@ -55,9 +55,9 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -55,9 +55,9 @@ class Projects::WikisController < Projects::ApplicationController
end end
def create def create
@page = WikiPage.new(@project_wiki) @page = WikiPages::CreateService.new(@project, current_user, wiki_params).execute
if @page.create(wiki_params) if @page.persisted?
redirect_to( redirect_to(
namespace_project_wiki_path(@project.namespace, @project, @page), namespace_project_wiki_path(@project.namespace, @project, @page),
notice: 'Wiki was successfully updated.' notice: 'Wiki was successfully updated.'
...@@ -122,15 +122,4 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -122,15 +122,4 @@ class Projects::WikisController < Projects::ApplicationController
params[:wiki].slice(:title, :content, :format, :message) params[:wiki].slice(:title, :content, :format, :message)
end end
def content
params[:wiki][:content]
end
def format
params[:wiki][:format]
end
def message
params[:wiki][:message]
end
end end
...@@ -25,4 +25,5 @@ class ProjectHook < WebHook ...@@ -25,4 +25,5 @@ class ProjectHook < WebHook
scope :note_hooks, -> { where(note_events: true) } scope :note_hooks, -> { where(note_events: true) }
scope :merge_request_hooks, -> { where(merge_requests_events: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true) }
scope :build_hooks, -> { where(build_events: true) } scope :build_hooks, -> { where(build_events: true) }
scope :wiki_page_hooks, -> { where(wiki_page_events: true) }
end end
...@@ -60,7 +60,7 @@ class SlackService < Service ...@@ -60,7 +60,7 @@ class SlackService < Service
end end
def supported_events def supported_events
%w(push issue merge_request note tag_push build) %w(push issue merge_request note tag_push build wiki_page)
end end
def execute(data) def execute(data)
...@@ -90,6 +90,8 @@ class SlackService < Service ...@@ -90,6 +90,8 @@ class SlackService < Service
NoteMessage.new(data) NoteMessage.new(data)
when "build" when "build"
BuildMessage.new(data) if should_build_be_notified?(data) BuildMessage.new(data) if should_build_be_notified?(data)
when "wiki_page"
WikiPageMessage.new(data)
end end
opt = {} opt = {}
...@@ -133,3 +135,4 @@ require "slack_service/push_message" ...@@ -133,3 +135,4 @@ require "slack_service/push_message"
require "slack_service/merge_message" require "slack_service/merge_message"
require "slack_service/note_message" require "slack_service/note_message"
require "slack_service/build_message" require "slack_service/build_message"
require "slack_service/wiki_page_message"
class SlackService
class WikiPageMessage < BaseMessage
attr_reader :user_name
attr_reader :title
attr_reader :project_name
attr_reader :project_url
attr_reader :wiki_page_url
attr_reader :action
attr_reader :description
def initialize(params)
@user_name = params[:user][:name]
@project_name = params[:project_name]
@project_url = params[:project_url]
obj_attr = params[:object_attributes]
obj_attr = HashWithIndifferentAccess.new(obj_attr)
@title = obj_attr[:title]
@wiki_page_url = obj_attr[:url]
@description = obj_attr[:content]
@action =
case obj_attr[:action]
when "create"
"created"
when "update"
"edited"
end
end
def attachments
description_message
end
private
def message
"#{user_name} #{action} #{wiki_page_link} in #{project_link}: *#{title}*"
end
def description_message
[{ text: format(@description), color: attachment_color }]
end
def project_link
"[#{project_name}](#{project_url})"
end
def wiki_page_link
"[wiki page](#{wiki_page_url})"
end
end
end
...@@ -32,6 +32,7 @@ class Service < ActiveRecord::Base ...@@ -32,6 +32,7 @@ class Service < ActiveRecord::Base
default_value_for :tag_push_events, true default_value_for :tag_push_events, true
default_value_for :note_events, true default_value_for :note_events, true
default_value_for :build_events, true default_value_for :build_events, true
default_value_for :wiki_page_events, true
after_initialize :initialize_properties after_initialize :initialize_properties
...@@ -53,6 +54,7 @@ class Service < ActiveRecord::Base ...@@ -53,6 +54,7 @@ class Service < ActiveRecord::Base
scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) }
scope :note_hooks, -> { where(note_events: true, active: true) } scope :note_hooks, -> { where(note_events: true, active: true) }
scope :build_hooks, -> { where(build_events: true, active: true) } scope :build_hooks, -> { where(build_events: true, active: true) }
scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) }
default_value_for :category, 'common' default_value_for :category, 'common'
...@@ -94,7 +96,7 @@ class Service < ActiveRecord::Base ...@@ -94,7 +96,7 @@ class Service < ActiveRecord::Base
end end
def supported_events def supported_events
%w(push tag_push issue merge_request) %w(push tag_push issue merge_request wiki_page)
end end
def execute(data) def execute(data)
......
...@@ -29,6 +29,10 @@ class WikiPage ...@@ -29,6 +29,10 @@ class WikiPage
# new Page values before writing to the Gollum repository. # new Page values before writing to the Gollum repository.
attr_accessor :attributes attr_accessor :attributes
def hook_attrs
attributes
end
def initialize(wiki, page = nil, persisted = false) def initialize(wiki, page = nil, persisted = false)
@wiki = wiki @wiki = wiki
@page = page @page = page
......
module WikiPages
class BaseService < ::BaseService
def hook_data(page, action)
hook_data = {
object_kind: page.class.name.underscore,
user: current_user.hook_attrs,
project: @project.hook_attrs,
object_attributes: page.hook_attrs,
# DEPRECATED
repository: @project.hook_attrs.slice(:name, :url, :description, :homepage)
}
page_url = Gitlab::UrlBuilder.build(page)
hook_data[:object_attributes].merge!(url: page_url, action: action)
hook_data
end
private
def execute_hooks(page, action = 'create')
page_data = hook_data(page, action)
@project.execute_hooks(page_data, :wiki_page_hooks)
@project.execute_services(page_data, :wiki_page_hooks)
end
end
end
module WikiPages
class CreateService < WikiPages::BaseService
def execute
page = WikiPage.new(@project.wiki)
if page.create(@params)
execute_hooks(page, 'create')
end
page
end
end
end
module WikiPages
class UpdateService < WikiPages::BaseService
def execute(page)
if page.update(@params[:content], @params[:format], @params[:message])
execute_hooks(page, 'update')
end
page
end
end
end
...@@ -62,6 +62,14 @@ ...@@ -62,6 +62,14 @@
%strong Build events %strong Build events
%p.light %p.light
This url will be triggered when a build status changes This url will be triggered when a build status changes
- if @service.supported_events.include?("wiki_page")
%div
= form.check_box :wiki_page_events, class: 'pull-left'
.prepend-left-20
= form.label :wiki_page_events, class: 'list-label' do
%strong Wiki Page events
%p.light
This url will be triggered when a wiki page is created/updated
- @service.fields.each do |field| - @service.fields.each do |field|
......
class AddEventFieldForWebHook < ActiveRecord::Migration
def change
add_column :web_hooks, :wiki_page_events, :boolean, default: false, null: false
end
end
class AddEventToServices < ActiveRecord::Migration
def change
add_column :services, :wiki_page_events, :boolean, default: true
end
end
...@@ -820,6 +820,7 @@ ActiveRecord::Schema.define(version: 20160419120017) do ...@@ -820,6 +820,7 @@ ActiveRecord::Schema.define(version: 20160419120017) do
t.boolean "build_events", default: false, null: false t.boolean "build_events", default: false, null: false
t.string "category", default: "common", null: false t.string "category", default: "common", null: false
t.boolean "default", default: false t.boolean "default", default: false
t.boolean "wiki_page_events", default: true
end end
add_index "services", ["category"], name: "index_services_on_category", using: :btree add_index "services", ["category"], name: "index_services_on_category", using: :btree
...@@ -1014,6 +1015,7 @@ ActiveRecord::Schema.define(version: 20160419120017) do ...@@ -1014,6 +1015,7 @@ ActiveRecord::Schema.define(version: 20160419120017) do
t.boolean "note_events", default: false, null: false t.boolean "note_events", default: false, null: false
t.boolean "enable_ssl_verification", default: true t.boolean "enable_ssl_verification", default: true
t.boolean "build_events", default: false, null: false t.boolean "build_events", default: false, null: false
t.boolean "wiki_page_events", default: false, null: false
end end
add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree
......
...@@ -20,6 +20,8 @@ module Gitlab ...@@ -20,6 +20,8 @@ module Gitlab
merge_request_url(object) merge_request_url(object)
when Note when Note
note_url note_url
when WikiPage
wiki_page_url
else else
raise NotImplementedError.new("No URL builder defined for #{object.class}") raise NotImplementedError.new("No URL builder defined for #{object.class}")
end end
...@@ -58,5 +60,9 @@ module Gitlab ...@@ -58,5 +60,9 @@ module Gitlab
project_snippet_url(snippet, anchor: dom_id(object)) project_snippet_url(snippet, anchor: dom_id(object))
end end
end end
def wiki_page_url
"#{Gitlab.config.gitlab.url}#{object.wiki.wiki_base_path}/#{object.slug}"
end
end end
end end
FactoryGirl.define do
factory :project_wiki do
project factory: :empty_project
user factory: :user
initialize_with { new(project, user) }
end
end
require 'ostruct'
FactoryGirl.define do
factory :wiki_page do
page = OpenStruct.new(url_path: 'some-name')
association :wiki, factory: :project_wiki, strategy: :build
initialize_with { new(wiki, page, true) }
end
end
...@@ -106,5 +106,14 @@ describe Gitlab::UrlBuilder, lib: true do ...@@ -106,5 +106,14 @@ describe Gitlab::UrlBuilder, lib: true do
end end
end end
end end
context 'when passing a WikiPage' do
it 'returns a proper URL' do
wiki_page = build(:wiki_page)
url = described_class.build(wiki_page)
expect(url).to eq "#{Gitlab.config.gitlab.url}#{wiki_page.wiki.wiki_base_path}/#{wiki_page.slug}"
end
end
end end
end end
require 'spec_helper'
describe SlackService::WikiPageMessage, models: true do
subject { described_class.new(args) }
let(:args) do
{
user: {
name: 'Test User',
username: 'Test User'
},
project_name: 'project_name',
project_url: 'somewhere.com',
object_attributes: {
title: 'Wiki page title',
url: 'url',
content: 'Wiki page description'
}
}
end
describe '#pretext' do
context 'when :action == "create"' do
before { args[:object_attributes][:action] = 'create' }
it 'returns a message that a new wiki page was created' do
expect(subject.pretext).to eq(
'Test User created <url|wiki page> in <somewhere.com|project_name>: '\
'*Wiki page title*')
end
end
context 'when :action == "update"' do
before { args[:object_attributes][:action] = 'update' }
it 'returns a message that a wiki page was updated' do
expect(subject.pretext).to eq(
'Test User edited <url|wiki page> in <somewhere.com|project_name>: '\
'*Wiki page title*')
end
end
end
describe '#attachments' do
let(:color) { '#345' }
context 'when :action == "create"' do
before { args[:object_attributes][:action] = 'create' }
it 'it returns the attachment for a new wiki page' do
expect(subject.attachments).to eq([
{
text: "Wiki page description",
color: color,
}
])
end
end
context 'when :action == "update"' do
before { args[:object_attributes][:action] = 'update' }
it 'it returns the attachment for an updated wiki page' do
expect(subject.attachments).to eq([
{
text: "Wiki page description",
color: color,
}
])
end
end
end
end
...@@ -75,6 +75,17 @@ describe SlackService, models: true do ...@@ -75,6 +75,17 @@ describe SlackService, models: true do
@merge_request = merge_service.execute @merge_request = merge_service.execute
@merge_sample_data = merge_service.hook_data(@merge_request, @merge_sample_data = merge_service.hook_data(@merge_request,
'open') 'open')
opts = {
title: "Awesome wiki_page",
content: "Some text describing some thing or another",
format: "md",
message: "user created page: Awesome wiki_page"
}
wiki_page_service = WikiPages::CreateService.new(project, user, opts)
@wiki_page = wiki_page_service.execute
@wiki_page_sample_data = wiki_page_service.hook_data(@wiki_page, 'create')
end end
it "should call Slack API for push events" do it "should call Slack API for push events" do
...@@ -95,6 +106,12 @@ describe SlackService, models: true do ...@@ -95,6 +106,12 @@ describe SlackService, models: true do
expect(WebMock).to have_requested(:post, webhook_url).once expect(WebMock).to have_requested(:post, webhook_url).once
end end
it "should call Slack API for wiki page events" do
slack.execute(@wiki_page_sample_data)
expect(WebMock).to have_requested(:post, webhook_url).once
end
it 'should use the username as an option for slack when configured' do it 'should use the username as an option for slack when configured' do
allow(slack).to receive(:username).and_return(username) allow(slack).to receive(:username).and_return(username)
expect(Slack::Notifier).to receive(:new). expect(Slack::Notifier).to receive(:new).
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment