[v1.4][F11-B2] TuitionCor #64

Zhu-Jiahui · 2018-03-08T16:41:22Z

nus-se-pr-bot · 2018-03-08T16:43:30Z

Hi @Zhu-Jiahui, your pull request title is invalid.

For phase A, it should be in the format of [Learning Outcome ID][Team ID] Your Name, where [Learning Outcome ID] has no dashes or spaces (e.g. [W3.1a]) and [Team ID] has one dash only and no spaces (e.g. [W14-A2] means Wednesday 2pm (14 hrs), Phase A, Team 2).

For phase B, it should be in the format of [v1.x][Team ID] Product Name, replacing v1.x with current milestone progress.

Please follow the instructions given strictly and edit your title for reprocessing.

Submit only one learning outcome per pull request (unless otherwise stated in instructions) and do remember to create your branches from the commit where the master branch is pointing at so that each PR you submit only consist of commits meant for the activity.

Note: this comment is posted by a bot. If you believe this is done in error, please create an issue at cs2103-pr-bot and add a link to this PR.

nus-se-pr-bot · 2018-03-09T03:59:26Z

Hi @Zhu-Jiahui,

Your Github username is not recognized. Please post here.

Note: this comment is posted by a bot. If you believe this is done in error, please create an issue at cs2103-pr-bot and add a link to this PR.

karrui

[v1.0 review]
DO NOT CLOSE THIS PR!
Make changes as requested/suggested throughout the weeks until v1.5 :)

Few notes:
Important: I do not see your updated target user profile. Please remember to update them so I can assess the validity or make suggestions.
Be reminded that you will implement 1 big feature each, and it should solve the target user’s problem.
You can also define a person and give a name (to easily refer to him/her later in the doc)

Good to see that you have created issues and v1.0 milestone.
Please remember to add v1.1 to v1.5 milestones by next tutorial -- they can be empty, but should have been created with deadlines.

More issues should be created, they can be related to implementing, documentation, or bugs. I recommend using tags to sort them out.

Please implement comments you think you should follow.

karrui · 2018-03-10T12:51:59Z

README.adoc

@@ -1,7 +1,7 @@
 = Address Book (Level 4)


I don't think this application is named Address Book (Level 4) anymore!

karrui · 2018-03-10T12:52:34Z

docs/DeveloperGuide.adoc

-|`*` |user with many persons in the address book |sort persons by name |locate a person easily
+|`* *` |user |do multi-layer searching |narrow down the options
+
+|`* *` |student |request to know the qualification of the tutor |make my decision wisely


As discussed in the tutorial, this does not really make much sense -- would a student / tutor use this application?

karrui · 2018-03-10T12:53:06Z

docs/DeveloperGuide.adoc

+
+|`* *` |student |request to know the qualification of the tutor |make my decision wisely
+
+|`* *` |tutor |request to know the grades of the students |make my decision wisely


karrui · 2018-03-10T12:54:58Z

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

