search

web-cat / pythy

Pythy – the Cloud-Based IDE for Novice Python Programmers
18 stars 3 forks source link

Default role in Upload Roster modal is "Lead Instructor" #11

Closed allevato closed 11 years ago

allevato commented 11 years ago

Since roles are just listed in order, the role selected by default in the Upload Roster modal dialog is "Lead Instructor", which could lead to someone easily accidentally uploading all their students with elevated privileges. It should be changed to make "Student" the default role.

ArturAguiar commented 11 years ago

Fixed by reversing the order of the collection on the select tag.

allevato commented 11 years ago

The Student role is created by the application when the database is seeded before first run and has a fixed ID -- it might be better to use that as the default selection instead of depending on order. If a system admin adds more course roles, they would get ordered first by this fix.

On Mon, Sep 23, 2013 at 9:30 AM, ArturAguiar notifications@github.comwrote:

Fixed by reversing the order of the collection on the select tag.

— Reply to this email directly or view it on GitHubhttps://github.com/web-cat/pythy/issues/11#issuecomment-24933181 .

Tony Allevato tony.allevato@gmail.com

ArturAguiar commented 11 years ago

That's true. I missed the fact that new roles can be added later on. Sorry about that.

This is my new attempt at solving this:

= select_tag :course_role_id, options_from_collection_for_select(CourseRole.all, :id, :name, "4")

The "4" parameter is the default value's id in the selection dropdown. It is 4 since that is the id of the course role for Student. Is it safe to say that the student role will always be 4, since these initial roles are created by the application?

allevato commented 11 years ago

Use the CourseRole::STUDENT_ID constant for extra safety. It'll probably never change but that's better having a magic number buried somewhere in a view.

On Mon, Sep 23, 2013 at 1:08 PM, ArturAguiar notifications@github.comwrote:

That's true. I missed the fact that new roles can be added later on. Sorry about that.

This is my new attempt at solving this:

= select_tag :course_role_id, options_from_collection_for_select(CourseRole.all, :id, :name, "4")

The "4" parameter is the default value's id in the selection dropdown. It is 4 since that is the id of the course role for Student. Is it safe to say that the student role will always be 4, since these initial roles are created by the application?

— Reply to this email directly or view it on GitHubhttps://github.com/web-cat/pythy/issues/11#issuecomment-24950094 .

Tony Allevato tony.allevato@gmail.com

ArturAguiar commented 11 years ago

I actually didn't know we had those constants already set. Very useful! (:

Fixed.