PostRevisor не может редактировать сообщения в удалённых темах

[pfaffman (Jay Pfaffman)]

tl;dr

Для некоторых постов вызов PostRevisor устанавливает post_id в nil. Я что, с ума схожу?

Ответ: Нет. PostRevisor обращается к post.topic, который равен nil для поста в удалённой теме. Затем он устанавливает post.topic в nil, что, в свою очередь, обнуляет post.topic_id.

Я считаю, что PostRevisor должен получать тему следующим образом:

  @topic = topic || Topic.with_deleted.find_by(id: post_topic_id)

а не так:

  @topic = topic

post=Post.find_by(topic_id: 179227, post_number: 12)
post.topic_id => 179227
pr=PostRevisor.new(post)
post.topic_id => nil

Полная история

Я работаю над скриптом, который исправляет ссылки goo.gl (сервис скоро прекратит работу, поэтому скрипт находит ссылки goo.gl, получает адрес, на который они перенаправляют, и заменяет ссылку goo.gl на конечный URL с помощью gsub. В основном всё работает.

Но

Для группы постов всё выглядит отлично, но затем PostRevisor падает, потому что post.acting_user равен nil. Затем в блоке rescue кажется, что topic_id равен nil, но сам post не равен nil, поскольку у него всё ещё есть post_number.

      begin
        puts "Revising (#{count}/#{total_posts}) https://mysite.com/t/#{post.topic_id}/#{post.post_number}"
        puts "missing topic_id for post #{post.id}" if !post.topic_id
        next if !post.topic_id
        PostRevisor.new(post).revise!(system_user, raw: new_raw, **revision_options)
      rescue => e
        puts "cannot revise (number: #{count} https://tw.forumosa.com/t/#{post.topic_id}/#{post.post_number}): #{e}"
      end

FIXING!!: https://goo.gl/maps/XaNG
B7qaZGzhBmM78 -----> https://www.google.com/maps/place/%E6%AD%A5%E9%81%93%E5%92%96%E5%95%A1%E9%A4%A8Cafe+Strada/@22.6300414,120.3153591,17z/d
ata=!3m1!4b1!4m5!3m4!1s0x346e04944a9b3471:0x520c1f01c3d62e57!8m2!3d22.6301115!4d120.3175543?shorturl=1
Revising (680/1773) https://mysite.com/t/207069/1817
cannot revise (number: 680 https://mysite.com/t//1817): undefined method `acting_user=' for nil

Для подавляющего большинства постов всё работает отлично, но для некоторой их части происходит сбой именно таким образом. Если я запускаю код вручную, строка за строкой, результат тот же. Однако начинает казаться, что если выполнить pr=PostRevisor.new(post), то в объекте pr у поста будет отсутствовать topic_id, а при инспекции самого объекта post окажется, что его topic_id теперь установлен в nil.

[merefield (Robert)]

Из любопытства, есть ли причина, по которой вы не редактируете модель Post напрямую?

[pfaffman (Jay Pfaffman)]

@Robert

Есть ли причина, по которой вы не редактируете модель Post напрямую?

Потому что, если вы меняете 3000 постов с помощью gsub и сложного регулярного выражения с кучей крайних случаев (исправить: goo.gl, http://goo.gl, https://goo.gl, но не трогать https://maps.app.goo.gl или https://map.goo.gl, а ещё вы можете получить ограничение скорости от goo.gl и так далее), вам может понадобиться вернуть пост до того, как вы его полностью испортили!

Было очень удобно иметь возможность просматривать правки, видеть состояние до и после, а также отменять изменения! Например, одна версия превращала https://maps.app.goo.gl/abd12 во что-то вроде https://maps.app.https://maps.google.com/;lkajw3rpoazse;flknmase;faijserfasefklasdfa.

[merefield (Robert)]

Это имеет смысл

[pfaffman (Jay Pfaffman)]

Несколько лет назад я пытался запустить похожий скрипт для одного клиента, и они сказали (моими словами, не их): «Друг, ты что, думаем, мы будем запускать твой код, который без разбора изменит миллион постов, и при этом не будет возможности откатить изменения? Подумай ещё раз».

[merefield (Robert)]

Да, другой подход — запустить его на резервной копии в контролируемой зоне. Но мне нравится ваше решение.

[pfaffman (Jay Pfaffman)]

Итак, вот инициализатор:

lib/post_revisor.rb

Таким образом, пост каким-то образом туда попадает, и хотя post_id имеет значение, post — нет?

[merefield (Robert)]

@Jay Pfaffman

post=Post.find_by(topic_id: 179227, post_number: 12)
post.topic_id => 179227
pr=PostRevisor.new(post)
post.topic_id => nil

Это мне воспроизвести не удалось.

Значение post.topic_id в последнем случае не nil, а, как и ожидалось, повторяет предыдущий результат.

[pfaffman (Jay Pfaffman)]

@merefield

У меня это не воспроизводится.

Да. На большинстве постов всё работало. С некоторыми что-то не так.

[63] pry(main)> post.topic_id
=> 179227
[64] pry(main)> post.topic
=> nil
[65] pry(main)>

Я почти уверен, что так быть не должно.

Но подождите:

 Topic.find(post.topic_id)
ActiveRecord::RecordNotFound: Couldn't find Topic with 'id'=179227 [WHERE "topics"."deleted_at" IS NULL]
from /var/www/discourse/vendor/bundle/ruby/3.3.0/gems/activerecord-7.2.2.1/lib/active_record/relation/finder_methods.rb:428:in `raise_record_not_found_exception!'

Тема удалена.

Так что вопрос в том, является ли ошибкой невозможность редактировать посты в удалённых темах.

Думаю, мне это не особо важно, и я просто заставлю свой скрипт проверять post.topic на nil, а не post.topic_id.

[merefield (Robert)]

Да, тут явно не хватает честности с внешним ключом!

Кажется, некоторые защитные механизмы (э-э-э) отключены, потому что таблицы обычно слишком большие.

Возможно, кто-то выполнил topic.delete вместо destroy. Ой.

[pfaffman (Jay Pfaffman)]

Мой поиск постов был таким: Post.where("raw like '%goo.gl%'"), и это возвращало посты из удалённых тем. При этом у этих постов есть topic_id, но самой темы нет. Кажется, есть способ заставить Topic.find возвращать удалённые темы (потому что администратор может их видеть, именно поэтому я так запутался. Эта маленькая корзина легко может остаться незамеченной.)

Так что это выглядит так:

deleted_topic = Topic.with_deleted.find_by(id: 123)

Может быть, мне следовало сделать именно это и обновить пост перед вызовом PostRevisor?

Но в UX я могу обновлять пост в удалённой теме, поэтому я думаю, что

def initialize(post, topic = post.topic)
  @post = post
  @topic = topic
  # Убедимся, что у нас только один экземпляр Topic
  post.topic = topic
end

следует заменить на

def initialize(post, topic = post.topic)
  @post = post
  @topic = topic || Topic.with_deleted.find_by(id: post.topic_id)
  # Убедимся, что у нас только один экземпляр Topic
  post.topic = topic
end

Я перемещу это в bug, на случай если кто-то другой тоже считает это ошибкой.

Но вот это работает:

        if !post.topic # посты в удалённых темах не имеют темы и ломают PostRevisor
           post.topic = Topic.with_deleted.find_by(id: post.topic_id)
           next if !post.topic
        end
        PostRevisor.new(post).revise!(system_user, raw: new_raw, **revision_options)

Были посты в 3 темах, которые приводили к краху Rails. Я сдаюсь с этим.

[pfaffman (Jay Pfaffman)]

И еще один вариант из вышеописанного, который выполняет deleted_topic = Topic.with_deleted.find_by(id: 123) для обновления post.topic, тоже работает.

Тем не менее, это всё ещё похоже на ошибку. Или, возможно, поскольку ядро делает что-то другое для управления этим, это не ошибка.

[merefield (Robert)]

Или отсутствует функция (и, следовательно, спецификация)?

В онлайн-режиме это никогда не будет вызываться для сообщений в удалённых темах?

Хотя я согласен, что система должна корректно обрабатывать это

[pfaffman (Jay Pfaffman)]

@Robert

Или отсутствует функция (и, следовательно, спецификация)?

Возможно.

@Robert

В онлайн-режиме это никогда не будет вызываться для постов в удалённых темах?

Очевидно, они что-то делают, чтобы это работало с постами в удалённых темах, вероятно, так же, как и я.

@Robert

Хотя я согласен, что это должно управляться корректно

Да. Тот, кто принимает такие решения, может переместить это обратно в Development, если считает это уместным.