WikiEducationFoundation / WikiEduDashboard

Wiki Education Foundation's Wikipedia course dashboard system
https://dashboard.wikiedu.org
MIT License
392 stars 630 forks source link

Course pages can end up in a broken state after a series of clicks #1438

Closed ragesoss closed 7 years ago

ragesoss commented 7 years ago

I've found a bug that seems to be related to React getting into a broken state on course pages.

Here's how I've been able to (inconsistently) replicate it:

(Open a different student drawer and try again if it doesn't break the first time.)

At this point, there is an error like this:

TypeError: rev.article is undefined

And any further navigation on the course page is broken until page reload.

heysusgarcia commented 7 years ago

@ragesoss I would like to give this issue a try! I will follow your steps to try to reproduce the bug and follow up.

ragesoss commented 7 years ago

@heysusgarcia awesome. let me know if you have any questions.

I'm able to replicate it on staging as well, for example here: https://dashboard-testing.wikiedu.org/courses/University_of_San_Francisco/Faculty_as_Wikipedians_Fall_2016_(Fall_2016)/students

It usually happens after the second time through with a different student.

You'll probably need to get a course with one or two students who have revisions to replicate it locally. You can add in some of the same usernames from that course (with similar course dates), if you don't have a suitable course with relevant data already set up. ( Add /manual_update to the root course path to pull in data.)

heysusgarcia commented 7 years ago

I was able to replicate the bug. It seems that this bug only happens when you

  1. open a student drawer that contains revisions
  2. you leave the drawer open
  3. navigate to one of the tabs on the navbar (i.e. Home, Timeline, Activity, Articles, Uploads)
  4. navigate back to the student tab

If you close the drawer before navigating to one of the other tabs and back, nothing happens.

I'm thinking it has something to do with the the Student component. When we first open the StudentDrawer (or 'click') the following is called: openDrawer() { RevisionStore.clear(); TrainingStatusStore.clear(); ServerActions.fetchRevisions(this.props.student.id, this.props.course.id); ServerActions.fetchTrainingStatus(this.props.student.id, this.props.course.id); return this.props.toggleDrawer(drawer_${this.props.student.id}); },

But if we leave this drawer open, and then try to navigate back to the Student tab rev.article.url is called in the StudentDrawer component. It fails because we haven't fetched any revisions at this point calling rev.article.url results in undefined.

I'm thinking we can probably get around this if we include a react lifecycle method (componentWillMount) in the Student component that checks if this.props.isOpen is true and if so we can just call this.openDrawer, or something along those lines. Let me know if this sounds like a good thing to try out/if you can think of any potential issues/disruptions to the way things are done/structured. I will try it out locally and see what I find.

ragesoss commented 7 years ago

Maybe the easiest way would be to reset the drawer state to closed via a componentWillUnmount() hook.

heysusgarcia commented 7 years ago

my changes are in PR#1449.