Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[#7285] Live Server: disallow team name to be valid email #9471

Merged
merged 3 commits into from
Feb 22, 2019
Merged

[#7285] Live Server: disallow team name to be valid email #9471

merged 3 commits into from
Feb 22, 2019

Conversation

junming403
Copy link
Member

@junming403 junming403 commented Feb 19, 2019 •
edited
Loading

Part of #7285

Disallow team name to be valid emails.

@junming403 junming403 changed the title [#7285] LibveStudent submit a team response: the student's team members should also be added to the session's responding students list [#7285] Live Server: Disallow team name to be valid email Feb 19, 2019
@junming403 junming403 changed the title [#7285] Live Server: Disallow team name to be valid email [#7285] Live Server: disallow team name to be valid email Feb 19, 2019
@junming403 junming403 changed the base branch from master to release February 19, 2019 15:36
@wkurniawan07
Copy link
Member

Closing and re-opening since PR was targeted at wrong branch previously.

@wkurniawan07 wkurniawan07 reopened this Feb 19, 2019
@junming403
Copy link
Member Author

Ready for review

xpdavid
xpdavid approved these changes Feb 21, 2019
View reviewed changes
Copy link
Contributor

@xpdavid xpdavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xpdavid xpdavid added c.Task Other non-user-facing works, e.g. refactoring, adding tests s.FinalReview The PR is ready for final review labels Feb 21, 2019
wkurniawan07
wkurniawan07 requested changes Feb 21, 2019
View reviewed changes
src/main/java/teammates/common/util/FieldValidator.java Outdated
@@ -144,6 +144,8 @@
+ "The value of '${fieldName}' field should be no longer than ${maxLength} characters.";
public static final String INVALID_NAME_ERROR_MESSAGE =
ERROR_INFO + " " + HINT_FOR_CORRECT_FORMAT_FOR_INVALID_NAME;
public static final String TEAM_NAME_IS_VALID_EMAIL_ERROR_MESSAGE =
"The field " + TEAM_NAME_FIELD_NAME + " is not acceptable to TEAMMATES as team name can't be a valid email";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"As team name can't be a valid email" sounds quite weird. Suggest the following rephrase:

... is not acceptable to TEAMMATES as the suggested value for team name can be mis-interpreted as an email.

Also, team name is the value for the constant TEAM_NAME_FIELD_NAME so let's reuse it.

@junming403
Copy link
Member Author

Updated, ready for review!

wkurniawan07
wkurniawan07 approved these changes Feb 21, 2019
View reviewed changes
Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wkurniawan07 wkurniawan07 added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.FinalReview The PR is ready for final review labels Feb 21, 2019
@xpdavid xpdavid merged commit df87c34 into TEAMMATES:release Feb 22, 2019
@xpdavid xpdavid added this to the V6.16.2 milestone Feb 24, 2019
@damithc
Copy link
Contributor

damithc commented Feb 26, 2019

Received this new error from the live server today. Could it be related to this PR?

java.lang.NullPointerException
teammates.common.util.Logger severe: Unexpected exception caught by ControllerServlet : java.lang.NullPointerException at teammates.common.datatransfer.questions.FeedbackContributionQuestionDetails.getTeamSubmissionArray(FeedbackContributionQuestionDetails.java:517) at teammates.common.datatransfer.questions.FeedbackContributionQuestionDetails.getQuestionResultsStatisticsHtmlQuestionView(FeedbackContributionQuestionDetails.java:253) at teammates.common.datatransfer.questions.FeedbackContributionQuest...

@junming403
Copy link
Member Author

Hi prof, I think it's not relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xpdavid xpdavid xpdavid approved these changes

@wkurniawan07 wkurniawan07 wkurniawan07 approved these changes

Assignees
No one assigned
Labels
c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V6.16.2
Development

Successfully merging this pull request may close these issues.
4 participants
@junming403 @wkurniawan07 @damithc @xpdavid