-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Confirm no boundwith breaking changes with Poppy #629
Comments
Screenshots don't display for some reason. 😢 Is it unintentional for the principle to display in the list of boundwiths? A bug in the UI? It seems like maybe the new endpoint gathers all the items on a holdings records via the bound with table and doesn't remove the one you looking at. How would it know? It's kind of like when you click on a subject heading in SearchWorks and the search result is the record from which you clicked the link. This is nice though, that the parent item now displays in the holdings of the child record but I see how it might confuse users immensely. I see they added a |
The bound-with section on the item record view is generated using data from the item record. There is a field,
|
Doing a |
I chatted with Charlotte this morning and showed her our boundwiths in Poppy test. She was unsure what might have gone wrong but confirms that there is no upgrade breaking changes or upgrade processes that we might have missed. She suggests we try to identify the bound-with records and edit/save them to trigger the UI display of the principle. She also shared the slide deck below and offered additional assistance if we need it. It is still not clear to me the mechanism that creates the UI display of the principle but based on the slides it is triggered by the save action. And then if all other relationships are deleted, the principle is automatically "cleaned up" e.g. the user can not delete/trashcan icon is grayed out. https://docs.google.com/presentation/d/1WRo8z_T63h5o2vqVBhfvTArubemBNNnUrayqtoypjvA/edit#slide=id.g29621cdef64_0_6 |
Just confirming that indeed, our version of mod-inventory (19.0.2) in production has this code that gets the bound with parts using the storage endpoint, to update the item record of the parent. https://github.com/folio-org/mod-inventory/blob/09c82d96006ce29effc0bf0aedef8a0babbfe167/src/main/java/org/folio/inventory/resources/Items.java#L874-L916 Related Jira ticket: https://folio-org.atlassian.net/browse/MODINV-443 So we need to figure out a way to update all our bound with parent item records... Or just wait until a user updates them 🤷♂️ I'll see if I can figure out how many bw parent items we have. |
This query will count the distinct itemId values in the bound_with_part table, here is from prod:
|
Did some more testing - editing the related instance doesn't work and checking in the item doesn't work even though it updates the Item Record last updated date. |
Whatever works. Should I get the item Id's for bw-parents from stage for her to test? Can bulk edit handle that many or will it need to be broken up? I guess we'll find out. |
I searched for "BWcreatedby" on folio-test because this is the note our bulk process adds. It doesn't look like the addition of that note has the desired effect on Poppy UI - see here https://folio-test.stanford.edu/inventory/view/1c1a4b4c-8de7-57f0-82d0-4c3ce04540da/466e561d-3d86-5701-9b1f-7d00f12855b4/c22376b9-4ab8-543c-8540-c93401269396?qindex=holdingsAdministrativeNotes&query=BWcreatedby&segment=holdings&sort=title |
I'll check in with Jeanette re: bulk edit |
I'm almost certain that the reason is because we are using the item-storage endpoint to update the item: https://github.com/sul-dlss/libsys-airflow/blob/f8f5ffd3cc36e5ef1419f623263a56795c9a27a9/libsys_airflow/plugins/folio/helpers/bw.py#L99-L102 |
Maybe I try testing changing the endpoint to use https://s3.amazonaws.com/foliodocs/api/mod-inventory/p/inventory.html#inventory_items__itemid__put locally and see what happens. |
Hmm, I was about to make a ticket in libsys-airflow but did some testing first. In folio-stage, I wanted to create the bound with relationship between the records Alissa linked me to for an example in folio-test.
View item record: |
I did a GET and PUT to inventory/items and added an admin note, the boundWithTitles is still populated but only with the ID of the boundwith child is in the list. I did another GET and PUT to item-storage/items and added another admin note and the boundWithTitles list is still populated, only with the bound with child. I had expected maybe the bw parent would show up with the GET and PUT using the inventory/items endpoint but it does not. So alas, our libsys-airflow process seems okay. Maybe it IS a later mod-inventory version (not Nolana version) that adds the bw parent to the list of boundWithTitles on the item record. |
Just testing in folio-test.
So now I wonder what the UI is doing to create the bw relationships. Just had to doublecheck the json, the item record
|
Testing in folio-test to see what the UI does to create bw relationships.
There is also a PUT to
The next network call is a GET to
|
Testing using the new endpoint PUT
|
I tried doing a PUT to re-establish the bw relationship:
I got a 204 response but when I check the item record in folio, it still doesn't have the other bw child. UPDATE: this happened b/c I used the wrong holdingsRecordId. Using the correct one
and the item record of the bw parent now shows the 2 bw children + itself in boundWithTitles list. |
Trying to see how the UI allows creating boundwiths with many children and one parent.
|
So, I think this is all to say, that we should update our airflow bw creation to use the new endpoint for Poppy. But that means when establishing new relationships, we need to send in the existing ones too to the PUT or else we will inadvertently unbind things that are bound with and then they'll be lost forever!!!! AAHHHhh!!! 🤯 |
While driving my kid home from daycare, I had the brilliant idea that maybe our assumption of the
But alas, the parent is a part of the bound with item, ergo it should get an entry as well (or so I assume is the thinking). Indeed, the item record example in this ticket has these entries in
The last one is the holdingsRecordId for the bw parent item
And now the item record shows the parent and child in the boundWithTitles array, see https://folio-stage.stanford.edu/inventory/view/1c1a4b4c-8de7-57f0-82d0-4c3ce04540da/466e561d-3d86-5701-9b1f-7d00f12855b4/c22376b9-4ab8-543c-8540-c93401269396. I agree that now, it is super confusing to figure out which is the parent and which is the child. But it probably doesn't matter after all. 🤷♂️ But I guess this makes it better and easier for us to make the bound with relationships in batch; we just need to POST additional data to the endpoint we are currently using and we don't have to futz around with grabbing all the bound with parts when using the other endpoint that the UI uses. |
I started writing a sql script to get the bw parent item and holdings record UUID's in a file to be used to create the bw parts data for these, if we want to go this route (we probably should even though it looks confusing). It is currently on
I only ran it agains the first 1,000 item record ID's. It takes about 30s to run for 1,000 ID's. |
I asked Charlotte if we should go the above route of POSTing the bw "parent" holdingsRecordId and itemId to the bound-with parts table. Her response is summarized here - No because original one is there already - that is why it gets to display correct when you 'touch' the item record. She suggests the original solution of editing the item records (e.g. adding an admin note). But, she offered to set up a meeting with the developer if we want to investigate further - it wouldn't be till end of April because of Q release work happening right now. @shelleydoljack I'm curious if you tried the above solution of test/poppy? In the stage example you include the "parent" doesn't display in the UI as I would expect, e.g. in Poppy they are always listed first. |
Her assumption that the "original one is there already" is false; we don't have the "parent's" holdingsRecordId and itemId in that bound-with-parts table because we created that data using the storage endpoint and not the new one available in Poppy, so it only contains the holdingsRecordId of the bw children. To me, this means that we should add that missing data to the table to fix the problem, since she thinks it is already there. My testing showed that adding an admin note didn't resolve the problem (I think, I re-read my comments and am certain that my adding the note didn't do anything; I'll try again to be certain). |
I tested adding the admin note in Nolana and it does not resolve the issue. It does resolve the issue in Poppy. I'm concerned that if we added the relationship in Nolana then something not understood happens in Poppy where the relationships "are already there". |
I used bulk edit to add a note to the bw parent item record and the process did not add the bw parent to the boundWithTitles array in the item record. See https://folio-test.stanford.edu/inventory/view/11bce934-49ff-56d4-b58b-62028e8cc8af/0e6b496a-5cb0-501c-850c-1e2429bc864d/269857c4-2543-5da5-adf9-173e35f1326c and the item data:
|
That is strange because when I add an admin note in the UI, the bw parent appears in the UI and (presumably in the boundWithTitles array?) Here is a migrated record that I have not edited - 36105039040733. Shelley, could you see what is in the bw table for this record? |
Great, so bulk edit and editing in the UI probably use different endpoints or maybe the logic that does the filling out of boudWithTitles array is not triggered with bulk edit somehow. 🤷 How annoying. Here is the data in bound-with-parts for barcode 36105039040733:
|
And after I added an admin note to 36105039040733
|
Charlotte is going to investigate a bit with Corrie and Charles ahead of scheduling a meeting. So I've provided some example records for them here. |
Charlotte's response
Shelley confirmed we don't need a meeting with Charles and Charlotte followed up with
@shelleydoljack do you want a separate ticket for this work? |
Yes, separate ticket. I'm guessing we want to update in test and stage before prod, yes? |
I think that depends on whether you want to update production data on Nolana or Poppy - do you have a preference? |
new ticket for work to update relationships #folio-tasks/304 |
Closing for now since we determined that our data is wrong. |
There are UI changes to boundwiths in Poppy and we want to confirm that nothing has changed in the underlying structure of the boundwith relationships.
We observed two changes in the UI. The first seems fine and I've determined how it is functioning. The second is the one that prompted concerns about unknown changes.
Inventory UI now displays items related to holdings even if the item is not directly related, e.g. via BW relationships - child holdings now “appear” to have items in the UI. See prod vs test. UI is using a new API /inventory/items-by-holdings-id
When a new boundwith relationship is created OR an item record with an existing bw relationship is edited, the "principle" now displays in the in the list of boundwiths. See screenshots below.
Item record for Geomagnetism applications in production
![Screenshot 2024-03-21 at 7.44.47 AM.png](https://camo.githubusercontent.com/704778b75f35cbb9b4d15520400809a27c6566f8c6b6fc98f7e9a16ba53264a7/68747470733a2f2f6170692e7a656e6875622e636f6d2f617474616368656446696c65732f65794a66636d467062484d694f6e73696257567a6332466e5a534936496b4a42614842424d5338355158633950534973496d563463434936626e56736243776963485679496a6f69596d7876596c39705a434a3966513d3d2d2d663937376638326362646435393532303865643233356666356333623137363833323032386634362f53637265656e73686f74253230323032342d30332d32312532306174253230372e34342e3437254532253830254146414d2e706e67)
Item record for Geomagnetism applications in production on test after editing the item record
![Screenshot 2024-03-21 at 7.46.24 AM.png](https://camo.githubusercontent.com/d95582b31d0dd1471e277e03d931054e2ef888e79112d2b538fd5858b7e677dd/68747470733a2f2f6170692e7a656e6875622e636f6d2f617474616368656446696c65732f65794a66636d467062484d694f6e73696257567a6332466e5a534936496b4a42614842424d6c41355158633950534973496d563463434936626e56736243776963485679496a6f69596d7876596c39705a434a3966513d3d2d2d623637646339636364343365616364623363383065366132383866663062353463376439623431322f53637265656e73686f74253230323032342d30332d32312532306174253230372e34362e3234254532253830254146414d2e706e67)
The text was updated successfully, but these errors were encountered: