From 2d9282f7a06a2c184a2aca0254c29f0c3509b911 Mon Sep 17 00:00:00 2001 From: Felipe Batista Date: Wed, 17 Apr 2019 19:20:03 -0300 Subject: [PATCH] Simplify settings for plugin, especially around URLs. The original version of this plugin had settings designed for a world where users generally didn't need to set a Zulip server URL, but at this point they always do. So it makes sense to refactor the settings for this plugin to match how all of our other integrations work with prioritizing the server URL first and not separating port from the rest of the URL. --- .../views/settings/_redmine_zulip.html.erb | 15 +-- redmine_zulip/config/locales/en.yml | 7 +- redmine_zulip/init.rb | 3 +- redmine_zulip/lib/zulip_hooks.rb | 102 ++++++++---------- 4 files changed, 51 insertions(+), 76 deletions(-) diff --git a/redmine_zulip/app/views/settings/_redmine_zulip.html.erb b/redmine_zulip/app/views/settings/_redmine_zulip.html.erb index 184e404..eeef0fa 100644 --- a/redmine_zulip/app/views/settings/_redmine_zulip.html.erb +++ b/redmine_zulip/app/views/settings/_redmine_zulip.html.erb @@ -1,3 +1,8 @@ +

+ <%= content_tag(:label, l(:zulip_settings_label_url)) %> + <%= text_field_tag 'settings[zulip_url]', @settings["zulip_url"] %> +

+

<%= content_tag(:label, l(:zulip_settings_label_email)) %> <%= text_field_tag 'settings[zulip_email]', @settings["zulip_email"] %> @@ -13,16 +18,6 @@ <%= text_field_tag 'settings[zulip_stream]', @settings["zulip_stream"] %>

-

- <%= content_tag(:label, l(:zulip_settings_label_server)) %> - <%= text_field_tag 'settings[zulip_server]', @settings["zulip_server"] %> -

- -

- <%= content_tag(:label, l(:zulip_settings_label_port)) %> - <%= text_field_tag 'settings[zulip_port]', @settings["zulip_port"] %> -

-

<%= content_tag(:label, l(:zulip_settings_label_projects)) %> <%= select_tag 'settings[projects][]', project_tree_options_for_select(Project.active, :selected => Project.find_by_id(@settings["projects"])), :multiple => true, :size => Project.active.size %> diff --git a/redmine_zulip/config/locales/en.yml b/redmine_zulip/config/locales/en.yml index 3650c68..0c04b9f 100644 --- a/redmine_zulip/config/locales/en.yml +++ b/redmine_zulip/config/locales/en.yml @@ -1,11 +1,10 @@ en: zulip_settings_header: Zulip Plugin Configuration - zulip_settings_label_stream: Stream name - zulip_settings_label_server: Server URL - zulip_settings_label_port: Server Port + zulip_settings_label_stream: Zulip stream + zulip_settings_label_url: Zulip URL zulip_settings_label_email: Zulip Email zulip_settings_label_api_key: Zulip API key zulip_settings_label_projects: Projects field_zulip_email: Zulip Email field_zulip_api_key: Zulip API key - field_zulip_stream: Stream name + field_zulip_stream: Zulip stream diff --git a/redmine_zulip/init.rb b/redmine_zulip/init.rb index 75e62a9..68bd8f9 100644 --- a/redmine_zulip/init.rb +++ b/redmine_zulip/init.rb @@ -20,6 +20,5 @@ :zulip_email => "", :zulip_api_key => "", :zulip_stream => "", - :zulip_server => "", - :zulip_port => ""} + :zulip_url => ""} end diff --git a/redmine_zulip/lib/zulip_hooks.rb b/redmine_zulip/lib/zulip_hooks.rb index ed98b70..2769f7b 100644 --- a/redmine_zulip/lib/zulip_hooks.rb +++ b/redmine_zulip/lib/zulip_hooks.rb @@ -54,97 +54,79 @@ def controller_issues_edit_after_save(context = {}) def configured(project) # The plugin can be configured as a system setting or per-project. - if !project.zulip_email.empty? && !project.zulip_api_key.empty? && - !project.zulip_stream.empty? && Setting.plugin_redmine_zulip["projects"] && - Setting.plugin_redmine_zulip["zulip_server"] + if project.zulip_email.present? && + project.zulip_api_key.present? && + project.zulip_stream.present? && + Setting.plugin_redmine_zulip["projects"] && + Setting.plugin_redmine_zulip["zulip_url"].present? # We have full per-project settings. return true - elsif Setting.plugin_redmine_zulip["projects"] && - Setting.plugin_redmine_zulip["zulip_email"] && - Setting.plugin_redmine_zulip["zulip_api_key"] && - Setting.plugin_redmine_zulip["zulip_stream"] && - Setting.plugin_redmine_zulip["zulip_server"] + end + if Setting.plugin_redmine_zulip["projects"] && + Setting.plugin_redmine_zulip["zulip_email"].present? && + Setting.plugin_redmine_zulip["zulip_api_key"].present? && + Setting.plugin_redmine_zulip["zulip_stream"].present? && + Setting.plugin_redmine_zulip["zulip_url"].present? # We have full global settings. return true end Rails.logger.info "Missing config, can't sent to Zulip!" - return false + false end def zulip_email(project) - if !project.zulip_email.empty? + if project.zulip_email.present? return project.zulip_email end - return Setting.plugin_redmine_zulip["zulip_email"] + Setting.plugin_redmine_zulip["zulip_email"] end def zulip_api_key(project) - if !project.zulip_api_key.empty? + if project.zulip_api_key.present? return project.zulip_api_key end - return Setting.plugin_redmine_zulip["zulip_api_key"] + Setting.plugin_redmine_zulip["zulip_api_key"] end def zulip_stream(project) - if !project.zulip_stream.empty? + if project.zulip_stream.present? return project.zulip_stream end - return Setting.plugin_redmine_zulip["zulip_stream"] + Setting.plugin_redmine_zulip["zulip_stream"] end - def zulip_server() - server = Setting.plugin_redmine_zulip["zulip_server"].sub( - %r{^https?:(//|\\\\)}i, "" - ) - # Remove /api suffix - server.slice!("/api") - server - end - - def zulip_port() - if Setting.plugin_redmine_zulip["zulip_port"].present? - return Setting.plugin_redmine_zulip["zulip_port"] - end - return 443 + def zulip_url() + Setting.plugin_redmine_zulip["zulip_url"] end - def zulip_api_basename() - basename = Setting.plugin_redmine_zulip["zulip_server"] - unless basename.end_with?("/api") - basename << "/api" - end - if zulip_port == 443 && !basename.start_with?("https://") - "https://#{basename}" - else - basename - end + def url(issue) + "#{Setting[:protocol]}://#{Setting[:host_name]}/issues/#{issue.id}" end - def url(issue) - return "#{Setting[:protocol]}://#{Setting[:host_name]}/issues/#{issue.id}" - end - - def send_zulip_message(content, project) + def send_zulip_message(content, project) + data = {"to" => zulip_stream(project), + "type" => "stream", + "subject" => project.name, + "content" => content} - data = {"to" => zulip_stream(project), - "type" => "stream", - "subject" => project.name, - "content" => content} + Rails.logger.info "Forwarding to Zulip: #{data['content']}" - Rails.logger.info "Forwarding to Zulip: #{data['content']}" + uri = URI("#{zulip_url}/v1/messages") - http = Net::HTTP.new(zulip_server(), zulip_port()) - http.use_ssl = true + req = Net::HTTP::Post.new(uri) + req.basic_auth(zulip_email(project), zulip_api_key(project)) + req["User-Agent"] = "ZulipRedmine/#{RedmineZulip::VERSION}" + req.set_form_data(data) - req = Net::HTTP::Post.new("#{zulip_api_basename()}/v1/messages") - req.basic_auth(zulip_email(project), zulip_api_key(project)) - req.add_field('User-Agent', "ZulipRedmine/#{RedmineZulip::VERSION}") - req.set_form_data(data) + res = Net::HTTP.start(uri.hostname, uri.port, use_ssl: true) do |http| + http.request(req) + end - res = http.request(req) - unless res.code == "200" - Rails.logger.error "Error while POSTing to Zulip: #{res.body}" - end - end + if res.code == "200" + Rails.logger.info "Zulip message sent!" + else + Rails.logger.error "Error while POSTing to Zulip: #{res.body}" + end + end end