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

Fix country note list ordering #274

Conversation

qwoprocks
Copy link
Member

Description

As per title. Changed from comparing note contents, to actual added order.

Fixes #

Testing

Added and deleted some country notes.

Remarks

NIL.

@qwoprocks qwoprocks added type.Task Something that needs to be done, but not a story, bug, or an epic. e.g. Move testing code into a new type.Bug A bug severity.Medium A flaw that causes occasional inconvenience to some users but they can continue to use the product. labels Oct 29, 2020
@qwoprocks qwoprocks added this to the v1.4 milestone Oct 29, 2020
@qwoprocks qwoprocks self-assigned this Oct 29, 2020
@qwoprocks qwoprocks added this to In progress in v1.4 via automation Oct 29, 2020
@codecov
Copy link

codecov bot commented Oct 29, 2020

Codecov Report

Merging #274 into master will increase coverage by 0.16%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #274      +/-   ##
============================================
+ Coverage     86.62%   86.78%   +0.16%     
- Complexity      944      945       +1     
============================================
  Files           113      113              
  Lines          2459     2459              
  Branches        275      274       -1     
============================================
+ Hits           2130     2134       +4     
+ Misses          252      248       -4     
  Partials         77       77              
Impacted Files Coverage Δ Complexity Δ
...ain/java/seedu/address/model/note/CountryNote.java 100.00% <ø> (+17.39%) 12.00 <0.00> (ø)
...main/java/seedu/address/model/country/Country.java 90.90% <50.00%> (+0.43%) 12.00 <2.00> (+1.00)
...rc/main/java/seedu/address/model/ModelManager.java 94.69% <100.00%> (+0.14%) 43.00 <1.00> (ø)

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 3d25144...70b4292. Read the comment docs.

Copy link

@raysonkoh raysonkoh left a comment

Choose a reason for hiding this comment

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

Manually tested country note add and country note delete, seems to be working as expected. So just to check, a country note will first be sorted by country code, then insertion order right?

@qwoprocks
Copy link
Member Author

Manually tested country note add and country note delete, seems to be working as expected. So just to check, a country note will first be sorted by country code, then insertion order right?

@raysonkoh yup correct

v1.4 automation moved this from In progress to Reviewer approved Oct 29, 2020
Copy link

@raysonkoh raysonkoh left a comment

Choose a reason for hiding this comment

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

LGTM

@qwoprocks qwoprocks merged commit e39af10 into AY2021S1-CS2103T-F11-4:master Oct 29, 2020
v1.4 automation moved this from Reviewer approved to Done Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity.Medium A flaw that causes occasional inconvenience to some users but they can continue to use the product. type.Bug A bug type.Task Something that needs to be done, but not a story, bug, or an epic. e.g. Move testing code into a new
Projects
No open projects
v1.4
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants