Add Total ConnectID Accounts Metric #497
Conversation
| @@ -70,6 +70,12 @@ def filter_users(country_code: str, credential: list[str]): | |||
| return [ConnectIdUser(**user_dict) for user_dict in data["found_users"]] | |||
|
|
|||
|
|
|||
| def fetch_user_counts() -> dict[str, int]: | |||
There was a problem hiding this comment.
We should cache this in a way we only fetch current month data from API (since previous months data change, so it can be cached more aggressively)
There was a problem hiding this comment.
Also, it doesn't seem very ideal to fetch this data from CID, is there no way to infer it on CCC?
There was a problem hiding this comment.
There is not a way to infer it on CCC, but I agree we should cache it. Honestly, we could just save it in postgres. We only need to fetch the current month (and maybe the last one if we haven't fetched since the month switched). We might even be able to do that nightly on a task so loading the table never causes a network call.
| @@ -78,6 +80,7 @@ def get_table_data_for_year_month( | |||
| date_paid__lt=end_date, | |||
| ) | |||
|
|
|||
| connectid_user_count_data = fetch_user_counts() | |||
There was a problem hiding this comment.
Doesn't seem ideal that we are fetching all months data but we need only the selected month data, so may be pass a month param to the API?
There was a problem hiding this comment.
Beyond the fetch comment, I think this metric isn't quite right. It should be cumulative to that point (the total number created through that month, not in that month). I don't have strong opinions whether to do that on the connect id side or here, but I believe that is the requirement.
| @@ -70,6 +70,12 @@ def filter_users(country_code: str, credential: list[str]): | |||
| return [ConnectIdUser(**user_dict) for user_dict in data["found_users"]] | |||
|
|
|||
|
|
|||
| def fetch_user_counts() -> dict[str, int]: | |||
There was a problem hiding this comment.
There is not a way to infer it on CCC, but I agree we should cache it. Honestly, we could just save it in postgres. We only need to fetch the current month (and maybe the last one if we haven't fetched since the month switched). We might even be able to do that nightly on a task so loading the table never causes a network call.
Product Description
This PR adds the Total ConnectID Accounts metric to the report.
Technical Summary
Ticket Link
Related ConnectID API PR
Safety Assurance
Safety story
Automated test coverage
QA Plan
Labels & Review