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

8.0 port module stock_reserve #8

Merged
merged 17 commits into from
Sep 11, 2014
Merged

Conversation

yvaucher
Copy link
Member

@yvaucher yvaucher commented Sep 3, 2014

No description provided.

@pedrobaeza
Copy link
Member

Can't you do this with reserved quants?

@yvaucher
Copy link
Member Author

yvaucher commented Sep 4, 2014

@pedrobaeza Here we want to be able to reserve a quantity of products manually and release them.
It also adds an expiration date on those reservations.

Here we want to be able to reserve some products from a specific location for a specific duration.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d593727 on yvaucher:8.0-stock_reserve into af9499e on OCA:8.0.

don't use half the quantity so that the reserved qty is different from
available qty
* check that multiple reservations are handled correctly
* check that various UOM are handled correctly
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7e81c6c on yvaucher:8.0-stock_reserve into af9499e on OCA:8.0.

test that reservations will trigger orderpoint procurements
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8061ad3 on yvaucher:8.0-stock_reserve into af9499e on OCA:8.0.

@gurneyalex
Copy link
Member

I've given the addon a good shake, added some tests and fixed an issue found on the way

LGTM 👍

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f475283 on yvaucher:8.0-stock_reserve into af9499e on OCA:8.0.


@api.multi
def action_view_reservations(self):
assert len(self._ids) == 1, "Expected 1 ID, got %r" % self._ids
Copy link
Member

Choose a reason for hiding this comment

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

Is the assertion necessary with the new api?
Also, if self._ids is a tuple, it will fail the string substitution so it should be put in a tuple (I don't know what self._ids is supposed to be so do not take my remark into account if it is irrelevant).

Copy link
Member

Choose a reason for hiding this comment

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

Please use the official self.ids instead.

Copy link
Member

Choose a reason for hiding this comment

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

in fact shouldn't it be @api.one?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it can be, although the meaning maybe it's not the same if he wants to limit the operation to one record each time.

Copy link
Member Author

Choose a reason for hiding this comment

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

@api.one would return a list of returned results I'll check if it works for an action

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's true, so the behaviour changes. It must be this way then

Copy link
Member Author

Choose a reason for hiding this comment

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

Just tested it, button action calling a method with decorator @api.one does nothing.

Copy link
Member

Choose a reason for hiding this comment

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

maybe this should be reported as an odoo bug, don't you think?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling a4e8885 on yvaucher:8.0-stock_reserve into af9499e on OCA:8.0.

@nbessi
Copy link

nbessi commented Sep 9, 2014

Effectivly it looks like a bug:

Decorate a record-style method where `self` is expected to be a 
singleton instance. The decorated method automatically loops on records,
and makes a list with the results. In case the method is decorated 
with  @returns, it concatenates the resulting instances

Maybe action button are explicitly decortated with multi and adding one has no effect. AFAIK there is not mutex test on those decorators.

@lepistone
Copy link
Member

The discussion on the decorator doesn't seem me to be this module's fault. So 👍, thanks!

@lepistone
Copy link
Member

Hi @pedrobaeza do you agree it's a problem in odoo and we can merge that one? thanks a lot!

@pedrobaeza
Copy link
Member

Yeah, sure. 👍

@lepistone
Copy link
Member

thanks!

lepistone added a commit that referenced this pull request Sep 11, 2014
@lepistone lepistone merged commit 974953e into OCA:8.0 Sep 11, 2014
@yvaucher yvaucher deleted the 8.0-stock_reserve branch September 11, 2014 15:39
guewen added a commit to camptocamp/stock-logistics-warehouse that referenced this pull request Apr 8, 2020
…mpletion_info

Rely on stock_move_common_dest as dependency
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