search

nus-cs2103-AY2223S1 / pe-dev-response

0 stars 0 forks source link

Lecture and Tutorial Zoom Link allowing other links that are not zoom links, including invalid links #5433

Open nus-pe-bot opened 1 year ago

nus-pe-bot commented 1 year ago

Screen Shot 2022-11-11 at 16.43.41.png

Screen Shot 2022-11-11 at 16.48.01.png

Entered command: addm m/EC1101E l/I3-AUDI Friday 16:00 - 18:00 lz/https://teams.microsoft.com/l/meetup-join/19:meeting_MjM2NzczMmEtMmRiNi00MGNhLWI1ZTYtMjI0ODQxMjI4NGNk t/COM1 B1-103 Wednesday 12:00 - 13:00 tz/https://nus-sg.zoom.us/CS2103T_tutorial a/Independent Project a/Team Project

Problem: Lecture and Tutorial Zoom Link allowing other links that are not zoom links, including invalid links

Justification: Users might think that the only links allowed are zoom links; but aside from that, if it has been specified that it is lecture zoom links, then the regex should actually follow the NUS zoom link format, e.g. https://nus-sg.zoom.us/* and not just limited by https://. I think this is justifiable because the application is used for NUS modules, which are most likely to use NUS zoom links. Otherwise, the wording could just be Lecture link or Tutorial meeting link.

Screen Shot 2022-11-11 at 16.53.25.png

Invalid links also pass this regex test. Limiting by https:// certainly is not enough.

[original: nus-cs2103-AY2223S1/pe-interim#5259] [original labels: type.FunctionalityBug severity.Low]

Yingming23 commented 1 year ago

Team's Response

The current function works exactly as how we intended as we have mentioned in our UG (a screenshot of it is attached below), and the user has the freedom to input anything as long as it is a valid URL. It is unreasonable to have overzealous input checks that would check whether the website for each URL exist. something.som is a valid URL but the domain does not host any website.

image.png

Duplicate status (if any):

--