Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][RM#3037] Allow user to disable Gravatar image
Date: 2018-03-06 09:49:18
Message-ID: CA+OCxowPwcOgJ0W9pEooZTyu+kA-isR1afSGbNwQQ-F1fe23GA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi

On Tue, Mar 6, 2018 at 6:59 AM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)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 <
> jdealmeidapereira(at)pivotal(dot)io> 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(dot)zabuawala(at)enterprisedb(dot)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 <
>>> jdealmeidapereira(at)pivotal(dot)io> 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 <
>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Khushboo Vashi 2018-03-06 10:09:44 Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query
Previous Message Murtuza Zabuawala 2018-03-06 09:22:46 [pgAdmin4][RM#2989] To fix the issue in Table node