Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix translation citation pars #3448

Merged
merged 23 commits into from
Sep 7, 2023
Merged

Conversation

saviit
Copy link
Contributor

@saviit saviit commented Aug 2, 2023

Corrects the handling of cited paragraphs/citations when translating documents:

  • preserve citations instead of converting them into regular translated paragraphs
  • if the cited document has a same-language translation to the current translation, uses the translated citation (or the original if no corresponding translations exist)

Resolves #3405, resolves #3388, resolves #3465.

TODO:

  • Fix citation mark/arrow link pointing to original document instead of translation
  • Fix citation mark/arrow popover bubble/text to fetch correct citation information
  • Add r="tr" attr to cite pars in translated documents, so we can get the correct reference in the citation mark/ParRef component
  • Add explicit area and area_end tags to translated area pars, so that area citations can be referenced in translations
  • Fix/update tests
  • Fix area citations
    • Area citations in translations should get the source translation's document id instead of the current (ie. citing) translation's base document.
    • ParRef reference for area citations is not correct
  • Fix synchronize_translations so it produces correct references to translated citations
    • if the original is edited, synchronize_translations will remove existing citations in translations and replace them with plain references to the original
    • synchronize_translations adds extra citation pars: references aren't correctly followed for area citations in translations, so SequenceMatcher gives wrong opcodes

Known issues:

  1. there is a corner case where cited plugins from preambles do not find their correct tasks

  1. cite pars in translated documents will trigger readmark errors infinitely when mousehovering over the cited par

  1. synchronize_translations does not insert new paragraphs from the original in the correct order

  1. TIM menu / paragraph menu item 'Follow reference' references original document instead of cited content doc (translation) in translations, but only for area citations (regular paragraph citation are correct)
  • appears to have been fixed, probably as a side effect of 7492ee9 or 8205bb1

@saviit saviit force-pushed the fix-translation-citation-pars branch 2 times, most recently from cf8912a to 13e63ba Compare August 7, 2023 11:18
@saviit saviit force-pushed the fix-translation-citation-pars branch from af5b0f0 to b07f5e4 Compare August 11, 2023 12:21
@saviit
Copy link
Contributor Author

saviit commented Aug 11, 2023

Tämä testattavana timdevs01-6:ssa, esim. https://timdevs01-6.it.jyu.fi/view/users/test-user-1/translate-cite-pars.

Kaipaa kommentteja.

@sijualle
Copy link
Contributor

Nuo kortin #3405 järjestykset 123/213 toimivat hyvin, mutta tuo tapaus jossa käännös on tehty aiemmin, ja sitten viittaja ottaa uusia viitteitä alkuperäisdokumentista ei toimi (312/321). Käyttötapauksena esim tilanne jossa on lähdedokumentti, jossa on yleisiä demontekoohjetta, josta sitten on viitteitä jokaisella yksittäisellä demosivulla.
Jos tuohon lähdedokumenttiin A tekee uuden ohjekappaleen, jonka yrittää ottaa yksittäiseen demosivuun B (jolla on jo aiemmin käännös olemassa), niin dokumentin B käännöksissä näkyy A:n alkuperäiskielinen kappale.
Ja vaikka tuon viitteen yrittäisi tehdä käännöksestä käännökseen niin että käy painamassa copy A:n käännöksessä ja painaa paste as reference B:n käännöksessä, niin B:n käännökseen ilmestyvä kappale osoittaa A:n kääntämättömän version kappaleeseen: https://timdevs01-6.it.jyu.fi/view/users/test-user-1/translate-cite-pars/312toka/312-viitaaja/en-US

Sama homma on tuon areaviittauksen kanssa. Jos ensin on A:n käännös, josta otetaan alueviite B:hen, ja luodan B:stä käännös, niin B:n käännöksen alue tulee suoraan A:n käännöksestä kuten pitää: https://timdevs01-6.it.jyu.fi/view/users/test-user-1/refarea/referrer2
Mutta jos B:n käännös on keretty tekemään ennen kuin ottaa A:sta alueviitteen niin B:n käännöksen alue osoittaa A:n kääntämättömään alueeseen: https://timdevs01-6.it.jyu.fi/view/users/test-user-1/refarea/referrer

@saviit
Copy link
Contributor Author

saviit commented Aug 12, 2023

järjestykset 123/213 toimivat hyvin, mutta tuo tapaus jossa käännös on tehty aiemmin, ja sitten viittaja ottaa uusia viitteitä alkuperäisdokumentista ei toimi (312/321)

Joo, tässä lienee nyt ongelmana se, että nuo keissit eivät mene samoja reittejä pitkin.

Toistan tähän nyt selkeyden vuoksi hieman tuolta kortin #3405 asiaa.

  1. dokumentti A -> käännös kielelle L
  2. dokumentti B -> lainaa A:n lohkoja
  3. dokumentti B -> käännös kielelle L

1-2-3: ok
1-3-2: vaihe 2 ei mene käännösreitin kautta vaan synkronoinnin läpi, ei tule viitettä A:n käännökseen
2-1-3: ok
2-3-1: ei voida saada viitteitä A:n käännöksiin kohdassa 3, koska niitä ei vielä ole olemassa. Pitäisi synkronoida kohdassa 1, mutta tässä ei liene järkeä, koska käytännössä pitäisi tarkistaa kaikki olemassa olevat dokumentit siltä varalta, että on viittaus johonkin A:n lohkoon.
3-1-2: vaihe 2 aiheuttaa synkronoinnin, mutta ei tuota viitteitä A:n käännettyihin lohkoihin (sama kuin 1-3-2)
3-2-1: kohdassa 2 dokumentti A:sta ei ole vielä käännöstä, joten ei voida saada oikeita viittauksia (sama kuin 2-3-1)

Eli nuo käyttötapaukset 1-3-2 ja 3-1-2 pitää korjata tuolla synchronize_translations.pyssä vastaamaan sitä, mitä tässä PR:ssä on jo toteutettu. Käyttötapaukset 2-3-1 ja 3-2-1 eivät nähdäkseni ole toteuttamiskelpoisia.

@saviit saviit force-pushed the fix-translation-citation-pars branch from b07f5e4 to 4ff88d4 Compare August 24, 2023 12:17
@saviit saviit force-pushed the fix-translation-citation-pars branch from 81c2e72 to 7e9d4f4 Compare August 28, 2023 11:29
@saviit
Copy link
Contributor Author

saviit commented Aug 28, 2023

1. dokumentti A -> käännös kielelle L
2. dokumentti B -> lainaa A:n lohkoja
3. dokumentti B -> käännös kielelle L

1-2-3: ok
1-3-2: vaihe 2 ei mene käännösreitin kautta vaan synkronoinnin läpi, ei tule viitettä A:n käännökseen
2-1-3: ok
2-3-1: ei voida saada viitteitä A:n käännöksiin kohdassa 3, koska niitä ei vielä ole olemassa. Pitäisi synkronoida kohdassa 1, mutta tässä ei liene järkeä, koska käytännössä pitäisi tarkistaa kaikki olemassa olevat dokumentit siltä varalta, että on viittaus johonkin A:n lohkoon.
3-1-2: vaihe 2 aiheuttaa synkronoinnin, mutta ei tuota viitteitä A:n käännettyihin lohkoihin (sama kuin 1-3-2)
3-2-1: kohdassa 2 dokumentti A:sta ei ole vielä käännöstä, joten ei voida saada oikeita viittauksia (sama kuin 2-3-1)

Tämä toimii nyt pääosin kaikille tapauksille, eli päivityksenä yllämainittuun:

1-3-2: ok
3-1-2: ok
2-3-1: toimii jos pakottaa synkkauksen lisäämällä tai poistamalla kokonaisen lohkon dokumenttiin B
3-2-1: (sama kuin 2-3-1)

2-3-1 ja 3-2-1 koskien tein jo uuden kortin mahdollisesta jatkoratkaisusta, mielestäni tähän PR:ään ei kannata enempää niitä hinkata.

Alueviittauksissa on vielä vähän ongelmia, katson jos saisin ne vielä korjattua tähän samaan PR:ään.

Päivitetty timdevsiin, testikeissit löytyy https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448.

@saviit saviit force-pushed the fix-translation-citation-pars branch from 90a133b to 623c641 Compare August 30, 2023 12:18
@saviit
Copy link
Contributor Author

saviit commented Aug 30, 2023

Alueviitteiden pitäisi nyt myös toimia:

  1. dokumentti A -> käännös kielelle L
  2. dokumentti B -> lainaa A:n lohkoja
  3. dokumentti B -> käännös kielelle L

1-2-3: ok
1-3-2: ok
2-1-3: ok
2-3-1: toimii kun pakottaa synkkauksen B:n alkuperäisessä lisäämällä/poistamalla lohkon
3-1-2: ok
3-2-1: toimii kun pakottaa synkkauksen B:n alkuperäisessä lisäämällä/poistamalla lohkon

