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

Update moving of prescriptions to completed list #129

Merged
merged 5 commits into from
Oct 26, 2023

Conversation

jingyu987
Copy link

Previously the check to move prescriptions was only done if it is a listCompleted command, changed it such that it checks after every command so if a user adds a prescription that has an end date that is already past, it goes into the completed list right away.

@jingyu987 jingyu987 added type.Task Something that needs to be done, but not under other labels priority.Medium Nice to have labels Oct 25, 2023
@jingyu987
Copy link
Author

halp i dont understand the tests that are failing idk why fail :(

@jingyu987 jingyu987 added the help wanted Extra attention is needed label Oct 25, 2023
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (d94a12b) 70.18% compared to head (ee71d2e) 70.17%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #129      +/-   ##
============================================
- Coverage     70.18%   70.17%   -0.01%     
  Complexity      546      546              
============================================
  Files            94       94              
  Lines          1972     1968       -4     
  Branches        230      229       -1     
============================================
- Hits           1384     1381       -3     
+ Misses          504      502       -2     
- Partials         84       85       +1     
Files Coverage Δ
...rc/main/java/seedu/address/logic/LogicManager.java 82.85% <85.71%> (-4.33%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@shyanyong shyanyong left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@ChongWeiJie29 ChongWeiJie29 left a comment

Choose a reason for hiding this comment

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

LGTM

@RoeReRe RoeReRe merged commit ff06be6 into AY2324S1-CS2103T-T15-2:master Oct 26, 2023
4 of 5 checks passed
@RoeReRe
Copy link

RoeReRe commented Oct 26, 2023

ps didnt know need me do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed priority.Medium Nice to have type.Task Something that needs to be done, but not under other labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants