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: add register intent prop for login segment call #34891

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

ahtesham-quraish
Copy link
Contributor

Description

Add register intent property on login successful in segment call. The reason for adding this is to know how many users have landed on register page and then move to login page to get login.

Supporting information

Jira VAN-1929

Testing instructions

Unit test has been updated

@ahtesham-quraish ahtesham-quraish requested a review from a team as a code owner June 3, 2024 07:55
@@ -1041,6 +1041,7 @@ class LoginSessionViewTest(ApiTestCase, OpenEdxEventsTestMixin):
USERNAME = "bob"
EMAIL = "[email protected]"
PASSWORD = "password"
REGISTER_INTENT = True
Copy link
Contributor

Choose a reason for hiding this comment

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

its not some bool. please check it from request in tests as well as you are doing actually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its changed

@@ -360,7 +360,8 @@ def _track_user_login(user, request):
{
'category': "conversion",
'label': request.POST.get('course_id'),
'provider': None
'provider': None,
'register_intent': bool(request.POST.get('register_intent')),
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be always true if there would be any value in register_intent attribute.
Like:

bool('False')
--> True

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its changed

Description:
Add register intent property for login successful segement call
VAN-1929
@ahtesham-quraish ahtesham-quraish merged commit a2aa6bd into master Jun 4, 2024
46 checks passed
@ahtesham-quraish ahtesham-quraish deleted the ahtesham/van-1929 branch June 4, 2024 13:43
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

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.

4 participants