public inbox for [email protected]  
help / color / mirror / Atom feed
[pgAdmin4][RM#3037] Allow user to disable Gravatar image
11+ messages / 3 participants
[nested] [flat]

* [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-05 08:12  Murtuza Zabuawala <[email protected]>
  0 siblings, 2 replies; 11+ messages in thread

From: Murtuza Zabuawala @ 2018-03-05 08:12 UTC (permalink / raw)
  To: pgadmin-hackers

Hi,

PFA patch to disable Gravatar image in Server mode.

Requirments & Issues:
- For security reasons.
- For systems which do not have internet access.
- Hangs pgAdmin4 while loading the page if connection has no internet
access (as described in the ticket)

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Attachments:

  [application/octet-stream] RM_3037.diff (7.5K, 3-RM_3037.diff)
  download | inline diff:
diff --git a/web/config.py b/web/config.py
index a728334..d4ddd9d 100644
--- a/web/config.py
+++ b/web/config.py
@@ -357,6 +357,11 @@ SQLALCHEMY_TRACK_MODIFICATIONS = False
 ON_DEMAND_RECORD_COUNT = 1000
 
 ##########################################################################
+# Allow users to display Gravatar image for their username in Server mode
+##########################################################################
+SHOW_GRAVATAR_IMAGE = True
+
+##########################################################################
 # Local config settings
 ##########################################################################
 
diff --git a/web/pgadmin/browser/__init__.py b/web/pgadmin/browser/__init__.py
index ca1edd0..2111df6 100644
--- a/web/pgadmin/browser/__init__.py
+++ b/web/pgadmin/browser/__init__.py
@@ -730,18 +730,18 @@ class BrowserPluginModule(PgAdminModule):
 @login_required
 def index():
     """Render and process the main browser window."""
-    # Get the Gravatar
-    Gravatar(
-        current_app,
-        size=100,
-        rating='g',
-        default='retro',
-        force_default=False,
-        use_ssl=True,
-        base_url=None
-    )
+    # Register Gravatar module with the app only if required
+    if config.SHOW_GRAVATAR_IMAGE:
+        Gravatar(
+            current_app,
+            size=100,
+            rating='g',
+            default='retro',
+            force_default=False,
+            use_ssl=True,
+            base_url=None
+        )
 
-    msg = None
     # Get the current version info from the website, and flash a message if
     # the user is out of date, and the check is enabled.
     if config.UPGRADE_CHECK_ENABLED:
@@ -761,7 +761,7 @@ def index():
             if response.getcode() == 200:
                 data = json.loads(response.read().decode('utf-8'))
                 current_app.logger.debug('Response data: %s' % data)
-        except Exception as e:
+        except Exception:
             current_app.logger.exception('Exception when checking for update')
 
         if data is not None:
diff --git a/web/pgadmin/browser/static/css/browser.css b/web/pgadmin/browser/static/css/browser.css
index 2fffd64..3ba330d 100644
--- a/web/pgadmin/browser/static/css/browser.css
+++ b/web/pgadmin/browser/static/css/browser.css
@@ -62,3 +62,7 @@ samp,
 .sql-editor-grid-container {
   font-family: 'Open Sans' !important;
 }
+
+.pg-login-icon {
+  font-size: 16px;
+}
diff --git a/web/pgadmin/browser/templates/browser/index.html b/web/pgadmin/browser/templates/browser/index.html
index 58ff43f..76c3e4c 100644
--- a/web/pgadmin/browser/templates/browser/index.html
+++ b/web/pgadmin/browser/templates/browser/index.html
@@ -1,5 +1,13 @@
 {% extends "base.html" %}
+
+{% if config.SERVER_MODE and config.SHOW_GRAVATAR_IMAGE -%}
+{% import 'browser/macros/gravatar_icon.macro' as IMG with context %}
+{% elif config.SERVER_MODE %}
+{% import 'browser/macros/static_user_icon.macro' as IMG with context %}
+{% endif %}
+
 {% block title %}{{ config.APP_NAME }}{% endblock %}
+
 {% block init_script %}
 try {
 require(
@@ -66,9 +74,11 @@ require.onResourceLoad = function (context, map, depMaps) {
     }, 400)
   }
 };
+
+{% if config.SERVER_MODE %}
 window.onload = function(e){
  setTimeout(function() {
-   var gravatarImg = '<img src="{{ username | gravatar }}" width="18" height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span class="caret"></span>';
+   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
    //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
    var navbarRight = document.getElementById("navbar-menu").getElementsByClassName("navbar-right")[0];
    if (navbarRight) {
@@ -77,8 +87,9 @@ window.onload = function(e){
    }
  }, 1000);
 };
-
+{% endif %}
 {% endblock %}
+
 {% block body %}
 <style>
 #pg-spinner {
diff --git a/web/pgadmin/browser/templates/browser/macros/gravatar_icon.macro b/web/pgadmin/browser/templates/browser/macros/gravatar_icon.macro
new file mode 100644
index 0000000..72ec97e
--- /dev/null
+++ b/web/pgadmin/browser/templates/browser/macros/gravatar_icon.macro
@@ -0,0 +1,8 @@
+{##########################################################################
+We wrote separate macro because if user choose to disable Gravatar then
+we will not associate our application with Gravatar module which will make
+'gravatar' filter unavailable in Jinja templates
+###########################################################################}
+{% macro PREPARE_HTML() -%}
+'<img src = "{{ username | gravatar }}" width = "18" height = "18" alt = "Gravatar image for {{ username }}" > {{ username }} <span class="caret"></span>';
+{%- endmacro %}
diff --git a/web/pgadmin/browser/templates/browser/macros/static_user_icon.macro b/web/pgadmin/browser/templates/browser/macros/static_user_icon.macro
new file mode 100644
index 0000000..6b72844
--- /dev/null
+++ b/web/pgadmin/browser/templates/browser/macros/static_user_icon.macro
@@ -0,0 +1,3 @@
+{% macro PREPARE_HTML() -%}
+'<i class="fa fa-user-circle pg-login-icon" aria-hidden="true"></i> {{ username }} <span class="caret"></span>';
+{%- endmacro %}
diff --git a/web/pgadmin/browser/tests/test_gravatar_image_display.py b/web/pgadmin/browser/tests/test_gravatar_image_display.py
new file mode 100644
index 0000000..a4526c9
--- /dev/null
+++ b/web/pgadmin/browser/tests/test_gravatar_image_display.py
@@ -0,0 +1,68 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+import config
+from pgadmin.utils.route import BaseTestGenerator
+from regression.python_test_utils import test_utils as utils
+from regression.test_setup import config_data as tconfig
+
+
+class TestLoginUserImage(BaseTestGenerator):
+    """
+    This class checks for user image after successful login.
+    - If SHOW_GRAVATAR_IMAGE config option is set to True then we will show
+    Gravatar on the Page.
+    - If SHOW_GRAVATAR_IMAGE config option is set to False then we will show
+    Static image on the Page.
+    """
+
+    scenarios = [
+        (
+            'Verify gravatar image on the page', dict(
+                email=(
+                    tconfig['pgAdmin4_login_credentials']['login_username']
+                ),
+                password=(
+                    tconfig['pgAdmin4_login_credentials']['login_password']
+                ),
+                respdata='Gravatar image for %s' %
+                         tconfig['pgAdmin4_login_credentials']
+                         ['login_username'],
+            )
+        )
+    ]
+
+    @classmethod
+    def setUpClass(cls):
+        "Logout first if already logged in"
+        utils.logout_tester_account(cls.tester)
+
+    def runTest(self):
+        # Login and check type of image in response
+        response = self.tester.post(
+            '/login', data=dict(
+                email=self.email,
+                password=self.password
+            ),
+            follow_redirects=True
+        )
+        # Should have gravatar image
+        if config.SHOW_GRAVATAR_IMAGE:
+            self.assertIn(self.respdata, response.data.decode('utf8'))
+        # Should not have gravatar image
+        else:
+            self.assertNotIn(self.respdata, response.data.decode('utf8'))
+
+    @classmethod
+    def tearDownClass(cls):
+        """
+        We need to again login the test client as soon as test scenarios
+        finishes.
+        """
+        utils.login_tester_account(cls.tester)


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-05 15:27  Joao De Almeida Pereira <[email protected]>
  parent: Murtuza Zabuawala <[email protected]>
  1 sibling, 1 reply; 11+ messages in thread

From: Joao De Almeida Pereira @ 2018-03-05 15:27 UTC (permalink / raw)
  To: Murtuza Zabuawala <[email protected]>; +Cc: pgadmin-hackers

Hi Murtuza,

Why are we doing this using templates? Can this be done with a request to
the backend to get the image and then retrieve the Gravatar if it is
defined or return a static image if not?

+
+{% if config.SERVER_MODE %}
 window.onload = function(e){
  setTimeout(function() {
-   var gravatarImg = '<img src="{{ username | gravatar }}" width="18"
height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
class="caret"></span>';
+   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
    //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
    var navbarRight =
document.getElementById("navbar-menu").getElementsByClassName("navbar-right")[0];
    if (navbarRight) {

what if we have

var gravatarImg = '<img src="/user/{{username}}/gravatar" width="18"
height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
class="caret"></span>';

And then the endpoint
/user/{{username}}/gravatar
would be responsible for returning a Gravatar or not depending on the
configuration?


Thanks
Joao

On Mon, Mar 5, 2018 at 3:13 AM Murtuza Zabuawala <
[email protected]> wrote:

> Hi,
>
> PFA patch to disable Gravatar image in Server mode.
>
> Requirments & Issues:
> - For security reasons.
> - For systems which do not have internet access.
> - Hangs pgAdmin4 while loading the page if connection has no internet
> access (as described in the ticket)
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-05 16:21  Murtuza Zabuawala <[email protected]>
  parent: Joao De Almeida Pereira <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Murtuza Zabuawala @ 2018-03-05 16:21 UTC (permalink / raw)
  To: Joao De Almeida Pereira <[email protected]>; +Cc: pgadmin-hackers

Hi Joao,

We are using Flask-Gravatar <https://pythonhosted.org/Flask-Gravatar/;
module for this and it is designed to work with template only.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Mon, Mar 5, 2018 at 8:57 PM, Joao De Almeida Pereira <
[email protected]> wrote:

> Hi Murtuza,
>
> Why are we doing this using templates? Can this be done with a request to
> the backend to get the image and then retrieve the Gravatar if it is
> defined or return a static image if not?
>
> +
> +{% if config.SERVER_MODE %}
>  window.onload = function(e){
>   setTimeout(function() {
> -   var gravatarImg = '<img src="{{ username | gravatar }}" width="18"
> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
> class="caret"></span>';
> +   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
>     //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
>     var navbarRight = document.getElementById("navbar-menu").
> getElementsByClassName("navbar-right")[0];
>     if (navbarRight) {
>
> what if we have
>
> var gravatarImg = '<img src="/user/{{username}}/gravatar" width="18"
> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
> class="caret"></span>';
>
> And then the endpoint
> /user/{{username}}/gravatar
> would be responsible for returning a Gravatar or not depending on the
> configuration?
>
>
> Thanks
> Joao
>
> On Mon, Mar 5, 2018 at 3:13 AM Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch to disable Gravatar image in Server mode.
>>
>> Requirments & Issues:
>> - For security reasons.
>> - For systems which do not have internet access.
>> - Hangs pgAdmin4 while loading the page if connection has no internet
>> access (as described in the ticket)
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-05 19:47  Joao De Almeida Pereira <[email protected]>
  parent: Murtuza Zabuawala <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Joao De Almeida Pereira @ 2018-03-05 19:47 UTC (permalink / raw)
  To: Murtuza Zabuawala <[email protected]>; +Cc: pgadmin-hackers

Hello,
I understand that, but what do we win by using it? As you have noticed by
now I am not very fond of using templates for everything, and I believe
this is one of the things that we can skip templates.
We might even shift this to a frontend concern and if we want there are
node libraries to get gravatars.

I was able to create a PoC as a sample of what I was talking about:
 - Add gravatar library to package.json
 - Created a Preferences cache. (js/preferences.js)
   - So this was the concept I was talking about in a previous email we
talked about caching.
      the getConfiguration and the getAllConfiguration are not great names,
but they return a Promise that is consumed in the load_gravatar. The idea
here is that we are caching the pgAdmin settings and when need need to
consume them, we wait for it to be available.
 - load_gravatar is using the pattern to retrieve the configuration from a
possible cache and then apply the gravatar


Things that need to be revisited in the PoC:
- No tests, just some spiked code
- Did not retrieve the username from the backend, we need to create an
endpoint that give us this
- The url is hardcoded to get the configuration
- Maybe the Preferences is not the correct place to get server_mode
configuration not sure, what do you think?


Thanks
Joao

On Mon, Mar 5, 2018 at 11:21 AM Murtuza Zabuawala <
[email protected]> wrote:

> Hi Joao,
>
> We are using Flask-Gravatar <https://pythonhosted.org/Flask-Gravatar/;
> module for this and it is designed to work with template only.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Mon, Mar 5, 2018 at 8:57 PM, Joao De Almeida Pereira <
> [email protected]> wrote:
>
>> Hi Murtuza,
>>
>> Why are we doing this using templates? Can this be done with a request to
>> the backend to get the image and then retrieve the Gravatar if it is
>> defined or return a static image if not?
>>
>> +
>> +{% if config.SERVER_MODE %}
>>  window.onload = function(e){
>>   setTimeout(function() {
>> -   var gravatarImg = '<img src="{{ username | gravatar }}" width="18"
>> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
>> class="caret"></span>';
>> +   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
>>     //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
>>     var navbarRight =
>> document.getElementById("navbar-menu").getElementsByClassName("navbar-right")[0];
>>     if (navbarRight) {
>>
>> what if we have
>>
>> var gravatarImg = '<img src="/user/{{username}}/gravatar" width="18"
>> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
>> class="caret"></span>';
>>
>> And then the endpoint
>> /user/{{username}}/gravatar
>> would be responsible for returning a Gravatar or not depending on the
>> configuration?
>>
>>
>> Thanks
>> Joao
>>
>> On Mon, Mar 5, 2018 at 3:13 AM Murtuza Zabuawala <
>> [email protected]> wrote:
>>
>>> Hi,
>>>
>>> PFA patch to disable Gravatar image in Server mode.
>>>
>>> Requirments & Issues:
>>> - For security reasons.
>>> - For systems which do not have internet access.
>>> - Hangs pgAdmin4 while loading the page if connection has no internet
>>> access (as described in the ticket)
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>


Attachments:

  [text/x-patch] poc-retrieve-gravatar.diff (8.3K, 3-poc-retrieve-gravatar.diff)
  download | inline diff:
diff --git a/web/package.json b/web/package.json
index 2707b334..8d62e7b5 100644
--- a/web/package.json
+++ b/web/package.json
@@ -66,6 +66,7 @@
     "exports-loader": "~0.6.4",
     "flotr2": "^0.1.0",
     "font-awesome": "^4.7.0",
+    "gravatar": "^1.6.0",
     "hard-source-webpack-plugin": "^0.4.9",
     "immutability-helper": "^2.2.0",
     "imports-loader": "^0.7.1",
diff --git a/web/pgadmin/browser/templates/browser/index.html b/web/pgadmin/browser/templates/browser/index.html
index 58ff43f8..0901f44a 100644
--- a/web/pgadmin/browser/templates/browser/index.html
+++ b/web/pgadmin/browser/templates/browser/index.html
@@ -5,6 +5,7 @@ try {
 require(
 ['sources/generated/app.bundle'],
 function() {
+    pgAdmin.applyAvatar(pgAdmin);
 },
 function() {
 /* TODO:: Show proper error dialog */
@@ -66,17 +67,6 @@ require.onResourceLoad = function (context, map, depMaps) {
     }, 400)
   }
 };
-window.onload = function(e){
- setTimeout(function() {
-   var gravatarImg = '<img src="{{ username | gravatar }}" width="18" height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span class="caret"></span>';
-   //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
-   var navbarRight = document.getElementById("navbar-menu").getElementsByClassName("navbar-right")[0];
-   if (navbarRight) {
-     var list = navbarRight.getElementsByTagName("LI")[0];
-     list.getElementsByTagName("a")[0].innerHTML = gravatarImg;
-   }
- }, 1000);
-};
 
 {% endblock %}
 {% block body %}
diff --git a/web/pgadmin/preferences/__init__.py b/web/pgadmin/preferences/__init__.py
index f1f571cd..e75aa4cb 100644
--- a/web/pgadmin/preferences/__init__.py
+++ b/web/pgadmin/preferences/__init__.py
@@ -21,6 +21,7 @@ from pgadmin.utils.ajax import success_return, \
     make_response as ajax_response, internal_server_error
 from pgadmin.utils.menu import MenuItem
 from pgadmin.utils.preferences import Preferences
+import config
 
 MODULE_NAME = 'preferences'
 
@@ -163,6 +164,10 @@ def preferences_s():
                     p['module'] = m['name']
                     res.append(p)
 
+    res.append({'name': 'pgadmin', 'configuration': [
+        {'server_mode': config.SERVER_MODE}
+    ]
+    })
     return ajax_response(
         response=res,
         status=200
diff --git a/web/pgadmin/static/bundle/app.js b/web/pgadmin/static/bundle/app.js
index a2b86f87..ec78c89b 100644
--- a/web/pgadmin/static/bundle/app.js
+++ b/web/pgadmin/static/bundle/app.js
@@ -1,6 +1,6 @@
 define('app', [
-  'babel-polyfill', 'sources/pgadmin', 'bundled_browser', 'pgadmin.datagrid',
-], function(babelPolyFill, pgAdmin) {
+  'babel-polyfill', 'sources/pgadmin', 'sources/preferences', 'sources/load_gravatar', 'bundled_browser', 'pgadmin.datagrid',
+], function(babelPolyFill, pgAdmin, preferences, gravatar) {
   var initializeModules = function(Object) {
     for (var key in Object) {
       var module = Object[key];
@@ -20,4 +20,7 @@ define('app', [
 
     // create menus after all modules are initialized.
   pgAdmin.Browser.create_menus();
+
+  pgAdmin.Settings = new preferences.Preferences();
+  pgAdmin.applyAvatar = gravatar.applyAvatar;
 });
diff --git a/web/pgadmin/static/js/load_gravatar.js b/web/pgadmin/static/js/load_gravatar.js
new file mode 100644
index 00000000..1372b9b6
--- /dev/null
+++ b/web/pgadmin/static/js/load_gravatar.js
@@ -0,0 +1,19 @@
+import gravatar from 'gravatar';
+
+export function applyAvatar(pgAdmin) {
+  pgAdmin.Settings.getConfiguration('server_mode')
+    .then(function(server_mode) {
+      const username = '[email protected]';
+      if (server_mode) {
+        const gravatarImg = '<img src="' + gravatar.url(username) + '" width="18"' +
+          ' height="18" alt="Gravatar image for ' + username + '"> ' + username + ' <span' +
+        ' class="caret">' + username + '</span>';
+
+        const navbarRight = document.getElementById('navbar-menu').getElementsByClassName('navbar-right')[0];
+        if (navbarRight) {
+          const list = navbarRight.getElementsByTagName('LI')[0];
+          list.getElementsByTagName('a')[0].innerHTML = gravatarImg;
+        }
+      }
+    });
+}
diff --git a/web/pgadmin/static/js/pgadmin.js b/web/pgadmin/static/js/pgadmin.js
index f0f93d9a..648157fe 100644
--- a/web/pgadmin/static/js/pgadmin.js
+++ b/web/pgadmin/static/js/pgadmin.js
@@ -101,5 +101,7 @@ define([], function() {
     return 0;
   };
 
+
+
   return pgAdmin;
 });
diff --git a/web/pgadmin/static/js/preferences.js b/web/pgadmin/static/js/preferences.js
new file mode 100644
index 00000000..c46098bf
--- /dev/null
+++ b/web/pgadmin/static/js/preferences.js
@@ -0,0 +1,33 @@
+import axios from 'axios';
+import _ from 'underscore';
+
+export class Preferences {
+  constructor() {
+    this.config = {};
+  }
+
+  getAllConfiguration() {
+    let self = this;
+    return axios.get('http://localhost:5050/preferences/get_all')
+      .then((response) => {
+        const node = _.findWhere(response.data, {'name': 'pgadmin'});
+        self.config = Object.assign({}, node['configuration'][0]);
+      });
+  }
+
+
+  getConfiguration(key) {
+    let self = this;
+    if (self.config[key] === undefined) {
+      return this.getAllConfiguration()
+        .then(() => {
+          return self.config[key];
+        });
+    }
+
+
+    return new Promise((resolve) => {
+      resolve(self.config[key]);
+    });
+  }
+}
diff --git a/web/yarn.lock b/web/yarn.lock
index 666a20e6..3c874496 100644
--- a/web/yarn.lock
+++ b/web/yarn.lock
@@ -1185,6 +1185,10 @@ bluebird@^3.0.0, bluebird@^3.0.5, bluebird@^3.3.0:
   version "3.5.0"
   resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.5.0.tgz#791420d7f551eea2897453a8a77653f96606d67c"
 
+blueimp-md5@^2.3.0:
+  version "2.10.0"
+  resolved "https://registry.yarnpkg.com/blueimp-md5/-/blueimp-md5-2.10.0.tgz#02f0843921f90dca14f5b8920a38593201d6964d"
+
 bn.js@^4.0.0, bn.js@^4.1.0, bn.js@^4.1.1, bn.js@^4.4.0:
   version "4.11.8"
   resolved "https://registry.yarnpkg.com/bn.js/-/bn.js-4.11.8.tgz#2cde09eb5ee341f484746bb0309b3253b1b1442f"
@@ -2456,6 +2460,10 @@ elliptic@^6.0.0:
     minimalistic-assert "^1.0.0"
     minimalistic-crypto-utils "^1.0.0"
 
+email-validator@^1.0.7:
+  version "1.1.1"
+  resolved "https://registry.yarnpkg.com/email-validator/-/email-validator-1.1.1.tgz#b07f3be7bac1dc099bc43e75f6ae399f552d5a80"
+
 emojis-list@^2.0.0:
   version "2.1.0"
   resolved "https://registry.yarnpkg.com/emojis-list/-/emojis-list-2.1.0.tgz#4daa4d9db00f9819880c79fa457ae5b09a1fd389"
@@ -3410,6 +3418,15 @@ graceful-fs@^4.0.0, graceful-fs@^4.1.2, graceful-fs@^4.1.6, graceful-fs@^4.1.9:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/graceful-readlink/-/graceful-readlink-1.0.1.tgz#4cafad76bc62f02fa039b2f94e9a3dd3a391a725"
 
+gravatar@^1.6.0:
+  version "1.6.0"
+  resolved "https://registry.yarnpkg.com/gravatar/-/gravatar-1.6.0.tgz#8bdc9b786ca725a8e7076416d1731f8d3331c976"
+  dependencies:
+    blueimp-md5 "^2.3.0"
+    email-validator "^1.0.7"
+    querystring "0.2.0"
+    yargs "^6.0.0"
+
 gulp-decompress@^1.2.0:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/gulp-decompress/-/gulp-decompress-1.2.0.tgz#8eeb65a5e015f8ed8532cafe28454960626f0dc7"
@@ -7768,6 +7785,12 @@ yallist@^2.1.2:
   version "2.1.2"
   resolved "https://registry.yarnpkg.com/yallist/-/yallist-2.1.2.tgz#1c11f9218f076089a47dd512f93c6699a6a81d52"
 
+yargs-parser@^4.2.0:
+  version "4.2.1"
+  resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-4.2.1.tgz#29cceac0dc4f03c6c87b4a9f217dd18c9f74871c"
+  dependencies:
+    camelcase "^3.0.0"
+
 yargs-parser@^5.0.0:
   version "5.0.0"
   resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-5.0.0.tgz#275ecf0d7ffe05c77e64e7c86e4cd94bf0e1228a"
@@ -7780,6 +7803,24 @@ yargs-parser@^7.0.0:
   dependencies:
     camelcase "^4.1.0"
 
+yargs@^6.0.0:
+  version "6.6.0"
+  resolved "https://registry.yarnpkg.com/yargs/-/yargs-6.6.0.tgz#782ec21ef403345f830a808ca3d513af56065208"
+  dependencies:
+    camelcase "^3.0.0"
+    cliui "^3.2.0"
+    decamelize "^1.1.1"
+    get-caller-file "^1.0.1"
+    os-locale "^1.4.0"
+    read-pkg-up "^1.0.1"
+    require-directory "^2.1.1"
+    require-main-filename "^1.0.1"
+    set-blocking "^2.0.0"
+    string-width "^1.0.2"
+    which-module "^1.0.0"
+    y18n "^3.2.1"
+    yargs-parser "^4.2.0"
+
 yargs@^7.0.0:
   version "7.1.0"
   resolved "https://registry.yarnpkg.com/yargs/-/yargs-7.1.0.tgz#6ba318eb16961727f5d284f8ea003e8d6154d0c8"


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-06 06:59  Murtuza Zabuawala <[email protected]>
  parent: Joao De Almeida Pereira <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Murtuza Zabuawala @ 2018-03-06 06:59 UTC (permalink / raw)
  To: Joao De Almeida Pereira <[email protected]>; +Cc: pgadmin-hackers

Hi Joao,

Your suggestion is good but in my own opinion it is overkill just to
replace code {{ username| gravatar }} as rest of the syntax is pure HTML,
This makes code much simpler and easy to understand.
Apart from that we will be rendering 'index.html' template anyways and I
don't see any extra overhead.

I may be wrong, Let's wait for the input from other community members.

Regards,
Murtuza

On Tue, Mar 6, 2018 at 1:17 AM, Joao De Almeida Pereira <
[email protected]> wrote:

> Hello,
> I understand that, but what do we win by using it? As you have noticed by
> now I am not very fond of using templates for everything, and I believe
> this is one of the things that we can skip templates.
> We might even shift this to a frontend concern and if we want there are
> node libraries to get gravatars.
>
> I was able to create a PoC as a sample of what I was talking about:
>  - Add gravatar library to package.json
>  - Created a Preferences cache. (js/preferences.js)
>    - So this was the concept I was talking about in a previous email we
> talked about caching.
>       the getConfiguration and the getAllConfiguration are not great
> names, but they return a Promise that is consumed in the load_gravatar. The
> idea here is that we are caching the pgAdmin settings and when need need to
> consume them, we wait for it to be available.
>  - load_gravatar is using the pattern to retrieve the configuration from a
> possible cache and then apply the gravatar
>
>
> Things that need to be revisited in the PoC:
> - No tests, just some spiked code
> - Did not retrieve the username from the backend, we need to create an
> endpoint that give us this
> - The url is hardcoded to get the configuration
> - Maybe the Preferences is not the correct place to get server_mode
> configuration not sure, what do you think?
>
>
> Thanks
> Joao
>
> On Mon, Mar 5, 2018 at 11:21 AM Murtuza Zabuawala <
> [email protected]> wrote:
>
>> Hi Joao,
>>
>> We are using Flask-Gravatar <https://pythonhosted.org/Flask-Gravatar/;
>> module for this and it is designed to work with template only.
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>> On Mon, Mar 5, 2018 at 8:57 PM, Joao De Almeida Pereira <
>> [email protected]> wrote:
>>
>>> Hi Murtuza,
>>>
>>> Why are we doing this using templates? Can this be done with a request
>>> to the backend to get the image and then retrieve the Gravatar if it is
>>> defined or return a static image if not?
>>>
>>> +
>>> +{% if config.SERVER_MODE %}
>>>  window.onload = function(e){
>>>   setTimeout(function() {
>>> -   var gravatarImg = '<img src="{{ username | gravatar }}" width="18"
>>> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
>>> class="caret"></span>';
>>> +   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
>>>     //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
>>>     var navbarRight = document.getElementById("navba
>>> r-menu").getElementsByClassName("navbar-right")[0];
>>>     if (navbarRight) {
>>>
>>> what if we have
>>>
>>> var gravatarImg = '<img src="/user/{{username}}/gravatar" width="18"
>>> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
>>> class="caret"></span>';
>>>
>>> And then the endpoint
>>> /user/{{username}}/gravatar
>>> would be responsible for returning a Gravatar or not depending on the
>>> configuration?
>>>
>>>
>>> Thanks
>>> Joao
>>>
>>> On Mon, Mar 5, 2018 at 3:13 AM Murtuza Zabuawala <
>>> [email protected]> wrote:
>>>
>>>> Hi,
>>>>
>>>> PFA patch to disable Gravatar image in Server mode.
>>>>
>>>> Requirments & Issues:
>>>> - For security reasons.
>>>> - For systems which do not have internet access.
>>>> - Hangs pgAdmin4 while loading the page if connection has no internet
>>>> access (as described in the ticket)
>>>>
>>>> --
>>>> Regards,
>>>> Murtuza Zabuawala
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>>
>>


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-06 09:49  Dave Page <[email protected]>
  parent: Murtuza Zabuawala <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Dave Page @ 2018-03-06 09:49 UTC (permalink / raw)
  To: Murtuza Zabuawala <[email protected]>; +Cc: Joao De Almeida Pereira <[email protected]>; pgadmin-hackers

Hi

On Tue, Mar 6, 2018 at 6:59 AM, Murtuza Zabuawala <
[email protected]> wrote:

> Hi Joao,
>
> Your suggestion is good but in my own opinion it is overkill just to
> replace code {{ username| gravatar }} as rest of the syntax is pure HTML,
> This makes code much simpler and easy to understand.
> Apart from that we will be rendering 'index.html' template anyways and I
> don't see any extra overhead.
>

For this particular issue, I'm inclined to agree.

I do however, like the idea of caching preferences (no-brainer really,
assuming we ensure we update/invalidate the cache when needed), and using
promises for accessing them.


>
> I may be wrong, Let's wait for the input from other community members.
>
> Regards,
> Murtuza
>
> On Tue, Mar 6, 2018 at 1:17 AM, Joao De Almeida Pereira <
> [email protected]> wrote:
>
>> Hello,
>> I understand that, but what do we win by using it? As you have noticed by
>> now I am not very fond of using templates for everything, and I believe
>> this is one of the things that we can skip templates.
>> We might even shift this to a frontend concern and if we want there are
>> node libraries to get gravatars.
>>
>> I was able to create a PoC as a sample of what I was talking about:
>>  - Add gravatar library to package.json
>>  - Created a Preferences cache. (js/preferences.js)
>>    - So this was the concept I was talking about in a previous email we
>> talked about caching.
>>       the getConfiguration and the getAllConfiguration are not great
>> names, but they return a Promise that is consumed in the load_gravatar. The
>> idea here is that we are caching the pgAdmin settings and when need need to
>> consume them, we wait for it to be available.
>>  - load_gravatar is using the pattern to retrieve the configuration from
>> a possible cache and then apply the gravatar
>>
>>
>> Things that need to be revisited in the PoC:
>> - No tests, just some spiked code
>> - Did not retrieve the username from the backend, we need to create an
>> endpoint that give us this
>> - The url is hardcoded to get the configuration
>> - Maybe the Preferences is not the correct place to get server_mode
>> configuration not sure, what do you think?
>>
>>
>> Thanks
>> Joao
>>
>> On Mon, Mar 5, 2018 at 11:21 AM Murtuza Zabuawala <
>> [email protected]> wrote:
>>
>>> Hi Joao,
>>>
>>> We are using Flask-Gravatar <https://pythonhosted.org/Flask-Gravatar/;
>>> module for this and it is designed to work with template only.
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>> On Mon, Mar 5, 2018 at 8:57 PM, Joao De Almeida Pereira <
>>> [email protected]> wrote:
>>>
>>>> Hi Murtuza,
>>>>
>>>> Why are we doing this using templates? Can this be done with a request
>>>> to the backend to get the image and then retrieve the Gravatar if it is
>>>> defined or return a static image if not?
>>>>
>>>> +
>>>> +{% if config.SERVER_MODE %}
>>>>  window.onload = function(e){
>>>>   setTimeout(function() {
>>>> -   var gravatarImg = '<img src="{{ username | gravatar }}" width="18"
>>>> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
>>>> class="caret"></span>';
>>>> +   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
>>>>     //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
>>>>     var navbarRight = document.getElementById("navba
>>>> r-menu").getElementsByClassName("navbar-right")[0];
>>>>     if (navbarRight) {
>>>>
>>>> what if we have
>>>>
>>>> var gravatarImg = '<img src="/user/{{username}}/gravatar" width="18"
>>>> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
>>>> class="caret"></span>';
>>>>
>>>> And then the endpoint
>>>> /user/{{username}}/gravatar
>>>> would be responsible for returning a Gravatar or not depending on the
>>>> configuration?
>>>>
>>>>
>>>> Thanks
>>>> Joao
>>>>
>>>> On Mon, Mar 5, 2018 at 3:13 AM Murtuza Zabuawala <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> PFA patch to disable Gravatar image in Server mode.
>>>>>
>>>>> Requirments & Issues:
>>>>> - For security reasons.
>>>>> - For systems which do not have internet access.
>>>>> - Hangs pgAdmin4 while loading the page if connection has no internet
>>>>> access (as described in the ticket)
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Murtuza Zabuawala
>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>>
>>>
>


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-06 17:26  Joao De Almeida Pereira <[email protected]>
  parent: Dave Page <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Joao De Almeida Pereira @ 2018-03-06 17:26 UTC (permalink / raw)
  To: Dave Page <[email protected]>; +Cc: Murtuza Zabuawala <[email protected]>; pgadmin-hackers

Hello,

I just sent a video that talks exactly about this, in the journey to have a
more maintainable code and easier to navigate, we sometimes increase
complexity for a period of time in order to get to the state that we want
to be.

In particular in this example I tried the following:
1. Demonstrate that we do not need templates to get things done
2. How to get the information we need to the place where it is needed, like
moving the information of the server mode to the front end where decisions
should be made

Sometimes just because we can do a simple change, doesn't mean we should do
it because it might bit us in the future.


The PoC looks like a huge amount of code that is not even related to
Gravatar, when we could just change a template file or 2, I hear that. But
what I am talking about is a change of paradigm and Architecture where the
Frontend is responsible for displaying the application and the Backend is
only responsible for giving information that the Frontend Requires. Instead
of the current mixed architecture where some things are done in the
Frontend some things are done in the Backend a more predictable
architecture would benefit the application and specially the developers.

That is why I always insist in refactor code as much as possible to ensure
that the code is readable and helps people follow it. This doesn't mean
abstracting classes of functions to make to code more generic, it means as
an example to split 1 functions of 500 lines into 10 functions of 50 lines
each or even 50 functions of 10 lines each but that are readable and not a
"goat trails" of ifs and elses that are very hard to read and follow.


Thanks
Joao

On Tue, Mar 6, 2018 at 4:49 AM Dave Page <[email protected]> wrote:

> Hi
>
> On Tue, Mar 6, 2018 at 6:59 AM, Murtuza Zabuawala <
> [email protected]> wrote:
>
>> Hi Joao,
>>
>> Your suggestion is good but in my own opinion it is overkill just to
>> replace code {{ username| gravatar }} as rest of the syntax is pure
>> HTML, This makes code much simpler and easy to understand.
>> Apart from that we will be rendering 'index.html' template anyways and I
>> don't see any extra overhead.
>>
>
> For this particular issue, I'm inclined to agree.
>
> I do however, like the idea of caching preferences (no-brainer really,
> assuming we ensure we update/invalidate the cache when needed), and using
> promises for accessing them.
>
>
>>
>> I may be wrong, Let's wait for the input from other community members.
>>
>> Regards,
>> Murtuza
>>
>> On Tue, Mar 6, 2018 at 1:17 AM, Joao De Almeida Pereira <
>> [email protected]> wrote:
>>
>>> Hello,
>>> I understand that, but what do we win by using it? As you have noticed
>>> by now I am not very fond of using templates for everything, and I believe
>>> this is one of the things that we can skip templates.
>>> We might even shift this to a frontend concern and if we want there are
>>> node libraries to get gravatars.
>>>
>>> I was able to create a PoC as a sample of what I was talking about:
>>>  - Add gravatar library to package.json
>>>  - Created a Preferences cache. (js/preferences.js)
>>>    - So this was the concept I was talking about in a previous email we
>>> talked about caching.
>>>       the getConfiguration and the getAllConfiguration are not great
>>> names, but they return a Promise that is consumed in the load_gravatar. The
>>> idea here is that we are caching the pgAdmin settings and when need need to
>>> consume them, we wait for it to be available.
>>>  - load_gravatar is using the pattern to retrieve the configuration from
>>> a possible cache and then apply the gravatar
>>>
>>>
>>> Things that need to be revisited in the PoC:
>>> - No tests, just some spiked code
>>> - Did not retrieve the username from the backend, we need to create an
>>> endpoint that give us this
>>> - The url is hardcoded to get the configuration
>>> - Maybe the Preferences is not the correct place to get server_mode
>>> configuration not sure, what do you think?
>>>
>>>
>>> Thanks
>>> Joao
>>>
>>> On Mon, Mar 5, 2018 at 11:21 AM Murtuza Zabuawala <
>>> [email protected]> wrote:
>>>
>>>> Hi Joao,
>>>>
>>>> We are using Flask-Gravatar <https://pythonhosted.org/Flask-Gravatar/;
>>>> module for this and it is designed to work with template only.
>>>>
>>>> --
>>>> Regards,
>>>> Murtuza Zabuawala
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>>
>>>> On Mon, Mar 5, 2018 at 8:57 PM, Joao De Almeida Pereira <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Murtuza,
>>>>>
>>>>> Why are we doing this using templates? Can this be done with a request
>>>>> to the backend to get the image and then retrieve the Gravatar if it is
>>>>> defined or return a static image if not?
>>>>>
>>>>> +
>>>>> +{% if config.SERVER_MODE %}
>>>>>  window.onload = function(e){
>>>>>   setTimeout(function() {
>>>>> -   var gravatarImg = '<img src="{{ username | gravatar }}" width="18"
>>>>> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
>>>>> class="caret"></span>';
>>>>> +   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
>>>>>     //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
>>>>>     var navbarRight =
>>>>> document.getElementById("navbar-menu").getElementsByClassName("navbar-right")[0];
>>>>>     if (navbarRight) {
>>>>>
>>>>> what if we have
>>>>>
>>>>> var gravatarImg = '<img src="/user/{{username}}/gravatar" width="18"
>>>>> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
>>>>> class="caret"></span>';
>>>>>
>>>>> And then the endpoint
>>>>> /user/{{username}}/gravatar
>>>>> would be responsible for returning a Gravatar or not depending on the
>>>>> configuration?
>>>>>
>>>>>
>>>>> Thanks
>>>>> Joao
>>>>>
>>>>> On Mon, Mar 5, 2018 at 3:13 AM Murtuza Zabuawala <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> PFA patch to disable Gravatar image in Server mode.
>>>>>>
>>>>>> Requirments & Issues:
>>>>>> - For security reasons.
>>>>>> - For systems which do not have internet access.
>>>>>> - Hangs pgAdmin4 while loading the page if connection has no internet
>>>>>> access (as described in the ticket)
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>> Murtuza Zabuawala
>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>>
>>>>
>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-07 13:08  Murtuza Zabuawala <[email protected]>
  parent: Joao De Almeida Pereira <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Murtuza Zabuawala @ 2018-03-07 13:08 UTC (permalink / raw)
  To: Joao De Almeida Pereira <[email protected]>; +Cc: Dave Page <[email protected]>; pgadmin-hackers

Hi Joao,

On Tue, Mar 6, 2018 at 10:56 PM, Joao De Almeida Pereira <
[email protected]> wrote:

> Hello,
>
> I just sent a video that talks exactly about this, in the journey to have
> a more maintainable code and easier to navigate, we sometimes increase
> complexity for a period of time in order to get to the state that we want
> to be.
>
> In particular in this example I tried the following:
> 1. Demonstrate that we do not need templates to get things done
>
That raised questions in mind,
​Why?
​
I
​f templates are bad then why every other framework provides them?
I've seen many projects using templates to serve HTML code, In pgAdmin4
also we are using templates just as starting point to load UI whether its
main page or query tool/debugger page.

2. How to get the information we need to the place where it is needed, like
> moving the information of the server mode to the front end where decisions
> should be made
>

​I agree and that's what we are doing in pgAdmin4.

> ​
>
>

> Sometimes just because we can do a simple change, doesn't mean we should
> do it because it might bit us in the future.
>
>
> The PoC looks like a huge amount of code that is not even related to
> Gravatar, when we could just change a template file or 2, I hear that. But
> what I am talking about is a change of paradigm and Architecture where the
> Frontend is responsible for displaying the application and the Backend is
> only responsible for giving information that the Frontend Requires. Instead
> of the current mixed architecture where some things are done in the
> Frontend some things are done in the Backend a more predictable
> architecture would benefit the application and specially the developers.
> ​
>
>

> That is why I always insist in refactor code as much as possible to ensure
> that the code is readable and helps people follow it. This doesn't mean
> abstracting classes of functions to make to code more generic, it means as
> an example to split 1 functions of 500 lines into 10 functions of 50 lines
> each or even 50 functions of 10 lines each but that are readable and not a
> "goat trails" of ifs and elses that are very hard to read and follow.
>

 ​I
​totally ​
agree that refacto
​ring​
code is good practice but in this
​particular ​
case I
​still ​
think
​that ​
using
​HTML ​
template is
​much ​
simpler and elegant solution
​.​

>
>
> Thanks
> Joao
>
> On Tue, Mar 6, 2018 at 4:49 AM Dave Page <[email protected]> wrote:
>
>> Hi
>>
>> On Tue, Mar 6, 2018 at 6:59 AM, Murtuza Zabuawala <murtuza.zabuawala@
>> enterprisedb.com> wrote:
>>
>>> Hi Joao,
>>>
>>> Your suggestion is good but in my own opinion it is overkill just to
>>> replace code {{ username| gravatar }} as rest of the syntax is pure
>>> HTML, This makes code much simpler and easy to understand.
>>> Apart from that we will be rendering 'index.html' template anyways and I
>>> don't see any extra overhead.
>>>
>>
>> For this particular issue, I'm inclined to agree.
>>
>> I do however, like the idea of caching preferences (no-brainer really,
>> assuming we ensure we update/invalidate the cache when needed), and using
>> promises for accessing them.
>>
>>
>>>
>>> I may be wrong, Let's wait for the input from other community members.
>>>
>>> Regards,
>>> Murtuza
>>>
>>> On Tue, Mar 6, 2018 at 1:17 AM, Joao De Almeida Pereira <
>>> [email protected]> wrote:
>>>
>>>> Hello,
>>>> I understand that, but what do we win by using it? As you have noticed
>>>> by now I am not very fond of using templates for everything, and I believe
>>>> this is one of the things that we can skip templates.
>>>> We might even shift this to a frontend concern and if we want there are
>>>> node libraries to get gravatars.
>>>>
>>>> I was able to create a PoC as a sample of what I was talking about:
>>>>  - Add gravatar library to package.json
>>>>  - Created a Preferences cache. (js/preferences.js)
>>>>    - So this was the concept I was talking about in a previous email we
>>>> talked about caching.
>>>>       the getConfiguration and the getAllConfiguration are not great
>>>> names, but they return a Promise that is consumed in the load_gravatar. The
>>>> idea here is that we are caching the pgAdmin settings and when need need to
>>>> consume them, we wait for it to be available.
>>>>  - load_gravatar is using the pattern to retrieve the configuration
>>>> from a possible cache and then apply the gravatar
>>>>
>>>>
>>>> Things that need to be revisited in the PoC:
>>>> - No tests, just some spiked code
>>>> - Did not retrieve the username from the backend, we need to create an
>>>> endpoint that give us this
>>>> - The url is hardcoded to get the configuration
>>>> - Maybe the Preferences is not the correct place to get server_mode
>>>> configuration not sure, what do you think?
>>>>
>>>>
>>>> Thanks
>>>> Joao
>>>>
>>>> On Mon, Mar 5, 2018 at 11:21 AM Murtuza Zabuawala <murtuza.zabuawala@
>>>> enterprisedb.com> wrote:
>>>>
>>>>> Hi Joao,
>>>>>
>>>>> We are using Flask-Gravatar <https://pythonhosted.org/Flask-Gravatar/;
>>>>> module for this and it is designed to work with template only.
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Murtuza Zabuawala
>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>> The Enterprise PostgreSQL Company
>>>>>
>>>>>
>>>>> On Mon, Mar 5, 2018 at 8:57 PM, Joao De Almeida Pereira <
>>>>> [email protected]> wrote:
>>>>>
>>>>>> Hi Murtuza,
>>>>>>
>>>>>> Why are we doing this using templates? Can this be done with a
>>>>>> request to the backend to get the image and then retrieve the Gravatar if
>>>>>> it is defined or return a static image if not?
>>>>>>
>>>>>> +
>>>>>> +{% if config.SERVER_MODE %}
>>>>>>  window.onload = function(e){
>>>>>>   setTimeout(function() {
>>>>>> -   var gravatarImg = '<img src="{{ username | gravatar }}"
>>>>>> width="18" height="18" alt="Gravatar image for {{ username }}"> {{ username
>>>>>> }} <span class="caret"></span>';
>>>>>> +   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
>>>>>>     //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
>>>>>>     var navbarRight = document.getElementById("navbar-menu").
>>>>>> getElementsByClassName("navbar-right")[0];
>>>>>>     if (navbarRight) {
>>>>>>
>>>>>> what if we have
>>>>>>
>>>>>> var gravatarImg = '<img src="/user/{{username}}/gravatar" width="18"
>>>>>> height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span
>>>>>> class="caret"></span>';
>>>>>>
>>>>>> And then the endpoint
>>>>>> /user/{{username}}/gravatar
>>>>>> would be responsible for returning a Gravatar or not depending on the
>>>>>> configuration?
>>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> Joao
>>>>>>
>>>>>> On Mon, Mar 5, 2018 at 3:13 AM Murtuza Zabuawala <murtuza.zabuawala@
>>>>>> enterprisedb.com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> PFA patch to disable Gravatar image in Server mode.
>>>>>>>
>>>>>>> Requirments & Issues:
>>>>>>> - For security reasons.
>>>>>>> - For systems which do not have internet access.
>>>>>>> - Hangs pgAdmin4 while loading the page if connection has no
>>>>>>> internet access (as described in the ticket)
>>>>>>>
>>>>>>> --
>>>>>>> Regards,
>>>>>>> Murtuza Zabuawala
>>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>
>>>>>>>
>>>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-07 13:50  Dave Page <[email protected]>
  parent: Murtuza Zabuawala <[email protected]>
  1 sibling, 1 reply; 11+ messages in thread

From: Dave Page @ 2018-03-07 13:50 UTC (permalink / raw)
  To: Murtuza Zabuawala <[email protected]>; +Cc: pgadmin-hackers

Hi

On Mon, Mar 5, 2018 at 8:12 AM, Murtuza Zabuawala <
[email protected]> wrote:

> Hi,
>
> PFA patch to disable Gravatar image in Server mode.
>
> Requirments & Issues:
> - For security reasons.
> - For systems which do not have internet access.
> - Hangs pgAdmin4 while loading the page if connection has no internet
> access (as described in the ticket)
>

If I run with the Gravatar disabled, then I get the following regression
failure:

======================================================================
FAIL: runTest (pgadmin.browser.tests.test_login.LoginTestCase)
Valid_Credentials
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/tests/test_login.py",
line 93, in runTest
    self.assertTrue(self.respdata in response.data.decode('utf8'))
AssertionError: False is not true

----------------------------------------------------------------------

It's fine when Gravatar is enabled (and the rest of the patch does too).

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-07 15:17  Murtuza Zabuawala <[email protected]>
  parent: Dave Page <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Murtuza Zabuawala @ 2018-03-07 15:17 UTC (permalink / raw)
  To: Dave Page <[email protected]>; +Cc: pgadmin-hackers

Hi Dave,

PFA updated patch.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Wed, Mar 7, 2018 at 7:20 PM, Dave Page <[email protected]> wrote:

> Hi
>
> On Mon, Mar 5, 2018 at 8:12 AM, Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Hi,
>>
>> PFA patch to disable Gravatar image in Server mode.
>>
>> Requirments & Issues:
>> - For security reasons.
>> - For systems which do not have internet access.
>> - Hangs pgAdmin4 while loading the page if connection has no internet
>> access (as described in the ticket)
>>
>
> If I run with the Gravatar disabled, then I get the following regression
> failure:
>
> ======================================================================
> FAIL: runTest (pgadmin.browser.tests.test_login.LoginTestCase)
> Valid_Credentials
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/tests/test_login.py",
> line 93, in runTest
>     self.assertTrue(self.respdata in response.data.decode('utf8'))
> AssertionError: False is not true
>
> ----------------------------------------------------------------------
>
> It's fine when Gravatar is enabled (and the rest of the patch does too).
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


Attachments:

  [application/octet-stream] RM_3037_v1.diff (8.7K, 3-RM_3037_v1.diff)
  download | inline diff:
diff --git a/web/config.py b/web/config.py
index a728334..d4ddd9d 100644
--- a/web/config.py
+++ b/web/config.py
@@ -357,6 +357,11 @@ SQLALCHEMY_TRACK_MODIFICATIONS = False
 ON_DEMAND_RECORD_COUNT = 1000
 
 ##########################################################################
+# Allow users to display Gravatar image for their username in Server mode
+##########################################################################
+SHOW_GRAVATAR_IMAGE = True
+
+##########################################################################
 # Local config settings
 ##########################################################################
 
diff --git a/web/pgadmin/browser/__init__.py b/web/pgadmin/browser/__init__.py
index 1589677..dd2542f 100644
--- a/web/pgadmin/browser/__init__.py
+++ b/web/pgadmin/browser/__init__.py
@@ -730,18 +730,18 @@ class BrowserPluginModule(PgAdminModule):
 @login_required
 def index():
     """Render and process the main browser window."""
-    # Get the Gravatar
-    Gravatar(
-        current_app,
-        size=100,
-        rating='g',
-        default='retro',
-        force_default=False,
-        use_ssl=True,
-        base_url=None
-    )
+    # Register Gravatar module with the app only if required
+    if config.SHOW_GRAVATAR_IMAGE:
+        Gravatar(
+            current_app,
+            size=100,
+            rating='g',
+            default='retro',
+            force_default=False,
+            use_ssl=True,
+            base_url=None
+        )
 
-    msg = None
     # Get the current version info from the website, and flash a message if
     # the user is out of date, and the check is enabled.
     if config.UPGRADE_CHECK_ENABLED:
@@ -761,7 +761,7 @@ def index():
             if response.getcode() == 200:
                 data = json.loads(response.read().decode('utf-8'))
                 current_app.logger.debug('Response data: %s' % data)
-        except Exception as e:
+        except Exception:
             current_app.logger.exception('Exception when checking for update')
 
         if data is not None:
diff --git a/web/pgadmin/browser/static/css/browser.css b/web/pgadmin/browser/static/css/browser.css
index 2fffd64..3ba330d 100644
--- a/web/pgadmin/browser/static/css/browser.css
+++ b/web/pgadmin/browser/static/css/browser.css
@@ -62,3 +62,7 @@ samp,
 .sql-editor-grid-container {
   font-family: 'Open Sans' !important;
 }
+
+.pg-login-icon {
+  font-size: 16px;
+}
diff --git a/web/pgadmin/browser/templates/browser/index.html b/web/pgadmin/browser/templates/browser/index.html
index 58ff43f..76c3e4c 100644
--- a/web/pgadmin/browser/templates/browser/index.html
+++ b/web/pgadmin/browser/templates/browser/index.html
@@ -1,5 +1,13 @@
 {% extends "base.html" %}
+
+{% if config.SERVER_MODE and config.SHOW_GRAVATAR_IMAGE -%}
+{% import 'browser/macros/gravatar_icon.macro' as IMG with context %}
+{% elif config.SERVER_MODE %}
+{% import 'browser/macros/static_user_icon.macro' as IMG with context %}
+{% endif %}
+
 {% block title %}{{ config.APP_NAME }}{% endblock %}
+
 {% block init_script %}
 try {
 require(
@@ -66,9 +74,11 @@ require.onResourceLoad = function (context, map, depMaps) {
     }, 400)
   }
 };
+
+{% if config.SERVER_MODE %}
 window.onload = function(e){
  setTimeout(function() {
-   var gravatarImg = '<img src="{{ username | gravatar }}" width="18" height="18" alt="Gravatar image for {{ username }}"> {{ username }} <span class="caret"></span>';
+   var gravatarImg = {{ IMG.PREPARE_HTML()|safe }}
    //$('#navbar-menu .navbar-right > li > a').html(gravatarImg);
    var navbarRight = document.getElementById("navbar-menu").getElementsByClassName("navbar-right")[0];
    if (navbarRight) {
@@ -77,8 +87,9 @@ window.onload = function(e){
    }
  }, 1000);
 };
-
+{% endif %}
 {% endblock %}
+
 {% block body %}
 <style>
 #pg-spinner {
diff --git a/web/pgadmin/browser/templates/browser/macros/gravatar_icon.macro b/web/pgadmin/browser/templates/browser/macros/gravatar_icon.macro
new file mode 100644
index 0000000..72ec97e
--- /dev/null
+++ b/web/pgadmin/browser/templates/browser/macros/gravatar_icon.macro
@@ -0,0 +1,8 @@
+{##########################################################################
+We wrote separate macro because if user choose to disable Gravatar then
+we will not associate our application with Gravatar module which will make
+'gravatar' filter unavailable in Jinja templates
+###########################################################################}
+{% macro PREPARE_HTML() -%}
+'<img src = "{{ username | gravatar }}" width = "18" height = "18" alt = "Gravatar image for {{ username }}" > {{ username }} <span class="caret"></span>';
+{%- endmacro %}
diff --git a/web/pgadmin/browser/templates/browser/macros/static_user_icon.macro b/web/pgadmin/browser/templates/browser/macros/static_user_icon.macro
new file mode 100644
index 0000000..6b72844
--- /dev/null
+++ b/web/pgadmin/browser/templates/browser/macros/static_user_icon.macro
@@ -0,0 +1,3 @@
+{% macro PREPARE_HTML() -%}
+'<i class="fa fa-user-circle pg-login-icon" aria-hidden="true"></i> {{ username }} <span class="caret"></span>';
+{%- endmacro %}
diff --git a/web/pgadmin/browser/tests/test_gravatar_image_display.py b/web/pgadmin/browser/tests/test_gravatar_image_display.py
new file mode 100644
index 0000000..a4526c9
--- /dev/null
+++ b/web/pgadmin/browser/tests/test_gravatar_image_display.py
@@ -0,0 +1,68 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+import config
+from pgadmin.utils.route import BaseTestGenerator
+from regression.python_test_utils import test_utils as utils
+from regression.test_setup import config_data as tconfig
+
+
+class TestLoginUserImage(BaseTestGenerator):
+    """
+    This class checks for user image after successful login.
+    - If SHOW_GRAVATAR_IMAGE config option is set to True then we will show
+    Gravatar on the Page.
+    - If SHOW_GRAVATAR_IMAGE config option is set to False then we will show
+    Static image on the Page.
+    """
+
+    scenarios = [
+        (
+            'Verify gravatar image on the page', dict(
+                email=(
+                    tconfig['pgAdmin4_login_credentials']['login_username']
+                ),
+                password=(
+                    tconfig['pgAdmin4_login_credentials']['login_password']
+                ),
+                respdata='Gravatar image for %s' %
+                         tconfig['pgAdmin4_login_credentials']
+                         ['login_username'],
+            )
+        )
+    ]
+
+    @classmethod
+    def setUpClass(cls):
+        "Logout first if already logged in"
+        utils.logout_tester_account(cls.tester)
+
+    def runTest(self):
+        # Login and check type of image in response
+        response = self.tester.post(
+            '/login', data=dict(
+                email=self.email,
+                password=self.password
+            ),
+            follow_redirects=True
+        )
+        # Should have gravatar image
+        if config.SHOW_GRAVATAR_IMAGE:
+            self.assertIn(self.respdata, response.data.decode('utf8'))
+        # Should not have gravatar image
+        else:
+            self.assertNotIn(self.respdata, response.data.decode('utf8'))
+
+    @classmethod
+    def tearDownClass(cls):
+        """
+        We need to again login the test client as soon as test scenarios
+        finishes.
+        """
+        utils.login_tester_account(cls.tester)
diff --git a/web/pgadmin/browser/tests/test_login.py b/web/pgadmin/browser/tests/test_login.py
index 3331b68..0f0a30e 100644
--- a/web/pgadmin/browser/tests/test_login.py
+++ b/web/pgadmin/browser/tests/test_login.py
@@ -62,13 +62,15 @@ class LoginTestCase(BaseTestGenerator):
         # This test case validates the valid/correct credentials and allow user
         # to login pgAdmin 4
         ('Valid_Credentials', dict(
-            email=(config_data['pgAdmin4_login_credentials']
-                   ['login_username']),
-            password=(config_data['pgAdmin4_login_credentials']
-                      ['login_password']),
-            respdata='Gravatar image for %s' %
-                     config_data['pgAdmin4_login_credentials']
-                     ['login_username']))
+            email=(config_data[
+                'pgAdmin4_login_credentials'
+            ]['login_username']),
+            password=(config_data[
+                'pgAdmin4_login_credentials'
+            ]['login_password']),
+            respdata='%s' % config_data['pgAdmin4_login_credentials']
+            ['login_username'])
+         )
     ]
 
     @classmethod


^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
@ 2018-03-07 16:35  Dave Page <[email protected]>
  parent: Murtuza Zabuawala <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Dave Page @ 2018-03-07 16:35 UTC (permalink / raw)
  To: Murtuza Zabuawala <[email protected]>; +Cc: pgadmin-hackers

Thanks, applied.

On Wed, Mar 7, 2018 at 3:17 PM, Murtuza Zabuawala <
[email protected]> wrote:

> Hi Dave,
>
> PFA updated patch.
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> On Wed, Mar 7, 2018 at 7:20 PM, Dave Page <[email protected]> wrote:
>
>> Hi
>>
>> On Mon, Mar 5, 2018 at 8:12 AM, Murtuza Zabuawala <
>> [email protected]> wrote:
>>
>>> Hi,
>>>
>>> PFA patch to disable Gravatar image in Server mode.
>>>
>>> Requirments & Issues:
>>> - For security reasons.
>>> - For systems which do not have internet access.
>>> - Hangs pgAdmin4 while loading the page if connection has no internet
>>> access (as described in the ticket)
>>>
>>
>> If I run with the Gravatar disabled, then I get the following regression
>> failure:
>>
>> ======================================================================
>> FAIL: runTest (pgadmin.browser.tests.test_login.LoginTestCase)
>> Valid_Credentials
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>   File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/tests/test_login.py",
>> line 93, in runTest
>>     self.assertTrue(self.respdata in response.data.decode('utf8'))
>> AssertionError: False is not true
>>
>> ----------------------------------------------------------------------
>>
>> It's fine when Gravatar is enabled (and the rest of the patch does too).
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


^ permalink  raw  reply  [nested|flat] 11+ messages in thread


end of thread, other threads:[~2018-03-07 16:35 UTC | newest]

Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-03-05 08:12 [pgAdmin4][RM#3037] Allow user to disable Gravatar image Murtuza Zabuawala <[email protected]>
2018-03-05 15:27 ` Joao De Almeida Pereira <[email protected]>
2018-03-05 16:21   ` Murtuza Zabuawala <[email protected]>
2018-03-05 19:47     ` Joao De Almeida Pereira <[email protected]>
2018-03-06 06:59       ` Murtuza Zabuawala <[email protected]>
2018-03-06 09:49         ` Dave Page <[email protected]>
2018-03-06 17:26           ` Joao De Almeida Pereira <[email protected]>
2018-03-07 13:08             ` Murtuza Zabuawala <[email protected]>
2018-03-07 13:50 ` Dave Page <[email protected]>
2018-03-07 15:17   ` Murtuza Zabuawala <[email protected]>
2018-03-07 16:35     ` Dave Page <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox