-
Notifications
You must be signed in to change notification settings - Fork 7
feat: Add a csv report of xblocks used in courses (redwood port) #794
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
base: opencraft-release/redwood.1
Are you sure you want to change the base?
feat: Add a csv report of xblocks used in courses (redwood port) #794
Conversation
Redwood port Private-ref: https://tasks.opencraft.com/browse/BB-10225 (cherry picked from commit 60d9220)
| try: | ||
| task_api.submit_xblocks_list_csv( | ||
| request, | ||
| course_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.
This should be the course id or the course variable right?
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.
Actually no, it should be course_key. In that case, do we need the step course = get_course_by_id(course_key)? It doesn't seem to be used for anything.
| ) | ||
| success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) | ||
| except Exception as e: | ||
| raise self.api_error(status.HTTP_400_BAD_REQUEST, str(e), 'Requested task is already running') |
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 module-level function, so there's no self.api_error available.
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.
Ah thanks, this is leftover from porting from master.
| succeeded_count = 0 | ||
| for overview in overviews: | ||
| try: | ||
| course = modulestore().get_course(overview.id) |
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.
Can't we directly use the course_id instead of fetching the overviews and using the overview.id?
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.
Yeah this is leftover code from when I was running it for all courses. I didn't refactor, because I wasn't sure if we'd use it yet.
| "Subsection Name": subsection.display_name, | ||
| "Unit Name": unit.display_name, | ||
| "Component Name": component.display_name, | ||
| "Xblock Type": component.location.block_type, |
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 would just return all the component types in a course right? I wonder if it would be better to filter out the default ones (like html, problem, video) from external ones (like pdf, timeline, short answer, etc)?
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.
Ah maybe, not sure. We can probably ask the client - they may be interested in all components, for stats like which percentage of the components are third party xblocks.
Redwood port of openedx#37630
Private-ref: https://tasks.opencraft.com/browse/BB-10225
(cherry picked from commit 60d9220)