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 mass commands for sale #150

Merged

Conversation

hakujitsu
Copy link

@hakujitsu hakujitsu commented Oct 24, 2020

Closes #150

Changes made

  • Refactor Sale class to contain person
    • Previously, the Sale model was refactored such that sale would no longer contain the person, only the id.
    • This poses issues when trying to print the buyer name.
    • Hence, the Sale model is refactored to include the Person. The JSONAdaptedSale is not affected, and will continue to store the person id.
    • When a person is edited, the list of sales is iterated through, and any relevant sales are updated with the new edited person.
  • Implement mass commands for sale add, sale delete and sale edit
    • sale add now has the ability to specify multiple buyers using c/1 c/2 c/3
    • sale delete now has the ability to delete multiple sales using s/1 s/2 s/3
    • sale edit now has the ability to edit multiple sales using s/1 s/2 s/3
  • Add test for EditCommandParser for sale edit

sale delete Demo
Screenshot 2020-10-24 at 10 06 29 PM
Screenshot 2020-10-24 at 10 07 11 PM

sale edit Demo
Screenshot 2020-10-24 at 10 09 00 PM
Screenshot 2020-10-24 at 10 09 06 PM

sale add Demo
Screenshot 2020-10-24 at 10 10 22 PM
Screenshot 2020-10-24 at 10 10 29 PM

@hakujitsu hakujitsu added this to the v1.3 milestone Oct 24, 2020
@hakujitsu hakujitsu self-assigned this Oct 24, 2020
@codecov-io
Copy link

codecov-io commented Oct 24, 2020

Codecov Report

Merging #150 into master will increase coverage by 1.16%.
The diff coverage is 91.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #150      +/-   ##
============================================
+ Coverage     73.67%   74.84%   +1.16%     
- Complexity     1250     1297      +47     
============================================
  Files           170      171       +1     
  Lines          4118     4226     +108     
  Branches        571      586      +15     
============================================
+ Hits           3034     3163     +129     
+ Misses          887      857      -30     
- Partials        197      206       +9     
Impacted Files Coverage Δ Complexity Δ
...main/java/seedu/address/commons/core/Messages.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
src/main/java/seedu/address/model/Model.java 100.00% <ø> (ø) 10.00 <0.00> (ø)
...u/address/logic/parser/sale/EditCommandParser.java 79.41% <66.66%> (+79.41%) 13.00 <0.00> (+13.00)
src/main/java/seedu/address/model/sale/Sale.java 91.37% <75.00%> (+0.30%) 26.00 <1.00> (ø)
...dress/logic/commands/sale/MassSaleCommandUtil.java 76.92% <76.92%> (ø) 5.00 <5.00> (?)
...address/logic/parser/sale/DeleteCommandParser.java 87.50% <80.00%> (-12.50%) 3.00 <2.00> (+1.00) ⬇️
.../seedu/address/logic/commands/sale/AddCommand.java 85.45% <91.17%> (+1.24%) 17.00 <5.00> (+7.00)
.../java/seedu/address/model/sale/UniqueSaleList.java 62.72% <91.66%> (+7.78%) 30.00 <8.00> (+7.00)
...seedu/address/logic/commands/sale/EditCommand.java 94.78% <93.33%> (+3.01%) 21.00 <3.00> (+7.00)
...edu/address/logic/commands/sale/DeleteCommand.java 92.50% <96.77%> (+4.03%) 15.00 <5.00> (+5.00)
... and 12 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 164aa8a...0b60798. Read the comment docs.

@hakujitsu hakujitsu marked this pull request as ready for review October 24, 2020 14:34
@hakujitsu hakujitsu marked this pull request as draft October 26, 2020 10:01
@hakujitsu hakujitsu marked this pull request as ready for review October 26, 2020 10:43
Copy link

@jmleong666 jmleong666 left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants