From 0b802d2585b52d4dfee20430db26d67d7502948a Mon Sep 17 00:00:00 2001 From: Marius Balteanu Date: Sun, 22 Feb 2026 02:58:06 +0000 Subject: [PATCH] Reverts r24440 due to wrong issue number (#35685). git-svn-id: https://svn.redmine.org/redmine/trunk@24441 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/concerns/issue/webhookable.rb | 56 ------- app/models/concerns/news/webhookable.rb | 29 ---- app/models/concerns/wiki_page/webhookable.rb | 30 ---- app/models/issue.rb | 7 +- app/models/news.rb | 6 +- app/models/time_entry.rb | 9 +- app/models/version.rb | 5 +- app/models/webhook.rb | 2 +- app/models/webhook_payload.rb | 148 +++++++++++++++++-- app/models/wiki_page.rb | 6 +- lib/redmine/acts/webhookable.rb | 86 ----------- lib/redmine/preparation.rb | 1 - test/unit/webhook_payload_test.rb | 39 ----- 13 files changed, 157 insertions(+), 267 deletions(-) delete mode 100644 app/models/concerns/issue/webhookable.rb delete mode 100644 app/models/concerns/news/webhookable.rb delete mode 100644 app/models/concerns/wiki_page/webhookable.rb delete mode 100644 lib/redmine/acts/webhookable.rb diff --git a/app/models/concerns/issue/webhookable.rb b/app/models/concerns/issue/webhookable.rb deleted file mode 100644 index 7c59329e5..000000000 --- a/app/models/concerns/issue/webhookable.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -# Redmine - project management software -# Copyright (C) 2006- Jean-Philippe Lang -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - -module Issue::Webhookable - extend ActiveSupport::Concern - - def webhook_payload(user, action) - h = super - if action == 'updated' && current_journal.present? - journal = journals.visible(user).find_by_id(current_journal.id) - if journal.present? - h[:data][:journal] = journal_payload(journal, user) - h[:timestamp] = journal.created_on.iso8601 - end - end - h - end - - private - - def journal_payload(journal, user) - { - id: journal.id, - created_on: journal.created_on.iso8601, - notes: journal.notes, - user: { - id: journal.user.id, - name: journal.user.name, - }, - details: journal.visible_details(user).map do |d| - { - property: d.property, - prop_key: d.prop_key, - old_value: d.old_value, - value: d.value, - } - end - } - end -end diff --git a/app/models/concerns/news/webhookable.rb b/app/models/concerns/news/webhookable.rb deleted file mode 100644 index 7ef866688..000000000 --- a/app/models/concerns/news/webhookable.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -# Redmine - project management software -# Copyright (C) 2006- Jean-Philippe Lang -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - -module News::Webhookable - extend ActiveSupport::Concern - - # TODO: remove this method once news have the updated_on column - def webhook_payload_timestamp(action) - ts = action == 'created' ? created_on : Time.now - - ts.iso8601 - end -end diff --git a/app/models/concerns/wiki_page/webhookable.rb b/app/models/concerns/wiki_page/webhookable.rb deleted file mode 100644 index 19bc4df20..000000000 --- a/app/models/concerns/wiki_page/webhookable.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -# Redmine - project management software -# Copyright (C) 2006- Jean-Philippe Lang -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - -module WikiPage::Webhookable - extend ActiveSupport::Concern - - def webhook_payload_ivars - { page: self, content: content } - end - - def webhook_payload_api_template - "app/views/wiki/show.api.rsb" - end -end diff --git a/app/models/issue.rb b/app/models/issue.rb index e920aea56..1790091b8 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -21,7 +21,6 @@ class Issue < ApplicationRecord include Redmine::SafeAttributes include Redmine::Utils::DateCalculation include Redmine::I18n - before_validation :default_assign, on: :create before_validation :force_default_value_on_noneditable_custom_fields, on: :create before_validation :clear_disabled_fields @@ -60,8 +59,6 @@ class Issue < ApplicationRecord :author_key => :author_id acts_as_mentionable :attributes => ['description'] - acts_as_webhookable - include Issue::Webhookable DONE_RATIO_OPTIONS = %w(issue_field issue_status) @@ -133,6 +130,10 @@ class Issue < ApplicationRecord after_create_commit :add_auto_watcher after_commit :create_parent_issue_journal + after_create_commit ->{ Webhook.trigger('issue.created', self) } + after_update_commit ->{ Webhook.trigger('issue.updated', self) } + after_destroy_commit ->{ Webhook.trigger('issue.deleted', self) } + # Returns a SQL conditions string used to find all issues visible by the specified user def self.visible_condition(user, options={}) Project.allowed_to_condition(user, :view_issues, options) do |role, user| diff --git a/app/models/news.rb b/app/models/news.rb index 40130ce4a..8834e473e 100644 --- a/app/models/news.rb +++ b/app/models/news.rb @@ -37,12 +37,14 @@ class News < ApplicationRecord acts_as_activity_provider :scope => proc {preload(:project, :author)}, :author_key => :author_id acts_as_watchable - acts_as_webhookable - include News::Webhookable after_create :add_author_as_watcher after_create_commit :send_notification + after_create_commit ->{ Webhook.trigger('news.created', self) } + after_update_commit ->{ Webhook.trigger('news.updated', self) } + after_destroy_commit ->{ Webhook.trigger('news.deleted', self) } + scope :visible, (lambda do |*args| joins(:project). where(Project.allowed_to_condition(args.shift || User.current, :view_news, *args)) diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb index cae3c7a1b..ed46137ea 100644 --- a/app/models/time_entry.rb +++ b/app/models/time_entry.rb @@ -46,7 +46,6 @@ class TimeEntry < ApplicationRecord acts_as_activity_provider :timestamp => "#{table_name}.created_on", :author_key => :user_id, :scope => proc {joins(:project).preload(:project)} - acts_as_webhookable validates_presence_of :author_id, :user_id, :activity_id, :project_id, :hours, :spent_on validates_presence_of :issue_id, :if => lambda {Setting.timelog_required_fields.include?('issue_id')} @@ -59,6 +58,10 @@ class TimeEntry < ApplicationRecord before_validation :set_author_if_nil validate :validate_time_entry + after_create_commit ->{ Webhook.trigger('time_entry.created', self) } + after_update_commit ->{ Webhook.trigger('time_entry.updated', self) } + after_destroy_commit ->{ Webhook.trigger('time_entry.deleted', self) } + scope :visible, (lambda do |*args| joins(:project). where(TimeEntry.visible_condition(args.shift || User.current, *args)) @@ -79,10 +82,6 @@ class TimeEntry < ApplicationRecord 'issue_id', 'activity_id', 'spent_on', 'custom_field_values', 'custom_fields' - def webhook_payload_api_template - "app/views/timelog/show.api.rsb" - end - # Returns a SQL conditions string used to find all time entries visible by the specified user def self.visible_condition(user, options={}) Project.allowed_to_condition(user, :view_time_entries, options) do |role, user| diff --git a/app/models/version.rb b/app/models/version.rb index 0f50d1ebd..5d9941792 100644 --- a/app/models/version.rb +++ b/app/models/version.rb @@ -123,6 +123,10 @@ class Version < ApplicationRecord before_destroy :nullify_projects_default_version after_save :update_default_project_version + after_create_commit ->{ Webhook.trigger('version.created', self) } + after_update_commit ->{ Webhook.trigger('version.updated', self) } + after_destroy_commit ->{ Webhook.trigger('version.deleted', self) } + belongs_to :project has_many :fixed_issues, :class_name => 'Issue', :foreign_key => 'fixed_version_id', :dependent => :nullify, :extend => FixedIssuesExtension @@ -130,7 +134,6 @@ class Version < ApplicationRecord acts_as_attachable :view_permission => :view_files, :edit_permission => :manage_files, :delete_permission => :manage_files - acts_as_webhookable VERSION_STATUSES = %w(open locked closed) VERSION_SHARINGS = %w(none descendants hierarchy tree system) diff --git a/app/models/webhook.rb b/app/models/webhook.rb index 8cc8b9f9e..ea1c3aa9c 100644 --- a/app/models/webhook.rb +++ b/app/models/webhook.rb @@ -94,7 +94,7 @@ class Webhook < ApplicationRecord end def setable_events - WebhookPayload.events + WebhookPayload::EVENTS end def setable_event_names diff --git a/app/models/webhook_payload.rb b/app/models/webhook_payload.rb index 31e97678d..90199d011 100644 --- a/app/models/webhook_payload.rb +++ b/app/models/webhook_payload.rb @@ -27,26 +27,150 @@ class WebhookPayload self.user = user end - def self.register_model(model, model_events) - raise ArgumentError, "model_events must be Array" unless model_events.is_a?(Array) - - @events ||= {} - @events[model.model_name.singular.to_sym] = model_events - end - - def self.events - @events ||= {} - end + EVENTS = { + issue: %w[created updated deleted], + wiki_page: %w[created updated deleted], + time_entry: %w[created updated deleted], + news: %w[created updated deleted], + version: %w[created updated deleted], + } def to_h type, action = event.split('.') - if self.class.events[type.to_sym]&.include?(action) - object.webhook_payload(user, action) + if EVENTS[type.to_sym].include?(action) + send("#{type}_payload", action) else raise ArgumentError, "invalid event: #{event}" end end + private + + def issue_payload(action) + issue = object + if issue.current_journal.present? + journal = issue.journals.visible(user).find_by_id(issue.current_journal.id) + end + ts = case action + when 'created' + issue.created_on + when 'deleted' + Time.now + else + journal&.created_on || issue.updated_on + end + h = { + type: event, + timestamp: ts.iso8601, + data: { + issue: ApiRenderer.new("app/views/issues/show.api.rsb", user).to_h(issue: issue) + } + } + if action == 'updated' && journal.present? + h[:data][:journal] = journal_payload(journal) + end + h + end + + def journal_payload(journal) + { + id: journal.id, + created_on: journal.created_on.iso8601, + notes: journal.notes, + user: { + id: journal.user.id, + name: journal.user.name, + }, + details: journal.visible_details(user).map do |d| + { + property: d.property, + prop_key: d.prop_key, + old_value: d.old_value, + value: d.value, + } + end + } + end + + def wiki_page_payload(action) + wiki_page = object + + ts = case action + when 'created' + wiki_page.created_on + when 'deleted' + Time.now + else + wiki_page.updated_on + end + + { + type: event, + timestamp: ts.iso8601, + data: { + wiki_page: ApiRenderer.new("app/views/wiki/show.api.rsb", user).to_h(page: wiki_page, content: wiki_page.content) + } + } + end + + def time_entry_payload(action) + time_entry = object + ts = case action + when 'created' + time_entry.created_on + when 'deleted' + Time.now + else + time_entry.updated_on + end + { + type: event, + timestamp: ts.iso8601, + data: { + time_entry: ApiRenderer.new("app/views/timelog/show.api.rsb", user).to_h(time_entry: time_entry) + } + } + end + + def news_payload(action) + news = object + ts = case action + when 'created' + news.created_on + when 'deleted' + Time.now + else # rubocop:disable Lint/DuplicateBranch + # TODO: fix this by adding a update_on column for news. + Time.now + end + { + type: event, + timestamp: ts.iso8601, + data: { + news: ApiRenderer.new("app/views/news/show.api.rsb", user).to_h(news: news) + } + } + end + + def version_payload(action) + version = object + ts = case action + when 'created' + version.created_on + when 'deleted' + Time.now + else + version.updated_on + end + { + type: event, + timestamp: ts.iso8601, + data: { + version: ApiRenderer.new("app/views/versions/show.api.rsb", user).to_h(version: version) + } + } + end + # given a path to an API template (relative to RAILS_ROOT), renders it and returns the resulting hash class ApiRenderer include ApplicationHelper diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index ad20fbfaa..331208a11 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -47,8 +47,6 @@ class WikiPage < ApplicationRecord :preload => [:content, {:wiki => :project}], :permission => :view_wiki_pages, :project_key => "#{Wiki.table_name}.project_id" - acts_as_webhookable - include WikiPage::Webhookable attr_accessor :redirect_existing_links attr_writer :deleted_attachment_ids @@ -64,6 +62,10 @@ class WikiPage < ApplicationRecord before_destroy :delete_redirects after_save :handle_children_move, :delete_selected_attachments + after_create_commit ->{ Webhook.trigger('wiki_page.created', self) } + after_update_commit ->{ Webhook.trigger('wiki_page.updated', self) } + after_destroy_commit ->{ Webhook.trigger('wiki_page.deleted', self) } + # eager load information about last updates, without loading text scope :with_updated_on, lambda {preload(:content_without_text)} diff --git a/lib/redmine/acts/webhookable.rb b/lib/redmine/acts/webhookable.rb deleted file mode 100644 index 8162ffd18..000000000 --- a/lib/redmine/acts/webhookable.rb +++ /dev/null @@ -1,86 +0,0 @@ -# frozen_string_literal: true - -# Redmine - project management software -# Copyright (C) 2006- Jean-Philippe Lang -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the Free Software Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - -module Redmine - module Acts - module Webhookable - def self.included(base) - base.extend ClassMethods - end - - module ClassMethods - def acts_as_webhookable(events = %w(created updated deleted)) - events = Array(events).map(&:to_s) - WebhookPayload.register_model(self, events) - - events.each do |event| - case event - when 'created' - after_create_commit ->{ Webhook.trigger(event_name('created'), self) } - when 'updated' - after_update_commit ->{ Webhook.trigger(event_name('updated'), self) } - when 'deleted' - after_destroy_commit ->{ Webhook.trigger(event_name('deleted'), self) } - end - end - - include Redmine::Acts::Webhookable::InstanceMethods - end - end - - module InstanceMethods - def event_name(action) - "#{self.class.model_name.singular}.#{action}" - end - - def webhook_payload(user, action) - { - type: event_name(action), - timestamp: webhook_payload_timestamp(action), - data: { - self.class.model_name.singular.to_sym => - WebhookPayload::ApiRenderer.new(webhook_payload_api_template, user).to_h(**webhook_payload_ivars) - } - } - end - - def webhook_payload_ivars - { self.class.model_name.singular.to_sym => self } - end - - def webhook_payload_api_template - "app/views/#{self.class.model_name.plural}/show.api.rsb" - end - - def webhook_payload_timestamp(action) - ts = case action - when 'created' - created_on - when 'updated' - updated_on - else - Time.now - end - - ts.iso8601 - end - end - end - end -end diff --git a/lib/redmine/preparation.rb b/lib/redmine/preparation.rb index 0dd20f15b..fec55eb46 100644 --- a/lib/redmine/preparation.rb +++ b/lib/redmine/preparation.rb @@ -22,7 +22,6 @@ module Redmine def self.prepare ApplicationRecord.include Redmine::Acts::Positioned ApplicationRecord.include Redmine::Acts::Mentionable - ApplicationRecord.include Redmine::Acts::Webhookable ApplicationRecord.include Redmine::I18n Scm::Base.add "Subversion" diff --git a/test/unit/webhook_payload_test.rb b/test/unit/webhook_payload_test.rb index 027071ffa..781b707aa 100644 --- a/test/unit/webhook_payload_test.rb +++ b/test/unit/webhook_payload_test.rb @@ -28,20 +28,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase @issue = @project.issues.first end - WebhookPayload.events.each do |type, actions| - actions.each do |action| - test "#{type} #{action} payload should be correct" do - model_class = type.to_s.classify.constantize - obj = model_class.first || model_class.generate! - p = WebhookPayload.new("#{type}.#{action}", obj, @dlopper) - assert h = p.to_h - assert_equal "#{type}.#{action}", h[:type] - assert Time.iso8601(h[:timestamp]) - assert h.dig(:data, type) - end - end - end - test "issue update payload should contain journal" do @issue.init_journal(@dlopper) @issue.subject = "new subject" @@ -49,7 +35,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('issue.updated', @issue, @dlopper) assert h = p.to_h assert_equal 'issue.updated', h[:type] - assert Time.iso8601(h[:timestamp]) assert j = h.dig(:data, :journal) assert_equal 'Dave Lopper', j[:user][:name] assert i = h.dig(:data, :issue) @@ -61,7 +46,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('issue.deleted', @issue, @dlopper) assert h = p.to_h assert_equal 'issue.deleted', h[:type] - assert Time.iso8601(h[:timestamp]) assert_nil h.dig(:data, :journal) assert i = h.dig(:data, :issue) assert_equal @issue.subject, i[:subject], i.inspect @@ -76,7 +60,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('wiki_page.created', page, @dlopper) assert h = p.to_h assert_equal 'wiki_page.created', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal 'Test_Page', h.dig(:data, :wiki_page, :title) assert_equal 'Test content', h.dig(:data, :wiki_page, :text) assert_equal @dlopper.name, h.dig(:data, :wiki_page, :author, :name) @@ -95,7 +78,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('wiki_page.updated', page, @dlopper) h = p.to_h assert_equal 'wiki_page.updated', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal 'Updated content', h.dig(:data, :wiki_page, :text) end @@ -110,7 +92,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('wiki_page.deleted', page, @dlopper) h = p.to_h assert_equal 'wiki_page.deleted', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal 'Test_Page', h.dig(:data, :wiki_page, :title) end @@ -120,7 +101,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('time_entry.created', time_entry, @dlopper) assert h = p.to_h assert_equal 'time_entry.created', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal time_entry.hours, h.dig(:data, :time_entry, :hours) end @@ -133,7 +113,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('time_entry.updated', time_entry, @dlopper) h = p.to_h assert_equal 'time_entry.updated', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal 2.5, h.dig(:data, :time_entry, :hours) end @@ -144,7 +123,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('time_entry.deleted', time_entry, @dlopper) h = p.to_h assert_equal 'time_entry.deleted', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal 4.25, h.dig(:data, :time_entry, :hours) end @@ -155,7 +133,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('news.created', news, @dlopper) assert h = p.to_h assert_equal 'news.created', h[:type] - assert_equal news.created_on.iso8601, h[:timestamp] assert_equal news.title, h.dig(:data, :news, :title) end @@ -168,7 +145,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('news.updated', news, @dlopper) h = p.to_h assert_equal 'news.updated', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal 'Updated title', h.dig(:data, :news, :title) end @@ -179,7 +155,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('news.deleted', news, @dlopper) h = p.to_h assert_equal 'news.deleted', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal 'eCookbook first release !', h.dig(:data, :news, :title) end @@ -189,7 +164,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('version.created', version, @dlopper) assert h = p.to_h assert_equal 'version.created', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal version.name, h.dig(:data, :version, :name) end @@ -202,7 +176,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('version.updated', version, @dlopper) h = p.to_h assert_equal 'version.updated', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal 'Updated name', h.dig(:data, :version, :name) end @@ -213,18 +186,6 @@ class WebhookPayloadTest < ActiveSupport::TestCase p = WebhookPayload.new('version.deleted', version, @dlopper) h = p.to_h assert_equal 'version.deleted', h[:type] - assert Time.iso8601(h[:timestamp]) assert_equal '0.1', h.dig(:data, :version, :name) end - - test "should generate payload for custom event" do - # Register a custom event for News - News.acts_as_webhookable %w(created updated deleted commented) - - news = News.first - p = WebhookPayload.new('news.commented', news, @dlopper) - assert h = p.to_h - assert_equal 'news.commented', h[:type] - assert Time.iso8601(h[:timestamp]) - end end