Reputation: 2215
Here is my register method. I changed the register method into a Admin tool. The admin tool works properly without the if statement. However, it does not like the way I am looking for existing users.
// POST: /Account/Register
[HttpPost]
[AllowAnonymous]
[ValidateAntiForgeryToken]
public async Task<ActionResult> Register(RegisterViewModel model)
{
if (ModelState.IsValid)
{
if(_context.Users.Contains(model.UserName))//Errors out here
{
var user = new ApplicationUser { UserName = model.UserName};
user.UserName = model.UserName;
var result = await UserManager.CreateAsync(user, model.Password);
if (result.Succeeded)
{
await this.UserManager.AddToRoleAsync(user.Id, model.Name);
return RedirectToAction("Index","User");
}
AddErrors(result);
}
}
return View(model);
}
Upvotes: 2
Views: 8495
Reputation: 93424
Try instead:
if(!_context.Users.Any(x => x.UserName == model.UserName))
Assuming the username in Users is called UserName.. Any returns true or false if the condition exists.
I think your logic is backwards, anyways. I think you want to create a user if they do NOT exist, which is what I did above.
It's probably better, however, to use the UserManager for this.
var user = await UserManager.FindByNameAsync(model.UserName);
if (user != null)
...
The reason why it's best to use the UserManager for everything is because you aren't creating multiple conflicting DbContexts (UserManager has one, and you create a separate one). This can lead to issues where you have objects created with one context that you try to use in the other, and it results in all kinds of headaches.
The most efficient way, however, is to simply try and create the user, and check the result for success or failure.
var result = await UserManager.CreateAsync(...)
if (result.Succeeded)
....
You're doing that above already, so what is the point of doing the extra check first?
Upvotes: 6