Skip to content

Commit 8d1b97e

Browse files
committed
prosopite todo list
1 parent 8eeb090 commit 8d1b97e

File tree

2 files changed

+146
-1
lines changed

2 files changed

+146
-1
lines changed

PROSOPITE_TODO.md

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
# Prosopite N+1 Query Issues
2+
3+
Issues detected by Prosopite during test suite run. Fix by adding eager loading (`includes`, `preload`) or restructuring queries.
4+
5+
## High Priority (20+ occurrences)
6+
7+
### app/decorators/casa_case_decorator.rb:34
8+
- **Method:** `map` in decorator
9+
- **Likely fix:** Add `includes` for associated records being accessed in iteration
10+
11+
### app/models/user.rb:84
12+
- **Method:** `create_preference_set`
13+
- **Query:** `SELECT "preference_sets".* FROM "preference_sets" WHERE "preference_sets"."user_id" = $1`
14+
- **Likely fix:** Check if preference_set already loaded before querying
15+
16+
### app/models/concerns/api.rb:11
17+
- **Method:** `initialize_api_credentials`
18+
- **Query:** `SELECT "api_credentials".* FROM "api_credentials" WHERE "api_credentials"."user_id" = $1`
19+
- **Likely fix:** Check if api_credentials already loaded before querying
20+
21+
### app/lib/importers/file_importer.rb:50
22+
- **Method:** `create_user_record`
23+
- **Queries:** Multiple user lookups during import
24+
- **Likely fix:** Batch lookups or use `Prosopite.pause` for intentional import operations
25+
26+
## Medium Priority (10-19 occurrences)
27+
28+
### app/validators/user_validator.rb:56
29+
- **Method:** `validate_uniqueness`
30+
- **Query:** `SELECT "users".* FROM "users" WHERE "users"."type" = $1 AND "users"."email" = $2`
31+
- **Likely fix:** Consider if validation is necessary during bulk operations
32+
33+
### app/lib/importers/supervisor_importer.rb:47
34+
- **Method:** `block in assign_volunteers`
35+
- **Query:** `SELECT "users".* FROM "users" INNER JOIN "supervisor_volunteers"...`
36+
- **Likely fix:** Preload volunteers before assignment loop
37+
38+
### app/lib/importers/supervisor_importer.rb:51
39+
- **Method:** `block in assign_volunteers`
40+
- **Query:** `SELECT "users".* FROM "users" WHERE "users"."id" = $1`
41+
- **Likely fix:** Batch load users by ID before iteration
42+
43+
### app/controllers/case_contacts/form_controller.rb:156
44+
- **Method:** `block in create_additional_case_contacts`
45+
- **Likely fix:** Eager load case contact associations
46+
47+
## Lower Priority (5-9 occurrences)
48+
49+
### app/models/court_date.rb:32
50+
- **Method:** `associated_reports`
51+
- **Likely fix:** Add `includes` for court report associations
52+
53+
### app/lib/importers/supervisor_importer.rb:23
54+
- **Method:** `block in import_supervisors`
55+
- **Query:** `SELECT "users".* FROM "users" WHERE "users"."type" = $1 AND "users"."email" = $2`
56+
- **Likely fix:** Batch check existing supervisors before import loop
57+
58+
## Lower Priority (2-4 occurrences)
59+
60+
### app/lib/importers/file_importer.rb:57
61+
- **Method:** `email_addresses_to_users`
62+
- **Likely fix:** Batch load users by email
63+
64+
### app/lib/importers/case_importer.rb:41
65+
- **Method:** `create_casa_case`
66+
- **Likely fix:** Preload or batch casa_case lookups
67+
68+
### app/decorators/contact_type_decorator.rb:14
69+
- **Method:** `last_time_used_with_cases`
70+
- **Likely fix:** Eager load case associations
71+
72+
### app/datatables/reimbursement_datatable.rb:25
73+
- **Method:** `block in data`
74+
- **Query:** `SELECT "addresses".* FROM "addresses" WHERE "addresses"."user_id" = $1`
75+
- **Likely fix:** Add `includes(:address)` to reimbursement query
76+
77+
### app/services/volunteer_birthday_reminder_service.rb:7
78+
- **Method:** `block in send_reminders`
79+
- **Likely fix:** Eager load volunteer associations
80+
81+
### app/models/contact_topic.rb:27
82+
- **Method:** `block in generate_for_org!`
83+
- **Likely fix:** Batch operations or use `Prosopite.pause` for setup
84+
85+
### app/models/casa_org.rb:82
86+
- **Method:** `user_count`
87+
- **Likely fix:** Use counter cache or single count query
88+
89+
### app/models/casa_org.rb:62
90+
- **Method:** `case_contacts_count`
91+
- **Likely fix:** Use counter cache or single count query
92+
93+
### app/lib/importers/volunteer_importer.rb:23
94+
- **Method:** `block in import_volunteers`
95+
- **Likely fix:** Batch check existing volunteers before import loop
96+
97+
### app/lib/importers/case_importer.rb:20
98+
- **Method:** `block in import_cases`
99+
- **Likely fix:** Batch check existing cases before import loop
100+
101+
### app/controllers/case_contacts/form_controller.rb:26
102+
- **Method:** `block (2 levels) in update`
103+
- **Likely fix:** Eager load contact associations
104+
105+
## Single Occurrence
106+
107+
### app/services/missing_data_export_csv_service.rb:40
108+
- **Method:** `full_data`
109+
110+
### app/policies/contact_topic_answer_policy.rb:18
111+
- **Method:** `create?`
112+
113+
### app/models/casa_case.rb:152
114+
- **Method:** `next_court_date`
115+
116+
### app/models/all_casa_admins/casa_org_metrics.rb:16
117+
- **Method:** `map`
118+
119+
### config/initializers/sent_email_event.rb:7
120+
- **Method:** `block in <top (required)>`
121+
- **Query:** `SELECT "casa_orgs".* FROM "casa_orgs" WHERE "casa_orgs"."id" = $1`
122+
- **Note:** Initializer callback - consider caching org lookup
123+
124+
## Notes
125+
126+
- **Importers:** Many N+1s occur in import code. Consider wrapping entire import operations in `Prosopite.pause { }` if the N+1 pattern is intentional for per-record processing, or batch-load records before iteration.
127+
128+
- **Decorators:** Add `includes` at the controller/query level before passing to decorators.
129+
130+
- **Callbacks:** User model callbacks (`create_preference_set`, `initialize_api_credentials`) fire on each create. Consider if these can be optimized or if the N+1 is acceptable for single-record operations.
131+
132+
## How to Fix
133+
134+
```ruby
135+
# Before (N+1)
136+
users.each { |u| u.orders.count }
137+
138+
# After (eager loading)
139+
users.includes(:orders).each { |u| u.orders.count }
140+
141+
# For intentional batch operations
142+
Prosopite.pause do
143+
records.each { |r| process_individually(r) }
144+
end
145+
```

spec/support/prosopite.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
# Test configuration
66
Prosopite.enabled = true
7-
Prosopite.raise = true # Fail specs on N+1 detection
7+
Prosopite.raise = false # Log only, don't fail specs
88
Prosopite.rails_logger = true
99
Prosopite.prosopite_logger = true
1010

0 commit comments

Comments
 (0)