fecgov / fecfile-web-api

Back-end API for FECfile application
Other
8 stars 2 forks source link

Create the amount calculation fields in the transaction table #843

Closed mjtravers closed 3 months ago

mjtravers commented 5 months ago

The aggregate calculations are currently being performed just-in-time during the transaction record READ operation.

This spike is to move the calculation from the view to the CREATE, UPDATE, and DELETE operations and save the result in the transactions_transaction table and see how it affects performance for the 3 calculations currently storing their results in the transaction view:

Since updating a transaction potentially impacts the aggregate of any transaction in it's topology/network, we need to do write an update that calculates the aggregates and updates the transactions. this should be doable with a single bulk update query that calculates the aggregates with a window function.

QA Notes

null

DEV Notes

null

Design

null

See full ticket and images here: FECFILE-181

mjtravers commented 4 months ago

From CR:

The new aggregation code needs to take into account when a transaction record is edited so that it is removed from a calculation group. For example, if there is a group of 3 transactions for an entity aggregation and one of the transactions has its contact changed, only the updated transaction in its new group is aggregated. The other 2 transactions that were part of the original group are do not have their new aggregation sums without the transaction that was removed from the group recalculated.

To account for this scenario, the trigger code should factor out the window function rollups into separate functions. Then the trigger can call them once for the NEW row record and, if the record has changed so that it is in a new group, then the rollup function can be called on the OLD row record values to update the old group transactions. In Postres triggers, NEW and OLD are provided to the function as part of the Postgres API. See here.

Also, a couple of unit tests are broken and need to be fixed:

======================================================================
FAIL: test_serialize_f3x_summary_instance (fecfiler.web_services.dot_fec.test_dot_fec_serializer.DotFECSerializerTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/nxg_fec_e2e/fecfiler/web_services/dot_fec/test_dot_fec_serializer.py", line 173, in test_serialize_f3x_summary_instance
    self.assertEqual(split_row[0], "F3XN")
AssertionError: '' != 'F3XN'
+ F3XN

======================================================================
FAIL: test_create_dot_fec (fecfiler.web_services.test_tasks.TasksTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/nxg_fec_e2e/fecfiler/web_services/test_tasks.py", line 57, in test_create_dot_fec
    self.assertEqual(lines[1][:5], "F3XN" + FS_STR)
AssertionError: '\x1cC000' != 'F3XN\x1c'
+ F3XN- - C000

Sending ticket back to sprint backlog to be worked on by the next developer who has the bandwidth.

mjtravers commented 3 months ago

Passes CR. Sending to QA. The code updates from this ticket did not fix #1991 so that ticket will be worked separately.

Unit tests pass: https://app.circleci.com/pipelines/github/fecgov/fecfile-web-api/3854/workflows/9c1e5c2e-0476-42bd-b665-9c96c482e3e5/jobs/11614

Image

WiseQA commented 3 months ago

QA review verified visual inspection per DEV screenshot above showing the successfully Unit tests pass.

As noted bug ticket 1991 will be reworked.

This ticket passes QA moved to Stage Ready.

exalate-issue-sync[bot] commented 3 months ago

akhorsand commented: Accepted during PI Planning Sprint Review on 7/8/2024