Skip to content

[4010][ADD] sales_team_security#104

Merged
yostashiro merged 11 commits into16.0from
4010-add-sales_team_security
Mar 30, 2024
Merged

[4010][ADD] sales_team_security#104
yostashiro merged 11 commits into16.0from
4010-add-sales_team_security

Conversation

@liuhehe1995
Copy link
Contributor

@liuhehe1995
Copy link
Contributor Author

This is a low priority task, I'll fix this failing checks later.

Copy link
Contributor

@kanda999 kanda999 left a comment

Choose a reason for hiding this comment

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

Functional review LGTM

@kanda999 kanda999 force-pushed the 4010-add-sales_team_security branch from f491eac to 30f12d1 Compare March 28, 2024 02:30
@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 4010-add-sales_team_security branch from 30f12d1 to c649804 Compare March 28, 2024 07:15
@AungKoKoLin1997
Copy link
Contributor

I just realized that tests from stock_picking_line_sequence are failed when we install mrp in env because of this latest update PR from odoo repo.
odoo/odoo#144276

@yostashiro
Copy link
Member

I just realized that tests from stock_picking_line_sequence are failed when we install mrp in env because of this latest update PR from odoo repo. odoo/odoo#144276

@AungKoKoLin1997 Does it actually break the functionality of stock_picking_line_sequence? Please follow up to fix the issue in the OCA repo.

@AungKoKoLin1997
Copy link
Contributor

I put auditlog module together with owner related modules in CI tests. Now the tests failed is from stock_picking_line_sequence. I still don't understand the reason of getting tests failed from sales_team_security if we put it together with auditlog. Its only happens in PR CI and there is no error when I test in dev env with these two modules together.

@yostashiro
Copy link
Member

Fixed the CI issue. Two points:

  1. Added odoo-addon-stock-picking-line-sequence @ git+https://github.com/qrtl/axls-oca.git@refs/pull/120/head#subdirectory=setup/stock_picking_line_sequence in test-requirements.txt.
  2. Updated .github/workflows/test.yml to seggregate sales_team_security from crm; the test setup of sales_team_secrity is incompatible with crm which adds a required field alias_id to the crm.team model.

Before merging this PR, we need to update test-requirements.txt once #120 is merged.

@AungKoKoLin1997
Copy link
Contributor

Updated .github/workflows/test.yml to seggregate sales_team_security from crm; the test setup of sales_team_secrity is incompatible with crm which adds a required field alias_id to the crm.team model.

@yostashiro I knew it is incompatible with crm but I doubt that this is the root case of tests failed because crm_exception runs after sales_team_security installed. Please let me test auditlog together with sales_team_security.

@AungKoKoLin1997
Copy link
Contributor

@yostashiro I think I was right. After I put together auditlog with sales_team_security, the tests are failed. I think there is no crm related modules and crm module is not installed in this CI. Then, you can also see the error message. That is not related with required fields alias_id.

@yostashiro yostashiro force-pushed the 4010-add-sales_team_security branch from 8741b92 to 36122b6 Compare March 30, 2024 14:56
@yostashiro
Copy link
Member

Thanks, Lin. I have reverted your last update and adjusted my comment in test.yml.

The incompatibility between sales_team_security and auditlog has been puzzling me, but let me go ahead and merge this as this has been causing hassles with gitaggregate, and with sales_team_security being a new module, it won't affect the production environment.

@yostashiro yostashiro merged commit b6a9a4e into 16.0 Mar 30, 2024
@yostashiro yostashiro deleted the 4010-add-sales_team_security branch April 3, 2024 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants