Apply suggestions from code review

Additionally moved issue sanitization from server to client.

Co-authored-by: DieGurke <maxi@kske.dev>
Co-authored-by: CyB3RC0nN0R <kske@outlook.de>

client/src/main/java/envoy/client/Main.java

@@ -19,7 +19,7 @@ import envoy.client.ui.Startup;
  */
 public class Main {
 
-	private static final boolean DEBUG = false;
+	private static final boolean debug = false;
 
 	/**
 	 * Starts the application.
@@ -29,7 +29,7 @@ public class Main {
 	 * @since Envoy Client v0.1-beta
 	 */
 	public static void main(String[] args) {
-		if (DEBUG) {
+		if (debug) {
 			// Put testing code here
 			System.out.println();
 			System.exit(0);

client/src/main/java/envoy/client/ui/controller/SettingsScene.java

@@ -36,12 +36,10 @@ public class SettingsScene {
 	 */
 	public void initializeData(SceneContext sceneContext, Client client) {
 		this.sceneContext = sceneContext;
-		final var	user	= client.getSender();
-		final var	online	= client.isOnline();
 		settingsList.getItems().add(new GeneralSettingsPane());
-		settingsList.getItems().add(new UserSettingsPane(sceneContext, user, online));
+		settingsList.getItems().add(new UserSettingsPane(sceneContext, client.getSender(), client.isOnline()));
 		settingsList.getItems().add(new DownloadSettingsPane(sceneContext));
-		settingsList.getItems().add(new BugReportPane( user, online));
+		settingsList.getItems().add(new BugReportPane(client.getSender(), client.isOnline()));
 	}
 
 	@FXML

client/src/main/java/envoy/client/ui/settings/BugReportPane.java

@@ -5,6 +5,7 @@ import javafx.scene.control.*;
 import javafx.scene.input.InputEvent;
 
 import envoy.client.event.SendEvent;
+import envoy.client.util.IssueUtil;
 import envoy.data.User;
 import envoy.event.EventBus;
 import envoy.event.IssueProposal;
@@ -41,7 +42,7 @@ public class BugReportPane extends OnlyIfOnlineSettingsPane {
 	public BugReportPane(User user, boolean online) {
 		super("Report a bug", online);
 		setSpacing(10);
-		setToolTipText("A bug can only be reported when being online");
+		setToolTipText("A bug can only be reported while being online");
 
 		// Displaying the label to ask for a title
 		titleLabel.setWrapText(true);
@@ -68,8 +69,9 @@ public class BugReportPane extends OnlyIfOnlineSettingsPane {
 		submitReportButton.setDisable(true);
 		submitReportButton.setOnAction(e -> {
 			EventBus.getInstance()
-				.dispatch(new SendEvent(new IssueProposal(titleTextField.getText(), errorDetailArea.getText(),
-						showUsernameInBugReport.isSelected() ? user.getName() : null, true)));
+				.dispatch(new SendEvent(new IssueProposal(titleTextField.getText(),
+						IssueUtil.sanitizeIssueDescription(errorDetailArea.getText(), showUsernameInBugReport.isSelected() ? user.getName() : null),
+						true)));
 		});
 		getChildren().add(submitReportButton);
 	}

client/src/main/java/envoy/client/ui/settings/OnlyIfOnlineSettingsPane.java

@@ -16,14 +16,14 @@ import javafx.scene.paint.Color;
  * <p>
  * Project: <strong>client</strong><br>
  * File: <strong>OnlyIfOnlineSettingsPane.java</strong><br>
- * Created: <strong>Aug 4, 2020</strong><br>
+ * Created: <strong>04.08.2020</strong><br>
  *
  * @author Leon Hofmeister
  * @since Envoy Client v0.2-beta
  */
 public abstract class OnlyIfOnlineSettingsPane extends SettingsPane {
 
-	private final Tooltip beOnlineReminder = new Tooltip("You need to be online to modify your acount.");
+	private final Tooltip beOnlineReminder = new Tooltip("You need to be online to modify your account.");
 
 	/**
 	 * @param title
@@ -32,11 +32,9 @@ public abstract class OnlyIfOnlineSettingsPane extends SettingsPane {
 	protected OnlyIfOnlineSettingsPane(String title, boolean online) {
 		super(title);
 
-		final var offline = !online;
+		setDisable(!online);
 
-		setDisable(offline);
-
-		if (offline) {
+		if (!online) {
 			final var infoLabel = new Label("You shall not pass!\n(... Unless you would happen to be online)");
 			infoLabel.setId("infoLabel-warning");
 			infoLabel.setWrapText(true);

client/src/main/java/envoy/client/util/IssueUtil.java

@@ -0,0 +1,40 @@
+package envoy.client.util;
+
+/**
+ * Provides methods to handle outgoing issues.
+ * <p>
+ * Project: <strong>client</strong><br>
+ * File: <strong>IssueUtil.java</strong><br>
+ * Created: <strong>20.08.2020</strong><br>
+ *
+ * @author Leon Hofmeister
+ * @since Envoy Client v0.2-beta
+ */
+public class IssueUtil {
+
+	/**
+	 *
+	 * @since Envoy Client v0.2-beta
+	 */
+	private IssueUtil() {}
+
+	/**
+	 * Performs actions to ensure the description of an issue will be displayed as
+	 * intended by the user.
+	 *
+	 * @param rawDescription the description to sanitize
+	 * @param username       the user who submitted the issue. Should be
+	 *                       {@code null} if he does not want to be named.
+	 * @return the sanitized description
+	 * @since Envoy Client v0.2-beta
+	 */
+	public static String sanitizeIssueDescription(String rawDescription, String username) {
+		// Appending the submitter name, if this option was enabled
+		rawDescription += username != null
+				? (rawDescription.endsWith("\n") || rawDescription.endsWith("<br>") ? "" : "<br>") + String.format("Submitted by user %s.", username)
+				: "";
+		// Markdown does not support "normal" line breaks. It uses "<br>"
+		rawDescription = rawDescription.replaceAll(System.getProperty("line.separator", "\r?\n"), "<br>");
+		return rawDescription;
+	}
+}

common/src/main/java/envoy/event/IssueProposal.java

@@ -6,31 +6,28 @@ package envoy.event;
  * <p>
  * Project: <strong>common</strong><br>
  * File: <strong>IssueProposal.java</strong><br>
- * Created: <strong>Aug 5, 2020</strong><br>
+ * Created: <strong>05.08.2020</strong><br>
  *
  * @author Leon Hofmeister
  * @since Envoy Common v0.2-beta
  */
 public class IssueProposal extends Event<String> {
 
-	private final String	submitterName;
 	private final String	description;
 	private final boolean	bug;
 
 	private static final long serialVersionUID = 1L;
 
 	/**
-	 * @param title         the title of the reported bug
-	 * @param description   the description of this bug
-	 * @param submitterName the user who submitted the bug
-	 * @param isBug         determines whether this {@code IssueProposal} is
-	 *                      supposed to be a
-	 *                      feature or a bug (true = bug, false = feature)
+	 * @param title       the title of the reported bug
+	 * @param description the description of this bug
+	 * @param isBug       determines whether this {@code IssueProposal} is
+	 *                    supposed to be a
+	 *                    feature or a bug (true = bug, false = feature)
 	 * @since Envoy Common v0.2-beta
 	 */
-	public IssueProposal(String title, String description, String submitterName, boolean isBug) {
+	public IssueProposal(String title, String description, boolean isBug) {
 		super(title);
-		this.submitterName	= submitterName;
 		this.description	= description;
 		bug					= isBug;
 	}
@@ -42,16 +39,9 @@ public class IssueProposal extends Event<String> {
 	public String getDescription() { return description; }
 
 	/**
-	 * @return the name of the user who sent this bug report
-	 * @since Envoy Common v0.2-beta
-	 */
-	public String getSubmitterName() { return submitterName; }
-
-	/**
-	 * @return whether this issue is supposed to be a bug - if false it is intended
+	 * @return whether this issue is supposed to be a bug - otherwise it is intended
 	 *         as a feature
 	 * @since Envoy Common v0.2-beta
 	 */
 	public boolean isBug() { return bug; }
-
 }

server/src/main/java/envoy/server/processors/IssueProposalProcessor.java

@@ -16,7 +16,7 @@ import envoy.util.EnvoyLog;
  * <p>
  * Project: <strong>server</strong><br>
  * File: <strong>IssueProposalProcessor.java</strong><br>
- * Created: <strong>Aug 5, 2020</strong><br>
+ * Created: <strong>05.08.2020</strong><br>
  *
  * @author Leon Hofmeister
  * @since Envoy Server v0.2-beta
@@ -30,16 +30,6 @@ public class IssueProposalProcessor implements ObjectProcessor<IssueProposal> {
 	public void process(IssueProposal issueProposal, long socketID, ObjectWriteProxy writeProxy) throws IOException {
 		// Do nothing if manually disabled
 		if (!issueReportingEnabled) return;
-		var issueDescription = issueProposal.getDescription();
-		// Appending the submitter name, if this option was enabled
-		issueDescription += issueProposal.getSubmitterName() != null
-				? (issueDescription.endsWith("\n") || issueDescription.endsWith("<br>") ? "" : "<br>")
-						+ String.format("Submitted by user %s.", issueProposal.getSubmitterName())
-				: "";
-		// Markdown does not support "\n". It uses "<br>"
-		issueDescription = issueDescription.replaceAll("\n", "<br>");
-		// We do not want any Windows artifacts to remain as that may cause problems
-		issueDescription = issueDescription.replaceAll("\r", "");
 		try {
 			final var	url			= new URL(
 					"https://git.kske.dev/api/v1/repos/zdm/envoy/issues?access_token=6d8ec2a72d64cbaf6319434aa2e7caf0130701b3");
@@ -51,7 +41,7 @@ public class IssueProposalProcessor implements ObjectProcessor<IssueProposal> {
 
 			final var json = String.format("{\"title\":\"%s\",\"body\":\"%s\",\"labels\":[240, %d]}",
 					issueProposal.get(),
-					issueDescription,
+					issueProposal.getDescription(),
 					// Label 240 should be user-made, label 117 bug and label 119 feature
 					issueProposal.isBug() ? 117 : 119);
 			try (final var os = connection.getOutputStream()) {
@@ -61,10 +51,11 @@ public class IssueProposalProcessor implements ObjectProcessor<IssueProposal> {
 			final var status = connection.getResponseCode();
 			if (status == 201) logger.log(Level.INFO, "Successfully created an issue");
 			else logger.log(Level.WARNING,
-					String.format("Tried creating an issue but received status code %d - Request params:title=%s,description=%s,json=%s",
+					String.format("Tried creating an issue for %s but received status code %d - Request params:title=%s,description=%s,json=%s",
+							url,
 							status,
 							issueProposal.get(),
-							issueDescription,
+							issueProposal.getDescription(),
 							json));
 		} catch (final IOException e) {
 			logger.log(Level.WARNING, "An error occurred while creating an issue: ", e);