Move entity aggregate out of trigger and into django code

[exalate-issue-sync]

This ticket is to move the Schedule A and Schedule B entity aggregate code out of database triggers and into a mechanism invoked on the django code.

from the broader outline in the comment below, the tasks for this ticket are limited to the following for schedule a/b entity aggregates:

• test and gather metrics on the business logic prior to changes
• move the business logic out of the trigger and into django code
  • removing the trigger code with migration
  • creating new code in api that calculates and synchronizes aggregates
    * identify potential breakout test coverage work that might block this ticket.
• test and gather metrics on the business logic after changes

QA Notes

null

DEV Notes

null

Design

null

See full ticket and images here: FECFILE-2726

Pull Request: #1877

[exalate-issue-sync]

Dan Fowlkes commented: Testing:

• run locust locally
• stress the number of transactions for a given contact in a committee

From Dev Sync: current plan to call calculation+update process intentionally in all needed places

Do it in: transactions/views.py save_transaction and destroy

{{calculate_entity_aggregates()}}

Use Django ORM to apply aggregation updates across all applicable committee contacts rather than looping in code.

[exalate-issue-sync]

Todd Lees commented: For posterity, the context of this ticket is:

We have 5 fields that we calculate with database triggers.

Business logic in triggers:

• schedule a/b entity aggregate
• schedule e election aggregate
• schedule c loan payment to date
• Itemization
• updating reports on un-amendment eligibility (a new transaction on a report means it can not be un-amended)

Tasks:

