lmc-eu / matej-client-php Goto Github PK

• Lmc\Matej\Model\Command\Sorting
• Static named constructor with string $userId and array $itemIds params

Matej 9 supports custom interaction types defined in Matej administration. Add support for any type of interaction.

Interaction will have two named constructors - withItem() and withAliasedItem(), both will accept interaction type as a string in first argument.

• Interaction:withItem('interaction-type', 'user-id', 'item-id')
• Interaction:withAliasedItem('interaction-type', 'user-id', 'item-id-alias', 'item-id')

Both will also accept timestamp as last optional argument. Remove all other arguments that are no longer supported by Matej.

$response = $matej->request()
    ->events()
    ->addInteraction(\Lmc\Matej\Model\Command\Interaction:withItem('purchase', 'user123',  'item123'))
    ->addUserMerge(...)
    ...
    ->send();

Interaction attributes will be added using methods setAttribute(string $key, mixed $value) and setAttributes(array $attributes).

• Class Lmc\Matej\Model\Command\Constants\InteractionType will be removed. It will be replaced by string variable.
• Methods for creating specific interaction type from Lmc\Matej\Model\Command\Constants\InteractionType eg. detailView, purchase will be removed. They'll be replaced by calling constructor directly.
• There will be no required arguments for Matej. Validation of passed arguments will be done server-side. Client will only validate that argument interactionType and argument keys are sensible strings (strings without whitespaces). Value has to be serializable to int, double, string or bool. Argument value validations typeIdentifier(), between(), greaterThan() will be removed.

Current implementation of recommendation endpoint is checking user ids for (interaction, merge, recommendation)

     (A,  null,  A)
     (A, A -> ?, ?)
     (?, ? -> A, A)

After implementing RAD-1015, client should also allow:

   (B, A -> B, B)

I am looking into matej api documentation, and there is also method for reseting only data,
https://matej-docs.rad.lmc.cz/api/endpoints/data.html#reset-data

Can this method be also added to this library?

Thanks

When implementing sorting and recommendations builder, we should add validation that interaction, merge and recommendation/sorting command affects the same user.

This corresponding server-side API behavior is defined in https://jira.int.lmc.cz/browse/RAD-690 .

• Lmc\Matej\Model\Command\Interaction
• Static named constructors for each interaction type (as in ItemPropertySetup command)
• We may abstract the public API from "e-shop" naming of the interactions and provide some domain-independent names of the static constructors.
  • Eg. Interaction::view(...) will be Matej's detailviews interaction
  • Similar for other types - for purchases we may have ::conversion (or something like that), also for bookmarks and ratings (I'm not sure about the abstract names though)
• value will be optional and 1.0 by default
• context will be also optional and default by default
• not sure if timestamp should be unix-timestamp or we should accept DateTimeInterface instead

Once the command is done, in separate commit (or in separate pull request) the EventsRequestBuilder should be extended by addInteraction and addInteractions methods.

E.g.

$matej = new Matej('client-id', 'api-key');
$matej->request()->setupItemProperties() // returns mixed

so IDE is not able to autocomplete.

• Lmc\Matej\Model\Command\UserBasedRecommendations (or maybe something shorter? Wouldn't UserRecommendations be enough?)

• We need to specify which params will be required and which optional (and what is the default value).

Also common scenario is to get batch of recommendations with same params except userId. We may think about a convenient way how to accomplish this (maybe add another named constructor accepting array of userIds and returning array of commands?). But this is just nice to have, it does not have to be part of the first version.

Matej 9 supports boosting rules in recommendation query.

Boosting rules will can be added similarly to sorting rules by passing instance of Lmc\Matej\Model\Command\Boost

$recommendation = UserRecommendation::create('user-id', 'test-scenario');
$recommendation->addBoost(Boost::create('valid_to >= NOW()', 2));

or in batch

$recommendation->addBoost([Boost::create('for_reccomendation >= NOW()', 3.5)]);

It will also support re-setting all boostings

$recommendation->setBoosts([Boost::create('for_reccomendation >= NOW()', 3.5)]);

setBoostings will also accept empty array. In that case it will effectively remove all boostings:

$recommendation->setBoosts([]);

Boost will accept exactly two arguments: query (str) and multiplier (double > 0).

dependecy: #108

There's an inconsistency between adding adding filters (which accepts only strings) and adding sorting (which accepts instances of Sorting) even though they are otherwise very similar. I decided to use api consistent with sorting based on the fact that BoostingRules require more than one argument.

Usage will be like this:

$response = $matej->request()
    ->recommendation(UserRecommendation::create(...))
    ->setInteraction(Interaction::purchase(...)) // optional
    ->setUserMerge(UserMerge::mergeInto('target', 'source')) // optional
    ->send();

• Lmc\Matej\Model\Command\UserMerge
• Static named constructor with helpful name (so you are sure which user is source and which target). Something like UserMerge::toTargetUserMergeSourceUser($targetUser, $sourceUser) :-).

Once the command is done, in separate commit (or in separate pull request) the EventsRequestBuilder should be extended by addUserMerge and addUserMerges methods.

Usage will be like this:

$response = $matej->request()
    ->campaign()
    ->addRecommendation(UserRecommendation::create(...))
    ->addRecommendation(UserRecommendation::create(...))
    ->addRecommendations([UserRecommendation::create(...), UserRecommendation::create(...)])
    ->addSorting(Sorting::create(...))
    ->addSorting(Sorting::create(...))
    ->addSortings([Sorting::create(...), Sorting::create(...)])
    ->send();

Assert at least one recommendation or sorting must be given before calling build().

We may use user-agent or some other header to send library version with the request, to be eg. able to do some deprecation warnings in the future.

Once DELETE /database endpoint is released, it should be supported by this API client as well.

Don't forget to document the behavior which limits this endpoint for test instances.

We may also add the requirement for -test instances right in to the call? Or in the BuilderFactory? Or somewhere... So that you cannot even try this call against non-test database :).

