public inbox for [email protected]  
help / color / mirror / Atom feed
From: Rahul Shirsat <[email protected]>
To: Dave Page <[email protected]>
Cc: Aditya Toshniwal <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure
Date: Wed, 30 Jun 2021 12:58:11 +0530
Message-ID: <CAKtn9dMcVSjEwjeOfZC5Kt7CD-YUmQ=W9fFN4XXGSCLsT-oyzw@mail.gmail.com> (raw)
In-Reply-To: <CAKtn9dNWNAO84Qg15TPoJgVMgah39jbQcxaWp6xDik2ykQQx-g@mail.gmail.com>
References: <CAKtn9dP2TmBCqHxV0szjdmCEVLbJwBNXA6rsumoOKeQJG1qyUw@mail.gmail.com>
	<CA+OCxoxE+OK3_KW_vu_pWY4+t5qyH985_Znp7v+uA2vmYRo2BQ@mail.gmail.com>
	<CAKtn9dOWXDQo9K_HuqGOmhq=AeVVzcE8d=J+8obRj014G=0jxQ@mail.gmail.com>
	<CA+OCxozh_DV2m7vcEAaD0rsY31OntJF5GdRBXmqj0PCWULStew@mail.gmail.com>
	<CAM9w-_kzsV0uvVyqswNCwnhED=3-Z=sL9=awoaUZwKxcvS=5Tg@mail.gmail.com>
	<CAKtn9dP3nj5j6ZL-9ExyBz0rnm2-S2FO+qKTcituB+Z=5sPswg@mail.gmail.com>
	<CA+OCxoyjCRCo3t6PvHR9LUctEoyPB9c-Vjo-hOK9Z82wRCn-Lg@mail.gmail.com>
	<CAM9w-_n3JKd5XsrDJ_NFkaNnhSaQyKhx5XjVq6cpVfb68B8xmw@mail.gmail.com>
	<CA+OCxozO3ea=CnCofz0RDii4wdMcGAe1JYN-HLmxLurtw7Kmwg@mail.gmail.com>
	<CAM9w-_=A5qZ=0g4Y==Ue_5esXX48JdYadpfQ1jf-hkJ6DhQvHg@mail.gmail.com>
	<CA+OCxozap1agLK-tWnBmcNA8COc7ahW70hEmBgM1KRANX-6DJw@mail.gmail.com>
	<CAKtn9dNWNAO84Qg15TPoJgVMgah39jbQcxaWp6xDik2ykQQx-g@mail.gmail.com>

Hi All,

Please find the attached patch for resolving this issue wrt above
suggestion.

Let me know if anyone has any queries.

On Tue, Jun 29, 2021 at 9:20 PM Rahul Shirsat <
[email protected]> wrote:

