Save e-learning venues as E-Learning in scraper#2823
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2823 +/- ##
=======================================
Coverage 46.34% 46.34%
=======================================
Files 250 250
Lines 5302 5302
Branches 1220 1220
=======================================
Hits 2457 2457
Misses 2845 2845 Continue to review full report at Codecov.
|
| "outDir": "build", | ||
| "target": "es2020", | ||
| "target": "es2018", | ||
| "lib": ["dom", "es2020"], |
There was a problem hiding this comment.
@ZhangYiJiang saw your comment about the target here #2784 (comment), I also added "dom" to lib because the URL() calls were throwing errors in my tests: Cannot find name 'URL'. Did you mean 'url'? Not sure if you've encountered this too but adding "dom" fixed this for me.
There was a problem hiding this comment.
That's probably not correct - this code will only be run in Node.js, and while dom lib will add URL as a global like in Node 10/12, it will also add a bunch of other globals like document and navigator which are not valid. The correct way to fix this is to use the @types/node typing, but it doesn't add globals unless it is imported at least once, so in the end it's probably best to just import URL explicitly import { URL } from 'url';
|
Deployment preview for |
| CS4238Timetable.map((lesson) => { | ||
| return { ...lesson, room: 'E-Learn_C' }; | ||
| }), |
There was a problem hiding this comment.
| CS4238Timetable.map((lesson) => { | |
| return { ...lesson, room: 'E-Learn_C' }; | |
| }), | |
| CS4238Timetable.map((lesson) => ({ ...lesson, room: 'E-Learn_C' })); |
| export function mapTimetableLesson(lesson: TimetableLesson, logger: Logger): TempRawLesson { | ||
| const { room, start_time, end_time, day, module, modgrp, activity, eventdate, csize } = lesson; | ||
| const { | ||
| room: providedRoom, |
There was a problem hiding this comment.
This avoids the || '' and ?. below
| room: providedRoom, | |
| room: providedRoom = '', |
There was a problem hiding this comment.
Oops, one semester has passed. As zoning restrictions will be lifted on 6 Dec, I'm not sure how these venue names will change. Let's hold off work on this PR until we have more information.
3 participants
Follow up of #2736.
This PR changes the scraper to save lesson venues as "E-Learning" when the API returns a
roomofE-Learn_*for lessons.To see current shaped of our scraped data: https://nusmods.com/api/v2/2020-2021/modules/CS1010.json
@ZhangYiJiang does this look right to you? Also would need some help to test this, if possible. 😅
Also, don't merge this yet. Still need to add the frontend changes.