Päivitetty timdevsiin, testikeissit/pohjat löytyy https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448. pohja-kansiota kopioimalla saa lisää valmiita pohjia.

PR:n esittelyssä on lueteltu tähän mennessä eteen tulleet bugit/puutteet, mitä en ole ehtinyt korjata.

@saviit saviit requested review from dezhidki and sijualle August 30, 2023 12:48
@sijualle
Copy link
Contributor

sijualle commented Aug 31, 2023

Minusta tuo toimii nyt normaaleilla kappaleviitteleillä hyvin. Vaikka b:hen ottaa uusia viitteitä jo käännösten tekemisen jälkeen niin b:n käännökset osoittaa oikein a:n käännöksiin. Ja tuo on hyvä että voi pakottaa tuon synkkauksen, eli jos A:n kääntää vasta myöhemmin niin sekin onnistuu.

Mutta areoita en saanut toimimaan. Jos B:n käännös oli tehty ennen alueviitteen lisäämistä B:hen, B:n käännökseen ei tullut mitään uutta kappaletta: https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/1-3-2/b1/fi
Jos taas oli tilanne jossa B:hen lisättiin alueviittaus, ja sitten vasta tehtiin B:n käännös, B:n käännökseen tuli alueviite joka ilmoitti että aluetta ei löydy: https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/3-1-2/b1/en-GB

@saviit saviit force-pushed the fix-translation-citation-pars branch from 623c641 to 0b8168e Compare August 31, 2023 11:53
@saviit
Copy link
Contributor Author

saviit commented Aug 31, 2023

Jes, kiitos testauksesta :)

Mutta areoita en saanut toimimaan. Jos B:n käännös oli tehty ennen alueviitteen lisäämistä B:hen, B:n käännökseen ei tullut mitään uutta kappaletta: https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/1-3-2/b1/fi
Jos taas oli tilanne jossa B:hen lisättiin alueviittaus, ja sitten vasta tehtiin B:n käännös, B:n käännökseen tuli alueviite joka ilmoitti että aluetta ei löydy: https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/3-1-2/b1/en-GB

Hmm, eilen lokaalisti testaten mielestäni areat toimivat. Noiden sun testien perusteella näyttää siltä että tuon a1 dokumentin käännöksessä kummassakaan keississä nuo käännetyt aluelohkot ei enää saa noita eksplisiittisiä area ja area_end attribuutteja (vaikka niiden asettaminen käsin olin juuri se ruma korjaus millä aiemmin ne sai toimimaan). Ja varmaan nyt sen takia b1 käännöstä tehdessä/synkatessa niitä ei löydy.

Testasin nyt uudestaan noi 1-3-2 ja 3-1-2 tapaukset sekä lokaalisti että tuolla timdevsissä:

Noissa tapauksissa toimii nyt mielestäni kaikki viitteet niin kuin pitää. Jos vertaa niitä a1 käännöksiä näiden A:n käännöksiin niin noissa a1 käännöksissä tosiaan puuttuu ne area/area_end attribuutit, ja tämä on minulle ihan mysteeri. En keksi oikein muuta kuin että jotain on mennyt mönkään kun eilen päivitin koodia timdevs01-6:een. Koodi ei ole eilisestä kuitenkaan tämän osalta miksikään muuttunut, ainut ero oli että otin master-haarasta tuoreimmat päivitykset.

Testaan vielä varmuudeksi kaikki keissit uudestaan sekä lokaalisti että timdevsissa.

@sijualle
Copy link
Contributor

a1 käännöksissä tosiaan puuttuu ne area/area_end attribuutit

Sain toistettua tuon.

Jos A:ssa on alue ennen kuin A:sta tehdään käännös, A:n käännökseen tulee area_-attribuutit oikein. Tällöin myös nuo viitteet näyttävät menevän oikein: https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/a_areafirst1/en-GB ja https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/b_areafirst1/en-GB

Mutta jos A:sta on jo käännös, ja A:han lisää uuden arean, A:n käännökseen tulevista kappaleista puuttuu area_-attribuutit. Silloin toistuu tuo että areaviite hajoaa:
https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/a_areafter/en-GB ja https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/b_areaafter/en-GB

Eli ilmeisesti tuo arean jälkeenpäin lisääminen ei laita käännökseen noita area_-attribuutteja