• Establish method of testing performance and correctness impact
• Dev working session on where this business logic should live - completed 15 Dec 2025
• Audit of triggers in databases (Especially prod) - [https://fecgov.atlassian.net/browse/FECFILE-2700|https://fecgov.atlassian.net/browse/FECFILE-2700|smart-link], complete
  ** Remove triggers and functions that don’t need to be there - OBE
• For each of the business logic in triggers items listed above:
  ** test and gather metrics on the business logic prior to changes
  ** move the business logic out of the trigger and into django code
  ** test and gather metrics on the business logic after changes

[exalate-issue-sync]

Dan Fowlkes commented: h1. Progress to date.

h2. Performance testing

I have a local locust test branch in which I’ve modified the locust testing to make all transaction updates to the same entity so that I can be sure that the metrics gathered reflect stressing the aggregation logic.

h2. Logic migration.

In the course of removing the SchedA/SchedB entity aggregate triggers, I came to a point where I could either also address SchedC and SchedE or delve into the db triggers further to better understand/untangle the overlap(?) between the related triggers. The former seemed simpler/faster (and ultimately more useful) so I took that fork. Later, in the course of troubleshooting failures of the related unit tests, I determined that I’d strayed into Itemization as well so, faced with the same decision point I made the same choice and ended up addressing Itemization as well.

In summary, for this ticket I have addressed 4 of the 5 fields calculated with database triggers that the ticket was originally meant to address:

• schedule a/b entity aggregate
• schedule e election aggregate
• schedule c loan payment to date
• Itemization

I did not address the fifth:

• updating reports on un-amendment eligibility (a new transaction on a report means it can not be un-amended)

All unit tests are passing.

h1. Current status.

h2. Logic migration.

I am doing one final refactoring pass.

h2. Performance testing

Once the above is complete I will re-run my modified locust testing branch, collect new metrics, and compare.

[exalate-issue-sync]

Dan Fowlkes commented: I ran all e2e tests against this new branch and get the same results as running them again the current develop branch – in my local, three tests fail.

I am currently running my modified locust testing again this new branch. So far the numbers are, frankly, pretty terrible.

[exalate-issue-sync]

Dan Fowlkes commented: After a short discussion with Todd, I have made updates to {{aggregrate_service}} in an effort to replace what previously was a Python loop with a database update + window + bulk update. I will need to re-test to see if the locust metrics have improved appreciably.

[exalate-issue-sync]

Dan Fowlkes commented: Updated the PR link to point to the new, signal-less PR.

[exalate-issue-sync]

Dan Fowlkes commented: h1. Locust testing (local)

h2. Reports

Before (develop branch): [^Locust_2026-01-08-21h56_locust_run.py_http___fecfile-api-proxy_8080.html]

After (PR-1840 implementation): [^Locust_2026-01-09-18h17_locust_run.py_http___fecfile-api-proxy_8080.html]

h2. Details

100 users, 5/sec

Ran {{load_mocked_committee_data}}with no arguments. Per committee we log: 10 reports, 100 contacts, 450 SchedA single (per committee or report?), 450 Sched B single, 50 SchedA tiered, 50 SchedB tiered. 

h2. Comparison

In nearly all cases the request and response times went up. The one exception in Schedule C, which deserve extra scrutiny to ensure that it was optimized rather than broken.

h3. Request: Average (ms)

||Operation||Before||After||Diff (%)||
|create_new_schedule_a_transaction|36.98|44.88|21|
|create_new_schedule_b_transaction|37.74|43.68|16|
|create_new_schedule_c_transaction|361.18|{color:#36b37e}97.1{color}|{color:#36b37e}-73{color}|
|create_new_schedule_d_transaction|50.45|58.69|16|
|delete_schedule_a_transaction|21.76|28.94|33|
|update_schedule_a_transaction|36.52|41.68|14|
|update_schedule_b_transaction|34.52|39.07|13|

h3. Response: (ms)

||Operation||Before||After||
|create_new_schedule_a_transaction|35 36 38 40 46 53 98 110|42 43 45 48 53 66 100 110|
|create_new_schedule_b_transaction|36 37 39 40 45 57 95 99|41 43 44 46 53 60 100 110|
|create_new_schedule_c_transaction|360 390 390 660 660 660 660 660|{color:#006644}110 110 110 120 120 120 120 120{color}|
|create_new_schedule_d_transaction|51 55 60 62 69 73 78 78|59 62 65 67 70 78 150 150|
|delete_schedule_a_transaction|22 22 22 22 22 22 22 22|32 32 32 32 32 32 32 32|
|update_schedule_a_transaction|35 36 38 39 44 50 81 85|40 42 44 46 51 55 66 91|
|update_schedule_b_transaction|33 34 35 38 41 44 54 92|38 39 40 42 46 53 69 96|

[exalate-issue-sync]

Dan Fowlkes commented: h1. Locust testing (load mirror)

h2. Reports

Before (develop branch): [^Locust_2026-01-14-20h50_locust_run.py_https___load-dev-api.app.cloud.gov.html]

After (PR-1840 implementation): [^Locust_2026-01-15-18h52_locust_run.py_https___load-dev-api.app.cloud.gov.html]

h2. Details

100 users, 5/sec

{{load_mocked_committee_data}}previously run.

h2. Comparison

In nearly all cases the request and response times went up. The one exception in Schedule C, which deserve extra scrutiny to ensure that it was optimized rather than broken.

h3. Request: Average (ms)

||Operation||Before||After||Diff (%)||
|create_new_schedule_a_transaction|135.77|184.36|36|
|create_new_schedule_b_transaction|128.79|185.6|44|
|create_new_schedule_c_transaction|9430.32|{color:#36b37e}288.58{color}|{color:#36b37e}-67{color}|
|create_new_schedule_d_transaction|360.48|369.08|2|
|delete_schedule_a_transaction|197.22|243.21|23|
|update_schedule_a_transaction|258.55|359.6|39|
|update_schedule_b_transaction|255.21|329.25|29|

h3. Response: (ms)

||Operation||Before||After||
|create_new_schedule_a_transaction|84 86 100 190 270 350 520 1200|96 160 200 270 350 460 1000 3700|
|create_new_schedule_b_transaction|84 87 98 190 270 340 480 690|100 160 200 260 350 480 950 6000|
|create_new_schedule_c_transaction|84 88 88 47000 47000 47000 47000 47000|{color:#006644}270 350 420 470 630 860 860 860{color}|
|create_new_schedule_d_transaction|85 87 100 860 1100 1400 1700 1700|91 130 280 620 1000 1400 2800 3900|
|delete_schedule_a_transaction|190 190 270 270 270 270 270 270|180 210 220 270 330 830 830 830|
|update_schedule_a_transaction|240 270 290 310 400 440 530 530|290 330 390 460 550 690 1400 4300|
|update_schedule_b_transaction|210 240 260 300 360 420 1000 1000|260 300 340 410 560 700 1200 3500|

[exalate-issue-sync]

Todd Lees commented: discussion tbd with todd/dan

[exalate-issue-sync]

Dan Fowlkes commented: Todd and I discussed ways to optimize our approach. I optimized the model save and delete to not do any aggregation-related logic unnecessarily and Todd created a [POC demonstrating his suggestion|https://github.com//pull/1869/] (with the latest develop branch updates merged in).

The POC adds a function [process_aggregation_for_entity()|https://github.com//pull/1869/changes/527d93827c50f59b05433c23e7cba8fbda953ac1#diff-d522f7b6df132c162cc78e4972c948f04b36295dc326024451cd29ec507a41c8R135] within the views {{process_aggregation()}} and comments out the [managers create()|https://github.com//pull/1869/changes/527d93827c50f59b05433c23e7cba8fbda953ac1#diff-909ced4f33e29e06f14baa9e8c9cc03b9c96f534a9b023648c2a4463a56bc986R127] and [a chunk of models save()|https://github.com//pull/1869/changes/527d93827c50f59b05433c23e7cba8fbda953ac1#diff-aba96e7195dfe2bb7e2ec1b10bb9b8805fe5d3ff0014eb1f3ebb94d0bfd6d946R257]. The latter bits were there to handle necessary cases and their removal resulted in 26 unit tests failing.

I have created a [new branch|https://github.com/fecgov/fecfile-web-api/tree/feature/2726-explicit_aggregation] based on the POC that [restores those bits and adds fixes and further optimization|https://github.com/fecgov/fecfile-web-api/commit/7e32e4281d5b03a1f1c9e840b414aafce78e277e]. It passes all unit tests.

As part of that new branch, I made some slight tweaks to Todd’s additions to cover more cases and was able to build on some logic-removal started in the POC to [comment out a bunch more|https://github.com/fecgov/fecfile-web-api/commit/7e32e4281d5b03a1f1c9e840b414aafce78e277e#diff-508248f6fb9bcf45cba6bc7a6c9d9964cc7e9568880cc979a67c2b9bbbf5eddd]. That latter win was somewhat offset, though, by new code that I added to recalculate aggregates for both old and new contacts when contacts change (another failing unit test).

h2. Justification for the POC-commented, now uncommented, aggregation code

We need to do aggregation on transaction creation and update. The uncommented {{Manager.create()}} and {{Transaction.save()}} code handled these.

{{Manager.create()}} calls {{update_aggregates_for_affected_transactions()}} for, and only for, aggregate-able schedules. We want to use {{create()}} because it ensures aggregation happens even with bulk operations (and direct INSERT statements?) that bypass {{save()}}. We set the {{in_manager_create}} flag to prevent double-triggering

The chunk in question in {{Transaction.save()}} ensures that updates to existing transactions (including any that don’t go through the manager) trigger aggregation recalculation so any modification to an existing transaction propagates aggregate changes downstream. In this implementation we are capturing an old snapshot for efficient delta calculations, handling itemization cascading to parents/children, and detecting contact changes requiring dual-chain updates.

In both cases we call {{update_aggregates_for_affected_transactions()}} which is already optimized with:

• delta-based updates (only updates affected transactions)
• conditional logic to skip non-aggregate schedules
• snapshot caching to avoid redundant queries
• guard against infinite recursion via in_manager_create flag

h2. Example Failing Test Cases

test_loan_payment_to_date (repayment amount update)

Broken: loan_payment_to_date was 1500 instead of 1200 because the aggregation system didn't process the amount change.
Fix: {{save()}} calls {{update_aggregates_for_affected_transactions()}} which recalculates loan payment totals.

test_itemization_with_parent_child_disbursement (parent-child relationship)

Broken: tier1 remained {{itemized=False}} because setting the parent relationship didn't trigger parent itemization recalculation.
Fix: {{save()}} calls {{_update_parent_itemization_service()}} which makes tier1 {{itemized=True}} (aggregate > 200 threshold).

test_tier3_itemization (cascade through hierarchies)

Broken: Without the {{save()}} additions, changing the amount didn't cascade itemization updates through the parent chain.
Fix: {{save()}} trigger recalculates itemization and cascades {{force_itemized=None}} up the chain.

test_get_entity_date_leapfrogging_and_contact_change (contact and date changes)

Broken: When first_transaction moved from contact_3 to contact_2, second_transaction (still contact_3) had aggregate=200 instead of 47, because the old contact's aggregate chain wasn't recalculated.
Fix: {{save()}} code + the {{views.py}} contact-change logic recalculate aggregates for both old and new contact chains.

Having said all of that…

h2. The Primary Question

Did Todd intend that I implement some other means of achieving all of the above using {{process_aggregation_for_entity()}} or patterning further code after it and negating the need for the aggregation code in {{create()}} and {{save()}} that he commented out? I have tried and failed to come up with a way to avoid uncommenting that code.

[exalate-issue-sync]

Dan Fowlkes commented: I [removed Manager.create() entirely|https://github.com/fecgov/fecfile-web-api/commit/be8939b21578f405fe24b8da80b6db635881f511] and the tests still pass.