most used content (table edition) #31
Conversation
|
Looking at the connect.posit.it table, I think it makes a good argument for separating anonymous and logged-in hits. |
| get_usage( | ||
| client, | ||
| from = format(date_range()$from_date, "%Y-%m-%dT%H:%M:%SZ"), | ||
| to = format(date_range()$to_date + hours(23) + minutes(59) + seconds(59), "%Y-%m-%dT%H:%M:%SZ") | ||
| ) | ||
| }) |> bindCache(date_range()$from_date, date_range()$to_date) |
There was a problem hiding this comment.
It feels wrong to format these dates here and not inside of get_usage(). Also, strictly speaking making this reactive now is extra complexity that is not needed right now. We know we will at some point do it, yes, but we should not take on the maintenance burden today for something we will do in the future. (Yes, in this sequence of PRs we expect to get to it quickly, but this is the same pattern that makes YAGNI so pernicious)
There was a problem hiding this comment.
Hmm, interesting. Yes, I can see how the formatting should take place inside the function. I was lumping the formatting and date math together, but I'll separate them.
This needs to be a reactive so that we can use bindCache(), which only works on reactives.
There was a problem hiding this comment.
I think I don't actually need the formatting at all. I just need to modify the end date to be "end of selected day".
There was a problem hiding this comment.
I do wonder whether these should be in the time zone of the Connect server.
There was a problem hiding this comment.
I do wonder whether these should be in the time zone of the Connect server.
This is going to end up being messy almost anyway you split it. Our API should either accept fully specified timestamps (with timezones) or only accept UTC (and our client code should convert to that form whatever is local) and then do what it needs to to make that work on the server (i.e. if the server is non-utc, then make sure the comparisons are timezone-aware and not comparing a UTC to some other timezone)
- remove date formatting - remove errant #nolint - add cache clear button
|
I wound up adding a "clear cache" button to make my own dev life easier… |
A prototype version of a "most used content" table.
Notes:
get_usage.Rsidecar file that allows the extension to run using the dev firehose endpoint or the legacy "shiny" and "static" usage endpoints.cachem::cache_disk(). The caches expire every 8 hours. The cache from the content API is static within that time window, and the cache from the metrics API is keyed on the day passed into the API (which has no effect now, but will when we add date filters to the UI).The latter two points mean that the content runs successfully on Dogfood and connect.posit.it:
Some thoughts that are top of mind for me:
DThandles sorting, every time you click a new column, it defaults to sorting it ascending, which is not the behavior I think fits best. From my research, there aren't any built-in options to change this on a per-column basis, and I don't want to get hacky at this early stage.