@saviit saviit force-pushed the fix-translation-citation-pars branch from aae8d93 to 8205bb1 Compare August 31, 2023 13:51
@saviit
Copy link
Contributor Author

saviit commented Aug 31, 2023

Mutta jos A:sta on jo käännös, ja A:han lisää uuden arean, A:n käännökseen tulevista kappaleista puuttuu area_-attribuutit.

Joo, enpä huomannut tarkistaa että synkatessa nuo tulisi myös asetettua :)
Nyt on päivitetty myös timdevsiin.

@saviit
Copy link
Contributor Author

saviit commented Sep 1, 2023

Oman manuaalisen testaamisen perusteella tämä toimii nyt niin kuin lupaa, sekä tavallisten että alueviittausten osalta.

Testitapauksia ja valmiita testipohjia löytyy https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448, jos tulee mieleen jokin skenaario mitä ei ole tässä PR:ssä vielä huomioitu/korjattu/testattu.

Toistaiseksi korjaamattomia havaittuja ongelmia on kaksi, molemmat näistä ovat jo omina kortteinaan:
#3464
#3465

Nämä on mielestäni parempi korjata erikseen.

@saviit saviit linked an issue Sep 1, 2023 that may be closed by this pull request
@sijualle
Copy link
Contributor

sijualle commented Sep 1, 2023

Nyt toimi tuon että jos alueviite on B:ssä ennen B:n kääntämistä, niin alue menee oikein ( https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/areasource/en-GB -> https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/areatarget_2/en-GB )

Mutta tuota alueen jälkeenpäin lisäämistä en saanut vielä toimimaan. Jos tänne https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/areatarget/fi lisää jonkun alueen viitteenä esim täältä https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/areasource niin se tulee kääntämättömään sivuun, mutta käännöksiin ei. Normaalit kappaleviiteet tulee myös käännöksiin

@saviit
Copy link
Contributor Author

saviit commented Sep 1, 2023

Mutta tuota alueen jälkeenpäin lisäämistä en saanut vielä toimimaan. Jos tänne https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/areatarget/fi lisää jonkun alueen viitteenä esim täältä https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/asds/areasource niin se tulee kääntämättömään sivuun, mutta käännöksiin ei. Normaalit kappaleviiteet tulee myös käännöksiin

Joo, sain nyt toistettua. Tuo onkin nyt vähän kinkkisempi, sillä tämä näyttäis johtuvan tuosta miten estetään synkkausta lisäämästä samaa viitettä monta kertaa (+ korvataan alkuperäiskieliset käännöksillä).

@saviit saviit force-pushed the fix-translation-citation-pars branch from bb2ffb4 to b71160c Compare September 5, 2023 10:46
@saviit
Copy link
Contributor Author

saviit commented Sep 5, 2023

Jokohan nyt olisi. Tuorein myös timdevsissä.
https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/aaaa/a ja https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/aaaa/b perusteella pitäisi nyt toimia oikein.

@sijualle
Copy link
Contributor

sijualle commented Sep 6, 2023

Nyt toimi hyvin, vaikka lisäsi uudet areat jälkeenpäin niin ilmestyi oikeine viitteineen myös käännettyyn versioon

@dezhidki
Copy link
Member

dezhidki commented Sep 7, 2023

cite pars in translated documents will trigger readmark errors infinitely when mousehovering over the cited par

Hmm, tuo ei ole kyllä ihan hyvää, koska palvelinloki täyttyy noista readmark-lokeista aivan liikaa. Sanoiusin, että se pitäisi ehkä vielä jollain tavalla ratkaista, että ainakaan ei tulisi mitään ylimääräistä spammia.

Testasin nyt hieman tällä sivulla

https://timdevs01-6.it.jyu.fi/view/users/test-user-1/3448/1-2-3-toka/b/en-US#uusi-lohko-assa

Esimerkiksi lohko "Lohko 2" siirtyessä tulee devtoolsissa

PUT https://timdevs01-6.it.jyu.fi/read/240/6F6QWwotYSpS/2 400

Kun katsoo itse elementtia, niin tuo dokumentin ID viittaa käännösdokumenttiin, mutta lohko ID viittaa alkuperäiseen käännökseen:

image

Mutta jos katsoo tuon tim-par-ref-komponenttia, niin siinähän on ihan täysin oikein se lohko-ID (41ziw6S6tzs4).
Ja vielä kun katsoo sen komponentin toimintaa dokumentissa, niin se kyllä viittaa täysin oikein käännösdokumenttiin:

