fix: issues related to course_id#109
Conversation
Faraz32123
force-pushed
the
fix/course_id_issues
branch
2 times, most recently
from
950bebd to
6197fbd
Compare
- bypass searching course_id by thread/comment in mongo and then search it in mysql - convert course_id(str) into CourseKey before passing it to CourseWaffleFlag's is_enabled method as it expects CourseKey
Faraz32123
force-pushed
the
fix/course_id_issues
branch
from
6197fbd to
52fa5d1
Compare
| thread = CommentThread().get(thread_id) | ||
| if thread: | ||
| return thread.get("course_id") | ||
| except bason_errors.InvalidId: |
There was a problem hiding this comment.
In which case(s) is this exception happening?
There was a problem hiding this comment.
if we have id whose type is not objectId. Like we try to search thread in mongo first and then in mysql.
So in case of id from mysql, when it tries to find thread with id 1 in mongo, the error raises.
There was a problem hiding this comment.
Got it! Please add a comment :)
| try: | ||
| thread = CommentThread().get(thread_id) | ||
| if thread: | ||
| return thread.get("course_id") |
There was a problem hiding this comment.
Shouldn't this be: return thread["course_id"]? (likewise below)
There was a problem hiding this comment.
In case of standalone threads, thread don't have course_id.
Just to avoid key error and returning None. Although we don't have standalone thread now as far as I know.
There was a problem hiding this comment.
Please add a comment to explain that. (and similar below)
Faraz32123
force-pushed
the
fix/course_id_issues
branch
from
a3f2b47 to
6724b1b
Compare
| # pylint: disable=C0415,E0401 | ||
| from opaque_keys.edx.locator import CourseKey # type: ignore[import-not-found] | ||
|
|
||
| if isinstance(course_id, str): | ||
| course_id = CourseKey.from_string(course_id) |
There was a problem hiding this comment.
can we move opaquekey import to try block or simply remove try-except?
There was a problem hiding this comment.
Done, moved the import to the try-except block.
3 participants
Uh oh!
There was an error while loading. Please reload this page.