OCA / delivery-carrier

Odoo Carriers And Deliveries Management
https://odoo-community.org/psc-teams/logistics-18
GNU Affero General Public License v3.0
110 stars 349 forks source link

RFC delivery carrier in v12 and above #192

Closed bealdav closed 2 years ago

bealdav commented 5 years ago

According different features implemented on carriers modules, I think we need to refactor several points like :

  1. remove carrier.account model https://github.com/OCA/delivery-carrier/blob/11.0/base_delivery_carrier_label/models/carrier_account.py to be replaced by server env or any equivalent features
  2. in the purpose to reduce useless code, always use package to define parcel (no more picking)
  3. open carriers features to the whole python ecosystem (or more) : other python ERP, other applications dealing with carriers, etc. An example has been made here http://akretion.github.io/roulier https://pypi.org/project/roulier
  4. other stuff

For now there is https://github.com/OCA/delivery-carrier/pull/187 to be merged. I don't want to block the merge by this RFC, just plan evolution

cc @Timo17100-c2c @yvaucher @guewen

bealdav commented 5 years ago

About 1. I propose to implement the same as that https://github.com/OCA/server-env/blob/12.0/mail_environment/models/ir_mail_server.py but for carrier.account . What do you think @yvaucher ? Can I make a PR on #187 ?

pedrobaeza commented 5 years ago

Please don't depend on all the *_environment stuff.

bealdav commented 5 years ago

In the whole ERP we need a good solution to manage credentials of external accounts. At Akretion we decided to use keychain (a version modified in v12) which itself based on server environment tools. C2C (and others) use server env only.

Please @pedrobaeza could you precise what is your generic option to deal with external credential accounts in the whole erp ? Thanks

pedrobaeza commented 5 years ago

You were talking about *_environment, not keychain. That's why I told you about not using that. keychain is good for keeping some secrets.

florian-dacosta commented 5 years ago

I guess the *_environment part can be put in a submodule then.

bealdav commented 5 years ago

That's OK for me to add another module. https://github.com/OCA/server-env/pull/22

bealdav commented 5 years ago
  1. @yvaucher @guewen @damdam-s are you open to modify base_delivery_label to avoid to deliver without package ?
yvaucher commented 5 years ago

@bealdav about your question on delivery without package. And following the call we had.

We might have some little things to tweak on the OCA/rma modules when returning goods. Nevertheless, it would make indeed more sense if we rely on pack only and not consider on itself picking as a pack. And we could enforce the creation of a pack if we print a label.

So let's go for labels generation only with packs.

One question about it, though. Do you still want a merged pdf of the labels on the picking? Or do you plan to have attachment only linked to the packages.

bealdav commented 5 years ago

@yvaucher thanks for your reply.

For labels, there several solutions. The easiest seems to be to provide a tree view of packages with an action from picking. Then selecting all records of the tree view you get all files in one click.

Duplicate info in picking is also possible with merged document.

angelmoya commented 5 years ago

There are not usual, but some users don't use packages, and want to use delivery carrier integration, in some ecommerce arround 90% of sales has only 1 product.

Picking has delivery address, and we can put tracking code on it to be shown to customer on portal.

On the other hand, now I ned to create parcel for more than one picking, to send only one package for two sales... in this project we have more then one sale order in the morning, and we send together.

A generic module to account could be great, on sprint code @jctrey is working on modules to get the tracking link for each carrier, and can use that as dependency.

Maybe we can merge #187 and then we can prepare this base module.

bealdav commented 5 years ago

Ok @angelmoya

1/ Our goal is to make user experience as easy as possible and make code as simple as possible for maintenance. But in odoo there several ways to do same thing just in the editor way. Then imagine, with all the uses cases of your cutomers and ours and these of other contributors, it can be over complicated for now and futures versions. Code comes from initial dev 6.1 version and needs to be review on some parts and try to become kiss (keep it simple as stupid) when possible. For that if user don't use package we have to :

2/ Your need to use a package for several picking is a very uncommon use case think. Maybe really specifics needs shouldn't determine common way to do things ? Problably you may think about merge picking before to deliver. At akretion we'll port https://github.com/OCA/sale-workflow/tree/8.0/sale_order_merge to v12. Our needs is to only merge picking not sale order (I imagine that's the same for you) On user side it seems confusing to share package across picking (which state of which object define you can add or not a product to package ?)

I hope it can help everybody to go the same solution

my 2 cents

bealdav commented 5 years ago

Please @angelmoya could you have a look to https://github.com/Timo17100-c2c/delivery-carrier/pull/1 I hope it can allow to use base_carrier_delivery_label for our both use Thanks

github-actions[bot] commented 2 years ago

There hasn't been any activity on this issue in the past 6 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this issue to never become stale, please ask a PSC member to apply the "no stale" label.