Thread: A new feature patch and a bug fix

Hi,

Let's begin with the bug fix. I've found that I can set the owner of an
object only from login role. pgAdmin doesn't allow me to set ownership
to a group. psql allows it, so I think this is a bug. The patch named
"dlgProperty.patch" fixes this issue.

I found this when I tried to add "reassign owned by" and "drop owned by"
functionalities to pgAdmin. I first thought I would add this with the
drop-user code but I prefer to make it available alone. So I used my
well known way : a new contextual menu (in fact two : "Reassign Owned
To..." and "Drop Owned"). The first one opens a wxSingleChoiceDialog
dialog for the user to choose a role from. The second one waits for a
confirmation and then fires the SQL statement. My problem is here : on a
user contextual menu, the only connection I can get is the maintenance
connection. So I will only be able to reassign or drop objects within
the maintenance database. I was wondering if there was a way to get a
handle to the last open connection ? or if there was another way ? for
example asking the user to choose the database to connect to and fire
the SQL statement ? or asking the user to select a database where he's
already connected to ?

Thanks.

Regards.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com
Index: pgadmin/dlg/dlgProperty.cpp
===================================================================
--- pgadmin/dlg/dlgProperty.cpp    (révision 6910)
+++ pgadmin/dlg/dlgProperty.cpp    (copie de travail)
@@ -432,7 +432,14 @@

 void dlgProperty::AddUsers(ctlComboBoxFix *cb1, ctlComboBoxFix *cb2)
 {
-    FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY usename"), cb1, cb2);
+    if (connection->BackendMinimumVersion(8, 2))
+    {
+        FillCombobox(wxT("SELECT rolname FROM pg_roles ORDER BY 1"), cb1, cb2);
+    }
+    else
+    {
+        FillCombobox(wxT("SELECT usename FROM pg_user UNION SELECT groname FROM pg_group ORDER BY 1"), cb1, cb2);
+    }
 }


Index: pgadmin/include/schema/pgRole.h
===================================================================
--- pgadmin/include/schema/pgRole.h    (révision 6910)
+++ pgadmin/include/schema/pgRole.h    (copie de travail)
@@ -75,8 +75,10 @@
     void iSetUpdateCatalog(const bool b) { updateCatalog=b; }
     wxArrayString& GetRolesIn() { return rolesIn; }
     wxArrayString& GetConfigList() { return configList; }
+
+    void ReassignOwnedTo();
+    void DropOwned();

-
     // Tree object creation
     void ShowTreeDetail(ctlTree *browser, frmMain *form=0, ctlListView *properties=0, ctlSQLBox *sqlPane=0);
     void ShowDependents(frmMain *form, ctlListView *referencedBy, const wxString &where);
@@ -89,6 +91,7 @@
     bool HasStats() { return false; }
     bool HasDepends() { return true; }
     bool HasReferences() { return true; }
+
 private:
     wxString password;
     wxDateTime accountExpires;
@@ -112,6 +115,21 @@
     pgGroupRole(const wxString& newName = wxT(""));
 };

+class reassignOwnedFactory : public contextActionFactory
+{
+public:
+    reassignOwnedFactory(menuFactoryList *list, wxMenu *mnu, wxToolBar *toolbar);
+    wxWindow *StartDialog(frmMain *form, pgObject *obj);
+    bool CheckEnable(pgObject *obj);
+};

+class dropOwnedFactory : public contextActionFactory
+{
+public:
+    dropOwnedFactory(menuFactoryList *list, wxMenu *mnu, wxToolBar *toolbar);
+    wxWindow *StartDialog(frmMain *form, pgObject *obj);
+    bool CheckEnable(pgObject *obj);
+};

+
 #endif
Index: pgadmin/frm/frmMain.cpp
===================================================================
--- pgadmin/frm/frmMain.cpp    (révision 6910)
+++ pgadmin/frm/frmMain.cpp    (copie de travail)
@@ -67,6 +67,7 @@
 #include "schema/pgTable.h"
 #include "schema/pgIndex.h"
 #include "schema/pgTrigger.h"
+#include "schema/pgRole.h"
 #include "schema/pgRule.h"
 #include "schema/pgServer.h"
 #include "slony/slCluster.h"
@@ -326,6 +327,8 @@
     new dropCascadedFactory(menuFactories, editMenu, 0);
     new truncateFactory(menuFactories, editMenu, 0);
     new truncateCascadedFactory(menuFactories, editMenu, 0);
+    new reassignOwnedFactory(menuFactories, editMenu, 0);
+    new dropOwnedFactory(menuFactories, editMenu, 0);
     editMenu->AppendSeparator();

     new separatorFactory(menuFactories);
Index: pgadmin/schema/pgRole.cpp
===================================================================
--- pgadmin/schema/pgRole.cpp    (révision 6910)
+++ pgadmin/schema/pgRole.cpp    (copie de travail)
@@ -11,6 +11,7 @@

 // wxWindows headers
 #include <wx/wx.h>
+#include <wx/choicdlg.h>

 // App headers
 #include "pgAdmin3.h"
@@ -51,7 +52,7 @@

 bool pgRole::DropObject(wxFrame *frame, ctlTree *browser, bool cascaded)
 {
-    if (GetUpdateCatalog())
+    if (GetUpdateCatalog())
     {
         wxMessageDialog dlg(frame,
             _("Deleting a superuser might result in unwanted behaviour (e.g. when restoring the database).\nAre you
sure?"),
@@ -277,7 +278,38 @@
 }


+void pgRole::ReassignOwnedTo()
+{
+    wxArrayString rolesList;
+    wxString rolesquery;
+
+    rolesquery = wxT("SELECT rolname FROM pg_roles ORDER BY rolname");
+    pgSetIterator roles(GetConnection(), rolesquery);
+    while (roles.RowsLeft())
+    {
+        rolesList.Add(roles.GetVal(wxT("rolname")));
+    }
+
+    wxSingleChoiceDialog dialog(0,
+      wxT("Choose the user that will own all objects owned by the selected role."),
+      wxT("Please select a role"),
+      rolesList);
+
+    if (dialog.ShowModal() == wxID_OK)
+    {
+        rolesquery = wxT("REASSIGN OWNED BY ") + GetQuotedFullIdentifier() + wxT(" TO ") +
dialog.GetStringSelection();
+        GetConnection()->ExecuteVoid(rolesquery);
+    }
+}

+
+void pgRole::DropOwned()
+{
+    wxString query = wxT("DROP OWNED BY ") + GetQuotedFullIdentifier();
+    GetConnection()->ExecuteVoid(query);
+}
+
+
 pgObject *pgRole::Refresh(ctlTree *browser, const wxTreeItemId item)
 {
     pgObject *role=0;
@@ -394,3 +426,43 @@

 pgGroupRoleFactory groupRoleFactory;
 static pgaCollectionFactory gcf(&groupRoleFactory, __("Group Roles"), roles_xpm);
+
+
+reassignOwnedFactory::reassignOwnedFactory(menuFactoryList *list, wxMenu *mnu, wxToolBar *toolbar) :
contextActionFactory(list)
+{
+    mnu->Append(id, _("Reassign Owned To..."), _("Reassigned objects owned by this role to another one."));
+}
+
+
+wxWindow *reassignOwnedFactory::StartDialog(frmMain *form, pgObject *obj)
+{
+    ((pgRole*)obj)->ReassignOwnedTo();
+
+    return 0;
+}
+
+bool reassignOwnedFactory::CheckEnable(pgObject *obj)
+{
+    return obj && obj->IsCreatedBy(loginRoleFactory) && ((pgRole*)obj)->GetConnection()->BackendMinimumVersion(8, 2);
+}
+
+
+dropOwnedFactory::dropOwnedFactory(menuFactoryList *list, wxMenu *mnu, wxToolBar *toolbar) :
contextActionFactory(list)
+{
+    mnu->Append(id, _("Drop Owned"), _("Drop objects owned by the selected role."));
+}
+
+wxWindow *dropOwnedFactory::StartDialog(frmMain *form, pgObject *obj)
+{
+    if (wxMessageBox(_("Are you sure you wish to drop all objets owned by the selected role?"), _("Drop objects"),
wxYES_NO)== wxNO) 
+        return 0;
+
+    ((pgRole*)obj)->DropOwned();
+
+    return 0;
+}
+
+bool dropOwnedFactory::CheckEnable(pgObject *obj)
+{
+    return obj && obj->IsCreatedBy(loginRoleFactory) && ((pgRole*)obj)->GetConnection()->BackendMinimumVersion(8, 2);
+}

Guillaume Lelarge wrote:
> Hi,
>
> Let's begin with the bug fix. I've found that I can set the owner of an
> object only from login role. pgAdmin doesn't allow me to set ownership
> to a group. psql allows it, so I think this is a bug. The patch named
> "dlgProperty.patch" fixes this issue.

OK... but can you really assign ownership to a group as well as a group
role? For example, from:
http://www.postgresql.org/docs/8.0/interactive/sql-altertable.html

----
This form changes the owner of the table, index, sequence, or view to
the specified user.
----

That implies just users to me.

> I found this when I tried to add "reassign owned by" and "drop owned by"
> functionalities to pgAdmin. I first thought I would add this with the
> drop-user code but I prefer to make it available alone. So I used my
> well known way : a new contextual menu (in fact two : "Reassign Owned
> To..." and "Drop Owned"). The first one opens a wxSingleChoiceDialog
> dialog for the user to choose a role from. The second one waits for a
> confirmation and then fires the SQL statement. My problem is here : on a
> user contextual menu, the only connection I can get is the maintenance
> connection. So I will only be able to reassign or drop objects within
> the maintenance database. I was wondering if there was a way to get a
> handle to the last open connection ? or if there was another way ? for
> example asking the user to choose the database to connect to and fire
> the SQL statement ? or asking the user to select a database where he's
> already connected to ?

Yeah, I think that's going to be the only way - present a list of
databases on the server. Or do it the other way round and add the
functionality to the database object and present a list of roles to
choose from.

/D

Dave Page wrote:
> Guillaume Lelarge wrote:
>> Let's begin with the bug fix. I've found that I can set the owner of an
>> object only from login role. pgAdmin doesn't allow me to set ownership
>> to a group. psql allows it, so I think this is a bug. The patch named
>> "dlgProperty.patch" fixes this issue.
>
> OK... but can you really assign ownership to a group as well as a group
> role? For example, from:
> http://www.postgresql.org/docs/8.0/interactive/sql-altertable.html
>
> ----
> This form changes the owner of the table, index, sequence, or view to
> the specified user.
> ----
>
> That implies just users to me.
>

I did it for a group, works great (8.1 and 8.2). Now, I don't really
know if this is really a feature or something overlooked. Either pgAdmin
is wrong either psql is.

>> I found this when I tried to add "reassign owned by" and "drop owned by"
>> functionalities to pgAdmin. I first thought I would add this with the
>> drop-user code but I prefer to make it available alone. So I used my
>> well known way : a new contextual menu (in fact two : "Reassign Owned
>> To..." and "Drop Owned"). The first one opens a wxSingleChoiceDialog
>> dialog for the user to choose a role from. The second one waits for a
>> confirmation and then fires the SQL statement. My problem is here : on a
>> user contextual menu, the only connection I can get is the maintenance
>> connection. So I will only be able to reassign or drop objects within
>> the maintenance database. I was wondering if there was a way to get a
>> handle to the last open connection ? or if there was another way ? for
>> example asking the user to choose the database to connect to and fire
>> the SQL statement ? or asking the user to select a database where he's
>> already connected to ?
>
> Yeah, I think that's going to be the only way - present a list of
> databases on the server. Or do it the other way round and add the
> functionality to the database object and present a list of roles to
> choose from.
>

I'll take a look at the query tool to see how to do this ("present a
list of databases on the server"). Another call to wxSingleChoiceDialod
seems a bad way to me... I'll need to add a new dialog, don't you think ?


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Guillaume Lelarge wrote:
> I did it for a group, works great (8.1 and 8.2). Now, I don't really
> know if this is really a feature or something overlooked. Either pgAdmin
> is wrong either psql is.

No, in 8.1+ it works for roles whether they are what we would call login
roles or group roles. In 8.0 and below we had separate users and groups,
but I'm not sure you could assign ownership to a group back then.

> I'll take a look at the query tool to see how to do this ("present a
> list of databases on the server"). Another call to wxSingleChoiceDialod
> seems a bad way to me... I'll need to add a new dialog, don't you think ?

Yes, I don't think it should be two dialogues.

/D

Dave Page wrote:
> Guillaume Lelarge wrote:
>> I did it for a group, works great (8.1 and 8.2). Now, I don't really
>> know if this is really a feature or something overlooked. Either pgAdmin
>> is wrong either psql is.
>
> No, in 8.1+ it works for roles whether they are what we would call login
> roles or group roles. In 8.0 and below we had separate users and groups,
> but I'm not sure you could assign ownership to a group back then.
>

You're right. I checked this on a 7.4 (I don't have a 8.0 release
available) and, with this release, a group can't be the owner of a table.

Do you think I should raise the issue (group owner on 8.1+) on
pgsql-hackers ?

>> I'll take a look at the query tool to see how to do this ("present a
>> list of databases on the server"). Another call to wxSingleChoiceDialod
>> seems a bad way to me... I'll need to add a new dialog, don't you think ?
>
> Yes, I don't think it should be two dialogues.
>

I'm working on it now.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Guillaume Lelarge wrote:
> Dave Page wrote:
>> Guillaume Lelarge wrote:
>>> I did it for a group, works great (8.1 and 8.2). Now, I don't really
>>> know if this is really a feature or something overlooked. Either pgAdmin
>>> is wrong either psql is.
>> No, in 8.1+ it works for roles whether they are what we would call login
>> roles or group roles. In 8.0 and below we had separate users and groups,
>> but I'm not sure you could assign ownership to a group back then.
>>
>
> You're right. I checked this on a 7.4 (I don't have a 8.0 release
> available) and, with this release, a group can't be the owner of a table.
>
> Do you think I should raise the issue (group owner on 8.1+) on
> pgsql-hackers ?

No - I think it just works because pg_group is a view over pg_role (or
whatever it's called ) on 8.1. I would try something like:

Index: pgadmin/dlg/dlgProperty.cpp
===================================================================
--- pgadmin/dlg/dlgProperty.cpp    (révision 6910)
+++ pgadmin/dlg/dlgProperty.cpp    (copie de travail)
@@ -432,7 +432,14 @@

 void dlgProperty::AddUsers(ctlComboBoxFix *cb1, ctlComboBoxFix *cb2)
 {
-    FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY usename"),
cb1, cb2);
+    if (connection->BackendMinimumVersion(8, 1))
+    {
+        FillCombobox(wxT("SELECT rolname FROM pg_roles ORDER BY 1"),
cb1, cb2);
+    }
+    else
+    {
+        FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY 1"),
cb1, cb2);
+    }
 }


>>> I'll take a look at the query tool to see how to do this ("present a
>>> list of databases on the server"). Another call to wxSingleChoiceDialod
>>> seems a bad way to me... I'll need to add a new dialog, don't you think ?
>> Yes, I don't think it should be two dialogues.
>>
>
> I'm working on it now.

'K.

/D

Dave Page wrote:
> Guillaume Lelarge wrote:
>> Dave Page wrote:
>>> Guillaume Lelarge wrote:
>>>> I did it for a group, works great (8.1 and 8.2). Now, I don't really
>>>> know if this is really a feature or something overlooked. Either pgAdmin
>>>> is wrong either psql is.
>>> No, in 8.1+ it works for roles whether they are what we would call login
>>> roles or group roles. In 8.0 and below we had separate users and groups,
>>> but I'm not sure you could assign ownership to a group back then.
>>>
>> You're right. I checked this on a 7.4 (I don't have a 8.0 release
>> available) and, with this release, a group can't be the owner of a table.
>>
>> Do you think I should raise the issue (group owner on 8.1+) on
>> pgsql-hackers ?
>
> No - I think it just works because pg_group is a view over pg_role (or
> whatever it's called ) on 8.1. I would try something like:
>
> Index: pgadmin/dlg/dlgProperty.cpp
> ===================================================================
> --- pgadmin/dlg/dlgProperty.cpp    (révision 6910)
> +++ pgadmin/dlg/dlgProperty.cpp    (copie de travail)
> @@ -432,7 +432,14 @@
>
>  void dlgProperty::AddUsers(ctlComboBoxFix *cb1, ctlComboBoxFix *cb2)
>  {
> -    FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY usename"),
> cb1, cb2);
> +    if (connection->BackendMinimumVersion(8, 1))
> +    {
> +        FillCombobox(wxT("SELECT rolname FROM pg_roles ORDER BY 1"),
> cb1, cb2);
> +    }
> +    else
> +    {
> +        FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY 1"),
> cb1, cb2);
> +    }
>  }
>

I'm OK with this.

Thanks.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Guillaume Lelarge wrote:
> Dave Page wrote:
>> Index: pgadmin/dlg/dlgProperty.cpp
>> ===================================================================
>> --- pgadmin/dlg/dlgProperty.cpp    (révision 6910)
>> +++ pgadmin/dlg/dlgProperty.cpp    (copie de travail)
>> @@ -432,7 +432,14 @@
>>
>>  void dlgProperty::AddUsers(ctlComboBoxFix *cb1, ctlComboBoxFix *cb2)
>>  {
>> -    FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY usename"),
>> cb1, cb2);
>> +    if (connection->BackendMinimumVersion(8, 1))
>> +    {
>> +        FillCombobox(wxT("SELECT rolname FROM pg_roles ORDER BY 1"),
>> cb1, cb2);
>> +    }
>> +    else
>> +    {
>> +        FillCombobox(wxT("SELECT usename FROM pg_user ORDER BY 1"),
>> cb1, cb2);
>> +    }
>>  }
>>
>
> I'm OK with this.

Please go ahead and commit at your leisure :-)

/D

Dave Page wrote:
> Guillaume Lelarge wrote:
> [...]
>>>> I'll take a look at the query tool to see how to do this ("present a
>>>> list of databases on the server"). Another call to wxSingleChoiceDialod
>>>> seems a bad way to me... I'll need to add a new dialog, don't you think ?
>>> Yes, I don't think it should be two dialogues.
>>>
>> I'm working on it now.
>
> 'K.
>

I've finally done it. It's the first time I create a new dialog, so it
probably need some tweaks. I attached the patch and a tar.gz file for
the new files.

Regards.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com
Index: pgadmin/include/schema/pgRole.h
===================================================================
--- pgadmin/include/schema/pgRole.h    (révision 6919)
+++ pgadmin/include/schema/pgRole.h    (copie de travail)
@@ -75,6 +75,8 @@
     void iSetUpdateCatalog(const bool b) { updateCatalog=b; }
     wxArrayString& GetRolesIn() { return rolesIn; }
     wxArrayString& GetConfigList() { return configList; }
+
+    void ReassignDropOwnedTo(frmMain *form);


     // Tree object creation
@@ -112,6 +114,13 @@
     pgGroupRole(const wxString& newName = wxT(""));
 };

+class reassignDropOwnedFactory : public contextActionFactory
+{
+public:
+    reassignDropOwnedFactory(menuFactoryList *list, wxMenu *mnu, wxToolBar *toolbar);
+    wxWindow *StartDialog(frmMain *form, pgObject *obj);
+    bool CheckEnable(pgObject *obj);
+};


 #endif
