Skip to content

Commit

Permalink
[#10156] Clean up unused backend CSV generation code (#10254)
Browse files Browse the repository at this point in the history
* clean up unused code in question/response details & attributes

* standardize getter/setter in question details

* fix bug where instructor can post comment for contrib question & refactor to make use of polymorphism in question details

* disallow participant comment on MSQ (follow V6)
  • Loading branch information
xpdavid authored and wkurniawan07 committed Jun 30, 2020
1 parent cab1303 commit b92bd1a
Show file tree
Hide file tree
Showing 96 changed files with 543 additions and 6,128 deletions.
9 changes: 2 additions & 7 deletions docs/snapshot-testing.md
Expand Up @@ -27,9 +27,9 @@ For such testing method to be effective, before the changes are committed, a *ma

## How do we use Snapshot Testing?

For the web page comparison, the tests are done in the front-end using Jest. [Jest has native support for snapshot testing (in fact, that is where the name is obtained from!)](https://jestjs.io/docs/en/snapshot-testing). Auto-update mode is activated by pressing `u` when running Jest under watch mode.
For the web page comparison and CSV content generation, the tests are done in the front-end using Jest. [Jest has native support for snapshot testing (in fact, that is where the name is obtained from!)](https://jestjs.io/docs/en/snapshot-testing). Auto-update mode is activated by pressing `u` when running Jest under watch mode.

For email and CSV content generation, the tests are done in the back-end. Auto-update mode is activated by setting the value of `test.snapshot.update` to `true` in `test.properties`.
For email generation, the tests are done in the back-end. Auto-update mode is activated by setting the value of `test.snapshot.update` to `true` in `test.properties`.

## When do we use Snapshot Testing?

Expand Down Expand Up @@ -58,11 +58,6 @@ The same idea applies to email content test:
EmailChecker.verifyEmailContent(email, recipient, subject, "/studentCourseJoinEmail.html");
```

And to CSV content test:

```java
CsvChecker.verifyCsvContent(csvContent, "/feedbackSessionResults.csv");
```

## When do we NOT use Snapshot Testing?

Expand Down
1,766 changes: 0 additions & 1,766 deletions src/main/java/teammates/common/datatransfer/FeedbackSessionResultsBundle.java

This file was deleted.

Expand Up @@ -161,27 +161,6 @@ public boolean isValid() {
return getInvalidityInfo().isEmpty();
}

public boolean isGiverAStudent() {
return giverType == FeedbackParticipantType.SELF
|| giverType == FeedbackParticipantType.STUDENTS;
}

public boolean isRecipientNameHidden() {
return recipientType == FeedbackParticipantType.NONE
|| recipientType == FeedbackParticipantType.SELF;
}

public boolean isRecipientAStudent() {
return recipientType == FeedbackParticipantType.SELF
|| recipientType == FeedbackParticipantType.STUDENTS
|| recipientType == FeedbackParticipantType.OWN_TEAM_MEMBERS
|| recipientType == FeedbackParticipantType.OWN_TEAM_MEMBERS_INCLUDING_SELF;
}

public boolean isRecipientInstructor() {
return recipientType == FeedbackParticipantType.INSTRUCTORS;
}

public boolean isResponseVisibleTo(FeedbackParticipantType userType) {
return showResponsesTo.contains(userType);
}
Expand Down Expand Up @@ -338,40 +317,6 @@ public boolean equals(Object obj) {
return true;
}

public void updateValues(FeedbackQuestionAttributes newAttributes) {
// These can't be changed anyway. Copy values to defensively avoid invalid parameters.
newAttributes.feedbackSessionName = this.feedbackSessionName;
newAttributes.courseId = this.courseId;

if (newAttributes.questionDetails == null) {
newAttributes.questionDetails = getQuestionDetails();
}

if (newAttributes.questionDescription == null) {
newAttributes.questionDescription = this.questionDescription;
}

if (newAttributes.giverType == null) {
newAttributes.giverType = this.giverType;
}

if (newAttributes.recipientType == null) {
newAttributes.recipientType = this.recipientType;
}

if (newAttributes.showResponsesTo == null) {
newAttributes.showResponsesTo = this.showResponsesTo;
}

if (newAttributes.showGiverNameTo == null) {
newAttributes.showGiverNameTo = this.showGiverNameTo;
}

if (newAttributes.showRecipientNameTo == null) {
newAttributes.showRecipientNameTo = this.showRecipientNameTo;
}
}

public void removeIrrelevantVisibilityOptions() {
List<FeedbackParticipantType> optionsToRemove = new ArrayList<>();

Expand Down
Expand Up @@ -2,7 +2,6 @@

import java.time.Instant;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;

Expand Down Expand Up @@ -89,7 +88,7 @@ public static FeedbackResponseAttributes valueOf(FeedbackResponse fr) {
}

public FeedbackQuestionType getFeedbackQuestionType() {
return responseDetails.questionType;
return responseDetails.getQuestionType();
}

public String getId() {
Expand Down Expand Up @@ -223,19 +222,6 @@ private FeedbackResponseDetails deserializeResponseFromSerializedString(String s
return JsonUtils.fromJson(serializedResponseDetails, questionType.getResponseDetailsClass());
}

/**
* Checks if this object represents a missing response.
* A missing response should never be written to the database.
* It should only be used as a representation.
*/
public boolean isMissingResponse() {
return responseDetails == null;
}

public static void sortFeedbackResponses(List<FeedbackResponseAttributes> frs) {
frs.sort(Comparator.comparing(FeedbackResponseAttributes::getId));
}

/**
* Returns a builder for {@link FeedbackResponseAttributes}.
*/
Expand Down
Expand Up @@ -6,10 +6,6 @@
import java.util.List;
import java.util.Objects;

import org.jsoup.Jsoup;
import org.jsoup.nodes.Element;
import org.jsoup.select.Elements;

import teammates.common.datatransfer.FeedbackParticipantType;
import teammates.common.util.Assumption;
import teammates.common.util.Const;
Expand Down Expand Up @@ -178,43 +174,6 @@ public boolean isCommentFromFeedbackParticipant() {
return isCommentFromFeedbackParticipant;
}

/**
* Converts comment text in form of string for csv i.e if it contains image, changes it into link.
*
* @return Comment in form of string
*/
public String getCommentAsCsvString() {
String htmlText = commentText;
StringBuilder comment = new StringBuilder(200);
comment.append(Jsoup.parse(htmlText).text());
convertImageToLinkInComment(comment, htmlText);
return SanitizationHelper.sanitizeForCsv(comment.toString());
}

/**
* Converts comment text in form of string.
*
* @return Comment in form of string
*/
public String getCommentAsHtmlString() {
String htmlText = commentText;
StringBuilder comment = new StringBuilder(200);
comment.append(Jsoup.parse(htmlText).text());
convertImageToLinkInComment(comment, htmlText);
return SanitizationHelper.sanitizeForHtml(comment.toString());
}

// Converts image in comment text to link.
private void convertImageToLinkInComment(StringBuilder comment, String htmlText) {
if (!(Jsoup.parse(htmlText).getElementsByTag("img").isEmpty())) {
comment.append(" Images Link: ");
Elements ele = Jsoup.parse(htmlText).getElementsByTag("img");
for (Element element : ele) {
comment.append(element.absUrl("src") + ' ');
}
}
}

/**
* Use only to match existing and known Comment.
*/
Expand Down

0 comments on commit b92bd1a

Please sign in to comment.