-
Notifications
You must be signed in to change notification settings - Fork 1
feat: adding AdSlot and Newsletter service #5
Changes from all commits
38beae3
68fa59e
c7e12b4
89b9310
230dae9
725a518
be2b0d7
2d16111
9330462
3680441
d6fe2d8
45d7f23
b8a80a4
00cdf09
359e924
5496771
ecb6c3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,3 +143,8 @@ class TestCase extends PHPUnit | |
| The LiveIntent client inherits from Laravel's Http Client. Therefore, all the methods available to that client, will also be available here. | ||
|
|
||
| For detailed documentation see [here](https://laravel.com/docs/8.x/http-client#testing). | ||
|
|
||
| ## Development | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great job splitting up docs! |
||
|
|
||
| For information about how to contribute to this project please see | ||
| [here](/docs/development.md). | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| ## Development | ||
|
|
||
| To contribute to this package first you need to clone the repo and then copy the `.env` file. | ||
|
|
||
| ### Clone Repo | ||
| ```php | ||
| git clone git@github.com:LiveIntent/sdk-php.git | ||
| ``` | ||
|
|
||
| After you clone the repo, then you need to copy the `.env.example` | ||
|
|
||
| ### Copy Env | ||
| ```php | ||
| cp .env.example .env | ||
| ``` | ||
|
|
||
| After you copy the `.env.example` file then you can install the dependencies. | ||
|
|
||
| ### Install Dependencies | ||
| ```php | ||
| composer install | ||
| ``` | ||
|
|
||
| If you get an error message to install PHP 8.0 then install via homebrew | ||
|
|
||
| https://stitcher.io/blog/php-8-upgrade-mac | ||
|
|
||
| Once the upgrade is successful, then you can try to install the dependencies. | ||
|
|
||
| ### Unit Tests | ||
|
|
||
| To run the unit tests | ||
|
|
||
| ```php | ||
| composer test | ||
| ``` | ||
|
|
||
| The SDK uses Heimdall/Bifrost to authenticate so you need to create a Client and use that Client's Secret and Identifier. | ||
|
|
||
| #### Create a Client in Bifrost | ||
|
|
||
| First you need to get a token from Bifrost using the `oauth/token` endpoint. | ||
|
|
||
| Grab the token and put it into the Authorize header in the Heimdall API. | ||
|
|
||
| You can use the Heimdall API to create your client. | ||
|
|
||
| ```php | ||
| { | ||
| "name": "testClient234", | ||
| "environment": "server", | ||
| "redirectUri": "https://qa-heimdall.liveintenteng.com/sign-in" | ||
| } | ||
| ``` | ||
|
|
||
| Take the response and copy the secret and identifier and put that into your `.env` | ||
|
|
||
| ```php | ||
| { | ||
| "secret": "<some secret>", | ||
| "id": <some id>, | ||
| "name": "<some name>", | ||
| "identifier": "<some identifier>", | ||
| "redirectUri": "https://qa-heimdall.liveintenteng.com/sign-in", | ||
| "created": "<some datetime>", | ||
| "updated": "<some datetime>" | ||
| } | ||
| ``` | ||
|
|
||
| #### Client Secret and Identifier | ||
|
|
||
| In the `.env` file | ||
|
|
||
| ```php | ||
| CLIENT_ID=<identifier from response> | ||
| CLIENT_SECRET=<secret from response> | ||
|
|
||
| LI_BASE_URL=http://localhost:3000 // Pointing to Heimdall | ||
| ``` | ||
|
|
||
|
|
||
| ### Linting | ||
| The installed linter will auto-format your code to comply with our agreed php coding standard. | ||
|
|
||
| To run the linter | ||
| ```php | ||
| composer lint | ||
| ``` | ||
|
|
||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| <?php | ||
|
|
||
| namespace LiveIntent; | ||
|
|
||
| class AdSlot extends Resource | ||
| { | ||
| // | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| <?php | ||
|
|
||
| namespace LiveIntent; | ||
|
|
||
| class Newsletter extends Resource | ||
| { | ||
| // | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?php | ||
|
|
||
| namespace LiveIntent\Services; | ||
|
|
||
| use LiveIntent\AdSlot; | ||
|
|
||
| /** | ||
| * @method \LiveIntent\AdSlot find($id) | ||
| * @method \LiveIntent\AdSlot create($attributes) | ||
| * @method \LiveIntent\AdSlot update($attributes) | ||
| */ | ||
| class AdSlotService extends AbstractResourceService | ||
| { | ||
| /** | ||
| * The base url for this entity. | ||
| */ | ||
| protected $baseUrl = '/ad-slot'; | ||
|
|
||
| /** | ||
| * The resource class for this entity. | ||
| */ | ||
| protected $objectClass = AdSlot::class; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -188,9 +188,18 @@ private function isSameRequest(array $a, Request $b) | |
| */ | ||
| private function getNormalizedRequestData(Request $request) | ||
| { | ||
| $data = $request->isJson() | ||
| ? json_decode(collect($request->data())->flip()->first(), true) | ||
| : $request->data(); | ||
| // Initialize data when not json | ||
| $data = $request->data(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: Do you think we'll ever need to do that again? If so, then maybe move this to a common/shared function?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't seen another place that we do this. I'm not sure if we'll ever need to do this again. |
||
|
|
||
| if ($request->isJson()) { | ||
| // Turn request data into an array | ||
| $temp = json_decode((string) collect($request->data()), true); | ||
|
|
||
| $keys = array_keys($temp); | ||
|
|
||
| // Grab the first array key and json_decode it | ||
| $data = json_decode(reset($keys), true); | ||
| } | ||
|
|
||
| $excludedKeys = ['version', 'client_id', 'client_secret']; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?php | ||
|
|
||
| namespace LiveIntent\Services; | ||
|
|
||
| use LiveIntent\Newsletter; | ||
|
|
||
| /** | ||
| * @method \LiveIntent\Newsletter find($id) | ||
| * @method \LiveIntent\Newsletter create($attributes) | ||
| * @method \LiveIntent\Newsletter update($attributes) | ||
| */ | ||
| class NewsletterService extends AbstractResourceService | ||
| { | ||
| /** | ||
| * The base url for this entity. | ||
| */ | ||
| protected $baseUrl = '/newsletter'; | ||
|
|
||
| /** | ||
| * The resource class for this entity. | ||
| */ | ||
| protected $objectClass = Newsletter::class; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,18 @@ | |
|
|
||
| abstract class Fixtures | ||
| { | ||
| /** @return string */ | ||
| public static function adSlotHash() | ||
| { | ||
| return '60220c073cc811ec941d0ab243175da9'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: Is any of this data depended on ids being stored in database or are these mocked? |
||
| } | ||
|
|
||
| /** @return int */ | ||
| public static function adSlotId() | ||
| { | ||
| return 834557; | ||
| } | ||
|
|
||
| /** @return string */ | ||
| public static function campaignHash() | ||
| { | ||
|
|
@@ -21,4 +33,22 @@ public static function lineItemHash() | |
| { | ||
| return '00009758365a11e7943622000a974651'; | ||
| } | ||
|
|
||
| /** @return string */ | ||
| public static function newsletterHash() | ||
| { | ||
| return '03fb20f13cc811ec941d0ab243175da9'; | ||
| } | ||
|
|
||
| /** @return int */ | ||
| public static function newsletterId() | ||
| { | ||
| return 36116; | ||
| } | ||
|
|
||
| /** @return string */ | ||
| public static function publisherHash() | ||
| { | ||
| return 'b3e515b13cc711ec941d0ab243175da9'; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| <?php | ||
|
|
||
| namespace Tests\Services; | ||
|
|
||
| use Tests\Fixtures; | ||
| use LiveIntent\AdSlot; | ||
| use LiveIntent\Exceptions\InvalidRequestException; | ||
|
|
||
| class AdSlotServiceTest extends ServiceTestCase | ||
| { | ||
| protected $serviceKey = 'adSlots'; | ||
|
|
||
| public function testIsFindable() | ||
| { | ||
| $adSlot = $this->service->find(Fixtures::adSlotId()); | ||
| $this->assertInstanceOf(AdSlot::class, $adSlot); | ||
|
|
||
| $adSlot = $this->service->find(Fixtures::adSlotHash()); | ||
| $this->assertInstanceOf(AdSlot::class, $adSlot); | ||
| } | ||
|
|
||
| public function testIsCreatableViaAttributesArray() | ||
| { | ||
| $adSlot = $this->service->create([ | ||
| 'name' => 'SDK Test', | ||
| 'newsletter' => Fixtures::newsletterHash(), | ||
| 'type' => 'image', | ||
| 'mediaType' => 'newsletter', | ||
| 'sizes' => [ | ||
| [ | ||
| 'width' => 500, | ||
| 'height' => 600, | ||
| 'floor' => 1.0, | ||
| 'deviceTypes' => [1,2,3], | ||
| ], | ||
| ], | ||
| 'adIndicatorId' => 1, | ||
| ]); | ||
|
|
||
| $this->assertNotNull($adSlot->id); | ||
| $this->assertInstanceOf(AdSlot::class, $adSlot); | ||
| } | ||
|
|
||
| public function testIsCreatableViaResourceInstance() | ||
| { | ||
| $adSlot = new AdSlot([ | ||
| 'name' => 'SDK Test', | ||
| 'newsletter' => Fixtures::newsletterHash(), | ||
| 'type' => 'image', | ||
| 'mediaType' => 'newsletter', | ||
| 'sizes' => [ | ||
| [ | ||
| 'width' => 500, | ||
| 'height' => 600, | ||
| 'floor' => 1.0, | ||
| 'deviceTypes' => [1,2,3], | ||
| ], | ||
| ], | ||
| 'adIndicatorId' => 1, | ||
| ]); | ||
|
|
||
| $adSlot = $this->service->create($adSlot); | ||
|
|
||
| $this->assertNotNull($adSlot->id); | ||
| $this->assertInstanceOf(AdSlot::class, $adSlot); | ||
| } | ||
|
|
||
| public function testIsUpdateableViaAttributesArray() | ||
|
rrmanalo623 marked this conversation as resolved.
|
||
| { | ||
| $adSlot = $this->service->find(Fixtures::adSlotId()); | ||
|
|
||
| $updatedName = 'SDK_TEST_UPDATE_NAME'; | ||
|
|
||
| $adSlot = $this->service->update([ | ||
| 'id' => $adSlot->id, | ||
| 'version' => $adSlot->version, | ||
| 'name' => $updatedName, | ||
| 'sizes' => [ | ||
| [ | ||
| 'width' => 400, | ||
| 'height' => 500, | ||
| 'floor' => 1.0, | ||
| 'deviceTypes' => [1,2,3], | ||
| ], | ||
| ], | ||
| ]); | ||
|
|
||
| $this->assertEquals($updatedName, $adSlot->name); | ||
| $this->assertInstanceOf(AdSlot::class, $adSlot); | ||
| } | ||
|
|
||
| public function testIsUpdateableViaResourceInstance() | ||
| { | ||
| $adSlot = $this->service->find(Fixtures::adSlotId()); | ||
| $updatedName = 'SDK_TEST_UPDATE_NAME'; | ||
|
|
||
| $adSlot->name = $updatedName; | ||
| $adSlot = $this->service->update($adSlot); | ||
|
|
||
| $this->assertEquals($updatedName, $adSlot->name); | ||
| $this->assertInstanceOf(AdSlot::class, $adSlot); | ||
| } | ||
|
|
||
| public function testThrowsWhenInvalidDataIsPassed() | ||
| { | ||
| $this->expectException(InvalidRequestException::class); | ||
|
|
||
| $this->service->create([ | ||
| 'name' => 'SDK Test', | ||
| 'newsletter' => Fixtures::newsletterHash(), | ||
| ]); | ||
| } | ||
| } | ||
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.
Question: this flag sounds risky =)
Why do we need it? And is this standard for company or just this one time off?
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.
That was added after running into this error https://github.com/LiveIntent/sdk-php/runs/5677792710?check_suite_focus=true
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.
I'm not sure if we can handle it differently or not, but that's why I added it.