"Purge" deleted user records (actor.usr) which result from a patron merge operation

[Dan Wells]

EG 2.10

Basic troubleshooting seems to reveal that, when merging patron accounts in the staff client, the non-lead account is simply flagged as "deleted" rather than actively purged from the DB. This has the unfortunate effect of forever occupying certain useful fields (e.g. usrname), and in a way not obvious to the end user.

In current Evergreen, manually "deleting" a patron record will in fact do a "purge" internally, retaining the row for DB integrity reasons, but otherwise filling uniquely-constrained fields with random data to avoid future collisions. Brief discussion on IRC found some support for having the merge operation automatically perform this same purging of data, so this bug is a stake in the ground to possibly move in that direction.

[Jessica Woolford (jwoolford)]

Confirmed in our 2.12 system and in the master sandbox. We would love to see this - we often get help desk inquiries in which the wrong patron was marked as the lead and the barcode and username need to be recovered.

[Michele Morgan (mmorgan)]

We recently became aware of this and discovered close to 4000 patron records in our system where the deleted flag is set to TRUE, but the patron information has not been purged. This is a confidentiality issue, we feel strongly that the patron data should be purged from the deleted record as part of the merge functionality.

Adding a link to the IRC discussion Dan mentions in the description:

http://irc.evergreen-ils.org/evergreen/2016-11-21#i_277155

[Rogan Hamby (rogan-hamby)]

patch with upgrade: f7dad5c7dd69b5f87a6bdfbd9b90acc2c99e127d
pg_tap test: 46177df1485529f4ab8b87920c3529e56ea4e532

[Bill Erickson (berick)]

Patch pushed:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1643709-user-merge-purges

Tested and signed off on Rogan's patches. Modified upgrade script to re-sync the usr_merge function to match current master.

[Bill Erickson (berick)]

Commits require additional sign-off.

[Bill Erickson (berick)]

Note my branch is 3.2 compatible only.

[Rogan Hamby (rogan-hamby)]

Pushed signoff here : user/rogan/lp1643709_purge_usrs_on_merge

[Bill Erickson (berick)]

Pinged Rogan to let him know the latest sign-off branch is missing a commit.

[Rogan Hamby (rogan-hamby)]

OK, assuming my brain is working better this morning here is the signoff: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/rogan/lp1709698

[Ben Shum (bshum)]

Looks fine to me, the PG test passes and the change makes sense.

But Rogan's latest linked branch does not have his signed-off-by line in the commit message. I think this is an issue with the origins of the branch content. Will wait to get confirmation from Rogan on which sign-off-by name to append to the commits and then proceed from there.

[Rogan Hamby (rogan-hamby)]

I'm going to blame the plague that claimed me shortly after the hack-a-way
(and probably contracted at the hack-a-way) for my thinko.

pinging you shortly

On Sun, Nov 18, 2018 at 10:20 PM Ben Shum <email address hidden> wrote:

> Looks fine to me, the PG test passes and the change makes sense.
>
> But Rogan's latest linked branch does not have his signed-off-by line in
> the commit message. I think this is an issue with the origins of the
> branch content. Will wait to get confirmation from Rogan on which sign-
> off-by name to append to the commits and then proceed from there.
>
> --
> You received this bug notification because you are subscribed to
> Evergreen.
> Matching subscriptions: evergreenbugs
> https://bugs.launchpad.net/bugs/1643709
>
> Title:
> "Purge" deleted user records (actor.usr) which result from a patron
> merge operation
>
> Status in Evergreen:
> Confirmed
>
> Bug description:
> EG 2.10
>
> Basic troubleshooting seems to reveal that, when merging patron
> accounts in the staff client, the non-lead account is simply flagged
> as "deleted" rather than actively purged from the DB. This has the
> unfortunate effect of forever occupying certain useful fields (e.g.
> usrname), and in a way not obvious to the end user.
>
> In current Evergreen, manually "deleting" a patron record will in fact
> do a "purge" internally, retaining the row for DB integrity reasons,
> but otherwise filling uniquely-constrained fields with random data to
> avoid future collisions. Brief discussion on IRC found some support
> for having the merge operation automatically perform this same purging
> of data, so this bug is a stake in the ground to possibly move in that
> direction.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/evergreen/+bug/1643709/+subscriptions
>

[Ben Shum (bshum)]

I got the signoff information from Rogan, but thinking about this bugfix, I'm not sure it'll be safe to backport as is. The upgrade SQL is based on master (and 3.2), and includes a new feature from bug 1776020.

As such, if we were to backport the upgrade script to 3.1, it would break 3.1 installations.

We *could* create a separate upgrade SQL script for 3.1 only, reserve the space in master and then run it again with a second upgrade script ID number for 3.2 and master.

So for example, 1137 would be the upgrade script for 3.1's version of the actor.usr_merge function. Then 1138 would be the 3.2 and master version.

Will bring up with other committers on best practice here.

Or we say this is only a fix for 3.2 and folks should upgrade to the latest and best.

See Ben's monologue ramblings in IRC too: http://irc.evergreen-ils.org/evergreen/2018-11-19#i_386093

[Ben Shum (bshum)]

Pushed to master and rel_3_2 as is. Special backport for rel_3_1 also included.

Upgrade script 1137 is for 3.1 only, 1138 is for 3.2 and master and beyond.

[Rogan Hamby (rogan-hamby)]

I have no feelings about back porting it and personally think it would be
fine for 3.2+ only but others may have seen more impactful situations than
I have.

On Tue, Nov 20, 2018 at 9:35 AM Ben Shum <email address hidden> wrote:

> Pushed to master and rel_3_2 as is. Special backport for rel_3_1 also
> included.
>
> Upgrade script 1137 is for 3.1 only, 1138 is for 3.2 and master and
> beyond.
>
> ** Also affects: evergreen/3.1
> Importance: Undecided
> Status: New
>
> ** Changed in: evergreen/3.1
> Milestone: None => 3.1.8
>
> ** Changed in: evergreen/3.1
> Status: New => Fix Committed
>
> ** Changed in: evergreen
> Status: Confirmed => Fix Committed
>
> --
> You received this bug notification because you are subscribed to
> Evergreen.
> Matching subscriptions: evergreenbugs
> https://bugs.launchpad.net/bugs/1643709
>
> Title:
> "Purge" deleted user records (actor.usr) which result from a patron
> merge operation
>
> Status in Evergreen:
> Fix Committed
> Status in Evergreen 3.1 series:
> Fix Committed
>
> Bug description:
> EG 2.10
>
> Basic troubleshooting seems to reveal that, when merging patron
> accounts in the staff client, the non-lead account is simply flagged
> as "deleted" rather than actively purged from the DB. This has the
> unfortunate effect of forever occupying certain useful fields (e.g.
> usrname), and in a way not obvious to the end user.
>
> In current Evergreen, manually "deleting" a patron record will in fact
> do a "purge" internally, retaining the row for DB integrity reasons,
> but otherwise filling uniquely-constrained fields with random data to
> avoid future collisions. Brief discussion on IRC found some support
> for having the merge operation automatically perform this same purging
> of data, so this bug is a stake in the ground to possibly move in that
> direction.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/evergreen/+bug/1643709/+subscriptions
>