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 add support for rental #164

Merged
merged 23 commits into from
Jun 21, 2015
Merged

8.0 add support for rental #164

merged 23 commits into from
Jun 21, 2015

Conversation

alexis-via
Copy link
Contributor

These rental modules were initially developed on https://code.launchpad.net/openerp-rental
So it's now time to move to github/OCA

Alexis de Lattre added 14 commits January 28, 2014 19:39
…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
@coveralls
Copy link

Coverage Status

Coverage decreased (-9.2%) to 74.15% when pulling d99cc5c on akretion:8.0-add-rental into 956a595 on OCA:8.0.

##############################################################################

from . import rental
from . import wizard
Copy link
Member

Choose a reason for hiding this comment

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

line 1 to 22 are not useful, just import no license mandatory, no explicit encoding, just ascii used
It's just a suggestion, you may keep as it if you want

@coveralls
Copy link

Coverage Status

Coverage decreased (-9.2%) to 74.15% when pulling b3217ed on akretion:8.0-add-rental into 956a595 on OCA:8.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-9.2%) to 74.15% when pulling b3217ed on akretion:8.0-add-rental into 956a595 on OCA:8.0.


In the menu *Warehouse > Configuration > Warehouses*, on the form view
of the warehouse, in the *Technical Information* tab, you will see two
additionnal stock locations: *Rental In* (stock of products to rent) and
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Replace "additionnal" with "additional".

@coveralls
Copy link

Coverage Status

Coverage decreased (-16.25%) to 67.1% when pulling d8c938f on akretion:8.0-add-rental into 956a595 on OCA:8.0.

@@ -27,6 +27,8 @@ virtualenv:

install:
- git clone https://github.com/OCA/maintainer-quality-tools.git ${HOME}/maintainer-quality-tools
- git clone https://github.com/OCA/account-closing.git ${HOME}/account-closing
- git clone https://github.com/OCA/server-tools.git ${HOME}/server-tools
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Those repositories should be added to oca_dependencies.txt and don't need to be added here, I think.

#: code:addons/sale_rental/rental.py:59
#, python-format
msgid "The rental product '%s' must have the option ''Must Have Start and End Dates' checked."
msgstr "The rental product '%s' must have the option ''Must Have Start and End Dates' checked."
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing translation

@bguillot
Copy link
Contributor

bguillot commented Jun 2, 2015

Thanks @alexis-via

👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-16.25%) to 67.1% when pulling bf10018 on akretion:8.0-add-rental into 956a595 on OCA:8.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-16.25%) to 67.1% when pulling bf10018 on akretion:8.0-add-rental into 956a595 on OCA:8.0.

'Wrong underlying model, should be product.product'
hw_product = self.env['product.product'].browse(
self.env.context['active_id'])
return _('RENT-%s') % hw_product.default_code
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

The stockable product that is used as a reference to create the service rental could have the default code set as empty. In that case the current code sets "RENT-false". I suggest to add the default code for the created rental service only if the associated product has a default_code set.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I can see that the default code is a required field in the wizard. Why is that, if the default_code field is not a required field at all in the product? What if the original product has no default 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
@alexis-via
Copy link
Contributor Author

@jbeficent I took into account your remarks. It deserves a 👍 :)

@alexis-via
Copy link
Contributor Author

@jbeficent Any other remarks ? We need a second positive review to merge this.

@JordiBForgeFlow
Copy link
Sponsor Member

I have some work in progress on tests and spanish translations. Will submit a separate PR.

Another remark, I tested with module 'stock_quant_manual_assign' from @pedrobaeza and it allows you to manually select the lot/serial that you want to rent. This is really a value added function, in cases where you rent serialized equipment.

@JordiBForgeFlow
Copy link
Sponsor Member

+1 to merge. Good work @alexis-via

@sebastienbeau
Copy link
Member

good work 👍

sebastienbeau added a commit that referenced this pull request Jun 21, 2015
@sebastienbeau sebastienbeau merged commit fe9ac6d into OCA:8.0 Jun 21, 2015
lmignon pushed a commit to acsone/sale-workflow that referenced this pull request Feb 11, 2021
Signed-off-by guewen
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

8 participants