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

[FIX] delivery: always permit pack weight compute #164677

Conversation

ethanrobv
Copy link
Contributor

Current behavior:
In a multi-company environment, say we have a reusable box which has been used by multiple companies. While the box actively contains some product of companyA, companyB is not permitted to view their own stock transfers.

Expected behavior:
The current status of a package should not affect the accessibility of a company's picking history.

Steps to reproduce:

  1. Setup 2 companies, for both:
    Enable packages
    Enable stock warehouse locations
    Enable multi-step routes -> set their in/out routes to 3-step (pick, pack, ship)

  2. Create a reusable box type package, don't assign it to either company

  3. In CompanyA, create a delivery using the reusable package and complete it so the package is fully emptied and ready to be reused

  4. Switch to CompanyB, create a picking (any kind) using the same reusable box -don't finish the transfer- then switch back to CompanyA

  5. Try to view Inventory transfers -> AccessError

Cause of the issue:
The delivery module adds the _compute_shipping_weight() method which is called on-demand when we try to open the transfers tree view. We will eventually look at packages from the picking that used the reusable package (which now 'belongs' to another company) and raise the AccessError.

Fix:
Use sudo() to read package records in the iteration over picking records.

We are only reading from pickings which belong to the current company, which makes the access check for the package records redundant (and as we see here problematic).

opw-3813917

@ethanrobv ethanrobv force-pushed the 15.0-opw-3813917-multi-company-access-package-records-in-use-etvi branch from 6deea52 to e9ac905 Compare May 7, 2024 08:31
**Current behavior:**
In a multi-company environment, say we have a reusable box which
has been used by multiple companies. While the box actively
contains some product of companyA, companyB is not permitted to
view their own stock transfers.

**Expected behavior:**
The current status of a package should not affect the
accessibility of a company's picking history.

**Steps to reproduce:**
1. Setup 2 companies, for both:
     Enable packages
     Enable stock warehouse locations
     Enable multi-step routes
     Set their in/out routes to 3-step (pick, pack, ship)

2. Create a reusable box type package, don't assign it to
     either company

3. In CompanyA, create a delivery using the reusable package and
     complete it so the package is fully emptied and ready to be
     reused

4. Switch to CompanyB, create a picking (any kind) using the
     same reusable box -don't finish the transfer- then switch
     back to CompanyA

5. Try to view Inventory transfers -> AccessError

**Cause of the issue:**
The delivery module adds the `_compute_shipping_weight()` method
which is called on-demand when we try to open the transfers tree
view. We will eventually look at packages from the picking that
used the reusable package (which now 'belongs' to another
company) and raise the AccessError.

**Fix:**
Use sudo() to read package records in the iteration over picking
records.

We are only reading from pickings which belong to the current
company, which makes the access check for the package records
redundant (and as we see here problematic).

opw-3813917
@ethanrobv ethanrobv force-pushed the 15.0-opw-3813917-multi-company-access-package-records-in-use-etvi branch from e9ac905 to 77850cb Compare May 7, 2024 08:32
@robodoo
Copy link
Contributor

robodoo commented May 7, 2024

Pull request status dashboard.

@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label May 7, 2024
Copy link
Member

@HANNICHE-Walid HANNICHE-Walid left a comment

Choose a reason for hiding this comment

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

LGTM

@HANNICHE-Walid HANNICHE-Walid marked this pull request as ready for review May 7, 2024 10:50
@C3POdoo C3POdoo requested a review from a team May 7, 2024 10:52
@vava-odoo vava-odoo requested review from a team and removed request for a team May 7, 2024 12:43
@Whenrow
Copy link
Contributor

Whenrow commented May 23, 2024

robodoo r+

robodoo pushed a commit that referenced this pull request May 23, 2024
**Current behavior:**
In a multi-company environment, say we have a reusable box which
has been used by multiple companies. While the box actively
contains some product of companyA, companyB is not permitted to
view their own stock transfers.

**Expected behavior:**
The current status of a package should not affect the
accessibility of a company's picking history.

**Steps to reproduce:**
1. Setup 2 companies, for both:
     Enable packages
     Enable stock warehouse locations
     Enable multi-step routes
     Set their in/out routes to 3-step (pick, pack, ship)

2. Create a reusable box type package, don't assign it to
     either company

3. In CompanyA, create a delivery using the reusable package and
     complete it so the package is fully emptied and ready to be
     reused

4. Switch to CompanyB, create a picking (any kind) using the
     same reusable box -don't finish the transfer- then switch
     back to CompanyA

5. Try to view Inventory transfers -> AccessError

**Cause of the issue:**
The delivery module adds the `_compute_shipping_weight()` method
which is called on-demand when we try to open the transfers tree
view. We will eventually look at packages from the picking that
used the reusable package (which now 'belongs' to another
company) and raise the AccessError.

**Fix:**
Use sudo() to read package records in the iteration over picking
records.

We are only reading from pickings which belong to the current
company, which makes the access check for the package records
redundant (and as we see here problematic).

opw-3813917

closes #164677

Signed-off-by: William Henrotin (whe) <whe@odoo.com>
@robodoo robodoo closed this May 23, 2024
@fw-bot
Copy link
Contributor

fw-bot commented May 27, 2024

@ethanrobv @Whenrow this pull request has forward-port PRs awaiting action (not merged or closed):

lohwswilson pushed a commit to lohwswilson/odoo that referenced this pull request Jun 3, 2024
**Current behavior:**
In a multi-company environment, say we have a reusable box which
has been used by multiple companies. While the box actively
contains some product of companyA, companyB is not permitted to
view their own stock transfers.

**Expected behavior:**
The current status of a package should not affect the
accessibility of a company's picking history.

**Steps to reproduce:**
1. Setup 2 companies, for both:
     Enable packages
     Enable stock warehouse locations
     Enable multi-step routes
     Set their in/out routes to 3-step (pick, pack, ship)

2. Create a reusable box type package, don't assign it to
     either company

3. In CompanyA, create a delivery using the reusable package and
     complete it so the package is fully emptied and ready to be
     reused

4. Switch to CompanyB, create a picking (any kind) using the
     same reusable box -don't finish the transfer- then switch
     back to CompanyA

5. Try to view Inventory transfers -> AccessError

**Cause of the issue:**
The delivery module adds the `_compute_shipping_weight()` method
which is called on-demand when we try to open the transfers tree
view. We will eventually look at packages from the picking that
used the reusable package (which now 'belongs' to another
company) and raise the AccessError.

**Fix:**
Use sudo() to read package records in the iteration over picking
records.

We are only reading from pickings which belong to the current
company, which makes the access check for the package records
redundant (and as we see here problematic).

opw-3813917

closes odoo#164677

Signed-off-by: William Henrotin (whe) <whe@odoo.com>
@fw-bot fw-bot deleted the 15.0-opw-3813917-multi-company-access-package-records-in-use-etvi branch June 6, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants