Notification read and dismiss raise under Mongoid 9 #908

Marking a notification as read and dismissing one both raise on every request on master. This is a regression from the Mongoid 8 → 9 upgrade in #892, so it is live now rather than something waiting on the rest of the upgrade.

Symptom

PUT and DELETE on /api/notifications raise:

Mongoid::Errors::InvalidQuery:
  Expression must be a Hash: #<ActionController::Parameters {"notificateable_id"=>"...", "notificateable_type"=>"Post", ...}>
    mongoid-9.0.11/lib/mongoid/criteria/queryable/selectable.rb:857:in `expr_query'

In production ExceptionLogger's rescue_from "Exception" catches it, so the client gets a 422 quoting a Mongoid internal error rather than a 500 — which is probably why it has read as noise rather than an outage.

Cause

Api::V1::NotificationsController#notification_params returns ActionController::Parameters:

def notification_params
  parameters = params.permit(:notificateable_id, :notificateable_type)
  parameters[:notificateable_type] = parameters[:notificateable_type].titleize
  parameters[:encrypted_notify_user_id] = current_user.encrypted_id
  parameters
end

and both actions feed that straight into a query:

notifications = Notification.where(notification_params)

ActionController::Parameters has not been a Hash since Rails 5. Mongoid 8 accepted it anyway; Mongoid 9 requires a query expression to be a real Hash and raises. index is unaffected because it builds its own criteria.

Scope

I audited app/ and lib/ for the same shape. This is the only Mongoid case. Everything else is either Active Record or Active Model, which still accept permitted Parameters, or a plain Hash built by hand — ReactionsController#reaction_params is the latter and is fine.

Why it was not caught

NotificationsController had no spec. Adding one fails immediately on both actions, which is how this was found.

Two N+1s sit in the same controller and are worth fixing alongside it, because Bullet.raise = true in the test environment means they make the endpoint untestable — any spec touching index dies on UnoptimizedQueryError before it can assert anything:

• Notification's after_initialize :set_defaults recomputes post_id by reaching through notificateable on every instantiation, including records read back from the database that already have the value stored. That is one extra query per notification loaded, anywhere in the app.
• The index aggregation titles each group through notificateable without eager loading it.

Fix

PR #909. to_h on the permitted parameters, guard set_defaults on post_id.blank? so it still backfills documents written before the field existed, and eager load notificateable on the index query.

🤖 Generated with Claude Code