PostRevisor non può revisionare post in topic eliminati

Bug
1

tl;dr

Per alcuni post, chiamare PostRevisor imposta post_id a nil. Sono pazzo?

Risposta: No. PostRevisor accede a post.topic, che è nil per un post in un topic eliminato. E poi imposta post.topic a nil, che a sua volta imposta post.topic_id a nil.

Penso che PostRevisor dovrebbe ottenere il topic in questo modo:

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

piuttosto che in questo modo:

  @topic = topic

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

La storia completa

Sto lavorando a uno script che corregge i link goo.gl (il servizio sta per essere dismesso, quindi lo script trova i link goo.gl, recupera a cosa sono reindirizzati e sostituisce l’URL goo.gl con quello a cui è diretto. Funziona per la maggior parte).

Ma

Per un certo numero di post, sembra che tutto vada bene, ma poi PostRevisor fallisce perché post.acting_user è nil. E poi, nel mio rescue, sembra che topic_id sia nil, ma non è il post ad essere nil, perché ha ancora 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

Per la stragrande maggioranza dei post, questo funziona bene, ma per un certo sottoinsieme di essi, fallisce in questo modo. E se eseguo il codice manualmente riga per riga, ottengo lo stesso risultato. Inizia a sembrare, però, che se faccio un pr=PostRevisor.new(post), vedo che il post nel record pr non ha topic_id e poi se ispeziono il post ora ha topic_id impostato a nil.

1 Mi Piace
2

Per curiosità, c’è un motivo per cui non stai modificando direttamente il modello Post?

3

Perché se stai modificando 3000 post con gsub e un’espressione regolare complicata con un sacco di casi limite (correzione: goo.gl, http://goo.gl, https://goo.gl, ma non toccare https://maps.app.goo.gl, o https://map.goo.gl, e forse potresti essere limitato nella frequenza da goo.gl, e così via) potresti voler recuperare il post prima di averlo completamente rovinato!

È stato molto utile poter guardare le modifiche e vedere il prima e il dopo ed essere in grado di annullare! Una versione trasformerebbe https://maps.app.goo.gl/abd12 in qualcosa come https://maps.app.https://maps.google.com/;lkajw3rpoazse;flknmase;faijserfasefklasdfa, per esempio.

1 Mi Piace
4

ha senso :slight_smile:

1 Mi Piace
5

Qualche anno fa c’era uno script simile che ho cercato di far eseguire a CDCK per un cliente e loro hanno detto (con parole mie, non loro) “Amico. Pensi che eseguiremo il tuo codice che cambia alla leggera un numero enorme di post e non c’è modo di annullarlo? Ripensaci.”

1 Mi Piace
6

Sì, un altro approccio è eseguirlo su un backup in un’area controllata. Ma adoro la tua soluzione.

1 Mi Piace
7

Ecco l’inizializzatore:

Quindi, in qualche modo il post ci arriva e, sebbene post_id abbia un valore, post no?

8

Questo non riesco a riprodurlo.
L’ultimo post.topic_id non è nil e, come previsto, è una ripetizione del risultato precedente

9

Sì. Ha funzionato sulla maggior parte dei post. C’è qualcosa di strano con alcuni di essi.

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

Sono abbastanza sicuro che non dovrebbe succedere. :person_shrugging:

Ma aspetta:

Topic.find(post.topic_id)
ActiveRecord::RecordNotFound: Impossibile trovare Topic con 'id'=179227 [DOVE "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!'

L’argomento è stato eliminato.

Quindi la domanda è se sia un bug il fatto che non si possano rivedere i post in argomenti eliminati.

Suppongo che non mi importerà e farò in modo che il mio script controlli post.topic per nil piuttosto che post.topic_id.

1 Mi Piace
10

C’è una bella mancanza di integrità lì con la chiave esterna!

Penso che alcuni dei meccanismi di protezione (eufemismo) siano disattivati perché le tabelle sono solitamente troppo grandi.

Forse qualcuno ha eseguito un topic.delete invece di destroy. Oops.

11

La mia ricerca dei post è stata come Post.where("raw like '%goo.gl%'"), e questo restituirà i post in argomenti eliminati. E poi quei post hanno un topic_id, ma non un topic. Penso che ci sia un modo per far sì che Topic.find restituisca argomenti eliminati (perché un amministratore può vedere quegli argomenti, motivo per cui ero così confuso. Quel piccolo cestino è facile da trascurare.)

Quindi è così:

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

Quindi forse avrei dovuto farlo e aggiornare il post prima di chiamare PostRevisor?

Ma nell’UX posso aggiornare un post in un argomento eliminato, quindi sto pensando che

def initialize(post, topic = post.topic)
  @post = post
  @topic = topic
  # Assicurati di avere una sola istanza di Topic
  post.topic = topic
end

dovrebbe essere

def initialize(post, topic = post.topic)
  @post = post
  @topic = topic || Topic.with_deleted.find_by(id: post_topic_id)
  # Assicurati di avere una sola istanza di Topic
  post.topic = topic
end

Sposto questo in Bug nel caso in cui qualcun altro pensi che lo sia.

Ma questo funziona:

        if !post.topic # i post negli argomenti eliminati non hanno un topic e rompono 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)

C’erano post in 3 argomenti che hanno causato il crash di rails. Ci rinuncio. :slight_smile:

1 Mi Piace
12

E un’altra versione di quanto sopra che esegue deleted_topic = Topic.with_deleted.find_by(id: 123) per aggiornare post.topic funziona anche.\n\nSembra ancora un bug, però. O forse, dato che il core fa qualcos’altro per gestire questo, non è un bug.

1 Mi Piace
13

O una funzionalità mancante (e quindi una specifica)?

Online, questo non verrà mai chiamato per i Post in Argomenti eliminati?

Concordo, tuttavia, sul fatto che dovrebbe gestirlo correttamente :thinking:

14

Forse

Stanno chiaramente facendo qualcosa per consentirlo sui post in argomenti eliminati, probabilmente come sto facendo io.

Sì. Qualcuno che prende tali decisioni può spostarlo di nuovo in Dev se lo ritiene appropriato.

15

Questo argomento è stato chiuso automaticamente 30 giorni dopo l’ultima risposta. Non sono più consentite nuove risposte.