Comment 33 for bug 1777675

Dan Wells (dbw2) wrote :

Okay, I have tested and re-reviewed this code. A few things:

1. The areas of redundant/separate fetching look completely eliminated, so thank you, Kyle, for cleaning that up!

2. Remington was also involved in testing, and I have pushed and signed-off on a commit from him which fixes a couple use cases.

3. I added two commits myself. The first simply restores a few "missing" columns from the checkin interface (i.e. makes the checkin row the expected 12 units wide). The second involves our shared discomfort with the IDL naming. However, rather than just "Inventory", I'm proposing a new label of "Latest Inventory". My reasoning is, if we still expect a future "All Inventories" entry, then "All Inventories" and "Latest Inventory" seem to go better together than "All Inventories" and "Inventory". Then again, it is just a label we can change without too much trouble, so I support either way. I feel like we *may* want to also change the underlying table name to "latest_inventory" to save on future developer comprehension, but have not done so yet. Still, it's probably now or never, and I would definitely sign-off on that :)

4. Finally, Jeff Godin contacted me while I was reviewing this as he has separate concerns, so I am pushing my branch for its sign-offs/fixes, but also assigning this bug to him for any changes he is still considering.

Latest branch is here:
http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dbwells/lp1777675-inventory-date-support-signoff-2