Skip to content

Commit

Permalink
3783: dedp course certificates in hubspot (#2193)
Browse files Browse the repository at this point in the history
* Hubspot properties upsert working

* sync user cert with hubspot

* Add unit tests

* add doc strings

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Repair test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* ruff changes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* ruff changes

* Add unit tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* ruff

* Sync hubspot props and contact cert save

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* ruff and format

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* maybe fix

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix

* gix

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
collinpreston and pre-commit-ci[bot] authored May 16, 2024
1 parent a7d1c8d commit 8719622
Show file tree
Hide file tree
Showing 12 changed files with 259 additions and 45 deletions.
8 changes: 7 additions & 1 deletion courses/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,8 @@ def process_course_run_grade_certificate(course_run_grade, should_force_create=F
Tuple[ CourseRunCertificate, bool, bool ]: A Tuple containing None or CourseRunCertificate object,
A bool representing if the certificate is created, A bool representing if a certificate is deleted
"""
from hubspot_sync.task_helpers import sync_hubspot_user

user = course_run_grade.user
course_run = course_run_grade.course_run

Expand All @@ -793,19 +795,20 @@ def process_course_run_grade_certificate(course_run_grade, should_force_create=F
delete_count, _ = CourseRunCertificate.objects.filter(
user=user, course_run=course_run
).delete()
sync_hubspot_user(user)
return None, False, (delete_count > 0)

elif should_create:
try:
certificate, created = CourseRunCertificate.objects.get_or_create(
user=user, course_run=course_run
)
sync_hubspot_user(user)
return certificate, created, False # noqa: TRY300
except IntegrityError:
log.warning(
f"IntegrityError caught processing certificate for {course_run.courseware_id} for user {user} - certificate was likely already revoked." # noqa: G004
)

return None, False, False


Expand Down Expand Up @@ -1009,6 +1012,8 @@ def generate_program_certificate(user, program, force_create=False): # noqa: FB
ProgramCertificate (or None if one was not found or created) paired
with a boolean indicating whether the certificate was newly created.
"""
from hubspot_sync.task_helpers import sync_hubspot_user

existing_cert_queryset = ProgramCertificate.objects.filter(
user=user, program=program
)
Expand All @@ -1028,6 +1033,7 @@ def generate_program_certificate(user, program, force_create=False): # noqa: FB
user.username,
program.title,
)
sync_hubspot_user(user)
_, created = ProgramEnrollment.objects.get_or_create(
program=program, user=user, defaults={"active": True, "change_status": None}
)
Expand Down
39 changes: 32 additions & 7 deletions courses/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1102,10 +1102,14 @@ def test_course_run_certificate( # noqa: PLR0913
exp_certificate,
exp_created,
exp_deleted,
mocker,
):
"""
Test that the certificate is generated correctly
"""
patched_sync_hubspot_user = mocker.patch(
"hubspot_sync.task_helpers.sync_hubspot_user",
)
passed_grade_with_enrollment.grade = grade
passed_grade_with_enrollment.passed = passed
if not paid:
Expand All @@ -1117,16 +1121,20 @@ def test_course_run_certificate( # noqa: PLR0913
certificate, created, deleted = process_course_run_grade_certificate(
passed_grade_with_enrollment
)
if created:
patched_sync_hubspot_user.assert_called_once_with(user)
assert bool(certificate) is exp_certificate
assert created is exp_created
assert deleted is exp_deleted


def test_course_run_certificate_idempotent(passed_grade_with_enrollment):
def test_course_run_certificate_idempotent(passed_grade_with_enrollment, mocker, user):
"""
Test that the certificate generation is idempotent
"""

patched_sync_hubspot_user = mocker.patch(
"hubspot_sync.task_helpers.sync_hubspot_user",
)
# Certificate is created the first time
certificate, created, deleted = process_course_run_grade_certificate(
passed_grade_with_enrollment
Expand All @@ -1135,6 +1143,8 @@ def test_course_run_certificate_idempotent(passed_grade_with_enrollment):
assert created
assert not deleted

patched_sync_hubspot_user.assert_called_once_with(user)

# Existing certificate is simply returned without any create/delete
certificate, created, deleted = process_course_run_grade_certificate(
passed_grade_with_enrollment
Expand All @@ -1148,7 +1158,6 @@ def test_course_run_certificate_not_passing(passed_grade_with_enrollment):
"""
Test that the certificate is not generated if the grade is set to not passed
"""

# Initially the certificate is created
certificate, created, deleted = process_course_run_grade_certificate(
passed_grade_with_enrollment
Expand Down Expand Up @@ -1425,10 +1434,13 @@ def test_generate_program_certificate_failure_not_all_passed(
assert len(ProgramCertificate.objects.all()) == 0


def test_generate_program_certificate_success_single_requirement_course(user):
def test_generate_program_certificate_success_single_requirement_course(user, mocker):
"""
Test that generate_program_certificate generates a program certificate for a Program with a single required Course.
"""
patched_sync_hubspot_user = mocker.patch(
"hubspot_sync.task_helpers.sync_hubspot_user",
)
course = CourseFactory.create()
program = ProgramFactory.create()
ProgramRequirementFactory.add_root(program)
Expand All @@ -1449,12 +1461,16 @@ def test_generate_program_certificate_success_single_requirement_course(user):
assert created is True
assert isinstance(certificate, ProgramCertificate)
assert len(ProgramCertificate.objects.all()) == 1
patched_sync_hubspot_user.assert_called_once_with(user)


def test_generate_program_certificate_success_multiple_required_courses(user):
def test_generate_program_certificate_success_multiple_required_courses(user, mocker):
"""
Test that generate_program_certificate generate a program certificate
"""
patched_sync_hubspot_user = mocker.patch(
"hubspot_sync.task_helpers.sync_hubspot_user",
)
courses = CourseFactory.create_batch(3)
program = ProgramFactory.create()
ProgramRequirementFactory.add_root(program)
Expand All @@ -1476,11 +1492,12 @@ def test_generate_program_certificate_success_multiple_required_courses(user):
assert created is True
assert isinstance(certificate, ProgramCertificate)
assert len(ProgramCertificate.objects.all()) == 1
patched_sync_hubspot_user.assert_called_once_with(user)


def test_generate_program_certificate_success_minimum_electives_not_met(user):
"""
Test that generate_program_certificate generate a program certificate
Test that generate_program_certificate does not generate a program certificate if minimum electives have not been met.
"""
courses = CourseFactory.create_batch(3)

Expand Down Expand Up @@ -1524,11 +1541,18 @@ def test_generate_program_certificate_success_minimum_electives_not_met(user):
assert len(ProgramCertificate.objects.all()) == 0


def test_force_generate_program_certificate_success(user, program_with_requirements): # noqa: F811
def test_force_generate_program_certificate_success(
user,
program_with_requirements, # noqa: F811
mocker,
):
"""
Test that force creating a program certificate with generate_program_certificate generates
a program certificate without matching program certificate requirements.
"""
patched_sync_hubspot_user = mocker.patch(
"hubspot_sync.task_helpers.sync_hubspot_user",
)
courses = CourseFactory.create_batch(3)
course_runs = CourseRunFactory.create_batch(3, course=factory.Iterator(courses))
CourseRunCertificateFactory.create_batch(
Expand All @@ -1545,6 +1569,7 @@ def test_force_generate_program_certificate_success(user, program_with_requireme
assert created is True
assert isinstance(certificate, ProgramCertificate)
assert len(ProgramCertificate.objects.all()) == 1
patched_sync_hubspot_user.assert_called_once_with(user)


def test_generate_program_certificate_already_exist(
Expand Down
3 changes: 2 additions & 1 deletion courses/management/commands/import_courserun.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,4 +275,5 @@ def handle(self, *args, **kwargs): # pylint: disable=unused-argument # noqa: C
)
)

self.stdout.write(self.style.SUCCESS(f"{success_count} course runs created")) # noqa: RET503
self.stdout.write(self.style.SUCCESS(f"{success_count} course runs created"))
return None
11 changes: 10 additions & 1 deletion courses/management/commands/test_manage_certificate.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,11 @@ def test_certificate_management_revoke_unrevoke_invalid_args(
(None, True),
],
)
def test_certificate_management_revoke_unrevoke_success(user, revoke, unrevoke):
def test_certificate_management_revoke_unrevoke_success(user, revoke, unrevoke, mocker):
"""Test that certificate revoke, un-revoke work as expected and manage the certificate access properly"""
mocker.patch(
"hubspot_sync.management.commands.configure_hubspot_properties._upsert_custom_properties",
)
course_run = CourseRunFactory.create()
certificate = CourseRunCertificateFactory(
course_run=course_run,
Expand All @@ -146,6 +149,9 @@ def test_certificate_management_create(mocker, user, edx_grade_json, revoked):
"""Test that create operation for certificate management command creates the certificates for a single user
when a user is provided
"""
mocker.patch(
"hubspot_sync.management.commands.configure_hubspot_properties._upsert_custom_properties",
)
edx_grade = CurrentGrade(edx_grade_json)
course_run = CourseRunFactory.create()
CourseRunEnrollmentFactory.create(
Expand Down Expand Up @@ -185,6 +191,9 @@ def test_certificate_management_create_no_user(mocker, edx_grade_json, user):
"""Test that create operation for certificate management command attempts to creates the certificates for all the
enrolled users in a run when no user is provided
"""
mocker.patch(
"hubspot_sync.management.commands.configure_hubspot_properties._upsert_custom_properties",
)
passed_edx_grade = CurrentGrade(edx_grade_json)
course_run = CourseRunFactory.create()
users = UserFactory.create_batch(4)
Expand Down
18 changes: 16 additions & 2 deletions courses/management/commands/test_manage_program_certificate.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,18 @@ def test_program_certificate_management_revoke_unrevoke_success(user, revoke, un
assert certificate.is_revoked is (False if unrevoke else True) # noqa: SIM211


def test_program_certificate_management_create(user, program_with_empty_requirements): # noqa: F811
def test_program_certificate_management_create(
user,
program_with_empty_requirements, # noqa: F811
mocker,
):
"""
Test that create operation for program certificate management command
creates the program certificate for a user
"""
mocker.patch(
"hubspot_sync.management.commands.configure_hubspot_properties._upsert_custom_properties",
)
courses = CourseFactory.create_batch(2)
program_with_empty_requirements.add_requirement(courses[0])
program_with_empty_requirements.add_elective(courses[1])
Expand All @@ -145,11 +152,18 @@ def test_program_certificate_management_create(user, program_with_empty_requirem
assert generated_certificates.count() == 1


def test_program_certificate_management_force_create(user, program_with_requirements): # noqa: F811
def test_program_certificate_management_force_create(
user,
program_with_requirements, # noqa: F811
mocker,
):
"""
Test that create operation for program certificate management command
forcefully creates the certificate for a user
"""
mocker.patch(
"hubspot_sync.management.commands.configure_hubspot_properties._upsert_custom_properties",
)
courses = CourseFactory.create_batch(3)
course_runs = CourseRunFactory.create_batch(3, course=factory.Iterator(courses))
CourseRunGradeFactory.create_batch(
Expand Down
11 changes: 10 additions & 1 deletion courses/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Signals for mitxonline course certificates
"""

from django.core.management import call_command
from django.db import transaction
from django.db.models.signals import post_save
from django.dispatch import receiver
Expand All @@ -11,14 +12,20 @@
CourseRunCertificate,
Program,
)
from hubspot_sync.task_helpers import sync_hubspot_user


@receiver(
post_save,
sender=CourseRunCertificate,
dispatch_uid="courseruncertificate_post_save",
)
def handle_create_course_run_certificate(sender, instance, created, **kwargs): # pylint: disable=unused-argument # noqa: ARG001
def handle_create_course_run_certificate(
sender, # pylint: disable=unused-argument # noqa: ARG001
instance,
created,
**kwargs, # pylint: disable=unused-argument # noqa: ARG001
):
"""
When a CourseRunCertificate model is created.
"""
Expand All @@ -34,3 +41,5 @@ def handle_create_course_run_certificate(sender, instance, created, **kwargs):
transaction.on_commit(
lambda: generate_multiple_programs_certificate(user, programs)
)
call_command("configure_hubspot_properties")
sync_hubspot_user(instance)
15 changes: 12 additions & 3 deletions courses/signals_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@
# pylint: disable=unused-argument
@patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback())
@patch("courses.signals.generate_multiple_programs_certificate", autospec=True)
def test_create_course_certificate(generate_program_cert_mock, mock_on_commit):
def test_create_course_certificate(generate_program_cert_mock, mock_on_commit, mocker):
"""
Test that generate_multiple_programs_certificate is called when a course
certificate is created
"""
mocker.patch(
"hubspot_sync.management.commands.configure_hubspot_properties._upsert_custom_properties",
)
user = UserFactory.create()
course_run = CourseRunFactory.create()
program = ProgramFactory.create()
Expand All @@ -38,11 +41,14 @@ def test_create_course_certificate(generate_program_cert_mock, mock_on_commit):
@patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback())
@patch("courses.signals.generate_multiple_programs_certificate", autospec=True)
def test_generate_program_certificate_if_not_live(
generate_program_cert_mock, mock_on_commit
generate_program_cert_mock, mock_on_commit, mocker
):
"""
Test that generate_multiple_programs_certificate is not called when a program is not live
"""
mocker.patch(
"hubspot_sync.management.commands.configure_hubspot_properties._upsert_custom_properties",
)
user = UserFactory.create()
course_run = CourseRunFactory.create()
program = ProgramFactory.create(live=False)
Expand All @@ -57,12 +63,15 @@ def test_generate_program_certificate_if_not_live(
@patch("courses.signals.transaction.on_commit", side_effect=lambda callback: callback())
@patch("courses.signals.generate_multiple_programs_certificate", autospec=True)
def test_generate_program_certificate_not_called(
generate_program_cert_mock, mock_on_commit
generate_program_cert_mock, mock_on_commit, mocker
):
"""
Test that generate_multiple_programs_certificate is not called when a course
is not associated with program.
"""
mocker.patch(
"hubspot_sync.management.commands.configure_hubspot_properties._upsert_custom_properties",
)
user = UserFactory.create()
course = CourseFactory.create()
course_run = CourseRunFactory.create(course=course)
Expand Down
6 changes: 4 additions & 2 deletions hubspot_sync/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def make_contact_sync_message_from_user(user: User) -> SimplePublicObjectInput:
Returns:
SimplePublicObjectInput: Input object for upserting User data to Hubspot
"""
from users.serializers import UserSerializer
from hubspot_sync.serializers import HubspotContactSerializer

contact_properties_map = {
"email": "email",
Expand All @@ -116,8 +116,10 @@ def make_contact_sync_message_from_user(user: User) -> SimplePublicObjectInput:
"type_is_professional": "typeisprofessional",
"type_is_educator": "typeiseducator",
"type_is_other": "typeisother",
"program_certificates": "program_certificates",
"course_run_certificates": "course_run_certificates",
}
properties = UserSerializer(user).data
properties = HubspotContactSerializer(user).data
properties.update(properties.pop("legal_address") or {})
properties.update(properties.pop("user_profile") or {})
hubspot_props = transform_object_properties(properties, contact_properties_map)
Expand Down
Loading

0 comments on commit 8719622

Please sign in to comment.