search

sipb / courseroad2

A 4-year academic planner for the MIT community.
https://courseroad.mit.edu/
MIT License
23 stars 5 forks source link

audit dropping some saved majors #188

Closed npfoss closed 5 years ago

npfoss commented 5 years ago

I think what's happening is that it only saves one at a time or something like that. Try adding a bunch of majors, clicking save, and then refreshing after.

This one is probably a bigger deal so I'm classifying it as MVP. Given that you can still add some thing and most people will only add one or two, I'm guessing this is going to be about 0.4 minor confusions/irritations per user, so it's not mortally pressing.

georgiashay commented 5 years ago

hmmm, I've having trouble reproducing this. How quickly/how many do you have to add?

npfoss commented 5 years ago

I've been able to reliably reproduce this with the following steps, leaving plenty of time between each and confirming in the Network tab that nothing is pending:

  1. login
  2. create new road
  3. add three majors in addition to GIRs
  4. refresh (automatically logs in)
  5. check that new road
  6. only one of the majors I just added are there
georgiashay commented 5 years ago

Hmm, ok yeah I have reproduced this now and it didn't even save any of the additional majors

georgiashay commented 5 years ago

I think I know what is going on here. When something on a road changes, you have to update the "changed" field on the road. What seems to be happening is that fireroad is sending a response of "update local," which we aren't currently handling. I might not be thinking about it right, but I can't see a reason it would ever produce update local if we are handling the roads correctly.

We aren't setting the "changed" field when the coursesOfStudy updates. Right now we set it in several different locations where we know a road is being updated. It might be better to set it in the watcher for whenever the roads object changes, but in that case it would also be best to refactor auth and fix some of the other problems with it at the same time.

npfoss commented 5 years ago

solution: "save" button near major autocomplete (or just every time you add) causes a sync with fireroad

wishdasher commented 5 years ago

Reassigning Georgia because she has context and ideas about other things to fix along with this. Hope that's okay; let me know if you would rather have me fix just this bug.