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

refactor: extract completion logic from pipe to implement it into rec… #213

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

andrey-canon
Copy link
Collaborator

@andrey-canon andrey-canon commented Aug 5, 2024

Description

This move the completion logic from the pipeline step to the receiver, this also adds some logs to monitor which actions is triggered the pearson task

Testing instructions

Before

After

Additional information

Include anything else that will help reviewers and consumers understand the change.

  • Does this change depend on other changes elsewhere?
  • Any special concerns or limitations? For example: deprecations, migrations, security, or accessibility.
  • Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.

Checklist for Merge

  • Tested in a remote environment
  • Updated documentation
  • Rebased master/main
  • Squashed commits

@github-actions github-actions bot added admin test size/m m lines label labels Aug 5, 2024
@johanseto johanseto self-assigned this Aug 5, 2024
@johanseto johanseto self-requested a review August 5, 2024 21:19
Copy link
Collaborator

@johanseto johanseto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a logic suggestion. I think test would pass with that change.

Comment on lines 398 to 415
if is_complete and not graded:
LOGGER.info(
"Initializing rti task for the user %s, action triggered by course completion status",
instance.user_id,
)

if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False):
real_time_import_task_v2.delay(
user_id=instance.user_id,
exam_id=str(instance.context_key),
action_name="rti",
)
else:
real_time_import_task.delay(
user_id=instance.user_id,
course_id=str(instance.context_key),
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if is_complete and not graded:
LOGGER.info(
"Initializing rti task for the user %s, action triggered by course completion status",
instance.user_id,
)
if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False):
real_time_import_task_v2.delay(
user_id=instance.user_id,
exam_id=str(instance.context_key),
action_name="rti",
)
else:
real_time_import_task.delay(
user_id=instance.user_id,
course_id=str(instance.context_key),
)
if (not is_complete) or graded:
return
LOGGER.info(
"Initializing rti task for the user %s, action triggered by course completion status",
instance.user_id,
)
if getattr(settings, "USE_PEARSON_ENGINE_SERVICE", False):
real_time_import_task_v2.delay(
user_id=instance.user_id,
exam_id=str(instance.context_key),
action_name="rti",
)
else:
real_time_import_task.delay(
user_id=instance.user_id,
course_id=str(instance.context_key),
)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very similar to my first approach if graded or not is_complete: but I decided to leave as similar as possible in order to make easier the review, anyway I will update that

@andrey-canon andrey-canon force-pushed the refactor_course_completion_case branch from eb1d459 to 59a6900 Compare August 6, 2024 18:49
@johanseto johanseto self-requested a review August 6, 2024 19:24
@andrey-canon andrey-canon merged commit 5871982 into master Aug 6, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants