WikiEducationFoundation / WikiEduDashboard

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

Common javascript error prevents overview from loading Description and ThisWeek #623

Closed ragesoss closed 8 years ago

ragesoss commented 8 years ago

See https://sentry.wikiedu.org/wikiedu/production/issues/117/

This seems to be happening a lot lately, and I was able to replicate it occasionally on Iceweasel; when it happens, the overview gets stuck on the loading animation.

Maybe it's a race condition for the different API calls that fire off when you load a course page?

ragesoss commented 8 years ago

I think maybe I broke things with 228b88ba19727e328d36b51b3c9e05ffcbc5d8a7

Could you take a look, @thenickcox? I maybe got ahead of myself with boldly meddling with React stuff.

thenickcox commented 8 years ago

@ragesoss @bmathews and I were looking at data syncing issues, and that's why I specifically used week.blocks instead of getting them out of the BlockStore. Problem is, we weren't confident enough to get rid of the process of getting them out of the BlockStore altogether, so we kept it in some places.

One solution I see here is just _.sortBy week.blocks, (blk) -> blk.order. And I wouldn't feel bad about that in the least.

@bmathews Can I get a second opinion?

ragesoss commented 8 years ago

@thenickcox sounds good to me.

bmathews commented 8 years ago

Sounds fine to me, or sort it on the server?

thenickcox commented 8 years ago

@ragesoss Might make more sense to sort on the server in the jbuilder, unless you can think of a reason we'd ever just want the blocks object not sorted by order.

ragesoss commented 8 years ago

@thenickcox as a general rule, I'm happy whenever we can move more things to the server. Feel free to make that switch. I'll open an issue for it.