l10n-italy
l10n-italy copied to clipboard
[MIG] assets_management -> l10n_it_asset_management: Migration to 16.0
Migrazione da https://github.com/OCA/l10n-italy/tree/2f71528a4b249e2adf33d18108247c80f15f55b1/assets_management (14.0
).
Sostituisce https://github.com/OCA/l10n-italy/pull/3271.
Il commit successivo alla migrazione risolve https://github.com/OCA/l10n-italy/issues/3775 per 16.0
.
Risolve https://github.com/OCA/l10n-italy/issues/3009 per 16.0
.
Risolve https://github.com/OCA/l10n-italy/issues/3318 per 16.0
.
Risolve https://github.com/OCA/l10n-italy/issues/3851 per 16.0
.
Risolve https://github.com/OCA/l10n-italy/issues/3874 per 16.0
.
Risolve https://github.com/OCA/l10n-italy/issues/3170 per 16.0
.
Domani faccio test.
lo rinimini in l10n_it_asset_management?
Ah grazie mi sono dimenticato di scriverlo: rispetto a https://github.com/OCA/l10n-italy/pull/3271 non c'è più la rinomina del modulo. Non l'ho aggiunta perché non ho trovato nessuna discussione o motivazione per aggiungerla. Tu sai mica chi l'ha deciso e perché è stato deciso di rinominarlo?
credo per sottolineare la specificità italiana del modulo, visto che la precedente PR faceva così manterrei la rinomina del modulo.
@SirAionTech era stato discusso qua https://github.com/OCA/l10n-italy/pull/3155#issuecomment-1429345028 ed anche io lo ritengo corretto (anche solo per non buttare via il lavoro di @odooNextev )
@SirAionTech era stato discusso qua #3155 (comment)
Esatto, nello specifico la discussione era partita qui.
Aggiunta rinomina del modulo, provata con https://github.com/OCA/OpenUpgrade
@SirAionTech potrebbe essere la buona occasione di aggiungere una copertura di test?
@SirAionTech potrebbe essere la buona occasione di aggiungere una copertura di test?
Sì, se/quando ho tempo li aggiungo.
A occhio la mancanza grossa di copertura è sul codice dei report, quindi potrei partire dal porting di https://github.com/OCA/l10n-italy/pull/3010.
@SirAionTech potrebbe essere la buona occasione di aggiungere una copertura di test?
Sì, se/quando ho tempo li aggiungo.
A occhio la mancanza grossa di copertura è sul codice dei report, quindi potrei partire dal porting di #3010.
@francesco-ooops migliorata la copertura grazie al porting di #3010, anche se non è cambiata quanto speravo. Ora mi limiterei a fare modifiche solo se ci sono problemi dovuti alla migrazione in sé.
Ciao @SirTakobi quando creo un nuovo cespite a partire da una fattura di acquisto erroneamente riporta la data anche nel campo "dismiss date"
Ciao @SirTakobi quando creo un nuovo cespite a partire da una fattura di acquisto erroneamente riporta la data anche nel campo "dismiss date"
Grazie della revisione!
Abbiamo visto poco fa che il problema esiste anche in 14.0
, creo la issue (https://github.com/OCA/l10n-italy/issues/3851) e correggo.
I test falliscono per https://github.com/OCA/l10n-italy/issues/3801.
Il modulo è incompatibile con account_asset
di Odoo Enterprise?
Se sì, conviene scriverlo da qualche parte
Il modulo è incompatibile con
account_asset
di Odoo Enterprise? Se sì, conviene scriverlo da qualche parte
Mi rispondo: è incompatibile.
Entrambi aggiungono asset_ids
a account.move.line
e in account_asset
punta a account.asset
mentre in l10n_it_asset_management
punta a asset.asset
Domani faccio test.
@filipposaviori quando riesci :)
Il modulo è incompatibile con
account_asset
di Odoo Enterprise? Se sì, conviene scriverlo da qualche parteMi rispondo: è incompatibile. Entrambi aggiungono
asset_ids
aaccount.move.line
e inaccount_asset
punta aaccount.asset
mentre inl10n_it_asset_management
punta aasset.asset
Ho aperto la issue https://github.com/OCA/l10n-italy/issues/3952 per chiarire che il problema esisteva già in 14.0
.
Visto che è un problema indipendente dalla migrazione e si può correggere anche dopo, sarei per mergiare la migrazione così com'è se non ci sono problemi legati alla migrazione in sé, cosa ne pensi? Anche perché questa migrazione (contando anche https://github.com/OCA/l10n-italy/pull/3271) è in ballo da quasi un anno.
In un secondo momento poi si potrà
scriverlo da qualche parte
o anche risolverlo (in https://discord.com/channels/753902328494424064/753902328494424070/1205446526319591435 hai scritto che vi organizzate per farlo) per 16.0
e magari anche per 14.0
, come soluzione della issue https://github.com/OCA/l10n-italy/issues/3952.
@OCA/local-italy-maintainers potete fare review?
Visto che è un problema indipendente dalla migrazione e si può correggere anche dopo, sarei per mergiare la migrazione così com'è se non ci sono problemi legati alla migrazione in sé, cosa ne pensi? Anche perché questa migrazione (contando anche #3271) è in ballo da quasi un anno.
A proposito potresti aggiungere anche "Nextev Srl [email protected]" nei contributori?
Visto che è un problema indipendente dalla migrazione e si può correggere anche dopo, sarei per mergiare la migrazione così com'è se non ci sono problemi legati alla migrazione in sé, cosa ne pensi? Anche perché questa migrazione (contando anche #3271) è in ballo da quasi un anno.
A proposito potresti aggiungere anche "Nextev Srl [email protected]" nei contributori?
Come hai notato in https://github.com/OCA/l10n-italy/pull/3271#issuecomment-1866010598
@odooNextev chiudiamo?
La posso chiudere però pensavo di trovare un po' del lavoro che avevo fatto qui nella nuova, invece mi sembra si sia ripartiti da nuovo
sono ripartito da nuovo a fare la migrazione, perché l'ho trovato più semplice che correggere una per una tutte le cose che avevo segnalato in https://github.com/OCA/l10n-italy/pull/3271#pullrequestreview-1736629246.
Ho comunque aggiunto un commit per metterti tra i contributori :smile:
@SirAionTech ok grazie 👍 scusa, ma infatti non ricordavo, perchè eri ripartito più o meno da 0? C'erano troppi cherrypick da fare per allineare la 14.0?
perchè eri ripartito più o meno da 0? C'erano troppi cherrypick da fare per allineare la 14.0?
No, sono ripartito da 0 a fare la migrazione
perché l'ho trovato più semplice che correggere una per una tutte le cose che avevo segnalato in https://github.com/OCA/l10n-italy/pull/3271#pullrequestreview-1736629246.
Ciao @SirAionTech, come l'altra volta non riesco a creare una PR sul tuo branch 😅 Il branch è questo, ho apportato delle modifiche per assicurare la compatibilità di account_asset di Enterprise
Ciao @SirAionTech, come l'altra volta non riesco a creare una PR sul tuo branch 😅 Il branch è questo, ho apportato delle modifiche per assicurare la compatibilità di account_asset di Enterprise
ma se sono incompatibili, perche' li rendi compatibili?
Ciao @SirAionTech, come l'altra volta non riesco a creare una PR sul tuo branch 😅 Il branch è questo, ho apportato delle modifiche per assicurare la compatibilità di account_asset di Enterprise
In https://github.com/SirAionTech/l10n-italy/compare/16.0-mig-assets_management...LorenzoC0:l10n-italy:16.0-mig-assets_management non hai il pulsante per creare la pull request?
@SirAionTech
Non vedo la tua repository nel dropdown dei fork disponibili 🤔
@SirAionTech Non vedo la tua repository nel dropdown dei fork disponibili 🤔
Che strano, il link che ho messo in https://github.com/OCA/l10n-italy/pull/3776#issuecomment-1946094498 dovrebbe selezionare già tutto.
Anche aprendolo da incognito vedo il branch selezionato:
@SirAionTech Non vedo la tua repository nel dropdown dei fork disponibili 🤔
il filtro su "base: 14.0" invece che 16.0 non ha nulla a che vedere?
@SirAionTech Non vedo la tua repository nel dropdown dei fork disponibili 🤔
il filtro su "base: 14.0" invece che 16.0 non ha nulla a che vedere?
Nemmeno così, non capisco 🤔 Vedo comunque tutte le altre repo forkate se lascio la ricerca vuota
ma se sono incompatibili, perche' li rendi compatibili?
Perchè nei casi di DB multi country e multi localizzazioni per l'Italia serve l10n_it_asset_management
e per altre nazioni serve account_asset
, quindi devono poter funzionare insieme
ma se sono incompatibili, perche' li rendi compatibili?
Perchè nei casi di DB multi country e multi localizzazioni per l'Italia serve
l10n_it_asset_management
e per altre nazioni serveaccount_asset
, quindi devono poter funzionare insieme
Capito, grazie mille della spiegazione