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

Multi-Word last name or first name #128

Open
javb99 opened this issue Mar 6, 2019 · 11 comments
Open

Multi-Word last name or first name #128

javb99 opened this issue Mar 6, 2019 · 11 comments
Labels
bug Something isn't working People This is a pco people feature issue.

Comments

@javb99
Copy link

javb99 commented Mar 6, 2019

A slack name with more than two words will not match to the user in Planning Center. This seems to be due to a split on spaces where element 0 is treated as the first name and element 1 is treated as the last name.

@pastorhudson
Copy link
Owner

Is this in reference to the app permissions lookup?

@javb99
Copy link
Author

javb99 commented Mar 7, 2019

Yes.

@pastorhudson
Copy link
Owner

Ok I need a bit more info. Can we assume that if there is a 3rd element then it's First, Middle, and Last name?

fl_name = {'first_name': message.sender['source']['real_name'].split()[0],
                       'middle_name': message.sender['source']['real_name'].split()[1],
                       'last_name': message.sender['source']['real_name'].split()[2]}

Or is there some sort of double last name edge case we're dealing with?

@javb99
Copy link
Author

javb99 commented Mar 8, 2019

It is a double last name case. For example: "Joseph Van Boxtel" should split to (First: "Joseph" Last: "Van Boxtel") The following seems to work.
fl_name = {'search_name_or_email': message.sender['source']['real_name']}
Do you know of a drawback to using this approach?

@pastorhudson
Copy link
Owner

That is perfect! I'll test it! Or a pull request would be great

@pastorhudson
Copy link
Owner

The only think I can think of is that right now we have check_name who's job is to verify they have at least a first and last name, and explain this needs to match Planning Center for the authentication to work. It's now obvious it doesn't quite work. What's your thoughts on this?

I think it's pretty messy how I have it currently implemented.

@javb99
Copy link
Author

javb99 commented Mar 8, 2019

It seems like this change would make check_name unneeded because a blank name on slack would probably not filter anyone out so the email check would still work (Edit: Can't have a blank slack name). That brings up the question should we check the name and the email in a replacement for check_name?

Also, it seems the get(message, app) checks the email, but get_apps(message) doesn't check the email so it would probably have issues if two people had the same name.

@pastorhudson
Copy link
Owner

Really good points.

I agree check name won't be needed anymore.
I also want to add the auth failure response to the authenticate.get so we can clean up the responces and get rid of duplicate code.

get_apps will need to be updated too.

@pastorhudson
Copy link
Owner

After testing fl_name = {'search_name_or_email': message.sender['source']['real_name']} I found it to be very very slow. It returns too many results that have to be parsed. Any other ideas?

@javb99
Copy link
Author

javb99 commented May 2, 2019

We could only do it conditionally if the full name has more than 2 components.

@pastorhudson
Copy link
Owner

It was so slow It feels like it Isn’t working at all. User will probably keep resubmitting the request 🤔

@pastorhudson pastorhudson added People This is a pco people feature issue. bug Something isn't working labels Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working People This is a pco people feature issue.
Projects
None yet
Development

No branches or pull requests

2 participants