User deleted email is sent even when deletion failed

Bug
1

While I was testing user deletion from the review queue to check what happens when a user clicks the guideline link of my login-required forum, something strange happened: The user received 2 emails that their account was deleted. In fact, the first one was a false alarm, so to speak. The user was not deleted, and still an email was sent.

I am not sure all of the following steps are really needed, but it was difficult to reproduce, and I am happy I found steps at all and didn’t want to try if other flag types or other ways of handling the flags work too.

Preparation

We need a custom flag that requires additional input from the user. I also enabled auto-hiding - no idea if that matters. Then we need the user we will delete (spammer), an admin user, and another user.

Steps to reproduce

  1. Spammer creates a topic
  2. Spammer flags their own post as spam
    image
    image691×255 6.74 KB
  3. The other user flags the post using “something else”
  4. Admin deletes the topic
    image
    image995×270 15.9 KB
  5. Admin restores the topic
  6. Admin flags the post using the custom flag type
    image
    image1124×845 35.4 KB

    (In this case the topic was hidden when I flagged it, not sure if that’s relevant)
  7. In the review queue, the admin selects “yes → delete user”

Results

  • The user was not deleted
  • But an email was sent:
    image
    image623×154 4.5 KB
  • There is no entry in the email logs, even though an email was sent
  • There is an error in the browser console: 201/perform/delete_user?version=1:1 Failed to load resource: the server responded with a status of 500 ()
  • There is an error in /logs: NoMethodError (undefined method score’ for nil)`
error 
NoMethodError (undefined method `score' for nil)
app/models/reviewable.rb:732:in `recalculate_score'
app/models/reviewable.rb:375:in `block in perform'
app/models/reviewable.rb:366:in `perform'
app/controllers/reviewables_controller.rb:273:in `perform'
app/controllers/application_controller.rb:428:in `block in with_resolved_locale'
app/controllers/application_controller.rb:428:in `with_resolved_locale'
lib/middleware/omniauth_bypass_middleware.rb:35:in `call'
lib/content_security_policy/middleware.rb:12:in `call'
lib/middleware/anonymous_cache.rb:415:in `call'
lib/middleware/csp_script_nonce_injector.rb:12:in `call'
config/initializers/008-rack-cors.rb:26:in `call'
lib/middleware/default_headers.rb:13:in `call'
config/initializers/100-quiet_logger.rb:20:in `call'
config/initializers/100-silence_logger.rb:29:in `call'
lib/middleware/enforce_hostname.rb:24:in `call'
lib/middleware/processing_request.rb:12:in `call'
lib/middleware/request_tracker.rb:385:in `call'


Backtrace

app/models/reviewable.rb:732:in `recalculate_score'
app/models/reviewable.rb:375:in `block in perform'
activerecord (7.2.2.1) lib/active_record/connection_adapters/abstract/transaction.rb:616:in `block in within_new_transaction'
activesupport (7.2.2.1) lib/active_support/concurrency/null_lock.rb:9:in `synchronize'
activerecord (7.2.2.1) lib/active_record/connection_adapters/abstract/transaction.rb:613:in `within_new_transaction'
activerecord (7.2.2.1) lib/active_record/connection_adapters/abstract/database_statements.rb:361:in `transaction'
activerecord (7.2.2.1) lib/active_record/transactions.rb:234:in `block in transaction'
activerecord (7.2.2.1) lib/active_record/connection_adapters/abstract/connection_pool.rb:415:in `with_connection'
activerecord (7.2.2.1) lib/active_record/connection_handling.rb:296:in `with_connection'
activerecord (7.2.2.1) lib/active_record/transactions.rb:233:in `transaction'
app/models/reviewable.rb:366:in `perform'
app/controllers/reviewables_controller.rb:273:in `perform'
actionpack (7.2.2.1) lib/action_controller/metal/basic_implicit_render.rb:8:in `send_action'
actionpack (7.2.2.1) lib/abstract_controller/base.rb:226:in `process_action'
actionpack (7.2.2.1) lib/action_controller/metal/rendering.rb:193:in `process_action'
actionpack (7.2.2.1) lib/abstract_controller/callbacks.rb:261:in `block in process_action'
activesupport (7.2.2.1) lib/active_support/callbacks.rb:121:in `block in run_callbacks'
app/controllers/application_controller.rb:428:in `block in with_resolved_locale'
i18n (1.14.7) lib/i18n.rb:353:in `with_locale'
app/controllers/application_controller.rb:428:in `with_resolved_locale'
activesupport (7.2.2.1) lib/active_support/callbacks.rb:130:in `block in run_callbacks'
activesupport (7.2.2.1) lib/active_support/callbacks.rb:141:in `run_callbacks'
actionpack (7.2.2.1) lib/abstract_controller/callbacks.rb:260:in `process_action'
actionpack (7.2.2.1) lib/action_controller/metal/rescue.rb:27:in `process_action'
actionpack (7.2.2.1) lib/action_controller/metal/instrumentation.rb:77:in `block in process_action'
activesupport (7.2.2.1) lib/active_support/notifications.rb:210:in `block in instrument'
activesupport (7.2.2.1) lib/active_support/notifications/instrumenter.rb:58:in `instrument'
activesupport (7.2.2.1) lib/active_support/notifications.rb:210:in `instrument'
actionpack (7.2.2.1) lib/action_controller/metal/instrumentation.rb:76:in `process_action'
actionpack (7.2.2.1) lib/action_controller/metal/params_wrapper.rb:259:in `process_action'
activerecord (7.2.2.1) lib/active_record/railties/controller_runtime.rb:39:in `process_action'
actionpack (7.2.2.1) lib/abstract_controller/base.rb:163:in `process'
actionview (7.2.2.1) lib/action_view/rendering.rb:40:in `process'
rack-mini-profiler (3.3.1) lib/mini_profiler/profiling_methods.rb:89:in `block in profile_method'
actionpack (7.2.2.1) lib/action_controller/metal.rb:252:in `dispatch'
actionpack (7.2.2.1) lib/action_controller/metal.rb:335:in `dispatch'
actionpack (7.2.2.1) lib/action_dispatch/routing/route_set.rb:67:in `dispatch'
actionpack (7.2.2.1) lib/action_dispatch/routing/route_set.rb:50:in `serve'
actionpack (7.2.2.1) lib/action_dispatch/journey/router.rb:53:in `block in serve'
actionpack (7.2.2.1) lib/action_dispatch/journey/router.rb:133:in `block in find_routes'
actionpack (7.2.2.1) lib/action_dispatch/journey/router.rb:126:in `each'
actionpack (7.2.2.1) lib/action_dispatch/journey/router.rb:126:in `find_routes'
actionpack (7.2.2.1) lib/action_dispatch/journey/router.rb:34:in `serve'
actionpack (7.2.2.1) lib/action_dispatch/routing/route_set.rb:896:in `call'
lib/middleware/omniauth_bypass_middleware.rb:35:in `call'
rack (2.2.13) lib/rack/tempfile_reaper.rb:15:in `call'
rack (2.2.13) lib/rack/conditional_get.rb:40:in `call'
rack (2.2.13) lib/rack/head.rb:12:in `call'
actionpack (7.2.2.1) lib/action_dispatch/http/permissions_policy.rb:38:in `call'
lib/content_security_policy/middleware.rb:12:in `call'
lib/middleware/anonymous_cache.rb:415:in `call'
lib/middleware/csp_script_nonce_injector.rb:12:in `call'
config/initializers/008-rack-cors.rb:26:in `call'
rack (2.2.13) lib/rack/session/abstract/id.rb:266:in `context'
rack (2.2.13) lib/rack/session/abstract/id.rb:260:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/cookies.rb:704:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/callbacks.rb:31:in `block in call'
activesupport (7.2.2.1) lib/active_support/callbacks.rb:101:in `run_callbacks'
actionpack (7.2.2.1) lib/action_dispatch/middleware/callbacks.rb:30:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/debug_exceptions.rb:31:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/show_exceptions.rb:32:in `call'
logster (2.20.1) lib/logster/middleware/reporter.rb:40:in `call'
lib/middleware/default_headers.rb:13:in `call'
lograge (0.14.0) lib/lograge/rails_ext/rack/logger.rb:18:in `call_app'
railties (7.2.2.1) lib/rails/rack/logger.rb:29:in `call'
config/initializers/100-quiet_logger.rb:20:in `call'
config/initializers/100-silence_logger.rb:29:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/request_id.rb:33:in `call'
lib/middleware/enforce_hostname.rb:24:in `call'
rack (2.2.13) lib/rack/method_override.rb:24:in `call'
rack (2.2.13) lib/rack/sendfile.rb:110:in `call'
plugins/discourse-prometheus/lib/middleware/metrics.rb:14:in `call'
rack-mini-profiler (3.3.1) lib/mini_profiler.rb:191:in `call'
lib/middleware/processing_request.rb:12:in `call'
message_bus (4.4.1) lib/message_bus/rack/middleware.rb:60:in `call'
lib/middleware/request_tracker.rb:385:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/remote_ip.rb:96:in `call'
rails_failover (2.2.0) lib/rails_failover/active_record/middleware.rb:67:in `block in call'
activerecord (7.2.2.1) lib/active_record/connection_handling.rb:376:in `with_role_and_shard'
activerecord (7.2.2.1) lib/active_record/connection_handling.rb:147:in `connected_to'
rails_failover (2.2.0) lib/rails_failover/active_record/middleware.rb:64:in `call'
rails_multisite (6.1.0) lib/rails_multisite/middleware.rb:26:in `call'
railties (7.2.2.1) lib/rails/engine.rb:535:in `call'
railties (7.2.2.1) lib/rails/railtie.rb:226:in `public_send'
railties (7.2.2.1) lib/rails/railtie.rb:226:in `method_missing'
rack (2.2.13) lib/rack/urlmap.rb:74:in `block in call'
rack (2.2.13) lib/rack/urlmap.rb:58:in `each'
rack (2.2.13) lib/rack/urlmap.rb:58:in `call'
unicorn (6.1.0) lib/unicorn/http_server.rb:634:in `process_client'
unicorn (6.1.0) lib/unicorn/http_server.rb:739:in `worker_loop'
unicorn (6.1.0) lib/unicorn/http_server.rb:547:in `spawn_missing_workers'
unicorn (6.1.0) lib/unicorn/http_server.rb:143:in `start'
unicorn (6.1.0) bin/unicorn:128:in `<top (required)>'
vendor/bundle/ruby/3.3.0/bin/unicorn:25:in `load'
vendor/bundle/ruby/3.3.0/bin/unicorn:25:in `<main>'

Side note:

2 Likes