@@ -9,6 +9,7 @@
 public class HelpCommand extends Command {

    public static final String COMMAND_WORD = "help";
+    public static final String COMMAND_ALIAS = "1";


Does this alias make much sense? It does not fit in the previous established way of aliasing commands. Might be confusing to the user.

karrui · 2018-03-10T12:55:35Z

docs/DeveloperGuide.adoc

@@ -10,7 +10,7 @@ ifdef::env-github[]
 :tip-caption: :bulb:
 :note-caption: :information_source:
 endif::[]
-:repoURL: https://github.com/se-edu/addressbook-level4/tree/master
+:repoURL: https://github.com/CS2103JAN2018-F11-B2/main

 By: `Team SE-EDU`      Since: `Jun 2016`      Licence: `MIT`


Update this line! Preferably XXX forked from Team SE-EDU

Will update in my commit

karrui · 2018-03-10T13:09:28Z

docs/UserGuide.adoc

 === Exiting the program : `exit`

 Exits the program. +
 Format: `exit`

+Alias Format: `x`
+
 === Saving the data


Might want to update the Command Summary below so that the existence of aliases is made known

karrui · 2018-03-10T13:13:30Z

src/test/java/seedu/address/logic/parser/AddressBookParserTest.java

    @Test
    public void parseCommand_redoCommandWord_returnsRedoCommand() throws Exception {
        assertTrue(parser.parseCommand(RedoCommand.COMMAND_WORD) instanceof RedoCommand);
        assertTrue(parser.parseCommand("redo 1") instanceof RedoCommand);
    }

+    @Test
+    public void parseCommand_redoCommandWord_returnsRedoCommandAlias() throws Exception {


Some error in the syntax here, remember that test naming syntax is featureUnderTest_testScenario_expectedBehavior(), so in this case it should be parseCommand_redoCommandAlias_returnsRedoCommand(). Note the change in position of the word alias

karrui · 2018-03-10T13:13:37Z

src/test/java/seedu/address/logic/parser/AddressBookParserTest.java

    @Test
    public void parseCommand_undoCommandWord_returnsUndoCommand() throws Exception {
        assertTrue(parser.parseCommand(UndoCommand.COMMAND_WORD) instanceof UndoCommand);
        assertTrue(parser.parseCommand("undo 3") instanceof UndoCommand);
    }

+    @Test
+    public void parseCommand_undoCommandWord_returnsUndoCommandAlias() throws Exception {


Same thing here

karrui · 2018-03-10T13:20:07Z

src/test/java/seedu/address/ui/testutil/GuiTestAssert.java

+    }
+
+    /**
+     *To be updated


Might want to set it as a TODO: so that it will be highlighted in your IDE. Make sure the basic functionalities of the program is working before you push it to master, as this is the "gold standard" of your application

karrui · 2018-03-10T13:23:13Z

docs/DeveloperGuide.adoc

@@ -806,9 +806,59 @@ Priorities: High (must have) - `* * \*`, Medium (nice to have) - `* \*`, Low (un



The target user profile is not updated. Please update it so I can assess its viability or make suggestions for improvements.

Will update in my commit

karrui · 2018-03-11T09:43:21Z

@Zhu-Jiahui, seems like you changed your Github username and did not update the mapping. Please contact the head TA to remap your github id.

@madsonic @olimhc please help to tell him if he does not see this.

karrui · 2018-03-14T03:13:17Z

Don't need to reply to all of them... Spamming my inbox :(

Zhu-Jiahui · 2018-03-14T03:14:23Z

Noted! Sorry! -- Sent from myMail for Android Wednesday, 14 March 2018, 11:13AM +08:00 from Kar Rui Lau [email protected] :

…

Don't need to reply to all of them... Spamming my inbox :( — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub , or mute the thread .

karrui · 2018-03-17T12:53:29Z

Reviewing now; but a few things to take note for next submission (i.e v1.2 onwards)

Comment here when you are ready for review and give a summary of the changes -- i.e
"Add xxx function"
"Change xxx", etc
So I can see what new changes there are :)

karrui

@Zhu-Jiahui @olimhc @shookshire
v1.1 reviewed, remember to not close your PR.

Some admin matters.
I would want you to mention the following in your Developer guide

For each team member what are the features (major and minor) you are proposing?
Within the scope of the project, how does it fit in (just a one or 2 line summary)
Please do this by end of day Monday 19 March.

karrui · 2018-03-17T12:57:30Z

README.adoc

-** A more sophisticated GUI that includes a list  panel and an in-built Browser.
-** More test cases, including automated GUI testing.
-** Support for _Build Automation_ using Gradle and for _Continuous Integration_ using Travis CI.
+* This is a desktop TuitionCor application. It has a GUI but most of the user interactions happen using a CLI (Command Line Interface).


This does not tell me anything about the application. What is TuitionCor?

Will update in my commit

karrui · 2018-03-17T12:59:04Z

docs/DeveloperGuide.adoc

@@ -10,7 +10,7 @@ ifdef::env-github[]
 :tip-caption: :bulb:
 :note-caption: :information_source:
 endif::[]
-:repoURL: https://github.com/se-edu/addressbook-level4/tree/master
+:repoURL: https://github.com/CS2103JAN2018-F11-B2/main forked from Team SE-EDU

 By: `Team SE-EDU`      Since: `Jun 2016`      Licence: `MIT`


Update this line too

Will update in my commit

karrui · 2018-03-17T12:59:13Z

docs/DeveloperGuide.adoc

@@ -10,7 +10,7 @@ ifdef::env-github[]
 :tip-caption: :bulb:
 :note-caption: :information_source:
 endif::[]
-:repoURL: https://github.com/se-edu/addressbook-level4/tree/master
+:repoURL: https://github.com/CS2103JAN2018-F11-B2/main forked from Team SE-EDU


Provide link to the Team SE-EDU repo too

Will update in my commit

karrui · 2018-03-17T12:59:42Z

docs/DeveloperGuide.adoc

@@ -782,13 +782,25 @@ See this https://github.com/se-edu/addressbook-level4/pull/599[PR] for the step-

 *Target user profile*:

-* has a need to manage a significant number of contacts
+<<<<<<< Updated upstream


Remove merge conflict arrows

Will update in my commit

karrui · 2018-03-17T13:00:12Z

docs/DeveloperGuide.adoc

-*Value proposition*: manage contacts faster than a typical mouse/GUI driven app
+*Value proposition*:
+TuitionCor is targeted at tuition coordinators who have to manage a large amount of contacts.
+The daily job-scope of a tuition coordinator involves the need to manage large amount of contacts and match the students to tutors according to their credentials, needs and location.


karrui · 2018-03-17T13:20:37Z

src/test/java/guitests/guihandles/HelpWindowHandle.java

@@ -27,6 +27,7 @@ public static boolean isWindowPresent() {

    /**
     * Returns the {@code URL} of the currently loaded page.
+     * @TODO: 14/3/2018 Remove this when no longer required


Remember to clear your TODOs if they are done (I don't know if this is applicable or what this TODO is doing)
Remember to make TODOs obvious to anyone seeing your code too -- good habit to have

Will update in my commit

karrui · 2018-03-17T13:22:01Z

src/test/java/seedu/address/logic/parser/AddressBookParserTest.java

    @Test
    public void parseCommand_clear() throws Exception {
-        assertTrue(parser.parseCommand(ClearCommand.COMMAND_WORD) instanceof ClearCommand);
-        assertTrue(parser.parseCommand(ClearCommand.COMMAND_WORD + " 3") instanceof ClearCommand);
+        assertTrue(parser.parseCommand(ClearCommand.COMMAND_ALIAS) instanceof ClearCommand);


Duplicate code? Seems like you removed the wrong lines

karrui · 2018-03-17T13:24:49Z

src/test/java/seedu/address/model/person/SearchContainsKeywordsPredicateTest.java

        assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build()));
+
+        // Keywords match phone, email and address, but does not match name
+        predicate = new SearchContainsKeywordsPredicate(Arrays.asList("12345", "[email protected]", "Main", "Street"));


Can also add more test cases like only phone number, only email, etc

karrui · 2018-03-17T13:25:34Z

src/test/java/seedu/address/ui/testutil/GuiTestAssert.java

+    }
+
+    /**
+     *TODO:


Will update in my commit

karrui · 2018-03-17T13:27:02Z

src/test/java/systemtests/SampleDataTest.java

@@ -46,6 +46,6 @@ private void deleteFileIfExists(String filePath) {
    @Test
    public void addressBook_dataFileDoesNotExist_loadSampleData() {
        Person[] expectedList = SampleDataUtil.getSamplePersons();
-        assertListMatching(getPersonListPanel(), expectedList);
+        assertListMatching(getStudentListPanel(), expectedList);


Might want to add test cases relating to the other panel newly introduced too

karrui · 2018-03-17T13:38:45Z

docs/UserGuide.adoc

 By: `Team SE-EDU`      Since: `Jun 2016`      Licence: `MIT`

 == Introduction

-AddressBook Level 4 (AB4) is for those who *prefer to use a desktop app for managing contacts*. More importantly, AB4 is *optimized for those who prefer to work with a Command Line Interface* (CLI) while still having the benefits of a Graphical User Interface (GUI). If you can type fast, AB4 can get your contact management tasks done faster than traditional GUI apps. Interested? Jump to the <<Quick Start>> to get started. Enjoy!
+TuitionCor is for those who *prefer to use a desktop app for managing client information*. More importantly, TuitionCor is *optimized for those who prefer to work with a Command Line Interface* (CLI) while still having the benefits of a Graphical User Interface (GUI). If you can type fast, TuitionCor can get your client management tasks done faster than traditional GUI apps. Interested? Jump to the <<Quick Start>> to get started. Enjoy!


Please add a Features section to this UserGuide too, detailing what each new version will bring, all the way to v1.5 (and v2.0 for the long term goals)
Basically add a timeline of expected features

karrui · 2018-03-17T13:47:43Z

docs/DeveloperGuide.adoc

@@ -160,7 +160,7 @@ image::UiClassDiagram.png[width="800"]

 *API* : link:{repoURL}/src/main/java/seedu/address/ui/Ui.java[`Ui.java`]

-The UI consists of a `MainWindow` that is made up of parts e.g.`CommandBox`, `ResultDisplay`, `PersonListPanel`, `StatusBarFooter`, `BrowserPanel` etc. All these, including the `MainWindow`, inherit from the abstract `UiPart` class.
+The UI consists of a `MainWindow` that is made up of parts e.g.`CommandBox`, `ResultDisplay`, `PersonListPanel`, `StatusBarFooter` etc. All these, including the `MainWindow`, inherit from the abstract `UiPart` class.


Missing documentation for new features, please remember to add them every week.

shookshire · 2018-03-19T14:24:56Z

Added Feature Contribution to DeveloperGuide

* Add test case for SetCommandParser * minor edits * some edits * Some minor edits based on the helpful suggestion of eugenepeh. Added features into developer guide. * minor edit * minor edits * minor edits * Add set command functionality to keep set of custom command words on top of default command words. Include test cases. * Update features: * importexport * set * Add documentation on set command in developer guide. * minor edits

Edit XmlAdaptedActivityTest

karrui · 2018-03-21T15:45:26Z

@Zhu-Jiahui @shookshire @olimhc Please note the new post about defining your major and minor enhancements - nus-cs2103-AY1718S2/forum#28
Namely, a major enhancement should span multiple stories, with stories spanning multiple tasks, and a minor enhancement should be a single story. Make sure your issue tracker and milestones are properly set up before the tutorial.

A demo issue tracker has also been released for your perusal. You may also find it at https://github.com/MyTAOrg/DemoGradle/issues

Replace dummy text fields with "-"

# Conflicts: # docs/DeveloperGuide.adoc

Developer guide: Sort implementation

DeveloperGuide, UserGuide and PPP Updates

Removed sorting form PPP

Update UGDG

Updates UI image

Update remove manual testing

update PPP

…into Integration

DeveloperGuide: Update instruction for testing & add one user story

Fixes Ui.png not displaying properly

DeveloperGuide: Minor formatting edit

final update to ppp

…into Integration

Fixes rendering error

nus-se-pr-bot added the FormatCheckRequested label Mar 8, 2018

Zhu-Jiahui changed the title ~~[v1.x][F11-B2] TuitionCor~~ [v1.0][F11-B2] TuitionCor Mar 9, 2018

nus-se-pr-bot added GithubUsernameRequested and removed FormatCheckRequested labels Mar 9, 2018

karrui assigned karrui and okkhoy Mar 10, 2018

karrui added team.B2 tutorial.F11 labels Mar 10, 2018

karrui reviewed Mar 10, 2018

View reviewed changes

karrui added the reviewed-v1_0 label Mar 10, 2018

Zhu-Jiahui changed the title ~~[v1.0][F11-B2] TuitionCor~~ [v1.1][F11-B2] TuitionCor Mar 16, 2018

karrui removed the GithubUsernameRequested label Mar 17, 2018

karrui reviewed Mar 17, 2018

View reviewed changes

karrui added the reviewed-v1_1 label Mar 17, 2018

karrui reviewed Mar 17, 2018

View reviewed changes

YuanQLLer pushed a commit to YuanQLLer/addressbook-level4 that referenced this pull request Mar 21, 2018

Merge pull request nus-cs2103-AY1718S2#64 from karenfrilya97/v1.2

1b11904

Edit XmlAdaptedActivityTest

destinnsy and others added 4 commits March 23, 2018 04:18

Merge pull request #34 from olimhc/SplitListV1.1

5b6e42a

Replace dummy text fields with "-"

Update sort implementation

05bd2a8

Merge branch 'master' into DeveloperGuide

6d5fefd

# Conflicts: # docs/DeveloperGuide.adoc

Merge pull request #56 from olimhc/DeveloperGuide

9480296

Developer guide: Sort implementation

olimhc and others added 30 commits April 15, 2018 15:23

Merge pull request #143 from olimhc/Integration

d92a41a

DeveloperGuide, UserGuide and PPP Updates

Removed sorting form PPP

4254e11

Merge pull request #144 from olimhc/Integration

6135761

Removed sorting form PPP

Update DeveloperGuide

a7f780f

Merge branch 'master' of https://github.com/CS2103JAN2018-F11-B2/main

fa216eb

Update userGuide

c1698aa

Update UI image

14ca32c

Merge pull request #146 from shookshire/master

10cf67c

Update UGDG

Updates UserGuide & README

4a35411

Merge pull request #147 from olimhc/Integration

ffed91a

Updates UI image

Update remove manual testing

5429c1b

minor updates to developerGuide

42968ae

fix errors in UGDG

22e83ff

Merge pull request #148 from shookshire/master

fbd58de

Update remove manual testing

update PPP

c1a8960

Merge pull request #150 from shookshire/master

b8611e1

update PPP

DeveloperGuide: Updates Instructions for manual testing

c41329b

Merge branch 'master' of https://github.com/CS2103JAN2018-F11-B2/main …

46a71d6

…into Integration

DeveloperGuide: Added one user story

d13e508

Merge pull request #151 from olimhc/Integration

98ca2a1

DeveloperGuide: Update instruction for testing & add one user story

Fixes Ui.png not displaying properly

091e062

Merge pull request #152 from olimhc/Integration

71db9fa

Fixes Ui.png not displaying properly

DeveloperGuide: Minor formatting edit

00cbff2

Merge pull request #153 from olimhc/Integration

9e9b30d

DeveloperGuide: Minor formatting edit

final update to ppp

b71d4ae

Merge pull request #154 from shookshire/master

53d60d1

final update to ppp

ReadMe: Fixes Rendering Error

262619c

Merge branch 'master' of https://github.com/CS2103JAN2018-F11-B2/main …

8b986b9

…into Integration

ReadMe: Minor change

8c07530

Merge pull request #155 from olimhc/Integration

df6c232

Fixes rendering error


		\|`* *` \|student \|request to know the qualification of the tutor \|make my decision wisely

		\|`* *` \|tutor \|request to know the grades of the students \|make my decision wisely

		@@ -806,9 +806,59 @@ Priorities: High (must have) - `* * \`, Medium (nice to have) - ` \*`, Low (un

+                  }
+                  /**
+                   *TODO:

[v1.4][F11-B2] TuitionCor #64

Are you sure you want to change the base?

[v1.4][F11-B2] TuitionCor #64

Conversation

Zhu-Jiahui commented Mar 8, 2018 • edited Loading

nus-se-pr-bot commented Mar 8, 2018

nus-se-pr-bot commented Mar 9, 2018

karrui left a comment • 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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karrui commented Mar 11, 2018

karrui commented Mar 14, 2018

Zhu-Jiahui commented Mar 14, 2018 via email

karrui commented Mar 17, 2018

karrui left a 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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shookshire commented Mar 19, 2018

karrui commented Mar 21, 2018

Zhu-Jiahui commented Mar 8, 2018 •

edited

Loading

karrui left a comment •

edited

Loading