-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[11.0] [MIG] sale_rental #819
Conversation
…the same SO as the rentals = the start date of the rentals, because we suppose that these products are "accessories" of the rental. Re-organise the order of some lines of code to make it more "logic".
Add list of limitations in the module description.
Add full FR translation for sale_start_end_dates Add partial FR translation for sale_rental
This bug was very annoying because it means that, if you had sale_rental installed, you are missing the link betweek purchase order lines and invoice lines (for purchase orders created from pickings)
…ew of sale order line, and instructions on how to do it Add demo data to automatically add admin and demo to group sale.group_mrp_properties, to have access to form view of sale order lines.
Add a link to the screencast in the module description
…y of the rental features
Fix translated string in code
Add option to copy image from product to rental service default_code should not be a required field in rental service wizard Add help message on rental_qty field
Replace openerp by odoo in import declarations
Fixed in commit #819 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functional review 👎
Can not create Rental service in Rental tab, an error is shown:
Traceback (most recent call last): File "/.repo_requirements/odoo/odoo/http.py", line 653, in _handle_exception return super(JsonRequest, self)._handle_exception(exception) File "/.repo_requirements/odoo/odoo/http.py", line 312, in _handle_exception raise pycompat.reraise(type(exception), exception, sys.exc_info()[2]) File "/.repo_requirements/odoo/odoo/tools/pycompat.py", line 87, in reraise raise value File "/.repo_requirements/odoo/odoo/http.py", line 695, in dispatch result = self._call_function(**self.params) File "/.repo_requirements/odoo/odoo/http.py", line 344, in _call_function return checked_call(self.db, *args, **kwargs) File "/.repo_requirements/odoo/odoo/service/model.py", line 97, in wrapper return f(dbname, *args, **kwargs) File "/.repo_requirements/odoo/odoo/http.py", line 337, in checked_call result = self.endpoint(*a, **kw) File "/.repo_requirements/odoo/odoo/http.py", line 939, in __call__ return self.method(*args, **kw) File "/.repo_requirements/odoo/odoo/http.py", line 517, in response_wrap response = f(*args, **kw) File "/home/odoo/OCB-11.0/addons/web/controllers/main.py", line 938, in call_button action = self._call_kw(model, method, args, {}) File "/home/odoo/OCB-11.0/addons/web/controllers/main.py", line 926, in _call_kw return call_kw(request.env[model], method, args, kwargs) File "/.repo_requirements/odoo/odoo/api.py", line 689, in call_kw return call_kw_multi(method, model, args, kwargs) File "/.repo_requirements/odoo/odoo/api.py", line 680, in call_kw_multi result = method(recs, *args, **kwargs) File "/home/odoo/build/OCA/sale-workflow/sale_rental/wizard/create_rental_product.py", line 77, in create_rental_product product = pp_obj.create(self._prepare_rental_product()) File "/home/odoo/build/OCA/sale-workflow/sale_rental/wizard/create_rental_product.py", line 53, in _prepare_rental_product day_uom_id = self.ref('product.product_uom_day') AttributeError: 'create.rental.product' object has no attribute 'ref'
Fixed in commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functional review.
LGTM 👍
sale_rental/models/stock.py
Outdated
'stock.location.route', string='Sell Rented Product Route') | ||
|
||
@api.onchange('rental_allowed') | ||
def _onchange_rrental_allowed(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This double "r" is a typo, right ?
Thanks @danhvophuong for your great work! How about getting this one merged? Or is there more to be fixed than just the "r" typo @alexis-via found? |
just "r" typo, I will fix it soon |
Great! and thanks for the good work! |
Hey @danhvophuong, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
Hey @danhvophuong it looks like there is some prob with the CLA, but as I saw you have already a merged pr so I assume there is a prob on OCA side. Can you confirm that you have signed the CLA, so @pedrobaeza can check this? Thanks |
Confirmed, I already signed CLA with email [email protected]. I am working on another environment so the configuration can be different |
So, when I see things right we should get a 'go' from @nikul-serpentcs and @alexis-via to merge this one. Thanks for a fast rereview... :-) |
@api.multi | ||
def _get_rental_date_planned(self): | ||
self.ensure_one() | ||
start_date = datetime.strptime( | ||
self.start_date, DEFAULT_SERVER_DATE_FORMAT) | ||
return start_date.strftime(DEFAULT_SERVER_DATETIME_FORMAT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your great work. I have just one suggestion after my review. Instead of "strptime" and "strftime" You can use functions "from_string" and "to_string" of fields.Datetime and fields.Date.
sale_rental/models/sale_order.py
Outdated
if line.rental_type == 'new_rental': | ||
sale_rental = self.env['sale.rental'].search( | ||
[('start_order_line_id', '=', line.id)], limit=1) | ||
sale_rental.out_move_id._action_cancel() | ||
sale_rental.in_move_id._action_cancel() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moves for rental will be cancelled in super function. You don't need to do it again.
sale_rental/models/sale_order.py
Outdated
if line.rental_type == 'new_rental': | ||
vals = { | ||
'company_id': line.order_id.company_id, | ||
'group_id': procurement_group, | ||
'sale_line_id': line.id, | ||
'date_planned': line._get_rental_date_planned(), | ||
'route_ids': line.order_id.warehouse_id.rental_route_id, | ||
'warehouse_id': line.order_id.warehouse_id or False, | ||
'partner_dest_id': line.order_id.partner_shipping_id | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please move this part into a new function of "sale.order.line", so that it can be extended easily.
@yweng8111 the adjustment is pushed |
sale_rental/models/sale_order.py
Outdated
line.order_id.procurement_group_id = procurement_group | ||
|
||
if line.rental_type == 'new_rental': | ||
vals = self.prepare_rental_values(procurement_group) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here should be
vals = line.prepare_rental_values(procurement_group)
Otherwise we will get the singleton Error, if the sale order has more than 1 sale order lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
@nikul-serpentcs @alexis-via Could you update your review? |
@danhvophuong squash commits |
Closing this as too old. Please feel free to reopen it if needed. |
No description provided.