Kpi aggregations

[anchit-chandran]

It's still WIP, but @mbarton discussed merging it into live so people can upload and view numbers. If a calculation method is not implemented, it is handled as 'Not implemented' in the table.

View calculation results at temp URL: https://npda.localhost/kpis/aggregation/pdu/PZ999 (note: you'd have to upload the dummy_csv_invalid first (or any csv but have updated values in there to ensure they get shown in aggregations due to particular KPI eligibilities).

Important changes:

• Adds modular CalculateKPIS class
• Adds associated tests for each kpi implemented so far (and architecture to make this easier) (NOTE: for debugging, I've set postcode for seeded patients as some particular name, as it's just a non-validated string field)

Note

have also patched call to imd_for_postcode globally across tests so now they run much faster, ~1-2 mins down to a few seconds, I should have done this earlier

• Updates to dummy_sheet_invalid.csv to ensure some kpi values show (e.g. has T1DM, setting ages around 12 etc)
• Adds KPI excel spreadsheets so centralised and version controlled changes
• Adds temp view with simple table as template visualising kpi results, accessible at URL above

[mbarton]

This is a big enough PR that I can't really review it in a meaningful sense but I've run it up locally and it calculates KPIs for dummy_sheet_invalid.csv and the tests all pass.

The debugging endpoint doesn't have any permissions checking on it but I guess that's fine since we're planning to replace it with dashboarding at the PDU level and upwards anyway.

So shall we merge this @anchit-chandran and then we can raise additional PRs against live to implement the other KPIs?

[mbarton]

Not as readable when debugging tests perhaps but we could use a UUID here instead:

email = f"npda_test_user_{str(uuid.uuid4())}@nhs.net"

[anchit-chandran]

much nicer, good shout! once i make this change, will merge

[anchit-chandran]

This is a big enough PR that I can't really review it in a meaningful sense but I've run it up locally and it calculates KPIs for dummy_sheet_invalid.csv and the tests all pass.

So shall we merge this @anchit-chandran and then we can raise additional PRs against live to implement the other KPIs?

Agreed it's definitely too large, the real review will come from people using and seeing whether numbers are as expected. Hopefully after the first few kinks, should be easy to sort as will be related to definition nuances; particularly if there's issues in KPI calculations 1-12, it'll lead to downstream issues in other calculations. But equally, a fix there will fix any downstream issues too.

An important note here is that the tests should definitely pass but that doesn't mean the calculations are correct as depends on KPI definitions which have required updating / confirming as I work through each calculation. Have made updates to the excel sheet which is git tracked in the repo

The debugging endpoint doesn't have any permissions checking on it but I guess that's fine since we're planning to replace it with dashboarding at the PDU level and upwards anyway.

Also re permissions - we're not using any real data currently? That was the main reason I hadn't put perms checks as just a quick debugging view with intention; but is quick to add the perms mixin

[mbarton]

Also re permissions - we're not using any real data currently? That was the main reason I hadn't put perms checks as just a quick debugging view with intention; but is quick to add the perms mixin

My only worry is that we might forget about it - so lets raise an issue to track removing it

[anchit-chandran]

Add issue and updated email field. Will merge

[mbarton]

Seen on STAGING (merged by @anchit-chandran 5 minutes and 21 seconds ago) Please check your changes!