Wishlist: Barcode creation date

Bug #1705332 reported by Josh Stompro
26
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Evergreen
Confirmed
Undecided
Unassigned

Bug Description

EG 2.10

It would be useful to have a creation date field in the actor.card table to store when barcodes are created/issued. This should be visible to staff in the staff clients, show all cards display.

This would be useful for a few reasons.

1. If a customer has multiple active cards, it would show staff which card was given out most recently. For cases where we only want one active card and the others will be marked inactive. It could be that the sort order of the barcodes already indicates this info, but this would make it clear.

2. If a library charges for replacement cards, but has a policy that gives out one free card every x years. This would allow staff to see the last time a new card was given out.

It may be possible to use the auditor.actor_card table to pull in the creation dates for existing cards, maybe just use the actor.usr creation date for those that existed before there was history or before a migration.

Josh

Tags: patron
Revision history for this message
Bill Ott (bott) wrote :

Here at GRPL, we've had a local customization for this for years. Here's a look at the commit against 2.10. I've also added changes to the Web client under 2.11, but I don't have those in a public repository yet.

https://github.com/grpl-eg/eg_2_10/commit/3d9c6f1a05f9dba6d2dd44bd135c1d71a7b1f55a

Changed in evergreen:
status: New → Confirmed
Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Bill, thank you for sharing this. How did you handle the initial switch over? How did you fill in the values for barcodes already created when you enabled this feature?

I've been trying to use the auditor.actor_usr_lifecycle table to get this info. My brain always has trouble with the design of the auditor tables. I get why they are the way they are, but the whole, 'always look at the next row to see what the audit_user did' always throws me. I think it can be done with window functions, lead(), to get the next line's card id.

But I don't know if it is really a good idea to try and depend on auditor data for an upgrade script. Maybe if this does make it in to a release, there should be no attempt to set old dates and creators, with some optional steps that the sysadmin could take to try and fill those in.

Josh

Revision history for this message
Bill Ott (bott) wrote :

We didn't try to do anything special. For us, the epoch of all pre-existing cards became the time/date of implementing the feature (2010-12-16 22:10:04), and the "creator" was the actor.id 1. We've just come to know that any card with this create_date was created prior to that time.

In hindsight, we could have taken every user with only one actor.card and set the date to the actor.create_date, or even used that for the lowest id'd actor.card for a given user, but again, we only planned to use it going forward.

Revision history for this message
Bill Erickson (berick) wrote :

I'll second the concern about using auditor data as the default option, since it's not uncommon to purge data from auditor tables to keep the size in check. Plus, if they're big enough, it can significantly slow down the upgrade. Of the alternate options, I prefer a canned date (a la Bill O's deployment) to creating a partially accurate data set.

Revision history for this message
Mike Rylander (mrylander) wrote :

ISTM that the main benefit of a preordained date is that it's a signal to staff that, for a given barcode, we don't know when it was created. Since, in the "all barcodes" UI, staff will be looking at the set of barcodes for exactly one user, I think having the same date for all the barcodes conveys, in context, the same signal. So, how about just using the create date for the user on all historical barcodes, and then we have, at least, accurate data on the first? The benefit would be that reports could reliably use the card as a proxy for the user, where useful.

Another option would be to allow the field to be NULL and only update the first card on the user. Since NULL means "unknown" in the database, that seems like the smallest lie...

Revision history for this message
Bill Ott (bott) wrote :

I did essentially leave the values NULL when we implemented the changes. The values were simply populated by the table definition:

 creator | integer | not null default 1
 create_date | timestamp with time zone | not null default now()

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I say let the field be null and don't set any dates. That way we're not lying at all.

I'm also thinking of the occasional mistakes that happen or other strange things when I've been asked to move barcodes around or even delete them in the database.

You really have no idea when a barcode was created after the fact. It might be a safe bet for many patrons that the lowest id barcode was created at the same time as the user, but there are no guarantees for any of them.

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

So how about for a plan:

For adding the feature, allow null values in creator and create_date and don't try to set default values initially during the upgrade.

Add some documentation and example queries for setting other values for barcodes that already exist, so each site can choose if they want to try to make some guesses. The release notes can mention the issues with a link to the documentation.

Josh

Revision history for this message
Jason Stephenson (jstephenson) wrote :

That works for me, but go with the consensus if everyone else disagrees with me about guessing for defaults.

Revision history for this message
Kathy Lussier (klussier) wrote :

+1 from me

Revision history for this message
Bill Erickson (berick) wrote :

-1 for me, nestled under my coder's hat.

It seems to me creator and create_date are fields one would assume have a value. This will invariably lead to confusion. It complicates the code / reports and serves no purpose for new installs. Maybe it's worth it in this case for historical data accuracy, but its value will diminish over time while any added complexity will not.

Revision history for this message
Bill Erickson (berick) wrote :

For my last comment, I'm not defending the Alamo, just offering my preference. It's a great feature either way. Don't let me hold it up. Big ++ to documenting example queries.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Just to contradict myself, I not opposed to adding a default of now in the database. I just think a null would be better for pre-existing ones, but if all of the upgraded ones have the same date, that would be a similar, though less obvious, clue.

tags: added: patron
Revision history for this message
Michele Morgan (mmorgan) wrote :

Attempting to kick this along a bit further.

In addition to making it easier for staff to see when cards were issued, creator and create_date in the actor.card table has statistical value, too. Currently it's not possible to answer the question:

How many cards were given out last month?

Not to be confused with how many new patrons were registered last month.

Bill's defaults make the most sense to me:

creator | integer | not null default 1
create_date | timestamp with time zone | not null default now()

I also like Josh's suggestion in comment #8 about linking to documentation and example queries from the release notes. This would provide some alternate strategies for handling existing data if the defaults don't work for some sites.

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Bill, have you upgraded to 3.3+ and brought this change along with that yet? Do you have the web client code you spoke about available to see?

Thanks
Josh

Revision history for this message
Bill Ott (bott) wrote :

I don't have it in a public repo, but here is the patch from my local install of 3.7.

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

Here is Bills patch applied to Master.

user/stompro/lp1705332_patron_barcode_date_creator_master

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1705332_patron_barcode_date_creator_master

I think we need an upgrade script to make the DB changes along with the release notes that have the examples of how to populate the empty data if wanted.

I'll work on that

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I found and email from Bill Ott to the dev list 11 years ago when he first shared this change, there was some discussion about changes to the user purge function that may need to be added in.

http://georgialibraries.markmail.org/thread/s4qs2ljgw7kr2r6u

Quote Bill Erickson
"3. As of 2.0, there is a user purging database function that scrubs away the
existence of a user. The staff version of this call requires that any
object linked to a staff user be handed over to another staff account during
purge. As such, any new fields added to the DB that link to (what in
practice would have to be) a staff account, need to be accounted for in the
purge routine. The code lives in 999.functions.global.sql and it's
called actor.usr_purge_data. In here, there are a series of update commands
grouped by schema for convenience. Something like this should do it:

UPDATE actor.card SET creator = dest_usr WHERE creator = src_usr;

-b
"

Something extra to add and test.

Josh

Revision history for this message
Josh Stompro (u-launchpad-stompro-org) wrote :

I updated my branch at
user/stompro/lp1705332_patron_barcode_date_creator_master

It now includes the sql changes, including the delete and merge function updates, and a start on release notes. This is all untested, and I'm still working on the release notes. I'll try and keep working on it as I have time.

I'm trying to figure out a few different examples of how the initial data could be set to show in the release notes.
Josh

Revision history for this message
Chris Sharp (chrissharp123) wrote :

PINES recently removed card replacement fees and we now have a similar need. Josh, I don't see a pull request here, but I'll give your branch some testing.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.