Skip to content

fix: commentable_ids should be list of ids rather then a comma separated string#811

Closed
xitij2000 wants to merge 1 commit intoopencraft-release/sumac.1from
kshitij/fix-forum
Closed

fix: commentable_ids should be list of ids rather then a comma separated string#811
xitij2000 wants to merge 1 commit intoopencraft-release/sumac.1from
kshitij/fix-forum

Conversation

@xitij2000
Copy link
Copy Markdown
Member

@xitij2000 xitij2000 commented Jan 13, 2026

This along with upgrading the openedx-forum package to v0.3.2 or above will fix the issue where in-context discussions are not filtered to the current unit.

note: this can be dropped when upgrading to teak

Testing instructions:

  • Create a tutor Sumac environment .
  • Create new discussion topics under different units.
  • They should all show up when you navigate to any unit.
  • Switch the sumac environment to this branch and upgrade the openedx-forum package to v0.3.2
  • Check that the discussion topics in each unit correspond to the current unit.

private ref: https://tasks.opencraft.com/browse/BB-10387

…ted string

note: this can be dropped when upgrading to teak
@xitij2000 xitij2000 marked this pull request as ready for review January 14, 2026 05:15
Copy link
Copy Markdown
Member

@farhaanbukhsh farhaanbukhsh left a comment

Choose a reason for hiding this comment

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

👍

✅ I tested this on the sandbox

  • Created a post in one unit
  • Tested that it isn't visible in any other unit
    ✅ I read through the code
    ❌ I checked for accessibility issues
    ❌ Includes documentation

@samuelallan72
Copy link
Copy Markdown
Member

@xitij2000 @farhaanbukhsh was there a reason this wasn't merged in the end?

Also, given the comment in the description:

note: this can be dropped when upgrading to teak

I guess we don't need this any more. I'll close this for now, since the linked ticket is closed and this seems no longer relevant.

@xitij2000 xitij2000 deleted the kshitij/fix-forum branch April 6, 2026 07:23
@xitij2000
Copy link
Copy Markdown
Member Author

@samuelallan72 This wasn't merged because the client was using a different branch and fixing it here wasn't important since it was already fixed in teak and most clients were already using that.

@samuelallan72
Copy link
Copy Markdown
Member

Thanks for confirming @xitij2000 :)

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