5
11
Fork
You've already forked tomato
1

Break out data export into one row per task log #178

Merged
edwardloveall merged 1 commit from el-task-log-export into main 2026年06月25日 21:07:06 +02:00

Rootable's export is modeled after our old system (the robot) which grouped
every task log onto one line per task. For example, these three task logs:

  • 50 lbs of Produce: apples and pears
  • 100 lbs of Dairy: eggs
  • 10 lbs of Bread: community didn't want it, took it to the food bank

would export like:

item types item weights item descriptions
Produce:Dairy:Bread 50:100:10 apples and pears:eggs:community didn't want it, took it to the food bank

That information is challenging to disentangle for people wanting to use
spreadsheet formulas, sorting and filtering to understand their data.

So we're splitting it up here to be one row per task log:

item type item weight item description
Produce 50 apple and pears
Dairy 100 eggs
Bread 10 community didn't want it, took it to the food bank

This changes several columns. Plurals are now singular e.g. volunteers is now
volunteer. The total weight column (now total location weight) is on every
row, summing the weight for every log at that location. The the donor and
recipients are merged into a location column because each log is for a
single location.

I tried for a day to make the csv exporter performant, I could not do it with
ActiveRecord alone. So I switched to a database view called task_log_summaries.
It sped up the generation of the csv exports by many, many times; something like
57s for a month of logs down to 350ms.

You never want every task log, so filtering is done by passing in a list ids
from some other model, like event_id, task_id, etc. Time range filtering is
also recomended for building a report. Associations can then be easily
pre-loaded.

The ExportService still needs to act on events, not task logs, because you can
have an event that appears in the export with no task logs. But again, all the
task log associations can be preloaded and it's still fast.

Rootable's export is modeled after our old system (the robot) which grouped every task log onto one line per task. For example, these three task logs: - 50 lbs of Produce: apples and pears - 100 lbs of Dairy: eggs - 10 lbs of Bread: community didn't want it, took it to the food bank would export like: | item types | item weights | item descriptions | | ------------------- | ------------ | ------------------------------------------------------------------------ | | Produce:Dairy:Bread | `50:100:10` | apples and pears:eggs:community didn't want it, took it to the food bank | That information is challenging to disentangle for people wanting to use spreadsheet formulas, sorting and filtering to understand their data. So we're splitting it up here to be one row per task log: | item type | item weight | item description | | --------- | ----------- | -------------------------------------------------- | | Produce | 50 | apple and pears | | Dairy | 100 | eggs | | Bread | 10 | community didn't want it, took it to the food bank | This changes several columns. Plurals are now singular e.g. `volunteers` is now `volunteer`. The `total weight` column (now `total location weight`) is on every row, summing the weight for every log at that location. The the `donor` and `recipients` are merged into a `location` column because each log is for a single location. I tried for a day to make the csv exporter performant, I could not do it with ActiveRecord alone. So I switched to a database view called task_log_summaries. It sped up the generation of the csv exports by many, many times; something like 57s for a month of logs down to 350ms. You never want _every_ task log, so filtering is done by passing in a list ids from some other model, like `event_id`, `task_id`, etc. Time range filtering is also recomended for building a report. Associations can then be easily pre-loaded. The `ExportService` still needs to act on events, not task logs, because you can have an event that appears in the export with no task logs. But again, all the task log associations can be preloaded and it's still fast.
Break out data export into one row per task log
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
217e7a5334
Rootable's export is modeled after our old system (the robot) which grouped
every task log onto one line per task. For example, these three task logs:
- 50 lbs of Produce: apples and pears
- 100 lbs of Dairy: eggs
- 10 lbs of Bread: community didn't want it, took it to the food bank
would export like:
| item types | item weights | item descriptions |
| ------------------- | ------------ | ------------------------------------------------------------------------ |
| Produce:Dairy:Bread | `50💯10` | apples and pears:eggs:community didn't want it, took it to the food bank |
That information is challenging to disentangle for people wanting to use
spreadsheet formulas, sorting and filtering to understand their data.
So we're splitting it up here to be one row per task log:
| item type | item weight | item description |
| --------- | ----------- | -------------------------------------------------- |
| Produce | 50 | apple and pears |
| Dairy | 100 | eggs |
| Bread | 10 | community didn't want it, took it to the food bank |
This changes several columns. Plurals are now singular e.g. `volunteers` is now
`volunteer`. The `total weight` column (now `total location weight`) is on every
row, summing the weight for every log at that location. The the `donor` and
`recipients` are merged into a `location` column because each log is for a
single location.
I tried for a day to make the csv exporter performant, I could not do it with
ActiveRecord alone. So I switched to a database view called task_log_summaries.
It sped up the generation of the csv exports by many, many times; something like
57s for a month of logs down to 350ms.
You never want _every_ task log, so filtering is done by passing in a list ids
from some other model, like `event_id`, `task_id`, etc. Time range filtering is
also recomended for building a report. Associations can then be easily
pre-loaded.
The `ExportService` still needs to act on events, not task logs, because you can
have an event that appears in the export with no task logs. But again, all the
task log associations can be preloaded and it's still fast.
thetizzo left a comment
Copy link

A few questions on some details but overall this looks good. Definitely seems like a far more readable report format.

A few questions on some details but overall this looks good. Definitely seems like a far more readable report format.
@ -0,0 +1,11 @@
class TaskLogSummary < ApplicationRecord
self.primary_key = :id
First-time contributor
Copy link

Isn't id the default primary key or do you need this because this is a view?

Isn't `id` the default primary key or do you need this because this is a view?
Author
Owner
Copy link

Yup, it's because it's a view. Postgres views can't have primary keys, so you have to tell rails which column to use. Here's a bit in the scenic readme about it.

Yup, it's because it's a view. Postgres views [can't have primary keys](https://stackoverflow.com/a/11667651/638966), so you have to tell rails which column to use. Here's a bit in [the scenic readme](https://github.com/scenic-views/scenic#can-i-use-this-view-to-back-a-model) about it.
@ -38,1 +39,3 @@
return [pickupless_row(event)] if has_no_pickup_tasks?(event)
task_logs = event.task_log_summaries
if has_no_logs_or_notes?(event, task_logs)
return []
First-time contributor
Copy link

I'm curious if this should return a logless_row to make this return value consistent across the 2 cases?

I'm curious if this should return a `logless_row` to make this return value consistent across the 2 cases?
Author
Owner
Copy link

This is from before my time, but if an event has no logs and no event note, we don't want to return anything. It represents a shift with no information to export, so we skip it.

This is from before my time, but if an event has no logs _and_ no event note, we don't want to return anything. It represents a shift with no information to export, so we skip it.
@ -39,0 +40,4 @@
if has_no_logs_or_notes?(event, task_logs)
return []
end
if task_logs.empty?
First-time contributor
Copy link

Does the task_logs.none? in has_no_logs_or_notes? cover this case? This reads like we're checking the same condition twice.

Does the `task_logs.none?` in `has_no_logs_or_notes?` cover this case? This reads like we're checking the same condition twice.
Author
Owner
Copy link

Not quite. has_no_logs_or_notes? was also critically checking the notes part. If there are notes but no logs, we want to display something. However, you're right that's it's confusing duplication, so I refactored a bit to try to make it more clear. Here's the truth table for an event that gets passed into the method:

logs notes output
no no nothing
no yes loggless
yes no full
yes yes full
Not quite. `has_no_logs_or_notes?` was also critically checking the notes part. If there are notes but no logs, we want to display something. However, you're right that's it's confusing duplication, so I refactored a bit to try to make it more clear. Here's the truth table for an event that gets passed into the method: | logs | notes | output | | ---- | ----- | -------- | | no | no | nothing | | no | yes | loggless | | yes | no | full | | yes | yes | full |
@ -0,0 +7,4 @@
task_logs.notes,
task_logs.task_id,-- to get location
task_logs.user_id,-- to get volunteer names
task_logs.event_id-- to get transportation details (event notes)
First-time contributor
Copy link

I like the comments here, helps make this very clear.

I like the comments here, helps make this very clear.
Author
Owner
Copy link

Thanks! That also reminded me that I wanted to put in a comment about window function as they are weird if you're not used to them.

Thanks! That also reminded me that I wanted to put in a comment about window function as they are weird if you're not used to them.
edwardloveall force-pushed el-task-log-export from 217e7a5334
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
to 51e8d19610 2026年06月23日 22:01:40 +02:00
Compare
edwardloveall force-pushed el-task-log-export from 51e8d19610 to c301bb3452
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
2026年06月24日 12:49:33 +02:00
Compare
thetizzo left a comment
Copy link

Looks great!

Looks great!
@ -39,0 +44,4 @@
# No information to export
[]
end
end
First-time contributor
Copy link

Love this refactor. Much easier to understand what this is doing now.

Love this refactor. Much easier to understand what this is doing now.
@ -0,0 +6,4 @@
-- If you've never used window functions before, this takes _every_ row in the
-- query, groups (partitions) everything with the same task_id and event_id,
-- sums the weight column, and puts that value in every row that matches the
-- same task_id and event_id.
First-time contributor
Copy link

Looks good!

Looks good!
edwardloveall force-pushed el-task-log-export from c301bb3452
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
to be4fb28f7d
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
2026年06月25日 20:59:18 +02:00
Compare
edwardloveall deleted branch el-task-log-export 2026年06月25日 21:08:17 +02:00
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
rootable/tomato!178
Reference in a new issue
rootable/tomato
No description provided.
Delete branch "el-task-log-export"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?