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] stock: product replenish wizard #38239

Closed
wants to merge 1 commit into from

Conversation

@sswapnesh
Copy link
Contributor

commented Oct 8, 2019

Description of the issue/feature this PR addresses: Fixes #38238

Current behavior before PR: default_get expects default_product_id or default_product_tmpl_id in the Context, while it was not passed in Action here.

Desired behavior after PR is merged: This commits adds default_product_id

Cc @sle-odoo

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@sle-odoo

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

in the original issue it is stated that it doesn't work with multiple product selected

by selecting multiple products in listview.

and it cannot work in the current code since product_id is a m2o on the wizard
i don't know if we can disable this action if multiple records are selected?

@sle-odoo

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

thanks for the PR btw 👍

@Feyensv

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

@sle-odoo there is a stupid multi attribute on ir.actions.report and ir.actions.act_window.view to disable the actions when on form view, but not to disable it on list view :D.
Not even sure it is really supported because nobody uses it in the code ^^.

multi = fields.Boolean(string='On Multiple Doc.', help="If set to true, the action will not be displayed on the right toolbar of a form view.")

@sswapnesh

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2019

@Feyensv I was going through the same. 😄

Maybe Raise Warning if multiple active_ids are there in default_get

@robodoo robodoo added the CI 🤖 label Oct 8, 2019
@sswapnesh

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2019

<field name="binding_view_types">form</field> does the job.

Reference: https://github.com/odoo/odoo/pull/24738/files


i don't know if we can disable this action if multiple records are selected?

@sle-odoo Disabled that on the list view.

sswapnesh added a commit to sswapnesh/odoo that referenced this pull request Oct 8, 2019
Followup on odoo#38239 (comment)
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Oct 8, 2019
@Feyensv
Feyensv approved these changes Oct 9, 2019
@Feyensv Feyensv changed the title [FIX] stock: Add Default product on replenish wizard [FIX] stock: product replenish wizard Oct 9, 2019
@Feyensv

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

@sswapnesh In fact, we could just remove the binding_model_id. The replenish button is already visible on the form views themselves as a button, there is no need to also have it in the Action tab.

Furthermore, it is bug-prone as you could open the replenish wizard on a service or consumable product, which doesn't have any sense.

cc: @sle-odoo

@sle-odoo

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

@Feyensv indeed, that would be the correct solution

@Swapnesh-SerpentCS

This comment has been minimized.

Copy link

commented Oct 9, 2019

In fact, we could just remove the binding_model_id

`
Will it be a stable change? 🤔

@Feyensv

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

Well, changing the binding_view_types or the context isn't more stable I think. But I'm not sure how far we can go for a "bug fix" 🤔

The replenish action shouldn't be accessible through the action dropdown 
(it makes no sense to replenish consumables or services).
It is already accessible on the product/template form when applicable.

Closes #38238
@Feyensv Feyensv force-pushed the sswapnesh:patch-16 branch from 0c5b4d6 to ba7409b Oct 9, 2019
@Feyensv Feyensv requested a review from sle-odoo Oct 9, 2019
@robodoo robodoo removed the CI 🤖 label Oct 9, 2019
@sle-odoo

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

robodoo r+
thanks guys

robodoo pushed a commit that referenced this pull request Oct 9, 2019
The replenish action shouldn't be accessible through the action dropdown 
(it makes no sense to replenish consumables or services).
It is already accessible on the product/template form when applicable.

Closes #38238

closes #38239

Signed-off-by: Simon Lejeune (sle) <sle@openerp.com>
@robodoo robodoo added the merging 👷 label Oct 9, 2019
robodoo pushed a commit that referenced this pull request Oct 9, 2019
The replenish action shouldn't be accessible through the action dropdown 
(it makes no sense to replenish consumables or services).
It is already accessible on the product/template form when applicable.

Closes #38238

closes #38239

Signed-off-by: Simon Lejeune (sle) <sle@openerp.com>
@robodoo robodoo added error 🙅 and removed r+ 👌 labels Oct 9, 2019
@robodoo

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

Staging failed: ci/runbot on c223de7194ed73090ee1eef4e2eff54e55ce61bc (view more at http://runbot.odoo.com/runbot/build/645903)

@Feyensv

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

@robodoo retry

@robodoo robodoo added CI 🤖 r+ 👌 and removed error 🙅 labels Oct 9, 2019
@sswapnesh

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2019

@Feyensv

you could open the replenish wizard on a service or consumable product, which doesn't have any sense.

IMO we should Raise Warning in that case (Maybe default_get)

@Feyensv

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

@sswapnesh No need, if we remove the binding_model_id, the Replenish action won't be shown in the 'Action' dropdown.
If you want to replenish a product, you need to use the button on the form view (which is only visible for storable products).

@sswapnesh

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2019

Oh, I see you have changed commit (and Removed Ownership as well 😉 )

@robodoo robodoo added the merging 👷 label Oct 9, 2019
@Feyensv

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

As I didn't keep the content of your commits, I didn't think it would matter :x.

@sswapnesh

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2019

Its Okay, I don't care for the Ownership (Specially in FOSS)

@robodoo robodoo closed this in 52d7008 Oct 9, 2019
@robodoo robodoo added merged 🎉 and removed merging 👷 labels Oct 9, 2019
@robodoo

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

Merged at 52d7008, thanks!

@sswapnesh sswapnesh deleted the sswapnesh:patch-16 branch Oct 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.