Skip to content

Commit 083ff69

Browse files
author
Luca Bruno
committed
Fix AdsPixel sharing implementation
Summary: AdsPixel sharing implementation broke HEAD + doesn't follow best practices + doesn't follow naming convention + missing PHPDoc + tests don't follow skippable interface Test Plan: ./vendor/bin/phpunit -vc test/ test/FacebookAdsTest/Object/AdsPixelTest.php
1 parent 6859cfe commit 083ff69

File tree

4 files changed

+239
-175
lines changed

4 files changed

+239
-175
lines changed

‎src/FacebookAds/Object/AdsPixel.php‎

Lines changed: 71 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
namespace FacebookAds\Object;
2626

27+
use FacebookAds\Cursor;
28+
use FacebookAds\Http\RequestInterface;
2729
use FacebookAds\Object\Fields\AdsPixelsFields;
2830
use FacebookAds\Object\Traits\CannotDelete;
2931
use FacebookAds\Object\Traits\FieldValidation;
@@ -33,64 +35,96 @@ class AdsPixel extends AbstractCrudObject {
3335
use FieldValidation;
3436

3537
/**
36-
* @var string[]
37-
**/
38-
protected static $fields = array(
39-
AdsPixelsFields::CODE,
40-
AdsPixelsFields::CREATION_TIME,
41-
AdsPixelsFields::ID,
42-
AdsPixelsFields::LAST_FIRED_TIME,
43-
AdsPixelsFields::NAME,
44-
AdsPixelsFields::RULE_VALIDATION,
45-
AdsPixelsFields::RULES,
46-
);
38+
* @return AdsPixelsFields
39+
*/
40+
public static function getFieldsEnum() {
41+
return AdsPixelsFields::getInstance();
42+
}
43+
44+
/**
45+
* @return string
46+
*/
47+
protected function getEndpoint() {
48+
return 'adspixels';
49+
}
4750

48-
public function sharePixel($business_id, $account_id) {
49-
return $this->getApi()->call(
51+
/**
52+
* @param int $business_id
53+
* @param string $account_id
54+
*/
55+
public function sharePixelWithAdAccount($business_id, $account_id) {
56+
$this->getApi()->call(
5057
'/'.$this->assureId().'/shared_accounts',
5158
RequestInterface::METHOD_POST,
5259
array(
5360
'business' => $business_id,
54-
'account_id' => $account_id))->getContent();
61+
'account_id' => $account_id,
62+
));
5563
}
5664

57-
public function sharePixelAgencies($business_id, $agency_id) {
58-
return $this->getApi()->call(
59-
'/'.$this->assureId().'/shared_agencies',
60-
RequestInterface::METHOD_POST,
65+
/**
66+
* @param $business_id
67+
* @param $account_id
68+
*/
69+
public function unsharePixelWithAdAccount($business_id, $account_id) {
70+
$this->getApi()->call(
71+
'/'.$this->assureId().'/shared_accounts',
72+
RequestInterface::METHOD_DELETE,
6173
array(
6274
'business' => $business_id,
63-
'agency_id' => $agency_id))->getContent();
75+
'account_id' => $account_id,
76+
));
6477
}
6578

66-
public function listAdAccounts($business_id) {
67-
$response = $this->getApi()->call(
68-
'/'.$this->assureId().'/shared_accounts',
69-
RequestInterface::METHOD_GET,
70-
array('business' => $business_id))->getContent();
71-
72-
return new Cursor($response, new AdAccount());
79+
/**
80+
* @param int $business_id
81+
* @param int $agency_id
82+
*/
83+
public function sharePixelWithAgency($business_id, $agency_id) {
84+
$this->getApi()->call(
85+
'/'.$this->assureId().'/shared_agencies',
86+
RequestInterface::METHOD_POST,
87+
array(
88+
'business' => $business_id,
89+
'agency_id' => $agency_id,
90+
));
7391
}
7492

75-
public function listSharedAgencies() {
76-
$response = $this->getApi()->call(
93+
/**
94+
* @param int $business_id
95+
* @param int $agency_id
96+
*/
97+
public function unsharePixelWithAgency($business_id, $agency_id) {
98+
$this->getApi()->call(
7799
'/'.$this->assureId().'/shared_agencies',
78-
RequestInterface::METHOD_GET)->getContent();
79-
80-
return new Cursor($response, new Business());
100+
RequestInterface::METHOD_DELETE,
101+
array(
102+
'business' => $business_id,
103+
'agency_id' => $agency_id,
104+
));
81105
}
82106

83107
/**
84-
* @return string
108+
* @param array $fields
109+
* @param array $params
110+
* @return Cursor
85111
*/
86-
protected function getEndpoint() {
87-
return 'adspixels';
112+
public function getAdAccounts(
113+
array $fields = array(), array $params = array()) {
114+
115+
return $this->getManyByConnection(
116+
AdAccount::className(), $fields, $params, 'shared_accounts');
88117
}
89118

90119
/**
91-
* @return AdsPixelsFields
120+
* @param array $fields
121+
* @param array $params
122+
* @return Cursor
92123
*/
93-
public static function getFieldsEnum() {
94-
return AdsPixelsFields::getInstance();
124+
public function getAgencies(
125+
array $fields = array(), array $params = array()) {
126+
127+
return $this->getManyByConnection(
128+
Business::className(), $fields, $params, 'shared_agencies');
95129
}
96130
}

‎test/FacebookAdsTest/AbstractTestCase.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ public function shouldSkipTest($key) {
4949
return $this->getSkippableFeaturesManager()->isSkipKey($key);
5050
}
5151

52+
public function skipIf($key) {
53+
if ($this->shouldSkipTest($key)) {
54+
$this->markTestSkipped();
55+
}
56+
}
57+
5258
/**
5359
* @return Config
5460
*/

‎test/FacebookAdsTest/Object/AdsPixelTest.php‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
<?php
2+
/**
3+
* Copyright 2014 Facebook, Inc.
4+
*
5+
* You are hereby granted a non-exclusive, worldwide, royalty-free license to
6+
* use, copy, modify, and distribute this software in source code or binary
7+
* form for use in connection with the web services and APIs provided by
8+
* Facebook.
9+
*
10+
* As with any software that integrates with the Facebook platform, your use
11+
* of this software is subject to the Facebook Developer Principles and
12+
* Policies [http://developers.facebook.com/policy/]. This copyright notice
13+
* shall be included in all copies or substantial portions of the software.
14+
*
15+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
18+
* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
20+
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
21+
* DEALINGS IN THE SOFTWARE.
22+
*
23+
*/
24+
25+
namespace FacebookAdsTest\Object;
26+
27+
use FacebookAds\Object\AbstractCrudObject;
28+
use FacebookAds\Object\AdAccount;
29+
use FacebookAds\Object\AdsPixel;
30+
use FacebookAds\Object\Business;
31+
use FacebookAds\Object\Fields\AdsPixelsFields;
32+
33+
class AdsPixelTest extends AbstractCrudObjectTestCase {
34+
35+
/**
36+
* AdsPixels can be created but only one per account can exist
37+
*/
38+
public function testCreate() {
39+
// make sure there's at least one pixel
40+
$account = new AdAccount($this->getConfig()->accountId);
41+
$pixels = $account->getAdsPixels();
42+
$pixel = null;
43+
44+
if (!$pixels->count()) {
45+
$pixel = new AdsPixel(null, $this->getConfig()->accountId);
46+
$pixel->{AdsPixelsFields::NAME} = 'WCA Pixel';
47+
$this->assertCanCreate($pixel);
48+
} else {
49+
$pixel = $pixels->current();
50+
}
51+
52+
$pixel = new AdsPixel(null, $this->getConfig()->accountId);
53+
$pixel->{AdsPixelsFields::NAME} = $this->getConfig()->testRunId;
54+
$this->assertCannotCreate($pixel);
55+
}
56+
57+
/**
58+
* @return AdsPixel
59+
*/
60+
protected function getAccountPixel() {
61+
$cursor = (new AdAccount($this->getConfig()->accountId))->getAdsPixels();
62+
$this->assertGreaterThanOrEqual(1, $cursor->count());
63+
64+
return $cursor->current();
65+
}
66+
67+
/**
68+
* @depends testCreate
69+
*/
70+
public function testCrud() {
71+
$pixel = $this->getAccountPixel();
72+
73+
$this->assertCanRead($pixel, array(
74+
AdsPixelsFields::NAME,
75+
));
76+
77+
$name = $pixel->{AdsPixelsFields::NAME};
78+
79+
$this->assertCanUpdate(
80+
$pixel,
81+
array(
82+
AdsPixelsFields::NAME => $this->getConfig()->testRunId.' updated',
83+
));
84+
85+
$pixel->{AdsPixelsFields::NAME} = $name;
86+
$pixel->update();
87+
88+
$this->assertCanFetchConnection($pixel, 'getAdAccounts', array(), array(
89+
'business' => $this->getConfig()->businessId,
90+
));
91+
$this->assertCanFetchConnection($pixel, 'getAgencies');
92+
93+
$this->assertCannotDelete($pixel);
94+
}
95+
96+
/**
97+
* @depends testCreate
98+
*/
99+
public function testShareWithAdAccount() {
100+
$this->skipIf('no_business_manager');
101+
102+
if (!$this->getConfig()->secondaryBusinessId) {
103+
$this->markTestSkipped('No secondary business provided in config');
104+
}
105+
106+
if (!$this->getConfig()->secondaryAccountId) {
107+
$this->markTestSkipped('No secondary ad account provided in config');
108+
}
109+
110+
$account_id_int = (int) substr($this->getConfig()->secondaryAccountId, 4);
111+
112+
$pixel = $this->getAccountPixel();
113+
114+
$pixel->sharePixelWithAdAccount(
115+
$this->getConfig()->secondaryBusinessId,
116+
$account_id_int);
117+
118+
$pixel->unsharePixelWithAdAccount(
119+
$this->getConfig()->secondaryBusinessId,
120+
$account_id_int);
121+
122+
}
123+
124+
/**
125+
* @depends testCreate
126+
*/
127+
public function testShareWithAgency() {
128+
$this->skipIf('no_business_manager');
129+
130+
if (!$this->getConfig()->secondaryAccountId) {
131+
$this->markTestSkipped('No secondary business provided in config');
132+
}
133+
134+
$pixel = $this->getAccountPixel();
135+
136+
$business = new Business($this->getConfig()->businessId);
137+
$cursor = $business->getAgencies();
138+
$cursor->setUseImplicitFetch(true);
139+
140+
$agency_id = null;
141+
foreach ($cursor as $agency) {
142+
if (
143+
$agency->{AbstractCrudObject::FIELD_ID}
144+
== $this->getConfig()->secondaryBusinessId
145+
) {
146+
$agency_id = $agency->{AbstractCrudObject::FIELD_ID};
147+
}
148+
}
149+
150+
if ($agency_id === null) {
151+
$this->markTestSkipped("Secondary business is not a valid agency");
152+
}
153+
154+
$pixel->sharePixelWithAgency(
155+
$this->getConfig()->businessId,
156+
$this->getConfig()->secondaryBusinessId);
157+
158+
$pixel->unsharePixelWithAgency(
159+
$this->getConfig()->businessId,
160+
$this->getConfig()->secondaryBusinessId);
161+
}
162+
}

0 commit comments

Comments
 (0)