Merge pull request #987 from ucfcdl/issue/984-remove-email-requiremen…

…t-for-creating-users Make the e-mail address requirement for creating new users optional.
ucfopen · Apr 3, 2017 · 430bbf0 · 430bbf0
2 parents 6ccab50 + e0838a0
commit 430bbf0
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 37 deletions.
diff --git a/fuel/app/classes/model/user.php b/fuel/app/classes/model/user.php
@@ -53,6 +53,8 @@ public function _event_before_save()
 		$profile_fields = $this->get('profile_fields');
 
 		$this->set('profile_fields', array_merge(static::$_default_profile_fields, $profile_fields));
+		//don't allow notifications to be sent if there's no e-mail address to send them to
+		if(empty($this->email)) $this->profile_fields['notify'] = false;
 	}
 
 	public static function find_current()

diff --git a/fuel/app/modules/lti/classes/controller/test.php b/fuel/app/modules/lti/classes/controller/test.php
@@ -131,21 +131,13 @@ public function post_learner()
 		$resource_link_id = static::get_and_unset_post('resource_link');
 		$custom_inst_id   = static::get_and_unset_post('custom_widget_instance_id');
 
-		$as_new_learner   = static::get_and_unset_post('new_learner') ?: false;
-		$as_instructor    = static::get_and_unset_post('as_instructor') ?: false;
-		$as_test_student  = static::get_and_unset_post('test_student') ?: false;
+		$as_instructor           = static::get_and_unset_post('as_instructor') ?: false;
+		$as_test_student         = static::get_and_unset_post('test_student') ?: false;
+		$as_new_learner_email    = static::get_and_unset_post('new_learner_email') ?: false;
+		$as_new_learner_no_email = static::get_and_unset_post('new_learner_no_email') ?: false;
 
 		switch (true)
 		{
-			case $as_new_learner:
-				$learner_params = $this->create_test_case([
-					'context_id'                => $context_id,
-					'resource_link_id'          => $resource_link_id,
-					'custom_widget_instance_id' => $custom_inst_id
-				], $lti_url, $this->create_new_random_user());
-				$learner_params[0]['user_id'] = '';
-				break;
-
 			case $as_instructor:
 				$learner_params = $this->create_test_case([
 					'roles'                     => 'Instructor',
@@ -170,6 +162,24 @@ public function post_learner()
 				$learner_params[0]['user_id'] = '';
 				break;
 
+			case $as_new_learner_email:
+				$learner_params = $this->create_test_case([
+					'context_id'                => $context_id,
+					'resource_link_id'          => $resource_link_id,
+					'custom_widget_instance_id' => $custom_inst_id
+				], $lti_url, $this->create_new_random_user());
+				$learner_params[0]['user_id'] = '';
+				break;
+
+			case $as_new_learner_no_email:
+				$learner_params = $this->create_test_case([
+					'context_id'                => $context_id,
+					'resource_link_id'          => $resource_link_id,
+					'custom_widget_instance_id' => $custom_inst_id
+				], $lti_url, $this->create_new_random_user(false));
+				$learner_params[0]['user_id'] = '';
+				break;
+
 			default:
 				$learner_params = $this->create_test_case([
 					'context_id'                => $context_id,
@@ -228,13 +238,13 @@ protected function create_test_case($custom_params, $endpoint, $user = false, $p
 		return [$params, $endpoint];
 	}
 
-	protected function create_new_random_user()
+	protected function create_new_random_user($with_email = true)
 	{
 		$rand = substr(md5(microtime()), 0, 10);
 
 		$user = new \Model_User([
 			'username' => 'test_lti_user'.$rand,
-			'email'    => 'test.lti.user'.$rand.'@materia.com',
+			'email'    => $with_email ? 'test.lti.user'.$rand.'@materia.com' : '',
 			'first'    => 'Unofficial Test',
 			'last'     => "User $rand"
 		]);

diff --git a/fuel/app/modules/lti/classes/ltiusermanager.php b/fuel/app/modules/lti/classes/ltiusermanager.php
@@ -116,7 +116,18 @@ protected static function get_or_create_user($launch, $search_field, $auth_drive
 		{
 			try
 			{
-				$user_id = $auth->create_user($launch->username, uniqid(), $launch->email, 1, ['created_by' => $launch->consumer_id]);
+				//username, password, email, group, profile fields, first name, last name, requires password, requires email
+				$user_id = $auth->create_user(
+					$launch->username,
+					uniqid(),
+					$launch->email,
+					1,
+					['created_by' => $launch->consumer_id],
+					$launch->first,
+					$launch->last,
+					false,
+					false
+				);
 				if ($user_id)
 				{
 					$user = \Model_User::find($user_id);
@@ -161,8 +172,7 @@ protected static function update_user_from_lti_request(\Model_User $user, $launc
 		if ( empty($user->last))  $items_to_update['last'] = $launch->last;
 		// NOTE: Since emails are generated if none exist then this value will
 		// not be empty when we expect it to.
-		if ( empty($user->email)) $items_to_update['email'] = $launch->email;
-
+		if ( empty($user->email) && !empty($launch->email)) $items_to_update['email'] = $launch->email;
 		if ( ! empty($items_to_update)) $auth->update_user($items_to_update, $user->username);
 	}
 }
diff --git a/fuel/packages/materiaauth/classes/auth/login/materiaauth.php b/fuel/packages/materiaauth/classes/auth/login/materiaauth.php
@@ -40,14 +40,18 @@ public function logout()
 	/**
 	 * Create new user
 	 *
-	 * @param   string
-	 * @param   string
-	 * @param   string  must contain valid email address
-	 * @param   int     group id
-	 * @param   Array
+	 * @param   string username
+	 * @param   string password
+	 * @param   string email
+	 * @param   int    group id
+	 * @param   Array  profile fields
+	 * @param   string first name
+	 * @param   string last name
+	 * @param   bool   requires password
+	 * @param   bool   requires email
 	 * @return  bool
 	 */
-	public function create_user($username, $password, $email, $group = 1, Array $profile_fields = [], $first_name = '', $last_name = '', $requires_password = true)
+	public function create_user($username, $password, $email = '', $group = 1, Array $profile_fields = [], $first_name = '', $last_name = '', $requires_password = true, $requires_email = true)
 	{
 		$first_name = trim($first_name);
 		$last_name  = trim($last_name);
@@ -58,16 +62,15 @@ public function create_user($username, $password, $email, $group = 1, Array $pro
 		{
 			throw new \SimpleUserUpdateException('Username or password is not given', 1);
 		}
-		if ( ! $email )
+		if ( $requires_email && ! $email )
 		{
 			throw new \SimpleUserUpdateException('Email not given', 2);
 		}
 
 		// just get the first user that has the same username or email
-		$same_user = \Model_User::query()
-			->where('username', '=', $username)
-			->or_where('email', '=', $email)
-			->get_one();
+		$same_user = \Model_User::query()->where('username', '=', $username);
+		if($email) $same_user->or_where('email', '=', $email);
+		$same_user = $same_user->get_one();
 
 		if ($same_user)
 		{

diff --git a/fuel/packages/materiaauth/tests/materiaauth.php b/fuel/packages/materiaauth/tests/materiaauth.php
@@ -110,7 +110,7 @@ public function test_creating_a_user_without_username()
 	 * @expectedException        \Auth\SimpleUserUpdateException
 	 * @expectedExceptionMessage Email not given
 	 */
-	public function test_creating_a_user_without_email()
+	public function test_creating_a_user_without_email_strict()
 	{
 		$values = $this->user_values('Test3', 'McTest3');
 
@@ -122,20 +122,53 @@ public function test_creating_a_user_without_email()
 			$values['profile_fields'],
 			$values['first'],
 			$values['last'],
-			false
+			false,
+			true
 		);
 
 		return false;
 	}
 
+	public function test_creating_a_user_without_email_relaxed()
+	{
+		$values = $this->user_values('Test3', 'McTest3');
+		$users_count = \Model_User::count();
+
+		$new_user_id = \Auth::instance()->create_user(
+			$values['username'],
+			$values['password'],
+			null,
+			1,
+			$values['profile_fields'],
+			$values['first'],
+			$values['last'],
+			false,
+			false
+		);
+
+		$this->assertEquals($new_user_id, $users_count + 1);
+
+		// null emails are converted to 'false' during the email sanitizing process
+		$values['email'] = false;
+
+		$new_user_lookup = \Model_User::find($new_user_id);
+		$confirm_properties = ['username','email','first','last'];
+		foreach($confirm_properties as $prop)
+		{
+			$this->assertEquals($new_user_lookup[$prop], $values[$prop]);
+		}
+		$this->assertEquals($new_user_lookup['profile_fields']['notify'], false);
+	}
+
 	public function test_promoting_user()
 	{
 		//confirm that the last user we created is not an author
 		$last_user_id = \Model_User::count();
 		$this->assertEquals(\RocketDuck\Perm_Manager::does_user_have_role([\RocketDuck\Perm_Role::AUTHOR], $last_user_id), false);
 
 		//promote the last user we created to an author
-		$r = \Auth::instance()::update_role($last_user_id, true);
+		$auth = \Auth::instance();
+		$r = $auth::update_role($last_user_id, true);
 		$this->assertEquals(\RocketDuck\Perm_Manager::does_user_have_role([\RocketDuck\Perm_Role::AUTHOR], $last_user_id), true);
 	}
 
@@ -146,7 +179,8 @@ public function test_demoting_user()
 		$this->assertEquals(\RocketDuck\Perm_Manager::does_user_have_role([\RocketDuck\Perm_Role::AUTHOR], $last_user_id), true);
 
 		//demote the last user we created back to a student
-		$r = \Auth::instance()::update_role($last_user_id, false);
+		$auth = \Auth::instance();
+		$r = $auth::update_role($last_user_id, false);
 		$this->assertEquals(\RocketDuck\Perm_Manager::does_user_have_role([\RocketDuck\Perm_Role::AUTHOR], $last_user_id), false);
 	}
 
@@ -169,7 +203,7 @@ public function test_logout()
 	 */
 	public function test_update_no_user_without_username()
 	{
-		$values = $this->user_values('Test3', 'McTest3');
+		$values = $this->user_values('Test4', 'McTest4');
 		\Auth::update_user($values);
 
 		return false;
@@ -182,8 +216,8 @@ public function test_update_no_user_without_username()
 	 */
 	public function test_update_no_user_with_username()
 	{
-		$values = $this->user_values('Test3', 'McTest3');
-		\Auth::update_user($values, 'user_Test3_McTest3');
+		$values = $this->user_values('Test4', 'McTest4');
+		\Auth::update_user($values, 'user_Test4_McTest4');
 
 		return false;
 	}

diff --git a/public/themes/default/lti/layouts/test_provider.php b/public/themes/default/lti/layouts/test_provider.php
@@ -178,8 +178,17 @@ function toggleEmbed()
 				<input type="hidden" class="context_id" name="context_id" />
 				<input type="hidden" class="resource_link" name="resource_link" />
 				<input type="hidden" class="custom_widget_instance_id" name="custom_widget_instance_id" />
-				<input type="hidden" id="new_learner" name="new_learner" value="new_learner" />
-				<input type="submit" value="As NEW Learner">
+				<input type="hidden" id="new_learner_email" name="new_learner_email" value="new_learner_email" />
+				<input type="submit" value="As NEW Learner WITH EMAIL">
+			</form>
+
+			<form onsubmit="setLtiUrl(this)" method="POST" target="embed_iframe" action="<?= $learner_endpoint ?>" >
+				<input type="hidden" class="lti_url" name="lti_url" />
+				<input type="hidden" class="context_id" name="context_id" />
+				<input type="hidden" class="resource_link" name="resource_link" />
+				<input type="hidden" class="custom_widget_instance_id" name="custom_widget_instance_id" />
+				<input type="hidden" id="new_learner_no_email" name="new_learner_no_email" value="new_learner_no_email" />
+				<input type="submit" value="As NEW Learner WITHOUT EMAIL">
 			</form>
 
 			<form onsubmit="setLtiUrl(this)" method="POST" target="embed_iframe" action="<?= $learner_endpoint ?>" >