-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
feat: expose functionality to extract unit content #37
Conversation
return None | ||
|
||
|
||
def _leaf_filter(block): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a custom leaf filter to exclude blocks that are not video or html, that way we limit any calculations/processing on nodes that aren't relevant to what we need.
length - the cummulative length of a block's children's content | ||
items - a list of dictionaries containing the content type and text for each child | ||
""" | ||
block = get_single_block(request, user_id, course_id, unit_usage_key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm only using load_single_xblock
once here on the parent block. I wasn't sure if I need to do that on each child block to check visibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to, no. load_single_xblock
should only return the children that the learner has access to, so long as will_recheck_access=False
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, thank you!
4d5653e
to
b2b8e6e
Compare
return length, items | ||
|
||
|
||
def get_block_content(request, user_id, course_id, unit_usage_key): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for my own understanding, what calls this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function will be used as part of https://2u-internal.atlassian.net/browse/COSMO-73 - essentially whatever code is putting together the prompt will call it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very easy to understand, so I appreciate the thought and care you took in putting this together.
return transcript | ||
|
||
|
||
def get_single_block(request, user_id, course_id, usage_key_string, course=None, will_recheck_access=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we'll ever want this to be True
because then we won't be checking learner access to the block and its children. I think it would be better not to expose this function parameter in that case, since we don't want someone to accidentally pass True
. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's a great point! I'll update this function.
length - the cummulative length of a block's children's content | ||
items - a list of dictionaries containing the content type and text for each child | ||
""" | ||
block = get_single_block(request, user_id, course_id, unit_usage_key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to, no. load_single_xblock
should only return the children that the learner has access to, so long as will_recheck_access=False
.
""" | ||
Given a specific block, return the content type and text of a pre-order traversal of the blocks children. | ||
""" | ||
leaf_nodes = traverse_block_pre_order(block, block_get_children, _leaf_filter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is slick!
self.course_id = 'course-v1:edx+test+23' | ||
|
||
@ddt.data( | ||
('video', True), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include html
here for completeness?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thanks for catching that!
b2b8e6e
to
69f8945
Compare
69f8945
to
285e0e2
Compare
COSMO-70
In order to provide unit specific content to a prompt template, the content needs to be extracted from a given block. Pieces of this code have been taken from the ai_aside repository, specifically the
text_utils.py
file.Content from a block is retrieved using a pre-order traversal, meaning that all content will be returned in the order a learner would be viewing the content.