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

feat: can_redeem checks for enrollment deadline #538

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

iloveagent57
Copy link
Contributor

@iloveagent57 iloveagent57 commented Aug 14, 2024

Description:
We want the can_redeem query to return false for a given (user, content key)
if the enrollment deadline for the content key is in the past. This is necessary because
some courses have runs with an advertised run for which the deadline is in the future,
but also have available recent runs where the upgrade deadline is in the past.

Depends on openedx/enterprise-catalog#909 and openedx/enterprise-subsidy#286

A can_redeem false payload for a passed enroll by date will have a reasons value like this:

"reasons": [
            {
                "reason": "beyond_enrollment_deadline",
                "user_message": "You can't enroll right now because the enrollment deadline for this course has passed.",
                "policy_uuids": [
                    "3cc0ad2c-ede1-476c-8271-97683855e41d"
                ],
                "metadata": {
                    "enterprise_administrators": [
                        {
                            "email": "enterprise_admin_test-enterprise@example.com",
                            "lms_user_id": 145
                        }
                    ]
                }
            }
        ]

Jira:
ENT-7472

Merge checklist:

  • ./manage.py makemigrations has been run
    • Note: This must be run if you modified any models.
      • It may or may not make a migration depending on exactly what you modified, but it should still be run.

Post merge:

  • Ensure that your changes went out to the stage instance
  • Deploy to prod instance

@iloveagent57 iloveagent57 changed the title feat: [wip] can_redeem checks for enrollment deadline feat: can_redeem checks for enrollment deadline Aug 14, 2024
@iloveagent57 iloveagent57 force-pushed the aed/can-redeem-enroll-by-date branch 7 times, most recently from bfd9b56 to e067bb1 Compare August 15, 2024 14:49
@@ -768,7 +770,7 @@ def can_redeem(self, request, enterprise_customer_uuid):
enterprise_customer_uuid, lms_user_id, content_key
)

if not redemptions and not redeemable_policies:
if not successful_redemptions and not redeemable_policies:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In local testing, I noticed I wasn't getting any reasons in my test responses even when I can an expected can_redeem=False result. I tend to use reversals a lot to set up test state, so I have many unsuccessful redemptions.

Copy link
Member

Choose a reason for hiding this comment

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

This seems reasonable!

We want the `can_redeem` query to return false for a given (user, content key)
if the enrollment deadline for the content key is in the past. This is necessary because
some courses have runs with an advertised run for which the deadline is in the future,
but also have available recent runs where the upgrade deadline is in the past.

ENT-7472
@@ -768,7 +770,7 @@ def can_redeem(self, request, enterprise_customer_uuid):
enterprise_customer_uuid, lms_user_id, content_key
)

if not redemptions and not redeemable_policies:
if not successful_redemptions and not redeemable_policies:
Copy link
Member

Choose a reason for hiding this comment

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

This seems reasonable!

@iloveagent57 iloveagent57 merged commit 61dc1f9 into main Aug 20, 2024
3 checks passed
@iloveagent57 iloveagent57 deleted the aed/can-redeem-enroll-by-date branch August 20, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants