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

16.0 mig cooperator worker #528

Merged
merged 14 commits into from
Oct 25, 2023
Merged

16.0 mig cooperator worker #528

merged 14 commits into from
Oct 25, 2023

Conversation

robinkeunen
Copy link
Member

@robinkeunen robinkeunen commented Oct 10, 2023

Description

Odoo task (if applicable)

https://gestion.coopiteasy.be/web#id=10786&action=475&active_id=469&model=project.task&view_type=form&menu_id=536

Checklist before approval

  • Tests are present (or not needed).
  • Credits/copyright have been changed correctly.
  • Change log snippet is present.
  • (If a new module) Moving this to OCA has been considered.

@robinkeunen robinkeunen force-pushed the 16.0-mig-cooperator_worker branch 2 times, most recently from 097eea5 to 7f1a73a Compare October 12, 2023 09:29
@robinkeunen robinkeunen changed the base branch from 16.0 to 16.0-mig-eater October 12, 2023 09:30
@codecov-commenter
Copy link

Codecov Report

Merging #528 (7f1a73a) into 16.0-mig-eater (3d7f339) will increase coverage by 1.12%.
The diff coverage is 93.42%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@                Coverage Diff                 @@
##           16.0-mig-eater     #528      +/-   ##
==================================================
+ Coverage           67.58%   68.70%   +1.12%     
==================================================
  Files                  53       60       +7     
  Lines                1672     1748      +76     
  Branches              230      236       +6     
==================================================
+ Hits                 1130     1201      +71     
- Misses                500      502       +2     
- Partials               42       45       +3     
Files Coverage Δ
cooperator_worker/__init__.py 100.00% <100.00%> (ø)
cooperator_worker/models/__init__.py 100.00% <100.00%> (ø)
cooperator_worker/models/product_template.py 100.00% <100.00%> (ø)
cooperator_worker/tests/__init__.py 100.00% <100.00%> (ø)
cooperator_worker/tests/test_base.py 100.00% <100.00%> (ø)
cooperator_worker/tests/test_workers.py 100.00% <100.00%> (ø)
cooperator_worker/models/res_partner.py 83.33% <83.33%> (ø)

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.

Mostly LGTM, some comments.

In the future, it's easier (and standard OCA process) to create a separate migration commit for every version through which you are migrating. You don't need to test every version; the process is a development aid and helps future Git archaeologists figure out why some changes were made.

cooperator_worker/models/res_partner.py Outdated Show resolved Hide resolved
cooperator_worker/tests/test_base.py Outdated Show resolved Hide resolved
@@ -0,0 +1 @@
git+https://github.com/coopiteasy/cooperative@16.0-mig-cooperator-static-test-helpers#subdirectory=setup/cooperator
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make sure to delete this commit before merging.

Copy link
Member Author

Choose a reason for hiding this comment

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

@carmenbianca are you sure I should remove this commit before merging ? The corresponding PR is still open OCA/cooperative#86

(test-requirements here was updated)

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want this merged sooner rather than later, it's OK to merge as-is. I didn't realise this wasn't an OCA repo.

It's still not ideal (we have to remember to remove that line again), but it'll do.

@robinkeunen
Copy link
Member Author

So if I understand correctly, here, I should do

  • a 13 commit changing the version
  • a 14 commit changing the version
  • a 15 commit adapting the tests
  • a 16 commit changing the version

Correct ?

@robinkeunen robinkeunen changed the base branch from 16.0-mig-eater to 16.0 October 24, 2023 12:30
victor-champonnois and others added 12 commits October 24, 2023 17:00
[REF] Split beesdoo_emc -> cooperator_worker_configuration

[ADD] migration script

[UPD] Update cooperator_worker_configuration.pot

[REF] split beesdoo_emc -> cooperator_eater_configuration

[UPD] Update cooperator_eater_configuration.pot

[REF] beesdoo_emc -> cooperator_info_session

[UPD] Update cooperator_info_session.pot

[REM] auto-install

[IMP] cooperator_worker_configuration: add help message on is_worker
auto_install was added during the migration. It is a regression
both cooperator and beesdoo_shift modules can be used
separatly. Making the link is a choice by the user.
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
increment versions of dependent modules after renaming of beesdoo_shift
and beesdoo_worker_status.
@robinkeunen robinkeunen force-pushed the 16.0-mig-cooperator_worker branch 3 times, most recently from 0e7f62b to c1c0ca8 Compare October 24, 2023 15:21
@carmenbianca
Copy link
Collaborator

Basically yes @ your question on commits.

@robinkeunen
Copy link
Member Author

/ocabot merge nobump

@github-grap-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-528-by-robinkeunen-bump-nobump, awaiting test results.

@github-grap-bot github-grap-bot merged commit 6b84e3c into 16.0 Oct 25, 2023
2 checks passed
@github-grap-bot
Copy link
Contributor

Congratulations, your PR was merged at 913f549. Thanks a lot for contributing to beescoop. ❤️

@github-grap-bot github-grap-bot deleted the 16.0-mig-cooperator_worker branch October 25, 2023 08:49
@polchampion
Copy link
Collaborator

polchampion commented Jan 23, 2024

functionaly tested on 16-test-cooperator

  • Configure on share product whether its owner can work and shop
  • Partner fields is_worker and can_shop are computed from the share they subscribed to. NOK for can_shop, which is initially true on the partner even when it is false on the share product. However, this existing behaviour is not new to 16, and has been discussed here and the situation would require a change:

As is : As long as a partner who is a cooperator doesn't have a working mode, can_shop remains true.
To be in cooperator_worker : if is_worker is true, can_shop should only depend on the shift status. Else, the partner's can_shop should match the share product. Also, is the share product allows to work, it should also always allow to shop.

@carmenbianca I created an internal task to improve this.

@polchampion
Copy link
Collaborator

Also @carmenbianca I realise that this module is really a shift module, since it is a glue module that adds shift features to cooperator . Should it be renamed shift_cooperator_worker ?

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

7 participants