Code review comment for lp:~bac/charms/precise/juju-gui/expose-cookies

Reviewers: mp+170118_code.launchpad.net,

Message:
Please take a look.

Description:
Expose 'use-analytics' via the charm.

https://code.launchpad.net/~bac/charms/precise/juju-gui/expose-cookies/+merge/170118

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/10395043/

Affected files:
   A [revision details]
   M config.yaml
   M config/config.js.template
   M hooks/backend.py
   M hooks/utils.py
   M revision
   M tests/test_utils.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision: <email address hidden>

Index: config.yaml
=== modified file 'config.yaml'
--- config.yaml 2013-06-12 08:59:15 +0000
+++ config.yaml 2013-06-18 15:38:18 +0000
@@ -140,3 +140,13 @@
        'add-apt-repository'.
      type: string
      default: ppa:juju-gui-charmers/stable
+ use-analytics:
+ description: |
+ The team developing the Juju GUI benefits from understanding how
+ different deployments use the tool. By enabling this setting,
+ anonymized usage data is reported back using Google Analytics. The
type
+ of data collected includes the charms that are deployed and the
number
+ of units per service. Use of analytics is optional but we hope you
will
+ allow us to improve our tool based on your experience.
+ type: boolean
+ default: true

Index: revision
=== modified file 'revision'
--- revision 2013-06-12 09:07:29 +0000
+++ revision 2013-06-18 15:39:35 +0000
@@ -1,1 +1,1 @@
-53
+54

Index: config/config.js.template
=== modified file 'config/config.js.template'
--- config/config.js.template 2013-04-26 21:05:03 +0000
+++ config/config.js.template 2013-06-18 15:38:18 +0000
@@ -22,5 +22,6 @@
    apiBackend: {{api_backend}}, // Value can be 'python' or 'go'.
    readOnly: {{readonly}},
    sandbox: {{sandbox}},
+ useAnalytics: {{use_analytics}},
    login_help: {{login_help}}
  };

Index: hooks/backend.py
=== modified file 'hooks/backend.py'
--- hooks/backend.py 2013-06-11 14:04:04 +0000
+++ hooks/backend.py 2013-06-18 15:38:18 +0000
@@ -108,7 +108,8 @@
              config['juju-gui-console-enabled'], config['login-help'],
              config['read-only'], config['staging'],
config['ssl-cert-path'],
              config['charmworld-url'], config['serve-tests'],
- secure=config['secure'], sandbox=config['sandbox'])
+ secure=config['secure'], sandbox=config['sandbox'],
+ use_analytics=config['use-analytics'])
          charmhelpers.open_port(80)
          charmhelpers.open_port(443)

Index: hooks/utils.py
=== modified file 'hooks/utils.py'
--- hooks/utils.py 2013-06-12 08:59:15 +0000
+++ hooks/utils.py 2013-06-18 15:38:18 +0000
@@ -360,7 +360,7 @@
  def start_gui(
          console_enabled, login_help, readonly, in_staging, ssl_cert_path,
          charmworld_url, serve_tests,
haproxy_path='/etc/haproxy/haproxy.cfg',
- config_js_path=None, secure=True, sandbox=False):
+ config_js_path=None, secure=True, sandbox=False,
use_analytics=False):
      """Set up and start the Juju GUI server."""
      with su('root'):
          run('chown', '-R', 'ubuntu:', JUJU_GUI_DIR)
@@ -400,6 +400,7 @@
          'protocol': json.dumps(protocol),
          'sandbox': json.dumps(sandbox),
          'charmworld_url': json.dumps(charmworld_url),
+ 'use_analytics': json.dumps(use_analytics),
      }
      if config_js_path is None:
          config_js_path = os.path.join(

Index: tests/test_utils.py
=== modified file 'tests/test_utils.py'
--- tests/test_utils.py 2013-06-11 14:04:04 +0000
+++ tests/test_utils.py 2013-06-18 15:38:18 +0000
@@ -637,7 +637,7 @@
          start_gui(
              False, 'This is login help.', True, True, ssl_cert_path,
              charmworld_url, True, haproxy_path='haproxy',
- config_js_path='config')
+ config_js_path='config', use_analytics=True)
          haproxy_conf = self.files['haproxy']
          self.assertIn('ca-base {0}'.format(ssl_cert_path), haproxy_conf)
          self.assertIn('crt-base {0}'.format(ssl_cert_path), haproxy_conf)
@@ -655,6 +655,7 @@
          self.assertIn("socket_url: 'wss://", js_conf)
          self.assertIn('socket_protocol: "wss"', js_conf)
          self.assertIn('charmworldURL: "http://charmworld.example"',
js_conf)
+ self.assertIn('useAnalytics: true', js_conf)
          apache_conf = self.files['juju-gui']
          self.assertIn('juju-gui/build-', apache_conf)
          self.assertIn('VirtualHost *:{0}'.format(WEB_PORT), apache_conf)
@@ -687,6 +688,16 @@
          self.assertIn('user: "admin"', js_conf)
          self.assertIn('password: "admin"', js_conf)

+ def test_start_gui_no_analytics(self):
+ ssl_cert_path = '/tmp/certificates/'
+ charmworld_url = 'http://charmworld.example'
+ start_gui(
+ False, 'This is login help.', False, False, ssl_cert_path,
+ charmworld_url, True, haproxy_path='haproxy',
+ config_js_path='config', use_analytics=False)
+ js_conf = self.files['config']
+ self.assertIn('useAnalytics: false', js_conf)
+

  class TestNpmCache(unittest.TestCase):
      """To speed building from a branch we prepopulate the NPM cache."""