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

ActivityCommand substring bugfix and contact command prevent duplicate name #123

Merged
merged 3 commits into from
Oct 29, 2019

Conversation

daekoon
Copy link

@daekoon daekoon commented Oct 28, 2019

Context

There was an edge case that happened when there were 2 contacts, with one contact's name being the substring of another.

Example:
Contact 1: George
Contact 2: George Yeoh

You will never be able to add Contact 1 in because when you search "George", it will always return 2 matches regardless.

Also, there is no validation to check if two person have the same name.

Changes

  • Added additional case for ActivityCommand, which now first checks if there is an exact match with the name. It will proceed to keyword based searching if there isn't.
  • Added validation for Contact command, to check for duplicate names
  • Added relevant tests
  • Fixed broken ActivityCommand tests
  • Update UserGuide and Developer Guide

Copy link

@liakify liakify left a comment

Choose a reason for hiding this comment

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

looks good to me, at some point we should centralise the name searching logic into a single place cause currently activity, expense, invite, disinvite supposed to use the same thing

after merging this then expense, invite, disinvite will encounter the same issue

@liakify liakify merged commit 0e02851 into master Oct 29, 2019
@liakify liakify deleted the activity-command-fix-name-substring-bug branch October 29, 2019 04:38
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

2 participants