Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENT-9704: fix: Fixed null email issue for leaderboard. #536

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ Unreleased

=========================

[10.2.0] - 2024-11-12
---------------------
* Fixed null email issue for leaderboard.


[10.1.0] - 2024-10-29
---------------------
* Added management command to pre-warm analytics data.
Expand Down
2 changes: 1 addition & 1 deletion enterprise_data/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Enterprise data api application. This Django app exposes API endpoints used by enterprises.
"""

__version__ = "10.1.0"
__version__ = "10.2.0"
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,55 @@ def get_completion_data_for_leaderboard_query(email_list: list):
GROUP BY email;
"""

@staticmethod
def get_engagement_data_for_leaderboard_null_email_only_query():
"""
Get the query to fetch the engagement data for leaderboard for NULL emails only.

Query should fetch the engagement data for like learning time, session length of
the enterprise learners to show in the leaderboard.

Returns:
(str): Query to fetch the engagement data for leaderboard.
"""
return """
SELECT
email,
ROUND(SUM(learning_time_seconds) / 3600, 1) as learning_time_hours,
SUM(is_engaged) as session_count,
CASE
WHEN SUM(is_engaged) = 0 THEN 0.0
ELSE ROUND(SUM(learning_time_seconds) / 3600 / SUM(is_engaged), 1)
END AS average_session_length
FROM fact_enrollment_engagement_day_admin_dash
WHERE enterprise_customer_uuid=%(enterprise_customer_uuid)s AND
(activity_date BETWEEN %(start_date)s AND %(end_date)s) AND
is_engaged = 1 AND
email is NULL
GROUP BY email;
"""

@staticmethod
def get_completion_data_for_leaderboard_null_email_only_query():
"""
Get the query to fetch the completions data for leaderboard for NULL emails.

Query should fetch the completion data for like course completion count of
the enterprise learners to show in the leaderboard.

Returns:
(list<str>): Query to fetch the completions data for leaderboard.
"""
return """
SELECT email, count(course_key) as course_completion_count
FROM fact_enrollment_admin_dash
WHERE enterprise_customer_uuid=%(enterprise_customer_uuid)s AND
(passed_date BETWEEN %(start_date)s AND %(end_date)s) AND
has_passed = 1 AND
email is NULL
GROUP BY email;
"""

@staticmethod
def get_leaderboard_data_count_query():
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from uuid import UUID

from enterprise_data.cache.decorators import cache_it
from enterprise_data.utils import find_first

from ..queries import FactEngagementAdminDashQueries
from ..utils import run_query
Expand Down Expand Up @@ -168,7 +169,14 @@ def get_engagement_time_series_data(self, enterprise_customer_uuid: UUID, start_

@cache_it()
def _get_engagement_data_for_leaderboard(
self, enterprise_customer_uuid: UUID, start_date: date, end_date: date, limit: int, offset: int
self,
enterprise_customer_uuid: UUID,
start_date: date,
end_date: date,
limit: int,
offset: int,
include_null_email: bool,

):
"""
Get the engagement data for leaderboard.
Expand All @@ -182,11 +190,12 @@ def _get_engagement_data_for_leaderboard(
end_date (date): The end date.
limit (int): The maximum number of records to return.
offset (int): The number of records to skip.
include_null_email (bool): If True, only fetch data for NULL emails.

Returns:
list[dict]: The engagement data for leaderboard.
"""
return run_query(
engagements = run_query(
query=self.queries.get_engagement_data_for_leaderboard_query(),
params={
'enterprise_customer_uuid': enterprise_customer_uuid,
Expand All @@ -198,9 +207,27 @@ def _get_engagement_data_for_leaderboard(
as_dict=True,
)

if include_null_email:
engagement_for_null_email = run_query(
query=self.queries.get_engagement_data_for_leaderboard_null_email_only_query(),
params={
'enterprise_customer_uuid': enterprise_customer_uuid,
'start_date': start_date,
'end_date': end_date,
},
as_dict=True,
)
engagements += engagement_for_null_email
return engagements

@cache_it()
def _get_completion_data_for_leaderboard_query(
self, enterprise_customer_uuid: UUID, start_date: date, end_date: date, email_list: list
self,
enterprise_customer_uuid: UUID,
start_date: date,
end_date: date,
email_list: list,
include_null_email: bool,
):
"""
Get the completion data for leaderboard.
Expand All @@ -213,11 +240,13 @@ def _get_completion_data_for_leaderboard_query(
start_date (date): The start date.
end_date (date): The end date.
email_list (list<str>): List of emails of the enterprise learners.
include_null_email (bool): If True, only fetch data for NULL emails.

Returns:
list[dict]: The engagement data for leaderboard.
"""
return run_query(

completions = run_query(
query=self.queries.get_completion_data_for_leaderboard_query(email_list),
params={
'enterprise_customer_uuid': enterprise_customer_uuid,
Expand All @@ -227,8 +256,28 @@ def _get_completion_data_for_leaderboard_query(
as_dict=True,
)

if include_null_email:
completions_for_null_email = run_query(
query=self.queries.get_completion_data_for_leaderboard_null_email_only_query(),
params={
'enterprise_customer_uuid': enterprise_customer_uuid,
'start_date': start_date,
'end_date': end_date,
},
as_dict=True,
)
completions += completions_for_null_email

return completions

def get_all_leaderboard_data(
self, enterprise_customer_uuid: UUID, start_date: date, end_date: date, limit: int, offset: int
self,
enterprise_customer_uuid: UUID,
start_date: date,
end_date: date,
limit: int,
offset: int,
total_count: int,
):
"""
Get the leaderboard data for the given enterprise customer.
Expand All @@ -239,32 +288,48 @@ def get_all_leaderboard_data(
end_date (date): The end date.
limit (int): The maximum number of records to return.
offset (int): The number of records to skip.
total_count (int): The total number of records.

Returns:
list[dict]: The leaderboard data.
"""
include_null_email = False
# If this is the last or only page, we need to include NULL emails record.
if total_count <= offset + limit:
include_null_email = True

engagement_data = self._get_engagement_data_for_leaderboard(
enterprise_customer_uuid=enterprise_customer_uuid,
start_date=start_date,
end_date=end_date,
limit=limit,
offset=offset,
include_null_email=include_null_email,
)
# If there is no data, no need to proceed.
if not engagement_data:
return []

engagement_data_dict = {engagement['email']: engagement for engagement in engagement_data}
engagement_data_dict = {
engagement['email']: engagement for engagement in engagement_data if engagement['email']
}
completion_data = self._get_completion_data_for_leaderboard_query(
enterprise_customer_uuid=enterprise_customer_uuid,
start_date=start_date,
end_date=end_date,
email_list=list(engagement_data_dict.keys()),
include_null_email=include_null_email,
)
for completion in completion_data:
email = completion['email']
engagement_data_dict[email]['course_completion_count'] = completion['course_completion_count']

if include_null_email:
engagement_data_dict['None'] = find_first(engagement_data, lambda x: x['email'] is None) or {}
completion = find_first(completion_data, lambda x: x['email'] is None) or \
{'course_completion_count': 'Unknown'}
engagement_data_dict['None']['course_completion_count'] = completion['course_completion_count']

return list(engagement_data_dict.values())

@cache_it()
Expand Down
10 changes: 6 additions & 4 deletions enterprise_data/api/v1/views/analytics_leaderboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,18 @@ def list(self, request, enterprise_uuid):
end_date = serializer.data.get('end_date', date.today())
page = serializer.data.get('page', 1)
page_size = serializer.data.get('page_size', 100)
leaderboard = FactEngagementAdminDashTable().get_all_leaderboard_data(
total_count = FactEngagementAdminDashTable().get_leaderboard_data_count(
enterprise_customer_uuid=enterprise_uuid,
start_date=start_date,
end_date=end_date,
limit=page_size,
offset=(page - 1) * page_size,
)
total_count = FactEngagementAdminDashTable().get_leaderboard_data_count(
leaderboard = FactEngagementAdminDashTable().get_all_leaderboard_data(
enterprise_customer_uuid=enterprise_uuid,
start_date=start_date,
end_date=end_date,
limit=page_size,
offset=(page - 1) * page_size,
total_count=total_count,
)
response_type = request.query_params.get('response_type', ResponseType.JSON.value)

Expand Down Expand Up @@ -102,6 +103,7 @@ def _stream_serialized_data(enterprise_uuid, start_date, end_date, total_count,
end_date=end_date,
limit=page_size,
offset=offset,
total_count=total_count,
)
yield from leaderboard
offset += page_size
Original file line number Diff line number Diff line change
Expand Up @@ -161,17 +161,18 @@ def __cache_engagement_data(enterprise_customer_uuid):
start_date=start_date,
end_date=end_date,
)
enterprise_engagement_table.get_all_leaderboard_data(
total_count = enterprise_engagement_table.get_leaderboard_data_count(
enterprise_customer_uuid=enterprise_customer_uuid,
start_date=start_date,
end_date=end_date,
limit=page_size,
offset=0,
)
enterprise_engagement_table.get_leaderboard_data_count(
enterprise_engagement_table.get_all_leaderboard_data(
enterprise_customer_uuid=enterprise_customer_uuid,
start_date=start_date,
end_date=end_date,
limit=page_size,
offset=0,
total_count=total_count,
)

@staticmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,18 @@ def setUp(self):
get_enrollment_date_range_patcher.start()
self.addCleanup(get_enrollment_date_range_patcher.stop)

@patch('enterprise_data.admin_analytics.database.tables.fact_engagement_admin_dash.run_query', MagicMock())
@patch('enterprise_data.admin_analytics.database.tables.fact_enrollment_admin_dash.run_query', MagicMock())
@patch('enterprise_data.admin_analytics.database.tables.skills_daily_rollup_admin_dash.run_query', MagicMock())
@patch(
'enterprise_data.admin_analytics.database.tables.fact_engagement_admin_dash.run_query',
MagicMock(return_value=[])
)
@patch(
'enterprise_data.admin_analytics.database.tables.fact_enrollment_admin_dash.run_query',
MagicMock(return_value=[])
)
@patch(
'enterprise_data.admin_analytics.database.tables.skills_daily_rollup_admin_dash.run_query',
MagicMock(return_value=[])
)
@patch('enterprise_data.api.v1.views.analytics_enrollments.FactEnrollmentAdminDashTable.get_top_enterprises')
@patch('enterprise_data.cache.decorators.cache.set')
@patch('enterprise_data.cache.decorators.cache.get')
Expand All @@ -47,5 +56,5 @@ def test_pre_warm_analytics_cache(self, mock_get_cache, mock_set_cache, mock_get

call_command('pre_warm_analytics_cache')

assert mock_get_cache.call_count == 24
assert mock_set_cache.call_count == 24
assert mock_get_cache.call_count == 23
assert mock_set_cache.call_count == 23
17 changes: 17 additions & 0 deletions enterprise_data/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,20 @@ def primary_subject_truncate(x):
return x
else:
return "other"


def find_first(iterable, condition):
"""
Find the first item in an iterable that satisfies the condition.
Arguments:
iterable (iterable): The iterable to search.
condition (function): The condition to satisfy.
Returns:
The first item that satisfies the condition, or None if no item satisfies the condition.
"""
try:
return next(item for item in iterable if condition(item))
except StopIteration:
return None
2 changes: 1 addition & 1 deletion enterprise_data_roles/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,5 @@ def request_user_has_explicit_access(*args, **kwargs):

rules.add_perm(
'can_access_enterprise',
request_user_has_implicit_access | request_user_has_explicit_access # pylint: disable=unsupported-binary-operation
request_user_has_implicit_access | request_user_has_explicit_access
)
Loading