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.

Sign up for GitHub

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

Jump to bottom

Fix mark and unmark commands #216

Merged
merged 18 commits into from
Nov 5, 2021
Merged

Fix mark and unmark commands #216

merged 18 commits into from
Nov 5, 2021

Conversation

Gordon25
Copy link
Collaborator

@Gordon25 Gordon25 commented Nov 2, 2021 •
edited

  • fix bug in mark/unmark parser (unable to parser index > 9)
  • fix order at which contacts / events are marked
  • fix bugs in cunmark, enumark command test
  • make mark/unmark compatible with undo/redo
  • make esort compatible with mark events
  • fix bug with multiple marking of a single contact/ event
  • fix bug in mark and unmark parser that allows empty argument

@Gordon25 Gordon25 added type.Bug Something isn't working priority.High Should be resolved first, failing which, the project will not work type.Test More test cases is needed severity.High A flaw that affects most users and causes major problems for users labels Nov 2, 2021
@Gordon25 Gordon25 added this to the v1.4 milestone Nov 2, 2021
@Gordon25 Gordon25 added this to In progress in tP Project Dashboard via automation Nov 2, 2021
This was linked to issues Nov 2, 2021
[PE-D] Bookmarking Contact Pinning Issue #189
Closed
[PE-D] cmark works even if the contact is bookmarked already #197
Closed
[PE-D] Marking and Unmarking Bug When Inputting Non-Single Digit Indexes #204
Closed
[PE-D] Bookmark - fails for index greater than 2 digits #176
Closed
Update mark and unmark #207
Closed
@Gordon25 Gordon25 self-assigned this Nov 3, 2021
@Gordon25
Fix bug in cunmark and enumark command test classes
54d9785 
Fix ordering operation of unmark and mark commands
@codecov
Copy link

codecov bot commented Nov 4, 2021 •
edited

Codecov Report

Merging #216 (49d922b) into master (976076d) will increase coverage by 1.41%.
The diff coverage is 96.80%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #216      +/-   ##
============================================
+ Coverage     69.41%   70.83%   +1.41%     
- Complexity     1029     1064      +35     
============================================
  Files           129      129              
  Lines          3404     3442      +38     
  Branches        450      448       -2     
============================================
+ Hits           2363     2438      +75     
+ Misses          849      816      -33     
+ Partials        192      188       -4
Impacted Files Coverage Δ
src/main/java/seedu/address/model/Model.java 100.00% <ø> (ø)
src/main/java/seedu/address/ui/ContactCard.java 0.00% <0.00%> (ø)
...n/java/seedu/address/ui/ContactCommandSummary.java 0.00% <ø> (ø)
src/main/java/seedu/address/ui/EventCard.java 0.00% <0.00%> (ø)
...ain/java/seedu/address/ui/EventCommandSummary.java 0.00% <ø> (ø)
...seedu/address/model/contact/UniqueContactList.java 93.84% <93.33%> (-1.24%) ⬇️
...ava/seedu/address/model/event/UniqueEventList.java 86.30% <93.75%> (+6.57%) ⬆️
src/main/java/seedu/address/model/event/Event.java 81.48% <94.11%> (+1.88%) ⬆️
...main/java/seedu/address/model/contact/Contact.java 89.90% <94.44%> (+1.01%) ⬆️
...u/address/logic/commands/contact/CEditCommand.java 94.87% <100.00%> (ø)
... and 19 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 976076d...49d922b. Read the comment docs.

@Gordon25
Merge branch 'master' into add-bookmark-command
0f55eb1 
# Conflicts:
#	src/main/java/seedu/address/model/Model.java
#	src/main/java/seedu/address/model/contact/Contact.java
#	src/main/java/seedu/address/model/event/Event.java
#	src/main/java/seedu/address/ui/ContactCard.java
#	src/test/java/seedu/address/logic/commands/contact/CAddCommandTest.java
#	src/test/java/seedu/address/logic/commands/contact/CMarkCommandTest.java
#	src/test/java/seedu/address/logic/commands/contact/CUnmarkCommandTest.java
#	src/test/java/seedu/address/logic/commands/event/EAddCommandTest.java
#	src/test/java/seedu/address/testutil/ContactBuilder.java
@Gordon25 Gordon25 marked this pull request as draft November 4, 2021 08:59
This was linked to issues Nov 4, 2021
[PE-D] Feature/Functionality: undo a bookmark bug #178
Closed
[PE-D] Inputting very large numbers for commands results in unexpected results. #203
Closed
Gordon25 and others added 6 commits November 5, 2021 00:36
@Gordon25
Make esort compatible with emark
585f57b 
Add test cases for markparser
@Gordon25
fix checkstyle issues
b7e839f
@Gordon25
Merge branch 'master' into add-bookmark-command
53b8e4e 
# Conflicts:
#	src/main/java/seedu/address/logic/commands/contact/CMarkCommand.java
#	src/main/java/seedu/address/logic/commands/event/EMarkCommand.java
#	src/main/java/seedu/address/model/ModelManager.java
#	src/test/java/seedu/address/logic/commands/ModelStub.java
@Gordon25
Fix CI errors
2af0c72
@xiangjunn
Fix mark and unmark having to undo twice to revert change
634547a
@Gordon25
Merge branch 'master' into add-bookmark-command
472595b 
# Conflicts:
#	src/main/java/seedu/address/model/ModelManager.java
#	src/main/java/seedu/address/model/event/UniqueEventList.java
#	src/test/java/seedu/address/logic/commands/ModelStub.java
@Gordon25 Gordon25 marked this pull request as ready for review November 5, 2021 05:15
@Gordon25 Gordon25 linked an issue Nov 5, 2021 that may be closed by this pull request
Update esort to handle bookmarked events #206
Closed
2 tasks
@Gordon25 Gordon25 removed a link to an issue Nov 5, 2021
Update esort to handle bookmarked events #206
Closed
2 tasks
@Gordon25 Gordon25 linked an issue Nov 5, 2021 that may be closed by this pull request
Update esort to handle bookmarked events #206
Closed
2 tasks
chunweii
chunweii requested changes Nov 5, 2021
View reviewed changes
Copy link
Collaborator

@chunweii chunweii left a comment

Choose a reason for hiding this comment

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

LGTM, except for a few issues:

  1. Remove the UG changes, since we are going to change it in the ug branch.
  2. There is a bug which will still mark an event/contact if there is an invalid index. cmark 100 1 will mark 1.
  3. If I enter emark or eunmark or cmark or cunmark by itself, nothing happens.
  4. See other comments

src/main/java/seedu/address/logic/commands/contact/CUnmarkCommand.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/commands/event/EMarkCommand.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/commands/event/EUnmarkCommand.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/event/EMarkCommandParser.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/contact/CMarkCommandParser.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/model/contact/UniqueContactList.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/model/contact/UniqueContactList.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/model/event/Event.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/model/event/Event.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/model/event/UniqueEventList.java Outdated Show resolved Hide resolved
tP Project Dashboard automation moved this from In progress to Review in progress Nov 5, 2021
@chunweii chunweii self-requested a review November 5, 2021 07:30
chunweii
chunweii requested changes Nov 5, 2021
View reviewed changes
Copy link
Collaborator

@chunweii chunweii left a comment

Choose a reason for hiding this comment

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

Some typo errors. There is another bug (See previous review point 3):

  1. If I enter emark or eunmark or cmark or cunmark by itself, nothing happens.

src/main/java/seedu/address/logic/commands/contact/CMarkCommand.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/commands/event/EMarkCommand.java Outdated Show resolved Hide resolved
@Gordon25 Gordon25 changed the title Fix mark command and add tests Fix mark and unmark commands Nov 5, 2021
chunweii
chunweii approved these changes Nov 5, 2021
View reviewed changes
Copy link
Collaborator

@chunweii chunweii left a comment

Choose a reason for hiding this comment

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

LGTM

tP Project Dashboard automation moved this from Review in progress to Reviewer approved Nov 5, 2021
@chunweii chunweii merged commit 188d2f6 into master Nov 5, 2021
tP Project Dashboard automation moved this from Reviewer approved to Done Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chunweii chunweii chunweii approved these changes

Assignees

@Gordon25 Gordon25

Labels
priority.High Should be resolved first, failing which, the project will not work severity.High A flaw that affects most users and causes major problems for users type.Bug Something isn't working type.Test More test cases is needed
Projects
tP Project Dashboard
Done
Milestone
v1.4
3 participants
@Gordon25 @chunweii @xiangjunn