From 67df724d07d7b911f0649d81fd205712f84e1716 Mon Sep 17 00:00:00 2001 From: Mike Virata-Stone Date: Tue, 2 Sep 2025 20:48:24 -0700 Subject: [PATCH 1/7] Add new root user role and make sure super user cannot update root users --- app/models/concerns/users/info.rb | 6 ++- app/models/concerns/users/user_manipulator.rb | 36 ++++++++++++---- app/models/role.rb | 27 ++++++++++++ app/models/user.rb | 4 ++ app/views/users/edit.html.erb | 10 +++-- config/locales/en.yml | 1 + spec/fixtures/users.yml | 7 ++++ spec/models/user_spec.rb | 41 +++++++++++++++++++ 8 files changed, 119 insertions(+), 13 deletions(-) create mode 100644 app/models/role.rb diff --git a/app/models/concerns/users/info.rb b/app/models/concerns/users/info.rb index 491050d5..3f40c019 100644 --- a/app/models/concerns/users/info.rb +++ b/app/models/concerns/users/info.rb @@ -2,8 +2,12 @@ module Users module Info extend ActiveSupport::Concern + def root_admin? + role == "root" + end + def super_admin? - role == "admin" + root_admin? || role == "admin" end def report_admin? diff --git a/app/models/concerns/users/user_manipulator.rb b/app/models/concerns/users/user_manipulator.rb index 2bd97b2b..e65c8489 100644 --- a/app/models/concerns/users/user_manipulator.rb +++ b/app/models/concerns/users/user_manipulator.rb @@ -3,6 +3,10 @@ module UserManipulator extend ActiveSupport::Concern attr_reader :original_email + def super_edit_user_access?(user) + super_admin? && role_object >= user.role_object + end + def can_invite_user? super_admin? || admin? end @@ -11,8 +15,14 @@ def can_invite_user_at?(organization) super_admin? || admin_at?(organization) end - def can_delete_user? - super_admin? + def can_delete_user?(user = nil) + if !user + # Checking if this user has general access to delete other users + super_admin? + else + # Deleting a specific other user + super_edit_user_access?(user) + end end def can_update_user?(user = nil) @@ -24,7 +34,7 @@ def can_update_user?(user = nil) true else # Updating a specific user requires update user permission at that organization - super_admin? || user.organizations.any? { |organization| can_update_user_at?(organization) } + super_edit_user_access?(user) || user.organizations.any? { |organization| can_update_user_at?(organization) } end end @@ -33,20 +43,20 @@ def can_force_password_reset?(user = nil) # Checking if this user has general access to force password resets super_admin? || admin? else - super_admin? || user.organizations.any? { |organization| can_force_password_reset_at?(organization) } + super_edit_user_access?(user) || user.organizations.any? { |organization| can_force_password_reset_at?(organization) } end end def can_update_user_details?(user) - super_admin? || user == self + super_edit_user_access?(user) || user == self end def can_update_user_role?(user) - super_admin? || user.organizations.any? { |organization| can_update_user_role_at?(organization) } + super_edit_user_access?(user) || user.organizations.any? { |organization| can_update_user_role_at?(organization) } end def can_update_password?(user) - super_admin? || user == self + super_edit_user_access?(user) || user == self end def can_update_user_at?(organization) @@ -78,7 +88,13 @@ def update_user(params) user = transaction do user = User.find(params[:id]) raise PermissionError unless can_update_user?(user) - user.update_details(permitted_params(params)) if can_update_user_details?(user) && params[:user].present? + + if can_update_user_details?(user) && params[:user].present? + user_permitted_params = permitted_params(params) + raise PermissionError if user_permitted_params[:role] == "root" && !root_admin? + user.update_details(user_permitted_params) + end + user.update_roles(self, params) if can_update_user_role?(user) user.update_password(self, params) if can_update_password?(user) user @@ -99,7 +115,9 @@ def destroy_user(params) raise PermissionError unless can_delete_user? transaction do - User.find(params[:id]).organization_users.each(&:destroy!) + user_to_delete = User.find(params[:id]) + raise PermissionError unless can_delete_user?(user_to_delete) + user_to_delete.organization_users.each(&:destroy!) end end diff --git a/app/models/role.rb b/app/models/role.rb new file mode 100644 index 00000000..69bd4b6e --- /dev/null +++ b/app/models/role.rb @@ -0,0 +1,27 @@ +class Role + include Comparable + attr_reader :value + + def initialize(value) + @value = value + end + + def <=>(other) + to_i <=> other.to_i + end + + def to_i + case value + when "root" + 3 + when "admin" + 2 + when "report" + 1 + when "none" + 0 + else + raise "Unknown role value: #{value.inspect}" + end + end +end diff --git a/app/models/user.rb b/app/models/user.rb index a757ce7c..62068a53 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -59,6 +59,10 @@ def valid_password?(password) super && !deleted? end + def role_object + @role_object ||= Role.new(role) + end + protected def email_updated? diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 7e5842ee..af9c2396 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -56,14 +56,18 @@ <% end %> - <% if current_user.super_admin? %> + <% if current_user.super_edit_user_access?(@user) %>
@@ -81,7 +85,7 @@ <%= link_to "Reset Password", reset_password_user_path(@user), method: :post, class: "btn btn-danger", data: confirm(title: "Send password reset email for: #{@user.name}") %> <% end %> - <% if !@user.deleted? && current_user.can_delete_user? %> + <% if !@user.deleted? && current_user.can_delete_user?(@user) %> <%= link_to "Delete", user_path(@user), method: :delete, class: "btn btn-danger", data: confirm(title: "Deleting User: #{@user.name}") %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 9dc75347..2f72e49e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -62,6 +62,7 @@ en: admin: Admin user: admin: Super Admin + root: Root Admin purchase: confirm_cancel_dialog: >

