5 files changed, 74 insertions, 17 deletions

diff --git a/core/controllers/welcome.php b/core/controllers/welcome.php
index 9e6f9542..5afe094e 100644
--- a/core/controllers/welcome.php
+++ b/core/controllers/welcome.php
@@ -455,16 +455,22 @@ class Welcome_Controller extends Template_Controller {
   }
 
   public function remove_from_group($group_id, $user_id) {
-    ORM::factory("group", $group_id)->remove(
-      ORM::factory("user", $user_id));
+    $group = ORM::factory("group", $group_id);
+    $user = ORM::factory("group", $user_id);
+    if ($group->loaded && $user->loaded) {
+      $group->remove($user);
+      $group->save();
+    }
     url::redirect("welcome");
   }
 
   public function add_to_group($user_id) {
     $group_name = $this->input->post("group_name");
     $group = ORM::factory("group")->where("name", $group_name)->find();
-    if ($group->loaded) {
-      $group->add(ORM::factory("user", $user_id));
+    $user = ORM::factory("group", $user_id);
+    if ($group->loaded && $user->loaded) {
+      $group->add($user);
+      $group->save();
     }
     url::redirect("welcome");
   }
diff --git a/core/tests/Access_Helper_Test.php b/core/tests/Access_Helper_Test.php
index 9b08b2cd..ce9276fd 100644
--- a/core/tests/Access_Helper_Test.php
+++ b/core/tests/Access_Helper_Test.php
@@ -280,8 +280,7 @@ class Access_Helper_Test extends Unit_Test_Case {
     foreach ($user->groups as $group) {
       $user->remove($group);
     }
-    // @todo remove this reload when http://dev.kohanaphp.com/ticket/959 is resolved
-    $user->reload();
+    $user->save();
     user::set_active($user);
 
     // This user can't edit anything
@@ -291,8 +290,11 @@ class Access_Helper_Test extends Unit_Test_Case {
     // Now add them to a group that has edit permission
     $group = group::create("access_test");
     $group->add($user);
+    $group->save();
     access::allow($group, "edit", $root);
-    user::set_active($user->reload());
+
+    $user = ORM::factory("user", $user->id);  // reload() does not flush related columns
+    user::set_active($user);
 
     // And verify that the user can edit.
     $this->assert_true(access::can("edit", $root));
diff --git a/modules/user/helpers/user.php b/modules/user/helpers/user.php
index 7d72a9b6..e7a3e165 100644
--- a/modules/user/helpers/user.php
+++ b/modules/user/helpers/user.php
@@ -94,7 +94,7 @@ class user_Core {
    * @return User_Model
    */
   static function create($name, $display_name, $password) {
-    $user = ORM::factory("user")->where("name", $name);
+    $user = ORM::factory("user")->where("name", $name)->find();
     if ($user->loaded) {
       throw new Exception("@todo USER_ALREADY_EXISTS $name");
     }
@@ -102,16 +102,13 @@ class user_Core {
     $user->name = $name;
     $user->display_name = $display_name;
     $user->password = $password;
-    $user->save();
-
-    // Everybody user
-    $group = ORM::factory("group", 1);
-    $group->add($user);
 
-    // Registered users
-    $group = ORM::factory("group", 2);
-    $group->add($user);
+    // Everybody group
+    $user->add(ORM::factory("group", 1));
+    // Registered Users group
+    $user->add(ORM::factory("group", 2));
 
+    $user->save();
     module::event("user_created", $user);
     return $user;
   }
diff --git a/modules/user/helpers/user_installer.php b/modules/user/helpers/user_installer.php
index 2de3a6cd..739a8815 100644
--- a/modules/user/helpers/user_installer.php
+++ b/modules/user/helpers/user_installer.php
@@ -62,8 +62,8 @@ class user_installer {
 
       $guest = user::create("guest", "Guest User", "");
       $guest->guest = true;
-      $guest->save();
       $guest->remove($registered);
+      $guest->save();
 
       $admin = user::create("admin", "Gallery Administrator", "admin");
       $admin->admin = true;
diff --git a/modules/user/tests/User_Groups_Test.php b/modules/user/tests/User_Groups_Test.php
new file mode 100644
index 00000000..f8f25cf1
--- /dev/null
+++ b/modules/user/tests/User_Groups_Test.php
@@ -0,0 +1,52 @@
+<?php defined("SYSPATH") or die("No direct script access.");
+/**
+ * Gallery - a web based photo album viewer and editor
+ * Copyright (C) 2000-2008 Bharat Mediratta
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+class User_Groups_Test extends Unit_Test_Case {
+  public function teardown() {
+    try {
+      $group = ORM::factory("group")->where("name", "user_groups_test")->find();
+      if ($group->loaded) {
+        $group->delete();
+      }
+    } catch (Exception $e) { }
+
+    try {
+      $user = ORM::factory("user")->where("name", "user_groups_test")->find();
+      if ($user->loaded) {
+        $user->delete();
+      }
+    } catch (Exception $e) { }
+  }
+
+  public function add_user_to_group_test() {
+    $user = ORM::factory("user");
+    $user->name = "user_groups_test";
+    $user->save();
+
+    $group = ORM::factory("group");
+    $group->name = "user_groups_test";
+    $group->save();
+
+    $group->add($user);
+    $group->save();
+
+    $this->assert_true($user->has($group));
+  }
+}