Fix segment priorities (#24)

* Fix segment priorities

* Formatting

* Remove unnecessary includes

* Slight simplification

* fix typo

Author: matthewelwell

src/Engine/Engine.php

@@ -139,6 +139,12 @@ private static function _getIdentityFeatureStatesDict(
 
         foreach ($identitySegments as $is) {
             foreach ($is->getFeatureStates() as $fs) {
+                $feature = $fs->getFeature();
+                $existing = $featureStates[$feature->getName()];
+                if ($existing != null && $existing->isHigherPriority($fs)) {
+                    continue;
+                }
+
                 $featureStates[$fs->getFeature()->getName()] = $fs;
             }
         }

src/Engine/Features/FeatureSegmentModel.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace Flagsmith\Engine\Features;
+
+use Flagsmith\Concerns\HasWith;
+use Flagsmith\Concerns\JsonSerializer;
+
+class FeatureSegmentModel
+{
+    use HasWith;
+    use JsonSerializer;
+
+    public int $priority;
+
+    /**
+     * Get priority.
+     * @return int
+     */
+    public function getPriority(): int
+    {
+        return $this->priority;
+    }
+
+    /**
+     * Build with priority.
+     * @param int $priority
+     * @return FeatureSegmentModel
+     */
+    public function withPriority(int $priority): self
+    {
+        return $this->with('priority', $priority);
+    }
+}

src/Engine/Features/FeatureStateModel.php

@@ -20,10 +20,12 @@ class FeatureStateModel
     public string $featurestate_uuid;
     public MultivariateFeatureStateValueModelList $multivariate_feature_state_values;
     public ?int $django_id = null;
+    public ?FeatureSegmentModel $feature_segment = null;
 
     private array $keys = [
         'feature' => 'Flagsmith\Engine\Features\FeatureModel',
         'multivariate_feature_state_values' => 'Flagsmith\Engine\Utils\Collections\MultivariateFeatureStateValueModelList',
+        'feature_segment' => 'Flagsmith\Engine\Features\FeatureSegmentModel',
     ];
 
     public function __construct()
@@ -127,6 +129,25 @@ public function withFeaturestateUuid(string $featurestateUuid): self
         return $this->with('featurestate_uuid', $featurestateUuid);
     }
 
+    /**
+     * get the feature ssegment.
+     * @return FeatureSegmentModel
+     */
+    public function getFeatureSegment(): ?FeatureSegmentModel
+    {
+        return $this->feature_segment;
+    }
+
+    /**
+     * build with a feature segment model.
+     * @param FeatureSegmentModel $featureSegment
+     * @return FeatureStateModel
+     */
+    public function withFeatureSegment(?FeatureSegmentModel $featureSegment): self
+    {
+        return $this->with('feature_segment', $featureSegment);
+    }
+
     /**
      * get the value.
      */
@@ -213,4 +234,22 @@ protected function setValues($values)
             $this->_value = $featureStateValue;
         }
     }
+
+    /**
+     * Another FeatureStateModel is deemed to be higher priority if and only if
+     * it has a FeatureSegment and either this.FeatureSegment is null or the
+     * value of other.FeatureSegment.priority is lower than that of
+     * this.FeatureSegment.priority.
+     *
+     * @param FeatureStateModel $other - the other FeatureStateModel to compare priority with
+     * @return bool - true if `this` is higher priority than `other`
+     */
+    public function isHigherPriority(FeatureStateModel $other): bool
+    {
+        if ($this->getFeatureSegment() == null || $other->getFeatureSegment() == null) {
+            return ($this->getFeatureSegment() != null && $other->getFeatureSegment() == null);
+        }
+
+        return $this->getFeatureSegment()->getPriority() < $other->getFeatureSegment()->getPriority();
+    }
 }

tests/Engine/EngineTests/EngineTestData

@@ -1,6 +1,7 @@
 <?php
 
 use Flagsmith\Engine\Features\FeatureModel;
+use Flagsmith\Engine\Features\FeatureSegmentModel;
 use Flagsmith\Engine\Features\FeatureStateModel;
 use Flagsmith\Engine\Features\FeatureTypes;
 use Flagsmith\Engine\Features\MultivariateFeatureOptionModel;
@@ -146,4 +147,41 @@ public function testGetValueUsesFeatuestateUuidForMultivariateValueCalculationIf
         // validation in with()
         $this->assertTrue(true);
     }
+
+    public function testIsHigherPriorityReturnsFalseForTwoNullFeatureSegments()
+    {
+        // Given
+        $featureState1 = (new FeatureStateModel())
+            ->withFeatureSegment(null);
+        $featureState2 = (new FeatureStateModel())
+            ->withFeatureSegment(null);
+
+        // Then
+        $this->assertFalse($featureState1->isHigherPriority($featureState2));
+        $this->assertFalse($featureState2->isHigherPriority($featureState1));
+    }
+
+    public function testIsHigherPriorityReturnsTrueWhenOtherFeatureStateHasNullFeatureSegment()
+    {
+        // Given
+        $featureState1 = (new FeatureStateModel())
+            ->withFeatureSegment((new FeatureSegmentModel())->withPriority(0));
+        $featureState2 = (new FeatureStateModel())
+            ->withFeatureSegment(null);
+
+        // Then
+        $this->assertTrue($featureState1->isHigherPriority($featureState2));
+    }
+
+    public function testIsHigherPriorityReturnsTrueWhenOtherFeatureStateHasLowerPriorityFeatureSegment()
+    {
+        // Given
+        $featureState1 = (new FeatureStateModel())
+            ->withFeatureSegment((new FeatureSegmentModel())->withPriority(0));
+        $featureState2 = (new FeatureStateModel())
+            ->withFeatureSegment((new FeatureSegmentModel())->withPriority(1));
+
+        // Then
+        $this->assertTrue($featureState1->isHigherPriority($featureState2));
+    }
 }