[MODINVSTOR-987] Constraint item level call number type ID to refer to a known type Created: 06/Oct/22  Updated: 05/Feb/24

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

Type: Bug Priority: P2
Reporter: Theodor Tolstoy (One-Group.se) Assignee: Pavlo Smahin
Resolution: Unresolved Votes: 1
Labels: back-end, compatibility-breaking, known-issue-morningglory, known-issue-nolana, known-issue-orchid, known-issue-poppy, needs-releasenote-orchid, release-notes-required, spike, support
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original estimate: Not Specified

Attachments: File HoldingsItemCallNumberTypes.mp4     PNG File image-2022-10-06-15-56-54-222.png     PNG File image-2022-11-12-12-28-26-510.png     PNG File image-2022-11-12-12-32-44-304.png     PNG File image-2022-11-12-12-41-31-568.png     File msplit00000006.mrc    
Issue links:
Blocks
blocks MODINV-757 Support new major item-storage interface Blocked
Gantt End to Start
has to be done after MODINVSTOR-995 Identify item records that have inval... Closed
Relates
relates to UXPROD-4046 NFR : General Spitfire Bugfixes/Tech ... In Progress
relates to MODOAIPMH-442 bad data in call number type field re... Closed
relates to MODINVSTOR-1064 Enhancing Data Validation and Constra... Draft
relates to MODOAIPMH-450 Spike - investigate possible approach... Closed
Sprint:
Development Team: Spitfire
Release: Quesnelia (R1 2024)
RCA Group: TBD

 Description   

Overview:

There was item, created via Data Import, that blocked the entire OAI-PMH feed for one of our libraries because the itemLevelCallNumberTypeID had been mapped to a call number instead fo the type. 
Steps to Reproduce:

  1. Log into some FOLIO environment
  2. Set up a mapping template like in the screenshot
  3. Import a MARC record with a 949 like this:
949   ‡a Printed Material ‡b 1 ‡d Non-circulating ‡f spccopymonojjl ‡g Restricted ‡h PN6728.H4498 2015
  1. Open the item in inventory, look at the CallNumberType

Expected Results:

Not sure, but I guess one of the following:

  • No callnumber type set on the item at all, since there is no type like that in the reference data
  • The record fails to import
  • The item fails to get created, and this is clearly logged somewhere
  • mod-inventory-storage updates their schemas so the uuid fields are typed as uuids
  • mod-inventory-storage fails records not aligning with the schema
  • mod-oai-pmh handles these issues gracefully, withholding this item/instance and lists this in the request-status API so libraries can locate the missing record.

Actual Results:

  • Item gets created in inventory-storage with a callnumber string where there should be a callNumberType UUID
  • The bad item blocks the OAI-PMH feed from outputting any records (i am told).

Scope:

  • Change the itemLevelCallNumberTypeId to be constrained to a UUID
  • Add a database constraint between this property and the call number type table (via schema.json)
  • Change the major implementation (module) version
  • Change the major item-storage interface version (in the descriptor and RAML docs)
  • Check whether itemLevelCallNumberTypeId is referred to in any other interfaces. If they are, repeat the UUID constraint and major version changes for those interfaces
  • Create issues for each module dependent on these interfaces

Additional Information:

Interested parties:

Magda Zacharska I am not exactly sure how OAI-PMH behaved in this case but was told that the feed was blocked until the error was corrected. 

Charlotte Whitt looking at the schemas for item.json it seems like any string is allowed. However, this is not the case and I think both the schema as well as /item-storage/items endpoint should have more checks in place.

Ann-Marie Breaux  Does Data Import look at the JSON schemas before they send things into inventory storage? If so, this issue would hopefully go away if the UUID properties in the Inventory schemas were having more accurate type definitions. If not, I think it would be good if the created records were validated before being loaded into Inventory.



 Comments   
Comment by Magda Zacharska [ 06/Oct/22 ]

Marc Johnson what would be the best approach to resolve such issues when inventory storage allows for invalid data? Should each consumer of inventory storage endpoints implement data validation to avoid cases as the one described in this ticket or should it be handled by inventory storage?

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

I think inventory-storage should correct the schemas, enabling  two things:
1. Use these schemas internally for validating input

2. Letting integrating modules (external as well internal) to validate before posting.

Comment by Marc Johnson [ 07/Oct/22 ]

Magda Zacharska Theodor Tolstoy (One-Group.se)

Thank you for asking the question.

Summary

I think folks expect that the value in the call number type ID field will refer to call number type ID.

I reckon that is a universal expectation, and to achieve it likely requires two steps:

  • change the call number type ID field definition to only allow UUIDs (this will stop folks accidentally putting call numbers in this field)
  • change the internal validation to only allow IDs that refer to an existing call number type (this will reduce the chances of folks trying to assign an ID that cannot be resolved)

I've expanded upon my reasoning below.

General view

what would be the best approach to resolve such issues when inventory storage allows for invalid data?

If the desired constraint(s) are intended to be universal, then these should be applied in mod-inventory-storage.

Should each consumer of inventory storage endpoints implement data validation to avoid cases as the one described in this ticket or should it be handled by inventory storage?

In general, I tend to think that consumers SHOULD be defensive when interpreting data and fail gracefully when something unexpected happens. To what degree that is achieved and what form it takes will depend upon the situation.

For example, I think consumers SHOULD expect that when trying to find a reference record by ID, no records might be found.

Specific to this issue view

In order to answer this questions, I think I need to understand the issue better.

Did OAI-PMH fail because the itemLevelCallNumberTypeID was not in a specific format (a UUID) or because when it tried to find the call number type based upon that value it didn't find one?

If it failed due to the field having a value in an unexpected format, then this could be resolved, as Theodor Tolstoy (One-Group.se) suggests with changes the the schema to constrain that field to be a UUID.

That would mean that attempts to use a call number in that field would fail validation.

However, if the consumer failed because it tried to use the call number type ID to find a call number type and couldn't find one, then this won't be sufficient to reduce the chances of this happening again.

As call number types are part of mod-inventory-storage, we can also add a constraint that means that the value in the call number type ID field refers to a call number type.

I think this is what folks are actually expecting.

I don't think a consumer should assume that this check will always be the case, because it is implementation specific, and thus should guard against not finding a record when trying to find them by ID.

The way that FOLIO is designed, references are not guaranteed to be resolvable. Unfortunately, there are many areas of the system where consumers are not resilient to this.

Comment by Marc Johnson [ 07/Oct/22 ]

Theodor Tolstoy (One-Group.se)

Letting integrating modules (external as well internal) to validate before posting.

How might that work?

Are you suggesting FOLIO introduce a new standard for record validation APIs?

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

Are you suggesting FOLIO introduce a new standard for record validation APIs?

Not really. Our data migration tools are using the JSON schemas in order to pre-validate the data during transformation in order to fail early and be able to report issues as early as possible. If schemas could be exposed by the API:s themselves, other internal modules could do this as well if they would like. For our tools, we are getting the schemas from Github, which is a bit of a hassle  at times.

Comment by Marc Johnson [ 07/Oct/22 ]

Theodor Tolstoy (One-Group.se)

Not really. Our data migration tools are using the JSON schemas in order to pre-validate the data during transformation in order to fail early and be able to report issues as early as possible.

Thanks for helping me understand that better.

If schemas could be exposed by the API:s themselves, other internal modules could do this as well if they would like. For our tools, we are getting the schemas from Github, which is a bit of a hassle at times.

Yeah, I can understand a desire to get access to those from a running instance of the module.

Comment by Ann-Marie Breaux (Inactive) [ 10/Oct/22 ]

Hi Marc Johnson Would the first step in fixing this be an adjustment in MODINVSTOR? If so, can we assign to that project and Prokopovych? Then if additional stories are needed for other modules, we can add them.

Alternatively, we could keep this bug as is, and block it on a newly-created MODINVSTOR task/story to change the call number type validation to only allow UUIDs. And since this is a schema change, there would need to be either a script or a way of handling any existing records with invalid data in the field. Would this affect both holdings and item records?

Comment by Marc Johnson [ 11/Oct/22 ]

Ann-Marie Breaux

Would the first step in fixing this be an adjustment in MODINVSTOR? If so, can we assign to that project and Prokopovych?

Sure, the scope of that work depends upon the questions I asked above.

Then if additional stories are needed for other modules, we can add them.

This will be an interface breaking change, many additional issues will be needed.

Alternatively, we could keep this bug as is, and block it on a newly-created MODINVSTOR task/story to change the call number type validation to only allow UUIDs.

I think that's up to the Support SIG, as that depends upon their workflow.

And since this is a schema change, there would need to be either a script or a way of handling any existing records with invalid data in the field.

Yes, we would need something to remove the invalid data. We may want to do this both during the upgrade (to ensure that it happens) and provide a script (to allow folks to run it outside of upgrades if they want, to reduce upgrade time).

Would this affect both holdings and item records?

The currently described scope would only affect the item.

What might I have missed that mean the holdings records are involved?

Comment by Ann-Marie Breaux (Inactive) [ 11/Oct/22 ]

Thank you for the additional explanation, Marc Johnson

Let's wait and get feedback from Support SIG on how they would like to proceed cc: Anya

If the decision is to change the schema, I would recommend noting this as a known issue in MG, Nolana, and then postponing a fix until Orchid. That way, all the various modules/teams could coordinate their related changes, and we would have time to test it carefully.

Comment by Ann-Marie Breaux (Inactive) [ 21/Oct/22 ]

Anya Any additional feedback from the Support SIG after y'all have a chance to review the comments? Right now this is not being worked on, and is not in any dev team's backlog. Thank you!

Comment by Charlotte Whitt [ 27/Oct/22 ]

Ann-Marie Breaux - I moved this ticket out of SUP, and made it to be assigned as MODSOURCE. The Support ticket can review this further at our next meeting.

CC: Anya Debra Howell

Comment by Ann-Marie Breaux (Inactive) [ 27/Oct/22 ]

Thank you, Charlotte Whitt If we go down the path of making a change to the Call number type in the Item schema, then include stories that make corresponding adjustments to all modules that interact with call number type (which would include fixing this bug), I recommend that change happen in Orchid, and all the related stories/bugs be attached to the MODINVSTOR story for the Item schema change, since that is what will introduce the breaking change.

I have no problem with Folijet updating validation for call number type, but I want to be sure that's done after any Inventory schema changes.

Marc Johnson When I mentioned the holdings record in addition to the item record - if we change call number type in the Item record to require UUID/validation, then it seems to me like we should make the same change for the call number type in the Holdings record. The Holdings and Item call number type and all call number fields (which can be set at the Holdings and/or the Item level) should behave the same way in both types of records.

Comment by Marc Johnson [ 28/Oct/22 ]

Ann-Marie Breaux

When I mentioned the holdings record in addition to the item record - if we change call number type in the Item record to require UUID/validation, then it seems to me like we should make the same change for the call number type in the Holdings record. The Holdings and Item call number type and all call number fields (which can be set at the Holdings and/or the Item level) should behave the same way in both types of records.

I may have lost the thread here, are you concurring with something I said above or asking for my advice / opinion?

The call number type ID for the holdings record is already constrained to be a UUID.

Is there further validation folks are intending to be applied to these fields?

Comment by Anya [ 31/Oct/22 ]

Support: Marc Johnson and Ann-Marie Breaux  what information would you like from the support sig - most libraries need OAI in order for their discovery systems to work. What is a workaround for this? 

 

What would be the release target date - could this be part of the bug-fest fix?

Comment by Marc Johnson [ 01/Nov/22 ]

Anya

what information would you like from the support sig - most libraries need OAI in order for their discovery systems to work.

I don't think I know how to answer that question. I gave my thoughts on what I think folks want in the above comment. Folks need to decide whether that is the course of action they want to take.

What is a workaround for this?

The only workaround I am aware of is to remove the incorrect data from those fields. Though I think Magda Zacharska might have commissioned some further investigative work into this issue.

What would be the release target date - could this be part of the bug-fest fix?

Depending on the direction folks choose, I believe these changes would be considered compatibility breaking. I wouldn't recommend them being included in a bug fix release.

Comment by Charlotte Whitt [ 01/Nov/22 ]

Ann-Marie Breaux - as Marc Johnson wrote

The call number type ID for the holdings record (https://github.com/folio-org/mod-inventory-storage/blob/master/ramls/holdingsrecord.json) is already constrained to be a UUID

and similar is implemented for the item record (https://github.com/folio-org/mod-inventory-storage/blob/master/ramls/item.json).

So in your comment from 4 days ago, I think there is a misunderstanding of the scope of the work.

What we need is a validation of the Data Import file, and a mechanism to prevent generation of a unspecified call number type, which is not present and defined in Settings > Inventory > Holdings and Item > Call number type.

Do you think these requirements would be relevant to discuss also with the MM-SIG?

CC: Felix Hemme Raegan Wiechert lew235

Comment by Marc Johnson [ 01/Nov/22 ]

Charlotte Whitt Ann-Marie Breaux

The call number type ID for the holdings record (https://github.com/folio-org/mod-inventory-storage/blob/master/ramls/holdingsrecord.json) is already constrained to be a UUID

I'd like to make explicit my intent with this comment. I was trying to check my understanding of what Ann-Marie Breaux said. It was limited to the holdings record.

As I understand it, itemLevelCallNumberTypeId on the item record, is not currently constrained and is the primary aspect of the scope thus far.

If I've misunderstood, please correct me.

Comment by Charlotte Whitt [ 01/Nov/22 ]

Marc Johnson - I referred to the GitHub documentation https://github.com/folio-org/mod-inventory-storage/blob/master/ramls/item.json - and here I read the spec:

(...)

},
        "typeId": {
          "type": "string",
          "description": "Effective Call Number Type Id is the call number type id of the item, if available, otherwise that of the holding.",
          "$ref": "uuid.json",
          "readonly": true
        }

I would understand that as the Call number type ID being constrained - but I'm not the developer here

Comment by Marc Johnson [ 02/Nov/22 ]

Charlotte Whitt

I referred to the GitHub documentation. .... I would understand that as the Call number type ID being constrained

There is every chance I'm taking the issue description too literally. The use of itemLevelCallNumberTypeID suggests to me that folks are referring to this property, which is currently unconstrained, and not the properties of the effective call number, which I believe was added after the majority of the OAI-PMH work.

Comment by Anya [ 07/Nov/22 ]

Support: We are looking for a release target - tagging Ann-Marie Breaux  cause this is assigned to Folijet. 

Comment by Ann-Marie Breaux (Inactive) [ 08/Nov/22 ]

Hi Marc Johnson and Charlotte Whitt I think this should be a MODINVSTOR issue, not MODSOURCE. MODSOURCE has no understanding or storage of Call Number Types.

I have attached a video HoldingsItemCallNumberTypes.mp4 that illustrates what happens in Data Import when the same invalid field mapping is used for creating a Holdings and an Item. Data Import gets different responses from Inventory. The holdings is blocked from creation because DI sends invalid data for Call number type. The item is created, even though DI sent the same invalid data for Call number type. I think both should fail with parallel error messages. From the conversation above, it sounds like the way to resolve that is to have Inventory validate/constrain the Item Call number type field in the same way that the Holdings Call number type field is.

That seems like the starting point to me. Inventory should not allow data supplied by Data Import to create/update an item to have unexpected data in the Item call number type field, just like it disallows that for data supplied to create/update a holdings. Inventory should only allow the Call number type values that are shown in Settings, which are represented behind-the-scenes as UUIDs. From the earlier comments, it sounds like that logic is in place for that same field in Inventory Holdings records, but not Inventory Item records.

Does that seem right to you, Magda Zacharska? Would that resolve the OAI-PMH issues?

If so, then Prokopovych would need to make that (breaking) change to the Item schema first, and then other teams would need to react to that schema change. For Data Import, I don't think we would actually need to change anything, since (hopefully) Inventory would begin failing the item creation and passing the same error as is passed for holdings creation.

I think it would also be helpful if Prokopovich could create a script that identifies item records with invalid Classification types, so that libraries could review and correct them.

Since we're late in the Nolana release cycle, my view is that it would be best for this to happen in Orchid and be logged as a known issue for MG and Nolana. Then Prok does their work ASAP, then publicizes to other teams whose apps interact with item records and may be affected.

All that said, I'm moving this to Prokopvych and MODINVSTOR. This is not a Data Import problem.

Khalilah Gambrell any additional thoughts? Comments?

Comment by Charlotte Whitt [ 08/Nov/22 ]

We need to be careful here; because what you suggest Ann-Marie Breaux is, that validation of data coming from Data Import is to be handled by Inventory.

Wouldn't it be more correct that Data Import did this validation before pushing the data to Inventory?

CC: Felix Hemme Raegan Wiechert Marc Johnson Taras Spashchenko

Comment by Theodor Tolstoy (One-Group.se) [ 12/Nov/22 ]

I just want to add that there are two levels of validation here that could happen:

First level: The property type/constraint - just as in most areas in FOLIO should be a UUID.  This basic type check should absolutely be done by Inventory IMO. 

Second level: The content of the property should be checked against a list of valid, existing, call number types. This list is typically very short and could (physically at least) fit in the memory of mod-inventory-storage. 

 

Having Data Import validating the data before posting will give data import a chance to better communicate and handle errors before they happen.

If Inventory-storage does not validate, then every other module/piece of code that integrates with it need to check for this, and risk creating a lot of dirty data that in turn leads to issues for the consumers like reporting and OAI-PMH. 

When I look at https://github.com/folio-org/mod-inventory-storage/blob/master/ramls/holdingsrecord.json,  as comparison, that schema has UUID as the type for all the relevant properties. I personally do not see the reason for this inconsistency in how the two schemas were implemented.

Compare 

with

Comment by Ann-Marie Breaux (Inactive) [ 14/Nov/22 ]

What Theodor Tolstoy (One-Group.se) said. I still feel like the first step is for Inventory to have the Item call number type field set up and behaving in the same way as the Holdings call number type. Once that's in place (with whatever cleanup script or work is required due to the Item schema change), then other apps can react as necessary. If other apps are not sending UUIDs, they'll either get error messages back from Inventory for creates/updates that contain text for the Item call number type field, or will fix what they are trying to send to Inventory so that problematic data is caught before that.

Comment by Marc Johnson [ 14/Nov/22 ]

Theodor Tolstoy (One-Group.se) Thank you for your comment

I just want to add that there are two levels of validation here that could happen:

First level: The property type/constraint - just as in most areas in FOLIO should be a UUID. This basic type check should absolutely be done by Inventory IMO.

Second level: The content of the property should be checked against a list of valid, existing, call number types.

I think this lines up with the approach I suggested in this comment

This list is typically very short and could (physically at least) fit in the memory of mod-inventory-storage.

mod-inventory-storage is a RMB based module, those typically use database constraints for this validation.

When I look at https://github.com/folio-org/mod-inventory-storage/blob/master/ramls/holdingsrecord.json, as comparison, that schema has UUID as the type for all the relevant properties. I personally do not see the reason for this inconsistency in how the two schemas were implemented.

This is broadly caused by a combination of three aspects: originally FOLIO did not constrain these fields in this way, that was only agreed after much of this was implemented; variation between development teams (it's likely different teams worked on these areas) and retrofitting this kind of validation is a compatibility breaking change (and thus is expensive to do).

Comment by Marc Johnson [ 14/Nov/22 ]

Ann-Marie Breaux

I still feel like the first step is for Inventory to have the Item call number type field set up and behaving in the same way as the Holdings call number type.

If this is decided, then we should start this work as soon as possible. A similarly small change was made to mod-users took most of a release cycle to complete all of the compensating work, and that had less impact than this.

Once that's in place (with whatever cleanup script or work is required due to the Item schema change)

We likely won't be able to automatically compensate. More likely, a script will be provided to identify the records using invalid references. Those will then either need removing or changing.

then other apps can react as necessary. If other apps are not sending UUIDs, they'll either get error messages back from Inventory for creates/updates that contain text for the Item call number type field, or will fix what they are trying to send to Inventory so that problematic data is caught before that.

I think it could be prudent to start some of the compensating work prior to the completion of the mod-inventory-storage changes, in order to give them the best chance of being included.

As these are compatibility breaking changes, we won't be able to get them into the hosted reference environments until all known consumers (at least 15 modules, likely more) have been updated. Some analysis will need to be done to figure out the full impact.

This might hold up other changes in mod-inventory-storage depending upon when they are made.

Comment by Charlotte Whitt [ 14/Nov/22 ]

Support SIG: Ann-Marie Breaux what would be the release target for this work?

Comment by Ann-Marie Breaux (Inactive) [ 14/Nov/22 ]

Hi Charlotte Whitt I don't think there are related changes that Data Import will need to make, aside from testing. This Inventory change should definitely get socialized to all the dev teams/apps that interact with item records, so that they can plan any related work that would be needed. A good way to do that is to create a wiki page explaining the changes, and then post about it in the Dev and PO Slack channels so that any dev teams/apps that need to review can do so.

I would recommend aiming to get the Inventory work done ASAP in Orchid, so that

1) the script to identify/changing existing problematic items can be tested

2) other teams can test, add stories, and do their related work in Orchid as well 

Otherwise, we may need to leave it as a known issue for Orchid, and plan the work in Poppy

Copying Khalilah Gambrell and Magda Zacharska in case they have any additional thoughts

Comment by Ann-Marie Breaux (Inactive) [ 14/Nov/22 ]

And Marc Johnson If the identification/cleanup script ends up being a one-time, on-demand one that needs to be run by each institution (or their hosting provider), please feel free to add it to the list here: https://folio-org.atlassian.net/wiki/display/FOLIOtips/Scripts+for+Inventory%2C+Source+Record+Storage%2C+and+Data+Import+Cleanup, with additional technical details on a linked page. Plus it would be important to include in the Orchid (or Poppy) release notes

Comment by Charlotte Whitt [ 14/Nov/22 ]

Marc Johnson - re.:

The first step is for Inventory to have the Item call number type field set up and behaving in the same way as the Holdings call number type.

> If this is decided, then we should start this work as soon as possible. A similarly small change was made to mod-users took most of a release cycle to complete all of the compensating work, and that had less impact than this.

So is this the ticket for doing that work, or do we want another one added to Jira. If the later, then maybe you are best suited person to write up that MODINVSTOR-xxx ticket with release target Orchid – if I got the right picture based on the long list of comments.

Thanks a lot for help describing and focusing on the solution on this

Comment by Ann-Marie Breaux (Inactive) [ 14/Nov/22 ]

Charlotte Whitt Marc Johnson My vote - turn this ticket into the needed work for MODINVSTOR. Rename it and update the description if required. Maybe even turn this into a small Inventory feature. Then any other necessary related tickets can be linked to it. Just don't lose all this history.

If you prefer to start fresh and create a new ticket, then please close this one and link the new clean one to this one.

Comment by Theodor Tolstoy (One-Group.se) [ 14/Nov/22 ]

Marc Johnson 

If this is decided, then we should start this work as soon as possible. A similarly small change was made to mod-users took most of a release cycle to complete all of the compensating work, and that had less impact than this.

What is the impact you are referring to? How do you know what kind of impact this would have?

Comment by Marc Johnson [ 15/Nov/22 ]

Theodor Tolstoy (One-Group.se)  thank you for your question

What is the impact you are referring to? How do you know what kind of impact this would have?

By impact, I mean the amount of compensating work that is needed in other parts of the system.

How do you know what kind of impact this would have?

I don't know.

I can say that at a minimum, there needs to be coordinated changes to at least 30 modules, to say they support the new versions of item-storage and inventory

Beyond that folks will need to do further analysis of what works needs to be done within each module. That does not include other integrations outside of centralised control.

The field is also likely used in other interface definitions because it is part of the item record, thus this list will likely get bigger once more analysis is done. And there might be a further knock on effect on interfaces provided by other modules.

If modules are using the property as intended, then the additional compensating work will likely be small. However, if that was the case, there likely wouldn't be the urgency for this change that there is (as it implies that some parts of the system are using them in an unintended way).

Based upon the conversation in the release group's meeting yesterday, it seems folks want the POs to be the coordinators of these changes and to discuss how this work can be planned.

Hkaplanian Khalilah Gambrell Did I recall that conversation reasonably?

Theodor Tolstoy (One-Group.se) Does this make sense and explain why I think these kinds of changes are pretty expensive / have a high impact area?

Appendix

Item-Storage Dependencies

ui-checkin
ui-checkout
ui-courses
ui-inventory
ui-inventory-es (might be able to be ignored)
ui-plugin-create-inventory-records
mod-circulation
mod-circulation-storage
mod-courses
mod-data-export
mod-feesfines
mod-inventory
mod-inventory-update
mod-orders
mod-remote-storage

Inventory Dependencies

ui-checkin
ui-checkout
ui-inventory
ui-inventory-es (might be able to be ignored)
ui-items
ui-orders
ui-plugin-create-inventory-records
ui-plugin-create-item
ui-plugin-find-po-line
ui-quick-marc
ui-receiving
ui-requests
mod-audit
mod-feesfines
mod-inn-reach
mod-ncip
mod-orders
mod-patron
mod-remote-storage
mod-rtac
mod-source-record-manager

Comment by Marc Johnson [ 15/Nov/22 ]

Charlotte Whitt 

 bq. So is this the ticket for doing that work, or do we want another one added to Jira. If the later, then maybe you are best suited person to write up that MODINVSTOR-xxx ticket with release target Orchid – if I got the right picture based on the long list of comments.

I would personally, use this issue for the main breaking change to item-storage and adding the constraint that the field value must refer to a known call number type.

I'll happily change the issue description if you and Theodor Tolstoy (One-Group.se) want me to.

After that, we'll need issues for:

  • Script to identify the records that would be non-compliant
  • Analysis of further interface impacts (to figure out how many other areas might be affected)
  • Making a compensating breaking change to the inventory interface
  • 30+ issues for each of the modules affected by these changes (even if only minimally, in terms of interface support)
Comment by Charlotte Whitt [ 15/Nov/22 ]

Marc Johnson - thank you Marc Johnson - That sounds great, if you would change the description of the work in this ticket, for the Prokopovych team to work on.

Marc Johnson Theodor Tolstoy (One-Group.se) Ann-Marie Breaux - now we need to line up who is then responsible for:

  • Script to identify the records that would be non-compliant
  • Analysis of further interface impacts (to figure out how many other areas might be affected)
  • Making a compensating breaking change to the inventory interface
  • 30+ issues for each of the modules affected by these changes (even if only minimally, in terms of interface support)
Comment by Marc Johnson [ 15/Nov/22 ]

Charlotte Whitt

now we need to line up who is then responsible for:

  • Script to identify the records that would be non-compliant
  • Analysis of further interface impacts (to figure out how many other areas might be affected)
  • Making a compensating breaking change to the inventory interface

I would expect these to be the responsibility of the Prokopovych team (assuming that is the team doing this work, they are the responsible team. The original work predates the team).

Comment by Charlotte Whitt [ 15/Nov/22 ]

Okay , so are you good to go, or do you need more input from me? Marc Johnson

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

Charlotte Whitt Marc Johnson:

Stating the obvious here, but we are going to fix the other properties as well, right? The ones that for some reason are typed as plain strings, when FOLIO expects them to contain UUIDs from the right reference data sets, right? 

  • item.holdingsRecordId
  • item.itemLevelCallNumberTypeId
  • item.materialTypeId
  • item.permanentLoanTypeId
  • item.temporaryLoanTypeId
  • item.permanentLocationId
  • item.temporaryLocationId
  • item.electronicAccess.relationshipId
  • item.notes.itemNoteTypeId
  • item.statisticalCodeIds
  •  

Properties I am not that sure of:

  • item.inTransitDestinationServicePointId

 

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

I'll happily change the issue description if you and Theodor Tolstoy (One-Group.se) want me to.

Go ahead!

Comment by Marc Johnson [ 15/Nov/22 ]

Theodor Tolstoy (One-Group.se)

The ones that for some reason are typed as plain strings, when FOLIO expects them to contain UUIDs from the right reference data sets, right?

Yes, mostly likely. Some of those likely already have the known reference constraint in place e.g. for holdingsRecordId

Stating the obvious here, but we are going to fix the other properties as well, right?

Are you asking within the scope of this work or generally?

I'm going to assume you meant the former when providing an answer.

Not at the moment, that hasn't been defined as part of the scope of this work.

We could include all (or some) of these in the scope of this issue / breaking change.

That increases the leverage of the basic work required for a breaking change. And thus reduces the effort over doing each one at once.

A trade off with that is it that the analysis and impact aspects of this work would need to be done for each field. That would likely significantly extend the amount of work each responsible team needs to do.

Folks would need to think about how willing they would be to do that within one release cycle and how likely that could be done for Orchid. The more we add, the less likely this can all be fit into Orchid. And that isn't something we can also back out of later in the release cycle.

Comment by Ann-Marie Breaux (Inactive) [ 16/Nov/22 ]

Maybe it's worth it to quickly review the other fields to see if they actually need changes, but could we fix this one, and not get too bogged down in every field of the item and holdings records?

We know this will be a helpful change, and (I thought) we came to an agreement. But I guess it's up to Prokopovych how much analysis and effort they want to put into this issue.

If this Jira gets broadened, I would just ask that the original description be retained at the bottom of the description field, or copied into a comment, so that we retain that history.  

Comment by Marc Johnson [ 16/Nov/22 ]

Ann-Marie Breaux Theodor Tolstoy (One-Group.se) Charlotte Whitt Khalilah Gambrell

I guess it's up to Prokopovych how much analysis and effort they want to put into this issue.

Given that folks have talked about the responsibility for impact in dependent modules being that of each owning team, much of the analysis burden falls to those teams.

The analysis the Prokopovych team would do is to determine which other interfaces are affected by the change (and thus, how much wider the potential impact is).

I think it's up to the POs to decide how much of this they want to bite off in one go.

Fewer batches of changes mean less basic overhead for the compatibility breaking changes at the trade off of needing to compensate for more changes in the same development cycle.

We know this will be a helpful change, and (I thought) we came to an agreement.

If this Jira gets broadened, I would just ask that the original description be retained at the bottom of the description field, or copied into a comment, so that we retain that history.

I'll hold off changing this JIRA until folks agree on what the scope of the change is.

Comment by Marc Johnson [ 21/Nov/22 ]

Theodor Tolstoy (One-Group.se) Ann-Marie Breaux Charlotte Whitt Magda Zacharska

What is the current status of this work, are folks expecting it to progress as only the item level call number type ID or further analysis over all / some of the other fields?

Comment by Charlotte Whitt [ 21/Nov/22 ]

In order for us to move forward on this, Marc Johnson, I suggest that we solve the immediately problem with the item level call number type ID.

And then we can create a task for identifying other fields with similar problem. Who would you suggest do that analysis?

Comment by Marc Johnson [ 22/Nov/22 ]

Charlotte Whitt

I suggest that we solve the immediately problem with the item level call number type ID.

Ok, unless anyone else responds, I'm going to consider that a product priority decision and will proceed with amending this issue as agreed above.

And then we can create a task for identifying other fields with similar problem. Who would you suggest do that analysis?

Depending upon how much analysis folks want to do, it may involve all of the teams involved in a breaking change to items (a rough indication of the scope of this can be found above).

Comment by Marc Johnson [ 22/Nov/22 ]

Charlotte Whitt Khalilah Gambrell Ann-Marie Breaux

I've amended this issue to include development scope instructions.

And I've completed the other tasks I believe are needed to move this work forward:

  • Script to identify the records that would be non-compliant MODINVSTOR-995 Closed
  • Analysis of further interface impacts (to figure out how many other areas might be affected) - I've included this
  • Make a compensating breaking change to the inventory interface MODINV-757 Blocked

That leaves the work for the POs to decide on how / when to schedule this and the 30+ issues for each of the modules affected (many are listed above) by these changes.

Comment by Anya [ 28/Nov/22 ]

Support: setting this to orchid. 

Comment by Marc Johnson [ 13/Feb/23 ]

Anya

Support: setting this to orchid.

This is a compatibility breaking change, it is too late in the Orchid release cycle to make this change.

As the Prokopovych team is being disbanded at the end of Orchid, I have set the release to unscheduled (as to not presume when the responsible team will pick it up).

I have left the team in place because folks get concerned when there is no development team specified.

cc: Khalilah Gambrell Charlotte Whitt

Comment by Anya [ 13/Feb/23 ]

Support : Thanks Marc Johnson 

Comment by Charlotte Whitt [ 27/Mar/23 ]

Support SIG: Khalilah Gambrell do we know by now, what development team will take over the work on mod-inventory-storage?

Comment by Joshua Barton [ 25/Apr/23 ]

We have another case of being affected by this issue. OAI feed was down entirely. Cause wa the same as original in ticket: bad value in Call Number Type upon creation of an item using Data Import field mapping that takes Call Number Type from a 9XX subfield. Cataloger accidentally input "Other scheme4" instead of "Other scheme" (stray typo even though we're using templates).

Comment by Anya [ 27/Apr/23 ]

Charlotte Whitt /Khalilah Gambrell  bringing Joshua Barton comment to your attention 

Comment by Khalilah Gambrell [ 28/Apr/23 ]

Hey Anya and Joshua Barton, team is reviewing the issue. Will provide an update as soon as possible.

Comment by Khalilah Gambrell [ 04/May/23 ]

Decision

  • Apply validation rule(s) in mod-inventory-storage
    • Need to determine if rules should be applied for both holdings call number type AND item call number type. Assume that if no item call number set then OAI-PMH populates with holdings call number. Is that correct, Magda Zacharska?
  • Implementation: If user imports record via data import then the item (and possibly holdings depending on Magda's response) record will fail to create and error will state failed due to invalid call number type value.
  • This is a breaking change and is most likely too late to implement for Poppy release but we will implement for "Q" release.
  • Need to address records with invalid values already stored (separate story to create)
    • Just clean up existing invalid call number types. Create a migration script and for module upgrade set invalid call number types to Empty value.
  • Need to discuss with Folijet to add validation on instance/bib/holdings/items mapping profile.
Comment by Julian Ladisch [ 05/May/23 ]

Please use "NOT VALID" option when adding the foreign key constraint so that the migration will complete fast.

https://www.postgresql.org/docs/current/sql-altertable.html#SQL-ALTERTABLE-NOTES :

Scanning a large table to verify a new foreign key or check constraint can take a long time, and other updates to the table are locked out until the ALTER TABLE ADD CONSTRAINT command is committed. The main purpose of the NOT VALID constraint option is to reduce the impact of adding a constraint on concurrent updates. With NOT VALID, the ADD CONSTRAINT command does not scan the table and can be committed immediately. After that, a VALIDATE CONSTRAINT command can be issued to verify that existing rows satisfy the constraint. The validation step does not need to lock out concurrent updates, since it knows that other transactions will be enforcing the constraint for rows that they insert or update; only pre-existing rows need to be checked. Hence, validation acquires only a SHARE UPDATE EXCLUSIVE lock on the table being altered. (If the constraint is a foreign key then a ROW SHARE lock is also required on the table referenced by the constraint.) In addition to improving concurrency, it can be useful to use NOT VALID and VALIDATE CONSTRAINT in cases where the table is known to contain pre-existing violations. Once the constraint is in place, no new violations can be inserted, and the existing problems can be corrected at leisure until VALIDATE CONSTRAINT finally succeeds.

Comment by Julian Ladisch [ 27/May/23 ]

GBV is not affected by this issue because it uses https://github.com/folio-org/mod-inventory-update . I remove "!!!ALL!!!" from "Affected institutions" field because it is wrong.

Comment by Charlotte Whitt [ 31/Jul/23 ]

Support SIG: We note this is set to Quesnelia R1 2024, and the status of the ticket is Draft. We acknowledge this as we are still in mid 2023.

Comment by Thomas Trutt [ 02/Oct/23 ]

FROM SUPPORt: Pavlo Smahin  please see above. 

Comment by Khalilah Gambrell [ 02/Oct/23 ]

Thomas Trutt  questions regarding priority and releases should go to the PO not developers. 

Comment by Charlotte Whitt [ 20/Nov/23 ]

Support SIG: Hi Khalilah Gambrell - this ticket looks like it is well described. Can we move it out of status Draft and to be Open, and it can be scheduled when to do the fix?

Comment by Khalilah Gambrell [ 20/Nov/23 ]

Hey Charlotte Whitt  - there is more additional discussion needed for this issue before we can proceed. 

Comment by Debra Howell [ 05/Feb/24 ]

From Support SIG: Khalilah Gambrell Are there any updates for this bug? Is it ready to be moved to Open?

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