-
-
Notifications
You must be signed in to change notification settings - Fork 526
[16.0][ADD] quality_control_product_manufacturer: Added new module #1553
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
base: 16.0
Are you sure you want to change the base?
Conversation
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 the PR!
<span class="float-end text-end"><t | ||
t-esc="record.qty.value" | ||
/></span> | ||
<span class="float-end text-end"> | ||
<t t-esc="record.qty.value" /> | ||
</span> |
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.
suggestion: There is a lot of refactoring in this commit: if it is needed, please move it to a dedicated [REF]
commit so that it's more clear which changes are needed for the improvement that is being added.
Adding the name
attribute to the pages can be considered a refactoring too.
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.
Removed changes which were resulted as part of formatting XML. Keeping only changes relevant to requirement and hence used [IMP] tag in the commit for "quality_control_oca" module.
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.
👍
@@ -0,0 +1,15 @@ | |||
{ | |||
"name": "Quality Control Product Manufacturer OCA", | |||
"version": "16.0.0.0.1", |
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.
chore: The first version number should be 16.0.1.0.0
, see https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#12version-numbers:
The version number in the module manifest should be the Odoo major version (e.g. 12.0) followed by the module x.y.z version numbers. For example: 12.0.1.0.0 is expected for the first release of an 12.0 module.
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.
Updated version as required
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.
👍
|
||
class QcInspection(models.Model): | ||
_inherit = "qc.inspection" | ||
_description = "Quality control inspection with Manufacturer details" |
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.
issue: This is would overwrite the description of the model qc.inspection
.
If you want to keep this note, please add it as a class docstring or a simple comment.
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.
Updated using comment in the model file introduced
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.
👍
<xpath expr="//page[@name='notes']" position="after"> | ||
<page | ||
string="Manufacturer" | ||
attrs="{'invisible': [('object_id', '=', False), ('object_id', '!=', 'product.product')]}" |
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.
suggestion: Since the fields in this page depend on the selected product, it would make sense to show it only if there is a product (product_id
) selected, what do you think?
question: I'm not sure the field object_id
could ever be product.product
: because it is a Reference
field and they are stored as model.name,model_id
, could you please check if this condition is working as expected?
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.
Updated domain using "product_id" as well.
Please find answer in below comment related to "object_id" query
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.
👍
<field name="arch" type="xml"> | ||
<xpath expr="//page[@name='notes']" position="after"> | ||
<page | ||
string="Manufacturer" |
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.
suggestion: Add a name
attribute: it is useful for inheriting views (as you have just found out in this exact PR 😉)
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.
Yes, it was missed and I have updated now
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.
👍
<field name="manufacturer" /> | ||
<field name="manufacturer_code" /> |
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.
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.
Apologies for misleading information as part of commit message, which lead you to check under "Test" form for "Reference" field. So, I have updated as required.
The model "qc.inspection" has form view under "Inspection" where object_id can be found with "product.product". Runboat installation as below:
Tree view:
Also, search is achievable without using store attribute for fields. Please find image as below where test1 has "manufacturer_code" as 1234556 and test3 has "manufacturer_code" as "123456" and result is as expected.
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 the explanation! It looks fine then 👍
@@ -0,0 +1,15 @@ | |||
{ | |||
"name": "Quality Control Product Manufacturer OCA", |
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.
note: The _oca
suffix in the module's folder can be removed: it is normally used to avoid name clashing with Odoo's modules.
For instance, the module quality_control_oca
has the _oca
suffix so it doesn't clash with Odoo enterprise's module quality_control
, see #481 (comment).
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.
Thank for the detailed reference. Removed "_oca" prefix in the newly introduced module
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.
👍
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.
chore: The commit's title should start with [IMP]
instead of [ADD]
, see https://www.odoo.com/documentation/16.0/contributing/development/git_guidelines.html#tag-and-module-name:
- [ADD] for adding new modules;
- [IMP] for improvements: most of the changes done in development version are incremental improvements not related to another tag;
[...]
In OCA we also have [MIG]
(see https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message).
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.
[ADD] tag was wrongly used though I was aware of OCA convention for commit message. Will be cautious here on while making a commit message.
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.
👍
811174f
to
d997b83
Compare
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.
Looks great 👏 thanks!
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.
👍
<span class="float-end text-end"><t | ||
t-esc="record.qty.value" | ||
/></span> | ||
<span class="float-end text-end"> | ||
<t t-esc="record.qty.value" /> | ||
</span> |
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.
👍
@@ -0,0 +1,15 @@ | |||
{ | |||
"name": "Quality Control Product Manufacturer OCA", | |||
"version": "16.0.0.0.1", |
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.
👍
|
||
class QcInspection(models.Model): | ||
_inherit = "qc.inspection" | ||
_description = "Quality control inspection with Manufacturer details" |
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.
👍
<field name="arch" type="xml"> | ||
<xpath expr="//page[@name='notes']" position="after"> | ||
<page | ||
string="Manufacturer" |
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.
👍
<field name="manufacturer" /> | ||
<field name="manufacturer_code" /> |
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 the explanation! It looks fine then 👍
@@ -0,0 +1,15 @@ | |||
{ | |||
"name": "Quality Control Product Manufacturer OCA", |
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.
👍
<xpath expr="//page[@name='notes']" position="after"> | ||
<page | ||
string="Manufacturer" | ||
attrs="{'invisible': [('object_id', '=', False), ('object_id', '!=', 'product.product')]}" |
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.
👍
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.
Code review, LGTM!
@@ -66,6 +66,9 @@ def _compute_product_id(self): | |||
store=True, | |||
help="Product associated with the inspection", | |||
) | |||
product_image = fields.Image( | |||
related="product_id.image_1920", string="Product Image", store=True |
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.
question (non-blocking): is it necessary to store this field? This would store a copy of the image on every inspection, instead of fetching it from the product itself.
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 its not necessary to have store for this field, will verify once and update it accordingly.
This PR has the |
Added "product_image" field to display product image related to the product selected under "product_id" in the Inspection.
Added module "quality_control_product_manufacturer" to display manufacturer details with respect to "product_id" available from option selected from field "Reference" in the Inspection.
d997b83
to
59c16b1
Compare
Added a field under "quality_control_oca" module in "qc.inspection" model to display product image with respect to product_id available as part of selection made in reference field of the model.
Added new module which bridges "quality_control_oca" and "product_manufacturer" in order to display details about Manufacturer and Manufacturer Code related to product_id available as part of selection made in Reference field in the Inspection form.