Import merged evaluation#2747
Conversation
…rged into another course
niklasmohrin
left a comment
niklasmohrin
left a comment
There was a problem hiding this comment.
I have some trouble understanding how all the parts come together here. In the issue it says that we have an assumption that two things exist together when they don't really have to, but here it is not obvious how this is split into two now
There was a problem hiding this comment.
We should add a test for this behavior
| evaluation = Evaluation.objects.get(cms_evaluation_links__cms_id=data["gguid"]) | ||
| # get the course from the evaluation in case it was merged into a different course than defined in the | ||
| # import data | ||
| course = evaluation.course |
There was a problem hiding this comment.
Is there any chance that we can choose the correct course once before calling import_evaluation? I think it is hard to track what course belongs to what now. In the calling code in import_events we have some if course is None: continue and I am not sure how this should interact with this new assignment here (my intuition is that if the original course is None but we find a course here then we should still run the import).
If this is not possible, then one (or both) of the two course variables needs a new name, I guess the course before this is something like course_from_event and this here is like a selected_course? Although selected_course really should just always be evaluation.course, so I guess we could also just use this everywhere?
richardebeling
left a comment
richardebeling
left a comment
There was a problem hiding this comment.
code change looks good but I agree that a test would be nice
3 participants
fix #2746; support importing evaluations that have been merged into another course