Fix broken CSV export features in COACH tabs #12919
Conversation
github-actions
bot
added
APP: Coach
Build Artifacts
|
|
Hi @radinamatic @pcenov! can we have QA on this? |
|
Hi @ozer550, great to see so many issues fixed! Here's what I was able to identify as remaining issues with the csv export:
|
|
For point 1 - this is a good point, Peter and would be an improvement, but it's pre-existing and so for now, let's leave as-is and open a follow up for improvement. For point 2 - Prathamesh can implement this as part of the PR. For point 3 - I think we need some design insight here, and maybe some suggestions of how to improve the experience. One of the complications is that the lessons and quizzes have different column headers/fields and having them in the same file might be confusing. There could be a way that we can approach this on the code side, even though it is a bit complex (Prathamesh and I discussed this a little bit). I wonder if there is a way that we can indicate through the UI that there will be two files downloaded, one for lessons and one for quizzes. Or, separate buttons that make it clearer that you are downloading the file for each individually, rather than one for the "learner" report. Maybe @jtamiace has some suggestions here. Not sure if adding some user text, changing the button, etc. might be a solution? For point 4 - I think this is also a question for @jtamiace of whether there should be a download option here, and what it should be. |
|
For point 3, if there isn't a way to have the downloads in the same file, I think it would be okay to deviate from the pattern to have separate download CSV icons for each lesson and quiz section like here: For point 4, we're removing the reports from the groups page right? If so, I've intentionally excluded the download button from that page since there is no report data. I'm going off an assumption that just the group enrollment list is not something they'd be interested in exporting |
| let filteredExams = this.exams.filter(exam => | ||
| this.getLearnersForExam(exam).includes(this.learner.id), | ||
| ); | ||
|
|
||
| let filteredLessons = this.lessons.filter(lesson => | ||
| this.getLearnersForLesson(lesson).includes(this.learner.id), | ||
| ); | ||
|
|
||
| filteredLessons = filteredLessons.map(lesson => { | ||
| lesson.status = this.getLessonStatusStringForLearner(lesson.id, this.learner.id); | ||
| return lesson; | ||
| }); | ||
|
|
||
| filteredExams = filteredExams.map(exam => { | ||
| exam.statusObj = this.getExamStatusObjForLearner(exam.id, this.learner.id); | ||
| return exam; | ||
| }); |
There was a problem hiding this comment.
Is there a particular reason for making and mutating the filtered* variables rather than chaining the functions like const filteredExams = this.exams.filter(...).map(...)?
There was a problem hiding this comment.
Ahh yes this can be fixed further. Are you suggesting something like this?
const filteredExams = this.exams
.filter(exam => this.getLearnersForExam(exam).includes(this.learner.id))
.map(exam => ({
...exam,
statusObj: this.getExamStatusObjForLearner(exam.id, this.learner.id),
}));
| @@ -8,7 +8,7 @@ | |||
| :text="$tr('back')" | |||
| /> | |||
| </p> | |||
| <ReportsControls /> | |||
| <ReportsControls @export="exportCSV" /> | |||
There was a problem hiding this comment.
Not sure, but the issue Peter noted about the download starting multiple times might be fixed w/ @export.prevent="exportCSV" or something similar.
There was a problem hiding this comment.
Hi Jacob! this does not seem to be working. Error in v-on handler: "TypeError: Cannot read properties of undefined (reading 'preventDefault')".
There was a problem hiding this comment.
Is it that the download is starting multiple times, or is it that there are two files that were causing that pop up? I think if it's the two files, and we use the design approach that Jessica suggests above, that will resolve the issue by "splitting up" the file download buttons. If there's something that's happening in the code side that is triggering the same export multiple times, this approach could work.
There was a problem hiding this comment.
Hi Marcella! I was only checking for the case when the CSV export button was downloading 2 files simultaneously. We won't face this with two different buttons.
|
Hi @ozer550 - could you have a look why there is a problem with the new build? |
ozer550
force-pushed
the
fix-csv-exports
branch
from
bacbaf2 to
7944be1
Compare
Done! Apologies for the build failure. |
|
Hi @ozer550, I confirm that point 3 is fixed now and there are separate export options for Lessons assigned and Quizzes assigned.
Can that be addressed in this PR as well? |
Summary
- fix recipients column not working.
- additional columns "Average Score" and "All Learners".
References
closes #12734 #12714
Reviewer guidance
- check that two different csv files are being downloaded, one for lessons and other for quizzes.
- check the content of the files to ensure csv is populating the correct data.
- check if the recipients column is working as expected now.
- check if the newly added columns All learners and Average Score is working as expected