Skip to content

Release 0.67.1 #3328

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.

Sign up for GitHub

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

Merged
odlbot merged 5 commits into from
May 11, 2026
Merged

Release 0.67.1 #3328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
52548c0
dashboard refactor phase 1 (#3320)
gumaerc May 11, 2026
587a45b
feat: added welcome to learner email once he login for the first time…
zamanafzal May 11, 2026
62a2ef1
fix: 404 on product pages when live=false (#3295)
daniellefrappier18 May 11, 2026
32ace1b
Merge branch 'release'
odlbot May 11, 2026
2ecabc5
Release 0.67.1
odlbot May 11, 2026
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
7 changes: 7 additions & 0 deletions RELEASE.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
Release Notes
=============

Version 0.67.1
--------------

- fix: 404 on product pages when live=false (#3295)
- feat: added welcome to learner email once he login for the first time (#3324)
- dashboard refactor phase 1 (#3320)

Version 0.67.0 (Released May 11, 2026)
--------------

Expand Down
11 changes: 1 addition & 10 deletions authentication/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,8 @@ def create_user(username, email, profile_data=None, user_extra=None):
defaults.update({"username": username})

with transaction.atomic():
user, created = User.objects.get_or_create(email=email, defaults=defaults)

user, _ = User.objects.get_or_create(email=email, defaults=defaults)
profile_api.ensure_profile(user, profile_data=profile_data)
if created:
transaction.on_commit(
lambda: user_created_actions(
user=user,
is_new=True,
details=profile_data or {},
)
)

return user

Expand Down
29 changes: 0 additions & 29 deletions authentication/api_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,35 +37,6 @@ def test_create_user(profile_data):
assert user.profile.name is None


@pytest.mark.django_db(transaction=True)
def test_create_user_triggers_plugins_for_new_users(mocker):
"""create_user should trigger user_created plugins for brand new users."""
mock_pm = mocker.Mock()
mocker.patch("authentication.api.get_plugin_manager", return_value=mock_pm)

user = api.create_user("new-user", "new@localhost", {"name": "New User"})

mock_pm.hook.user_created.assert_called_once_with(
user=user,
user_data={"profile": {"name": "New User"}},
)


@pytest.mark.django_db(transaction=True)
def test_create_user_does_not_retrigger_plugins_for_existing_users(mocker):
"""create_user should not trigger user_created plugins for existing users."""
user = UserFactory.create(email="existing@localhost")
mock_pm = mocker.Mock()
mocker.patch("authentication.api.get_plugin_manager", return_value=mock_pm)

resolved = api.create_user(
"another-username", user.email, {"name": "Existing User"}
)

assert resolved.id == user.id
mock_pm.hook.user_created.assert_not_called()


@pytest.mark.parametrize(
"mock_method",
["profiles.api.ensure_profile"],
Expand Down
2 changes: 2 additions & 0 deletions authentication/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.views import View

from main.middleware.apisix_user import ApisixUserMiddleware, decode_apisix_headers
from profiles.tasks import send_welcome_email

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -100,6 +101,7 @@ def get(
redirect_url = f"{settings.MITOL_NEW_USER_LOGIN_URL}?{params}"
profile.save()

send_welcome_email.delay(request.user.id)
profile.has_logged_in = True
profile.save()
Comment on lines +104 to 106
Copy link
Copy Markdown

@sentry sentry Bot May 11, 2026

Choose a reason for hiding this comment

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

Bug: A failure during profile.save() after dispatching the send_welcome_email task could cause multiple welcome emails to be sent to a user on subsequent logins.
Severity: MEDIUM

Suggested Fix

Move the send_welcome_email.delay() call to after the profile.save() call has successfully completed. Alternatively, wrap the state change and the task dispatch in a database transaction using transaction.on_commit to ensure the task is only enqueued if the database write is successful. Adding an idempotency check within the send_welcome_email task would also prevent duplicates.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: authentication/views.py#L104-L106

Potential issue: The `send_welcome_email.delay()` Celery task is dispatched before the
database transaction that sets `profile.has_logged_in = True` is committed.
Specifically, the task is queued at line 104, but the `profile.save()` call that
persists the state change is at line 106. If `profile.save()` fails due to a transient
database error, the `has_logged_in` flag remains `False`. However, the welcome email
task has already been enqueued. On every subsequent login attempt, the `if not
profile.has_logged_in` condition will evaluate to true, causing another welcome email to
be dispatched until the save operation finally succeeds. The `send_welcome_email` task
lacks idempotency checks, so it sends an email every time it is invoked.

Did we get this right? 👍 / 👎 to inform future reviews.


Expand Down
20 changes: 20 additions & 0 deletions authentication/views_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ def test_custom_login_view_authenticated_user_needs_onboarding(
"authentication.views.settings.MITOL_NEW_USER_LOGIN_URL", "/onboarding"
)
mocker.patch("authentication.views.decode_apisix_headers", return_value={})
mock_send_welcome_email = mocker.patch(
"authentication.views.send_welcome_email.delay"
)

response = CustomLoginView().get(request)

Expand All @@ -188,6 +191,7 @@ def test_custom_login_view_authenticated_user_needs_onboarding(
assert response.url == f"/onboarding?{urlencode({'next': expected_redirect})}"
else:
assert response.url == expected_redirect
mock_send_welcome_email.assert_called_once_with(request.user.id)


def test_custom_login_view_authenticated_user_who_has_logged_in_before(mocker):
Expand All @@ -199,11 +203,15 @@ def test_custom_login_view_authenticated_user_who_has_logged_in_before(mocker):
)
request.user = MagicMock(is_anonymous=False)
request.user.profile = MagicMock(has_logged_in=True)
mock_send_welcome_email = mocker.patch(
"authentication.views.send_welcome_email.delay"
)

response = CustomLoginView().get(request)

assert response.status_code == 302
assert response.url == "/should-be-redirect?foo"
mock_send_welcome_email.assert_not_called()


def test_custom_login_view_anonymous_user(mocker):
Expand Down Expand Up @@ -231,9 +239,13 @@ def test_custom_login_view_first_time_login_sets_has_logged_in(mocker):

mock_user = MagicMock()
mock_user.is_anonymous = False
mock_user.id = 123
mock_user.profile = mock_profile

request.user = mock_user
mock_send_welcome_email = mocker.patch(
"authentication.views.send_welcome_email.delay"
)

response = CustomLoginView().get(request)

Expand All @@ -243,6 +255,7 @@ def test_custom_login_view_first_time_login_sets_has_logged_in(mocker):
# Verify that has_logged_in was set to True and profile was saved
assert mock_profile.has_logged_in is True
mock_profile.save.assert_called_once()
mock_send_welcome_email.assert_called_once_with(mock_user.id)


class LoginOrgUserRedirectParams(NamedTuple):
Expand Down Expand Up @@ -286,6 +299,9 @@ def test_login_org_user_redirect(
# Set up user profile based on test scenario
user.profile.has_logged_in = has_logged_in
user.profile.save()
mock_send_welcome_email = mocker.patch(
"authentication.views.send_welcome_email.delay"
)

header_str = b64encode(
json.dumps(
Expand Down Expand Up @@ -319,3 +335,7 @@ def test_login_org_user_redirect(

user.profile.refresh_from_db()
assert user.profile.has_logged_in is True
if has_logged_in:
mock_send_welcome_email.assert_not_called()
else:
mock_send_welcome_email.assert_called_once_with(user.id)
Loading
Loading