Skip to content
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

Explore disabling InventoryItem #4333

Closed
1 task
awwaiid opened this issue May 5, 2024 · 9 comments · Fixed by #4607
Closed
1 task

Explore disabling InventoryItem #4333

awwaiid opened this issue May 5, 2024 · 9 comments · Fixed by #4607
Assignees
Labels
core team Groomed but likely needs expert knowledge stale

Comments

@awwaiid
Copy link
Collaborator

awwaiid commented May 5, 2024

Summary

When the Event system is fully adopted we'll want to remove InventoryItems altogether. As a step in that direction, for this ticket explore turning InventoryItem writes into a NOP or experimenting with removing it altogether. Then run the tests / interactive in Event mode and see what breaks!

Things to consider

No response

Criteria for Completion

  • Recommend a strategy or provide a initial step to removing InventoryItems
@awwaiid awwaiid added the core team Groomed but likely needs expert knowledge label May 5, 2024
@dorner
Copy link
Collaborator

dorner commented May 10, 2024

Started looking at this... there really isn't a way to do this halfway and validate it. A whole crapload of specs fail because they're testing very specific methods or items that won't exist. Basically the end result of this would be a branch where all reference to inventory items and any method that deals with them are completely expunged.

One thing that tripped me up is that a bunch of specs do storage_location.items, which goes through inventory_items and which fail a lot. :(

@cielf
Copy link
Collaborator

cielf commented May 16, 2024

Hmm. I'm not sure that I've throughly tested the inventory history feature with event sourcing. I expect we'll need to have it only go back to the snapshot date, yes?

@dorner
Copy link
Collaborator

dorner commented May 17, 2024

Yep. We can only show history via events.

Copy link
Contributor

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

@github-actions github-actions bot added the stale label Jun 17, 2024
@cielf
Copy link
Collaborator

cielf commented Jun 17, 2024

I believe the status on this is that we have determined that we will need the inventoryitem until such time as we don't care about the inventory history before event sourcing. This is estimated to be (hopefully) January 1 2026. Especially as the pre-event sourcing inventory history is suspect, we should be able to get stakeholder buy-in for that.

@dorner
Copy link
Collaborator

dorner commented Jun 17, 2024

We will need InventoryItem, the model, but we should be removing all code that creates, updates or reads them.

@cielf
Copy link
Collaborator

cielf commented Jun 17, 2024

Except for whatever is necessary to use the versions.

@dorner
Copy link
Collaborator

dorner commented Jun 17, 2024

Yup. Just that one place.

@cielf
Copy link
Collaborator

cielf commented Jun 21, 2024

I'm adding a "Waiting for" to the HE Project, and putting in an entry about this dated Jan 1 2026. Closing it as a current issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core team Groomed but likely needs expert knowledge stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants