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

12.0 split beesdoo inventory #420

Merged
merged 12 commits into from
Aug 1, 2022
Merged

Conversation

robinkeunen
Copy link
Member

@robinkeunen robinkeunen commented Jun 24, 2022

  • Split beesdo_inventory into
    1. stock_picking_responsible
    2. stock_picking_max_shipping_date
    3. stock_picking_copy_quantity
  • Rename xml ids (no migration scripts, the views were never inherited)
  • Add translations for each module
  • Add test coverage
  • + minor refactorings
  • submit to the OCA.

fixes :

related PR :

Notes :

  • Hereunder, we discuss wether oca/stock_move_line_auto_fill can replace stock_picking_copy_quantity
  • RED CI can't adapt test-requirements.txt to include stock_picking_responsible since pip setup files are not generated on oca/stock-logistics-workflow

@robinkeunen robinkeunen marked this pull request as ready for review June 28, 2022 16:22
@robinkeunen robinkeunen force-pushed the 12.0-split-beesdoo_inventory branch from aa8a6cb to 37fbd5e Compare June 28, 2022 17:02
@robinkeunen
Copy link
Member Author

@carmenbianca I can't get the tests to pass because I can't adapt test-requirements.txt to include stock_picking_responsible since pip setup files are not generated on oca/stock-logistics-workflow . What do you suggest we do ?

cf OCA/stock-logistics-workflow#1026

@carmenbianca
Copy link
Collaborator

@robinkeunen You own that PR, so you can add the setup file yourself, no?

@robinkeunen
Copy link
Member Author

Ha yes, I had missed the setup directory on that repo. I'll try doing that.

@robinkeunen robinkeunen force-pushed the 12.0-split-beesdoo_inventory branch 2 times, most recently from d45fef1 to 2bb0900 Compare June 30, 2022 12:22
@robinkeunen
Copy link
Member Author

@victor-champonnois @carmenbianca this pr is open for reviews

@vdewulf this PR is open for tests on all databases

beedoo_inventory had four functionalities :

  1. adds responsible field,
    • moved to new module stock_picking_responsible
  2. add max_shipping_date field
  3. add button to copy quantity to stock pickings.
  4. restricts picking's available product to those sold by supplier as "main supplier"
    • nothing to test, will be done later

@vdewulf
Copy link

vdewulf commented Jul 11, 2022

@robinkeunen

Tested on lenid-test (lenid.test.coopiteasy.be)

  1. stock_picking_responsible => it's installed and there is a "responsible field" on the WH/IN (I created a PO, validated, and set the responsible from Administrator (by default) to Antonin. Antonin was added to the followers. Seems ok.

  2. button to copy quantity to stock pickings. It's removed, indeed. I installed oca/stock_move_line_auto_fill
    After installation, it's not possible to modify the quantity on the line of rht WH.IN directly (needs to click on the "burger" icon on the right to open a pop up where the quantity can be updated). This is annoying and we had this issue when migrating supermarket from v9 to v12. It's linked to the fact that users are set in the group "Gérer les emplacements multiples et les entrepôts"
    I removed the administrator from this group and I retrieve the "normal" WH/IN view.
    The button "remplissage automatique" is added my the module and works fine (the Fait column is filled in with the Demande initiale value).
    CONCLUSION: ok to add the OCA module but when installation occurs on customers database, we'll have a lot of complaints if we don't remove all the users from the group "Gérer les emplacements multiples et les entrepôts". This should be done by the scripts or functional people need to do it on each customer database after deployement.

  3. restricts picking's available product to those sold by supplier as "main supplier"
    Not tested.

@robinkeunen robinkeunen force-pushed the 12.0-split-beesdoo_inventory branch from 2bb0900 to 37e827c Compare July 11, 2022 20:13
@robinkeunen
Copy link
Member Author

@vdewulf merci pour les test

CONCLUSION: ok to add the OCA module but when installation occurs on customers database, we'll have a lot of complaints if we don't remove all the users from the group "Gérer les emplacements multiples et les entrepôts". This should be done by the scripts or functional people need to do it on each customer database after deployement.

I added to the migration script a bit of code to remove all users from group "stock.group_tracking_lot" aka "Manage Packages" sometimes translated to "Gérer les emplacements multiples et les entrepôts".

The change was deployed on the test server on all bases who had beesdoo_inventory installed.

@vdewulf
Copy link

vdewulf commented Jul 12, 2022

@robinkeunen can you tell me which DB are in this situation? I just went to Demain and Ouftcoop, they don't have beesdoo_inventory. I don't want to make it manually on all DB ;)

@robinkeunen
Copy link
Member Author

@vdewulf Here you are:

> ociedoo list-db --module beesdoo_inventory
bablmarket-test
bees-test
coopeco-test-backup
lasource-test
lenid-test
lenid-test-polln-shift
lepedalo-test
lepedalo-test-2022-06-28
lesptitspots-test
restore-bloum-test-5o351omr
spp-test
spp-test-product-state
spp-test-theo-2022-02-28
vervicoop-test

@vdewulf
Copy link

vdewulf commented Jul 12, 2022

@robinkeunen
I checked on the following database and there are still users in the "stock.group_tracking_lot" group :

  • spp-test
  • bees (265 users in test versus 5 in prod)

On this db, the group was empty:

  • babl market
    I stopped the control here, something went wrong.

@robinkeunen robinkeunen force-pushed the 12.0-split-beesdoo_inventory branch from 37e827c to a3c122e Compare July 13, 2022 10:21
@robinkeunen
Copy link
Member Author

@vdewulf
Alright, digged a bit deeper. The button is visible if the user is in any of these groups :

  • stock.group_tracking_lot : Manage Packages
  • stock.group_stock_multi_locations : Manage Multiple Stock Locations
  • stock.group_tracking_owner : Manage Different Stock Owners

I confused in stock.group_tracking_lot in my previous comment with Manage Multiple Stock Locations.

I adapted my script to remove all users from these three groups. It is being updated on all relevant databases right now.

@robinkeunen robinkeunen force-pushed the 12.0-split-beesdoo_inventory branch from a3c122e to e6b8d5b Compare July 13, 2022 12:01
@robinkeunen
Copy link
Member Author

@vdewulf the script to remove users from groups displaying the "burger icon" was run on all supermarket database, can you test again ?

Copy link
Collaborator

@carmenbianca carmenbianca left a comment

Choose a reason for hiding this comment

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

one comment, otherwise LGTM.

also left a review at OCA/stock-logistics-workflow#1026

"version": "12.0.1.0.1",
"version": "12.0.2.0.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to bump. ocabot does it for us.

Copy link
Member Author

Choose a reason for hiding this comment

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

I need to do it to control my migration scripts. I'll nobump merge it (or patch merge it ?) .

Copy link
Collaborator

Choose a reason for hiding this comment

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

ahh makes sense. merge nobump is ok

@robinkeunen
Copy link
Member Author

@vdewulf avant que je ne reprenne tout à zero, si tu peux valider ceci, je le mergerai déjà.

@vdewulf
Copy link

vdewulf commented Aug 1, 2022

@robinkeunen
I checked the group "Paramètres techniques + Gérer plusieurs emplacement de stock" on the following databases:

  • bablmarket-test => empty
  • bees-test ==> error when goiing into the database (KeyError: 'beesdoo.shift.sheet') + I don't think it's the DB you updated in the meantime.
  • lasource-test => no user ok
  • lenid-test => no user ok
  • lenid-test-polln-shift => not tested
  • lepedalo-test => ok
  • lepedalo-test-2022-06-28 => not tested
  • lesptitspots-test => not tested
  • restore-bloum-test-5o351omr => not tested
  • spp-test => ok
  • spp-test-product-state => not tested
  • spp-test-theo-2022-02-28 => not tested
  • vervicoop-test => ok

Conclusion: ok!

Just to make sure it's not forgotten, there is still this that has not been done yet:
restricts picking's available product to those sold by supplier as "main supplier"

@robinkeunen
Copy link
Member Author

@vdewulf thanks! The main supplier filter depends on another refactoring. It's in this issue #441

@robinkeunen robinkeunen merged commit 7068f9a into 12.0 Aug 1, 2022
@robinkeunen robinkeunen deleted the 12.0-split-beesdoo_inventory branch August 1, 2022 16:32
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.

4 participants