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.

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

Prompt for operator name #721

Merged
merged 12 commits into from
Jul 2, 2023
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,23 +79,23 @@ The Tabulator includes several example contest configuration files and associate

## Command-Line Interface

Alternatively, you can run the Tabulator using the command-line interface by including the flag `-cli` and then supplying a path to an existing config file, e.g.:
Alternatively, you can run the Tabulator using the command-line interface by including the flag `--cli` and then supplying a path to an existing config file, e.g.:

`$ rcv -cli path/to/config`
`$ rcv --cli path/to/config`

Or, if you're compiling and running using Gradle:

`$ gradlew run --args="-cli path/to/config"`
`$ gradlew run --args="--cli path/to/config"`

You can also activate a special `convert-to-cdf` function via the command line to export the CVR as a NIST common data format (CDF) .json instead of tabulating the results, e.g.:

`$ rcv -cli path/to/config convert-to-cdf`
`$ rcv --cli path/to/config convert-to-cdf`

This option is available in the GUI by selecting the "Conversion > Convert CVRs in Current Config to CDF" menu option.

Or, again, if you're compiling and running using Gradle:

`$ gradlew run --args="-cli path/to/config convert-to-cdf"`
`$ gradlew run --args="--cli path/to/config convert-to-cdf"`

Note: if you convert a source to CDF and that source uses an overvoteLabel or an undeclaredWriteInLabel, the label will be represented differently in the generated CDF source file than it was in the original CVR source. When you create a new config using this generated CDF source file and you need to set overvoteLabel, you should use "overvote". If you need to set undeclaredWriteInLabel, you should use "Undeclared Write-ins".

Expand Down
11 changes: 6 additions & 5 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ repositories {
dependencies {
implementation 'org.apache.commons:commons-csv:1.9.0'
implementation 'org.apache.poi:poi-ooxml:5.2.2'
implementation 'commons-cli:commons-cli:1.5.0'
implementation 'com.fasterxml.jackson.core:jackson-core:2.13.3'
implementation 'com.fasterxml.jackson.core:jackson-annotations:2.13.3'
implementation 'com.fasterxml.jackson.core:jackson-databind:2.13.3'
Expand All @@ -29,11 +30,11 @@ application {
mainClass = "${moduleName}.Main"
}

// Uncomment below to simulate running from the CLI
//run {
// standardInput = System.in
// args = ["-cli", "path/to/config"]
//}
// Required to accept a name and tiebreak interactively via CLI
run {
standardInput = System.in
args = ["-cli", "path/to/config"]
HEdingfield marked this conversation as resolved.
Show resolved Hide resolved
}
HEdingfield marked this conversation as resolved.
Show resolved Hide resolved

// ### Checkstyle plugin settings
checkstyle {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
requires javafx.graphics;
requires java.xml;
requires org.apache.poi.ooxml;
requires commons.cli;
// enable reflexive calls from network.brightspots.rcv into javafx.fxml
opens network.brightspots.rcv;
// our main module
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/network/brightspots/rcv/GuiConfigController.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import javafx.scene.control.TableView;
import javafx.scene.control.TextArea;
import javafx.scene.control.TextField;
import javafx.scene.control.TextInputDialog;
import javafx.scene.control.cell.PropertyValueFactory;
import javafx.stage.DirectoryChooser;
import javafx.stage.FileChooser;
Expand Down Expand Up @@ -460,6 +461,7 @@ public void menuItemTabulateClicked() {
if (filePathAndTempStatus != null) {
if (GuiContext.getInstance().getConfig() != null) {
setGuiIsBusy(true);
askUserForName();
TabulatorService service = new TabulatorService(
filePathAndTempStatus.getKey(), filePathAndTempStatus.getValue());
setUpAndStartService(service);
Expand All @@ -478,6 +480,7 @@ public void menuItemConvertToCdfClicked() {
if (filePathAndTempStatus != null) {
if (GuiContext.getInstance().getConfig() != null) {
setGuiIsBusy(true);
askUserForName();
ConvertToCdfService service = new ConvertToCdfService(
filePathAndTempStatus.getKey(), filePathAndTempStatus.getValue());
setUpAndStartService(service);
Expand Down Expand Up @@ -945,6 +948,21 @@ private ConfigComparisonResult compareConfigs() {
return comparisonResult;
}

private void askUserForName() {
TextInputDialog dialog = new TextInputDialog();
dialog.setTitle("Enter your name");
dialog.setHeaderText("For auditing purposes, enter the name(s) of everyone currently "
+ "operating this machine.");
dialog.setContentText("Name:");
Optional<String> result = dialog.showAndWait();
HEdingfield marked this conversation as resolved.
Show resolved Hide resolved

if (result.isPresent()) {
Logger.info("Name(s) of operators, as entered interactively: " + result.get());
HEdingfield marked this conversation as resolved.
Show resolved Hide resolved
} else {
Logger.info("Operator(s) did not enter a name.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we block this / throw an exception here instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I prefer this, since when developing, I often hit tabulate a lot and can just hit cancel.

My reasoning is that we can't guarantee they'll enter a correct name, so why pretend to validate? They could enter foo instead of empty string. Just let them do empty string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, but if that's the case, why do we force the user to provide a value in the CLI version? Shouldn't we treat this consistently?

I'm ok with allowing the user to bypass without entering a name but, if they do, we should probably log a warning (or maybe just an info) saying the user didn't provide a name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We already log an INFO that user didn't provide a name (Operator(s) did not enter a name.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can update the CLI to not fail on empty string, but I fear it's more hidden there. Especially when running via gradle, the line <===========--> 85% EXECUTING [20s] shows up after the request for a name, so it could be hidden.

In other words, empty string in CLI is more likely to be accidental, whereas in tabulation, it's more obvious.

Happy to make changes in either direction!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think an argument in favor of throwing an exception / requiring this in both cases is that the user might accidentally bypass the message box with a mis-click or mis-key. I say we should go forward with it. It's really easy to just enter "asdf" when testing here anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or aoeu, depending on keyboard layout ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Whoa whoa whoa, are you one of those? I'm impressed... 👽

}
}

private boolean checkForSaveAndContinue() {
boolean willContinue = false;
ConfigComparisonResult comparisonResult = compareConfigs();
Expand Down
110 changes: 76 additions & 34 deletions src/main/java/network/brightspots/rcv/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,17 @@

package network.brightspots.rcv;

import java.util.ArrayList;
import java.util.List;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Scanner;
import java.util.stream.Stream;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.CommandLineParser;
import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.HelpFormatter;
import org.apache.commons.cli.Option;
import org.apache.commons.cli.Options;
import org.apache.commons.cli.ParseException;

/**
* Main entry point to RCTab.
Expand All @@ -42,41 +51,36 @@ public static void main(String[] args) {
Logger.setup();
logSystemInfo();

// Determine if user intends to use the command-line interface, and gather args if so
boolean useCli = false;
List<String> argsCli = new ArrayList<>();
for (String arg : args) {
if (!useCli && arg.equals("-cli")) {
useCli = true;
} else if (useCli) {
argsCli.add(arg);
}
}

if (!useCli) {
// Launch the GUI
// Check if args contains string "--cli"
if (Arrays.stream(args).filter(arg -> arg.equals("--cli")).findAny().isEmpty()) {
// --cli not found. Launch the GUI
launch(args);
} else {
Logger.info("Tabulator is being used via the CLI.");
// Check for unexpected input
if (argsCli.size() == 0) {
Logger.severe(
"""
No config file path provided on command line!
Please provide a path to the config file!
See README.md for more details.""");
System.exit(1);
} else if (argsCli.size() > 2) {
Logger.severe(
"Too many arguments! Max is 2 but got: %d\n" + "See README.md for more details.",
argsCli.size());
System.exit(2);
CommandLine cmd = parseArgsForCli(args);
String path = cmd.getOptionValue("cli");
String name = cmd.getOptionValue("name");
Boolean convertToCdf = cmd.hasOption("convert-to-cdf");

if (name == null || name.isEmpty()) {
// Prompt user for their name
System.out.println("Enter name(s) of operators for auditing purposes:");
HEdingfield marked this conversation as resolved.
Show resolved Hide resolved

Scanner sc = new Scanner(System.in, StandardCharsets.UTF_8);
if (sc.hasNextLine()) {
name = sc.nextLine();
Logger.info("Name(s) of operators, as entered interactively: " + name);
} else {
Logger.severe("Must supply name(s) as a CLI argument or run on an interactive shell");
System.exit(1);
}
} else {
Logger.info("Name(s) of operators, as entered in a command-line argument: " + name);
}
// Path to either: config file for configuring the tabulator, or Dominion JSONs
String providedPath = argsCli.get(0);
// Session object will manage the tabulation process
TabulatorSession session = new TabulatorSession(providedPath);
if (argsCli.size() == 2 && argsCli.get(1).equals("convert-to-cdf")) {

Logger.info("Tabulator is being used via the CLI");
artoonie marked this conversation as resolved.
Show resolved Hide resolved

TabulatorSession session = new TabulatorSession(path);
if (convertToCdf) {
session.convertToCdf();
} else {
session.tabulate();
Expand All @@ -86,6 +90,44 @@ public static void main(String[] args) {
System.exit(0);
}

// Call this function if using the command line interface. Do not call if --cli
// has not been passed as an argument; it will fail.
private static CommandLine parseArgsForCli(String[] args) {
// Remove all args that start with "-D" -- these are added automatically when running via
// IntelliJ
Stream<String> filteredArgs = Arrays.stream(args).filter(arg -> !arg.startsWith("-D"));
args = filteredArgs.toArray(String[]::new);

Options options = new Options();

Option inputPath = new Option("c", "cli", true, "launch command-line version");
inputPath.setRequired(true);
options.addOption(inputPath);

Option doConvert = new Option("x", "convert-to-cdf", false, "convert to CDF");
doConvert.setRequired(false);
options.addOption(doConvert);

Option name = new Option("n", "name", true, "name of current operator");
name.setRequired(false);
options.addOption(name);

CommandLineParser parser = new DefaultParser();
CommandLine cmd = null;

try {
cmd = parser.parse(options, args);
} catch (ParseException e) {
Logger.severe(e.getMessage());
HelpFormatter formatter = new HelpFormatter();
formatter.printHelp("RCTab", options);

System.exit(1);
}

return cmd;
}

private static void logSystemInfo() {
Logger.info("Launching %s version %s...", APP_NAME, APP_VERSION);
Logger.info(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ private static void checkConfigVersionMatchesApp(ContestConfig config) {
Logger.severe(
"Can't use a config with older version %s in newer version %s of the app! To "
+ "automatically migrate the config to the newer version, load it in the graphical "
+ "version of the app (i.e. don't use the -cli flag when starting the tabulator).",
+ "version of the app (i.e. don't use the --cli flag when starting the tabulator).",
version, Main.APP_VERSION);
}
}
Expand Down