[10.0][MIG] website snippet country dropdown by HaTry · Pull Request #349 · OCA/website

HaTry · 2017-05-30T10:36:18Z

Migration of website snippet country dropdown to v10.0.

The option of insert this snippet as been removed.
Because of Odoo seems to even forbid dragging the snippet into a <form> element that is inside an
editable area.
Add test.

oca-clabot · 2017-05-30T10:36:21Z

Hey @HaTry, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

felix (no github login found)

Appreciation of efforts,
OCA CLAbot

yajo

Generally OK, but still needed to fix some comments.

Besides those, please squash the translation commits.

yajo · 2017-05-30T10:52:24Z

@@ -0,0 +1,2 @@
+
+import test


You don't have to import tests, this folder is autoimported when needed.

yajo · 2017-05-30T10:53:32Z

+    var country_code_test = 'ES';
+    var vat_number_test = 'B01010101';
+
+    tour.register("demo_page", {


Don't register the tour with this name. Tours share a global namespace, and demo_page seems like you are testing another thing. Better call it website_snippet_country_dropdown_tour_demo_page for example.

yajo · 2017-05-30T10:54:34Z

+        run: function(){
+            var complete_field = $("#complete_field").val();
+            if(complete_field !== country_code_test + vat_number_test)
+                console.log("error");


Since this is not very common, future reviewers will appreciate little comments about why you are doing this check in a function and why this console.log.

yajo · 2017-05-30T10:54:53Z

+                console.log("error");
+        }
+    }]);
+})


Configure your editor to always add a newline at the end of file, and do it.

yajo · 2017-05-30T10:56:19Z

@@ -0,0 +1,2 @@
+
+import test_snippet_country_dropdown


You need to put from . import before.

You also need the license boilerplate as you can see in the module template.

yajo · 2017-05-30T10:56:40Z

@@ -0,0 +1,12 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.


This license header is not accepted. Use the official one as I linked above.

yajo · 2017-05-30T10:56:58Z

+        self.phantom_js(
+            "/",
+            "odoo.__DEBUG__.services['web_tour.tour'].run('demo_page')",
+            "odoo.__DEBUG__.services['web_tour.tour'].tours.demo_page.ready"


You will have to change the tour name as above.

yajo · 2017-05-30T10:57:44Z

        <script type="text/javascript"
                src="/website_snippet_country_dropdown/static/src/js/website_snippet_country_dropdown.js"/>
+        <script type="text/javascript"
+                src="/website_snippet_country_dropdown/static/src/js/web_tour_country_dropdown.js"/>


Test tour assets should be added in a demo view to avoid overloading the main frontend.

yajo

Better, although still some things to fix.

yajo · 2017-05-30T12:25:59Z

+# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).

-import test
+from . import tests


Oh sorry I think I mistaked before in the comment. You don't have to import tests in the addon's __init__.py; never.
This module gets autoimported when running the unit tests.

yajo · 2017-05-30T12:27:12Z

        content: "Validate Text",
        trigger: '#no_country_field',
        run: function(){
+            // This funtion alow to evaluate a hide html element


"function"
"allow"
"hidden"

yajo · 2017-05-30T12:27:32Z

        trigger: '#no_country_field',
        run: function(){
+            // This funtion alow to evaluate a hide html element
+            // Imposible to do it though trigger


"Impossible"
"through"

yajo · 2017-05-30T12:28:03Z

+# © 2016 Jairo Llopis <jairo.llopis@tecnativa.com>
+# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
+
+from . import test_snippet_country_dropdown


Remember to add a newline character at the end of all files. Usually you can configure your text editor to do it automatically.

yajo · 2017-05-30T12:28:40Z

+        self.phantom_js(
+            "/",
+            "odoo.__DEBUG__.services['web_tour.tour'].run('website_snippet_country_dropdown_tour_demo_page')",
+            "odoo.__DEBUG__.services['web_tour.tour'].tours.website_snippet_country_dropdown_tour_demo_page.ready"


Limit to 79 chars/line. You'll probably have to join 2 strings to get to that.

OsoTranquilo

Tested in a local machine. O.K.

OsoTranquilo

Tested again in a local machine. O.K.
Last minor changes be good.

pedrobaeza · 2017-05-30T16:31:57Z

Is the CLA signed?

yajo · 2017-05-31T07:21:25Z

So I believe; @jbeficent?

yajo · 2017-05-31T08:26:35Z

I pushed a commit here to see if it fixes travis.

yajo requested changes May 30, 2017

View reviewed changes

pedrobaeza mentioned this pull request May 30, 2017

Migration to version 10.0 #262

Closed

35 tasks

OsoTranquilo approved these changes May 30, 2017

View reviewed changes

yajo and others added 3 commits May 30, 2017 18:18

Fix indentation of copyright header and remove UTF-8 characters.

86242ac

OCA Transbot updated translations from Transifex

ec4223f

[MIG] website_snippet_country_dropdown: Migration to 10.0

c836a5a

HaTry force-pushed the 10.0-mig-website_snippet_country_dropdown branch from 0e9abc9 to c836a5a Compare May 30, 2017 16:19

yajo approved these changes May 30, 2017

View reviewed changes

yajo self-assigned this May 31, 2017

yajo added the needs fixing label May 31, 2017

yajo added this to the 10.0 milestone May 31, 2017

Add sass and fix website_seo_redirection test.

3623658

pedrobaeza approved these changes Jun 9, 2017

View reviewed changes

pedrobaeza merged commit dabf2e2 into OCA:10.0 Jun 9, 2017

		@@ -0,0 +1,2 @@

		import test_snippet_country_dropdown No newline at end of file

		@@ -0,0 +1,12 @@
		# -- coding: utf-8 --
		# Part of Odoo. See LICENSE file for full copyright and licensing details.

Uh oh!

Conversation

HaTry commented May 30, 2017

Uh oh!

oca-clabot commented May 30, 2017

Uh oh!

yajo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yajo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OsoTranquilo left a comment

Choose a reason for hiding this comment

Uh oh!

OsoTranquilo left a comment

Choose a reason for hiding this comment

Uh oh!

pedrobaeza commented May 30, 2017

Uh oh!

yajo commented May 31, 2017

Uh oh!

yajo commented May 31, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants