Merge Command supports optional timestamp argument. #136
Conversation
JakubTesarek
force-pushed
the
jt@add_optional_merge_timestamp
branch
from
69ea290
to
1de9e9a
Compare
README.md
Outdated
| @@ -175,7 +175,7 @@ $response = $matej->request() | |||
| ->addItemProperty(ItemProperty::create('item-id', ['valid_from' => time(), 'title' => 'Title'])) | |||
| ->addItemProperties([/* array of ItemProperty objects */]) | |||
| // Merge user | |||
| ->addUserMerge(UserMerge::mergeInto('target-user-id', 'source-user-id')) | |||
| ->addUserMerge(UserMerge::mergeInto('target-user-id', 'source-user-id'), time()) | |||
There was a problem hiding this comment.
I don't think this case with time() as the value should be in the example - this replicates the default behavior (time() is used if no other value is provided), so this exact implementation is not useful. Maybe put there some other value and mention the current timestamp is used if no other value is given?
There was a problem hiding this comment.
I've added whole section about merging. I don't go over details as they are described in matej documentation.
| @@ -12,12 +12,13 @@ public function shouldGenerateCorrectSignature(): void | |||
| { | |||
| $sourceUserId = 'source-user'; | |||
| $targetUserId = 'target-user'; | |||
| $timestamp = time(); | |||
There was a problem hiding this comment.
Maybe use some static value instead of time(), which is the same as the default one - to make sure the setters actually sets the value. This test would pass even it the timestamp value would not be given to the constructors.
README.md
Outdated
| @@ -175,7 +175,7 @@ $response = $matej->request() | |||
| ->addItemProperty(ItemProperty::create('item-id', ['valid_from' => time(), 'title' => 'Title'])) | |||
| ->addItemProperties([/* array of ItemProperty objects */]) | |||
| // Merge user | |||
| ->addUserMerge(UserMerge::mergeInto('target-user-id', 'source-user-id')) | |||
| ->addUserMerge(UserMerge::mergeInto('target-user-id', 'source-user-id'), time()) | |||
There was a problem hiding this comment.
| ->addUserMerge(UserMerge::mergeInto('target-user-id', 'source-user-id'), time()) | |
| ->addUserMerge(UserMerge::mergeInto('target-user-id', 'source-user-id', time())) |
JakubTesarek
force-pushed
the
jt@add_optional_merge_timestamp
branch
from
396b624
to
6d35eec
Compare
No description provided.