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

Implement archive commands #124

Merged
merged 12 commits into from
Oct 20, 2020

Conversation

jmleong666
Copy link

@jmleong666 jmleong666 commented Oct 19, 2020

Closes #119.

New features/changes

  • Implement archiving contacts. Sends the specified contact by index in the contact list to the archive. Usage: archive add INDEX, INDEX must be a positive integer. Can only be used when the contact list is displayed in the GUI.
  • Implement listing contacts in the archive. Lists contacts in the archive, replacing the contact list in the GUI. Usage: archive list.
  • Implement removing contacts from the archive back to the contact list. Usage: archive remove INDEX, INDEX must be a positive integer. Can only be used when the archive list is displayed in the GUI.
  • Note: contact edit and contact delete work on the archived list, and behave the same way since they modify entries on the currently displayed list.
  • Change the implementation of contact add command to avoid id collisions.

Implementations

Archive features

  • Add a new boolean flag archived under the Person model. Introduce two new filters, PREDICATE_SHOW_ARCHIVED_PERSONS and PREDICATE_SHOW_UNARCHIVED_PERSONS to display the appropriate list when contact list or archive list command is entered. Filter contacts by the latter predicate on startup.
  • When the archive add command is entered, check whether the correct list is displayed during the execution of the command. If incorrect, throw a CommandException with the appropriate message. Otherwise replace the target Person with a copy of the Person with the edited archived flag. The archive remove implementation is similar to the archive command.

Contact add command change

  • When a new Person is created, its id is based on the size of the currently displayed list, and this may cause id collisions if there have been past deletions or the contact is added when the smaller list is displayed. Add a new method to Model to get all Persons in the StonksBook and assign the new Person's id based on the largest id value in the all Persons list during the execution of the AddCommand.

Testing

  • Add tests for the three new commands, which include tests for checking whether the displayed list is correct for the archive and unarchive commands.
  • Add some TypicalPersons which are archived by default to allow for archive command tests.
  • Update json files to reflect the addition of the archived attribute.

Possible improvements

  • Disallow users to edit contacts in the archive.
  • Introduce the archive delete command to reduce the responsibility of the contact delete command.

Demo (note: contact archive renamed to archive add)
stonksbook_archive_demo

@jmleong666 jmleong666 added this to the v1.3 milestone Oct 19, 2020
@jmleong666 jmleong666 self-assigned this Oct 19, 2020
@jmleong666 jmleong666 marked this pull request as ready for review October 19, 2020 12:48
@jmleong666 jmleong666 marked this pull request as draft October 19, 2020 13:31
@jmleong666 jmleong666 marked this pull request as ready for review October 20, 2020 07:00
@codecov-io
Copy link

codecov-io commented Oct 20, 2020

Codecov Report

Merging #124 into master will increase coverage by 0.46%.
The diff coverage is 91.07%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #124      +/-   ##
============================================
+ Coverage     75.15%   75.62%   +0.46%     
- Complexity     1060     1093      +33     
============================================
  Files           150      156       +6     
  Lines          3309     3405      +96     
  Branches        441      450       +9     
============================================
+ Hits           2487     2575      +88     
- Misses          673      677       +4     
- Partials        149      153       +4     
Impacted Files Coverage Δ Complexity Δ
...main/java/seedu/address/commons/core/Messages.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
src/main/java/seedu/address/ui/HelpWindow.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../seedu/address/logic/parser/AddressBookParser.java 78.00% <25.00%> (-2.86%) 21.00 <0.00> (ø)
...du/address/logic/commands/contact/EditCommand.java 90.47% <50.00%> (-2.12%) 12.00 <0.00> (ø)
...ss/logic/parser/archive/ArchiveCommandsParser.java 57.14% <57.14%> (ø) 4.00 <4.00> (?)
...edu/address/logic/commands/contact/AddCommand.java 75.75% <88.88%> (+1.68%) 10.00 <2.00> (+1.00)
...in/java/seedu/address/commons/enums/GroupEnum.java 100.00% <100.00%> (ø) 1.00 <0.00> (ø)
...edu/address/logic/commands/archive/AddCommand.java 100.00% <100.00%> (ø) 8.00 <8.00> (?)
...du/address/logic/commands/archive/ListCommand.java 100.00% <100.00%> (ø) 3.00 <3.00> (?)
.../address/logic/commands/archive/RemoveCommand.java 100.00% <100.00%> (ø) 8.00 <8.00> (?)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bebb74...09c1904. Read the comment docs.

@jmleong666 jmleong666 marked this pull request as draft October 20, 2020 13:15
@jmleong666 jmleong666 marked this pull request as ready for review October 20, 2020 13:34
Copy link

@AaronnSeah AaronnSeah left a comment

Choose a reason for hiding this comment

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

Just a small change needed, besides that LGTM. Also, just to let you know that you can add sales to archived contacts as well

@@ -8,6 +8,12 @@ contact find Find contacts given search keyword(s)
contact delete Deletes a contact contact delete INDEX
contact sort Sorts contact list contact sort KEYWORD [ORDER]

Archive commands
----------------------
archive add Sends a contact to the archive contact archive INDEX

Choose a reason for hiding this comment

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

the example usage is not updated?

Copy link
Author

Choose a reason for hiding this comment

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

Whoops my bad. Will update on another feature branch.

@AaronnSeah AaronnSeah merged commit 6b08b08 into AY2021S1-CS2103T-T11-1:master Oct 20, 2020
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.

As a well-connected salesman, I can archive inactive contacts
3 participants