Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions RELEASE.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
Release Notes
=============

Version 0.66.11
---------------

- fix flaky test (#3319)
- video ui tweaks (#3322)
- fix: change the video detail page route & load more videos feature on video collection page (#3316)
- feat: added module section on course product page (#3242)
- translations UI design revisions (#3314)
- refactor ocw video etl (#3308)
- fix: update metadata for video pages (#3310)
- Both ETL and webhook should populate OVS resource_category when applicable. (#3313)
- feat: Welcome to learn email on new account setup (#3189)

Version 0.66.10 (Released May 07, 2026)
---------------

Expand Down
10 changes: 9 additions & 1 deletion authentication/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,17 @@ def create_user(username, email, profile_data=None, user_extra=None):
defaults.update({"username": username})

with transaction.atomic():
user, _ = User.objects.get_or_create(email=email, defaults=defaults)
user, created = 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 {},
)
)
Comment on lines +36 to +46
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The create_user function calls ensure_profile twice for new users: once directly and again via an on_commit hook, causing a redundant database write.
Severity: LOW

Suggested Fix

Remove the direct, synchronous call to profile_api.ensure_profile(user, profile_data=profile_data) from the create_user function. The profile creation should be handled exclusively by the on_commit hook (user_created_actions) to ensure it is only called once after the user is successfully saved to the database.

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/api.py#L36-L46

Potential issue: When a new user is created via the `create_user` function, the
`ensure_profile` function is invoked twice. The first call happens synchronously within
the function body. The second call is triggered asynchronously after the database
transaction commits, via an `on_commit` hook that executes `user_created_actions`. While
the `ensure_profile` function is idempotent due to its use of `update_or_create`, this
redundant call results in an unnecessary database write operation (a create followed by
an update) for every new user. This is a logic error that introduces inefficiency.

Also affects:

  • profiles/plugins.py:12~21
  • profiles/api.py:13~22

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


return user

Expand Down
29 changes: 29 additions & 0 deletions authentication/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,35 @@ 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
10 changes: 10 additions & 0 deletions frontends/api/src/mitxonline/hooks/courses/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
import { coursesQueries } from "./queries"
import type {
CourseOutlineModule,
CourseOutlineModuleCounts,
CourseOutlineResponse,
} from "./queries"

export { coursesQueries }
export type {
CourseOutlineModule,
CourseOutlineModuleCounts,
CourseOutlineResponse,
}
24 changes: 24 additions & 0 deletions frontends/api/src/mitxonline/hooks/courses/queries.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
import { queryOptions } from "@tanstack/react-query"
import type {
CourseOutlineResponse as GeneratedCourseOutlineResponse,
CoursesApiApiV2CoursesListRequest,
PaginatedCourseWithCourseRunsSerializerV2List,
} from "@mitodl/mitxonline-api-axios/v2"
import { coursesApi } from "../../clients"

type CourseOutlineResponse = GeneratedCourseOutlineResponse
type CourseOutlineModule = CourseOutlineResponse["modules"][number]
type CourseOutlineModuleCounts = CourseOutlineModule["counts"]

const coursesKeys = {
root: ["mitxonline", "courses"],
coursesList: (opts?: CoursesApiApiV2CoursesListRequest) => [
...coursesKeys.root,
"list",
opts,
],
courseOutline: (coursewareId: string) => [
...coursesKeys.root,
"outline",
coursewareId,
],
}

const coursesQueries = {
Expand All @@ -23,6 +33,20 @@ const coursesQueries = {
return coursesApi.apiV2CoursesList(opts).then((res) => res.data)
},
}),
courseOutline: (coursewareId: string) =>
queryOptions({
queryKey: coursesKeys.courseOutline(coursewareId),
queryFn: async (): Promise<CourseOutlineResponse> => {
return coursesApi
.courseOutlineRetrieveV3({ course_id: coursewareId })
.then((res) => res.data)
},
}),
}

export { coursesQueries, coursesKeys }
export type {
CourseOutlineResponse,
CourseOutlineModule,
CourseOutlineModuleCounts,
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ const transactionLine = (
quantity: 1,
CEUs: "0.0",
content_title: faker.company.catchPhrase(),
content_type: "",
readable_id: `course-v1:MITxT+${faker.string.alphanumeric(6)}`,
start_date: faker.date.past().toISOString(),
end_date: faker.date.future().toISOString(),
total_paid: faker.commerce.price({ min: 50, max: 500 }),
discount: "0.00",
price: faker.commerce.price({ min: 50, max: 500 }),
content_type: "",
...overrides,
})

Expand Down
2 changes: 2 additions & 0 deletions frontends/api/src/mitxonline/test-utils/urls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ const programCollections = {
const courses = {
coursesList: (opts?: CoursesApiApiV2CoursesListRequest) =>
`${API_BASE_URL}/api/v2/courses/${queryify(opts, { explode: false })}`,
courseOutline: (coursewareId: string) =>
`${API_BASE_URL}/api/v3/courses/${encodeURIComponent(coursewareId)}/ol_openedx_outline/`,
}

const pages = {
Expand Down
6 changes: 6 additions & 0 deletions frontends/main/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ const nextConfig = {
},
async redirects() {
return [
{
// can be removed once fastly redirect is in place
source: "/video-playlist/detail/:id",
destination: "/video/:id",
permanent: true,
},
{
// can be removed once fastly redirect is in place
source: "/attach/:code",
Expand Down
Loading
Loading