On Tue, 2011-08-09 at 00:30 +0200, Guillaume Lelarge wrote:
> On Sun, 2011-07-24 at 15:09 +0100, Dave Page wrote:
> > On Sun, Jul 24, 2011 at 2:23 PM, Guillaume Lelarge
> > <guilla...@lelarge.info> wrote:
> > > On Sun, 2011-07-24 at 14:11 +0200, Guillaume Lelarge wrote:
> > >> On Sun, 2011-07-24 at 12:58 +0100, Dave Page wrote:
> > >> > The former fix seems more appropriate. Operations in frmMain shouldn't
> > >> > cause dialogues to close.
> > >> >
> > >>
> > >> Fine with me. That should be simpler to do.
> > >>
> > >> > Simple fix seems to be to store a pointer to the dialog in pgObject;
> > >> > if not null, disallow drop, or if trying to view properties, use the
> > >> > pointer to raise the existing dialogue. Clear the pointer when the
> > >> > dialog is closed.
> > >> >
> > >>
> > >> I thought of something similar, but this is OK with me.
> > >>
> > >> I expect it to be quite a big patch. Should I work on 1.14 or master?
> > >> this is surely a bug, so 1.14 should be OK. But I'm afraid this is too
> > >> much changes to code it on a beta release. Any strong opinion on this
> > >> matter?
> > >>
> > >
> > > And another question. Let's say I open a table properties' dialog, and I
> > > try to refresh the whole database. Should it refresh everything except
> > > this table, or nothing and complains about properties' dialogs being
> > > open, preventing the object to be refreshed?
> >
> > Nothing. "There are properties dialogues open for one or more objects
> > that would be refreshed. Please close the properties dialogues and try
> > again."
> >
>
> Just a quick update. Still working on it, and I have good results so
> far. I'm now able to prevent refresh and drop (refresh for all objects,
> drop for schemas only) if an object's properties dialog is open.
>
> Patch attached. Obviously WIP. Not so obvious, it's dirty code (some
> wxLogError, not really efficient code, etc). But comments very welcome.
>
Seems it hits final release. See attached patch. Protects against server
or database disconnection, and browser refresh. If a user tries to open
many times the same object properties, it will raise the previous window
rather than opening a new one.
I think it's pretty neat :) It's been a huge pgAdmin's issue for a long
time.
Any comments before I commit it?
--
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com
>From 0494c55cfc140754aaeb4640cba88831a159c8b0 Mon Sep 17 00:00:00 2001
From: Guillaume Lelarge <guilla...@lelarge.info>
Date: Sun, 24 Jul 2011 18:24:13 +0200
Subject: [PATCH] Prevent refreshing or dropping an object
... when its property window is opened.
Should work with server disconnection, database disconnection, and objects
refresh. If an object's properties window is already opened, it'll raise this
window, and not open another one.
Per many requests, last one being from Bogdan Timofte.
---
pgadmin/dlg/dlgProperty.cpp | 71 +++++++++++++------
pgadmin/frm/events.cpp | 7 ++
pgadmin/frm/frmMain.cpp | 135 ++++++++++++++++++++-----------------
pgadmin/include/dlg/dlgProperty.h | 1 +
pgadmin/include/schema/pgObject.h | 9 +++
pgadmin/schema/pgDatabase.cpp | 18 ++++--
pgadmin/schema/pgObject.cpp | 33 +++++++++
pgadmin/schema/pgServer.cpp | 20 ++++--
8 files changed, 201 insertions(+), 93 deletions(-)
diff --git a/pgadmin/dlg/dlgProperty.cpp b/pgadmin/dlg/dlgProperty.cpp
index 85d35b0..df996af 100644
--- a/pgadmin/dlg/dlgProperty.cpp
+++ b/pgadmin/dlg/dlgProperty.cpp
@@ -165,6 +165,9 @@ dlgProperty::~dlgProperty()
if (GetWindowStyle() & wxRESIZE_BORDER)
settings->WriteSize(prop, GetSize());
+
+ if (obj)
+ obj->SetWindowPtr(NULL);
}
@@ -242,7 +245,7 @@ int dlgProperty::Go(bool modal)
{
wxASSERT(factory != 0);
- pgObject *obj = mainForm->GetBrowser()->GetObject(mainForm->GetBrowser()->GetSelection());
+ obj = mainForm->GetBrowser()->GetObject(mainForm->GetBrowser()->GetSelection());
// restore previous position and size, if applicable
wxString prop = wxT("Properties/") + wxString(factory->GetTypeName());
@@ -1209,6 +1212,9 @@ dlgProperty *dlgProperty::CreateDlg(frmMain *frame, pgObject *node, bool asNew,
wxASSERT(factory);
dlg->InitDialog(frame, node);
+
+ if (currentNode)
+ currentNode->SetWindowPtr(dlg);
}
}
return dlg;
@@ -1223,18 +1229,29 @@ bool dlgProperty::CreateObjectDialog(frmMain *frame, pgObject *node, pgaFactory
if (!conn || conn->GetStatus() != PGCONN_OK || !conn->IsAlive())
return false;
}
- dlgProperty *dlg = CreateDlg(frame, node, true, factory);
- if (dlg)
- {
- dlg->SetTitle(wxGetTranslation(dlg->factory->GetNewString()));
+ dlgProperty *dlg = NULL;
- dlg->CreateAdditionalPages();
- dlg->Go();
- dlg->CheckChange();
- }
- else
- wxMessageBox(_("Not implemented."));
+ if (node)
+ dlg = node->GetWindowPtr();
+
+ if (dlg)
+ dlg->Raise();
+ else
+ {
+ dlg = CreateDlg(frame, node, true, factory);
+
+ if (dlg)
+ {
+ dlg->SetTitle(wxGetTranslation(dlg->factory->GetNewString()));
+
+ dlg->CreateAdditionalPages();
+ dlg->Go();
+ dlg->CheckChange();
+ }
+ else
+ wxMessageBox(_("Not implemented."));
+ }
return true;
}
@@ -1257,20 +1274,30 @@ bool dlgProperty::EditObjectDialog(frmMain *frame, ctlSQLBox *sqlbox, pgObject *
return false;
}
- dlgProperty *dlg = CreateDlg(frame, node, false);
+ dlgProperty *dlg = NULL;
- if (dlg)
- {
- wxString typeName = dlg->factory->GetTypeName();
- dlg->SetTitle(wxString(wxGetTranslation(typeName)) + wxT(" ") + node->GetFullIdentifier());
+ if (node)
+ dlg = node->GetWindowPtr();
- dlg->CreateAdditionalPages();
- dlg->Go();
+ if (dlg)
+ dlg->Raise();
+ else
+ {
+ dlg = CreateDlg(frame, node, false);
- dlg->CheckChange();
- }
- else
- wxMessageBox(_("Not implemented."));
+ if (dlg)
+ {
+ wxString typeName = dlg->factory->GetTypeName();
+ dlg->SetTitle(wxString(wxGetTranslation(typeName)) + wxT(" ") + node->GetFullIdentifier());
+
+ dlg->CreateAdditionalPages();
+ dlg->Go();
+
+ dlg->CheckChange();
+ }
+ else
+ wxMessageBox(_("Not implemented."));
+ }
return true;
}
diff --git a/pgadmin/frm/events.cpp b/pgadmin/frm/events.cpp
index f725d39..4ddfae2 100644
--- a/pgadmin/frm/events.cpp
+++ b/pgadmin/frm/events.cpp
@@ -801,6 +801,13 @@ bool frmMain::dropSingleObject(pgObject *data, bool updateFinal, bool cascaded)
if (!data || !data->CanDrop())
return false;
+ if (data->GetWindowPtr())
+ {
+ wxString msg = _("There are properties dialogues open for one or more objects that would be refreshed. Please close the properties dialogues and try again.");
+ wxMessageBox(msg, _("Cannot drop object"), wxICON_WARNING | wxOK);
+ return false;
+ }
+
if (data->GetSystemObject())
{
wxMessageDialog msg(this, data->GetTranslatedMessage(CANNOTDROPSYSTEM),
diff --git a/pgadmin/frm/frmMain.cpp b/pgadmin/frm/frmMain.cpp
index d59a9d1..2a11fc5 100644
--- a/pgadmin/frm/frmMain.cpp
+++ b/pgadmin/frm/frmMain.cpp
@@ -510,71 +510,84 @@ void frmMain::CreateMenus()
void frmMain::Refresh(pgObject *data)
{
+ bool done = false;
+ pgObject *obj = NULL;
+
StartMsg(data->GetTranslatedMessage(REFRESHINGDETAILS));
browser->Freeze();
wxTreeItemId currentItem = data->GetId();
-
- // Scan the child nodes and make a list of those that are expanded
- // This is not an exact science as node names may change etc.
- wxArrayString expandedNodes;
- GetExpandedChildNodes(currentItem, expandedNodes);
-
- browser->DeleteChildren(currentItem);
-
- // refresh information about the object
- data->SetDirty();
-
- pgObject *newData = data->Refresh(browser, currentItem);
-
- bool done = !data->GetConnection() || data->GetConnection()->GetStatus() == PGCONN_OK;
-
- if (newData != data)
- {
- wxLogInfo(wxT("Deleting %s %s for refresh"), data->GetTypeName().c_str(), data->GetQuotedFullIdentifier().c_str());
-
- if (data == currentObject)
- currentObject = newData;
-
- if (newData)
- {
- wxLogInfo(wxT("Replacing with new node %s %s for refresh"), newData->GetTypeName().c_str(), newData->GetQuotedFullIdentifier().c_str());
-
- newData->SetId(currentItem); // not done automatically
- browser->SetItemData(currentItem, newData);
-
- // Update the node text if this is an object, as it may have been renamed
- if (!newData->IsCollection())
- browser->SetItemText(currentItem, newData->GetDisplayName());
-
- delete data;
- }
- else
- {
- wxLogInfo(wxT("No object to replace: vanished after refresh."));
-
- // If the connection is dead, just return here
- if (data->GetConnection()->GetStatus() != PGCONN_OK)
- {
- CheckAlive();
- browser->Thaw();
- return;
- }
-
- wxTreeItemId delItem = currentItem;
- currentItem = browser->GetItemParent(currentItem);
- browser->SelectItem(currentItem);
- browser->Delete(delItem);
- }
- }
-
- if (currentItem)
- {
- execSelChange(currentItem, currentItem == browser->GetSelection());
-
- // Attempt to expand any child nodes that were previously expanded
- ExpandChildNodes(currentItem, expandedNodes);
- }
+ if (currentItem)
+ obj = browser->GetObject(currentItem);
+
+ if (obj && obj->CheckOpenDialogs(browser, currentItem))
+ {
+ wxString msg = _("There are properties dialogues open for one or more objects that would be refreshed. Please close the properties dialogues and try again.");
+ wxMessageBox(msg, _("Cannot refresh browser"), wxICON_WARNING | wxOK);
+ }
+ else
+ {
+ // Scan the child nodes and make a list of those that are expanded
+ // This is not an exact science as node names may change etc.
+ wxArrayString expandedNodes;
+ GetExpandedChildNodes(currentItem, expandedNodes);
+
+ browser->DeleteChildren(currentItem);
+
+ // refresh information about the object
+ data->SetDirty();
+
+ pgObject *newData = data->Refresh(browser, currentItem);
+
+ done = !data->GetConnection() || data->GetConnection()->GetStatus() == PGCONN_OK;
+
+ if (newData != data)
+ {
+ wxLogInfo(wxT("Deleting %s %s for refresh"), data->GetTypeName().c_str(), data->GetQuotedFullIdentifier().c_str());
+
+ if (data == currentObject)
+ currentObject = newData;
+
+ if (newData)
+ {
+ wxLogInfo(wxT("Replacing with new node %s %s for refresh"), newData->GetTypeName().c_str(), newData->GetQuotedFullIdentifier().c_str());
+
+ newData->SetId(currentItem); // not done automatically
+ browser->SetItemData(currentItem, newData);
+
+ // Update the node text if this is an object, as it may have been renamed
+ if (!newData->IsCollection())
+ browser->SetItemText(currentItem, newData->GetDisplayName());
+
+ delete data;
+ }
+ else
+ {
+ wxLogInfo(wxT("No object to replace: vanished after refresh."));
+
+ // If the connection is dead, just return here
+ if (data->GetConnection()->GetStatus() != PGCONN_OK)
+ {
+ CheckAlive();
+ browser->Thaw();
+ return;
+ }
+
+ wxTreeItemId delItem = currentItem;
+ currentItem = browser->GetItemParent(currentItem);
+ browser->SelectItem(currentItem);
+ browser->Delete(delItem);
+ }
+ }
+
+ if (currentItem)
+ {
+ execSelChange(currentItem, currentItem == browser->GetSelection());
+
+ // Attempt to expand any child nodes that were previously expanded
+ ExpandChildNodes(currentItem, expandedNodes);
+ }
+ }
browser->Thaw();
EndMsg(done);
diff --git a/pgadmin/include/dlg/dlgProperty.h b/pgadmin/include/dlg/dlgProperty.h
index 42e400c..73d0ec7 100644
--- a/pgadmin/include/dlg/dlgProperty.h
+++ b/pgadmin/include/dlg/dlgProperty.h
@@ -167,6 +167,7 @@ protected:
pgConn *connection;
pgDatabase *database;
+ pgObject *obj;
frmMain *mainForm;
wxPanel *sqlPane;
diff --git a/pgadmin/include/schema/pgObject.h b/pgadmin/include/schema/pgObject.h
index 0562b32..4cd928c 100644
--- a/pgadmin/include/schema/pgObject.h
+++ b/pgadmin/include/schema/pgObject.h
@@ -351,6 +351,14 @@ public:
wxString qtDbString(const wxString &str);
+ void SetWindowPtr(dlgProperty *dlgprop);
+ dlgProperty* GetWindowPtr()
+ {
+ return dlg;
+ }
+
+ bool CheckOpenDialogs(ctlTree *browser, wxTreeItemId node);
+
protected:
void CreateList3Columns(ctlListView *properties, const wxString &left = _("Object"), const wxString &middle = _("Owner"), const wxString &right = _("Value"));
void CreateListColumns(ctlListView *properties, const wxString &left = _("Property"), const wxString &right = _("Value"));
@@ -375,6 +383,7 @@ private:
int type;
OID oid, xid;
wxString providers, labels;
+ dlgProperty *dlg;
friend class pgaFactory;
};
diff --git a/pgadmin/schema/pgDatabase.cpp b/pgadmin/schema/pgDatabase.cpp
index 86898e7..8eac72b 100644
--- a/pgadmin/schema/pgDatabase.cpp
+++ b/pgadmin/schema/pgDatabase.cpp
@@ -1251,11 +1251,19 @@ wxWindow *disconnectDatabaseFactory::StartDialog(frmMain *form, pgObject *obj)
ctlTree *browser = form->GetBrowser();
pgDatabase *database = (pgDatabase *)obj;
- database->Disconnect();
- database->UpdateIcon(browser);
- browser->DeleteChildren(obj->GetId());
- browser->SelectItem(browser->GetItemParent(obj->GetId()));
- form->execSelChange(browser->GetItemParent(obj->GetId()), true);
+ if (obj->CheckOpenDialogs(browser, browser->GetSelection()))
+ {
+ wxString msg = _("There are properties dialogues open for one or more objects that would be refreshed. Please close the properties dialogues and try again.");
+ wxMessageBox(msg, _("Cannot disconnect database"), wxICON_WARNING | wxOK);
+ }
+ else
+ {
+ database->Disconnect();
+ database->UpdateIcon(browser);
+ browser->DeleteChildren(obj->GetId());
+ browser->SelectItem(browser->GetItemParent(obj->GetId()));
+ form->execSelChange(browser->GetItemParent(obj->GetId()), true);
+ }
return 0;
}
diff --git a/pgadmin/schema/pgObject.cpp b/pgadmin/schema/pgObject.cpp
index e339c2f..1de2fd6 100644
--- a/pgadmin/schema/pgObject.cpp
+++ b/pgadmin/schema/pgObject.cpp
@@ -155,6 +155,7 @@ pgObject::pgObject(pgaFactory &_factory, const wxString &newName)
expandedKids = false;
needReread = false;
hintShown = false;
+ dlg = NULL;
}
@@ -173,6 +174,7 @@ pgObject::pgObject(int newType, const wxString &newName)
expandedKids = false;
needReread = false;
hintShown = false;
+ dlg = NULL;
}
@@ -1069,6 +1071,33 @@ pgConn *pgObject::GetConnection() const
}
+bool pgObject::CheckOpenDialogs(ctlTree *browser, wxTreeItemId node)
+{
+ pgObject *obj = browser->GetObject(node);
+ if (obj && obj->GetWindowPtr())
+ return true;
+
+ wxTreeItemIdValue cookie;
+ wxTreeItemId child = browser->GetFirstChild(node, cookie);
+
+ while (child.IsOk())
+ {
+ obj = browser->GetObject(child);
+ if (obj && obj->GetWindowPtr())
+ return true;
+
+ if (browser->IsExpanded(child))
+ {
+ if (CheckOpenDialogs(browser, child))
+ return true;
+ }
+
+ child = browser->GetNextChild(node, cookie);
+ }
+
+ return false;
+}
+
//////////////////////////////////////////////////////////////
bool pgServerObject::CanDrop()
@@ -1796,4 +1825,8 @@ wxString pgObject::GetPrivilegeName(wxChar privilege)
}
}
+void pgObject::SetWindowPtr(dlgProperty *dlgprop)
+{
+ dlg = dlgprop;
+}
diff --git a/pgadmin/schema/pgServer.cpp b/pgadmin/schema/pgServer.cpp
index 92d5d11..ffea42c 100644
--- a/pgadmin/schema/pgServer.cpp
+++ b/pgadmin/schema/pgServer.cpp
@@ -1808,6 +1808,7 @@ wxWindow *stopServiceFactory::StartDialog(frmMain *form, pgObject *obj)
}
form->EndMsg(done);
}
+
return 0;
}
@@ -1854,11 +1855,20 @@ disconnectServerFactory::disconnectServerFactory(menuFactoryList *list, wxMenu *
wxWindow *disconnectServerFactory::StartDialog(frmMain *form, pgObject *obj)
{
- pgServer *server = (pgServer *)obj;
- server->Disconnect(form);
- server->UpdateIcon(form->GetBrowser());
- form->GetBrowser()->DeleteChildren(obj->GetId());
- form->execSelChange(obj->GetId(), true);
+ if (obj->CheckOpenDialogs(form->GetBrowser(), form->GetBrowser()->GetSelection()))
+ {
+ wxString msg = _("There are properties dialogues open for one or more objects that would be refreshed. Please close the properties dialogues and try again.");
+ wxMessageBox(msg, _("Cannot disconnect database"), wxICON_WARNING | wxOK);
+ }
+ else
+ {
+ pgServer *server = (pgServer *)obj;
+ server->Disconnect(form);
+ server->UpdateIcon(form->GetBrowser());
+ form->GetBrowser()->DeleteChildren(obj->GetId());
+ form->execSelChange(obj->GetId(), true);
+ }
+
return 0;
}
--
1.7.6
--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
