Skip to content

fix: Fix thread api call to work with teams#269

Open
saraburns1 wants to merge 3 commits intomasterfrom
team_discussion_fix
Open

fix: Fix thread api call to work with teams#269
saraburns1 wants to merge 3 commits intomasterfrom
team_discussion_fix

Conversation

@saraburns1
Copy link
Copy Markdown
Contributor

@saraburns1 saraburns1 commented Mar 26, 2026

This was found during investigation into openedx/openedx-platform#36841

Current functionality - Team posts do not show up at all, even right after creating. Discussion posts are fine.

Screen.Recording.2026-03-26.at.1.16.07.PM.mov

New functionality - Team posts are there (due to sending correct 'context' value to api). Discussion posts remain unchanged.
context is set to 'standalone' here for Teams threads, but the default context in the api when finding threads to display was 'course' which is why nothing was being returned.

Screen.Recording.2026-03-26.at.1.17.47.PM.mov

@saraburns1 saraburns1 marked this pull request as ready for review March 26, 2026 17:25
"user_id": user_id,
"group_id": group_id,
"group_ids": group_ids,
"context": kwargs.get("context", None),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Everything else in here is using an explicit param instead of kwargs, would updating get_user_threads to have an explicit context param make more sense?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i can do that!

Copy link
Copy Markdown

@brian-smith-tcril brian-smith-tcril left a comment

Choose a reason for hiding this comment

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

LGTM! Would probably be good to get someone with more python expertise to take a quick look though.

@saraburns1 saraburns1 requested a review from a team March 30, 2026 18:32
@taimoor-ahmed-1
Copy link
Copy Markdown
Contributor

@saraburns1 changes look good, can we add a unit test for this change?

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.

3 participants