PostRevisor ne peut pas modifier les messages dans les sujets supprimés

Bug
1

tl;dr

Pour certains messages, l’appel de PostRevisor met post_id à nil. Suis-je fou ?

Réponse : Non. PostRevisor accède à post.topic, qui est nil pour un message dans un sujet supprimé. Et ensuite, il met post.topic à nil, ce qui à son tour met post.topic_id à nil.

Je pense que PostRevisor devrait obtenir le sujet comme ceci :

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

plutôt que comme ceci :

  @topic = topic

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

L’histoire complète

Je travaille sur un script qui corrige les liens goo.gl (le service étant bientôt abandonné, le script trouve les liens goo.gl, récupère vers quoi ils redirigent et remplace l’URL goo.gl par celle vers laquelle elle est dirigée. Cela fonctionne la plupart du temps.

Mais

Pour un certain nombre de messages, tout semble bien se passer, mais ensuite PostRevisor échoue car post.acting_user est nil. Et ensuite, dans mon rescue, il semble que topic_id soit nil, mais ce n’est pas le post qui est nil, car il a toujours un 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/data=!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

Pour la grande majorité des messages, cela fonctionne très bien, mais pour un sous-ensemble d’entre eux, cela échoue comme ceci. Et si j’exécute le code ligne par ligne à la main, j’obtiens la même chose. Il commence à ressembler à ceci, cependant, si je fais un pr=PostRevisor.new(post), je vois que le message dans l’enregistrement pr n’a pas de topic_id et ensuite si j’inspecte le post, il a maintenant topic_id défini à nil.

1 « J'aime »
2

Juste par curiosité, y a-t-il une raison pour laquelle vous n’éditez pas directement le modèle Post ?

3

Parce que si vous modifiez 3000 publications avec gsub et une expression régulière compliquée avec un tas de cas limites (correction : goo.gl, http://goo.gl, https://goo.gl, mais ne touchez pas https://maps.app.goo.gl, ou https://map.goo.gl, et peut-être que vous atteignez la limite de débit de goo.gl, et ainsi de suite) vous pourriez vouloir récupérer la publication avant de l’avoir complètement ruinée !

C’est vraiment génial de pouvoir examiner les modifications et voir l’avant et l’après, et de pouvoir annuler ! Une version transformerait https://maps.app.goo.gl/abd12 en quelque chose comme https://maps.app.https://maps.google.com/;lkajw3rpoazse;flknmase;faijserfasefklasdfa, par exemple.

1 « J'aime »
4

cela a du sens :slight_smile:

1 « J'aime »
5

Il y a quelques années, il y avait un script similaire que j’ai essayé de faire exécuter à CDCK pour un client et ils ont dit (dans mes mots, pas les leurs) : « Mec. Tu penses qu’on va exécuter ton code qui change des milliards de posts sans possibilité d’annuler ? Repense-y. »

1 « J'aime »
6

Oui, une autre approche consiste à l’exécuter sur une sauvegarde dans une zone contrôlée. Mais j’adore votre solution.

1 « J'aime »
7

Voici l’initialiseur :

Donc, d’une manière ou d’une autre, le message arrive et bien que post_id ait une valeur, post n’en a pas ?

8

Je ne peux pas reproduire cela.

Le dernier post.topic_id n’est pas nul et, comme prévu, une répétition du résultat précédent.

9

Ouais. Ça a fonctionné sur la plupart des messages. Il y a quelque chose d’étrange avec certains d’entre eux.

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

Je suis à peu près sûr que ce n’est pas censé se produire. :person_shrugging:

Mais attendez :

Topic.find(post.topic_id)
ActiveRecord::RecordNotFound: Impossible de trouver le sujet avec 'id'=179227 [OÙ "topics"."deleted_at" EST NUL]
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!'

Le sujet est supprimé.

La question est donc de savoir s’il s’agit d’un bug qui empêche la révision des messages dans les sujets supprimés.

Je suppose que je ne m’en soucierai pas et que mon script vérifiera post.topic pour nil plutôt que post.topic_id.

1 « J'aime »
10

Il y a un manque d’intégrité assez flagrant là avec la clé étrangère !

Je pense que certains garde-fous (ahem) sont désactivés car les tables sont généralement trop grandes.

Peut-être que quelqu’un a effectué un topic.delete et non destroy. Oups.

11

Ma recherche des publications était comme Post.where(\"raw like '%goo.gl%'), et cela renverrait des publications dans des sujets supprimés. Et ensuite, ces publications ont un topic_id, mais pas un topic. Je pense qu’il y a un moyen de faire en sorte que Topic.find renvoie des sujets supprimés (car un administrateur peut voir ces sujets, c’est pourquoi j’étais si confuse. Cette petite poubelle est facile à manquer.)

Donc, c’est comme ça :

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

Alors peut-être aurais-je dû faire cela et mettre à jour la publication avant d’appeler PostRevisor ?

Mais dans l’UX, je peux mettre à jour une publication dans un sujet supprimé, donc je pense que

def initialize(post, topic = post.topic)
  @post = post
  @topic = topic
  # Make sure we have only one Topic instance
  post.topic = topic
end

devrait être

def initialize(post, topic = post.topic)
  @post = post
  @topic = topic || Topic.with_deleted.find_by(id: post_topic_id)
  # Make sure we have only one Topic instance
  post.topic = topic
end

Je vais déplacer cela vers Bug au cas où quelqu’un d’autre penserait que c’est le cas.

Mais cela fonctionne :

        if !post.topic # posts in delted topics have no topic and break 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)

Il y avait des publications dans 3 sujets qui ont fait planter Rails. J’abandonne. :slight_smile:

1 « J'aime »
12

Et une autre version de ce qui précède qui utilise deleted_topic = Topic.with_deleted.find_by(id: 123) pour mettre à jour post.topic fonctionne également.

Cela ressemble toujours à un bug, cependant. Ou peut-être que comme le cœur fait autre chose pour gérer cela, ce n’est pas un bug.

1 « J'aime »
13

Ou une fonctionnalité manquante (et donc une spécification) ?

En ligne, cela ne sera jamais appelé pour des Posts dans des Topics supprimés ?

Je suis d’accord, cependant, qu’il devrait gérer cela correctement :thinking:

14

Peut-être

Ils font clairement quelque chose pour permettre que cela fonctionne sur des messages dans des sujets supprimés, probablement comme je le fais.

Ouais. Quelqu’un qui prend de telles décisions peut le déplacer vers Dev s’il pense que c’est approprié.

15

Ce sujet a été automatiquement fermé 30 jours après la dernière réponse. Les nouvelles réponses ne sont plus autorisées.