damithc / testrepo3

0 stars 0 forks source link

FeedbackSessionPublish: students cannot see results if there are no questions for them to answer #185

Open damithc opened 10 years ago

damithc commented 10 years ago

From dam...@gmail.com on November 10, 2013 09:15:43

I would like to check: If there are no questions for students in a session (e.g. a session created for instructor to give feedback to students), we don't send 'published' emails to students?

Original issue: http://code.google.com/p/teammatespes/issues/detail?id=1288

damithc commented 10 years ago

From dam...@gmail.com on November 09, 2013 18:48:46

In fact students can't even see the result if there are no questions for them to answer. They were only able to see results (and received 'published' alerts) after I added a dummy question for students to answer. Changing issue title.

Summary: FeedbackSessionPublish: students cannot see results if there are no questions for them to answer (was: FeedbackSessionPublish: emails not sent if there are no questions for students?)
Status: Accepted

damithc commented 10 years ago

From arnold.k...@gmail.com on December 10, 2013 00:13:24

Actually, it seems like students can't even see the session if there is no question for the student to answer. (unless the student happen to be the instructor too then the student/instructor can see both the session and the results from the student home page).

Can I do this next?

damithc commented 10 years ago

From dam...@gmail.com on December 10, 2013 00:28:59

Sure.

damithc commented 10 years ago

From arnold.k...@gmail.com on December 10, 2013 00:36:46

Just reporting an oddity:

Just now I did this:

As instructor: 1) Created a course and enrolled the students 2) Added a feedback session with no question for students, fill in the answer and publish the result

However, I forgot to join the course as a student as the invitation email hasn't arrived yet. The weird thing is I get the result publication email for the feedback session above (but of course I can;t open it as I'm not enrolled yet).

After I'm enrolled I login to Teammates and again cannot see the session. Clicking the link from the result publication email gave me "There are currently no responses for you for this feedback session."

Owner: arnold.k...@gmail.com

damithc commented 10 years ago

From arnold.k...@gmail.com on December 10, 2013 00:54:07

EDIT: quoted wrong code, sorry

It seems like there is explicit code that disallows students to see sessions / receive emails about it if there are no questions to them:

// Allow viewing if session is viewable to students
return isFeedbackSessionViewableToStudents(session);

}

private boolean isFeedbackSessionViewableToStudents(FeedbackSessionAttributes session) throws EntityDoesNotExistException { // Allow students to view if there are questions for them List questions = fqLogic.getFeedbackQuestionsForStudents( session.feedbackSessionName, session.courseId);

return (session.isVisible() && !questions.isEmpty()) ? true : false;

}

Is there any scenario where students are not allowed to see/know about a session (e.g. intra-instructor feedback sessions) or is it safe to allow students to see all sessions as long as the status is visible? One drawback I can think of is students may see a session in which he will never have to participate and never be able to see the result of, which might be weird.

damithc commented 10 years ago

From arnold.k...@gmail.com on December 10, 2013 01:05:03

One more remark:

The reason why instructors who are also students can see what normal students can't see is because in the code the "Instructor-check" works by sending the email address of the user (regardless of whether the user is a student or an instructor) and seeing if the email exists in the instructor database. If it does and the instructor is the instructor of the course, the code will show even instructor-only sessions, even if the email logged in as a student.

I'm not sure if this is a problem or not, but it does cause manual testing via a common student/instructor account unreliable :S

damithc commented 10 years ago

From dam...@gmail.com on December 10, 2013 01:07:30

There could be sessions that we don't want students to see. E.g. tutors recording feedback ABOUT students. Then, there could be sessions that students don't have to submit anything, but they should be able to see. e.g. instructor giving feedback TO students. Ideally, 'opening' and 'closing' alerts should reach students if they have to submit something. 'published' alerts should reach students if they have any responses to see. May be visibility should follow the same rule. Not sure how hard it is to do though.

damithc commented 10 years ago

From dam...@gmail.com on December 10, 2013 01:10:24

I think there is another issue about this somewhere. When a person login as a student, he should see only the student stuff even if he is also an instructor of the course. Otherwise it is confusing.

damithc commented 10 years ago

From arnold.k...@gmail.com on December 11, 2013 00:20:18

Ideally, 'opening' and 'closing' alerts should reach students if they have to submit something. 'published' alerts should reach students if they have any responses to see. May be visibility should follow the same rule. Not sure how hard it is to do though.

All these have a check in their corresponding FeedbackLogic methods so I think it's doable. We just need to figure out the correct conditions to achieve it (currently, only restrict students if there's no question for them to answer).

I think there is another issue about this somewhere. When a person login as a student, he should see only the student stuff even if he is also an instructor of the course. Otherwise it is confusing.

Found it at issue 1094 . This one might be harder to do though, will comment there.

damithc commented 10 years ago

From arnold.k...@gmail.com on December 11, 2013 18:07:45

My current idea is to use the giverType and the recipientType to figure out who to send emails to and who to show the session to.

Open/close emails should be sent according to the giverType Publish emails should be sent according to the recipientType The problem with this is when the type points to "Self" as who it's pointing to have to be checked dynamically (I haven't checked if "Self" is actually stored in the database though, will have to read the code more); and when the type points to "Team" as we'll have to figure out who's in the team (actually, is it true that when the recipient is "Team" (whether own or other) all students will receive something anyway? So the publish emails can be sent to all students when the recipient is "Team")

For visibility, the session should be visible to both giver and recipient but this can be done in two ways: 1) Make it visible from both group from the beginning This is simpler but can confuse recipients when the session is visible but they can't do anything with it until the result is published. 2) Make it visible to givers first and then make it visible to recipients only after the result is published Might be more intuitive. Implementing would involve checking the status of the session first before deciding to show to recipients or not

Any feedback on this?

Cc: ravenxce

damithc commented 10 years ago

From arnold.k...@gmail.com on December 12, 2013 06:33:35

Some follow-up from the above idea (some of this are just my own notes):

Possible Feedback Giver: SELF <- only session creator can submit. no need to send email to anybody STUDENTS <- send open/close email to all students INSTRUCTORS <- send open/close email to all instructors TEAMS <- send open/close email to all students

Possible Feedback Recipient: SELF <- send published email to all giver STUDENTS <- send published email to all students INSTRUCTORS <- send published email to all instructors TEAMS <- send published email to all students OWN_TEAM <- send published email to all students OWN_TEAM_MEMBERS <- send published email to all students OWN_TEAM_MEMBERS_INCLUDING_SELF <- send published email to all students NONE <- Depending on visibility setting(?)

-- Thought process -- What is isValidViewer for? What are RECEIVER and RECEIVER_TEAM_MEMBERS FeedbackParticipantType? (they are not valid recipient but valid viewer?)

actually what happens when after a giver/recipient type is set the visibility option is customized? This means the visibility depend on the options not the giver/recipient type?

Are the recipients more accurately reflected by FeedbackQuestionAttributes.showResponsesTo instead of recipientType? Can I assume then that the showResponseTo list does not have NONE in it? -- end thought process --

-- conclusion, correct me if I'm wrong -- For open/close emails, it is safe just to check the giverType and send accordingly to the possible givers as the above.

For published email, first check the List FeedbackQuestionAttributes.showResponsesTo. Then if it contains the above possible recipients, send as above. If it contains: RECEIVER -> check receiverType and send accordingly RECEIVER_TEAM_MEMBERS -> check receiverType and send accordingly Assume no NONE (NONE is not isValidViewer)

A big assumption here is when one student gets a feedback result, all students get feedback results (i.e. not possible only for some students to get results).

Of course only one email will be sent for one session (instead of one email per question) so the above will be aggregated first for all questions in the session before actually sending the emails).

For visibility of the session itself, we can also follow the above rules (change send email to -> visible to). If we want to stagger visibility as in option 2) in the previous comment, we can keep two separate list for givers and viewers and only make it visible to the correct list at appropriate times.

-- Trade-offs -- Should we compute the giver/recipient lists for a session (i.e. the aggregate of all questions' giver/recipient of that session) on the fly or precompute and keep it in a list? On-the-fly:

precompute:

In the current code, I believe most things are computed on the fly

damithc commented 10 years ago

From arnold.k...@gmail.com on December 22, 2013 22:55:32

Some things were updated by revision 69ea9feda055 From a quick skim the emailing problem seems rectified but the visibility problems has yet to be fixed

damithc commented 10 years ago

From arnold.k...@gmail.com on January 05, 2014 18:42:50

Status: Accepted
Owner: ---

damithc commented 10 years ago

From arnold.k...@gmail.com on January 09, 2014 18:37:00

Blockedon: teammatespes:1475

damithc commented 10 years ago

From arnold.k...@gmail.com on January 13, 2014 18:27:31

Labels: -Difficulty-Medium Difficulty-High