TEAMMATES / teammates

This is the project website for the TEAMMATES feedback management tool for education
https://teammatesv4.appspot.com/
GNU General Public License v2.0
1.66k stars 3.31k forks source link

InstructorFeedbackResultsPage is displaying all responses for custom setting instructor who is only supposed to see one section #2982

Closed franklingu closed 9 years ago

damithc commented 9 years ago

Don't forget this one too. :-)

franklingu commented 9 years ago

I have mentioned in the email and also #2981. currently if the recipient is student, none or instructor, accessControl is not going to kick in. if you want, we can change the policy but the current idea is that if the receivers are like nearly all students already, then it is not private enough that some tutors are kept out of those question.

damithc commented 9 years ago

That policy doesn't make sense. Why should a tutor whose access is limited to section 1 see a response from a student in section 2 just because the response is given to 'none'? Another thing: when the receiver is 'students in this course' that doesn't mean everybody can see it. It is not a single response that the whole class can see. Each student gets an individual response, which may be visible to that student only (based on visibility settings). So we can't say that all instructors must be able to see it too. Take CE1 peer review for example. Students submit reviews for two other students. Why should a tutor see peer reviews that are not related to his section? The basic principle is that when a tutor is limited to a section, he should not see things that are not related to his section at all. Otherwise it is not consistent with the permission settings shown in the course edit page.

franklingu commented 9 years ago

Well, cause the students in the tutors' section can receive it--and that may imply that tutor can receive as well.

so do we change to only if recipients are instructors, we disable privilege checking, otherwise checking will be done?

Gu Junchao Major in Computer Engineering, National University of Singapore Primary Email: franklingujunchao@gmail.com franklingujunchao@gmail.com NUS Email: A0105750@nus.edu.sg A0105750@nus.edu.sg

On Fri, Feb 13, 2015 at 12:44 AM, Damith C. Rajapakse < notifications@github.com> wrote:

That policy doesn't make sense. Why should a tutor whose access is limited to section 1 see a response from a student in section 2 just because the response is given to 'none'? Another thing: when the receiver is 'students in this course' that doesn't mean everybody can see it. It is not a single response that the whole class can see. Each student gets an individual response, which may be visible to that student only (based on visibility settings). So we can't say that all instructors must be able to see it too. Take CE1 peer review for example. Students submit reviews for two other students. Why should a tutor see peer reviews that are not related to his section? The basic principle is that when a tutor is limited to a section, he should not see things that are not related to his section at all. Otherwise it is not consistent with the permission settings shown in the course edit page.

— Reply to this email directly or view it on GitHub https://github.com/TEAMMATES/repo/issues/2982#issuecomment-74104918.

damithc commented 9 years ago

"students can receive" doesn't mean it is a single response all students can see. It is 'one response per student' and visibility can be anything. So we cannot assume it is a very 'public' response which can be shared with all instructors. Yes, it should be access controlled. Be guided by the basic principle: if a tutor is given access to a section only, as far as possible he should not see things not related to that section.

kanghj commented 9 years ago

For a response from a student in Section A to a student in Section B, should an instructor who has permission to view Section A, but not Section B, be able to see this response?

damithc commented 9 years ago

We discussed this sometime back. I don't remember the decision. To be of the safe side,we can disallow it.