[MODINVSTOR-923] Inventory Storage Batch Sync APIs upsert functionality mints new HRIDs despite previously assigned to posted records Created: 08/Jun/22  Updated: 26/Oct/22  Resolved: 15/Sep/22

Status: Closed
Project: mod-inventory-storage
Components: None
Affects versions: None
Fix versions: None

Type: Bug Priority: P2
Reporter: Theodor Tolstoy (One-Group.se) Assignee: Julian Ladisch
Resolution: Won't Do Votes: 0
Labels: data-migration, metadatamanagement, support
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original estimate: Not Specified

Attachments: PNG File image-2022-06-08-10-09-44-244.png     PNG File image-2022-06-08-10-10-13-977.png     PNG File image-2022-06-08-10-11-20-800.png     PNG File image-2022-06-08-10-11-49-310.png    
Issue links:
Relates
relates to MODINVSTOR-958 Keep old hrid when updating instance,... Open
relates to MODINVSTOR-924 Batch update with optimistic locking ... Closed
Sprint: CP: Sprint 148, CP Sprint 146, CP: Sprint 147
Development Team: Core: Platform
Release: Nolana (R3 2022)
Affected Institution:
5 Colleges, BNCF, Caltech, Chalmers, Cornell, Duke, Grand Valley State University, HBZ, Lehigh, MI State University/Library of Michigan, Middle Tennessee State University, MO State, Okanagan College, OTHER, SHL, Simmons, Skidmore College, Spokane Public Libraries, St. Michael's College, STU, TAMU, Texas Southmost College, Trinity College, Universidad de Concepción, University of Alabama, University of Chicago, University of Colorado, University of Leipzig, University of Tennessee Martin, University of Zaragoza, Villanova University, Warner, Washington College, Washington-Jefferson
RCA Group: TBD

 Description   

Overview:

The Inventory Storage Batch Sync APIs upsert functionality is used extensively at initial data migration from legacy systems and is an important lifeline for fixing wide mistakes or corrections that needs to happen to records on a larger scale (millions of records)

With the wider adoption of the HRIDs, and the strict dependencies between SRS and Inventory, the minting of HRIDs, and their immutability is a foundation much other work is built on.  This bug shows how HRIDs are getting replaced in certain circumstances.

Steps to Reproduce:

  1. Fetch a Inventory record from Lotus bugfest, like (baseUrl/instance-storage/instances/f33faeb2-5151-4e10-b8e2-17b6815d98cd
  2. Note down the hrid
  3. Post back the record using the batch update APIs (/instance-storage/batch/synchronous?upsert=true) with the correct _version, but without the hrid

Expected Results:

The HRID stays the same,
Actual Results:

FOLIO mints a new HRID for the record , 
Additional Information:

This will likely break the connection with SRS, since the HRID is stored in multiple places on the SRS records, rendering FOLIO unstable in areas of QuickMARC and DataImport

See screenshots below
Fetched record:

POSTED record:


**

Note: The single record API rejects the record: PUT /instance-storage/instances/$id

The hrid field cannot be changed: new=null, old=inst000000000015

Interested parties:

MM sig, Data Import Data Migration Subgroup



 Comments   
Comment by Kyle Banerjee [ 08/Jun/22 ]

The Inventory Storage Batch Sync APIs upsert functionality is used extensively at initial data migration from legacy systems and is an important lifeline for fixing wide mistakes or corrections that needs to happen to records on a larger scale (millions of records)

 

Aside that identifiers that change undermine the very purpose of identifiers, many libraries depend on the ability to set this value because it is a foreign key in external systems for integrations. 

Comment by Brooks Travis [ 08/Jun/22 ]

Charlotte Whitt Ian Walls FYI

Comment by Charlotte Whitt [ 08/Jun/22 ]

Kyle Banerjee - I have set the priority to P2, is that compliant with your understanding as well?

Comment by Kyle Banerjee [ 08/Jun/22 ]

Charlotte Whitt Based on my experience, I believe P2 accurately conveys priority

Comment by Julian Ladisch [ 08/Jun/22 ]

Why do you know the correct _version property but not the correct hrid property?

Comment by Julian Ladisch [ 08/Jun/22 ]

This Jira is asking for a PATCH API that updates some properties and keeps other properties.

Comment by Kyle Banerjee [ 08/Jun/22 ]

Julian Ladisch Desirable behavior is that you don't need to know correct _version property and that HRID is retained https://folio-org.atlassian.net/browse/MODINVSTOR-910

https://folio-org.atlassian.net/browse/SUP-74 

Comment by Marc Johnson [ 09/Jun/22 ]

Kyle Banerjee Charlotte Whitt Theodor Tolstoy (One-Group.se)

The HRID stays the same

As I understand it, these APIs are intended to receive a complete representation of record.

This expectation suggests that folks want them to accept a partial representation of the record and combine that with the stored representation on an update (when the upsert parameter is provided).

I think this is what Julian Ladisch is referring to when he describes a PATCH API.

How wrong is my interpretation of the expectations?

Comment by Anya [ 13/Jun/22 ]

Support: Jakub Skoczen what is the release target? Thanks 

Comment by Anya [ 27/Jun/22 ]

Support: Updated the release to be morning glory bugfix. 

Comment by Jakub Skoczen [ 15/Aug/22 ]

Julian Ladisch Marc Johnson Given that the PR has been open for more than three weeks now and that there is a disagreement if this should be included in MG, should we postpone until Nolana? Theodor Tolstoy (One-Group.se) how badly do we need this for MG?

Comment by Marc Johnson [ 15/Aug/22 ]

Jakub Skoczen

Given that the PR has been open for more than three weeks now and that there is a disagreement if this should be included in MG, should we postpone until Nolana?

I don't know. Who has expressed disagreement about whether it should be included or not?

(I've given feedback on how we've chosen to implement the changes, as requested, I don't consider that a question of inclusion or not. I do think that it could be better if a smaller change had been made as an interim measure for Morning Glory)

Comment by Jakub Skoczen [ 05/Sep/22 ]

Marc Johnson Ok, I reread your comments and it's clear to me that the disagreement is conceptual and not specific to which version the code is released in.

Theodor Tolstoy (One-Group.se) can you clarify in your description what the "expected result" is? I assume that the system returning an error status when an HRID is omitted NOT the expected behavior but instead you expect a successful operation with the old HRID be used?

Comment by Marc Johnson [ 05/Sep/22 ]

Jakub Skoczen

I reread your comments and it's clear to me that the disagreement is conceptual and not specific to which version the code is released in.

Yes, it is a conceptual concern. The logic in this module has been moved back and forth between mechanisms across multiple teams (for example, MODINVSTOR-625 Closed ). This is rather unfortunate, because it leads to more work and confusion for developers.

and not specific to which version the code is released in.

At one point, I think it was intended that this would be back ported to Morning Glory. I think the scope of the changes in that pull request are a lot for a back port.

Comment by Charlotte Whitt [ 06/Sep/22 ]

Support SIG: The developers are discussing the work. The release date has been postponed to Nolana.

Comment by Theodor Tolstoy (One-Group.se) [ 15/Sep/22 ]

Theodor Tolstoy (One-Group.se) can you clarify in your description what the "expected result" is? I assume that the system returning an error status when an HRID is omitted NOT the expected behavior but instead you expect a successful operation with the old HRID be used?

Sorry for the late reply Jakub Skoczen , but I would expect that i would get the new record back, but that the folio-minted (. the old one, the FOLIO-generated, the immutable one. ) HRID stays the same

Comment by Adam Dickmeiss [ 15/Sep/22 ]

This issue came up 3 years ago: https://folio-org.atlassian.net/browse/MODINVSTOR-397

Comment by Jakub Skoczen [ 15/Sep/22 ]

Theodor Tolstoy (One-Group.se) then what has been implemented so far will not meet your expectations.

We have discussed internally in the Platform team that HRID is already special (meaning that it can be only set during record creation and never changed) so we are going to instead implement a special handling for this field during update as described in MODINVSTOR-958 Open .

I am closing this ticket as won't do.

Comment by Theodor Tolstoy (One-Group.se) [ 06/Oct/22 ]

Sounds right. Thank you Jakub Skoczen 

Generated at Thu Feb 08 22:22:36 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100246-sha1:7a5c50119eb0633d306e14180817ddef5e80c75d.