Skip to content

[T09-1] PlanWithEase - #7

Open
lycjackie wants to merge 486 commits into
nusCS2113-AY1819S2:masterfrom
CS2113-AY1819S2-T09-1:master
Open

[T09-1] PlanWithEase#7
lycjackie wants to merge 486 commits into
nusCS2113-AY1819S2:masterfrom
CS2113-AY1819S2-T09-1:master

Conversation

@lycjackie

Copy link
Copy Markdown

fzdy1914 and others added 25 commits March 18, 2019 16:24
We are currently using the DTD files from `http://checkstyle.sourceforge.net/`
and `http://puppycrawl.com/` in our config file of checkstyle.

However, due to security reason, checkstyle decided to remove DTDs from
above websites and ask users to use the DTD files from
`https://checkstyle.org/`[1].

Let's update the addresses of DTD files correspondingly.

Meanwhile, update the version of suppression DTD file to 1.2 because it is
the version suggested from checkstyle. [2]

[1] checkstyle/checkstyle#6478
[2] https://checkstyle.org/config_filters.html#SuppressionFilter_Examples
When comparing if two modules are same using `Module#isSameModule(...)`,
the equality of `Name`, `Code` and `Credits` fields are compared.

However, this comparison may be too strict, and may result in duplicate
modules being created.

e.g. `Module#isSameModule(...)` will return false for the below modules:

  Code: AA1234
  Name: Intro to ABC
  Credits: 4

  Code: AA1234
  Name: Intro to ABC
  Credits: 12

However, they are logically referring to the same module.

We should instead make `Code` the identity field, and make `Name`,
`Credits` and `Tag` data fields of `Module` objects.

Firstly, `Credits` or `Tag` should not be considered as identity fields
because their values are not unique enough.

Secondly, `Name` should not be considered as an identity field because
some modules have identical names, but refer to different modules.

For instance, the below two modules are two separate entities:

  Code: GEK1531
  Name: Cyber Security
  Credits: 4

  Code: GET1004
  Name: Cyber Security
  Credits: 4

Let's update the `Module#isSameModule(...)` equality comparison to
reflect the changes in identity fields for `Module` objects, and fix the
affected unit tests as well.
The current version of TestFX dependency we are using is 4.0.12-alpha.

TestFX version 4.0.15-alpha has been announced for a while and it gives
better support for JDK11[1], which we may wish to upgrade to in the
future.

To keep us updated with TestFX's development, let's upgrade TestFX
dependency to its latest stable version 4.0.15-alpha.

[1] TestFX/TestFX#640 (comment)
ConfigTest contains an unused ExpectedException. It appears that the
ExpectedException was first introduced in [1].
However, since its introduction, there have been no tests within
ConfigTest that potentially throws an expected exception.

Hence, to make our code tidier and to remove unnecessary code, let's
remove the ExpectedException together with their imports.

[1] se-edu@75942c9
#86)

Upstream se-edu/addressbook-level4@e09826e updated the addresses of
Checkstyle DTD files for security reason.

