WIP: Boost Campaigns (&24)
mentioned in merge request !207 (closed)
added MR::Awaiting Review scoped label
added 1 commit
- d587d4cf - (wip)(refactor): Campaigns should use their own ES index and cql table
added 16 commits
- 99168bdd...1674767b - 15 commits from branch
master
- fdfd433c - Merge remote-tracking branch 'upstream/master' into goal/boost-campaigns-e24
- 99168bdd...1674767b - 15 commits from branch
added 4 commits
Toggle commit listadded 18 commits
- 434afa0a...5797d6ea - 16 commits from branch
master
- 986398b1 - (chore): Tweaks on Resolver ACL ignoring
- e83768eb - Merge remote-tracking branch 'upstream/master' into goal/boost-campaigns-e24
- 434afa0a...5797d6ea - 16 commits from branch
added 2 commits
- Developer
Is this awaiting review with a WIP: accidentally left on, or the other way around?
automatically removed MR::Awaiting Review label
added 30 commits
- 130b7876...e33eedaf - 29 commits from branch
master
- 8f3875e0 - Merge remote-tracking branch 'origin/master' into goal/boost-campaigns-e24
- 130b7876...e33eedaf - 29 commits from branch
added Squad::Yellow scoped label
added 9 commits
- 23abc088...88286ad8 - 8 commits from branch
master
- f9178dcc - Merge remote-tracking branch 'origin/master' into goal/boost-campaigns-e24
- 23abc088...88286ad8 - 8 commits from branch
added 1 commit
- 3e9e4235 - (feat): implemented new offsets from v2/boost/feeds
added 11 commits
- a821454e...3f81d85a - 9 commits from branch
master
- 89ef1571 - Merge remote-tracking branch 'origin/master' into goal/boost-campaigns-e24
- 7fb97f93 - (test): Fix foreign test failures due to Resolver changes
- a821454e...3f81d85a - 9 commits from branch
added 1 commit
- 9ca3cbb4 - (fix): Circular DI issue on Entities\Resolver
added 1 commit
- e9a5016e - (fix): Campaigns not being listed because of defaults
27 28 $today = strtotime(date('Y-m-d') . ' 00:00:00') * 1000; 29 $start = strtotime(date('Y-m-d', $start) . ' 00:00:00') * 1000; 30 $end = strtotime(date('Y-m-d', $end) . ' 23:59:59') * 1000; 31 32 if ($start < $today) { 33 throw new CampaignException('Campaign start should not be in the past'); 34 } elseif ($start >= $end) { 35 throw new CampaignException('Campaign end before starting'); 36 } 37 38 $campaign 39 ->setStart($start) 40 ->setEnd($end); 41 42 return $campaign; - Owner
I think, generally, delegates should not return values and be
void
?
27 28 $today = strtotime(date('Y-m-d') . ' 00:00:00') * 1000; 29 $start = strtotime(date('Y-m-d', $start) . ' 00:00:00') * 1000; 30 $end = strtotime(date('Y-m-d', $end) . ' 23:59:59') * 1000; 31 32 if ($start < $today) { 33 throw new CampaignException('Campaign start should not be in the past'); 34 } elseif ($start >= $end) { 35 throw new CampaignException('Campaign end before starting'); 36 } 37 38 $campaign 39 ->setStart($start) 40 ->setEnd($end); 41 42 return $campaign; - Owner
also, some PHP 7 scala typing would be
118 return $this; 119 } 120 121 /** 122 * @param int $quality 123 * @return Iterator 124 */ 125 public function setQuality(int $quality) 126 { 127 $this->quality = $quality; 128 return $this; 129 } 130 131 public function getList() 132 { 133 $response = $this->manager->getList(['limit' => $this->limit, 'from' => $this->from, 'offset' => $this->offset, 'type' => $this->type, 'owner_guid' => $this->ownerGuid, 'state' => $this->state, 'rating' => $this->rating, 'quality' => $this->quality,]); - Owner
can we format this?
- Last updated by Mark Harding
125 131 126 132 $sorted = array_filter($sorted, function ($entity) { return (bool) $entity; }); 127 133 128 // Filter out forbidden entities 134 // Filter out forbidden entities, if not ignoring ACL 135 136 $ignoreAcl = $this->opts['ignoreAcl'] ?? false; 129 137 130 $sorted = array_filter($sorted, function ($entity) { 131 return $this->acl->read($entity, $this->user); 132 //&& !Flags::shouldFail($entity); 133 }); 138 if (!$ignoreAcl) { - Owner
Should this not be the job of the
Security\ACL::$ignore
? cc\ @brianhatchet with his new RBAC wisdom. - Owner
patching the acl skipping in multiple different places doesn't seem right
39 throw new NotImplementedException(); 40 } 41 42 /** 43 * @param Campaign $campaign 44 * @return bool 45 * @throws Exception 46 */ 47 public function add(Campaign $campaign) 48 { 49 $cql = "INSERT INTO boost_campaigns (type, guid, owner_guid, json_data, delivery_status) VALUES (?, ?, ?, ?, ?)"; 50 $values = [ 51 $campaign->getType(), 52 new Varint($campaign->getGuid()), 53 new Varint($campaign->getOwnerGuid()), 54 json_encode([ - Owner
Perhaps move this outside of values to avoid confusion?
- Owner
Looking good from a first quick glance! ACl needs some clarification. Whats the remaining steps?