> Hi Dave / Aditya,
>
> For a time being, Let's make a call to gettext conditional instead of
> passing dynamic parameters for this scenario at least. With this, we won't
> be touching the *.po files and translations will do its task smoothly.
> I have already checked for the string with weird unprintable characters,
> and there were none.
>
> e.g.
> Instead of -  *title = gettext('%s objects?', obj.label);*
>
> *if (drop) {*
> *   title = gettext('Drop objects?');*
> *}*
> *else {*
>    *title = gettext('Reassign objects?');*
> *}*
>
> I have tried the above code and it looks good to me.
>
> Please suggest.
>
> On Tue, Jun 29, 2021 at 7:31 PM Dave Page <[email protected]> wrote:
>
>>
>>
>> On Tue, Jun 29, 2021 at 2:55 PM Aditya Toshniwal <
>> [email protected]> wrote:
>>
>>> Hi,
>>>
>>> On Tue, Jun 29, 2021 at 7:14 PM Dave Page <[email protected]> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Jun 29, 2021 at 2:41 PM Aditya Toshniwal <
>>>> [email protected]> wrote:
>>>>
>>>>> Dave,
>>>>>
>>>>> Somehow, the new text strings are added to PO with incorrect
>>>>> translations. That is causing the issue.
>>>>> Either they should be empty or fixed.
>>>>>
>>>>
>>>> Then the source problem should be fixed. There's no point at all in
>>>> putting fixes directly in the PO files as they'll be overwritten prior to
>>>> release anyway.
>>>>
>>> The translations submitted by translators are updated in the PO file
>>> right ? And then they're compiled to MO ?
>>>
>>
>> Right.
>>
>>
>>> It's the same like Rahul will be submitting the translations. Please
>>> correct me if I'm wrong.
>>>
>>
>> That depends if he's changed the original message or the translated
>> message. It's impossible to see without reading megabytes of text.
>>
>> And in any case, he's updated translations that haven't been touched in
>> ages - why would they suddenly have broken? (hint: if the upstream message
>> has changed, it'll be marked as fuzzy or untranslated - in other words,
>> gettext knows how to handle that).
>>
>>
>>>
>>>>
>>>>>
>>>>> On Tue, Jun 29, 2021 at 7:01 PM Dave Page <[email protected]> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> Please send the patch without updates to the po files. Those get
>>>>>> updated as part of the release process.
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> On Tue, Jun 29, 2021 at 2:00 PM Rahul Shirsat <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hi Hackers,
>>>>>>>
>>>>>>> Thanks Aditya for pointing out the issue. Please find the attached
>>>>>>> patch which contains all the .po files corrected with %s.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Rahul Shirsat.
>>>>>>>
>>>>>>> On Tue, Jun 29, 2021 at 4:31 PM Aditya Toshniwal <
>>>>>>> [email protected]> wrote:
>>>>>>>
>>>>>>>> Hi Rahul,
>>>>>>>>
>>>>>>>> I did "make msg-extract" and "make msg-update" and looking at the
>>>>>>>> PO files I think it is not updated correctly.
>>>>>>>> For instance, the below message has msgstr without %s. I corrected
>>>>>>>> it and the error was gone.
>>>>>>>>
>>>>>>>> #: pgadmin/browser/server_groups/servers/roles/static/js/role.js:766
>>>>>>>> #, fuzzy, python-format
>>>>>>>> msgid "%s Objects"
>>>>>>>> msgstr "Obiekty"
>>>>>>>>
>>>>>>>> The one below had 2 %s in msgstr and I corrected it to fix the
>>>>>>>> error.
>>>>>>>>
>>>>>>>> #: pgadmin/browser/server_groups/servers/roles/static/js/role.js:767
>>>>>>>> #, fuzzy, python-format
>>>>>>>> msgid "Are you sure you wish to %s all the objects owned by the
>>>>>>>> selected role?"
>>>>>>>> msgstr "Czy na pewno skasować %s \"%s\" i wszystkie obiekty zależne
>>>>>>>> od niego?"
>>>>>>>>
>>>>>>>>
>>>>>>>> You have to update the .po files to match the total %s and send the
>>>>>>>> patch.
>>>>>>>>
>>>>>>>> On Tue, Jun 29, 2021 at 1:56 PM Dave Page <[email protected]>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Tue, Jun 29, 2021 at 4:38 AM Rahul Shirsat <
>>>>>>>>> [email protected]> wrote:
>>>>>>>>>
>>>>>>>>>> I feel gettext sometimes won't escape the characters as it should
>>>>>>>>>> be.
>>>>>>>>>>
>>>>>>>>>> I now tried to escape those using some utils.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> That won't work either. The string being passed to gettext()
>>>>>>>>> *must* be in the gettext call.
>>>>>>>>>
>>>>>>>>> When gettext extracts strings to create/update the catalogs, it
>>>>>>>>> will search the code for all gettext calls, and then extract a string
>>>>>>>>> constant from the first argument. You cannot have variables, function calls
>>>>>>>>> or expressions in there. It *must* be a string constant.
>>>>>>>>>
>>>>>>>>> Keep in mind that msgextract is scanning the source code; it's not
>>>>>>>>> executing it. There are many examples in the code, e.g. (from node.js):
>>>>>>>>>
>>>>>>>>> title = gettext('Drop %s?', obj.label);
>>>>>>>>>
>>>>>>>>> I don't see anything obviously wrong with the existing code. Are
>>>>>>>>> you sure there are no weird unprintable characters in there?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Please find the updated patch.
>>>>>>>>>>
>>>>>>>>>> On Mon, Jun 28, 2021 at 9:33 PM Dave Page <[email protected]>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Jun 28, 2021 at 4:57 PM Rahul Shirsat <
>>>>>>>>>>> [email protected]> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>
>>>>>>>>>>>> Please find the attached patch for fixation of jenkins failure.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> That won't work - you can't include variables (or string
>>>>>>>>>>> building operations) in the first argument to gettext calls, as there won't
>>>>>>>>>>> be any way to extract a complete message into the catalogs. The way it's
>>>>>>>>>>> being done at the moment is correct (I don't know why it's failing, but
>>>>>>>>>>> it's the correct way to structure the gettext calls).
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Dave Page
>>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>
>>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> *Rahul Shirsat*
>>>>>>>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Dave Page
>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>
>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Thanks,
>>>>>>>> Aditya Toshniwal
>>>>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>>>>>> <http://edbpostgres.com;
>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Rahul Shirsat*
>>>>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EDB: https://www.enterprisedb.com
>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Aditya Toshniwal
>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>>> <http://edbpostgres.com;
>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: https://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EDB: https://www.enterprisedb.com
>>>>
>>>>
>>>
>>> --
>>> Thanks,
>>> Aditya Toshniwal
>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>> <http://edbpostgres.com;
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> Dave Page
>> Blog: https://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EDB: https://www.enterprisedb.com
>>
>>
>
> --
> *Rahul Shirsat*
> Senior Software Engineer | EnterpriseDB Corporation.
>


-- 
*Rahul Shirsat*
Senior Software Engineer | EnterpriseDB Corporation.


Attachments:

  [application/octet-stream] jenkins_failure_fix_v4.patch (3.0K, 3-jenkins_failure_fix_v4.patch)
  download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/roles/static/js/role.js b/web/pgadmin/browser/server_groups/servers/roles/static/js/role.js
index 557e44b7a..75b5acc9c 100644
--- a/web/pgadmin/browser/server_groups/servers/roles/static/js/role.js
+++ b/web/pgadmin/browser/server_groups/servers/roles/static/js/role.js
@@ -760,31 +760,37 @@ define('pgadmin.node.role', [
 
                   let roleReassignData = this.view.model.toJSON(),
                     roleOp = roleReassignData.role_op,
-                    confirmBoxTitle = utils.titleize(roleOp);
-
-                  alertify.confirm(
-                    gettext('%s Objects', confirmBoxTitle),
-                    gettext('Are you sure you wish to %s all the objects owned by the selected role?', roleOp),
-                    function() {
-                      axios.post(
-                        finalUrl,
-                        roleReassignData
-                      ).then(function (response) {
-                        if(response.data)
-                          alertify.success(response.data.info);
-                      }).catch(function (error) {
-                        try {
-                          const err = error.response.data;
-                          alertify.alert(
-                            gettext('Role reassign/drop failed.'),
-                            err.errormsg
-                          );
-                        } catch (e) {
-                          console.warn(e.stack || e);
-                        }
-                      });
-                    },
-                    function() { return true; }
+                    title, msg;
+
+                  if(roleOp == 'reassign') {
+                    title = gettext('Reassign Objects');
+                    msg = gettext('Are you sure you wish to reassign all the objects owned by the selected role?');
+                  }
+                  else {
+                    title = gettext('Drop Objects');
+                    msg = gettext('Are you sure you wish to drop all the objects owned by the selected role?');
+                  }
+
+                  alertify.confirm(title, msg, function() {
+                    axios.post(
+                      finalUrl,
+                      roleReassignData
+                    ).then(function (response) {
+                      if(response.data)
+                        alertify.success(response.data.info);
+                    }).catch(function (error) {
+                      try {
+                        const err = error.response.data;
+                        alertify.alert(
+                          gettext('Role reassign/drop failed.'),
+                          err.errormsg
+                        );
+                      } catch (e) {
+                        console.warn(e.stack || e);
+                      }
+                    });
+                  },
+                  function() { return true; }
                   ).set('labels', {
                     ok: gettext('Yes'),
                     cancel: gettext('No'),


view thread (22+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure
  In-Reply-To: <CAKtn9dMcVSjEwjeOfZC5Kt7CD-YUmQ=W9fFN4XXGSCLsT-oyzw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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