summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormkanat%bugzilla.org <>2006-10-21 05:47:15 +0200
committermkanat%bugzilla.org <>2006-10-21 05:47:15 +0200
commit810ac042bacc992cc1cb3ae324b0d61b0615b697 (patch)
treeefdc749bd2703a762dea4388eff8cd68c0a6c849
parentea2d2a47281ac947297587c2619df190bf3c23c4 (diff)
downloadbugzilla-810ac042bacc992cc1cb3ae324b0d61b0615b697.tar.gz
bugzilla-810ac042bacc992cc1cb3ae324b0d61b0615b697.tar.xz
Bug 352243: Make editusers.cgi use Bugzilla::User for basic user updates
Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=justdave
-rw-r--r--Bugzilla/Object.pm43
-rw-r--r--Bugzilla/User.pm97
-rwxr-xr-xeditusers.cgi78
3 files changed, 127 insertions, 91 deletions
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 5d80a9d0f..a2ca8ff94 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -149,17 +149,28 @@ sub update {
my $dbh = Bugzilla->dbh;
my $table = $self->DB_TABLE;
my $id_field = $self->ID_FIELD;
+
+ my $old_self = $self->new($self->id);
- my $columns = join(', ', map {"$_ = ?"} $self->UPDATE_COLUMNS);
- my @values;
+ my (@update_columns, @values, %changes);
foreach my $column ($self->UPDATE_COLUMNS) {
- my $value = $self->{$column};
- trick_taint($value) if defined $value;
- push(@values, $value);
+ if ($old_self->{$column} ne $self->{$column}) {
+ my $value = $self->{$column};
+ trick_taint($value) if defined $value;
+ push(@values, $value);
+ push(@update_columns, $column);
+ # We don't use $value because we don't want to detaint this for
+ # the caller.
+ $changes{$column} = [$old_self->{$column}, $self->{$column}];
+ }
}
+ my $columns = join(', ', map {"$_ = ?"} @update_columns);
+
$dbh->do("UPDATE $table SET $columns WHERE $id_field = ?", undef,
- @values, $self->id);
+ @values, $self->id) if @values;
+
+ return \%changes;
}
###############################
@@ -452,9 +463,27 @@ data into the database. Returns a newly created object.
=item C<update>
+=over
+
+=item B<Description>
+
Saves the values currently in this object to the database.
Only the fields specified in L</UPDATE_COLUMNS> will be
-updated. Returns nothing and takes no parameters.
+updated, and they will only be updated if their values have changed.
+
+=item B<Params> (none)
+
+=item B<Returns>
+
+A hashref showing what changed during the update. The keys are the column
+names from L</UPDATE_COLUMNS>. If a field was not changed, it will not be
+in the hash at all. If the field was changed, the key will point to an arrayref.
+The first item of the arrayref will be the old value, and the second item
+will be the new value.
+
+If there were no changes, we return a reference to an empty hash.
+
+=back
=back
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 33c8535f5..ce778c728 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -67,8 +67,8 @@ use constant MATCH_SKIP_CONFIRM => 1;
use constant DEFAULT_USER => {
'id' => 0,
- 'name' => '',
- 'login' => '',
+ 'realname' => '',
+ 'login_name' => '',
'showmybugslink' => 0,
'disabledtext' => '',
'disable_mail' => 0,
@@ -82,8 +82,8 @@ use constant DB_TABLE => 'profiles';
# fixed one day.
use constant DB_COLUMNS => (
'profiles.userid AS id',
- 'profiles.login_name AS login',
- 'profiles.realname AS name',
+ 'profiles.login_name',
+ 'profiles.realname',
'profiles.mybugslink AS showmybugslink',
'profiles.disabledtext',
'profiles.disable_mail',
@@ -101,6 +101,18 @@ use constant VALIDATORS => {
realname => \&_check_realname,
};
+sub UPDATE_COLUMNS {
+ my $self = shift;
+ my @cols = qw(
+ disable_mail
+ disabledtext
+ login_name
+ realname
+ );
+ push(@cols, 'cryptpassword') if exists $self->{cryptpassword};
+ return @cols;
+};
+
################################################################################
# Functions
################################################################################
@@ -117,6 +129,29 @@ sub new {
return $class->SUPER::new(@_);
}
+sub update {
+ my $self = shift;
+ my $changes = $self->SUPER::update(@_);
+ my $dbh = Bugzilla->dbh;
+
+ if (exists $changes->{login_name}) {
+ # If we changed the login, silently delete any tokens.
+ $dbh->do('DELETE FROM tokens WHERE userid = ?', undef, $self->id);
+ # And rederive regex groups
+ $self->derive_regexp_groups();
+ }
+
+ # Logout the user if necessary.
+ Bugzilla->logout_user($self)
+ if (exists $changes->{login_name} || exists $changes->{disabledtext}
+ || exists $changes->{cryptpassword});
+
+ # XXX Can update profiles_activity here as soon as it understands
+ # field names like login_name.
+
+ return $changes;
+}
+
################################################################################
# Validators
################################################################################
@@ -127,13 +162,18 @@ sub _check_disabledtext { return trim($_[1]) || ''; }
# This is public since createaccount.cgi needs to use it before issuing
# a token for account creation.
sub check_login_name_for_creation {
- my ($self, $name) = @_;
+ my ($invocant, $name) = @_;
$name = trim($name);
$name || ThrowUserError('user_login_required');
validate_email_syntax($name)
|| ThrowUserError('illegal_email_address', { addr => $name });
- is_available_username($name)
- || ThrowUserError('account_exists', { email => $name });
+
+ # Check the name if it's a new user, or if we're changing the name.
+ if (!ref($invocant) || $invocant->login ne $name) {
+ is_available_username($name)
+ || ThrowUserError('account_exists', { email => $name });
+ }
+
return $name;
}
@@ -153,12 +193,36 @@ sub _check_password {
sub _check_realname { return trim($_[1]) || ''; }
################################################################################
+# Mutators
+################################################################################
+
+sub set_disabledtext { $_[0]->set('disabledtext', $_[1]); }
+sub set_disable_mail { $_[0]->set('disable_mail', $_[1]); }
+
+sub set_login {
+ my ($self, $login) = @_;
+ $self->set('login_name', $login);
+ delete $self->{identity};
+ delete $self->{nick};
+}
+
+sub set_name {
+ my ($self, $name) = @_;
+ $self->set('realname', $name);
+ delete $self->{identity};
+}
+
+sub set_password { $_[0]->set('cryptpassword', $_[1]); }
+
+
+################################################################################
# Methods
################################################################################
# Accessors for user attributes
-sub login { $_[0]->{login}; }
-sub email { $_[0]->{login} . Bugzilla->params->{'emailsuffix'}; }
+sub name { $_[0]->{realname}; }
+sub login { $_[0]->{login_name}; }
+sub email { $_[0]->login . Bugzilla->params->{'emailsuffix'}; }
sub disabledtext { $_[0]->{'disabledtext'}; }
sub is_disabled { $_[0]->disabledtext ? 1 : 0; }
sub showmybugslink { $_[0]->{showmybugslink}; }
@@ -187,7 +251,7 @@ sub identity {
if (!defined $self->{identity}) {
$self->{identity} =
- $self->{name} ? "$self->{name} <$self->{login}>" : $self->{login};
+ $self->name ? $self->name . " <" . $self->login. ">" : $self->login;
}
return $self->{identity};
@@ -199,7 +263,7 @@ sub nick {
return "" unless $self->id;
if (!defined $self->{nick}) {
- $self->{nick} = (split(/@/, $self->{login}, 2))[0];
+ $self->{nick} = (split(/@/, $self->login, 2))[0];
}
return $self->{nick};
@@ -767,7 +831,7 @@ sub derive_regexp_groups {
AND isbless = 0
AND grant_type = ?});
while (my ($group, $regexp, $present) = $sth->fetchrow_array()) {
- if (($regexp ne '') && ($self->{login} =~ m/$regexp/i)) {
+ if (($regexp ne '') && ($self->login =~ m/$regexp/i)) {
$group_insert->execute($id, $group, GRANT_REGEXP) unless $present;
} else {
$group_delete->execute($id, $group, GRANT_REGEXP) if $present;
@@ -1101,10 +1165,11 @@ sub match_field {
# skip confirmation for exact matches
if ((scalar(@{$users}) == 1)
- && (lc(@{$users}[0]->{'login'}) eq lc($query)))
+ && (lc(@{$users}[0]->login) eq lc($query)))
+
{
$cgi->append(-name=>$field,
- -values=>[@{$users}[0]->{'login'}]);
+ -values=>[@{$users}[0]->login]);
next;
}
@@ -1117,7 +1182,7 @@ sub match_field {
if (scalar(@{$users}) == 1) { # exactly one match
$cgi->append(-name=>$field,
- -values=>[@{$users}[0]->{'login'}]);
+ -values=>[@{$users}[0]->login]);
$need_confirm = 1 if $params->{'confirmuniqueusermatch'};
@@ -1282,7 +1347,7 @@ sub wants_bug_mail {
#
# We do them separately because if _any_ of them are set, we don't want
# the mail.
- if ($wants_mail && $changer && ($self->{'login'} eq $changer)) {
+ if ($wants_mail && $changer && ($self->login eq $changer)) {
$wants_mail &= $self->wants_mail([EVT_CHANGED_BY_ME], $relationship);
}
diff --git a/editusers.cgi b/editusers.cgi
index 5f356fb40..a1c82db9f 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -226,9 +226,6 @@ if ($action eq 'search') {
my $otherUser = check_user($otherUserID, $otherUserLogin);
$otherUserID = $otherUser->id;
- my $logoutNeeded = 0;
- my @changedFields;
-
# Lock tables during the check+update session.
$dbh->bz_lock_tables('profiles WRITE',
'profiles_activity WRITE',
@@ -245,73 +242,19 @@ if ($action eq 'search') {
action => "modify",
object => "user"});
- my $login = $cgi->param('login');
- my $password = $cgi->param('password');
- my $realname = trim($cgi->param('name') || '');
- my $disabledtext = trim($cgi->param('disabledtext') || '');
- my $disable_mail = $cgi->param('disable_mail') ? 1 : 0;
+ $vars->{'loginold'} = $otherUser->login;
# Update profiles table entry; silently skip doing this if the user
# is not authorized.
+ my %changes;
if ($editusers) {
- my @values;
-
- if ($login ne $otherUser->login) {
- # Validating untaints for us.
- $login || ThrowUserError('user_login_required');
- validate_email_syntax($login)
- || ThrowUserError('illegal_email_address', {addr => $login});
- is_available_username($login)
- || ThrowUserError('account_exists', {email => $login});
-
- push(@changedFields, 'login_name');
- push(@values, $login);
- $logoutNeeded = 1;
-
- # Since we change the login, silently delete any tokens.
- $dbh->do('DELETE FROM tokens WHERE userid = ?', {}, $otherUserID);
- }
- if ($realname ne $otherUser->name) {
- # The real name may be anything; we use a placeholder for our
- # INSERT, and we rely on displaying code to FILTER html.
- trick_taint($realname);
- push(@changedFields, 'realname');
- push(@values, $realname);
- }
- if ($password) {
- # Validating untaints for us.
- validate_password($password) if $password;
- push(@changedFields, 'cryptpassword');
- push(@values, bz_crypt($password));
- $logoutNeeded = 1;
- }
- if ($disabledtext ne $otherUser->disabledtext) {
- # The disable text may be anything; we use a placeholder for our
- # INSERT, and we rely on displaying code to FILTER html.
- trick_taint($disabledtext);
- push(@changedFields, 'disabledtext');
- push(@values, $disabledtext);
- $logoutNeeded = 1;
- }
- if ($disable_mail != $otherUser->email_disabled) {
- push(@changedFields, 'disable_mail');
- push(@values, $disable_mail);
- }
- if (@changedFields) {
- push (@values, $otherUserID);
- $logoutNeeded && Bugzilla->logout_user($otherUser);
- $dbh->do('UPDATE profiles SET ' .
- join(' = ?,', @changedFields).' = ? ' .
- 'WHERE userid = ?',
- undef, @values);
- # XXX: should create profiles_activity entries.
- #
- # We create a new user object here because it needs to
- # read information that may have changed since this
- # script started.
- my $newprofile = new Bugzilla::User($otherUserID);
- $newprofile->derive_regexp_groups();
- }
+ $otherUser->set_login($cgi->param('login'));
+ $otherUser->set_name($cgi->param('name'));
+ $otherUser->set_password($cgi->param('password'))
+ if $cgi->param('password');
+ $otherUser->set_disabledtext($cgi->param('disabledtext'));
+ $otherUser->set_disable_mail($cgi->param('disable_mail'));
+ %changes = %{$otherUser->update()};
}
# Update group settings.
@@ -399,8 +342,7 @@ if ($action eq 'search') {
delete_token($token);
$vars->{'message'} = 'account_updated';
- $vars->{'loginold'} = $otherUser->login;
- $vars->{'changed_fields'} = \@changedFields;
+ $vars->{'changed_fields'} = [keys %changes];
$vars->{'groups_added_to'} = \@groupsAddedTo;
$vars->{'groups_removed_from'} = \@groupsRemovedFrom;
$vars->{'groups_granted_rights_to_bless'} = \@groupsGrantedRightsToBless;