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

refs #85 fix bug with unnamed contacts #87

Merged
merged 2 commits into from
Aug 29, 2019
Merged

Conversation

julien-nc
Copy link
Member

I guess this should do it.

Signed-off-by: Julien Veyssier <[email protected]>
@tacruc
Copy link
Collaborator

tacruc commented Aug 29, 2019

Maybe not ??? but an empty string?

@julien-nc
Copy link
Member Author

Well I don't want to face other problems like generating an avatar image for an empty string etc...

@tacruc
Copy link
Collaborator

tacruc commented Aug 29, 2019

Ok

@tacruc
Copy link
Collaborator

tacruc commented Aug 29, 2019

I found why there is no FN attribute, the cards just have the N attribute.

@julien-nc
Copy link
Member Author

Wait, shouldn't we read 'N' when 'FN' is not set? What does the 'N' field contain?

@julien-nc
Copy link
Member Author

What do you think about something like

'FN'=>$c['FN'] ?? $c['N'] ?? '???',

?

@tacruc
Copy link
Collaborator

tacruc commented Aug 29, 2019

private function N2FN(string N) {
  return explode(N,';')[3]+explode(N,';')[1]+explode(N,';')[0]
}

'FN'=>$c['FN'] ?? $this->N2FN($c['N']) ?? '???',

@julien-nc
Copy link
Member Author

I can't understand what you wrote unless you explain what's inside 'N' field.

@tacruc
Copy link
Collaborator

tacruc commented Aug 29, 2019

Doe;John;;Dr;
taken from https://en.m.wikipedia.org/wiki/VCard#Properties

@julien-nc
Copy link
Member Author

Ok and do you know when/why 'N' is set and 'FN' is not?

@tacruc
Copy link
Collaborator

tacruc commented Aug 29, 2019

Its an address book I created by importing an CSV file with kaddressbook. It might be an error in the import process.

@jancborchardt
Copy link
Member

Maybe @skjnldsv from @nextcloud/contacts can help with that Contacts-related question? :)

@julien-nc
Copy link
Member Author

Well then N2FN could be more secure by verifying the explode gives at least 4 elements. It could be called once and put in a variable too.

Do we agree on something like:

private function N2FN(string $n) {
  if ($n) {
    $spl = explode($n,';');
    if (count($spl) >= 4) {
      return $spl[3] + $spl[1] + $spl[0];
    }
    else {
      return null;
    }
  }
  else {
    return null;
  }
}

'FN'=>$c['FN'] ?? $this->N2FN($c['N']) ?? '???',

?

@tacruc
Copy link
Collaborator

tacruc commented Aug 29, 2019

Looks good, we probably need to add some spaces between the parts.

Signed-off-by: Julien Veyssier <[email protected]>
@julien-nc
Copy link
Member Author

Could you give it a little try?

@jancborchardt
Copy link
Member

Briefly tested and seems good, but I can’t judge the code.

@julien-nc julien-nc merged commit 9b8bb67 into master Aug 29, 2019
@julien-nc julien-nc deleted the fix-contact-noname branch August 29, 2019 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants