`NoMethodError in TopicsController#show` fout na het samenvoegen van 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)]

Bedankt voor de gedetailleerde beschrijving. Ik ben het ermee eens dat we dit moeten opschonen, ik zet hier voorlopig een pr-welcome op.

[Michael12 (Michael Phillips)]

Bedankt beiden voor het gedetailleerde rapport en de follow-up. Ik liep net tegen een vergelijkbaar probleem aan na het samenvoegen van onderwerpen met agenda-evenementen en kon er niet achter komen waarom het onderwerp kapot ging. Fijn om te zien dat dit gedocumenteerd is.

Ik zal de PR’s hiervoor in de gaten houden. Ondertussen is het nuttig om te weten dat het tijdelijk uitschakelen van de plugin als workaround helpt. Laat het me weten als ik ergens mee kan helpen testen.

[JammyDodger]

FWIW, ik denk dat ik de repro kan vereenvoudigen:

• Schakel zowel Agenda ingeschakeld als Discourse-evenement inschakelen in
• Maak als beheerder Topic1
• Maak als beheerder Topic2 en voeg een evenement toe met behulp van ‘Evenement maken’ uit het menu van de componist met de cirkel met een plus
• Ga als beheerder naar Topic1 en gebruik de topicsleutel om de OP te selecteren en samen te voegen met Topic2 met ‘Chronologische volgorde behouden na samenvoegen’ aangevinkt

Op dit punt is het evenement niet langer de OP van het onderwerp voor Topic2

• Verwijder als beheerder Topic2 met behulp van de post-verwijdering van de OP of de optie voor het verwijderen van het onderwerp

Resultaat: borkity bork. Verwijderd onderwerp ontoegankelijk. Fout in /logs van NoMethodError (undefined method ‘category_id’ for nil)

---

Zou de oplossing hier zijn om de samenvoeging te voorkomen die het evenement als OP zou vervangen?

Bij het proberen een evenementpost (een correct geplaatste in een OP) samen te voegen met een ander onderwerp, wordt dit geblokkeerd met dit in de logs:

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

(De UI-fout is algemener, ‘Er is een fout opgetreden bij het verplaatsen van berichten.’, wat fijn zou zijn als het gedetailleerder was)

Zou iets soortgelijks kunnen worden toegepast om te voorkomen dat een samenvoeging het evenement uit de OP-positie verplaatst?