ryanhugh / searchneu

Search over Classes, Professors and Employees at NEU!
https://searchneu.com
GNU Affero General Public License v3.0
74 stars 18 forks source link

Occasionally, a single prereq may appear to have brackets around it #30

Closed edward-shen closed 6 years ago

edward-shen commented 6 years ago

https://searchneu.com/MATH+3175 image

Expected behavior: Prerequisites: MATH 2321 and MATH 2331

ryanhugh commented 6 years ago

The prereqs for this class are :

"prereqs":
            {
                "type": "and",
                "values": [
                {
                    "type": "or",
                    "values": [
                    {
                        "subject": "MATH",
                        "classUid": "2321_1423690526",
                        "classId": "2321"
                    },
                    {
                        "subject": "MATH",
                        "classUid": "2321_554670764",
                        "classId": "2321"
                    }]
                },
                {
                    "subject": "MATH",
                    "classUid": "2331_1389146835",
                    "classId": "2331"
                }]
            },

This is a bug in getReqsString, lets fix it after you merge your stuff!

ryanhugh commented 6 years ago

Looks like this like here:

if (_.uniq(childBranch.prereqs.values).length === 1) {

Needs to check the uniqness based on the classId of the class, instead of just comparing the entire objects:

{
  classId: ...
  classUid: ...
  subject: ...
}

So like, if all the objects in childBranch.prereqs.values have the same classId we would also want to go into that if statement.

edward-shen commented 6 years ago

A quick and dirty fix would be to use Lodash's uniqBy and do some post processing, but it doesn't fix the root of the issue.

Also if we ever update lodash, we should prepare for some breaking updates: (https://stackoverflow.com/questions/31740155/lodash-remove-duplicates-from-array https://github.com/lodash/lodash/wiki/Changelog#jan-12-2016--diff--docs

ryanhugh commented 6 years ago

I updated almost all of the npm packages other other day and we should be on lodash@^4.17.4 :)

This is an issue because of the classUid vs classId thing. This will happen if a class has prereqs with two classes that have the same classId but different classUids. When we refactor the scraping code to get rid of the classUids, this will go away. For now, I'm down to just use lodash's uniqBy.

ryanhugh commented 6 years ago

Fixed

https://github.com/ryanhugh/searchneu/commit/70f94046d2cd9b9113ef8bb79ee7c5a67e0cafc9