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

Improve Name Extraction in Banking Matcher Plugin #112

Merged
merged 4 commits into from
Jun 27, 2024

Conversation

MarcMichalsky
Copy link
Contributor

This commit improves the name extraction for the db mode.

  • if there is more than one valid name bit, the last bit will always be the last name
  • name bits following the first bit recognised as a last name will not become first names again

Comparison of the results

/**
 * Dummy function
 */
private function isDBFirstName($name_bit) {
  $fistnames = ['John', 'Jane', 'Alice', 'Bob', 'Arthur'];
  return in_array($name_bit, $fistnames);
}

Original function

Name: John Doe, First names: John, Last names: Doe
Name: Jane Alice Doe, First names: Jane Alice, Last names: Doe
Name: Arthur Smith, First names: Arthur, Last names: Smith
Name: Alice Jane Arthur, First names: Alice Jane Arthur, Last names: 
Name: Bob King Arthur, First names: Bob Arthur, Last names: King

Modified function

Name: John Doe, First names: John, Last names: Doe
Name: Jane Alice Doe, First names: Jane Alice, Last names: Doe
Name: Arthur Smith, First names: Arthur, Last names: Smith
Name: Alice Jane Arthur, First names: Alice Jane, Last names: Arthur
Name: Bob King Arthur, First names: Bob, Last names: King Arthur

This commit improves the name extraction for the db mode.
- if there is more than one valid name bit, the last bit will always be the last name
- name bits following the first bit recognised as a last name will not become first names again
@jensschuppe
Copy link
Collaborator

I'm pretty sure @bjendres would like this to depend on a configuration option for the matcher plugin with a default fallback to the current behavior in case someone relies on it.

@MarcMichalsky
Copy link
Contributor Author

Well, we could of course just add another mode.

This commit improves the name extraction by adding `db2` as an alternative mode.
- if there is more than one valid name bit, the last bit will always be the last name
- name bits following the first bit recognised as a last name will not become first names again
@bjendres
Copy link
Member

I'm pretty sure @bjendres would like this to depend on a configuration option for the matcher plugin with a default fallback to the current behavior in case someone relies on it.

I don't think that's necessary, since @MarcMichalsky introduced a new name_mode called db2.

Copy link
Member

@bjendres bjendres left a comment

Choose a reason for hiding this comment

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

Looks neat and clean, but haven't tested - assuming that you did :)

If you have the time, it'd be great to add a unit test.

@jensschuppe
Copy link
Collaborator

I don't think that's necessary, since @MarcMichalsky introduced a new name_mode called db2.

Yep, after I raised that concern … well, actually still your concern I guess ;-)

@MarcMichalsky thanks, that was quick!

@MarcMichalsky
Copy link
Contributor Author

Yes, I did that because of @jensschuppe's comment.

Sometimes I lack the perspective of a service provider who serves many customers. I usually only look at problems like this from our perspective. 🙈

As soon as I have time, I'll be happy to write a test for this.

@bjendres
Copy link
Member

As soon as I have time, I'll be happy to write a test for this.

We'll wait for that, then. But if it becomes clear you won't be able to in the near future, give us a shout.

@MarcMichalsky
Copy link
Contributor Author

Added more logic to cover the common case of <lastname>, <firstname> pattern.

@jensschuppe jensschuppe added enhancement status:reviewed and tested Code has received thorough review and test and is ready to be committed/merged labels Jun 27, 2024
@jensschuppe jensschuppe added this to the 1.14 milestone Jun 27, 2024
@jensschuppe jensschuppe merged commit 08fb8f2 into systopia:master Jun 27, 2024
@jensschuppe
Copy link
Collaborator

Merged, thanks!

Let's add docs for those CiviBanking plugins, I created #114 as a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement status:reviewed and tested Code has received thorough review and test and is ready to be committed/merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants