Chat-Upload-Fehler

Fehler
1

Hallo, ich möchte ein Problem melden, das ich beim Hochladen von Dateien oder Bildern in Chat-DMs gefunden habe.

  • Wenn Benutzer A Datei A in einem Beitrag oder Chat hochlädt, funktioniert dies einwandfrei und die Datei wird korrekt registriert.

  • Aber wenn Benutzer B Datei A herunterlädt und dann versucht, sie erneut in einem anderen Chat hochzuladen:

    • Wenn Benutzer B die Datei ohne Text sendet, erhält er eine Fehlermeldung:

      Message is too short, must have a minimum of 1 character.

    • Wenn Benutzer B die Datei mit Text sendet, wird nur der Text gesendet – der Anhang fehlt.

Wenn Benutzer A jedoch dieselbe Datei erneut hochlädt, funktioniert dies einwandfrei und die Datei ist in der Nachricht enthalten.

Meine Frage:
Ist dies das beabsichtigte Verhalten, bei dem nur der ursprüngliche Uploader eine hochgeladene Datei wiederverwenden kann? Oder sollten andere Benutzer auch in der Lage sein, dieselbe Datei in einem Chat hochzuladen und zu senden?

image
image1096×578 35.9 KB

1 „Gefällt mir“
3

Ja, das klingt nach einem kleinen Fehler. Jemand wird sich das in den nächsten Wochen ansehen. Gerne können Sie in der Zwischenzeit eine PR-Anfrage stellen.

1 „Gefällt mir“
5

Es hat eine Weile gedauert, bis ich das durchforstet habe, aber ich glaube, ich habe die Ursache gefunden. Ich glaube, es hat mit diesem Code zu tun:

def fetch_uploads(params:, guardian:)
  return [] if !SiteSetting.chat_allow_uploads
  guardian.user.uploads.where(id: params.upload_ids) # Speziell hier
end

Wenn eine neue Chat-Nachricht erstellt wird, wird diese aufgerufen, um die Uploads abzurufen, die an die Nachricht angehängt werden sollen. Möglicherweise, um die Eigentümerschaft sicherzustellen, durchläuft die Methode den Benutzer des Guardian, um die Uploads abzurufen, damit nur Uploads zugelassen werden, die diesem Benutzer gehören.
Das Problem ist, dass Uploads dedupliziert werden, wie hier zu sehen ist (hier):

# Haben wir diesen Upload bereits?
@upload = Upload.find_by(sha1: sha1)

# ...

# Gib den vorherigen Upload zurück, falls vorhanden
if @upload
  add_metadata!
  UserUpload.find_or_create_by!(user_id: user_id, upload_id: @upload.id) if user_id
  return @upload
end

Ich glaube, dass eine mögliche Lösung darin besteht, die UserUploads zu durchlaufen, anstatt nur die Uploads zu berücksichtigen, die dem Benutzer gehören. UserUpload verknüpft Uploads mit mehreren Benutzern, was anscheinend das ist, was wir brauchen. Ich bin mir noch nicht zu 100 % sicher, wie ich es richtig machen kann; es wird spät, also werde ich mich ins Bett legen, aber wenn niemand anderes es behebt, werde ich versuchen, später zurückzukommen und an einem PR zu arbeiten. :slightly_smiling_face:

3 „Gefällt mir“
6

Das ist ein fantastischer Fang und genau richtig. :hugs:

Uploads haben eine user_id, aber das ist nur der ursprüngliche Benutzer, der den Upload erstellt hat.

Die Umwandlung dieses Codes in:

Upload.where(id: params.upload_ids).joins(:user_uploads).where(user_uploads: { user_id: guardian.user.id })

wird den Trick tun.

Können Sie bitte einen PR einreichen!

7

Danke, ich war mir nicht sicher, wie ich diese Abfrage am besten erstellen kann (ich bin hier ziemlich neu :slightly_smiling_face: ). Ich werde an einem PR arbeiten.

3 „Gefällt mir“
8

Ok, ich habe den PR endlich fertiggestellt. Entschuldigung für die Verzögerung, Lebensprobleme kamen dazwischen.

FIX: In Chatnachrichten, Uploads nach UserUpload filtern, nicht nach Upload.user von clechasseur · Pull Request #34596 · discourse/discourse

9

Ich sehe, dass einige der Tests fehlgeschlagen sind, aber es sind keine Tests, die ich angefasst habe. Ich hatte auch Probleme, die Tests in diesen Dateien lokal auszuführen – ich habe dieselben duplicate key-Fehler erhalten, die ich hier sehe… Ich bin mir nicht sicher, ob ich etwas kaputt gemacht habe oder ob die Tests manchmal fehlerhaft sind…

10

Eine gute Faustregel ist, die fehlgeschlagenen Tests mindestens dreimal erneut auszuführen und erst dann in Panik zu geraten, wenn sie zum dritten Mal fehlschlagen :winking_face_with_tongue:

Das gesagt, @j.jaffeux kann vielleicht helfen?

1 „Gefällt mir“
11

Ja, das sind nur unzuverlässige und andere zufällige Fehler, Ihre Änderungen sind in Ordnung. Ich führe die PR-Tests erneut aus. Wenn sie bestanden werden oder nur mit demselben zufälligen Sequenzfehler fehlschlagen, werde ich genehmigen und zusammenführen.

1 „Gefällt mir“
12

Ich bin zu sehr Horrorfan, um nicht zu wissen, was passiert, wenn man etwas dreimal tut :sweat_smile:

2 „Gefällt mir“
13

Dieser PR wurde zusammengeführt, nochmals vielen Dank für den Beitrag @clechasseur :rocket:

6 „Gefällt mir“