-
Notifications
You must be signed in to change notification settings - Fork 4
get a data set of contact tags through a mailing list #6
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
base: master
Are you sure you want to change the base?
Conversation
src/Entity/Contact.php
Outdated
|
|
||
| public function addTag(string $key, string $value): self | ||
| { | ||
| $this->tags[$key] = $value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As written in the official documentation this should be a simple array of strings and not a key => value array.
Could you please change it and add a check if the tag already exists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't understand....why the value should be true or false if it's a string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be grateful to add a method to delete a tag, and related test cases 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thank you for your time reviewing my pull request.
As requested, I have added the method removeTag and also updated the ContactSerializerTest.php.
|
|
||
| public function getTags(string $callerInfo="createContact"): array | ||
| { | ||
| $newTags = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you return the tags property?
| $json = [ | ||
| 'email_address' => $object->getEmail(), | ||
| 'fields' => $object->getFields(), | ||
| 'tags' => $object->getTags(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2)[1]['function']), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's that?
| 'FirstName' => 'John', | ||
| 'LastName' => 'Doe', | ||
| ], | ||
| 'tags' => [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the json tags should be like that (key: string => value: boolean)?

No description provided.