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>
This commit is contained in:
parent
cf40420eb1
commit
450626837e
File diff suppressed because one or more lines are too long
@ -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);
|
||||
|
@ -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
|
||||
|
@ -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);
|
||||
}
|
||||
|
@ -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);
|
||||
|
40
client/src/main/java/envoy/client/util/IssueUtil.java
Normal file
40
client/src/main/java/envoy/client/util/IssueUtil.java
Normal file
@ -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;
|
||||
}
|
||||
}
|
File diff suppressed because one or more lines are too long
@ -6,14 +6,13 @@ 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;
|
||||
|
||||
@ -22,15 +21,13 @@ public class IssueProposal extends Event<String> {
|
||||
/**
|
||||
* @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)
|
||||
* @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; }
|
||||
|
||||
}
|
||||
|
File diff suppressed because one or more lines are too long
@ -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);
|
||||
|
Reference in New Issue
Block a user