RedTurtle / redturtle.volto

Helper package to setup a RedTurtle's Plone site ready to work with Volto
GNU General Public License v2.0
1 stars 1 forks source link

[fix] add adapter for fix relationfield deserializer #86

Closed eikichi18 closed 8 months ago

eikichi18 commented 9 months ago

Problema con il deseriazer dei relationfield perché di base di andava a prendere l'id di un oggetto, che veniva poi utilizzato come path, per poi utilizzarlo all'interno di un restrictedTraverse per andare a recuperarsi l'oggetto.

Questa cosa però creava problemi nel momento in cui degli utenti avevano accesoo solo a certe sezioni del sito non potendo accedere a quelle superiori

eikichi18 commented 9 months ago

le cofigurazioni di flake e black di questo pacchetto vanno in conflitto

coveralls commented 9 months ago

Pull Request Test Coverage Report for Build 7340529013


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/redturtle/volto/restapi/deserializer/relationfield.py 40 46 86.96%
<!-- Total: 40 46 86.96% -->
Totals Coverage Status
Change from base Build 6907137559: 0.8%
Covered Lines: 843
Relevant Lines: 1318

💛 - Coveralls
eikichi18 commented 9 months ago

@cekk per quanto riguarda i test, io penso che i test di plone.restapi all'interno di test_dxfield_deserializer per quando riguarda i campi relazione, coprano tutte le casistiche.

cekk commented 9 months ago

@cekk per quanto riguarda i test, io penso che i test di plone.restapi all'interno di test_dxfield_deserializer per quando riguarda i campi relazione, coprano tutte le casistiche.

però non intercettavano il problema che hai sistemato tu, o sbaglio?

eikichi18 commented 9 months ago

è vero, ma con la modifica che abbiamo fatto il problema non è più replicabile, dalle prove che ho fatto non arriviamo più li con un path

cekk commented 9 months ago

si ma il test serve proprio per mostrare quello che hai sistemato. In teoria dovresti farlo prima del fix, in modo che si rompa. Poi metti il fix e vedi che diventa verde. Così poi lo prendi e lo metti anche nella pr di restapi (c'è?)

eikichi18 commented 9 months ago

Su plone.restapi c'è il test che prova il funzionamento del deserializer del RelationField.

L'unico modo che mi viene in mente per testare l'errore attualmente, sarebbe mockure il deserializer per farlo rompere nel test e poi usare quello modificato per mostrare che adesso lo fa bene.

Mentre secondo me i test all'interno di plone.restapi, che testano già quel deserializer, dovrebbero già coprire la funzionalità che abbiamo modificato.

cekk commented 9 months ago

allora non la testano bene perché avrebbero dovuto rompersi ;) E' questo quello che intendo: se siete arrivati a replicare questo errore, vuol dire che ci si può scrivere sopra un test (che evidentemente prima non lo copriva). Facendolo girare senza il fix deve rompersi, mentre aggiungendo il fix, dare luce verde

mamico commented 9 months ago

E' una funzionalità nostra o una fix per plone.restapi? Nel secondo caso serve una pull request su quel prodotto.

eikichi18 commented 9 months ago

Io questa la farei come fix su plone.restapi, però intanto ho bisogno di rilasciarla per alcuni clienti che sono bloccati