Skip to content

Commit

Permalink
remove explicit project column from notifications
Browse files Browse the repository at this point in the history
  • Loading branch information
ulferts committed Aug 21, 2024
1 parent 6fe005d commit 6d407d4
Show file tree
Hide file tree
Showing 30 changed files with 129 additions and 108 deletions.
1 change: 0 additions & 1 deletion app/contracts/notifications/create_contract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class CreateContract < ::ModelContract
attribute :recipient
attribute :subject
attribute :reason
attribute :project
attribute :actor
attribute :resource
attribute :journal
Expand Down
1 change: 0 additions & 1 deletion app/models/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class Notification < ApplicationRecord

belongs_to :recipient, class_name: "User"
belongs_to :actor, class_name: "User"
belongs_to :project
belongs_to :journal
belongs_to :resource, polymorphic: true

Expand Down
11 changes: 11 additions & 0 deletions app/models/queries/notifications/filters/project_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,15 @@

class Queries::Notifications::Filters::ProjectFilter < Queries::Notifications::Filters::NotificationFilter
include Queries::Filters::Shared::ProjectFilter::Optional

# This is currently work package specific same as all the other parts of the NotificationQuery
self.model = WorkPackage

def joins
<<~SQL.squish
JOIN #{WorkPackage.table_name}
ON #{WorkPackage.table_name}.id = #{Notification.table_name}.resource_id
AND #{Notification.table_name}.resource_type = 'WorkPackage'
SQL
end
end
10 changes: 10 additions & 0 deletions app/models/queries/notifications/group_bys/group_by_project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ def self.key
:project
end

def joins
# Only Notifications for work_packages are currently supported via the query.
# E.g. the visible statement used in the query is WorkPackage specific.
<<~SQL.squish
JOIN work_packages
ON notifications.resource_id = work_packages.id
AND notifications.resource_type = 'WorkPackage'
SQL
end

def name
:project_id
end
Expand Down
16 changes: 12 additions & 4 deletions app/models/queries/notifications/orders/project_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,23 @@ def self.key
end

def joins
:project
<<~SQL.squish
JOIN #{WorkPackage.table_name} work_packages_order
ON work_packages_order.id = #{Notification.table_name}.resource_id
AND #{Notification.table_name}.resource_type = 'WorkPackage'
JOIN #{Project.table_name}
ON #{Project.table_name}.id = work_packages_order.project_id
SQL
end

protected

def order(scope)
order_string = "projects.name"
order_string += " DESC" if direction == :desc
with_raise_on_invalid do
order_string = "#{Project.table_name}.name"
order_string += " DESC" if direction == :desc

scope.order(order_string)
scope.order(order_string)
end
end
end
1 change: 0 additions & 1 deletion app/services/notifications/create_from_model_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ def update_or_create_notification(recipient_id, reason)
def create_notification(recipient_id, reason)
notification_attributes = {
recipient_id:,
project:,
resource:,
journal:,
actor: user_with_fallback,
Expand Down
17 changes: 1 addition & 16 deletions app/services/notifications/set_attributes_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,5 @@
#++

module Notifications
class SetAttributesService < ::BaseServices::SetAttributes
private

def set_default_attributes(params)
super

set_default_project unless model.project
end

##
# Try to determine the project context from the journal (if any)
# or the resource if it has a project set
def set_default_project
model.project = model.journal&.project || model.resource.try(:project)
end
end
class SetAttributesService < ::BaseServices::SetAttributes; end
end
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ def create_date_alert_notification(user, work_package, reason)
create_service = Notifications::CreateService.new(user:)
create_service.call(
recipient_id: user.id,
project_id: work_package.project_id,
resource: work_package,
reason:
)
Expand Down
33 changes: 33 additions & 0 deletions db/migrate/20240820123011_remove_project_from_notification.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# 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.
#
# See COPYRIGHT and LICENSE files for more details.
#++

class RemoveProjectFromNotification < ActiveRecord::Migration[7.1]
def change
remove_reference :notifications, :project
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ def wrap(notifications)
# because it is a polymorphic association. That being as it is, currently only
# work packages are assigned.
def set_resource(notifications)
work_packages_by_id = WorkPackage.where(id: notifications.pluck(:resource_id).uniq).index_by(&:id)
work_packages_by_id = WorkPackage
.includes(:project)
.where(id: notifications.pluck(:resource_id).uniq)
.index_by(&:id)

notifications.each do |notification|
notification.resource = work_packages_by_id[notification.resource_id]
Expand Down
26 changes: 23 additions & 3 deletions lib/api/v3/notifications/notification_representer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,18 @@ class NotificationRepresenter < ::API::Decorators::Single
}
end

link :project do
{
href: api_v3_paths.project(represented.resource.project.id),
name: represented.resource.project.name
}
end

associated_resource :actor,
representer: ::API::V3::Users::UserRepresenter,
skip_render: ->(*) { represented.actor.nil? },
v3_path: :user

associated_resource :project

associated_resource :journal,
as: :activity,
representer: ::API::V3::Activities::ActivityRepresenter,
Expand All @@ -87,11 +92,26 @@ class NotificationRepresenter < ::API::Decorators::Single

polymorphic_resource :resource

resource :project,
getter: ->(*) {
::API::V3::Projects::ProjectRepresenter
.create(represented.resource.project, current_user:, embed_links:)
},
link: ->(*) {
{
href: api_v3_paths.project(represented.resource.project.id),
title: represented.resource.project.name
}
},
setter: ->(*) {
raise NotImplementedError
}

def _type
"Notification"
end

self.to_eager_load = %i[project actor journal]
self.to_eager_load = %i[actor journal]
end
end
end
Expand Down
4 changes: 1 addition & 3 deletions spec/contracts/notifications/create_contract_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
end
end

let(:notification_context) { build_stubbed(:project) }
let(:notification_resource) { build_stubbed(:journal) }
let(:notification_recipient) { build_stubbed(:user) }
let(:notification_subject) { "Some text" }
Expand All @@ -46,8 +45,7 @@
let(:notification_mail_reminder_sent) { false }

let(:notification) do
Notification.new(project: notification_context,
recipient: notification_recipient,
Notification.new(recipient: notification_recipient,
subject: notification_subject,
reason: notification_reason,
resource: notification_resource,
Expand Down
5 changes: 2 additions & 3 deletions spec/factories/notification_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
mail_alert_sent { false }
reason { :mentioned }
recipient factory: :user
project
resource { association :work_package, project: }
resource { association :work_package }

trait :for_milestone do
resource { association :work_package, :is_milestone, project: }
resource { association :work_package, :is_milestone }
end
# journal and actor are not listed by intend.
# They will be set in the after_build callback.
Expand Down
2 changes: 0 additions & 2 deletions spec/features/notifications/navigation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@
shared_let(:notification) do
create(:notification,
recipient:,
project:,
resource: work_package,
journal: work_package.journals.last)
end

shared_let(:second_notification) do
create(:notification,
recipient:,
project:,
resource: second_work_package,
journal: second_work_package.journals.last)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,14 @@
reason: :mentioned,
recipient: user,
resource: work_package,
actor:,
project:)
actor:)
end

shared_let(:notification_date_alert) do
create(:notification,
reason: :date_alert_due_date,
recipient: user,
resource: work_package,
project:)
resource: work_package)
end

let(:center) { Pages::Notifications::Center.new }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,14 @@ def create_alertable(**attributes)
create(:notification,
reason: :date_alert_due_date,
recipient: user,
resource: milestone_wp_future,
project:)
resource: milestone_wp_future)
end

shared_let(:notification_wp_start_past) do
create(:notification,
reason: :date_alert_start_date,
recipient: user,
resource: wp_start_past,
project:)
resource: wp_start_past)
end

# notification created by CreateDateAlertsNotificationsJob
Expand All @@ -121,30 +119,26 @@ def create_alertable(**attributes)
create(:notification,
reason: :date_alert_due_date,
recipient: user,
resource: wp_double_notification,
project:)
resource: wp_double_notification)
end

shared_let(:notification_wp_double_mention) do
create(:notification,
reason: :mentioned,
recipient: user,
resource: wp_double_notification,
project:)
resource: wp_double_notification)
end

shared_let(:notification_wp_double_alerts) do
due = create(:notification,
reason: :date_alert_due_date,
recipient: user,
resource: wp_double_alert,
project:)
resource: wp_double_alert)

start = create(:notification,
reason: :date_alert_start_date,
recipient: user,
resource: wp_double_alert,
project:)
resource: wp_double_alert)

[start, due]
end
Expand All @@ -153,16 +147,14 @@ def create_alertable(**attributes)
create(:notification,
reason: :date_alert_due_date,
recipient: user,
resource: wp_unset_date,
project:)
resource: wp_unset_date)
end

shared_let(:notification_wp_due_today) do
create(:notification,
reason: :date_alert_due_date,
recipient: user,
resource: wp_due_today,
project:)
resource: wp_due_today)
end

let(:center) { Pages::Notifications::Center.new }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,47 +28,41 @@
let(:notification_watched) do
create(:notification,
recipient:,
project:,
resource: work_package,
reason: :watched)
end

let(:notification_assigned) do
create(:notification,
recipient:,
project: project2,
resource: work_package2,
reason: :assigned)
end

let(:notification_responsible) do
create(:notification,
recipient:,
project: project3,
resource: work_package3,
reason: :responsible)
end

let(:notification_mentioned) do
create(:notification,
recipient:,
project: project3,
resource: work_package4,
reason: :mentioned)
end

let(:notification_date) do
create(:notification,
recipient:,
project: project3,
resource: work_package5,
reason: :date_alert_start_date)
end

let(:notification_shared) do
create(:notification,
recipient:,
project: project3,
resource: work_package6,
reason: :shared)
end
Expand Down
Loading

0 comments on commit 6d407d4

Please sign in to comment.