Merge branch 'master' into 6238-appveyor

.travis.yml

@@ -17,6 +17,12 @@ addons:
   firefox: "46.0"
 
 before_install:
+  - |
+      if ! git diff --name-only $TRAVIS_COMMIT_RANGE | grep -qvE '(.md)|(.txt)|(.png)|(.jpg)|(.gif)|^(LICENSE)|^(docs)'
+      then
+        echo "Only doc files were updated, not running the CI."
+        exit
+      fi
   - "/sbin/start-stop-daemon --start --quiet --pidfile /tmp/custom_xvfb_99.pid \
     --make-pidfile --background --exec /usr/bin/Xvfb -- :99 -ac -screen 0 1440x900x16"
 

docs/settingUp.md

@@ -23,6 +23,13 @@ The instructions in all parts of this document work for Linux, OS X, and Windows
    ```sh
    git remote add upstream https://github.com/TEAMMATES/teammates.git
    ```
+
+ **Verification:** Use the command `git remote -v` and the following lines should be part of the output:
+
+  ```
+    upstream        https://github.com/TEAMMATES/teammates.git (fetch)
+    upstream        https://github.com/TEAMMATES/teammates.git (push)
+  ```
 
 More information can be found at [this documentation](https://help.github.com/articles/fork-a-repo/).
 
@@ -32,12 +39,16 @@ More information can be found at [this documentation](https://help.github.com/ar
    ```sh
    ./gradlew appengineDownloadSdk
    ```
-
+
+ **Verification:** Check your Gradle folder (the directory can be found with the command `./gradlew printUserHomeDir`). A folder named `appengine-sdk` should be present.
+
 1. Run this command to create the main config files (these are not under revision control because their contents vary from developer to developer):
    ```sh
    ./gradlew setup
    ```
 
+ **Verification:** The file named `.project` should be added to the project root directory.
+
 1. Modify the following config files:
    * `gradle.properties`<br>
       If you want to use a JDK other than the one specified in your PATH variable, add the value to the variable `org.gradle.java.home`.
@@ -56,6 +67,8 @@ Eclipse IDE is our preferred development environment. Support requests related t
    * [Google Plugin for Eclipse](https://developers.google.com/eclipse/docs/download). Use the correct version for your Eclipse IDE.<br>
      Be careful to omit other plugins shown on the screen (e.g Google App Engine Tools for Android, GWT plugin).
    * The latest stable [TestNG Eclipse plugin](http://testng.org/doc/download.html).
+
+   **Verification:** You can check if the plugins were installed successfully by going to `Help → Installation Details`.
 
 1. Configure Eclipse as follows (if you worry that these settings will interfere with your other projects, you can use a separate Eclipse instance for TEAMMATES):
    * Google App Engine: Go to `Eclipse → Preferences → Google → App Engine`, click the `Add` button, and point it to where Gradle keeps the downloaded SDK.<br>
@@ -64,6 +77,10 @@ Eclipse IDE is our preferred development environment. Support requests related t
    * Text encoding: Go to `Eclipse → Preferences → General → Workspace`, change the `Text file encoding` setting from `Default` to `Other: UTF-8`.
    * JRE: Go to `Eclipse → Preferences → Java → Installed JRE` and ensure a JDK (not a JRE) is selected (use **JDK 1.7**, as recommended by GAE).
      One of the items in the [troubleshooting guide](troubleshooting-guide.md) explains how to do this.
+
+   **Verification:** Go to `Eclipse → Preferences → Java → Installed JREs`, select the JDK 1.7 entry, and choose Edit. It should result in something like this:
+      ![devverification-guide-1.png](images/devverification-guide-1.png)
+
    * Indentation: In TEAMMATES, we use 4 spaces in place of tabs for indentations.
      Go to `Eclipse → Preferences` and configure for all the languages used in TEAMMATES:
      * Java: `Java → Code Style → Formatter → Edit → Tab policy → Spaces only`
@@ -79,6 +96,8 @@ Eclipse IDE is our preferred development environment. Support requests related t
    ./gradlew resetEclipseDeps
    ```
    This command can be run again whenever the dependencies need to be updated.
+
+ **Verification:** The file named `.classpath` should be added to the project root directory.
 
 1. Import the project to your Eclipse instance.
    * Start Eclipse and go to `File → Import...`.

src/main/java/teammates/common/datatransfer/FeedbackRankOptionsQuestionDetails.java

@@ -17,7 +17,7 @@
 import teammates.common.util.Templates;
 import teammates.common.util.Templates.FeedbackQuestion.FormTemplates;
 import teammates.common.util.Templates.FeedbackQuestion.Slots;
-import teammates.ui.controller.PageData;
+import teammates.ui.pagedata.PageData;
 import teammates.ui.template.ElementTag;
 import teammates.ui.template.InstructorFeedbackResultsResponseRow;
 

src/main/java/teammates/common/datatransfer/FeedbackRankRecipientsQuestionDetails.java

@@ -15,7 +15,7 @@
 import teammates.common.util.Templates;
 import teammates.common.util.Templates.FeedbackQuestion.FormTemplates;
 import teammates.common.util.Templates.FeedbackQuestion.Slots;
-import teammates.ui.controller.PageData;
+import teammates.ui.pagedata.PageData;
 import teammates.ui.template.ElementTag;
 import teammates.ui.template.InstructorFeedbackResultsResponseRow;
 

src/main/java/teammates/common/util/StringHelper.java

@@ -307,6 +307,31 @@ public static Set<String> removeExtraSpace(Set<String> strSet) {
         }
         return result;
     }
+
+    /**
+     * Replaces every character in {@code str} that does not match
+     * {@code regex} with the character {@code replacement}.
+     * 
+     * @param str String to be replaced.
+     * @param regex Pattern that every character is to be matched against.
+     * @param replacement Character unmatching characters should be replaced with.
+     * @return String with all unmatching characters replaced; null if input is null.
+     */
+    public static String replaceIllegalChars(String str, String regex, char replacement) {
+        if (str == null) {
+            return null;
+        }
+
+        char[] charArray = str.toCharArray();
+
+        for (int i = 0; i < charArray.length; i++) {
+            if (!isMatching(Character.toString(charArray[i]), regex)) {
+                charArray[i] = replacement;
+            }
+        }
+
+        return String.valueOf(charArray);
+    }
 
     private static String byteArrayToHexString(byte[] bytes) {
         StringBuilder sb = new StringBuilder(bytes.length * 2);

src/main/java/teammates/logic/api/Logic.java

@@ -1930,8 +1930,8 @@ public boolean hasGiverRespondedForSession(String userEmail, String feedbackSess
         return feedbackResponsesLogic.hasGiverRespondedForSession(userEmail, feedbackSessionName, courseId);
     }
 
-    public boolean isCourseHasResponses(String courseId) {
-        return feedbackResponsesLogic.isCourseHasResponses(courseId);
+    public boolean hasResponsesForCourse(String courseId) {
+        return feedbackResponsesLogic.hasResponsesForCourse(courseId);
     }
 
     /**

src/main/java/teammates/logic/core/FeedbackResponsesLogic.java

@@ -255,7 +255,7 @@ public List<FeedbackResponseAttributes> getViewableFeedbackResponsesForQuestionI
         return viewableResponses;
     }
 
-    public boolean isNameVisibleTo(
+    public boolean shouldNameBeVisibleToUser(
             FeedbackQuestionAttributes question,
             FeedbackResponseAttributes response,
             String userEmail,
@@ -277,6 +277,13 @@ public boolean isNameVisibleTo(
             }
         }
 
+        return shouldFeedbackParticipantNameBeVisibleToUser(question, response,
+                userEmail, role, isGiverName, roster);
+    }
+
+    private boolean shouldFeedbackParticipantNameBeVisibleToUser(
+            FeedbackQuestionAttributes question, FeedbackResponseAttributes response,
+            String userEmail, UserRole role, boolean isGiverName, CourseRoster roster) {
         List<FeedbackParticipantType> showNameTo = isGiverName
                                                  ? question.showGiverNameTo
                                                  : question.showRecipientNameTo;
@@ -350,10 +357,8 @@ public boolean isResponseOfFeedbackQuestionVisibleToStudent(FeedbackQuestionAttr
                 || question.recipientType.equals(FeedbackParticipantType.GIVER)
                    && question.giverType.equals(FeedbackParticipantType.STUDENTS);
 
-        if (isStudentRecipientType && question.isResponseVisibleTo(FeedbackParticipantType.RECEIVER)) {
-            return true;
-        }
-        if (question.recipientType.isTeam() && question.isResponseVisibleTo(FeedbackParticipantType.RECEIVER)) {
+        if ((isStudentRecipientType || question.recipientType.isTeam())
+                && question.isResponseVisibleTo(FeedbackParticipantType.RECEIVER)) {
             return true;
         }
         if (question.giverType == FeedbackParticipantType.TEAMS
@@ -363,7 +368,7 @@ public boolean isResponseOfFeedbackQuestionVisibleToStudent(FeedbackQuestionAttr
         return question.isResponseVisibleTo(FeedbackParticipantType.RECEIVER_TEAM_MEMBERS);
     }
 
-    public boolean isCourseHasResponses(String courseId) {
+    public boolean hasResponsesForCourse(String courseId) {
         return frDb.hasFeedbackResponseEntitiesForCourse(courseId);
     }
 
@@ -421,11 +426,32 @@ public void updateFeedbackResponse(
 
         // Create a copy.
         FeedbackResponseAttributes newResponse = new FeedbackResponseAttributes(updatedResponse);
-
         FeedbackResponseAttributes oldResponse = new FeedbackResponseAttributes(oldResponseEntity);
 
         // Copy values that cannot be changed to defensively avoid invalid
         // parameters.
+        copyFixedValuesFromOldToNew(newResponse, oldResponse);
+
+        if (newResponse.recipient.equals(oldResponse.recipient)
+                && newResponse.giver.equals(oldResponse.giver)) {
+            try {
+                frDb.updateFeedbackResponseOptimized(newResponse, oldResponseEntity);
+            } catch (EntityDoesNotExistException e) {
+                Assumption.fail();
+            }
+        } else {
+            // Recreate response to prevent possible future id conflict.
+            recreateResponse(newResponse, oldResponse);
+        }
+    }
+
+    /**
+     * Copies values that cannot be changed to defensively avoid invalid parameters.
+     * @param newResponse  values are copied from oldResponse
+     * @param oldResponse  values are copied to newResponse
+     */
+    private void copyFixedValuesFromOldToNew(FeedbackResponseAttributes newResponse,
+            FeedbackResponseAttributes oldResponse) {
         newResponse.courseId = oldResponse.courseId;
         newResponse.feedbackSessionName = oldResponse.feedbackSessionName;
         newResponse.feedbackQuestionId = oldResponse.feedbackQuestionId;
@@ -446,18 +472,6 @@ public void updateFeedbackResponse(
         if (newResponse.recipientSection == null) {
             newResponse.recipientSection = oldResponse.recipientSection;
         }
-
-        if (newResponse.recipient.equals(oldResponse.recipient)
-                && newResponse.giver.equals(oldResponse.giver)) {
-            try {
-                frDb.updateFeedbackResponseOptimized(newResponse, oldResponseEntity);
-            } catch (EntityDoesNotExistException e) {
-                Assumption.fail();
-            }
-        } else {
-            // Recreate response to prevent possible future id conflict.
-            recreateResponse(newResponse, oldResponse);
-        }
     }
 
     private void recreateResponse(
@@ -484,19 +498,25 @@ private void recreateResponse(
     public void updateFeedbackResponsesForChangingTeam(
             String courseId, String userEmail, String oldTeam, String newTeam) {
 
-        FeedbackQuestionAttributes question;
-
-        List<FeedbackResponseAttributes> responsesFromUser =
-                getFeedbackResponsesFromGiverForCourse(courseId, userEmail);
+        deleteResponsesFromUserToTeam(courseId, userEmail);
+        deleteResponsesFromTeamToUser(courseId, userEmail);
+
+        boolean isOldTeamEmpty = studentsLogic.getStudentsForTeam(oldTeam, courseId).isEmpty();
+        if (isOldTeamEmpty) {
+            deleteTeamResponses(courseId, oldTeam);
+        }
+    }
 
-        for (FeedbackResponseAttributes response : responsesFromUser) {
-            question = fqLogic.getFeedbackQuestion(response.feedbackQuestionId);
-            if (question.giverType == FeedbackParticipantType.TEAMS
-                    || isRecipientTypeTeamMembers(question)) {
-                frDb.deleteEntity(response);
-            }
+    private void deleteTeamResponses(String courseId, String oldTeam) {
+        List<FeedbackResponseAttributes> responsesToOldTeam =
+                getFeedbackResponsesForReceiverForCourse(courseId, oldTeam);
+        for (FeedbackResponseAttributes response : responsesToOldTeam) {
+            frDb.deleteEntity(response);
         }
+    }
 
+    private void deleteResponsesFromTeamToUser(String courseId, String userEmail) {
+        FeedbackQuestionAttributes question;
         List<FeedbackResponseAttributes> responsesToUser =
                 getFeedbackResponsesForReceiverForCourse(courseId, userEmail);
 
@@ -506,29 +526,35 @@ public void updateFeedbackResponsesForChangingTeam(
                 frDb.deleteEntity(response);
             }
         }
+    }
+
+    private void deleteResponsesFromUserToTeam(String courseId, String userEmail) {
+        FeedbackQuestionAttributes question;
 
-        if (studentsLogic.getStudentsForTeam(oldTeam, courseId).isEmpty()) {
-            List<FeedbackResponseAttributes> responsesToTeam =
-                    getFeedbackResponsesForReceiverForCourse(courseId, oldTeam);
-            for (FeedbackResponseAttributes response : responsesToTeam) {
+        List<FeedbackResponseAttributes> responsesFromUser =
+                getFeedbackResponsesFromGiverForCourse(courseId, userEmail);
+
+        for (FeedbackResponseAttributes response : responsesFromUser) {
+            question = fqLogic.getFeedbackQuestion(response.feedbackQuestionId);
+            if (question.giverType == FeedbackParticipantType.TEAMS
+                    || isRecipientTypeTeamMembers(question)) {
                 frDb.deleteEntity(response);
             }
         }
     }
 
+    /**
+     * Updates responses for a student when his section changes.
+     */
     public void updateFeedbackResponsesForChangingSection(
             String courseId, String userEmail, String oldSection, String newSection)
             throws EntityDoesNotExistException, InvalidParametersException {
+        updateSectionOfResponsesFromUser(courseId, userEmail, newSection);
+        updateSectionOfResponsesToUser(courseId, userEmail, newSection);
+    }
 
-        List<FeedbackResponseAttributes> responsesFromUser =
-                getFeedbackResponsesFromGiverForCourse(courseId, userEmail);
-
-        for (FeedbackResponseAttributes response : responsesFromUser) {
-            response.giverSection = newSection;
-            frDb.updateFeedbackResponse(response);
-            frcLogic.updateFeedbackResponseCommentsForResponse(response.getId());
-        }
-
+    private void updateSectionOfResponsesToUser(String courseId, String userEmail, String newSection)
+            throws InvalidParametersException, EntityDoesNotExistException {
         List<FeedbackResponseAttributes> responsesToUser =
                 getFeedbackResponsesForReceiverForCourse(courseId, userEmail);
 
@@ -537,7 +563,18 @@ public void updateFeedbackResponsesForChangingSection(
             frDb.updateFeedbackResponse(response);
             frcLogic.updateFeedbackResponseCommentsForResponse(response.getId());
         }
+    }
+
+    private void updateSectionOfResponsesFromUser(String courseId, String userEmail, String newSection)
+            throws InvalidParametersException, EntityDoesNotExistException {
+        List<FeedbackResponseAttributes> responsesFromUser =
+                getFeedbackResponsesFromGiverForCourse(courseId, userEmail);
 
+        for (FeedbackResponseAttributes response : responsesFromUser) {
+            response.giverSection = newSection;
+            frDb.updateFeedbackResponse(response);
+            frcLogic.updateFeedbackResponseCommentsForResponse(response.getId());
+        }
     }
 
     public boolean updateFeedbackResponseForChangingTeam(StudentEnrollDetails enrollment,

src/main/java/teammates/logic/core/FeedbackSessionsLogic.java

@@ -2127,9 +2127,9 @@ private void addVisibilityToTable(Map<String, boolean[]> visibilityTable,
             UserRole role,
             CourseRoster roster) {
         boolean[] visibility = new boolean[2];
-        visibility[Const.VISIBILITY_TABLE_GIVER] = frLogic.isNameVisibleTo(
+        visibility[Const.VISIBILITY_TABLE_GIVER] = frLogic.shouldNameBeVisibleToUser(
                 question, response, userEmail, role, true, roster);
-        visibility[Const.VISIBILITY_TABLE_RECIPIENT] = frLogic.isNameVisibleTo(
+        visibility[Const.VISIBILITY_TABLE_RECIPIENT] = frLogic.shouldNameBeVisibleToUser(
                 question, response, userEmail, role, false, roster);
         visibilityTable.put(response.getId(), visibility);
     }

src/main/java/teammates/ui/controller/Action.java

@@ -27,6 +27,7 @@
 import teammates.logic.api.GateKeeper;
 import teammates.logic.api.Logic;
 import teammates.logic.api.TaskQueuer;
+import teammates.ui.pagedata.PageData;
 
 /** An 'action' to be performed by the system. If the logged in user is allowed
  * to perform the requested action, this object can talk to the back end to
@@ -337,7 +338,7 @@ private void setRedirectPage(String redirectUrl) {
         statusToAdmin = "Redirecting user to " + redirectUrl;
     }
 
-    protected String getAuthenticationRedirectUrl() {
+    public String getAuthenticationRedirectUrl() {
         return authenticationRedirectUrl;
     }
 

src/main/java/teammates/ui/controller/AdminAccountDetailsPageAction.java

@@ -8,6 +8,7 @@
 import teammates.common.datatransfer.CourseDetailsBundle;
 import teammates.common.exception.EntityDoesNotExistException;
 import teammates.common.util.Const;
+import teammates.ui.pagedata.AdminAccountDetailsPageData;
 
 public class AdminAccountDetailsPageAction extends Action {
 

src/main/java/teammates/ui/controller/AdminAccountManagementPageAction.java

@@ -8,6 +8,7 @@
 import teammates.common.datatransfer.AccountAttributes;
 import teammates.common.datatransfer.InstructorAttributes;
 import teammates.common.util.Const;
+import teammates.ui.pagedata.AdminAccountManagementPageData;
 
 public class AdminAccountManagementPageAction extends Action {
 

src/main/java/teammates/ui/controller/AdminActivityLogPageAction.java

@@ -18,6 +18,7 @@
 import teammates.common.util.StatusMessageColor;
 import teammates.common.util.TimeHelper;
 import teammates.common.util.Version;
+import teammates.ui.pagedata.AdminActivityLogPageData;
 
 import com.google.appengine.api.log.AppLogLine;
 

src/main/java/teammates/ui/controller/AdminEmailComposePageAction.java

@@ -3,6 +3,7 @@
 import teammates.common.util.Const;
 import teammates.common.util.StatusMessage;
 import teammates.common.util.StatusMessageColor;
+import teammates.ui.pagedata.AdminEmailComposePageData;
 
 public class AdminEmailComposePageAction extends Action {