`NoMethodError in TopicsController#show` error after merging topics

[blu3id (Mike)]

Priority/Severity: Low Priority / High Severity

Platform: 3.5.0.beta8-dev with Discourse Calendar (and Event) - 0.5 0162ed5

Description:

After merging topics and deleting a merged post the target topic becomes inaccessible due to server error ([NoMethodError: undefined method 'category_id' for nil).

It is expected that the topic will function normally and not cause a server error.

Full trace

plugins/discourse-calendar/app/serializers/discourse_post_event/event_serializer.rb:137:in `category_id'
(eval at /home/discourse/.bundle/gems/ruby/3.3.0/gems/active_model_serializers-0.8.4/lib/active_model/serializer.rb:467):5:in `_fast_attributes'
active_model_serializers (0.8.4) lib/active_model/serializer.rb:468:in `rescue in attributes'
active_model_serializers (0.8.4) lib/active_model/serializer.rb:455:in `attributes'
active_model_serializers (0.8.4) lib/active_model/serializer.rb:480:in `_serializable_hash'
active_model_serializers (0.8.4) lib/active_model/serializer.rb:359:in `serializable_hash'
active_model_serializers (0.8.4) lib/active_model/serializer.rb:347:in `as_json'
activesupport (7.2.2.1) lib/active_support/json/encoding.rb:23:in `encode'
activesupport (7.2.2.1) lib/active_support/json/encoding.rb:23:in `encode'
activesupport (7.2.2.1) lib/active_support/core_ext/object/json.rb:42:in `to_json'
active_model_serializers (0.8.4) lib/active_model/serializer.rb:331:in `to_json'
multi_json (1.15.0) lib/multi_json/adapters/oj.rb:56:in `dump'
multi_json (1.15.0) lib/multi_json/adapters/oj.rb:56:in `dump'
multi_json (1.15.0) lib/multi_json/adapter.rb:25:in `dump'
multi_json (1.15.0) lib/multi_json.rb:139:in `dump'
app/controllers/topics_controller.rb:1383:in `block (2 levels) in perform_show_response'
actionpack (7.2.2.1) lib/action_controller/metal/mime_responds.rb:224:in `respond_to'
app/controllers/topics_controller.rb:1377:in `perform_show_response'
app/controllers/topics_controller.rb:191:in `show'
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 (4.0.0) lib/mini_profiler/profiling_methods.rb:116: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.17) lib/rack/tempfile_reaper.rb:15:in `call'
rack (2.2.17) lib/rack/conditional_get.rb:27:in `call'
rack (2.2.17) 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/csp_script_nonce_injector.rb:12:in `call'
config/initializers/008-rack-cors.rb:14:in `call'
rack (2.2.17) lib/rack/session/abstract/id.rb:266:in `context'
rack (2.2.17) lib/rack/session/abstract/id.rb:260:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/cookies.rb:704:in `call'
activerecord (7.2.2.1) lib/active_record/migration.rb:674: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/executor.rb:16:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/actionable_exceptions.rb:18: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'
railties (7.2.2.1) lib/rails/rack/logger.rb:41: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/remote_ip.rb:96:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/request_id.rb:33:in `call'
rack (2.2.17) lib/rack/method_override.rb:24:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/executor.rb:16:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/static.rb:27:in `call'
rack (2.2.17) lib/rack/sendfile.rb:110:in `call'
lib/middleware/missing_avatars.rb:22:in `call'
actionpack (7.2.2.1) lib/action_dispatch/middleware/host_authorization.rb:143:in `call'
rack-mini-profiler (4.0.0) lib/mini_profiler.rb:334:in `call'
lib/middleware/processing_request.rb:12:in `call'
message_bus (4.4.1) lib/message_bus/rack/middleware.rb:60: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.17) lib/rack/urlmap.rb:74:in `block in call'
rack (2.2.17) lib/rack/urlmap.rb:58:in `each'
rack (2.2.17) 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)>'
bin/unicorn:96:in `load'
bin/unicorn:96:in `block in <main>'
bin/unicorn:95:in `fork'
bin/unicorn:95:in `<main>'

Reproducible steps:

On 3.5.0.beta8-dev with Discourse Calendar (and Event) - 0.5 0162ed5 enabled:

1. Create a new topic topic 1 - that will be deleted with a single post as user1 who is tl1
2. Create new topic topic 2 - that will break with a single post ensure Calendar Event is part of this post.
3. As user1 delete your post in topic 1 - that will be deleted locking the topic and replacing your post with “(topic deleted by author)”
4. As user2 who has moderator permissions:
5. Select posts in topic 1 - that will be deleted and select the first (now deleted) post > move to > topic 2 - that will break with preserve chronological order after merging checked.
6. As user2 delete the newly merged first post in topic 2 - that will break (the post that was just merged that reads “(topic deleted by author)”) this will make topic 2 - that will break break and throw the above error.

A workaround - disable the calendar-and-event plugin. This will enable you to visit the broken topic and undelete the first post. Then re-enable the plugin.

Discussion:

This also identified a slightly less severe bug with Discourse where performing the above with the calendar-and-event plugin disabled will make the topic unlisted - see separate bug report:

Topic unlisted to all users including admins after merging topics

Priority/Severity: Low Priority / Medium Severity Platform: 3.5.0.beta8-dev Description: After merging topics and deleting a merged post the topic efectively becomes unlisted without being marked as such and doesn’t show up even to tl4 users or Admins. It is expected that the topic will function normally and not become effectively unlisted. Speculating - It may also be the case that this topic is now viewed as “deleted” and potentially be removed automatically as part of cleanup. Reproducib…

[sam (Sam Saffron)]

Thanks heaps for the detailed write up. Agree we should clean this up, going to put a pr-welcome on this for now.

[Michael12 (Michael Phillips)]

Thanks both for the detailed report and the follow-up. I just ran into a similar issue after merging topics with calendar events and couldn’t figure out why the topic was breaking. Glad to see this is documented.

I’ll keep an eye on any PRs for this. In the meantime, disabling the plugin temporarily as a workaround is helpful to know. Let me know if there’s anything I can help test.

[JammyDodger]

FWIW I think I can simplify the repro:

• Enable both Calendar enabled and Discourse post event enabled
• As admin, create Topic1
• As admin, create Topic2 and include a post event using ‘Create event’ from the circle-with-a-plus composer menu
• As admin, go to Topic1 and use the topic wrench to select the OP and merge into Topic2 with ‘preserve chronological order after merging` checked

At this stage the post event is now no longer the OP of the topic for Topic2

• As admin, delete Topic2 using either the post delete of the OP or the topic delete option

Result: borkity bork. Deleted topic inaccessible. Error in /logs of NoMethodError (undefined method ‘category_id’ for nil)

---

Would the fix here be to prevent the merge that would replace the event as the OP?

When trying to merge an event post (one correctly placed in an OP) into another topic it’s blocked with this in the logs:

Failed to process hijacked response correctly : ActiveRecord::RecordNotSaved : An event can only be in the first post of a topic

(The UI error is more generic, ‘There was an error moving posts.`, which would be nice if it was more verbose)

Could something similar be applied to prevent a merge moving the event out of the OP position?