Index: pgadmin/frm/frmMain.cpp
===================================================================
--- pgadmin/frm/frmMain.cpp    (révision 6919)
+++ pgadmin/frm/frmMain.cpp    (copie de travail)
@@ -67,6 +67,7 @@
 #include "schema/pgTable.h"
 #include "schema/pgIndex.h"
 #include "schema/pgTrigger.h"
+#include "schema/pgRole.h"
 #include "schema/pgRule.h"
 #include "schema/pgServer.h"
 #include "slony/slCluster.h"
@@ -326,6 +327,7 @@
     new dropCascadedFactory(menuFactories, editMenu, 0);
     new truncateFactory(menuFactories, editMenu, 0);
     new truncateCascadedFactory(menuFactories, editMenu, 0);
+    new reassignDropOwnedFactory(menuFactories, editMenu, 0);
     editMenu->AppendSeparator();

     new separatorFactory(menuFactories);
Index: pgadmin/schema/pgRole.cpp
===================================================================
--- pgadmin/schema/pgRole.cpp    (révision 6919)
+++ pgadmin/schema/pgRole.cpp    (copie de travail)
@@ -11,12 +11,15 @@

 // wxWindows headers
 #include <wx/wx.h>
+#include <wx/choicdlg.h>

 // App headers
 #include "pgAdmin3.h"
 #include "utils/misc.h"
 #include "schema/pgRole.h"
 #include "frm/frmMain.h"
+#include "dlg/dlgReassignDropOwned.h"
+#include "dlg/dlgSelectConnection.h"
 #include "utils/pgDefs.h"
 #include "schema/pgDatabase.h"
 #include "schema/pgTablespace.h"
@@ -51,7 +54,7 @@

 bool pgRole::DropObject(wxFrame *frame, ctlTree *browser, bool cascaded)
 {
-    if (GetUpdateCatalog())
+    if (GetUpdateCatalog())
     {
         wxMessageDialog dlg(frame,
             _("Deleting a superuser might result in unwanted behaviour (e.g. when restoring the database).\nAre you
sure?"),
@@ -276,8 +279,43 @@
     }
 }

+void pgRole::ReassignDropOwnedTo(frmMain *form)
+{
+    wxString query;
+
+    dlgReassignDropOwned rdo(form, GetConnection(), this);
+    if (rdo.ShowModal() != wxID_CANCEL)
+    {
+        pgConn *conn;
+        conn = new pgConn(GetConnection()->GetHost(),
+                     rdo.GetDatabase(),
+                     GetConnection()->GetUser(),
+                     GetConnection()->GetPassword(),
+                     GetConnection()->GetPort(),
+                     GetConnection()->GetSslMode());
+
+        if (conn->GetStatus() == PGCONN_OK)
+        {
+            if (rdo.IsReassign())
+            {
+                if (wxMessageBox(_("Are you sure you wish to reassign all objets owned by the selected role?"),
_("Reassignobjects"), wxYES_NO) == wxNO) 
+                    return;

+                query = wxT("REASSIGN OWNED BY ") + GetQuotedFullIdentifier() + wxT(" TO ") + rdo.GetRole();
+            }
+            else
+            {
+                if (wxMessageBox(_("Are you sure you wish to drop all objets owned by the selected role?"), _("Drop
objects"),wxYES_NO) == wxNO) 
+                    return;

+                query = wxT("DROP OWNED BY ") + GetQuotedFullIdentifier();
+            }
+            conn->ExecuteVoid(query);
+        }
+    }
+}
+
+
 pgObject *pgRole::Refresh(ctlTree *browser, const wxTreeItemId item)
 {
     pgObject *role=0;
@@ -394,3 +432,22 @@

 pgGroupRoleFactory groupRoleFactory;
 static pgaCollectionFactory gcf(&groupRoleFactory, __("Group Roles"), roles_xpm);
+
+
+reassignDropOwnedFactory::reassignDropOwnedFactory(menuFactoryList *list, wxMenu *mnu, wxToolBar *toolbar) :
contextActionFactory(list)
+{
+    mnu->Append(id, _("Reassign/Drop Owned..."), _("Reassigned or drop objects owned by the selected role."));
+}
+
+
+wxWindow *reassignDropOwnedFactory::StartDialog(frmMain *form, pgObject *obj)
+{
+    ((pgRole*)obj)->ReassignDropOwnedTo(form);
+
+    return 0;
+}
+
+bool reassignDropOwnedFactory::CheckEnable(pgObject *obj)
+{
+    return obj && obj->IsCreatedBy(loginRoleFactory) && ((pgRole*)obj)->GetConnection()->BackendMinimumVersion(8, 2);
+}
Index: pgadmin/dlg/module.mk
===================================================================
--- pgadmin/dlg/module.mk    (révision 6919)
+++ pgadmin/dlg/module.mk    (copie de travail)
@@ -35,6 +35,7 @@
     $(srcdir)/dlg/dlgPackage.cpp \
     $(srcdir)/dlg/dlgPgpassConfig.cpp \
     $(srcdir)/dlg/dlgProperty.cpp \
+    $(srcdir)/dlg/dlgReassignDropOwned.cpp \
     $(srcdir)/dlg/dlgRole.cpp \
     $(srcdir)/dlg/dlgRule.cpp \
     $(srcdir)/dlg/dlgSchema.cpp \
Index: pgadmin/ui/module.mk
===================================================================
--- pgadmin/ui/module.mk    (révision 6919)
+++ pgadmin/ui/module.mk    (copie de travail)
@@ -39,6 +39,7 @@
     $(srcdir)/ui/dlgOperator.xrc \
     $(srcdir)/ui/dlgPackage.xrc \
     $(srcdir)/ui/dlgPgpassConfig.xrc \
+    $(srcdir)/ui/dlgReassignDropOwned.xrc \
     $(srcdir)/ui/dlgRepCluster.xrc \
     $(srcdir)/ui/dlgRepClusterUpgrade.xrc \
     $(srcdir)/ui/dlgRepListen.xrc \

Hi Guillaume,

On 22/12/2007, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> I've finally done it. It's the first time I create a new dialog, so it
> probably need some tweaks. I attached the patch and a tar.gz file for
> the new files.

Finally, some feedback for you!!

- In pgRole::ReassignDropOwnedTo, objets should be objects.

- In pgRole::ReassignDropOwnedTo an error should be raised if the
connection cannot be opened.

- The database selection in dlgReassignDropOwned should respect the DB
restriction setting for the server.

- Remember to append a 'd' to all size/position values in XRC files to
denote cross-platform friendly dialog units.

- The dialog needs some work. Get yourself a copy of XRCed - it will
help enormously. Try to keep the layout and sizing in line with other
dialogues. Use sizers to keep everything in line rather than absolute
positioning (new dialogues do this, older ones might not). It's a bit
fiddly until you get the hang of them, but it does make things much
easier to get right across platforms.

Thanks, Dave

Hi Dave,

Finally, I found some time to work on my patches :)

Dave Page wrote:
> On 22/12/2007, Guillaume Lelarge <guillaume@lelarge.info> wrote:
>> I've finally done it. It's the first time I create a new dialog, so it
>> probably need some tweaks. I attached the patch and a tar.gz file for
>> the new files.
>
> Finally, some feedback for you!!
>
> - In pgRole::ReassignDropOwnedTo, objets should be objects.
>

Oops, I used the french word.

> - In pgRole::ReassignDropOwnedTo an error should be raised if the
> connection cannot be opened.
>
> - The database selection in dlgReassignDropOwned should respect the DB
> restriction setting for the server.
>
> - Remember to append a 'd' to all size/position values in XRC files to
> denote cross-platform friendly dialog units.
>

All are done.

> - The dialog needs some work. Get yourself a copy of XRCed - it will
> help enormously. Try to keep the layout and sizing in line with other
> dialogues. Use sizers to keep everything in line rather than absolute
> positioning (new dialogues do this, older ones might not). It's a bit
> fiddly until you get the hang of them, but it does make things much
> easier to get right across platforms.
>

I'm not sure I get this right. It took me much more time than the four
previous stuff. XRCed is not really easy getting used to. So bad there's
no better (free) XRC editor out there. Anyways, I've done something I'm
quite happy with. But perhaps I need to add more sizers ?

Cheers.


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com
Index: pgadmin/include/schema/pgRole.h
===================================================================
--- pgadmin/include/schema/pgRole.h    (révision 7050)
+++ pgadmin/include/schema/pgRole.h    (copie de travail)
@@ -75,8 +75,9 @@
     void iSetUpdateCatalog(const bool b) { updateCatalog=b; }
     wxArrayString& GetRolesIn() { return rolesIn; }
     wxArrayString& GetConfigList() { return configList; }
+
+    void ReassignDropOwnedTo(frmMain *form);

-
     // Tree object creation
     void ShowTreeDetail(ctlTree *browser, frmMain *form=0, ctlListView *properties=0, ctlSQLBox *sqlPane=0);
     void ShowDependents(frmMain *form, ctlListView *referencedBy, const wxString &where);
@@ -112,6 +113,12 @@
     pgGroupRole(const wxString& newName = wxT(""));
 };

+class reassignDropOwnedFactory : public contextActionFactory
+{
+public:
+    reassignDropOwnedFactory(menuFactoryList *list, wxMenu *mnu, wxToolBar *toolbar);
+    wxWindow *StartDialog(frmMain *form, pgObject *obj);
+    bool CheckEnable(pgObject *obj);
+};

-
 #endif
Index: pgadmin/frm/frmMain.cpp
===================================================================
--- pgadmin/frm/frmMain.cpp    (révision 7050)
+++ pgadmin/frm/frmMain.cpp    (copie de travail)
@@ -67,6 +67,7 @@
 #include "schema/pgTable.h"
 #include "schema/pgIndex.h"
 #include "schema/pgTrigger.h"
+#include "schema/pgRole.h"
 #include "schema/pgRule.h"
 #include "schema/pgServer.h"
 #include "slony/slCluster.h"
@@ -326,6 +327,7 @@
     new dropCascadedFactory(menuFactories, editMenu, 0);
     new truncateFactory(menuFactories, editMenu, 0);
     new truncateCascadedFactory(menuFactories, editMenu, 0);
+    new reassignDropOwnedFactory(menuFactories, editMenu, 0);
     editMenu->AppendSeparator();

     new separatorFactory(menuFactories);
Index: pgadmin/schema/pgRole.cpp
===================================================================
--- pgadmin/schema/pgRole.cpp    (révision 7050)
+++ pgadmin/schema/pgRole.cpp    (copie de travail)
@@ -11,12 +11,15 @@

 // wxWindows headers
 #include <wx/wx.h>
+#include <wx/choicdlg.h>

 // App headers
 #include "pgAdmin3.h"
 #include "utils/misc.h"
 #include "schema/pgRole.h"
 #include "frm/frmMain.h"
+#include "dlg/dlgReassignDropOwned.h"
+#include "dlg/dlgSelectConnection.h"
 #include "utils/pgDefs.h"
 #include "schema/pgDatabase.h"
 #include "schema/pgTablespace.h"
@@ -51,7 +54,7 @@

 bool pgRole::DropObject(wxFrame *frame, ctlTree *browser, bool cascaded)
 {
-    if (GetUpdateCatalog())
+    if (GetUpdateCatalog())
     {
         wxMessageDialog dlg(frame,
             _("Deleting a superuser might result in unwanted behaviour (e.g. when restoring the database).\nAre you
sure?"),
@@ -276,8 +279,47 @@
     }
 }

+void pgRole::ReassignDropOwnedTo(frmMain *form)
+{
+    wxString query;
+
+    dlgReassignDropOwned rdo(form, GetConnection(), this, GetServer()->GetDbRestriction());
+    if (rdo.ShowModal() != wxID_CANCEL)
+    {
+        pgConn *conn;
+        conn = new pgConn(GetConnection()->GetHost(),
+                     rdo.GetDatabase(),
+                     GetConnection()->GetUser(),
+                     GetConnection()->GetPassword(),
+                     GetConnection()->GetPort(),
+                     GetConnection()->GetSslMode());
+
+        if (conn->GetStatus() == PGCONN_OK)
+        {
+            if (rdo.IsReassign())
+            {
+                if (wxMessageBox(_("Are you sure you wish to reassign all objects owned by the selected role?"),
_("Reassignobjects"), wxYES_NO) == wxNO) 
+                    return;

+                query = wxT("REASSIGN OWNED BY ") + GetQuotedFullIdentifier() + wxT(" TO ") + rdo.GetRole();
+            }
+            else
+            {
+                if (wxMessageBox(_("Are you sure you wish to drop all objects owned by the selected role?"), _("Drop
objects"),wxYES_NO) == wxNO) 
+                    return;

+                query = wxT("DROP OWNED BY ") + GetQuotedFullIdentifier();
+            }
+            conn->ExecuteVoid(query);
+        }
+        else
+        {
+            wxMessageBox(wxT("Connection failed: ") + conn->GetLastError());
+        }
+    }
+}
+
+
 pgObject *pgRole::Refresh(ctlTree *browser, const wxTreeItemId item)
 {
     pgObject *role=0;
@@ -394,3 +436,22 @@

 pgGroupRoleFactory groupRoleFactory;
 static pgaCollectionFactory gcf(&groupRoleFactory, __("Group Roles"), roles_xpm);
+
+
+reassignDropOwnedFactory::reassignDropOwnedFactory(menuFactoryList *list, wxMenu *mnu, wxToolBar *toolbar) :
contextActionFactory(list)
+{
+    mnu->Append(id, _("Reassign/Drop Owned..."), _("Reassigned or drop objects owned by the selected role."));
+}
+
+
+wxWindow *reassignDropOwnedFactory::StartDialog(frmMain *form, pgObject *obj)
+{
+    ((pgRole*)obj)->ReassignDropOwnedTo(form);
+
+    return 0;
+}
+
+bool reassignDropOwnedFactory::CheckEnable(pgObject *obj)
+{
+    return obj && obj->IsCreatedBy(loginRoleFactory) && ((pgRole*)obj)->GetConnection()->BackendMinimumVersion(8, 2);
+}
Index: pgadmin/dlg/module.mk
===================================================================
--- pgadmin/dlg/module.mk    (révision 7050)
+++ pgadmin/dlg/module.mk    (copie de travail)
@@ -35,6 +35,7 @@
     $(srcdir)/dlg/dlgPackage.cpp \
     $(srcdir)/dlg/dlgPgpassConfig.cpp \
     $(srcdir)/dlg/dlgProperty.cpp \
+    $(srcdir)/dlg/dlgReassignDropOwned.cpp \
     $(srcdir)/dlg/dlgRole.cpp \
     $(srcdir)/dlg/dlgRule.cpp \
     $(srcdir)/dlg/dlgSchema.cpp \
Index: pgadmin/ui/module.mk
===================================================================
--- pgadmin/ui/module.mk    (révision 7050)
+++ pgadmin/ui/module.mk    (copie de travail)
@@ -39,6 +39,7 @@
     $(srcdir)/ui/dlgOperator.xrc \
     $(srcdir)/ui/dlgPackage.xrc \
     $(srcdir)/ui/dlgPgpassConfig.xrc \
+    $(srcdir)/ui/dlgReassignDropOwned.xrc \
     $(srcdir)/ui/dlgRepCluster.xrc \
     $(srcdir)/ui/dlgRepClusterUpgrade.xrc \
     $(srcdir)/ui/dlgRepListen.xrc \

Hi Guillaume

On Feb 5, 2008 9:46 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> Hi Dave,
>
> Finally, I found some time to work on my patches :)

:-)

> I'm not sure I get this right. It took me much more time than the four
> previous stuff. XRCed is not really easy getting used to. So bad there's
> no better (free) XRC editor out there. Anyways, I've done something I'm
> quite happy with. But perhaps I need to add more sizers ?

Yeah, there's still some problems.

- On Windows, the dialogue is too small. Removing the size form the
dialogue iutself seems to cure that in my limited testing.

- There should be a border around the tab control so it's not tight to
the edges of the dialogue. I think we use 5d elsewhere.

- I'm not actually sure we need the tab control there anyway - maybe
the change password dialogue would be a better model?

- It should probably be a fixed size dialogue.

I heard a rumour that DialogBlocks now supports XRC format. Not sure
it if can do a better job.

Unfortunately I don't have time at the moment to hack it about myself,
otherwise I'd see what I could do.

/D

Dave Page wrote:
> On Feb 5, 2008 9:46 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> [...]
>> I'm not sure I get this right. It took me much more time than the four
>> previous stuff. XRCed is not really easy getting used to. So bad there's
>> no better (free) XRC editor out there. Anyways, I've done something I'm
>> quite happy with. But perhaps I need to add more sizers ?
>
> Yeah, there's still some problems.
>
> - On Windows, the dialogue is too small. Removing the size form the
> dialogue iutself seems to cure that in my limited testing.
>

Because of the fixed size dialog, I think I have to specify the dialog's
size, don't I ?

> - There should be a border around the tab control so it's not tight to
> the edges of the dialogue. I think we use 5d elsewhere.
>
> - I'm not actually sure we need the tab control there anyway - maybe
> the change password dialogue would be a better model?
>

I wasn't sure either. I used the frmBackup.xrc as a model to build
dlgReassignDropOwned.xrc. But the tab seems too much. I removed it...
but now it really looks like my previous dlgReassignDropOwned.xrc.

> - It should probably be a fixed size dialogue.
>

+1

The XRC file attached is one with a dialog size specified, a modal
dialog, and no notebook/panel/tab widgets. But I'm not sure about the
correct sizing. XRCed shows me the dialog in a way and pgAdmin in
another one. I don't quite understand this.

> I heard a rumour that DialogBlocks now supports XRC format. Not sure
> it if can do a better job.
>

I tried DialogBlocks a few times but I never was able to use it on my
Kubuntu.


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com
<?xml version="1.0" encoding="UTF-8"?>
<resource>
  <object class="wxDialog" name="dlgReassignDropOwned">
    <title>Reassign/Drop Owned</title>
    <size>170,80d</size>
    <style>wxDEFAULT_DIALOG_STYLE|wxCAPTION|wxSYSTEM_MENU|wxDIALOG_MODAL</style>
    <object class="wxRadioButton" name="rbReassignTo">
      <label>&Reassign objects to</label>
      <pos>5,7</pos>
    </object>
    <object class="wxComboBox" name="cbRoles">
      <content/>
      <pos>180,5</pos>
      <style>wxCB_READONLY|wxCB_DROPDOWN</style>
    </object>
    <object class="wxRadioButton" name="rbDrop">
      <label>&Drop</label>
      <pos>5,37</pos>
    </object>
    <object class="wxStaticText" name="stDatabases">
      <label>&From Database</label>
      <pos>5,82</pos>
    </object>
    <object class="wxComboBox" name="cbDatabases">
      <content/>
      <pos>180,82</pos>
      <style>wxCB_READONLY|wxCB_DROPDOWN</style>
    </object>
    <object class="wxButton" name="wxID_OK">
      <label>&OK</label>
      <default>1</default>
      <pos>70,60d</pos>
    </object>
    <object class="wxButton" name="wxID_CANCEL">
      <label>&Cancel</label>
      <pos>115,60d</pos>
    </object>
  </object>
</resource>

On Feb 5, 2008 4:43 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> Dave Page wrote:
> > On Feb 5, 2008 9:46 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> > [...]
> >> I'm not sure I get this right. It took me much more time than the four
> >> previous stuff. XRCed is not really easy getting used to. So bad there's
> >> no better (free) XRC editor out there. Anyways, I've done something I'm
> >> quite happy with. But perhaps I need to add more sizers ?
> >
> > Yeah, there's still some problems.
> >
> > - On Windows, the dialogue is too small. Removing the size form the
> > dialogue iutself seems to cure that in my limited testing.
> >
>
> Because of the fixed size dialog, I think I have to specify the dialog's
> size, don't I ?

Not necessarily - the sizer should be able to handle it. Look at the
passowrd dialogue though... I was kinda thinking sequentially :-)

/D

Dave Page wrote:
> On Feb 5, 2008 4:43 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
>> Dave Page wrote:
>>> On Feb 5, 2008 9:46 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
>>> [...]
>>>> I'm not sure I get this right. It took me much more time than the four
>>>> previous stuff. XRCed is not really easy getting used to. So bad there's
>>>> no better (free) XRC editor out there. Anyways, I've done something I'm
>>>> quite happy with. But perhaps I need to add more sizers ?
>>> Yeah, there's still some problems.
>>>
>>> - On Windows, the dialogue is too small. Removing the size form the
>>> dialogue iutself seems to cure that in my limited testing.
>>>
>> Because of the fixed size dialog, I think I have to specify the dialog's
>> size, don't I ?
>
> Not necessarily - the sizer should be able to handle it. Look at the
> passowrd dialogue though... I was kinda thinking sequentially :-)
>

The password dialogue is the frmPassword.xrc file, right ? I don't see
the difference between it and my xrc file. It's a fixed size dialog,
style is the same. All widgets have a specific position, only entry one
have a specific size.

Or perhaps you talked about the dlgConnect.xrc file ? It's also a fixed
size dialog but there's a wxFlexGridSizer. Yeah, I think that's the one
you're talking about. I'll use it to build my dialog...


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com

On Feb 6, 2008 9:55 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
>
> The password dialogue is the frmPassword.xrc file, right ? I don't see
> the difference between it and my xrc file. It's a fixed size dialog,
> style is the same. All widgets have a specific position, only entry one
> have a specific size.

Yeah, that one. It's fixed size unlike yours (by which I mean that
it's not sizeable, not that you have specified the size explicitly),
doesn't have the tab set, and have nice spacing between all the
controls within the dialogue boundaries. It's very basic and should be
easy to copy.

Regards, Dave

On Feb 6, 2008 10:49 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
>
> OK. Here is one, fixed but not specified size, no tab, nice spacing (at
> least, it seems to me on my linux laptop). Is it getting better ? :)

OK, I've tweaked it on windows. There were 2 things I changed:

- Defined the fixed size for the dialogue as it was mch wider than the
controls on Windows.
- Widened the space for the labels as some languages are more verbose
than English.

Please check it on your linux box and if it looks OK, go ahead and commit :-)

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
The Oracle-compatible database company

Dave Page wrote:
> On Feb 6, 2008 10:49 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote:
>> OK. Here is one, fixed but not specified size, no tab, nice spacing (at
>> least, it seems to me on my linux laptop). Is it getting better ? :)
>
> OK, I've tweaked it on windows. There were 2 things I changed:
>
> - Defined the fixed size for the dialogue as it was mch wider than the
> controls on Windows.
> - Widened the space for the labels as some languages are more verbose
> than English.
>
> Please check it on your linux box and if it looks OK, go ahead and commit :-)
>

It works great on my linux box. The dialog takes a bit more place but it
feels better. Thanks a lot :)

Now back to my "set schema" patch.


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com