Canceling a purchase order erases all purchase details, including any items that may have been added to the inventory.

diff --git a/spec/fixtures/users.yml b/spec/fixtures/users.yml index 113bb0de..2d49efb9 100644 --- a/spec/fixtures/users.yml +++ b/spec/fixtures/users.yml @@ -3,6 +3,13 @@ root: primary_number: (408) 555-1234 email: root@stockaid-domain.com encrypted_password: <%= User.new.send(:password_digest, "Sudopwd123") %> + role: root + +super_user: + name: Super User + primary_number: (408) 556-1234 + email: admin@stockaid-domain.com + encrypted_password: <%= User.new.send(:password_digest, "Sudopwd123") %> role: admin acme_root: diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b5d33ebe..79bf1140 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2,6 +2,7 @@ describe User, type: :model do let(:root) { users(:root) } + let(:super_user) { users(:super_user) } let(:acme_root) { users(:acme_root) } let(:acme_normal) { users(:acme_normal) } let(:foo_inc_root) { users(:foo_inc_root) } @@ -21,9 +22,20 @@ end end + describe "#root_admin?" do + it "tells if the user is a root admin for all of the site" do + expect(root).to be_a_root_admin + expect(super_user).to_not be_a_root_admin + + expect(acme_root).to_not be_a_root_admin + expect(acme_normal).to_not be_a_root_admin + end + end + describe "#super_admin?" do it "tells if the user is an admin for all of the site" do expect(root).to be_a_super_admin + expect(super_user).to be_a_super_admin expect(acme_root).to_not be_a_super_admin expect(acme_normal).to_not be_a_super_admin @@ -33,6 +45,7 @@ describe "#admin?" do it "tells if the user is an admin at any organization" do expect(root.admin?).to be_truthy + expect(super_user.admin?).to be_truthy expect(acme_root.admin?).to be_truthy expect(foo_inc_root.admin?).to be_truthy @@ -43,6 +56,7 @@ describe "#admin_at?" do it "tells if the user is an admin for a particular organization" do expect(root.admin_at?(acme)).to be_truthy + expect(super_user.admin_at?(acme)).to be_truthy expect(acme_root.admin_at?(acme)).to be_truthy expect(foo_inc_root.admin_at?(foo_inc)).to be_truthy @@ -60,9 +74,36 @@ expect(root.member_at?(acme)).to be_falsey expect(root.member_at?(foo_inc)).to be_falsey + expect(super_user.member_at?(acme)).to be_falsey + expect(super_user.member_at?(foo_inc)).to be_falsey expect(acme_root.member_at?(foo_inc)).to be_falsey expect(acme_normal.member_at?(foo_inc)).to be_falsey expect(foo_inc_root.member_at?(acme)).to be_falsey end end + + describe "#role_object" do + it "allows comparing roles" do + expect(root.role_object).to be > super_user.role_object + expect(super_user.role_object).to be < root.role_object + + expect(root.role_object).to be > acme_root.role_object + expect(acme_root.role_object).to be < root.role_object + + expect(super_user.role_object).to be > acme_root.role_object + expect(acme_root.role_object).to be < super_user.role_object + + expect(super_user.role_object).to be == super_user.role_object + expect(root.role_object).to be == root.role_object + + expect(super_user.role_object).to be <= super_user.role_object + expect(root.role_object).to be >= root.role_object + + expect(acme_root.role_object).to be == acme_root.role_object + expect(acme_root.role_object).to be == acme_normal.role_object + + expect(acme_root.role_object).to be <= acme_root.role_object + expect(acme_root.role_object).to be >= acme_normal.role_object + end + end end From a66a3b8eb7953339bcf0542aebe01daf9d2f0b0a Mon Sep 17 00:00:00 2001 From: Mike Virata-Stone Date: Tue, 2 Sep 2025 21:14:44 -0700 Subject: [PATCH 2/7] Prevent selecting external transfer unless root permission --- app/controllers/items_controller.rb | 2 ++ app/models/item.rb | 8 +++++--- app/views/items/edit_stock.html.erb | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb index 4aad8127..b1e5335c 100644 --- a/app/controllers/items_controller.rb +++ b/app/controllers/items_controller.rb @@ -39,6 +39,8 @@ def update # rubocop:disable Metrics/AbcSize item_params[:value]&.delete!(",") item_event_params = params.require(:item).permit(:edit_amount, :edit_method, :edit_reason, :edit_source) + raise PermissionError if item_event_params[:edit_reason] == "transfer_external" && !current_user.root_admin? + @item.assign_attributes item_params @item.mark_event item_event_params @item.update_bins!(params) diff --git a/app/models/item.rb b/app/models/item.rb index 3d413c89..5fa4b1f0 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -49,9 +49,11 @@ def self.for_category(category_id) end end - def self.selectable_edit_reasons - @selectable_edit_reasons ||= edit_reasons.reject do |x| - %w[donation donation_adjustment adjustment order_adjustment purchase reconciliation transfer].include?(x) + UNSELECTABLE_EDIT_REASONS = Set.new(%w[donation donation_adjustment adjustment order_adjustment purchase reconciliation transfer]) + + def self.selectable_edit_reasons(user) + edit_reasons.reject do |x| + UNSELECTABLE_EDIT_REASONS.include?(x) || (x == "transfer_external" && !user.root_admin?) end end diff --git a/app/views/items/edit_stock.html.erb b/app/views/items/edit_stock.html.erb index 0b7692f5..47c5e24d 100644 --- a/app/views/items/edit_stock.html.erb +++ b/app/views/items/edit_stock.html.erb @@ -55,7 +55,7 @@
<%= f.label 'Reason for changing quantity in stock:' %>
- <% Item.selectable_edit_reasons.each do |reason, index| %> + <% Item.selectable_edit_reasons(current_user).each do |reason, index| %> From 45c1c54bc8ddb5549e9ce256f68fd9064305e52c Mon Sep 17 00:00:00 2001 From: Mike Virata-Stone Date: Tue, 2 Sep 2025 22:21:00 -0700 Subject: [PATCH 3/7] Be able to set up notifications --- app/models/concerns/users/user_manipulator.rb | 10 ++++- app/models/notification.rb | 27 +++++++++++++ app/models/notification_subscription.rb | 3 ++ app/models/user.rb | 40 +++++++++++++++++++ app/views/users/edit.html.erb | 18 ++++++++- .../20250903041633_create_notifications.rb | 26 ++++++++++++ db/schema.rb | 33 ++++++++++++++- 7 files changed, 153 insertions(+), 4 deletions(-) create mode 100644 app/models/notification.rb create mode 100644 app/models/notification_subscription.rb create mode 100644 db/migrate/20250903041633_create_notifications.rb diff --git a/app/models/concerns/users/user_manipulator.rb b/app/models/concerns/users/user_manipulator.rb index e65c8489..e8fb99df 100644 --- a/app/models/concerns/users/user_manipulator.rb +++ b/app/models/concerns/users/user_manipulator.rb @@ -3,6 +3,10 @@ module UserManipulator extend ActiveSupport::Concern attr_reader :original_email + def can_subscribe_to_notifications? + root_admin? + end + def super_edit_user_access?(user) super_admin? && role_object >= user.role_object end @@ -84,7 +88,7 @@ def invite_user(params) invitation.invite_mailer(self).deliver_now end - def update_user(params) + def update_user(params) # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity user = transaction do user = User.find(params[:id]) raise PermissionError unless can_update_user?(user) @@ -95,6 +99,10 @@ def update_user(params) user.update_details(user_permitted_params) end + if can_subscribe_to_notifications? && user == self + user.update_subscriptions(params.require(:subscriptions).permit(Notification::SUBSCRIPTION_TYPES.keys)) + end + user.update_roles(self, params) if can_update_user_role?(user) user.update_password(self, params) if can_update_password?(user) user diff --git a/app/models/notification.rb b/app/models/notification.rb new file mode 100644 index 00000000..40802107 --- /dev/null +++ b/app/models/notification.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class Notification < ApplicationRecord + belongs_to :user + belongs_to :reference, polymorphic: true, optional: true + belongs_to :triggered_by_user, class_name: "User", optional: true + + # Notification types + SPOILAGE = "spoilage" + DELETED_DONATIONS = "deleted_donations" + DELETED_PURCHASES = "deleted_purchases" + + SUBSCRIPTION_TYPES = { + spoilage: { + type: SPOILAGE, + label: "Notify me when spoilages are added" + }.freeze, + deleted_donations: { + type: DELETED_DONATIONS, + label: "Notify me when donations are deleted" + }.freeze, + deleted_purchases: { + type: DELETED_PURCHASES, + label: "Notify me when purchases are deleted" + }.freeze + }.freeze +end diff --git a/app/models/notification_subscription.rb b/app/models/notification_subscription.rb new file mode 100644 index 00000000..0906d0d5 --- /dev/null +++ b/app/models/notification_subscription.rb @@ -0,0 +1,3 @@ +class NotificationSubscription < ApplicationRecord + belongs_to :user +end diff --git a/app/models/user.rb b/app/models/user.rb index 62068a53..41866be4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -6,6 +6,8 @@ class User < ApplicationRecord has_many :user_invitations, foreign_key: :invited_by_id has_many :orders, through: :organizations has_many :donations + has_many :notification_subscriptions + has_many :notifications validates :name, :primary_number, :email, presence: true validate :phone_numbers_are_different @@ -63,6 +65,13 @@ def role_object @role_object ||= Role.new(role) end + def subscribed_to?(type) + subscription = notification_subscriptions.find { |x| x.notification_type == type } + return false unless subscription + + subscription.enabled + end + protected def email_updated? @@ -79,6 +88,37 @@ def update_details(details) update! details end + def update_subscriptions(subscription_params) + Notification::SUBSCRIPTION_TYPES.each do |sub_type, sub| + if subscription_params[sub_type] == "true" + subscribe!(sub[:type]) + elsif subscription_params[sub_type] == "false" + unsubscribe!(sub[:type]) + end + end + end + + def subscribe!(type) + return if subscribed_to?(type) + + subscription = notification_subscriptions.find { |x| x.notification_type == type } + + if subscription + subscription.enabled = true + subscription.save! + else + notification_subscriptions.create!(notification_type: type, enabled: true) + end + end + + def unsubscribe!(type) + return unless subscribed_to?(type) + + subscription = notification_subscriptions.find { |x| x.notification_type == type } + subscription.enabled = false + subscription.save! + end + def update_roles(updater, params) return unless params[:roles] diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index af9c2396..1d7f666b 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -63,10 +63,10 @@
@@ -74,6 +74,20 @@ <% end %> <% end %> + + <% if current_user.can_subscribe_to_notifications? && current_user == @user %> + <% Notification::SUBSCRIPTION_TYPES.each do |sub_type, sub| %> +
+ +
+ <% end %> + +
+ <% end %>
diff --git a/db/migrate/20250903041633_create_notifications.rb b/db/migrate/20250903041633_create_notifications.rb new file mode 100644 index 00000000..1cee71c3 --- /dev/null +++ b/db/migrate/20250903041633_create_notifications.rb @@ -0,0 +1,26 @@ +class CreateNotifications < ActiveRecord::Migration[6.1] + def change + create_table :notification_subscriptions do |t| + t.references :user, null: false, foreign_key: true, index: true + t.string :notification_type, null: false + t.boolean :enabled, null: false + + t.timestamps null: false + t.index %i[user_id notification_type], unique: true, name: "idx_notif_subs_on_uid_notif_type" + t.index %i[user_id notification_type enabled], name: "idx_notif_subs_on_uid_notif_type_enabled" + end + + create_table :notifications do |t| + t.references :user, null: false, foreign_key: true + t.references :triggered_by_user, foreign_key: { to_table: :users } + t.references :reference, polymorphic: true + t.string :title, null: false + t.text :message, null: false + t.datetime :completed_at + + t.timestamps null: false + t.index %i[user_id created_at] + t.index %i[user_id completed_at] + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 14c009fa..4a9f0d4d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2025_03_31_000215) do +ActiveRecord::Schema.define(version: 2025_09_03_041633) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -220,6 +220,34 @@ t.index ["sku"], name: "index_items_on_sku", unique: true end + create_table "notification_subscriptions", force: :cascade do |t| + t.bigint "user_id", null: false + t.string "notification_type", null: false + t.boolean "enabled", null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["user_id", "notification_type", "enabled"], name: "idx_notif_subs_on_uid_notif_type_enabled" + t.index ["user_id", "notification_type"], name: "idx_notif_subs_on_uid_notif_type", unique: true + t.index ["user_id"], name: "index_notification_subscriptions_on_user_id" + end + + create_table "notifications", force: :cascade do |t| + t.bigint "user_id", null: false + t.bigint "triggered_by_user_id" + t.string "reference_type" + t.bigint "reference_id" + t.string "title", null: false + t.text "message", null: false + t.datetime "completed_at" + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["reference_type", "reference_id"], name: "index_notifications_on_reference" + t.index ["triggered_by_user_id"], name: "index_notifications_on_triggered_by_user_id" + t.index ["user_id", "completed_at"], name: "index_notifications_on_user_id_and_completed_at" + t.index ["user_id", "created_at"], name: "index_notifications_on_user_id_and_created_at" + t.index ["user_id"], name: "index_notifications_on_user_id" + end + create_table "order_details", id: :serial, force: :cascade do |t| t.integer "order_id", null: false t.integer "quantity", null: false @@ -598,6 +626,9 @@ add_foreign_key "item_program_ratio_values", "item_program_ratios" add_foreign_key "item_program_ratio_values", "programs" add_foreign_key "items", "item_program_ratios" + add_foreign_key "notification_subscriptions", "users" + add_foreign_key "notifications", "users" + add_foreign_key "notifications", "users", column: "triggered_by_user_id" add_foreign_key "order_details", "items" add_foreign_key "organization_addresses", "addresses" add_foreign_key "organization_addresses", "organizations" From f1bdbaaed3ef430d340315b21f2f4c829310a6a3 Mon Sep 17 00:00:00 2001 From: Mike Virata-Stone Date: Tue, 2 Sep 2025 23:39:32 -0700 Subject: [PATCH 4/7] Notifications for spoilage --- app/controllers/items_controller.rb | 1 + app/controllers/notifications_controller.rb | 24 +++++++++++ app/helpers/application_helper.rb | 9 ++++ app/helpers/notifications_helper.rb | 11 +++++ app/models/concerns/users/user_manipulator.rb | 4 +- app/models/notification.rb | 37 ++++++++++++++++ app/models/user.rb | 4 ++ app/views/common/_navbar.html.erb | 10 +++++ app/views/notifications/index.html.erb | 28 ++++++++++++ app/views/notifications/show.html.erb | 43 +++++++++++++++++++ config/locales/en.yml | 1 + config/routes.rb | 1 + .../20250903041633_create_notifications.rb | 1 + db/schema.rb | 1 + 14 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 app/controllers/notifications_controller.rb create mode 100644 app/helpers/notifications_helper.rb create mode 100644 app/views/notifications/index.html.erb create mode 100644 app/views/notifications/show.html.erb diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb index b1e5335c..dfedd2df 100644 --- a/app/controllers/items_controller.rb +++ b/app/controllers/items_controller.rb @@ -47,6 +47,7 @@ def update # rubocop:disable Metrics/AbcSize if @item.save flash[:success] = "'#{@item.description}' updated" + Notification.notify_spoilage(current_user, @item, item_event_params) redirect_to items_path(category_id: @item.category.id) else redirect_to :back, alert: @item.errors.full_messages.to_sentence diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb new file mode 100644 index 00000000..d4860124 --- /dev/null +++ b/app/controllers/notifications_controller.rb @@ -0,0 +1,24 @@ +class NotificationsController < ApplicationController + require_permission :can_subscribe_to_notifications? + + def index + end + + def show + @notification = current_user.notifications.find(params[:id]) + end + + def update + @notification = current_user.notifications.find(params[:id]) + + if params[:mark_read] == "true" + @notification.update!(completed_at: Time.zone.now) unless @notification.read? + redirect_to notifications_path, flash: { success: "Marked message as read" } + elsif params[:mark_unread] == "true" + @notification.update!(completed_at: nil) unless @notification.unread? + redirect_to notifications_path, flash: { success: "Marked message as unread" } + else + raise "Update method undefined!" + end + end +end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index c676ff2b..6b4816a7 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -78,6 +78,15 @@ def external_link(object, prefix: nil) end end + def notifications_badge + return unless current_user.can_subscribe_to_notifications? + + @notifications_badge_count ||= current_user.unread_notification_count + return unless @notifications_badge_count.positive? + + tag.span(class: "badge") { @notifications_badge_count.to_s } + end + def showing_tab?(tab_id) params[:show_tab] == tab_id || @show_tab == tab_id end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb new file mode 100644 index 00000000..81ea65b8 --- /dev/null +++ b/app/helpers/notifications_helper.rb @@ -0,0 +1,11 @@ +module NotificationsHelper + def notification_reference_link(notification) + return unless @notification.reference + + if @notification.reference.is_a?(Item) + link_to @notification.reference.description, edit_stock_item_path(@notification.reference) + else + "Reference link unable to be determined" + end + end +end diff --git a/app/models/concerns/users/user_manipulator.rb b/app/models/concerns/users/user_manipulator.rb index e8fb99df..a6b65ed4 100644 --- a/app/models/concerns/users/user_manipulator.rb +++ b/app/models/concerns/users/user_manipulator.rb @@ -3,7 +3,9 @@ module UserManipulator extend ActiveSupport::Concern attr_reader :original_email - def can_subscribe_to_notifications? + def can_subscribe_to_notifications?(type = nil) + # nil type means general notification subscription check, explicit type + # asks if can subscribe to that type. Right now, they are the same thing. root_admin? end diff --git a/app/models/notification.rb b/app/models/notification.rb index 40802107..bf3adbfd 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -24,4 +24,41 @@ class Notification < ApplicationRecord label: "Notify me when purchases are deleted" }.freeze }.freeze + + def self.notify_spoilage(current_user, item, params) + return unless params[:edit_amount] && params[:edit_method] && params[:edit_reason] + return unless params[:edit_reason] == "spoilage" + + notify!(Notification::SPOILAGE, title: "Spoilage for item #{item.description}", message: <<~MESSAGE, triggered_by_user: current_user, reference: item) + Spoilage update for item ##{item.id} (#{item.category.description} - #{item.description}): + + Stock #{params[:edit_method]} by #{params[:edit_amount]}. + + Reason: #{params[:edit_source]} + MESSAGE + end + + def self.notify!(type, title:, message:, triggered_by_user: nil, reference: nil) + caught_error = nil + + NotificationSubscription.includes(:user).where(notification_type: type, enabled: true).find_each do |subscription| + begin + next unless subscription.user.can_subscribe_to_notifications?(type) + + create!(title: title, message: message, user: subscription.user, triggered_by_user: triggered_by_user, reference: reference) + rescue StandardError => e + caught_error = e + end + end + + raise caught_error if caught_error + end + + def unread? + completed_at.blank? + end + + def read? + completed_at.present? + end end diff --git a/app/models/user.rb b/app/models/user.rb index 41866be4..f7627e07 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -72,6 +72,10 @@ def subscribed_to?(type) subscription.enabled end + def unread_notification_count + notifications.where(completed_at: nil).count + end + protected def email_updated? diff --git a/app/views/common/_navbar.html.erb b/app/views/common/_navbar.html.erb index 99d694a2..c63431eb 100644 --- a/app/views/common/_navbar.html.erb +++ b/app/views/common/_navbar.html.erb @@ -117,6 +117,7 @@
  • @@ -146,6 +147,15 @@
    • <% end %> + <% if current_user.can_subscribe_to_notifications? %> +
  • + <%= link_to notifications_path do %> + <%= notifications_badge %> Notifications + <% end %> +
    • + + <% end %> +
  • <%= link_to "Edit User", edit_user_path(current_user) %>
  • <%= link_to "Logout", destroy_user_session_path, method: :delete %>
    • diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb new file mode 100644 index 00000000..2c225449 --- /dev/null +++ b/app/views/notifications/index.html.erb @@ -0,0 +1,28 @@ +<% content_for :title, "Notifications" %> + +<% content_for :content do %> + + + + + + + + + + + <% current_user.notifications.order(created_at: :desc).each do |notification| %> + + + + + + + <% end %> + +
    TitleDate
    + <% if notification.unread? %> + + <% end %> + <%= link_to notification.title, notification_path(notification) %><%= local_time_ago notification.created_at %>
    +<% end %> diff --git a/app/views/notifications/show.html.erb b/app/views/notifications/show.html.erb new file mode 100644 index 00000000..edd3e40f --- /dev/null +++ b/app/views/notifications/show.html.erb @@ -0,0 +1,43 @@ +<% content_for :title, "Notification #{@notification.id}" %> + +<% content_for :content do %> +

    + <%= @notification.title %> +

    + +

    + Triggered at: <%= local_time @notification.created_at %> +

    + + <% if @notification.triggered_by_user %> +

    + Triggered by user: <%= @notification.triggered_by_user.name %> +

    + <% end %> + +
    + + <%= simple_format @notification.message %> + + <% if @notification.reference %> +
    + + <%= notification_reference_link @notification %> + <% end %> + +
    + + <%= form_tag notification_path(@notification), method: :patch do %> + <% if @notification.unread? %> + + <%= link_to "Cancel", notifications_path, class: "btn btn-default" %> + <% else %> +

    + Read at: <%= local_time @notification.completed_at %> +

    + + + <%= link_to "Cancel", notifications_path, class: "btn btn-default" %> + <% end %> + <% end %> +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 2f72e49e..3d38819a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -47,6 +47,7 @@ en: purchase_shipment_received: "Shipment recieved" purchase_shipment_deleted: "Shipment deleted" reconciliation: "Inventory reconciliation" + spoilage: "Spoilage recorded" transfer: "Legacy transferred items" transfer_external: "Externally transferred items" transfer_internal: "Internally transferred items" diff --git a/config/routes.rb b/config/routes.rb index ac6475d4..d76ea08b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -88,6 +88,7 @@ end resources :net_suite_errors, only: %i[index show destroy] + resources :notifications, only: %i[index show update] resources :orders, only: %i[index new create edit update] do collection do diff --git a/db/migrate/20250903041633_create_notifications.rb b/db/migrate/20250903041633_create_notifications.rb index 1cee71c3..7788b34d 100644 --- a/db/migrate/20250903041633_create_notifications.rb +++ b/db/migrate/20250903041633_create_notifications.rb @@ -7,6 +7,7 @@ def change t.timestamps null: false t.index %i[user_id notification_type], unique: true, name: "idx_notif_subs_on_uid_notif_type" + t.index %i[notification_type enabled], name: "idx_notif_subs_on_notif_type_enabled" t.index %i[user_id notification_type enabled], name: "idx_notif_subs_on_uid_notif_type_enabled" end diff --git a/db/schema.rb b/db/schema.rb index 4a9f0d4d..f62c0b30 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -226,6 +226,7 @@ t.boolean "enabled", null: false t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false + t.index ["notification_type", "enabled"], name: "idx_notif_subs_on_notif_type_enabled" t.index ["user_id", "notification_type", "enabled"], name: "idx_notif_subs_on_uid_notif_type_enabled" t.index ["user_id", "notification_type"], name: "idx_notif_subs_on_uid_notif_type", unique: true t.index ["user_id"], name: "index_notification_subscriptions_on_user_id" From 9f033db7efab9ca9dbe9f4f426bcd75578897ba1 Mon Sep 17 00:00:00 2001 From: Mike Virata-Stone Date: Tue, 2 Sep 2025 23:44:47 -0700 Subject: [PATCH 5/7] Address RuboCop issues --- .rubocop.yml | 3 +++ app/helpers/application_helper.rb | 2 +- app/helpers/notifications_helper.rb | 6 +++--- app/models/concerns/users/user_manipulator.rb | 2 +- app/models/notification.rb | 12 +++++------- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 744039dd..3531c4e4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -52,6 +52,9 @@ Naming/VariableNumber: Style/Documentation: Enabled: false +Style/EmptyMethod: + Enabled: false + Style/FetchEnvVar: Enabled: false diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6b4816a7..91aa6360 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -82,7 +82,7 @@ def notifications_badge return unless current_user.can_subscribe_to_notifications? @notifications_badge_count ||= current_user.unread_notification_count - return unless @notifications_badge_count.positive? + return unless @notifications_badge_count > 0 tag.span(class: "badge") { @notifications_badge_count.to_s } end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 81ea65b8..a7e97328 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,9 +1,9 @@ module NotificationsHelper def notification_reference_link(notification) - return unless @notification.reference + return unless notification.reference - if @notification.reference.is_a?(Item) - link_to @notification.reference.description, edit_stock_item_path(@notification.reference) + if notification.reference.is_a?(Item) + link_to notification.reference.description, edit_stock_item_path(notification.reference) else "Reference link unable to be determined" end diff --git a/app/models/concerns/users/user_manipulator.rb b/app/models/concerns/users/user_manipulator.rb index a6b65ed4..77978887 100644 --- a/app/models/concerns/users/user_manipulator.rb +++ b/app/models/concerns/users/user_manipulator.rb @@ -3,7 +3,7 @@ module UserManipulator extend ActiveSupport::Concern attr_reader :original_email - def can_subscribe_to_notifications?(type = nil) + def can_subscribe_to_notifications?(_type = nil) # nil type means general notification subscription check, explicit type # asks if can subscribe to that type. Right now, they are the same thing. root_admin? diff --git a/app/models/notification.rb b/app/models/notification.rb index bf3adbfd..6f1acf2d 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -29,7 +29,7 @@ def self.notify_spoilage(current_user, item, params) return unless params[:edit_amount] && params[:edit_method] && params[:edit_reason] return unless params[:edit_reason] == "spoilage" - notify!(Notification::SPOILAGE, title: "Spoilage for item #{item.description}", message: <<~MESSAGE, triggered_by_user: current_user, reference: item) + notify!(Notification::SPOILAGE, title: "Spoilage for item #{item.description}", message: <<~MESSAGE, triggered_by_user: current_user, reference: item) # rubocop:disable Layout/LineLength Spoilage update for item ##{item.id} (#{item.category.description} - #{item.description}): Stock #{params[:edit_method]} by #{params[:edit_amount]}. @@ -42,13 +42,11 @@ def self.notify!(type, title:, message:, triggered_by_user: nil, reference: nil) caught_error = nil NotificationSubscription.includes(:user).where(notification_type: type, enabled: true).find_each do |subscription| - begin - next unless subscription.user.can_subscribe_to_notifications?(type) + next unless subscription.user.can_subscribe_to_notifications?(type) - create!(title: title, message: message, user: subscription.user, triggered_by_user: triggered_by_user, reference: reference) - rescue StandardError => e - caught_error = e - end + create!(title: title, message: message, user: subscription.user, triggered_by_user: triggered_by_user, reference: reference) + rescue StandardError => e + caught_error = e end raise caught_error if caught_error From cda6d6132bf62eddf2e2282036bdf94de5839060 Mon Sep 17 00:00:00 2001 From: Mike Virata-Stone Date: Wed, 3 Sep 2025 00:05:15 -0700 Subject: [PATCH 6/7] Notify on deleted donations --- app/controllers/donations_controller.rb | 10 +++++++++- app/helpers/notifications_helper.rb | 5 ++++- app/models/donation.rb | 1 + app/models/notification.rb | 6 ++++++ app/views/donations/show.html.erb | 4 +++- 5 files changed, 23 insertions(+), 3 deletions(-) diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index 9a902613..f9dc0a39 100644 --- a/app/controllers/donations_controller.rb +++ b/app/controllers/donations_controller.rb @@ -36,7 +36,13 @@ def create end def show - @donation = Donation.active_with_includes.find(params[:id]) + @donation = + if params[:allow_deleted] == "true" + Donation.with_includes.find(params[:id]) + else + Donation.active_with_includes.find(params[:id]) + end + redirect_to donations_path unless current_user.can_view_donation?(@donation) end @@ -67,6 +73,7 @@ def destroy begin @donation.soft_delete + Notification.notify_deleted_donation(current_user, @donation) flash[:success] = "Donation '#{@donation.id}' deleted!" rescue DeletionError => e flash[:error] = e.message @@ -83,6 +90,7 @@ def destroy def destroy_closed donation = Donation.find(params[:id]) donation.soft_delete_closed + Notification.notify_deleted_donation(current_user, donation) flash[:success] = "Closed donation '#{donation.id}' deleted!" redirect_to closed_donations_path end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index a7e97328..8a516dd3 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -2,7 +2,10 @@ module NotificationsHelper def notification_reference_link(notification) return unless notification.reference - if notification.reference.is_a?(Item) + case notification.reference + when Donation + link_to "Donation #{notification.reference.id}", donation_path(notification.reference, allow_deleted: true) + when Item link_to notification.reference.description, edit_stock_item_path(notification.reference) else "Reference link unable to be determined" diff --git a/app/models/donation.rb b/app/models/donation.rb index 7885f96b..92802537 100644 --- a/app/models/donation.rb +++ b/app/models/donation.rb @@ -14,6 +14,7 @@ class Donation < ApplicationRecord validate :not_changing_after_closed + scope :with_includes, -> { includes(:county, :user, donor: :addresses, donation_details: { item: :category }) } scope :active_with_includes, -> { active.includes(:county, :user, donor: :addresses, donation_details: { item: :category }) } scope :closed_with_includes, -> { closed.includes(:county, :user, donor: :addresses, donation_details: { item: :category }) } scope :deleted_with_includes, -> { deleted.includes(:county, :user, donor: :addresses, donation_details: { item: :category }) } diff --git a/app/models/notification.rb b/app/models/notification.rb index 6f1acf2d..51e33d57 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -38,6 +38,12 @@ def self.notify_spoilage(current_user, item, params) MESSAGE end + def self.notify_deleted_donation(current_user, donation) + notify!(Notification::DELETED_DONATIONS, title: "Donation deleted", message: <<~MESSAGE, triggered_by_user: current_user, reference: donation) + Donation ##{donation.id} was deleted. + MESSAGE + end + def self.notify!(type, title:, message:, triggered_by_user: nil, reference: nil) caught_error = nil diff --git a/app/views/donations/show.html.erb b/app/views/donations/show.html.erb index 629994c0..bc853c45 100644 --- a/app/views/donations/show.html.erb +++ b/app/views/donations/show.html.erb @@ -27,7 +27,9 @@
    - <% if @donation.closed? %> + <% if @donation.soft_deleted? %> + This donation is deleted. + <% elsif @donation.closed? %> <% if current_user.can_sync_donation?(@donation) %> <%= sync_donation_button(@donation) %> <% end %> From 0c54f83c695d90ee452ec9b7194a054dff9882e3 Mon Sep 17 00:00:00 2001 From: Mike Virata-Stone Date: Wed, 3 Sep 2025 00:07:31 -0700 Subject: [PATCH 7/7] Add unused notification for purchases --- app/models/notification.rb | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/app/models/notification.rb b/app/models/notification.rb index 51e33d57..be96b300 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -25,6 +25,19 @@ class Notification < ApplicationRecord }.freeze }.freeze + def self.notify_deleted_donation(current_user, donation) + notify!(Notification::DELETED_DONATIONS, title: "Donation deleted", message: <<~MESSAGE, triggered_by_user: current_user, reference: donation) + Donation ##{donation.id} was deleted. + MESSAGE + end + + # TODO: Purchases can only be canceled... do we notify on this or drop this notification? + def self.notify_deleted_purchase(current_user, purchase) + notify!(Notification::DELETED_PURCHASES, title: "Purchase deleted", message: <<~MESSAGE, triggered_by_user: current_user, reference: purchase) + Purchase ##{purchase.id} was deleted. + MESSAGE + end + def self.notify_spoilage(current_user, item, params) return unless params[:edit_amount] && params[:edit_method] && params[:edit_reason] return unless params[:edit_reason] == "spoilage" @@ -38,12 +51,6 @@ def self.notify_spoilage(current_user, item, params) MESSAGE end - def self.notify_deleted_donation(current_user, donation) - notify!(Notification::DELETED_DONATIONS, title: "Donation deleted", message: <<~MESSAGE, triggered_by_user: current_user, reference: donation) - Donation ##{donation.id} was deleted. - MESSAGE - end - def self.notify!(type, title:, message:, triggered_by_user: nil, reference: nil) caught_error = nil