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

Zijian/implement find for status #161

Merged
merged 18 commits into from
Oct 20, 2020
Merged

Zijian/implement find for status #161

merged 18 commits into from
Oct 20, 2020

Conversation

BobbyZhouZijian
Copy link

@BobbyZhouZijian BobbyZhouZijian commented Oct 15, 2020

Find now works for status.

Must search for the exact word of the status. E.g. "incomplete" will find all incomplete tasks, but "inc" will not match any.

Have not refactored the docs yet. Waiting for some kind guy to fix it. I'm too sleepy now and need to rest.

Fix #146
Fix #17

@MarcTzh
Copy link

MarcTzh commented Oct 16, 2020

Hi Zijian, I conducted some boundary value analysis and found that it works as intended for the positive test cases but when i put in find status: it does not throw any error, it says 0 tasks listed, following equivalence partitioning i shall stop further tests

@codecov-io
Copy link

codecov-io commented Oct 16, 2020

Codecov Report

Merging #161 into master will increase coverage by 0.84%.
The diff coverage is 92.15%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #161      +/-   ##
============================================
+ Coverage     73.39%   74.24%   +0.84%     
- Complexity      462      488      +26     
============================================
  Files            74       74              
  Lines          1421     1460      +39     
  Branches        159      168       +9     
============================================
+ Hits           1043     1084      +41     
+ Misses          332      330       -2     
  Partials         46       46              
Impacted Files Coverage Δ Complexity Δ
...main/java/seedu/address/commons/core/Messages.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...java/seedu/address/logic/commands/FindCommand.java 100.00% <ø> (ø) 6.00 <0.00> (ø)
...ain/java/seedu/address/model/task/Description.java 85.71% <ø> (ø) 8.00 <0.00> (ø)
src/main/java/seedu/address/model/task/Title.java 80.00% <ø> (ø) 6.00 <0.00> (ø)
...ress/model/task/TaskContainsKeywordsPredicate.java 87.23% <70.00%> (-5.08%) 22.00 <9.00> (+3.00) ⬇️
...c/main/java/seedu/address/model/task/DateTime.java 84.21% <80.00%> (-1.51%) 12.00 <4.00> (+4.00) ⬇️
...ain/java/seedu/address/logic/parser/CliSyntax.java 85.71% <100.00%> (+2.38%) 1.00 <0.00> (ø)
.../seedu/address/logic/parser/FindCommandParser.java 100.00% <100.00%> (+25.00%) 19.00 <15.00> (+14.00)
src/main/java/seedu/address/model/task/Status.java 84.21% <100.00%> (+9.21%) 9.00 <3.00> (+4.00)
src/main/java/seedu/address/model/task/Type.java 93.33% <100.00%> (ø) 11.00 <0.00> (ø)
... and 3 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 3c6dfa1...2669988. Read the comment docs.

@MarcTzh
Copy link

MarcTzh commented Oct 17, 2020

A few problems I found:
find status: XXX where XXX is an invalid input, returns 0 tasks found as well

Error message displayed should state that status can only be of 2 values: complete and incomplete.

find date:01-01-202 is also accepted and the date 01-01-2020 matches it, should this be considered an incorrect input

Copy link

@MarcTzh MarcTzh left a comment

Choose a reason for hiding this comment

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

Minor changes

docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/UserGuide.md Show resolved Hide resolved
docs/UserGuide.md Outdated Show resolved Hide resolved
docs/UserGuide.md Show resolved Hide resolved
src/main/java/seedu/address/model/task/Status.java Outdated Show resolved Hide resolved
Copy link

@MarcTzh MarcTzh left a comment

Choose a reason for hiding this comment

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

one minor change, LGTM

docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
@BobbyZhouZijian BobbyZhouZijian merged commit a82e660 into AY2021S1-CS2103T-T12-3:master Oct 20, 2020
for (Map.Entry<Prefix, List<String>> entry : keywords.entrySet()) {
Prefix prefix = entry.getKey();
List<String> words = entry.getValue();
if (isMatched(prefix.getPrefix(), words, task)) {
return true;
Copy link

Choose a reason for hiding this comment

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

I feel the original code is correct?

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.

Implement find by status As a user, I can find tasks by due date.
4 participants