GitLab

Save the original and call it so MergeUsers will pick up any
future changes to RT::User::BeforeWipeout.

If a user record has other users merged into it and they are
not unmerged before shredding, the merged users are left in
a state where they can't be loaded because they still point
to another user (EffectiveId), but that id is no longer in the
DB.

When a user with another user merged into it is shredded,
the attributes on that user are also shredded, but the
merged user will remain, along with attributes that may point
to the now missing user id. This script cleans up attributes
if the merged-into user record is now gone.

When running shredder to clean up users, the effective id
for a merged ticket can be shredded. Add error checking
when loading an effective id to detect this case. On
failure, return the id of the current user id and warn
that the user it was merged into is missing.

Disabling a user did not disable all of the users merged into them.
This caused users disabled users to apparently still appear via the web
UI in user searches, as their merged-in (non-disabled) users often still
matched the search.

Explicitly propagate Disabled updates on users down into their merged-in
equivalents.  Note that changes to sub-users do not propagate upward;
loading them takes explicit effort, changing them could be more
surpising action-at-a-distance, and introduces difficulties with dealing
with cycles.

03573403 switched several uses of $RT::SystemUser to $self->CurrentUser,
on the basis that the current user should be propagated throughout,
rather than fixed to the current user.  However, the change in
CanonicalizeEmailAddress was overzealous, as that can be called as a
class method.

As the $canonical_user in this context is lexical, is not altered, and
should not be ACL'd in any way, hardcoding the system user is correct;
restore it.

This allows the Creator of the EffectiveId to serve as a signal as to who merged the users

This caused not merged users at the end of the collection
to be skipped.

_RecordCount is used in SB's Next to stop the iterator.
If we change return value, but don't change storage then
iterator stops earlier.

Failure to do so would cause the following to fail:
    my $id = $users->First->id;
    $users->GotoFirstItem;
    is( $id, $users->First->id );

...as the firt user would have been marked "already seen" and thus would
be ignored on successive iterations through the collection.

This allows merged users to be properly picked up when loaded via an
RT::CurrentUser object, such as during login.

An equivalent core change, on 4.0/currentuser-attributes, is also
necessary to prevent inconsistency.

c7467a91 missed the one case in which users do not have an attribute --
namely, when the user object does not exist yet.  During user creation,
do not present the "Merge User" box at all, as merging would not work.

Hook::LexWrap is unnecessary: the wrapping being done was not lexical,
and it was not being used for its abilities to alter return or argument
values.  Remove it, as using it anywhere causes _all_ stack traces to
lack argument values at all levels of the stacktrace.

$canonical_self->SetComments() calls a $self->Load( $self->id ) during
the process of updating the Comments.  Because this now loads the data
of the user we are merged into, the information recorded on $merged was
always contentless -- namely, it always showed the the user had been
merged into itself.

Swap the order of the ->SetComments calls, to ensure that
$canonical_self is still the user being merged in when it is inspected
to record data on $merge.

As CanonicalizeEmailAddress uses ->LoadByCols to look up the email
address, and we override LoadByCols to chase EffectiveId after a
successful LoadByCols call, looking up EffectiveId again is unnecessary.

The recursive call is kept, on the possibility that
CanonicalizeEmailAddressMatch will match the _new_, canonicalized form.
This preserves the previous behavior.

As the path to get both user objects uses ->Load, which calls
->LoadByCols, which we have overridden, we already have the most
canonical form.  Checking it for EffectiveId is unnecessary and
wasteful.