duplicate entries for a copy in asset.latest_inventory table

Bug #1883171 reported by tji@sitka.bclibraries.ca
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Undecided
Unassigned

Bug Description

EG 3.3, 3.5

We found duplicate records for a copy in asset.latest_inventory table, though we did not recreate the issue when testing on 3.5. Some examples are listed below. The interval between the two timestamps varies widely, indicating the cause is unlikely double scanning.

  id | inventory_workstation | inventory_date | copy
--------+-----------------------+-------------------------------+----------
  89543 | 9914 | 2020-04-20 10:51:48.717988-07 | 19224
  89544 | 9914 | 2020-04-20 10:51:48.738987-07 | 19224

 209215 | 9479 | 2020-06-09 10:09:46.085538-07 | 39840
 209214 | 9479 | 2020-06-09 10:09:46.04205-07 | 39840

   2802 | 9479 | 2019-12-12 17:40:06.975586-08 | 24767
   2801 | 9479 | 2019-12-12 13:06:38.223602-08 | 24767

   6328 | 9479 | 2019-12-21 13:14:07.360586-08 | 45898
   6329 | 7187 | 2019-12-20 13:04:39.142012-08 | 45898

   4859 | 9914 | 2020-05-20 09:23:11.591998-07 | 33950
   4858 | 9479 | 2019-12-18 17:51:12.433881-08 | 33950

   8533 | 9914 | 2020-05-14 10:59:50.187457-07 | 54776
   8532 | 9479 | 2020-01-05 14:00:34.211264-08 | 54776

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

Marking this Confirmed.

We have found duplicate entries for copies in asset.latest_inventory in our 3.6.1 production system, but have not been able to reproduce the creation of duplicates.

Changed in evergreen:
status: New → Confirmed
Revision history for this message
Michele Morgan (mmorgan) wrote :

Just a bit more info.

The sample duplicate rows that Tina shared above all have consecutive id numbers. So the duplicates are being created at the same time, but subsequent inventory updates will update one of the duplicate rows, resulting in the different timestamps. So it is possible those could be created from double scans at checkin with the modifier turned on.

We have similar entries in our production database.

Looking through the latest_inventory table and logs in our own system, however, I'm not convinced that all duplicates are the result of double scanning. Here is an example from our production asset.latest_inventory of a few rows that show 4 entries for the same copy:

id inventory_workstation inventory_date copy

205693 4883 2021-08-10 10:36:14.22955-04 8017953
205674 4883 2021-08-10 10:25:49.328738-04 8017953
205662 4883 2021-08-10 10:25:49.154828-04 8017953
205648 4883 2021-08-10 10:25:48.913449-04 8017953

205671 4883 2021-08-10 10:36:14.22955-04 9045334
205658 4883 2021-08-10 10:25:49.328738-04 9045334
205652 4883 2021-08-10 10:25:49.154828-04 9045334
205645 4883 2021-08-10 10:25:48.913449-04 9045334

205862 4883 2021-08-10 10:36:14.22955-04 9045336
205844 4883 2021-08-10 10:25:49.328738-04 9045336
205824 4883 2021-08-10 10:25:49.154828-04 9045336
205794 4883 2021-08-10 10:25:48.913449-04 9045336

205866 4883 2021-08-10 10:36:14.22955-04 9045351
205847 4883 2021-08-10 10:25:49.328738-04 9045351
205828 4883 2021-08-10 10:25:49.154828-04 9045351
205798 4883 2021-08-10 10:25:48.913449-04 9045351

Given the 4 identical timestamps across items, this was likely created by choosing Update Inventory multiple times on a group of items in Item status. There are 55 items that show these 4 identical timestamps.

tags: added: inventory
Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I am working on a proposal to fix this bug and thought I would solicit feedback from the community before I complete it.

I see four approaches to a fix:

1. The simplest would be to implement what appears to be the intent the asset.latest_inventory table by adding a unique constraint on the copy column and modifying the Evergreen code as necessary to prevent it from adding multiple entries.

2. A ramification of the above is that we end up with a table that has two useful data columns and a 1:1 mapping to the asset.copy table. It would be simpler from a schematic point of view to drop the asset.latest_inventory table and move the useful columns, inventory_workstation and inventory_date, to the copy table. The code would then be modified accordingly.

This solution has the drawback of requiring changes to reports. The asset.latest_inventory table could be replaced with a view in order to prevent the requirement of modifying existing reports. In this case, we end up with an extra view for the sake of backward compatibility.

3. It may be desirable to store multiple, different inventory dates for a copy. This would allow historical tracking of when a copy was “seen” for the purposes of inventory. In this case, the current table could be renamed, a unique constraint added on the copy and inventory_date columns, and the current table name redefined as a view limited to the most recent inventory table entry. This arrangement could facilitate the future expansion of additional fields to track how the inventory occurred (check-in/item status, etc.) as well as the status of the copy at the time of the inventory, and so forth. It could be an initial step before moving on to the next solution.

4. Implement an inventory module for Evergreen. This option goes well beyond the scope of resolving this one issue, requires a good deal more upfront specification and planning, as well as a good deal more work. This should be the subject of its own bug if someone desires greater inventory capability in Evergreen.

If you have an interest/preference for any of the above, please add a comment below.

Revision history for this message
Terran McCanna (tmccanna) wrote :

Although 4 would be wonderful long-term, I think 3 would be the best short-term solution to provide the most flexibility for various institutional needs.

Revision history for this message
Galen Charlton (gmc) wrote :

I also favor #3.

Revision history for this message
Elaine Hardy (ehardy) wrote :

Since we have several failed attempts at developing an inventory module because of development costs, I favor #3

Revision history for this message
Jason Boyer (jboyer) wrote :

I'm not a fan of 2 for the same reasons outlined in the original bug 1940663.

I'd suggest 1 or 3. The first option is good in that it's adding a trigger to a table and telling admins to cleanup any existing dupes (or do it automatically like the duplicate report schedule fix). Number 3 is good in that it does bring us one notch closer to a more fully featured inventory module without necessarily requiring significant changes.

While 4 is a great long term plan that definitely wouldn't be backported and would take a significant amount of time to implement.

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

I would favor #3. Second choice would be #1.

#3 would offer some enhanced functionality, and would be a step toward more. It would also help to mitigate effects of related bug 1940663 since it would preserve existing entries rather than update them.

#1 sounds like it could be a quick fix if #3 does not pan out for some reason.

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

Thanks, everyone! I will base my proposal on solution #3.

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

In order to implement #3, I plan to add new table (asset.copy_inventory) with an appropriate IDL entry (aci) and a link to the acp (asset.copy) object. The asset.latest_inventory table (alci IDL object) will be converted into a view. Necessary changes to the back end and staff client will be made in order to update the new asset.copy_inventory table. I can do the latter in one of two ways:

One is to expose the new table/object in the staff client so that they are readily available and possibly manipulable by staff. This could be done in item status, for instance. This would require rather extensive modifications to several of the web staff client templates and JavaScript files.

The second option is not to expose the new field in the staff client, leaving the latest inventory date exposed, and rerouting any updates to that field to the new table. The advantage of this approach is that it modifies fewer interfaces than the previous option.

In both cases, the new table and the latest inventory view will be available in reports.

Edit:

I prefer the second option as it also requires fewer documentation and training updates. It also hides the backend changes from general staff so that they could be later exposed in an inventory-specific manner.

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

Hi Jason,

Thanks for your thoughts on this. We also agree that the second option would be the best approach. The latest inventory is the most important datapoint for staff view via the client. More in depth inventory data can be obtained if needed via reports.

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

While working on the database upgrade script for the table changes, I realized that with the existing table and my new table definition it is possible to set the inventory_workstation and inventory_date to NULL.

I have decided to add a NOT NULL constraint to the inventory_date because what's the point in allowing the date to be NULL?

However, for the inventory_workstation, I can see an argument for allowing the field to be NULL. By allowing it to be NULL and adding ON DELETE SET NULL to the REFERENCES constraint, one can delete workstation entries without having to delete inventory entries. When workstations are deleted, the corresponding inventory_workstation fields will be set to NULL.

At the same time, I can see the above as being considered a loss of too much information by some users. It could be that you really want to keep the inventory_workstation as it is the only current link to where the item was seen, particularly for floating items. By not allowing NULL in this column and maintaining the link to actor.workstion, these entries will prevent workstations from being deleted.

All of that said, I don't think workstation deletion is a common practice. As of right now, I think it is only possible to do in the database, so I may be worried over nothing.

If anyone has any opinions about the inventory_workstation being allowed to be NULL, please let me know by commenting on the bug.

Revision history for this message
Jason Boyer (jboyer) wrote :

Deleting a workstation was clearly intended to be possible at some point in the development of Evergreen because there are multiple aw links with ON DELETE SET NULL, including circulation which I would imagine is as or more important as inventory.

But the fact that money.bnm_payment and action.in_house_use (at least) do NOT have that set on their aw constraint it doesn't really matter which way it goes, because you can't realistically delete workstations without disabling (multiple) constraints or also deleting a lot of other things anyway.

tl;dr: You may as well allow the aw link to be null for consistency with the past but in the end it probably doesn't matter.

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

Jason, I heartily agree with adding a NOT NULL constraint to the inventory_date field. As for the inventory_workstation field, having no constraint is acceptable, and consistent, as Jason Boyer points out.

I can see a need to delete workstations, and would like to see enhancements to workstation management at some point. I wouldn't want to introduce changes to further complicate such a process.

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

Thanks, Jason and Michele!

I will leave the inventory_workstation column as is. That means copy inventory will prevent workstation deletions and will need to be updated to NULL before the actor.workstation can be deleted. This will have the side effect of breaking the link between the inventory entry and the workstation be a deliberate act.

Changed in evergreen:
status: Confirmed → In Progress
Revision history for this message
Jason Stephenson (jstephenson) wrote :

A branch which addresses this bug and bug 1940663 is available in the working repository:

user/dyrcona/lp1883171-lp1940663-inventory-date-changes-rebase

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1883171-lp1940663-inventory-date-changes-rebase

The complete change set consists of the top 6 commits on the branch and includes release notes, backend (Perl) code changes, database & IDL changes, web staff client changes, Perl live tests, and pgtap tests for the database changes and functionality.

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
status: In Progress → Confirmed
milestone: none → 3.9-beta
tags: added: pullrequest
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I have added an additional commit to the branch to address something that came up during testing, so it is now the top 7 commits on the branch. This last one could be squashed into one of the others if desired.

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

NOBLE has tested this code extensively on both concerto and production data and we are satisfied that it resolves the issue. While multiple copy inventory rows will be recorded for an item in asset.copy_inventory, the asset.latest_inventory view will always reflect the most recent inventory information for the item.

My signoff is at:

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

user/mmorgan/lp1883171_lp1940663_inventory_date_changes_signoff

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

I'll mark this one fix-committed (rather than duplicate, or invalid) as its complaint is covered by the now-merged branch from bug 1940663. Thanks, Jason and all!

Changed in evergreen:
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers