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

[10.0][ADD]stock_picking_auto_revert #593

Closed

Conversation

AaronHForgeFlow
Copy link
Contributor

This module adds a button on pickings to revert a done picking. Basically, it will return all the done quantities, same as return wizard. And then it will duplicate the original picking.

Why to do this? Well, people make mistakes, a lot. And then fixing manually the picking is quite confusing and time consuming.

stock_cancel module is not good, because several reasons:

  • The quant history is not properly handled, and it is not possible to handled with the current design
  • If a user cancels and old picking, all the history should be recreated from the time the wrong picking was completed. This is not happening right now, and it will be probably very difficult to implement.
  • In newer versions, previous point become even worse, it will be impossible to track the quant affected by a specific stock_move_line, and the quant may have suffer changes at the time of cancelling.

When to use this module:

  • To fix users errors after completing pickings with mistakes.
  • The module will check the return can be done (all quantities available) otherwise will not let the user to continue and manual amended will be needed.

cc @ForgeFlow

@AaronHForgeFlow AaronHForgeFlow force-pushed the 10.0-add-stock_picking_auto_revert branch from 90cf0c7 to 6b9406a Compare December 12, 2019 15:08
@rousseldenis
Copy link
Sponsor Contributor

@AaronHForgeFlow As I can read, the idea behind code is quite the same as in stock_cancel.

Why not refactoring that one to avoid confusion (as there is already another one 'stock_picking_back2draft) ?

We could in that case upgrade module major version.

@scigghia @eLBati @pedrobaeza @yvaucher @alexis-via @rschnapka

@pedrobaeza
Copy link
Member

Well, if that's the case, then better to avoid modules with duplicated features.

@AaronHForgeFlow
Copy link
Contributor Author

Actually, the features are not duplicated.

  • stock_picking_back2draft handles confirmed pickings but not done pickings.
  • stock_picking_auto_revert handles done pickings, but not confirmed or draft pickings.

Now the question, is, should we add the features to the existing module instead of putting them in a separate module?

I feel like some companies may want the ability to set to draft pickings that are ready but not to do quick returns of the ones that are done. I would let the two modules separated.

Would that be fine?

@rousseldenis
Copy link
Sponsor Contributor

Actually, the features are not duplicated.

  • stock_picking_back2draft handles confirmed pickings but not done pickings.
  • stock_picking_auto_revert handles done pickings, but not confirmed or draft pickings.

Now the question, is, should we add the features to the existing module instead of putting them in a separate module?

I feel like some companies may want the ability to set to draft pickings that are ready but not to do quick returns of the ones that are done. I would let the two modules separated.

Would that be fine?

@AaronHForgeFlow For the duplicate features here in particular, I spoke about stock_cancel.

I mentioned the other modules in order to take care of duplicates in terms of understanding from end user.

So, could you think about improving code of stock_cancel ?

@AaronHForgeFlow
Copy link
Contributor Author

@rousseldenis The thing is, I can propose an amendment to stock_cancel in order to remove the correct stock_move from history.

However, I find impossible to make stock_cancel to work in newer versions. As there is not link between the move and the quant I find stock_cancel impossible to migrate in a way it does its job. That is why I prefer to make stock_cancel deprecated in this case.

Another option would be to do the amendment for stock_cancel for version 10, and then make the module deprecated in v11 and v12. Then I will propose stock_picking_auto_revert for those versions.

Would that work?

@rousseldenis
Copy link
Sponsor Contributor

@rousseldenis The thing is, I can propose an amendment to stock_cancel in order to remove the correct stock_move from history.

However, I find impossible to make stock_cancel to work in newer versions. As there is not link between the move and the quant I find stock_cancel impossible to migrate in a way it does its job. That is why I prefer to make stock_cancel deprecated in this case.

Another option would be to do the amendment for stock_cancel for version 10, and then make the module deprecated in v11 and v12. Then I will propose stock_picking_auto_revert for those versions.

Would that work?

stock_cancel is not yet migrated in v11/12, so IMHO you can change behaviour of stock_cancel in v10 and then let migration process 'FLOW' 😄

@AaronHForgeFlow
Copy link
Contributor Author

😄 it is fine for this version, but stock_cancel seems to thick to flow for too long 😁 I will propose the RFC for newer versions

@AaronHForgeFlow
Copy link
Contributor Author

Ok. Created the fix for stock_cancel here #594

Closing this, but brace yourself, coming soon in newer versions 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants