learning-unlimited / ESP-Website

A website to help manage the logistics of large, short-term educational programs
84 stars 57 forks source link

Make force_show_required_modules less terrible #1496

Open lenaqr opened 9 years ago

lenaqr commented 9 years ago

See https://github.com/learning-unlimited/ESP-Website/commit/8cd2b388556af856d02794ef6b24e544f60b50dd, https://github.com/learning-unlimited/ESP-Website/pull/1500

A nicer solution for this general problem would be to make force_show_required_modules specific to student reg (which it should be, since it's on the SCRMI which is supposed to be specific to student reg), and create a counterpart on the CRMI for teacher reg.

mgersh commented 9 years ago

Or we could make it less terrible by deleting it. I ran the script from #1692 and no one has ever set that field to False except for two MIT programs in 2010. Even if in theory someone wanted to do that in the future, they could entirely replicate its effect by setting things to not required and then changing the required_label.

mgersh commented 9 years ago

(I was using the script wrong, it was used on various other sites a total of five times, most recently in spring 2014. My overall conclusion doesn't change.)

lenaqr commented 9 years ago

That is not what's terrible about it, the terrible part is that there are some core modules (like student and teacher) for which this option makes sense, and some core modules (like manage, volunteer, json, and onsite) for which it doesn't. Really it's the notion of "core modules" that is terrible because we don't know what they are.

mgersh commented 9 years ago

Sure, I agree with you on that. But this issue claims to be about a specific SCRMI option, so I assumed that it was about that rather than program modules being weird in general. I don't think your proposed solution addresses the general weirdness any better than mine does? (Vaguely related: I was recently working on a project which made me wish that we had this option for manage as well.)

lenaqr commented 9 years ago

Sure, I meant for this issue to be about the mechanism, rather than the SCRMI option that toggles it.

lenaqr commented 9 years ago

I believe what I was thinking with my proposed solution is that then individual core modules and/or their corresponding *RMIs would own the appropriate behavior for handling required modules of that category. But if in fact it's the case that we want this behavior everywhere, maybe we do want to delete the SCRMI option and make it solely based on whether there are any required modules in that category. And the way to fix the problems alluded to in the initial post would be to not require login unless there are required modules. And with this we would be able to make required manage modules as well.