public inbox for [email protected]
help / color / mirror / Atom feedFrom: Akshay Joshi <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: RM #1250 Collection node counts
Date: Tue, 9 Aug 2016 16:30:05 +0530
Message-ID: <CANxoLDe9jmBzFpsXxOCe4EtqaogCfCN_GkY7MqV+yeK_M8novA@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxowLXixHXAfJx_2MOLDDUjvZTV7h8DZOQuTXbDcKHUdxQw@mail.gmail.com>
References: <CANxoLDd4-bmH9jtT1Ki4MD3kBmt15MF2yOWsa7LyGWcgo-kGzQ@mail.gmail.com>
<CA+OCxowgJNaBc2bAGamw6zB=v1sbpzU7cqF8RP_YbWzfA6PmHA@mail.gmail.com>
<CANxoLDd=7_1hv6kujs6AwiL3fO_DRXydWZeNxTQR6cKxn5kDbA@mail.gmail.com>
<CA+OCxowLXixHXAfJx_2MOLDDUjvZTV7h8DZOQuTXbDcKHUdxQw@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
Hi Dave
I have implemented the logic as per your suggestion. When user will expand
the collection node label will get updated with collection count. Attached
is the new patch file, please review it and let me know the review
comments.
On Mon, Aug 8, 2016 at 5:50 PM, Dave Page <[email protected]> wrote:
>
>
> On Mon, Aug 8, 2016 at 1:18 PM, Akshay Joshi <
> [email protected]> wrote:
>
>>
>>
>> On Mon, Aug 8, 2016 at 5:33 PM, Dave Page <[email protected]> wrote:
>>
>>> Hi
>>>
>>> On Mon, Aug 8, 2016 at 11:39 AM, Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Hi All
>>>>
>>>> I have fixed the RM#1250 "Collection node counts". To fix this RM I
>>>> need to do following changes
>>>>
>>>> - Move "check_precondition" function from module's view class to
>>>> global level within that python file itself, so that module class will use
>>>> it.
>>>> - Modified "get_nodes" function of each module's class, run the sql
>>>> query to count the number of objects and pass the count to
>>>> "generate_browser_collection_node" function to display the
>>>> collection count.
>>>> - Reuse SQL queries which is used to fetch nodes. Make that query
>>>> as inner query like "SELECT count(*) FROM( <node's> query ) AS
>>>> collection_count". For that I'll have to remove semicolon's from some of
>>>> the SQL queries.
>>>>
>>>> One case is not handled with this patch and that is on "Refresh" of
>>>> collection node, count is not updated. If user refresh the parent node then
>>>> it will be updated. I'll create a separate RM for that.
>>>>
>>>
>>> Sorry Akshay, but I really don't like the way you've done this. It seems
>>> like an unnecessarily large patch, and if I'm reading the patch correctly,
>>> it doubles the amount of SQL queries run against the database when
>>> navigating the tree, and introduces race conditions where the count
>>> displayed could be different from the actual number of nodes.
>>>
>>> I was expecting to see this implemented by watching for tree events
>>> (e.g. 'added' and 'removed') and using those events to update the label on
>>> the parent node, if that node is a collection. That should just be a few
>>> lines, and should be correct at all times right?
>>>
>>
>> With current implementation children's of any collection node will be
>> fetched/added when user will expand that collection node, in that case we
>> will update the label once the node gets expanded. For example initially we
>> will show "Databases" and when it gets expanded then we will update it to
>> "Databases (5)".
>>
>
> Right, but you also need to allow for removal and addition of children
> when the node is already expanded, and refreshes. Plus my other comments
> are still valid I believe - race condition, double the SQL and a very large
> change late in the beta cycle which isn't ideal.
>
>
>>
>>>
>>>>
>>>> Attached is the patch file. Please review it and let me know the review
>>>> comments.
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>> *Principal Software Engineer *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>>>
>>>>
>>>> --
>>>> Sent via pgadmin-hackers mailing list ([email protected])
>>>> To make changes to your subscription:
>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>>
>> --
>> *Akshay Joshi*
>> *Principal Software Engineer *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
--
*Akshay Joshi*
*Principal Software Engineer *
*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
--
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[application/octet-stream] RM_1250.patch (4.6K, 3-RM_1250.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
index 2d84870..ca0c269 100644
--- a/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
+++ b/web/pgadmin/browser/server_groups/servers/templates/servers/servers.js
@@ -237,6 +237,8 @@ function($, _, S, pgAdmin, pgBrowser, alertify) {
pgBrowser.serverInfo = pgBrowser.serverInfo || {};
pgBrowser.serverInfo[data._id] = _.extend({}, data);
+ // Call added method of node.js
+ pgAdmin.Browser.Node.callbacks.added.apply(this, arguments);
return true;
},
/* Reload configuration */
diff --git a/web/pgadmin/browser/server_groups/templates/server_groups/server_groups.js b/web/pgadmin/browser/server_groups/templates/server_groups/server_groups.js
index 4221280..316b1f7 100644
--- a/web/pgadmin/browser/server_groups/templates/server_groups/server_groups.js
+++ b/web/pgadmin/browser/server_groups/templates/server_groups/server_groups.js
@@ -10,6 +10,7 @@ function($, _, pgAdmin, Backbone) {
label: '{{ _('Server Group') }}',
width: '350px',
height: '250px',
+ is_collection: true,
Init: function() {
/* Avoid multiple registration of menus */
if (this.initialized)
diff --git a/web/pgadmin/browser/templates/browser/js/collection.js b/web/pgadmin/browser/templates/browser/js/collection.js
index a6c1aab..12c80b7 100644
--- a/web/pgadmin/browser/templates/browser/js/collection.js
+++ b/web/pgadmin/browser/templates/browser/js/collection.js
@@ -50,6 +50,7 @@ function($, _, S, pgAdmin, Backbone, Alertify, Backform) {
}
},
hasId: false,
+ is_collection: true,
// A collection will always have a collection of statistics, when the node
// it represent will have some statistics.
hasCollectiveStatistics: true,
diff --git a/web/pgadmin/browser/templates/browser/js/node.js b/web/pgadmin/browser/templates/browser/js/node.js
index 494240d..4cc8438 100644
--- a/web/pgadmin/browser/templates/browser/js/node.js
+++ b/web/pgadmin/browser/templates/browser/js/node.js
@@ -633,6 +633,27 @@ function($, _, S, pgAdmin, Menu, Backbone, Alertify, pgBrowser, Backform) {
this, [undefined, i]
);
},
+ added: function(item, data, browser) {
+ var b = browser || pgBrowser,
+ t = b.tree,
+ pItem = t.parent(item),
+ pData = pItem && t.itemData(pItem),
+ pNode = pData && pgBrowser.Nodes[pData._type];
+
+ // Check node is a collection or not.
+ if (pNode && pNode.is_collection) {
+ /* If 'collection_count' is not present in data
+ * it means tree node expanded first time, so we will
+ * kept collection count and label in data itself.
+ */
+ if (!('collection_count' in pData)) {
+ pData.collection_count = 0;
+ pData._label = pData.label;
+ }
+ pData.collection_count++;
+ t.setLabel(pItem, {label: (pData._label + ' <span>(' + pData.collection_count + ')</span>')});
+ }
+ },
// Callback called - when a node is selected in browser tree.
selected: function(item, data, browser) {
// Show the information about the selected node in the below panels,
@@ -691,9 +712,34 @@ function($, _, S, pgAdmin, Menu, Backbone, Alertify, pgBrowser, Backform) {
return true;
},
removed: function(item) {
- var self = this;
+ var self = this,
+ t = pgBrowser.tree,
+ pItem = t.parent(item),
+ pData = pItem && t.itemData(pItem),
+ pNode = pData && pgBrowser.Nodes[pData._type];
+
+ // Check node is a collection or not.
+ if (pNode && pNode.is_collection &&
+ 'collection_count' in pData)
+ {
+ pData.collection_count--;
+ t.setLabel(pItem, {label: (pData._label + ' <span>(' + pData.collection_count + ')</span>')});
+ }
+
setTimeout(function() { self.clear_cache.apply(self, item); }, 0);
},
+ unloaded: function(item) {
+ var self = this,
+ t = pgBrowser.tree,
+ data = item && t.itemData(item);
+
+ // In case of unload remove the collection counter
+ if (self.is_collection && 'collection_count' in data)
+ {
+ delete data.collection_count;
+ t.setLabel(item, {label: data._label});
+ }
+ },
refresh: function(n, i) {
var self = this,
t = pgBrowser.tree,
view thread (6+ 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]
Subject: Re: RM #1250 Collection node counts
In-Reply-To: <CANxoLDe9jmBzFpsXxOCe4EtqaogCfCN_GkY7MqV+yeK_M8novA@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