diff --git a/ReleaseNotes-2.9 b/ReleaseNotes-2.9 index 387b8b85bd7..59d81b8c8bc 100644 --- a/ReleaseNotes-2.9 +++ b/ReleaseNotes-2.9 @@ -32,6 +32,7 @@ UI: kerberos authentication. Read more in the OBS Admin Guide. * Users can see the job history list of a specific package via the package binaries page * New GPG key details dialog + * RSS Feeds for User's Notifications is now available API: * Kerberos authentication mode diff --git a/src/api/app/controllers/person_controller.rb b/src/api/app/controllers/person_controller.rb index efe6618ebc4..28fa2b29661 100644 --- a/src/api/app/controllers/person_controller.rb +++ b/src/api/app/controllers/person_controller.rb @@ -266,7 +266,7 @@ def change_password(login, password) # GET /person//token def tokenlist user = User.get_by_login(params[:login]) - @list = user.tokens + @list = user.service_tokens end # POST /person//token @@ -280,7 +280,7 @@ def command_token if params[:project] || params[:package] pkg = Package.get_by_project_and_name( params[:project], params[:package] ) end - @token = Token.create( user: user, package: pkg ) + @token = Token::Service.create( user: user, package: pkg ) end class TokenNotFound < APIException @@ -291,7 +291,7 @@ class TokenNotFound < APIException def delete_token user = User.get_by_login(params[:login]) - token = Token.where( user_id: user.id, id: params[:id] ).first + token = Token::Service.where( user_id: user.id, id: params[:id] ).first raise TokenNotFound, "Specified token \"#{params[:id]}\" got not found" unless token token.destroy render_ok diff --git a/src/api/app/controllers/trigger_controller.rb b/src/api/app/controllers/trigger_controller.rb index ae97f5e6fc4..a22521cadd2 100644 --- a/src/api/app/controllers/trigger_controller.rb +++ b/src/api/app/controllers/trigger_controller.rb @@ -19,7 +19,7 @@ def runservice return end - token = Token.find_by_string auth[6..-1] + token = Token::Service.find_by_string(auth[6..-1]) unless token render_error message: "Token not found", status: 404 diff --git a/src/api/app/controllers/webui/feeds_controller.rb b/src/api/app/controllers/webui/feeds_controller.rb index fae1a040afb..06674e6771a 100644 --- a/src/api/app/controllers/webui/feeds_controller.rb +++ b/src/api/app/controllers/webui/feeds_controller.rb @@ -40,4 +40,17 @@ def commits @commits = @commits.where(["datetime <= ?", @finish]) unless @finish.nil? @commits = @commits.order("datetime desc") end + + def notifications + token = Token::Rss.find_by_string(params[:token]) + if token + @configuration = ::Configuration.first + @user = token.user + @notifications = token.user.combined_rss_feed_items + @host = ::Configuration.obs_url + else + flash[:error] = "Unknown Token for RSS feed" + redirect_back(fallback_location: root_path) + end + end end diff --git a/src/api/app/controllers/webui/users/rss_tokens_controller.rb b/src/api/app/controllers/webui/users/rss_tokens_controller.rb new file mode 100644 index 00000000000..963c40b7056 --- /dev/null +++ b/src/api/app/controllers/webui/users/rss_tokens_controller.rb @@ -0,0 +1,20 @@ +module Webui + module Users + class RssTokensController < WebuiController + before_action :require_login + + def create + token = User.current.rss_token + if token + flash[:success] = "Successfully re-generated your RSS feed url" + token.regenerate_string + token.save + else + flash[:success] = "Successfully generated your RSS feed url" + User.current.create_rss_token + end + redirect_back(fallback_location: user_notifications_path) + end + end + end +end diff --git a/src/api/app/jobs/cleanup_notifications.rb b/src/api/app/jobs/cleanup_notifications.rb index 0e06db4b6be..93461e998fc 100644 --- a/src/api/app/jobs/cleanup_notifications.rb +++ b/src/api/app/jobs/cleanup_notifications.rb @@ -1,5 +1,5 @@ class CleanupNotifications < ApplicationJob def perform - Notifications::RssFeedItem.cleanup + Notification::RssFeedItem.cleanup end end diff --git a/src/api/app/jobs/send_event_emails.rb b/src/api/app/jobs/send_event_emails.rb index 12fc3a658eb..57151a4f8c2 100644 --- a/src/api/app/jobs/send_event_emails.rb +++ b/src/api/app/jobs/send_event_emails.rb @@ -16,16 +16,21 @@ def perform subscribers = event.subscribers next if subscribers.empty? EventMailer.event(subscribers, event).deliver_now - - event.subscriptions.each do |subscription| - Notifications::RssFeedItem.create( - subscriber: subscription.subscriber, - event_type: event.eventtype, - event_payload: event.payload, - subscription_receiver_role: subscription.receiver_role - ) - end + create_rss_notifications(event) end true end + + private + + def create_rss_notifications(event) + event.subscriptions.each do |subscription| + Notification::RssFeedItem.create( + subscriber: subscription.subscriber, + event_type: event.eventtype, + event_payload: event.payload, + subscription_receiver_role: subscription.receiver_role + ) + end + end end diff --git a/src/api/app/models/event_subscription.rb b/src/api/app/models/event_subscription.rb index 39026cc06da..2b4fa03fc11 100644 --- a/src/api/app/models/event_subscription.rb +++ b/src/api/app/models/event_subscription.rb @@ -60,7 +60,6 @@ def enabled? # created_at :datetime # updated_at :datetime # group_id :integer indexed -# receive :boolean # channel :integer default("disabled"), not null # # Indexes diff --git a/src/api/app/models/group.rb b/src/api/app/models/group.rb index 420b4db1b15..7d2adcf61a5 100644 --- a/src/api/app/models/group.rb +++ b/src/api/app/models/group.rb @@ -11,7 +11,7 @@ class Group < ApplicationRecord has_many :event_subscriptions, dependent: :destroy, inverse_of: :group has_many :reviews, dependent: :nullify, as: :reviewable - has_many :rss_feed_items, -> { order(created_at: :desc) }, class_name: 'Notifications::RssFeedItem', dependent: :destroy + has_many :rss_feed_items, -> { order(created_at: :desc) }, class_name: 'Notification::RssFeedItem', as: :subscriber, dependent: :destroy validates :title, format: { with: %r{\A[\w\.\-]*\z}, diff --git a/src/api/app/models/notifications/base.rb b/src/api/app/models/notification.rb similarity index 53% rename from src/api/app/models/notifications/base.rb rename to src/api/app/models/notification.rb index 402f386386d..5e2e270cf66 100644 --- a/src/api/app/models/notifications/base.rb +++ b/src/api/app/models/notification.rb @@ -1,15 +1,10 @@ -class Notifications::Base < ApplicationRecord - self.table_name = "notifications" +class Notification < ApplicationRecord + belongs_to :subscriber, polymorphic: true - belongs_to :user - belongs_to :group + serialize :event_payload, Hash - def subscriber=(subscriber) - if subscriber.is_a? User - self.user = subscriber - elsif subscriber.is_a? Group - self.group = subscriber - end + def event + @event ||= event_type.constantize.new(event_payload) end end @@ -18,8 +13,6 @@ def subscriber=(subscriber) # Table name: notifications # # id :integer not null, primary key -# user_id :integer indexed -# group_id :integer indexed # type :string(255) not null # event_type :string(255) not null # event_payload :text(65535) not null @@ -27,9 +20,10 @@ def subscriber=(subscriber) # delivered :boolean default(FALSE) # created_at :datetime not null # updated_at :datetime not null +# subscriber_type :string(255) indexed => [subscriber_id] +# subscriber_id :integer indexed => [subscriber_type] # # Indexes # -# index_notifications_on_group_id (group_id) -# index_notifications_on_user_id (user_id) +# index_notifications_on_subscriber_type_and_subscriber_id (subscriber_type,subscriber_id) # diff --git a/src/api/app/models/notifications/rss_feed_item.rb b/src/api/app/models/notification/rss_feed_item.rb similarity index 61% rename from src/api/app/models/notifications/rss_feed_item.rb rename to src/api/app/models/notification/rss_feed_item.rb index 96649b196a7..8521f1c6b19 100644 --- a/src/api/app/models/notifications/rss_feed_item.rb +++ b/src/api/app/models/notification/rss_feed_item.rb @@ -1,21 +1,18 @@ -class Notifications::RssFeedItem < Notifications::Base +class Notification::RssFeedItem < Notification MAX_ITEMS_PER_USER = 10 MAX_ITEMS_PER_GROUP = 10 def self.cleanup User.all_without_nobody.find_in_batches batch_size: 500 do |batch| batch.each do |user| - if user.is_active? - ids = user.rss_feed_items.pluck(:id).slice(MAX_ITEMS_PER_USER..-1) - user.rss_feed_items.where(id: ids).delete_all - else - user.rss_feed_items.delete_all - end + offset = user.is_active? ? MAX_ITEMS_PER_USER : 0 + ids = user.rss_feed_items.offset(offset).pluck(:id) + user.rss_feed_items.where(id: ids).delete_all end end Group.find_in_batches batch_size: 500 do |batch| batch.each do |group| - ids = group.rss_feed_items.pluck(:id).slice(MAX_ITEMS_PER_GROUP..-1) + ids = group.rss_feed_items.offset(MAX_ITEMS_PER_GROUP).pluck(:id) group.rss_feed_items.where(id: ids).delete_all end end @@ -27,8 +24,6 @@ def self.cleanup # Table name: notifications # # id :integer not null, primary key -# user_id :integer indexed -# group_id :integer indexed # type :string(255) not null # event_type :string(255) not null # event_payload :text(65535) not null @@ -36,9 +31,10 @@ def self.cleanup # delivered :boolean default(FALSE) # created_at :datetime not null # updated_at :datetime not null +# subscriber_type :string(255) indexed => [subscriber_id] +# subscriber_id :integer indexed => [subscriber_type] # # Indexes # -# index_notifications_on_group_id (group_id) -# index_notifications_on_user_id (user_id) +# index_notifications_on_subscriber_type_and_subscriber_id (subscriber_type,subscriber_id) # diff --git a/src/api/app/models/package.rb b/src/api/app/models/package.rb index e3373bfccd2..303595e3925 100644 --- a/src/api/app/models/package.rb +++ b/src/api/app/models/package.rb @@ -124,9 +124,9 @@ class PutFileNoPermission < APIException; setup 403; end validate :valid_name has_one :backend_package, foreign_key: :package_id, dependent: :destroy, inverse_of: :package - has_one :token, foreign_key: :package_id, dependent: :destroy + has_one :token, class_name: 'Token::Service', foreign_key: :package_id, dependent: :destroy - has_many :tokens, dependent: :destroy, inverse_of: :package + has_many :tokens, class_name: 'Token::Service', dependent: :destroy, inverse_of: :package def self.check_access?(package) return false if package.nil? diff --git a/src/api/app/models/token.rb b/src/api/app/models/token.rb index 8b72583b7fe..26f6ee4d52e 100644 --- a/src/api/app/models/token.rb +++ b/src/api/app/models/token.rb @@ -1,23 +1,10 @@ class Token < ApplicationRecord - belongs_to :user, foreign_key: 'user_id', inverse_of: :tokens + belongs_to :user, foreign_key: 'user_id', inverse_of: :service_tokens belongs_to :package, inverse_of: :tokens - validates :user_id, presence: true - after_create :update_token - - def self.find_by_string(token) - token = Token.where(string: token.to_s).includes(:package, :user).first - return unless token && token.user_id + has_secure_token :string - # package found and user has write access - token - end - - def update_token - # base64 with a length that is a multiple of 3 avoids trailing "=" chars - self.string = SecureRandom.base64(30) # 30 bytes leads to 40 chars string - save! - end + validates :user_id, presence: true end # == Schema Information @@ -28,6 +15,7 @@ def update_token # string :string(255) indexed # user_id :integer not null, indexed # package_id :integer indexed +# type :string(255) # # Indexes # diff --git a/src/api/app/models/token/rss.rb b/src/api/app/models/token/rss.rb new file mode 100644 index 00000000000..25658ccbdb1 --- /dev/null +++ b/src/api/app/models/token/rss.rb @@ -0,0 +1,24 @@ +class Token::Rss < Token +end + +# == Schema Information +# +# Table name: tokens +# +# id :integer not null, primary key +# string :string(255) indexed +# user_id :integer not null, indexed +# package_id :integer indexed +# type :string(255) +# +# Indexes +# +# index_tokens_on_string (string) UNIQUE +# package_id (package_id) +# user_id (user_id) +# +# Foreign Keys +# +# tokens_ibfk_1 (user_id => users.id) +# tokens_ibfk_2 (package_id => packages.id) +# diff --git a/src/api/app/models/token/service.rb b/src/api/app/models/token/service.rb new file mode 100644 index 00000000000..2d1debc9d0f --- /dev/null +++ b/src/api/app/models/token/service.rb @@ -0,0 +1,24 @@ +class Token::Service < Token +end + +# == Schema Information +# +# Table name: tokens +# +# id :integer not null, primary key +# string :string(255) indexed +# user_id :integer not null, indexed +# package_id :integer indexed +# type :string(255) +# +# Indexes +# +# index_tokens_on_string (string) UNIQUE +# package_id (package_id) +# user_id (user_id) +# +# Foreign Keys +# +# tokens_ibfk_1 (user_id => users.id) +# tokens_ibfk_2 (package_id => packages.id) +# diff --git a/src/api/app/models/user.rb b/src/api/app/models/user.rb index c656c3e9738..3a68eb6488e 100644 --- a/src/api/app/models/user.rb +++ b/src/api/app/models/user.rb @@ -36,7 +36,8 @@ class User < ApplicationRecord has_many :comments, dependent: :destroy, inverse_of: :user has_many :status_messages has_many :messages - has_many :tokens, dependent: :destroy, inverse_of: :user + has_many :service_tokens, class_name: 'Token::Service', dependent: :destroy, inverse_of: :user + has_one :rss_token, class_name: 'Token::Rss', dependent: :destroy has_many :reviews, dependent: :nullify, as: :reviewable @@ -54,7 +55,7 @@ class User < ApplicationRecord # users have 0..1 user_registration records assigned to them has_one :user_registration - has_many :rss_feed_items, -> { order(created_at: :desc) }, class_name: 'Notifications::RssFeedItem', dependent: :destroy + has_many :rss_feed_items, -> { order(created_at: :desc) }, class_name: 'Notification::RssFeedItem', as: :subscriber, dependent: :destroy scope :all_without_nobody, -> { where("login != ?", nobody_login) } @@ -904,6 +905,12 @@ def display_name address.format end + def combined_rss_feed_items + Notification::RssFeedItem.where(subscriber: self).or( + Notification::RssFeedItem.where(subscriber: groups) + ).order(created_at: :desc, id: :desc).limit(Notification::RssFeedItem::MAX_ITEMS_PER_USER) + end + private def can_modify_project_internal(project, ignore_lock) diff --git a/src/api/app/views/event_mailer/_actions.text.erb b/src/api/app/views/event_mailer/_actions.text.erb index 35f336ac310..0509636612b 100644 --- a/src/api/app/views/event_mailer/_actions.text.erb +++ b/src/api/app/views/event_mailer/_actions.text.erb @@ -1,8 +1,8 @@ Actions: <% event['actions'].each do |a| -%> <% if %w(submit maintenance_incident maintenance_release).include? a['type'] -%> -<%= render partial: 'submit_action', locals: { a: a } -%> +<%= render partial: 'event_mailer/submit_action', locals: { a: a } -%> <% else -%> -<%= render partial: "#{a['type']}_action", locals: { a: a } -%> +<%= render partial: "event_mailer/#{a['type']}_action", locals: { a: a } -%> <% end -%> <% end -%> diff --git a/src/api/app/views/event_mailer/request_create.text.erb b/src/api/app/views/event_mailer/request_create.text.erb index b80c30e2896..59a0005d0a9 100644 --- a/src/api/app/views/event_mailer/request_create.text.erb +++ b/src/api/app/views/event_mailer/request_create.text.erb @@ -5,14 +5,14 @@ Description: <%= event['description']%> <%-end-%> -<%= render partial: 'actions', locals: { event: event } %> +<%= render partial: 'event_mailer/actions', locals: { event: event } %> To REVIEW against the previous version: osc request show --diff <%= event['number']%> To ACCEPT the request: osc request accept <%= event['number']%> --message="reviewed ok." - + To DECLINE the request: osc request decline <%= event['number']%> --message="declined for reason xyz (see ... for background / policy / ...)." diff --git a/src/api/app/views/event_mailer/request_statechange.text.erb b/src/api/app/views/event_mailer/request_statechange.text.erb index e1ac15acc74..e517a0266e8 100644 --- a/src/api/app/views/event_mailer/request_statechange.text.erb +++ b/src/api/app/views/event_mailer/request_statechange.text.erb @@ -9,4 +9,4 @@ Comment: <%= event['comment'] %> <% end -%> -<%= render partial: 'actions', locals: { event: event } %> +<%= render partial: 'event_mailer/actions', locals: { event: event } %> diff --git a/src/api/app/views/event_mailer/review_wanted.text.erb b/src/api/app/views/event_mailer/review_wanted.text.erb index 8df68c2fbad..44c56b353cb 100644 --- a/src/api/app/views/event_mailer/review_wanted.text.erb +++ b/src/api/app/views/event_mailer/review_wanted.text.erb @@ -19,4 +19,4 @@ Review reason set by <%= event['who'] %>: Request description: <%= event['description'] %> -<%= render partial: 'actions', locals: { event: event } -%> +<%= render partial: 'event_mailer/actions', locals: { event: event } -%> diff --git a/src/api/app/views/layouts/webui/webui.html.erb b/src/api/app/views/layouts/webui/webui.html.erb index a12ea084d79..bfead88944e 100644 --- a/src/api/app/views/layouts/webui/webui.html.erb +++ b/src/api/app/views/layouts/webui/webui.html.erb @@ -37,6 +37,9 @@ <% end %> <% end %> <%= csrf_meta_tag %> + <% if User.current.try(:rss_token) %> + <%= auto_discovery_link_tag(:rss, user_rss_notifications_url(token: User.current.rss_token.string, format: :rss), { title: 'Notifications' }) %> + <% end %>
diff --git a/src/api/app/views/webui/feeds/notifications.rss.builder b/src/api/app/views/webui/feeds/notifications.rss.builder new file mode 100644 index 00000000000..e9c42eadad3 --- /dev/null +++ b/src/api/app/views/webui/feeds/notifications.rss.builder @@ -0,0 +1,25 @@ +xml.rss version: '2.0' do + xml.channel do + xml.title "#{@user.realname} (#{@user.login}) notifications" + xml.description "Event notifications from #{@configuration['title']}" + xml.link url_for only_path: false, controller: 'main', action: 'index' + xml.language "en" + xml.pubDate Time.now + xml.generator @configuration['title'] + + @notifications.each do |notification| + xml.item do + xml.title notification.event.subject + # We render the event_mailer view to 'description' + xml.description render( + template: "event_mailer/#{notification.event.template_name}", + layout: false, + formats: :text, + locals: { event: notification.event.expanded_payload }) + xml.category "#{notification.event_type}/#{notification.subscription_receiver_role}" + xml.pubDate notification.created_at + xml.author @configuration['title'] + end + end + end +end diff --git a/src/api/app/views/webui/user/show.html.haml b/src/api/app/views/webui/user/show.html.haml index ef14a1275af..88aba248fdf 100644 --- a/src/api/app/views/webui/user/show.html.haml +++ b/src/api/app/views/webui/user/show.html.haml @@ -45,6 +45,9 @@ %br/ = link_to(sprited_text('email', 'Change your notifications'), user_notifications_path) %br/ + - if @displayed_user.rss_token + = link_to(sprited_text('feeds', 'RSS for notifications'), user_rss_notifications_path(token: @displayed_user.rss_token.string, format: 'rss'), { title: 'RSS Feed for Notifications' }) + %br/ .grid_12.omega.box.box-shadow #involved diff --git a/src/api/app/views/webui/users/subscriptions/index.html.haml b/src/api/app/views/webui/users/subscriptions/index.html.haml index 6a71e33d9d9..49ba04bc418 100644 --- a/src/api/app/views/webui/users/subscriptions/index.html.haml +++ b/src/api/app/views/webui/users/subscriptions/index.html.haml @@ -13,3 +13,17 @@ = render partial: 'webui/subscriptions/subscriptions_form' %p= submit_tag 'Update' +%br/ +%br/ +%h3 RSS Feed += form_tag(user_rss_token_path, id: 'generate_rss_token_form', method: :post) do + %p + - if @user.rss_token + For accessing to your RSS feed of notifications use the following url + = link_to(user_rss_notifications_url(token: @user.rss_token.string, format: 'rss'), user_rss_notifications_url(token: @user.rss_token.string, format: 'rss'), target: '_blank') + %br/ + %br/ + Whenever you need to re-create the feed url just do it here + - else + No feed url exists already for your notifications + %p= submit_tag 'Generate Url' diff --git a/src/api/config/routes.rb b/src/api/config/routes.rb index c1555470c7d..8fa2733d290 100644 --- a/src/api/config/routes.rb +++ b/src/api/config/routes.rb @@ -58,6 +58,7 @@ def self.public_or_about_path?(request) get 'main/news' => :news, as: :news_feed get 'main/latest_updates' => :latest_updates, as: :latest_updates_feed get 'project/latest_commits/:project' => :commits, defaults: { format: 'atom' }, constraints: cons, as: 'commits_feed' + get 'user/feed/:token' => :notifications, defaults: { format: 'rss' }, as: :user_rss_notifications end resources :attribs, constraints: cons, only: [:create, :update, :destroy], controller: 'webui/attribute' do @@ -353,6 +354,10 @@ def self.public_or_about_path?(request) get 'users/(:user)/requests' => :index, as: 'user_requests' end + controller 'webui/users/rss_tokens' do + post 'users/rss_tokens' => :create, as: 'user_rss_token' + end + controller 'webui/groups' do get 'groups' => :index get 'group/show/:title' => :show, constraints: {:title => /[^\/]*/}, as: 'group_show' diff --git a/src/api/db/migrate/20170619111734_alter_notifications_to_use_polymorphic.rb b/src/api/db/migrate/20170619111734_alter_notifications_to_use_polymorphic.rb new file mode 100644 index 00000000000..b22c70785ab --- /dev/null +++ b/src/api/db/migrate/20170619111734_alter_notifications_to_use_polymorphic.rb @@ -0,0 +1,15 @@ +class AlterNotificationsToUsePolymorphic < ActiveRecord::Migration[5.0] + def change + add_reference :notifications, :subscriber, polymorphic: true + + Notification.find_in_batches batch_size: 500 do |batch| + batch.each do |notification| + notification.subscriber = notification.user || notification.group + notification.save! + end + end + + remove_reference :notifications, :group + remove_reference :notifications, :user + end +end diff --git a/src/api/db/migrate/20170621083718_add_st_ito_tokens.rb b/src/api/db/migrate/20170621083718_add_st_ito_tokens.rb new file mode 100644 index 00000000000..92838557bf6 --- /dev/null +++ b/src/api/db/migrate/20170621083718_add_st_ito_tokens.rb @@ -0,0 +1,5 @@ +class AddStItoTokens < ActiveRecord::Migration[5.0] + def change + add_column :tokens, :type, :string + end +end diff --git a/src/api/db/structure.sql b/src/api/db/structure.sql index 05b44a579f7..4cd4b1e8846 100644 --- a/src/api/db/structure.sql +++ b/src/api/db/structure.sql @@ -713,8 +713,6 @@ CREATE TABLE `messages` ( CREATE TABLE `notifications` ( `id` int(11) NOT NULL AUTO_INCREMENT, - `user_id` int(11) DEFAULT NULL, - `group_id` int(11) DEFAULT NULL, `type` varchar(255) NOT NULL, `event_type` varchar(255) NOT NULL, `event_payload` text NOT NULL, @@ -722,9 +720,10 @@ CREATE TABLE `notifications` ( `delivered` tinyint(1) DEFAULT '0', `created_at` datetime NOT NULL, `updated_at` datetime NOT NULL, + `subscriber_type` varchar(255) DEFAULT NULL, + `subscriber_id` int(11) DEFAULT NULL, PRIMARY KEY (`id`), - KEY `index_notifications_on_user_id` (`user_id`), - KEY `index_notifications_on_group_id` (`group_id`) + KEY `index_notifications_on_subscriber_type_and_subscriber_id` (`subscriber_type`,`subscriber_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; CREATE TABLE `package_issues` ( @@ -1095,6 +1094,7 @@ CREATE TABLE `tokens` ( `string` varchar(255) COLLATE utf8_unicode_ci DEFAULT NULL, `user_id` int(11) NOT NULL, `package_id` int(11) DEFAULT NULL, + `type` varchar(255) COLLATE utf8_unicode_ci DEFAULT NULL, PRIMARY KEY (`id`), UNIQUE KEY `index_tokens_on_string` (`string`) USING BTREE, KEY `user_id` (`user_id`) USING BTREE, @@ -1477,6 +1477,8 @@ INSERT INTO `schema_migrations` (version) VALUES ('20170516140442'), ('20170607110443'), ('20170614083014'), +('20170619111734'), +('20170621083718'), ('20170621100321'), ('20170621103748'), ('20170628115727'), diff --git a/src/api/spec/controllers/webui/feeds_controller_spec.rb b/src/api/spec/controllers/webui/feeds_controller_spec.rb index 7c696a23823..11a8fa5c0de 100644 --- a/src/api/spec/controllers/webui/feeds_controller_spec.rb +++ b/src/api/spec/controllers/webui/feeds_controller_spec.rb @@ -62,4 +62,42 @@ describe "GET latest_updates" do skip end + + describe 'GET #notifications' do + let(:user) { create(:confirmed_user) } + let(:payload) { + { author: "heino", description: "I want this role", number: 1899, + actions: [{action_id: 2004, type: "add_role", person: "heino", role: "maintainer", targetproject: user.home_project.to_param}], + state: "new", + when: "2017-06-27T10:34:30", + who: "heino"} + } + let!(:rss_notification) { create(:rss_notification, event_payload: payload, subscriber: user, event_type: 'Event::RequestCreate') } + + context 'with a working token' do + render_views + before do + ::Configuration.update(obs_url: 'http://localhost') + user.create_rss_token + get :notifications, params: { token: user.rss_token.string, format: 'rss' } + end + after do + ::Configuration.update(obs_url: nil) + end + + it { expect(assigns(:notifications)).to eq(user.combined_rss_feed_items) } + it { expect(response).to have_http_status(:success) } + it { is_expected.to render_template("webui/feeds/notifications") } + it { expect(response.body).to match(/heino wants to be maintainer in project/) } + end + + context 'with an invalid token' do + before do + get :notifications, params: { token: 'faken_token', format: 'rss' } + end + + it { expect(flash[:error]).to eq("Unknown Token for RSS feed") } + it { is_expected.to redirect_to(root_url) } + end + end end diff --git a/src/api/spec/controllers/webui/users/rss_token_controller_spec.rb b/src/api/spec/controllers/webui/users/rss_token_controller_spec.rb new file mode 100644 index 00000000000..00ca5301136 --- /dev/null +++ b/src/api/spec/controllers/webui/users/rss_token_controller_spec.rb @@ -0,0 +1,34 @@ +require 'rails_helper' + +RSpec.describe Webui::Users::RssTokensController do + describe 'POST #create' do + let(:user) { create(:confirmed_user) } + + before do + login(user) + end + + context 'with a user with an existent token' do + before do + @last_token = user.create_rss_token.string + post :create + end + + it { expect(flash[:success]).to eq("Successfully re-generated your RSS feed url") } + it { is_expected.to redirect_to(user_notifications_path) } + it { expect(user.reload.rss_token.string).not_to eq(@last_token) } + end + + context 'with a user without a token' do + before do + @last_token = user.rss_token + post :create + end + + it { expect(flash[:success]).to eq("Successfully generated your RSS feed url") } + it { is_expected.to redirect_to(user_notifications_path) } + it { expect(user.reload.rss_token).not_to be_nil } + it { expect(@last_token).to be_nil } + end + end +end diff --git a/src/api/spec/factories/notification.rb b/src/api/spec/factories/notification.rb index de7b0fa54c2..45145342a60 100644 --- a/src/api/spec/factories/notification.rb +++ b/src/api/spec/factories/notification.rb @@ -1,10 +1,10 @@ FactoryGirl.define do - factory :notification, class: 'Notifications::Base' do + factory :notification do event_type 'FakeEventType' - event_payload 'FakeJsonPayload' + event_payload { { fake: 'payload' } } subscription_receiver_role 'owner' end - factory :rss_notification, parent: :notification, class: 'Notifications::RssFeedItem' do + factory :rss_notification, parent: :notification, class: 'Notification::RssFeedItem' do end end diff --git a/src/api/spec/jobs/send_event_emails_spec.rb b/src/api/spec/jobs/send_event_emails_spec.rb index c0114d44ba1..e757ea14147 100644 --- a/src/api/spec/jobs/send_event_emails_spec.rb +++ b/src/api/spec/jobs/send_event_emails_spec.rb @@ -30,27 +30,27 @@ end it "creates an rss notification for user's email" do - notification = Notifications::Base.find_by(user: user) + notification = Notification.find_by(subscriber: user) - expect(notification.type).to eq('Notifications::RssFeedItem') + expect(notification.type).to eq('Notification::RssFeedItem') expect(notification.event_type).to eq('Event::CommentForProject') - expect(notification.event_payload).to include('how are things?') + expect(notification.event_payload['comment_body']).to include('how are things?') expect(notification.subscription_receiver_role).to eq('all') expect(notification.delivered).to be_falsey end it "creates an rss notification for group's email" do - notification = Notifications::RssFeedItem.find_by(group: group) + notification = Notification::RssFeedItem.find_by(subscriber: group) - expect(notification.type).to eq('Notifications::RssFeedItem') + expect(notification.type).to eq('Notification::RssFeedItem') expect(notification.event_type).to eq('Event::CommentForProject') - expect(notification.event_payload).to include('how are things?') + expect(notification.event_payload['comment_body']).to include('how are things?') expect(notification.subscription_receiver_role).to eq('all') expect(notification.delivered).to be_falsey end it 'only creates two notifications' do - expect(Notifications::Base.count).to eq(2) + expect(Notification.count).to eq(2) end end end diff --git a/src/api/spec/models/notification/rss_feed_item_spec.rb b/src/api/spec/models/notification/rss_feed_item_spec.rb new file mode 100644 index 00000000000..d41cba4d9a7 --- /dev/null +++ b/src/api/spec/models/notification/rss_feed_item_spec.rb @@ -0,0 +1,85 @@ +require "rails_helper" + +RSpec.describe Notification::RssFeedItem do + let(:payload) { { comment: 'SuperFakeComment', requestid: 1 } } + let(:delete_package_event) { Event::DeletePackage.new(payload) } + + describe '.cleanup' do + let(:user) { create(:confirmed_user) } + let(:user2) { create(:confirmed_user) } + let(:group) { create(:group, users: [user, user2]) } + let(:unconfirmed_user) { create(:user) } + let(:max_items_per_user) { Notification::RssFeedItem::MAX_ITEMS_PER_USER } + let(:max_items_per_group) { Notification::RssFeedItem::MAX_ITEMS_PER_GROUP } + let(:greater_than_max_items_per_user) { max_items_per_user + 3 } + let(:greater_than_max_items_per_group) { max_items_per_group + 5 } + + context 'without any RSS items' do + it { expect{ Notification::RssFeedItem.cleanup }.not_to(change{ Notification::RssFeedItem.count }) } + end + + context 'with a single users' do + context 'and not enough RSS items' do + before do + create_list(:rss_notification, max_items_per_user, subscriber: user) + end + + it { expect { Notification::RssFeedItem.cleanup }.not_to(change { Notification::RssFeedItem.count }) } + end + + context 'and enough RSS items' do + context 'for an active user' do + before do + create_list(:rss_notification, greater_than_max_items_per_user, subscriber: user) + end + + it { expect { Notification::RssFeedItem.cleanup }.to change { Notification::RssFeedItem.count }.by(-3) } + end + + context 'for a non active user' do + before do + create_list(:rss_notification, greater_than_max_items_per_user, subscriber: unconfirmed_user) + end + + it { expect { Notification::RssFeedItem.cleanup }.to change { Notification::RssFeedItem.count }.by(-greater_than_max_items_per_user) } + end + end + end + + context 'with multiple users' do + context 'all of them active' do + before do + create_list(:rss_notification, greater_than_max_items_per_user, subscriber: user) + create_list(:rss_notification, greater_than_max_items_per_user, subscriber: user2) + end + + it { expect { Notification::RssFeedItem.cleanup }.to change { user.rss_feed_items.count }.by(-3) } + it { expect { Notification::RssFeedItem.cleanup }.to change { user2.rss_feed_items.count }.by(-3) } + end + + context 'being a mixture of active and non active users' do + before do + create_list(:rss_notification, greater_than_max_items_per_user, subscriber: user) + create_list(:rss_notification, greater_than_max_items_per_user, subscriber: unconfirmed_user) + end + + it { expect { Notification::RssFeedItem.cleanup }.to change { user.rss_feed_items.count }.by(-3) } + it { expect { Notification::RssFeedItem.cleanup }.to change { unconfirmed_user.rss_feed_items.count }.by(-greater_than_max_items_per_user) } + end + end + + context 'with users and group notifications' do + before do + create_list(:rss_notification, greater_than_max_items_per_user, subscriber: user) + create_list(:rss_notification, greater_than_max_items_per_user, subscriber: user2) + create_list(:rss_notification, greater_than_max_items_per_user, subscriber: unconfirmed_user) + create_list(:rss_notification, greater_than_max_items_per_group, subscriber: group) + end + + it { expect { Notification::RssFeedItem.cleanup }.to change { user.rss_feed_items.count }.by(-3) } + it { expect { Notification::RssFeedItem.cleanup }.to change { user2.rss_feed_items.count }.by(-3) } + it { expect { Notification::RssFeedItem.cleanup }.to change { group.rss_feed_items.count }.by(-5) } + it { expect { Notification::RssFeedItem.cleanup }.to change { unconfirmed_user.rss_feed_items.count }.by(-greater_than_max_items_per_user) } + end + end +end diff --git a/src/api/spec/models/notification_rss_feed_item.rb b/src/api/spec/models/notification_rss_feed_item.rb deleted file mode 100644 index adb610ac0a7..00000000000 --- a/src/api/spec/models/notification_rss_feed_item.rb +++ /dev/null @@ -1,82 +0,0 @@ -require "rails_helper" - -RSpec.describe Notifications::RssFeedItem do - describe '.cleanup' do - let(:user) { create(:confirmed_user) } - let(:user2) { create(:confirmed_user) } - let(:group) { create(:group, users: [user, user2]) } - let(:unconfirmed_user) { create(:user) } - let(:max_items_per_user) { Notifications::RssFeedItem::MAX_ITEMS_PER_USER } - let(:max_items_per_group) { Notifications::RssFeedItem::MAX_ITEMS_PER_GROUP } - let(:greater_than_max_items_per_user) { max_items_per_user + 3 } - let(:greater_than_max_items_per_group) { max_items_per_group + 5 } - - context 'without any RSS items' do - it { expect { Notifications::RssFeedItem.cleanup }.not_to change { Notifications::RssFeedItem.count } } - end - - context 'with a single users' do - context 'and not enough RSS items' do - before do - create_list(:rss_notification, max_items_per_user, user: user) - end - - it { expect { Notifications::RssFeedItem.cleanup }.not_to change { Notifications::RssFeedItem.count } } - end - - context 'and enough RSS items' do - context 'for an active user' do - before do - create_list(:rss_notification, greater_than_max_items_per_user, user: user) - end - - it { expect { Notifications::RssFeedItem.cleanup }.to change { Notifications::RssFeedItem.count }.by(-3) } - end - - context 'for a non active user' do - before do - create_list(:rss_notification, greater_than_max_items_per_user, user: unconfirmed_user) - end - - it { expect { Notifications::RssFeedItem.cleanup }.to change { Notifications::RssFeedItem.count }.by(-greater_than_max_items_per_user) } - end - end - end - - context 'with multiple users' do - context 'all of them active' do - before do - create_list(:rss_notification, greater_than_max_items_per_user, user: user) - create_list(:rss_notification, greater_than_max_items_per_user, user: user2) - end - - it { expect { Notifications::RssFeedItem.cleanup }.to change { user.rss_feed_items.count }.by(-3) } - it { expect { Notifications::RssFeedItem.cleanup }.to change { user2.rss_feed_items.count }.by(-3) } - end - - context 'being a mixture of active and non active users' do - before do - create_list(:rss_notification, greater_than_max_items_per_user, user: user) - create_list(:rss_notification, greater_than_max_items_per_user, user: unconfirmed_user) - end - - it { expect { Notifications::RssFeedItem.cleanup }.to change { user.rss_feed_items.count }.by(-3) } - it { expect { Notifications::RssFeedItem.cleanup }.to change { unconfirmed_user.rss_feed_items.count }.by(-greater_than_max_items_per_user) } - end - end - - context 'with users and group notifications' do - before do - create_list(:rss_notification, greater_than_max_items_per_user, user: user) - create_list(:rss_notification, greater_than_max_items_per_user, user: user2) - create_list(:rss_notification, greater_than_max_items_per_user, user: unconfirmed_user) - create_list(:rss_notification, greater_than_max_items_per_group, group: group) - end - - it { expect { Notifications::RssFeedItem.cleanup }.to change { user.rss_feed_items.count }.by(-3) } - it { expect { Notifications::RssFeedItem.cleanup }.to change { user2.rss_feed_items.count }.by(-3) } - it { expect { Notifications::RssFeedItem.cleanup }.to change { group.rss_feed_items.count }.by(-5) } - it { expect { Notifications::RssFeedItem.cleanup }.to change { unconfirmed_user.rss_feed_items.count }.by(-greater_than_max_items_per_user) } - end - end -end diff --git a/src/api/spec/models/notification_spec.rb b/src/api/spec/models/notification_spec.rb new file mode 100644 index 00000000000..2bc17aa27a5 --- /dev/null +++ b/src/api/spec/models/notification_spec.rb @@ -0,0 +1,13 @@ +require "rails_helper" + +RSpec.describe Notification do + let(:payload) { {comment: 'SuperFakeComment', requestid: 1} } + let(:delete_package_event) { Event::DeletePackage.new(payload) } + + describe '#event' do + subject { create(:rss_notification, event_type: 'Event::DeletePackage', event_payload: payload).event } + + it { expect(subject.class).to eq(delete_package_event.class) } + it { expect(subject.payload).to eq(delete_package_event.payload) } + end +end diff --git a/src/api/spec/models/user_spec.rb b/src/api/spec/models/user_spec.rb index b1c7214d444..4f00f53aefd 100644 --- a/src/api/spec/models/user_spec.rb +++ b/src/api/spec/models/user_spec.rb @@ -727,4 +727,60 @@ end end end + + describe '#combined_rss_feed_items' do + let(:max_items_per_user) { Notification::RssFeedItem::MAX_ITEMS_PER_USER } + let(:max_items_per_group) { Notification::RssFeedItem::MAX_ITEMS_PER_GROUP } + let(:group) { create(:group) } + let!(:groups_user) { create(:groups_user, user: confirmed_user, group: group) } + + context 'with a lot notifications from the user' do + before do + create_list(:rss_notification, max_items_per_group, subscriber: group) + create_list(:rss_notification, max_items_per_user + 5, subscriber: confirmed_user) + create_list(:rss_notification, 3, subscriber: user) + end + + subject { confirmed_user.combined_rss_feed_items } + + it { expect(subject.count).to be(max_items_per_user) } + it { expect(subject.any? { |x| x.subscriber != confirmed_user }).to be_falsey } + it { expect(subject.any? { |x| x.subscriber == group }).to be_falsey } + it { expect(subject.any? { |x| x.subscriber == user }).to be_falsey } + end + + context 'with a lot notifications from the group' do + before do + create_list(:rss_notification, 5, subscriber: confirmed_user) + create_list(:rss_notification, max_items_per_group - 1, subscriber: group) + create_list(:rss_notification, 3, subscriber: user) + end + + subject { confirmed_user.combined_rss_feed_items } + + it { expect(subject.count).to be(max_items_per_user) } + it { expect(subject.select { |x| x.subscriber == confirmed_user }.length).to eq(1) } + it { expect(subject.select { |x| x.subscriber == group }.length).to eq(max_items_per_user - 1) } + it { expect(subject.any? { |x| x.subscriber == user }).to be_falsey } + end + + context 'with a notifications mixed' do + let(:batch) { max_items_per_user / 4 } + + before do + create_list(:rss_notification, max_items_per_user + batch, subscriber: confirmed_user) + create_list(:rss_notification, batch, subscriber: group) + create_list(:rss_notification, batch, subscriber: confirmed_user) + create_list(:rss_notification, batch, subscriber: group) + create_list(:rss_notification, 3, subscriber: user) + end + + subject { confirmed_user.combined_rss_feed_items } + + it { expect(subject.count).to be(max_items_per_user) } + it { expect(subject.select { |x| x.subscriber == confirmed_user }.length).to be >= batch * 2 } + it { expect(subject.select { |x| x.subscriber == group }.length).to eq(batch * 2) } + it { expect(subject.any? { |x| x.subscriber == user }).to be_falsey } + end + end end diff --git a/src/api/test/fixtures/event_mailer/repo_delete_request b/src/api/test/fixtures/event_mailer/repo_delete_request index b292c58d407..71fff821b5a 100644 --- a/src/api/test/fixtures/event_mailer/repo_delete_request +++ b/src/api/test/fixtures/event_mailer/repo_delete_request @@ -29,7 +29,7 @@ To REVIEW against the previous version: To ACCEPT the request: osc request accept REQUESTID --message="reviewed ok." - + To DECLINE the request: osc request decline REQUESTID --message="declined for reason xyz (see ... for background / policy / ...)." diff --git a/src/api/test/fixtures/event_mailer/request_event b/src/api/test/fixtures/event_mailer/request_event index cd4d3a01dda..8ec5aaa4665 100644 --- a/src/api/test/fixtures/event_mailer/request_event +++ b/src/api/test/fixtures/event_mailer/request_event @@ -29,7 +29,7 @@ To REVIEW against the previous version: To ACCEPT the request: osc request accept REQUESTID --message="reviewed ok." - + To DECLINE the request: osc request decline REQUESTID --message="declined for reason xyz (see ... for background / policy / ...)." diff --git a/src/api/test/fixtures/event_mailer/set_bugowner_event b/src/api/test/fixtures/event_mailer/set_bugowner_event index bc14ffa11a1..bd08d85290c 100644 --- a/src/api/test/fixtures/event_mailer/set_bugowner_event +++ b/src/api/test/fixtures/event_mailer/set_bugowner_event @@ -29,7 +29,7 @@ To REVIEW against the previous version: To ACCEPT the request: osc request accept REQUESTID --message="reviewed ok." - + To DECLINE the request: osc request decline REQUESTID --message="declined for reason xyz (see ... for background / policy / ...)." diff --git a/src/api/test/fixtures/event_mailer/tom_gets_mail_too b/src/api/test/fixtures/event_mailer/tom_gets_mail_too index e37ad02ebe9..57f09570c3c 100644 --- a/src/api/test/fixtures/event_mailer/tom_gets_mail_too +++ b/src/api/test/fixtures/event_mailer/tom_gets_mail_too @@ -31,7 +31,7 @@ To REVIEW against the previous version: To ACCEPT the request: osc request accept REQUESTID --message="reviewed ok." - + To DECLINE the request: osc request decline REQUESTID --message="declined for reason xyz (see ... for background / policy / ...)." diff --git a/src/api/test/fixtures/events.yml b/src/api/test/fixtures/events.yml index 86066a4fd4b..63f4381bea8 100644 --- a/src/api/test/fixtures/events.yml +++ b/src/api/test/fixtures/events.yml @@ -1,7 +1,7 @@ build_fails_with_deleted_user_and_request: id: 1 eventtype: Event::BuildFail - payload: '{"project":"BaseDistro","package":"pack1","repository":"10.2","arch":"i586","requestid":"666","sender":"no_longer_there","rev":"5"}' + payload: '{"project":"BaseDistro","package":"pack1","repository":"10.2","arch":"i586","sender":"no_longer_there","rev":"5"}' queued: 0 lock_version: 0 created_at: 2013-08-30 14:56:50.000000000 Z @@ -10,7 +10,7 @@ build_fails_with_deleted_user_and_request: build_failure_for_iggy: id: 6 eventtype: Event::BuildFail - payload: '{"project":"home:Iggy","package":"TestPack","repository":"10.2","arch":"i586","release":"42.1","versrel":"1.0-42","readytime":"1377958901","srcmd5":"1ac07842727acaf13d0e2b3213b47785","rev":"2","revtime":"1377958897","reason":"new + payload: '{"project":"home:Iggy","package":"TestPack","repository":"10.2","arch":"i586","release":"42.1","versrel":"1.0-42","readytime":"1377958901","srcmd5":"1ac07842727acaf13d0e2b3213b47785","rev":"2","reason":"new build","bcnt":"1","verifymd5":"1ac07842727acaf13d0e2b3213b47785","workerid":"build12"}' queued: 0 lock_version: 0 @@ -20,7 +20,7 @@ build_failure_for_iggy: build_failure_for_reader: id: 19 eventtype: Event::BuildFail - payload: '{"project":"home:Iggy","package":"TestPack","repository":"10.2","arch":"i586","release":"42.1","versrel":"1.0-42","readytime":"1377958901","srcmd5":"1ac07842727acaf13d0e2b3213b47785","rev":"2","revtime":"1377958897","reason":"new + payload: '{"project":"home:Iggy","package":"TestPack","repository":"10.2","arch":"i586","release":"42.1","versrel":"1.0-42","readytime":"1377958901","srcmd5":"1ac07842727acaf13d0e2b3213b47785","rev":"2","reason":"new build","bcnt":"1","verifymd5":"1ac07842727acaf13d0e2b3213b47785","workerid":"build12"}' queued: 0 lock_version: 0 @@ -39,7 +39,7 @@ service_failure_for_iggy: build_success_from_deleted_project: id: 2 eventtype: Event::BuildSuccess - payload: '{"project":"NotLongerThere","package":"IsAlsoGone","user":"Iggy","arch":"WhoCares"}' + payload: '{"project":"NotLongerThere","package":"IsAlsoGone","arch":"WhoCares"}' queued: 0 lock_version: 0 created_at: 2013-08-30 14:56:50.000000000 Z diff --git a/src/api/test/functional/source_services_test.rb b/src/api/test/functional/source_services_test.rb index 2f49b66c739..e3f97640469 100644 --- a/src/api/test/functional/source_services_test.rb +++ b/src/api/test/functional/source_services_test.rb @@ -498,12 +498,12 @@ def test_run_service_via_token assert_response :success doc = REXML::Document.new(@response.body) alltoken = doc.elements['//data'].text - assert_equal 40, alltoken.length + assert_equal 24, alltoken.length post '/person/tom/token?cmd=create&project=home:tom&package=service' assert_response :success doc = REXML::Document.new(@response.body) token = doc.elements['//data'].text - assert_equal 40, token.length + assert_equal 24, token.length # ANONYMOUS reset_auth