rubyforgood
Add events view
Fixes #4079.
This adds a simple events view for org admins:
Missing some features:
- CSV export
- Filter by storage location and item
We can do that as a fast follow.
@dorner I was going to ask for this! Obviously needing the tests to pass, but if we can I'd like to see something like a date on the entries . Is it possible for us to say whether it is new or edit?
Also - we need to make the column headers more relatable -- because to them, "Event" is likely to sound like "the thing I did" -- which to them will be more like what the "resource" is (I'm assuming resource is the itemizable). What would you think of something like "internal id" and "Refers to".?
Tried it on a fresh setup on local. Did not turn on the events. 1/ Was able to get at it -- I had assumed this would be behind the events flag 2/ Got the following:
I don't know if that's a problem with the seed, or what, but I'll need that to work to be able to kick the tires enough to give a better opinion. (Maybe it just doesn't work without the flag and needs to be hidden?) Thanks.
It was definitely rushed, you are 100% correct. Didn't even have time to see if the tests and lint finished before logging off. No problem at all waiting for fixes.
Btw I didn't put it behind the flag because the events are still happening, they're just not being used. But if you think it'd be confusing, I can put it behind the flag.
I think we'll need to rethink the terminology for in front of the users. A new user is not going to have any idea of what "Event History" means. Maybe just "History"? I'd be ok with putting it in front of the early adopters and explaining what it is to each of them as is, so if you could put it in behind the flag that'd be great.
At this point, I see this as mainly a helping-with-diagnoses tool, but it may end up being something someone ends up relying on.
@cielf I've made the suggested changes. The crash was due to SnapshotEvents having a different format than the rest (not sure why I didn't come across it before).
Should still have a fast-follow where I add more tests and filters.
I didn't add the event flag because that's not merged yet :) I can put it in as soon as it is.
Ok...just dipping my toe in the water of kicking the tires. Here's what I did, what I expected (given the expected use as a troubleshooting tool) and what I see:
I started with a fresh seeded setup, and then I went and edited a purchase, to add an item to it (since the seed has none). I expected that I would see something like a note that i edited a purchase, with the new values, at the time that I made the changes. Instead, I see it at what i assume is the "purchase date", since it is bang-on midnight, which is not when the thing happened. The 'bang-on midnight' may be a seed thing, though, because it is the correct date when I add a new one.
As for the event flag - I think we should have it before it goes live, but it's actually a bit convenient to have it not on at this point.
Good point - the history date doesn't differentiate between creation and update. I'll put something in there for that.
Hmm... actually that's a lot harder than it sounds. I think leaving it as is is probably better (i.e. a "Distribution" event could mean creation or update). I'll check the case you mentioned.
If it doesn't give the whole story, we might want to keep it to ourselves, though. In my mind its purpose was for walking through what happened step-by-step. Almost a log. Let's talk about it Sunday.
I've added type and storage location filters:
Unfortunately I'm reeeeally behind on tests (i.e. I don't have any for any of this). Hoping to get some of them in on Monday.
I kinda love the little funnel gizmo now that I tried it out. Nice touch.
The one thing I've got some qualms about is the ordering. It is the opposite of the default order on everything else, so there may be confusion. This is not me saying to change it, because when we are using it for the purpose of "ok, then what happened... then what happened next..." the chronological order makes good sense and will be easier to use.
It might only be tripping my "what's going on here" wires because I'm looking at it on the seed, which is all from today. If you have real data, that has been collected over time, it'll be clearer? I think we can send it to the early adopters like this, but we should consider if there is a reasonable cue we can provide - just because it's the opposite of the order everywhere else. (Maybe even just say History (in chronological order) as the title?)
Alas, the Filter by storage location isn't filtering completely. I tried it with Pawnee Main Bank(Office), and I'm still seeing the snapshot for Bulk Storage Location. Everything else looks reasonable in that case.
Snapshots aren't per-storage-location, they're global. So by design the filter includes all snapshots.
Another thing I'm wondering is whether we should make the initial date range smaller.
This is coloured by my picture of it as a troubleshooting tool -- most of the time I would hope we'll just be looking at the last month or two - or at least we would start by looking at the last short while.
What do you think?
(Edit: I just tried it with the full history for MHM, and it was pretty fast! So that part of my concern is allayed somewhat.)
The default view is unfortunately hardcoded into the helpers. We really should find a way to be able to break that out. :(
Snapshots aren't per-storage-location, they're global. So by design the filter includes all snapshots.
But they say that they belong to a storage location in the view....
They have multiple storage locations. If you scroll down you'll see both. There isn't a great way to split that up in the view.
For now. We probably will introduce more in the future for performance purposes.
Ok. I think that might end up being confusing to the users, but I'm willing to early adopt this as it is and kick that particular can down the road until we see if people end up using this for anything but troubleshooting.
I'd like @awwaiid to take a look at this from a technical pov. (Though I'm pretty sure it will pass with flying colours once you clear the failing tests/lint.)
@cielf I added a separate background color for snapshot events which should at least make it more obvious that the two rows are grouped together.
Lint and tests should be fixed.
(nods) I can foresee a future, if this is useful to the banks for something other than this troubleshooting, where we hide the snapshots by default, and have an "include system snapshots" checkbox, but that's good for now.
@awwaiid Can you do a quick pass on this from a technical pov. LGTM from a functional pov. Thank you.
@dorner Ok. This is late-breaking, but we've got the user information on the event, yes? That could actually be handy when we are talking through troubleshooting with the banks. But I'm totally willing to have it be a fast follow-up in the interest of getting this in.