ubc / compair

ComPAIR: a peer review application pairing student answers for deeper learning through comparison of peer work
http://ubc.github.io/compair
GNU General Public License v3.0
36 stars 11 forks source link

Access to import/export classlist #382

Closed lenglund closed 7 years ago

lenglund commented 8 years ago

Right now, instructors no longer have the ability to import or export the classlist from the manage users screen. This presents a bit of an issue for those instructors who want to set up testing accounts, especially on the sandbox site.

I recall our discussion re: hiding the options for LTI users, but I don't recall why we are hiding the options for non-LTI users. If there isn't a reason to remove the options for non-LTI users, I would suggest the following:

Thoughts?

andrew-gardener commented 8 years ago

I reviewed all the LTI issues and I think I just interpreted the manage users changes incorrectly at some point.

I'll re-enable the import/export buttons for instuctors and admins for all courses that don't support lti membership.

lenglund commented 7 years ago

Can someone refresh me how we handle passwords and display names when the ComPAIR logins are used but no passwords are in the import file? How are these generated again, and how should instructors pass login info to students?

This is what the app screen says: "If a display name is not given, the user is assigned an anonymous display name default, e.g., Student1234. If a password is not given, the default password saved for the user is the username." Is this still true?

Also, is it true the usernames and student numbers have to be unique? And can group names be imported in the initial import, if added to the end? Or does this have to be separate?

lenglund commented 7 years ago

Updated in 807f08d2f3f650b37a4df8f857aa4177ee6cbcb0. (Need to look at tests.)

andrew-gardener commented 7 years ago

It is actually a randomly generated password right now. We can easily change this to the username if there are no security concerns though.

lenglund commented 7 years ago

If it's a randomly-generated password, how would an instructor or admin give the login info to students?

andrew-gardener commented 7 years ago

It's a bit or an overlooked problem right now. Setting it to something like username or student number (if present) is definitely a better solution in my mind accept maybe for security concerns (since these are guessable)

lenglund commented 7 years ago

Can we simply make passwords a requirement of the import? Then instructors will have their own copy to reference in passing on to students... @xcompass, any other ideas?

xcompass commented 7 years ago

@andrew-gardener @lenglund I thought the password is not required, but if they want to send out the passwords to the students, they can generate the passwords and upload them when importing. In this way, they can set it to whatever they want (username or student number). Isn't it the case right now?

andrew-gardener commented 7 years ago

Alright, I reviewed the code a bit and remembered that we changed it from randomly generated to being blank when empty.

Ok just to help map all the ComPAIR account import situations for passwords:

The current problem I see is that teachers cannot mass set existing student passwords on via import.

The way I see it there are three solutions:

xcompass commented 7 years ago

Thanks @andrew-gardener. I would like to support the use case that instructor is able to mass update passwords, in case they set the wrong passwords when importing first time. We only need to change the logic for your last bullet points:

In this case, we need to display a warning saying that the students passwords will be updated and any updates by students will be overridden.

lenglund commented 7 years ago

The second option gets my vote. This is the easiest way to make sure instructors know what to send the student because they'll have set a password manually.

xcompass commented 7 years ago

But it will be annoying for those who have their party auth enabled, because they don't need to set any passwords.

lenglund commented 7 years ago

When importing: username/password is required for creating ComPAIR accounts (password not required for third-party), password can be left blank for existing ComPAIR accounts (if no account, error thrown; if account, password overwritten IF user has not logged in yet), group is overwritten with each import (but nothing else), order for CSV to be updated and include groups (Username, Password, Student Number, First Name, Last Name, Email, Display Name, Group), duplicate student numbers mean user is not added, export should match import other than password column and add role column.

So action items:

lenglund commented 7 years ago

First part of To Dos completed in 779e64f6807ede0630cff83de4ff69770e779f6c.

andrew-gardener commented 7 years ago

upon reviewing the exported classlist, one additional column that we should add is for cas username (in case they imported via cwl).

Currently if importing by cwl, new users will not have a username set in the username table. This would make it much harder to import again via the exported file.

It might also be useful to add cas username to the manage users screen's class list table as well (though it will take up more room on the screen). Maybe a toggle to show the column?

Edit: actually forget the last point. we aren't showing username in the table anyways...

andrew-gardener commented 7 years ago

459 finishes the to dos. It also includes adding cas_username to the exported classlist csv (along with the group_name)