image

Eli siis voisiko tuon lohkon ref-id ja ref-t muuttaa toimimaan samalla logiikalla kuin tim-par-refin parid kuten olet tehnyt tässä PRssä:

{%- if t.attrs.rd and t.attrs.r == 'tr' and (t.attrs.rp or t.attrs.ra) -%}
<tim-par-ref docid="{{ t.attrs.rd }}"
{% if not t.attrs.ra -%}
parid="{{ t.attrs.rp }}">
{% else -%}
parid="{{ t.target.id }}">
{%- endif %}
</tim-par-ref>
{%- else -%}
<tim-par-ref
docid="{{ t.target.doc_id }}"
parid="{{ t.target.id }}">
</tim-par-ref>
{%- endif -%}

Jotain mallia

         {% if t.attrs.rd and t.attrs.r == 'tr' and t.attrs.rp %}
             ref-doc-id="{{ t.attrs.rd }}"
             ref-id="{{ t.attrs.rp }}"
             ref-t="{{ t.target.hash }}"
         {% elif t.target %}
             ref-id="{{ t.target.id }}"
             ref-t="{{ t.target.hash }}"
             ref-attrs="{{ t.target.attrs_str }}"
             {% if t.attrs.rd and t.attrs.rp and t.attrs.r == 'tr' %}
                 ref-doc-id="{{ t.attrs.rd }}"
             {% else %}
                 ref-doc-id="{{ t.target.doc_id }}"
             {% endif %}
         {% endif %}

Eli jos lohko on käännöslohko, niin napataan viite suoraan rp-arvosta.

Preserve citations instead of converting them into translated pars

Find and use corresponding citation pars from original's translations, if any
Add explicit area and area_end attrs to translated area paragraphs

Reference area citations correctly in translations
Fix adding area citations

Take area pars into account when matching between citation source and target
@saviit saviit force-pushed the fix-translation-citation-pars branch from 3684b70 to 6da6974 Compare September 7, 2023 08:23
@dezhidki dezhidki force-pushed the fix-translation-citation-pars branch from 6da6974 to 2455877 Compare September 7, 2023 08:38
Copy link
Member

@dezhidki dezhidki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Katsoin läpi ja myös minun puolestani OK. Vähän testasin timdevs02:ssa, ja tuntui, että käännöksiä omaavien dokumenttien muokkaus tuntuu ehkä vähän hitaammalta. En kuitenkaan ole mitannut asiaa, joten en osaa sanoa varmaksi.

Sitten tutkin kaikki tuotannon preamblet, jossa viitataan lohkoihin (54 kpl). Ainoastaan Ohj1 ja Ohj2 demojen preambleissa oli sellainen, että viitataan jossain toisessa dokumentissa oleviin plugineihin (esim. ks. https://tim.jyu.fi/view/kurssit/tie/ohj2/2021k/demot/templates/preambles/preamble/fi). Niissä kuitenkin viitattu plugin ei ole käännetty, joten vikaa ei ilmaannu.

Se on kuitenkin sellainen regressio, että se olisi hyvää korjata ennemmin kuin myöhemmin. Hyväksyn kuitenkin tämän PRn, sillä testini mukaan tuotannon dokumenttien editointi ja katsominen toimii normaalisti.

@saviit
Copy link
Contributor Author

saviit commented Sep 7, 2023

Vähän testasin timdevs02:ssa, ja tuntui, että käännöksiä omaavien dokumenttien muokkaus tuntuu ehkä vähän hitaammalta. En kuitenkaan ole mitannut asiaa, joten en osaa sanoa varmaksi.

Joo, tuossa tulee hidastumista väkisinkin kun muutoksia tallentaa, sillä nyt nuo viitteet käydään oikeasti kulkemassa läpi (ja tehdään tarvittavat kopioinnit/poistamiset). Aikaisemmin vain raa'asti kopioitiin/poistettiin ns. vanhempi-dokumentin lohkoja käännökselle, viitteillä ei sinällään tehty mitään. Jotain optimoinnin paikkaa tuossa varmasti olisi, jos ehtii käydä dokumenttilohkojen iterointeja läpi, nyt mennään aika suoraviivaisesti kun alunperin yritin saada tämän puristettua elokuun loppuun mennessä kasaan.

@dezhidki dezhidki merged commit ab35868 into master Sep 7, 2023
@dezhidki dezhidki deleted the fix-translation-citation-pars branch September 28, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants