(chore) Minor refactor to make boost campaign metrics get method more...

(chore) Minor refactor to make boost campaign metrics get method more descriptive getImpressionsMet - #631

Core/Boost/Campaigns/Delegates/PaymentsDelegate.php

@@ -220,7 +220,7 @@ class PaymentsDelegate
 
  $impressionsMet = $this->metrics
  ->setCampaign($campaign)
- ->get();
+ ->getImpressionsMet();
 
  // Calculate the cost of the impressions met
 

Core/Boost/Campaigns/Dispatcher.php

@@ -62,10 +62,11 @@ class Dispatcher
  public function syncIfImpressionsThresholdMet(): void
  {
  if ($this->campaign->isDelivering()) {
- $impressionsMet = $this->campaign->getImpressionsMet();
- $this->campaign = $this->metrics->syncImpressionsMet();
+ $currentImpressionsMet = $this->campaign->getImpressionsMet();
+ $newImpressionsMet = $this->metrics->getImpressionsMet();
 
- if ($this->campaign->getImpressionsMet() - $impressionsMet >= $this->impressionsSyncThreshold) {
+ if ($newImpressionsMet - $currentImpressionsMet >= $this->impressionsSyncThreshold) {
+ $this->campaign->setImpressionsMet($newImpressionsMet);
  error_log("[BoostCampaignsDispatcher] Saving updated {$this->campaign->getUrn()}...");
  $this->manager->sync($this->campaign);
  }

Core/Boost/Campaigns/Manager.php

@@ -317,10 +317,7 @@ class Manager
  throw new CampaignException('Campaign should be in [created] or [approved] state in order to cancel it');
  }
 
- $campaign = $this->metrics
- ->setCampaign($campaign)
- ->syncImpressionsMet();
-
+ $campaign->setImpressions($this->metrics->setCampaign($campaign)->getImpressionsMet());
  $campaign->setRevokedTimestamp(time() * 1000);
 
  $this->paymentsDelegate->onStateChange($campaign);

Core/Boost/Campaigns/Metrics.php

 <?php
-/**
- * Metrics
- * @author edgebal
- */
 
 namespace Minds\Core\Boost\Campaigns;
 
@@ -42,36 +38,43 @@ class Metrics
  * @return Metrics
  * @throws Exception
  */
- public function setCampaign(Campaign $campaign)
+ public function setCampaign(Campaign $campaign): self
  {
  $this->campaign = $campaign;
  return $this;
  }
 
  /**
- * @return bool
  * @throws Exception
  */
- public function increment()
+ public function increment(): void
  {
- // Increment general boosts counter
+ $this->incrementGlobalBoostCounter();
+ $this->incrementCampaignBoostCounter();
+ $this->incrementEntityCounters();
+ }
 
+ protected function incrementGlobalBoostCounter(): void
+ {
  $this->counters
  ->setEntityGuid(0)
  ->setMetric('boost_impressions')
  ->increment();
+ }
 
-  // Increment boost counter
-
+ protected function incrementCampaignBoostCounter(): void
+ {
  $this->counters
  ->setEntityGuid($this->campaign->getGuid())
  ->setMetric('boost_impressions')
  ->increment();
+ }
 
- // Increment entity counters
- // NOTE: Campaigns have a _single_ entity, for now. Refactor this when we support multiple
- // Ideally, we should use a composite URN, like: urn:campaign-entity:100000321:(urn:activity:100000500)
+ protected function incrementEntityCounters(): void
+ {
  foreach ($this->campaign->getEntityUrns() as $entityUrn) {
+ // NOTE: Campaigns have a _single_ entity, for now. Refactor this when we support multiple
+ // Ideally, we should use a composite URN, like: urn:campaign-entity:100000321:(urn:activity:100000500)
  $entity = $this->resolver->single(new Urn($entityUrn));
 
  if ($entity) {
@@ -86,33 +89,17 @@ class Metrics
  ->increment();
  }
  }
-
- return true;
  }
 
  /**
  * @return int
  * @throws Exception
  */
- public function get()
+ public function getImpressionsMet(): int
  {
  return $this->counters
  ->setEntityGuid($this->campaign->getGuid())
  ->setMetric('boost_impressions')
  ->get(false);
  }
-
- /**
- * @return Campaign
- * @throws Exception
- */
- public function syncImpressionsMet()
- {
- $count = $this->get();
-
- $this->campaign
- ->setImpressionsMet($count);
-
- return $this->campaign;
- }
 }

Spec/Core/Boost/Campaigns/DispatcherSpec.php

@@ -56,22 +56,18 @@ class DispatcherSpec extends ObjectBehavior
  ->willReturn($this->metrics);
 
  $this->metrics
- ->syncImpressionsMet()
- ->shouldBeCalled()
- ->willReturn($campaign);
-
- $campaign->getImpressionsMet()
+ ->getImpressionsMet()
  ->shouldBeCalled()
  ->willReturn(1);
 
- $this->manager->sync($campaign)
+ $this->manager->sync($initialCampaign)
  ->shouldNotBeCalled();
 
- $campaign->shouldBeCompleted(Argument::any())
+ $initialCampaign->shouldBeCompleted(Argument::any())
  ->shouldBeCalled()
  ->willReturn(false);
 
- $campaign->shouldBeStarted(Argument::any())
+ $initialCampaign->shouldBeStarted(Argument::any())
  ->shouldBeCalled()
  ->willReturn(false);
 
@@ -82,6 +78,9 @@ class DispatcherSpec extends ObjectBehavior
  Campaign $initialCampaign,
  Campaign $campaign
  ) {
+ $initialCampaign->getUrn()
+ ->willReturn('urn:campaign:1000');
+
  $campaign->getUrn()
  ->willReturn('urn:campaign:1000');
 
@@ -103,23 +102,24 @@ class DispatcherSpec extends ObjectBehavior
  ->willReturn($this->metrics);
 
  $this->metrics
- ->syncImpressionsMet()
+ ->getImpressionsMet()
  ->shouldBeCalled()
- ->willReturn($campaign);
+ ->willReturn(10);
 
- $campaign->getImpressionsMet()
+ $initialCampaign
+ ->setImpressionsMet(10)
  ->shouldBeCalled()
- ->willReturn(10);
+ ->willReturn($initialCampaign);
 
- $this->manager->sync($campaign)
+ $this->manager->sync($initialCampaign)
  ->shouldBeCalled()
- ->willReturn($campaign);
+ ->willReturn($initialCampaign);
 
- $campaign->shouldBeCompleted(Argument::any())
+ $initialCampaign->shouldBeCompleted(Argument::any())
  ->shouldBeCalled()
  ->willReturn(false);
 
- $campaign->shouldBeStarted(Argument::any())
+ $initialCampaign->shouldBeStarted(Argument::any())
  ->shouldBeCalled()
  ->willReturn(false);
 
@@ -136,6 +136,11 @@ class DispatcherSpec extends ObjectBehavior
  ->shouldBeCalled()
  ->willReturn($campaign);
 
+ $this->metrics
+ ->setCampaign($campaign)
+ ->shouldBeCalled()
+ ->willReturn($this->metrics);
+
  $campaign->isDelivering()
  ->shouldBeCalled()
  ->willReturn(false);
@@ -164,6 +169,11 @@ class DispatcherSpec extends ObjectBehavior
  ->shouldBeCalled()
  ->willReturn($campaign);
 
+ $this->metrics
+ ->setCampaign($campaign)
+ ->shouldBeCalled()
+ ->willReturn($this->metrics);
+
  $campaign->isDelivering()
  ->shouldBeCalled()
  ->willReturn(false);