Some classes use new self() while others use new static(). Perhaps it would be a good idea to use static everywhere so factory methods work as expected should inheritance get involved.

The same goes for self:: versus static::. Why not use static:: so static members can be overridden?

Also consider using @return static instead of @return self so code-completion works in these cases. For example - phpStorm complains about line 14 even though the call would succeed:

Matej9 makes all scenario arguments except userId and scenario optional.

Remove arguments count, rotationRate, rotationTime, responseProperties from UserRecommendation::create(). It will accept exactly 2 arguments: userId and scenario. All other arguments can be provided using current fluent api:

$response = $matej->request()
    ->recommendation(UserRecommendation::create('user-id', 'test-scenario')
        ->setCount(5)
        ->setRotationRate(0.3)
    )
    ->send();

This argument handling is already used for arguments allowSeen and modelName.
Some arguments setters already return instance of UserRecommendation that's being modified. Return it also from methods that currently don't: setCount(), setScenario(), setRotationRate(), setRotationTime().

UserRecommendation::UserRecommendation() will not return arguments that were not set by the user and such arguments will not be send to Matej.

I considered using api similar to #107 but in this case it's not optimal. The fluent api is already used in this case and the arguments variable but known.

We should replace Matej::VERSION constant, which needs to be updated each release, with https://github.com/Ocramius/PackageVersions , which allows to read the version based on actual package version installed via Composer.

Current way we update the constant manually each release also causes BC break false positive each time the constant value is changed (as it is considered BC break by https://github.com/Roave/BackwardCompatibilityCheck )

However, the PackageVersions requires PHP 7 ¯\_(ツ)_/¯, so we are blocked until we need to support PHP-5 version of Matej client.

Consider implementing eg. SplObjectStorage into Lmc\Matej\Model\Response to allow simplier acess to commandResponses.

However set/unset methods must be not implement (ie. throw an exception), as the object is not allowed to be modified after creation.

Usage will be like this:

$response = $matej->request()
    ->sorting(Sorting::create(...))
    ->setInteraction(Interaction::purchase(...)) // optional
    ->setUserMerge(UserMerge::mergeInto('target', 'source')) // optional
    ->send();

As a developer, I'd like to have new RecommendationResponse format as default behaviour.

When sending an UserRecommendationRequest, we're omitting parameters argument (due to it's default being null.) This is correct at this time.

For the future, we'd like to change this default to ['item_id'] so that Matej returns a list of objects in recommendation responses - and deprecate returning list of item ids on Matejs side.

We should validate data passed to commands in corresponding setters (or constructors), so invalid data are not even send to Matej API (which would reject them).

Eg. user_id (and other ids) must conform (according to JSON schema) "type_identifier": { "type": "string", "pattern": "^[0-9A-Za-z-_]+$", "maxLength": 100 } etc.

• Use assertion library (https://github.com/beberlei/assert or https://github.com/webmozart/assert)
• Throw custom exception
• Add information about the required value also to PHPDoc.

Simple method to call /healthcheck endpoint of the API to return its status.

May be used in some overal HTTP system checks.

Integration tests on Travis (#40) are currently being skipped.

We should enable them (ie. set api key and account id via environment variables) once test instance of Matej is available (see RAD-697).

Each request should contain Matej-Request-Id header with unique value. (maybe UUIDv4 via https://github.com/ramsey/uuid)

There should also be some documented way how to dump this ID for debugging purposes.

Also response always contains Matej-Response-Id header, so it should be somehow accessible as well.