Let's follow and merge the changes from se-edu#984 into master branch.
…iveInteger() to isNonZeroUnsignedInteger() (#87)

Upstream se-edu/addressbook-level4@148e301 refactored
StringUtilTest#isUnsignedPositiveInteger() to
isNonZeroUnsignedInteger() to maintain consistent naming convention.

Let's follow and merge the changes from se-edu#975 into master branch.
…nfigTest (#88)

Upstream se-edu/addressbook-level4@7c2e953 removed an unused
ExpectedException to make the codebase tidier and remove unnecessary
code.

Let's follow and merge the changes from se-edu#985 into master branch.
The new command will need some sample data in order to test if the
command was correctly implemented.

Let's update the current sample data to allow the user to test if the
new command is working as desired.
As a new command will be added, the 'RequirementCategoryBuilder' was
validated to see if there are any conflicting attributes or methods
that may cause a problem at a later stage.

Upon insepction, a few comments and methods have to be updated. Let's
update the 'RequirementCategoryBuilder' class so that it will not have
any conflicting issues when the new command is implemented.
Currently, there is no Ui to display the list of degree planners.

Let's add Ui to display them.
Currently, MainWindow does not include UI for DegreePlannerList.
Moreover, BrowserPanel is not needed to be displayed.

Let's update MainWindow to display Ui for DegreePlannerList and minimize
the size for BrowserPanel and place it at the bottom so that it does not
interrupt other pannels.

In addition, with UI for DegreePlannerList included, it will have problem
activating 'Help' button on the menu when MainWindow is resized to
current minimum size.

Hence, let's increase the current minimum size for MainWindow to prevent
this problem.
Currently, DegreePlannerList does not have a feature to list all
degree planners.

Let's add PlannerListAllCommand to list all degree planners
Let's update logic to include PlannerListAllCommand to allow application
to take in 'planner_list_all' command to list all degree planners.
Currently, there are no Java classes to handle UI for DegreePlannerList.

Let's add Java Classes to handle UI for DegreePlannerList:

- view/DegreePlannerListCard
- view/DegreePlannerListPanel
Currently, MainWindow class does not include DegreePlannerList.

Let's update MainWindow Class to include and handle DegreePlannerList.
Since the application does not need BrowserPanel,
Let's update BrowserPanel to be hidden for now.
Currently, test does not include PlannerListAllCommand.

Let's add Test for the command.
BrowserPannel is to be updated to be hidden as mentioned in the
previous commit: 'View: update BrowserPanel to be hidden'.

Let's update HelpCommandSystemTest to not attempt clicking on the
hidden browserPannel
Currently, there is no module added into the degree planner list.

Let's update SampleDegreePlannerUtil to include the modules when there
is no initial data for now for demostration purpose.

SampleDataUtil will be updated to contain added modules only after
model and storage for degree planner list are updated.
In order for the program to be able to handle a new command, a class
must first be created to handle the logic flow for the new command.

In this case, a command to add a module to an existing requirment
category is to be implemented.

Let's create a skeleton 'RequirmentAddCommand' class with uninitialzed
methods to show how the logic would flow for the new command. Let's also
add in instructions on how the command is to be used as well as error
messages that will be displayed if an error was to occur.
The methods created in the 'RequirmentAddCommand' class is to have a
skeleton for the logic flow of the new command. However the methods
created have not yet been referenced in the 'Model', 'ModelManager',
'AddressBook' and 'AddCommandTest'

Let's add the method created in the 'RequirementAddCommand' class so
that the application is able to reference to these methods.
The skeleton for the new command have been created where a module is
added to an existing requirment category in the 'RequirementAddCommand'
class. However, the logic to how the commands work have not been added
yet.

Let's add the logic for the commands in the
'UniqueRequirementCategoryList' class to allow the new command to work
properly.

* The 'location()' method will return the position of the desired
requirement category in the requirment category list

* The 'isModuleInRequirmentCategory()' method checks if the requirement
category already contains the module to be added

* The 'doesModuleExistInApplication()' method checks if a module exists in the
existing moduleList in the application

* The 'addModuleToRequirmentCategory()' method adds the module entered
to a requirement category
After the skeleton and logic for the new command is implemented, the
way the data is read from the user's input must be implemented as well
so that the data can be passed to the logic and then stored into the
data storage files.

Let's implement the parser needed in order to read the user's input data
and pass it to 'RequirmentAddCommand' class.

* 'RequirmentAddCommandParser' class was created to parse the input data
based on the 'name' and 'code' prefixes.

- The 'RequirmentAddCommandParser' parsers the name to be the requirment
  category to add the module to

- The 'RequirementAddCommandParser' parsers the code to be the modules
  that will be added to the requirment category

- The 'RequirementAddCommandParser' will then create a temporary
  list of modules with temporary data, except for the module codes
  which will be validated in the 'RequirementAddCommand' class

* A 'parseCodes' method was created in 'ParserUtil' to return back a
  list of module codes
After the logic and parsing of the user input have been implemented, the
application itself must be able to take in the command and call the
relevant classess implemented for the command to work.

Let's allow the application to be able to take in the command by
updating 'AddressBookParser'
The new command adds an existing module to a requirment category.

In order for the application to test for desired behaviours, new
testcases have been created to test the response of the application if
undesired inputs were entered.

New testcases added are:
 - a null input is given
 - a non-existent requirement category is given
 - a non-existent module is given
 - a valid input is given
chyeo and others added 30 commits April 14, 2019 20:49
Currently, there is no tests for `DegreePlanner`, `Semester`, `Year`
and `UniqueDegreePlannerList` classes.

Let's add the abovementioned classes to cover different test cases.
Currently, there are no tests for `RequirementCategory`and 
`UniqueRequirementCategoryList` classes.

The `requirement_add` command test cases also need to be updated to
reflect the current implementation of our application.

Let's add and update the test cases for the above-mentioned classes.

  [1/3] Test/requirement: adding test cases for 'RequirementCategory' classes
  [2/3] test: update 'requirement_add' command test cases
  [3/3] Requirement: refactor 'collectors' to 'StringUtil'
Updating the user guide for 'planner_add', 'planner_remove' and 'planner_suggest'
Let's update the User Guide `Requirements Tracker` section.
…227)

Let's update the user guide to match with the updates on the
features.

Note: After the discussion, there has been refactoring from:

* 'planner_list' to 'planner_show'
* 'planner_list_all' to 'planner_list'
Let’s update the future enhancement for the application to grow
Let’s update the `DeveloperGuide` to improve our explanation to describe
the implementation better for our reader.
Let's update the User Guide for v1.4 release.
The PPP is not an indicative of what I have done in the project.

Let's update the PPP to reflects my work.
To prepare for v1.4 release, let's bump `MainApp#VERSION` to v1.4.
Let's update our user guide to fix typo.
Let's update Ngo Wei Lin's Personal Project Portfolio to showcase his
contributions to the project.
Let's rename all `addressbook` instances to our application.
Let's update the diagrams to reflect the latest changes
Let's update the personal portfolio to reflect current changes.
Let's update the Developer Guide to include the current implementation
and design considerations for the Edit Module feature.
Let's fix some inconsistencies in the command usage and User Guide.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants