Tidy up username creation/deletion and add tests

This commit removes username as a parameter for creating and editing
users, using just email instead for consistency.

It also moves validation checks to the validate() function which is
always called, in case the result is updated directly.

Tests also added.

lib/GADS/API.pm

@@ -711,7 +711,6 @@ sub _post_add_user_account
         firstname             => $body->{firstname},
         surname               => $body->{surname},
         email                 => $body->{email},
-        username              => $body->{email},
         freetext1             => $body->{freetext1},
         freetext2             => $body->{freetext2},
         title                 => $body->{title},

lib/GADS/Schema/Result/User.pm

@@ -877,6 +877,8 @@ sub update_user
         $values->{account_request} = $params{account_request};
     }
 
+    my $original_username = $self->username;
+
     foreach my $field ($site->user_fields)
     {
         next if !exists $params{$field->{name}};
@@ -888,14 +890,8 @@ sub update_user
 
     my $audit = GADS::Audit->new(schema => $self->result_source->schema, user => $current_user);
 
-    if (lc $values->{username} ne lc $self->username)
-    {
-        $self->result_source->schema->resultset('User')->active->search({
-            username => $values->{username},
-        })->count
-            and error __x"Email address {username} already exists as an active user", username => $values->{username};
-        $audit->login_change("Username ".$self->username." (id ".$self->id.") being changed to $values->{username}");
-    }
+    $audit->login_change("Username $original_username (id ".$self->id.") being changed to ".$self->username)
+        if $original_username && $self->is_column_changed('username');
 
     # Coerce view_limits to value expected, ensure all removed if exists
     $params{view_limits} = []
@@ -1140,7 +1136,21 @@ sub for_data_table
 
 sub validate
 {   my $self = shift;
+    # Update value field
     $self->value(_user_value({firstname => $self->firstname, surname => $self->surname}));
+
+    # Check existing user rename, check both email address and username
+    foreach my $f (qw/username email/)
+    {
+        if ($self->is_column_changed($f) || !$self->id)
+        {
+            my $search = { $f => $self->$f };
+            $search->{id} = { '!=' => $self->id }
+                if $self->id;
+            $self->result_source->resultset->active->search($search)->next
+                and error __x"{username} already exists as an active user", username => $self->$f;
+        }
+    }
 }
 
 sub export_hash

lib/GADS/Schema/ResultSet/User.pm

@@ -47,6 +47,8 @@ sub create_user
 
     error __"An email address must be specified for the user"
         if !$params{email};
+    panic "username is no longer accepted for create_user - use email instead"
+        if $params{username};
 
     error __x"User {email} already exists", email => $params{email}
         if $self->active(email => $params{email})->count;
@@ -207,7 +209,6 @@ sub upload
             firstname             => defined $user_mapping{forename} ? $row->[$user_mapping{forename}] : '',
             surname               => defined $user_mapping{surname} ? $row->[$user_mapping{surname}] : '',
             email                 => defined $user_mapping{email} ? $row->[$user_mapping{email}] : '',
-            username              => defined $user_mapping{email} ? $row->[$user_mapping{email}] : '',
             freetext1             => defined $user_mapping{$freetext1} ? $row->[$user_mapping{$freetext1}] : '',
             freetext2             => defined $user_mapping{$freetext2} ? $row->[$user_mapping{$freetext2}] : '',
             title                 => $title_id,

t/010_permissions.t

@@ -358,7 +358,6 @@ foreach my $test (qw/single all/)
         try {
             $user = $schema->resultset('User')->create_user(
                 current_user     => $sheet->$usertype,
-                username         => "$usertype\@example.com",
                 email            => "$usertype\@example.com",
                 firstname        => 'Joe',
                 surname          => 'Bloggs',

t/024_user.t

@@ -18,7 +18,6 @@ my %template = (
     surname          => 'Bloggs',
     firstname        => 'Joe',
     email            => '[email protected]',
-    username         => '[email protected]',
 );
 
 my $user = $schema->resultset('User')->create_user(%template);
@@ -29,6 +28,20 @@ my $u = $schema->resultset('User')->find($user_id);
 
 is($u->value, "Bloggs, Joe", "User created successfully");
 
+# Check cannot rename to existing user
+my $existing = $schema->resultset('User')->next->username;
+ok($existing ne $u->username, "Testing username different to that of test");
+try { $u->update({ email => $existing }) };
+like($@, qr/already exists/, "Unable to rename user to existing username");
+
+# Check cannot create same username as existing
+try { $schema->resultset('User')->create_user(%template, email => $existing) };
+like($@, qr/already exists/, "Unable to create user with existing username");
+
+# Same directly in resultset
+try { $schema->resultset('User')->create({email => $existing}) };
+like($@, qr/already exists/, "Unable to create user with existing username");
+
 $site->update({ register_organisation_mandatory => 1 });
 try { $schema->resultset('User')->create_user(%template, email => '[email protected]') };
 like($@, qr/Please select a Organisation/, "Failed to create user missing org");