Issue 1779 in teammatespes: instructorFeedbackResults: receiver email is shown as %GENERAL% when there is no specific recipient

[teamma...@googlecode.com]

Status: Accepted
Owner: ----
CC: shrianu...@gmail.com
Labels: Type-Bug

New issue 1779 by dam...@gmail.com: instructorFeedbackResults: receiver
email is shown as %GENERAL% when there is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

May be also do a .contains("@") before displaying the email?
Related to Issue 1725


--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[teamma...@googlecode.com]

Updates:
Owner: winsonta...@gmail.com
Cc: -shrianu...@gmail.com
Labels: Priority-Medium Reviewer-James

Comment #1 on issue 1779 by dam...@gmail.com: instructorFeedbackResults:

receiver email is shown as %GENERAL% when there is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

Winson, do this one instead. It is a bit more urgent. Remember to add a
test case.

[teamma...@googlecode.com]

Updates:
Status: Started

Comment #2 on issue 1779 by winsonta...@gmail.com:

instructorFeedbackResults: receiver email is shown as %GENERAL% when there
is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

Ok, prof. Noted.

[teamma...@googlecode.com]

Updates:
Status: ReadyForReview

Comment #3 on issue 1779 by winsonta...@gmail.com:

instructorFeedbackResults: receiver email is shown as %GENERAL% when there
is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

[Summary]
I hid the mailto using display:none if the email is the default value for
when recipient is nobody specific.

Also, I did not do the assertion .contains("@")
because when the recipient is a team, the email does not contain "@" (the
email is the team name).

For the test, I added a response for InstructorFeedbackResultsPageUiTest
for nobody specific.

https://codereview.appspot.com/87910044/

[Issue reporting]
On a side note, I also tried adding a response for when giver and recipient
is a team, for the
InstructorFeedbackResultsPageUiTest, in accordance with the template in
typicalDataBundle.json.
However, it doesn't appear in the InstructorFeedbackResultsPage. Im not
sure if this is a
mistake on my part as I'm not familiar with the requirements of adding a
response in the json.
I've removed this change in the submitted to-be-reviewed code.

[teamma...@googlecode.com]

Comment #4 on issue 1779 by dam...@gmail.com: instructorFeedbackResults:

receiver email is shown as %GENERAL% when there is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

When the recipient is a team, we should not show the mailto link right?

[teamma...@googlecode.com]

Comment #5 on issue 1779 by winsonta...@gmail.com:

instructorFeedbackResults: receiver email is shown as %GENERAL% when there
is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

Oh, you're right, for the sort by recipient,
currently we show an invalid mailto link, but I think we should hide them.

For the sort by giver, we show something like:

From: Team 1 [student...@gmail.com's Team],

so I thought it was fine, but I have just realized the email address
contains even the "...'s Team" part.
So for this case, do you want me to
1. keep the text "student...@gmail.com's Team" and just fix the email
address, or
2. do you want to just hide the mailto link altogether?
I personally prefer hiding the link altogether, because it looks cleaner.

Another thing is if I'm to implement this, I have a question regarding the
test case involved.
As I mentioned, I cannot seem to add a response for when giver and
recipient is a team, even though
I add this at the end of InstructorFeedbackResultsPageUiTest.json:
"response14":{
"feedbackSessionName":"First Session",
"courseId":"CFResultsUiT.CS2104",
"feedbackQuestionId":"4",
"feedbackQuestionType":"TEXT",
"giverEmail":"CFResultsU...@gmail.com",
"recipientEmail":"Team 2",
"responseMetaData":{
"value":"Response from team 1 (by alice) to team 2."
}
},

[teamma...@googlecode.com]

Comment #6 on issue 1779 by dam...@gmail.com: instructorFeedbackResults:

receiver email is shown as %GENERAL% when there is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

For the first question (i.e. when sorted by 'giver'), I prefer to keep the
fix the email rather than hide it.
For the second question, let's see if Arnold has any tips.

[teamma...@googlecode.com]

Updates:
Status: ChangesRequested

Comment #7 on issue 1779 by winsonta...@gmail.com:

instructorFeedbackResults: receiver email is shown as %GENERAL% when there
is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

Just to update, I've implemented it, with minor refactoring, and manually
tested it. All tests pass, but I haven't written any test cases yet for the
reason I mentioned earlier.

And can I change the status to ChangesRequested? because it's not ready to
review.

[teamma...@googlecode.com]

Comment #8 on issue 1779 by dam...@gmail.com: instructorFeedbackResults:

receiver email is shown as %GENERAL% when there is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

Try to complete the tests. See if other json files have data that fits your
scenario so that you can figure out what data to put in the json file.

[teamma...@googlecode.com]

Comment #9 on issue 1779 by winsonta...@gmail.com:

instructorFeedbackResults: receiver email is shown as %GENERAL% when there
is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

Dr Damith, thanks, that helps. I found out that the reason the response
does not show up is because the question is not visible to instructors. I
should come up with the patch tonight.

[teamma...@googlecode.com]

Updates:
Status: ReadyForReview

Comment #10 on issue 1779 by winsonta...@gmail.com:

instructorFeedbackResults: receiver email is shown as %GENERAL% when there
is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

Sorry for the delay, here it is

https://codereview.appspot.com/89180044/

[teamma...@googlecode.com]

Updates:
Status: Delivered

Comment #11 on issue 1779 by dam...@gmail.com: instructorFeedbackResults:

receiver email is shown as %GENERAL% when there is no specific recipient
http://code.google.com/p/teammatespes/issues/detail?id=1779

This issue was updated by revision a59619c86162.