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

11.0 mig website_sale_product_brand #238

Merged

Conversation

ernestotejeda
Copy link
Member

@ernestotejeda ernestotejeda commented Jun 4, 2018

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

You need to declare oca_dependencies.txt with product-attribute repo.
You should also use the new OCA readme system

is related to this brand.

The blog here explains the HOWTO :
http://www.serpentcs.com/serpentcs-odoo-ecommerce-shop-brands-contribution
Copy link
Member

Choose a reason for hiding this comment

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

Do this link give any relevant info? The README.rst info should be the more self contained possible

Copy link
Member

Choose a reason for hiding this comment

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

Indeed no links outside OCA

It will allow you to filter product based on its brand.

While shopping online, we have seen various eShops having a feature to shop by brands
which ODOO does not yet provide officially.Website Sale Product Brand fills the gap at certain
Copy link
Member

Choose a reason for hiding this comment

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

Need a space between officially. and Website

'category': 'e-commerce',
'author': "Serpent Consulting Services Pvt. Ltd.,"
"Odoo Community Association (OCA)",
'website': 'http://www.serpentcs.com',
Copy link
Member

Choose a reason for hiding this comment

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

============

To install this module, you need to install following module:
->https://github.com/OCA/product-attribute/tree/11.0/product_brand
Copy link
Member

Choose a reason for hiding this comment

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

Dependencies are already documented in the manifest file

Copy link
Member

Choose a reason for hiding this comment

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

That's not true. You have to indicate dependencies outside of the current repo, which is the case.

@pedrobaeza
Copy link
Member

@ernestotejeda you have to include in oca_dependencies.txt file in the root the repository product-attribute for having correct Travis and runbot.

@rafaelbn rafaelbn added this to the 11.0 milestone Jun 5, 2018
@rafaelbn rafaelbn self-requested a review June 5, 2018 05:03
Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

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

Set as WIP cc @Tecnativa

'name': 'Product Brand Filtering in Website',
'category': 'e-commerce',
'author': "Serpent Consulting Services Pvt. Ltd.,"
"Odoo Community Association (OCA)",
Copy link
Member

Choose a reason for hiding this comment

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

@JayVora-SerpentCS @pedrobaeza we are maintaining this module from v9 with fix + improvement + migrating. Do you think is fair to add here Tecnativa o not?

Copy link
Sponsor

Choose a reason for hiding this comment

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

Fine to add @rafaelbn ! Thanks.

@rafaelbn
Copy link
Member

rafaelbn commented Jun 5, 2018

cc @Tecnativa

@@ -0,0 +1,95 @@
.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg
Copy link

Choose a reason for hiding this comment

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

Png

Copy link
Member

Choose a reason for hiding this comment

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

Better to use new README by fragments system. @ernestotejeda check other of your PRs where I have done it.

@@ -0,0 +1,62 @@
<?xml version="1.0" encoding="utf-8"?>
<openerp>
Copy link

Choose a reason for hiding this comment

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

No description provided.

@rafaelbn
Copy link
Member

Please @ernestotejeda could you review this PR , we can't review it. Thanks! 😄

Copy link

@MeetKD MeetKD left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@pedrobaeza pedrobaeza force-pushed the 11.0-mig-website_sale_product_brand branch from 307bade to 2d94091 Compare June 13, 2018 08:37
@rafaelbn rafaelbn closed this Jun 13, 2018
@rafaelbn rafaelbn reopened this Jun 13, 2018
@@ -19,6 +19,10 @@
"security/ir.model.access.csv",
"views/product_brand.xml",
],
'demo': [
"demo/assets.xml",
"data/brand_data.xml"
Copy link
Member

Choose a reason for hiding this comment

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

Demo data goes into demo folder. Also please append trailing ,

var steps = [
{
trigger: "a[href='/page/product_brands']",
content: _t("Go to 'Product brand' page"),
Copy link
Member

Choose a reason for hiding this comment

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

If this tour is only for testing (not for guiding first-time users), then don't translate strings.

@@ -0,0 +1,4 @@
# -*- coding: utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

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

🔥

class UICase(HttpCase):
def setUp(self):
"""Ensure website lang is en_US."""
super(UICase, self).setUp()
Copy link
Member

Choose a reason for hiding this comment

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

🔥 This method does nothing? 🤔

Copy link

@JGarcia-Panach JGarcia-Panach left a comment

Choose a reason for hiding this comment

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

Fucntional review 👍

@OCA OCA deleted a comment from JGarcia-Panach Jun 13, 2018
Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

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

Please @ernestotejeda review technical commments, Functionally is OK 👍

<record id="product_brand_apple_demo" model="product.brand">
<field name="name">Apple</field>
</record>
</odoo>
Copy link
Member

Choose a reason for hiding this comment

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

Add an EOF empty line

<record id="product.product_product_9" model="product.product">
<field name="product_brand_id" ref="product_brand_apple_demo"/>
</record>
</odoo>
Copy link
Member

Choose a reason for hiding this comment

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

Add an EOF empty line

@pedrobaeza pedrobaeza force-pushed the 11.0-mig-website_sale_product_brand branch from eaa5741 to e23cb6d Compare June 20, 2018 22:26
@pedrobaeza
Copy link
Member

As @yajo's comments have been honored, I merge this.

@pedrobaeza pedrobaeza merged commit c5c39da into OCA:11.0 Jun 20, 2018
@pedrobaeza pedrobaeza mentioned this pull request Jun 20, 2018
22 tasks
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