Importer evaluation merge and other things#2649
Importer evaluation merge and other things#2649janno42 wants to merge 7 commits intoe-valuation:mainfrom
Conversation
janno42
requested review from
hansegucker,
jooooosef,
niklasmohrin and
richardebeling
janno42
force-pushed
the
importer-evaluation-merge
branch
from
0d387e1 to
c978d7c
Compare
niklasmohrin
left a comment
niklasmohrin
left a comment
There was a problem hiding this comment.
First pass with some suggestions; haven't looked at evaluation merging or UI yet
|
|
||
| self.assertEqual(course.semester, self.semester) | ||
| self.assertEqual(course.cms_id, EXAMPLE_DATA["events"][0]["gguid"]) | ||
| self.assertEqual(course.course_links.first().cms_id, EXAMPLE_DATA["events"][0]["gguid"]) |
There was a problem hiding this comment.
wouldn't it be nicer if we would use course.cms_links as the reverse name? (same for evaluation)
There was a problem hiding this comment.
wouldn't it be confusing that course.cms_links returns CourseLinks?
There was a problem hiding this comment.
Hm, fair point. I feel that from the pov of a course, I cannot grasp what a course link is, it could well be a link to the moodle or something. That's why I feel the word CMS must be in there. Now if a course has a "CMS link", I would be fine if that is a specialized type evap.cms.models.CourseLink
I would also be fine with cms_course_links
|
|
||
| self.assertEqual(course.semester, self.semester) | ||
| self.assertEqual(course.cms_id, EXAMPLE_DATA["events"][0]["gguid"]) | ||
| self.assertEqual(course.course_links.first().cms_id, EXAMPLE_DATA["events"][0]["gguid"]) |
There was a problem hiding this comment.
also, I suppose it should be list(course.course_links.values_list(flat=True)) == [...]
janno42
force-pushed
the
importer-evaluation-merge
branch
from
c978d7c to
2a2d3b4
Compare
…e updated by importer
janno42
force-pushed
the
importer-evaluation-merge
branch
from
2a2d3b4 to
880038c
Compare
janno42
force-pushed
the
importer-evaluation-merge
branch
from
880038c to
a396113
Compare
2 participants
see commit messages, best reviewed individually