Import/Export command #232

Scrubbius · 2018-11-05T17:49:21Z

import commands added on top of the export ones.

…r.java

coveralls · 2018-11-06T00:56:39Z

Coverage decreased (-5.7%) to 65.148% when pulling 5c77c98 on Scrubbius:ImportCommands into f77c7fd on CS2103-AY1819S1-W16-2:master.

sharan8 · 2018-11-06T03:01:51Z

Hey Wen Jun, some thoughts & suggestions:

It might it be worth exploring taking in -csv or -xml as an input parameter for abstracted exportvolunteer and importvolunteer commands instead, and applying the corresponding execution? Felt that this may help reduce duplication of code through applying the concept of abstraction.
The files could perhaps be exported to the user's current working directory instead of Desktop, as this is a portable application and doing so would allow the user to access and manage the files related to the application in a more consolidated manner. This is also what is done in the exportcert command, and thus we may be able to achieve more consistency across the application by doing so. A more specific suggestion would be to export it to a folder in the user's current working directory. Feel free to draw inspiration from my implementation of the exportcert command here.
In order to accommodate duplicate event/volunteer names, the naming of the exported files could be updated to include perhaps the event ID or volunteer ID as well (e.g. <EVENTNAME>_<EVENTID>.csv). With the current implementation, unique events with the same name will have their exported files overwriting each other.

Looking forward to test out how your feature works!

src/main/java/seedu/address/logic/commands/ExportEventXmlCommand.java

sharan8 · 2018-11-06T03:07:06Z

src/main/java/seedu/address/logic/commands/ExportEventXmlCommand.java

+
+        // Setting up file path
+        File output = new File(System.getProperty("user.home") + "/Desktop/"
+                + event.getName().toString() + ".xml");


Suggestion to include the event ID here as well, such that the export file name takes the format <EVENTNAME>_<EVENTID>.xml

sharan8 · 2018-11-06T03:12:50Z

src/main/java/seedu/address/logic/commands/ExportEventXmlCommand.java

+        DOMSource source = new DOMSource(doc);
+
+        // Setting up file path
+        File output = new File(System.getProperty("user.home") + "/Desktop/"


Would personally suggest extracting System.getProperty("user.home") + "/Desktop/" out to a field named SAVE_PATH, so as to be able to plug it in multiple places without worrying about errors or having to edit each usage location separately if you wish to update the save path.

src/main/java/seedu/address/logic/commands/ImportVolunteerCsvCommand.java

sharan8 · 2018-11-06T03:21:11Z

src/main/java/seedu/address/logic/commands/ExportEventXmlCommand.java

+
+        try {
+            createEventXml(model, selectedEvent);
+        } catch (Exception e) {


It might be useful to find out what type(s) of Exception may result from running this method call, and handling them more specifically

Used it as a blanket statement for now because of possible file creation exceptions when handling the xml transformer and writing throws out to an error at creating the file. I will update the individual exceptions accordingly although they have the same handler

iMarbles · 2018-11-06T03:26:19Z

src/main/java/seedu/address/logic/commands/ExportVolunteerCsvCommand.java

+     */
+    private void createVolunteerCsv(ObservableList<Volunteer> list, ArrayList<Index> index) throws Exception {
+        // Setting up file path
+        File output = new File(System.getProperty("user.home") + "/Desktop/"


Desktop might not be a good choice. Better to use relative path

iMarbles · 2018-11-06T03:30:07Z

src/main/java/seedu/address/logic/commands/ImportVolunteerCsvCommand.java

+
+                if (!model.hasVolunteer(volunteer)) {
+                    model.addVolunteer(volunteer);
+                    model.commitAddressBook();


Why is the commit here? Placing it here means:

When user uses the undo command, it only removes the add of the last volunteer added.
It is better to put the model.commitAddressBook() method outside. -> When ALL the volunteers have been added.

noted and will update accordingly

Kratious · 2018-11-06T03:40:03Z

src/main/java/seedu/address/logic/parser/ExportVolunteerXmlCommandParser.java

+    public ExportVolunteerXmlCommand parse(String userInput) throws ParseException {
+        requireNonNull(userInput);
+
+        Index index;


Does this command only take in 1 volunteer to export in the current state?
Maybe should have a good use case for this command before updating, XML to XML doesn't seem to be a feature that makes sense from the end user perspective.

Current state takes in only 1 volunteer, but it calls out all the events this volunteer has participated in and logs the information about these events as well into the xml output. Main focus of this feature is to list all the events a volunteer has participated as an export.

Kratious · 2018-11-06T03:48:56Z

src/main/java/seedu/address/logic/commands/ExportVolunteerCsvCommand.java

+
+        try {
+            createVolunteerCsv(model.getFilteredVolunteerList(), index);
+        } catch (Exception e) {


Like Sharan mentioned, should handle specific exceptions instead of using a catch all Exception

Kratious · 2018-11-06T03:51:26Z

src/main/java/seedu/address/logic/commands/ExportVolunteerCsvCommand.java

+        // Setting up writer & stringbuilder for appending
+        PrintWriter pw = new PrintWriter(output);
+        StringBuilder sb = new StringBuilder();
+        String csvSplit = ",";


Might want to test this with fields that could contain commas, like Address

Assumption on the csv entries dont have characters which messes with the tuple splits, like comma (,)and bracer ("").

You should not work under this assumption. Users should be allowed to enter commas for addresses. Try to find a way to clean the input. Maybe this link would be helpful.

https://stackoverflow.com/questions/4617935/is-there-a-way-to-include-commas-in-csv-columns-without-breaking-the-formatting

…nd.java Co-Authored-By: Scrubbius <36285531+Scrubbius@users.noreply.github.com>

…ommand.java Co-Authored-By: Scrubbius <36285531+Scrubbius@users.noreply.github.com>

ExportEventXml now has event ID on part of the filename File directory created for the specific exports as well with the desktop as a fall-back

Scrubbius · 2018-11-06T05:28:48Z

Not going to split the export command into -csv and -xml as part of the parameters since the xml command takes in only 1 volunteer at a time while the csv command merges all into one file.

Maybe the xml variant can be updated in the future to take in more than one volunteer index and produce the files accordingly but for now it is an auxiliary command for individual information exporting.

…to ImportCommands Conflicts resolved

…mmas Update exportvolunteercsv to have compatibility with the import format

Scrubbius and others added 10 commits November 5, 2018 23:14

Add back export XML/CSV command and parsers

21c6a0a

Updated for checkstyle errors

9862b23

Rename ExportEventXMLCommandParser.java to ExportEventXmlCommandParse…

8c4074b

…r.java

Update for checkstyle errors

a7f1023

Merge branch 'master' of https://github.com/Scrubbius/main

a128ae1

no message

cdb2c9d

import commands added

d75b703

Import commands added

5b44503

checkstyle fix

aaff9ec

Update importcsvcommand

02989d1

Scrubbius changed the title ~~Import commands~~ Import command Nov 6, 2018

Scrubbius added the status.Ongoing The issue is currently being worked on. note: remove this label before closing an issue. label Nov 6, 2018

Scrubbius added 2 commits November 6, 2018 08:29

Update checkstyle

40d0d9e

checkstyle corrections

b7ec682

Scrubbius added status.Ready The PR is ready to be reviewed. note: remove this label before merging a PR. and removed status.Ongoing The issue is currently being worked on. note: remove this label before closing an issue. labels Nov 6, 2018

sharan8 self-requested a review November 6, 2018 02:48

sharan8 reviewed Nov 6, 2018

View reviewed changes

src/main/java/seedu/address/logic/commands/ExportEventXmlCommand.java Outdated Show resolved Hide resolved

sharan8 reviewed Nov 6, 2018

View reviewed changes

src/main/java/seedu/address/logic/commands/ImportVolunteerCsvCommand.java Show resolved Hide resolved

sharan8 reviewed Nov 6, 2018

View reviewed changes

src/main/java/seedu/address/logic/commands/ImportVolunteerCsvCommand.java Outdated Show resolved Hide resolved

sharan8 reviewed Nov 6, 2018

View reviewed changes

iMarbles reviewed Nov 6, 2018

View reviewed changes

Kratious reviewed Nov 6, 2018

View reviewed changes

sharan8 and others added 3 commits November 6, 2018 12:30

Update src/main/java/seedu/address/logic/commands/ExportEventXmlComma…

01bc12f

…nd.java Co-Authored-By: Scrubbius <36285531+Scrubbius@users.noreply.github.com>

Update src/main/java/seedu/address/logic/commands/ImportVolunteerCsvC…

59412ad

…ommand.java Co-Authored-By: Scrubbius <36285531+Scrubbius@users.noreply.github.com>

Update src/main/java/seedu/address/logic/commands/ImportVolunteerCsvC…

84350ff

…ommand.java Co-Authored-By: Scrubbius <36285531+Scrubbius@users.noreply.github.com>

Scrubbius added status.Ongoing The issue is currently being worked on. note: remove this label before closing an issue. and removed status.Ready The PR is ready to be reviewed. note: remove this label before merging a PR. labels Nov 6, 2018

Update based on reviews

eb59cc4

ExportEventXml now has event ID on part of the filename File directory created for the specific exports as well with the desktop as a fall-back

Scrubbius added 3 commits November 6, 2018 13:33

Merge branch 'ImportCommands' of https://github.com/Scrubbius/main in…

35e929f

…to ImportCommands Conflicts resolved

Fix compile error

c82b8e1

update for checkstyle

d071a94

Scrubbius added status.Ready The PR is ready to be reviewed. note: remove this label before merging a PR. and removed status.Ongoing The issue is currently being worked on. note: remove this label before closing an issue. labels Nov 6, 2018

Scrubbius added 2 commits November 6, 2018 18:51

Update importvolunteercsv to read delimiters for csv in bracer and co…

279d5f2

…mmas Update exportvolunteercsv to have compatibility with the import format

Correcting syntax error in ImportVolunteerCsvCommand

5c77c98

Scrubbius changed the title ~~Import command~~ Import/Export command Nov 6, 2018

iMarbles merged commit ce6a42d into CS2103-AY1819S1-W16-2:master Nov 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import/Export command #232

Import/Export command #232

Scrubbius commented Nov 5, 2018

coveralls commented Nov 6, 2018 •

edited

Loading

sharan8 commented Nov 6, 2018 •

edited

Loading

sharan8 Nov 6, 2018

sharan8 Nov 6, 2018 •

edited

Loading

sharan8 Nov 6, 2018

Scrubbius Nov 6, 2018

iMarbles Nov 6, 2018

iMarbles Nov 6, 2018

Scrubbius Nov 6, 2018

Kratious Nov 6, 2018 •

edited

Loading

Scrubbius Nov 6, 2018 •

edited

Loading

Kratious Nov 6, 2018

Kratious Nov 6, 2018

Scrubbius Nov 6, 2018

iMarbles Nov 6, 2018

Scrubbius commented Nov 6, 2018

Import/Export command #232

Import/Export command #232

Conversation

Scrubbius commented Nov 5, 2018

coveralls commented Nov 6, 2018 • edited Loading

sharan8 commented Nov 6, 2018 • edited Loading

Choose a reason for hiding this comment

sharan8 Nov 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kratious Nov 6, 2018 • edited Loading

Choose a reason for hiding this comment

Scrubbius Nov 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Scrubbius commented Nov 6, 2018

coveralls commented Nov 6, 2018 •

edited

Loading

sharan8 commented Nov 6, 2018 •

edited

Loading

sharan8 Nov 6, 2018 •

edited

Loading

Kratious Nov 6, 2018 •

edited

Loading

Scrubbius Nov 6, 2018 •

edited

Loading