Skip to content

Conversation

@scabudlan
Copy link

No description provided.


public function addTag(string $key, string $value): self
{
$this->tags[$key] = $value;
Copy link
Owner

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?

Copy link
Author

Choose a reason for hiding this comment

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

Hi, good day.
The key/value pair will work if the value is only true/false.

The updates is working, and this is the result:
EmailOctopus-Result

Copy link
Owner

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?

Copy link
Owner

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 😄

Copy link
Author

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.

@scabudlan scabudlan requested a review from Ideneal November 4, 2023 15:20

public function getTags(string $callerInfo="createContact"): array
{
$newTags = [];
Copy link
Owner

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']),
Copy link
Owner

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' => [
Copy link
Owner

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)?

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.

2 participants