Skip to content

Conversation

@floranpagliai
Copy link
Contributor

#71

@floranpagliai floranpagliai force-pushed the main branch 2 times, most recently from 953ff33 to 1843720 Compare May 21, 2024 17:38
@floranpagliai floranpagliai changed the title Regex segment local evaluation with getype cast expression as int Regex segment local evaluation with int trait May 21, 2024
switch ($this->operator) {
case (SegmentConditions::EQUAL):
$condition = $traitValue == $castedValue;
$condition = $traitValue === $castedValue;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As value are cast on line 91, might as well make a strict comparison

break;
case (SegmentConditions::NOT_EQUAL):
$condition = $traitValue != $castedValue;
$condition = $traitValue !== $castedValue;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

case (SegmentConditions::IN):
if (in_array($traitValueType, ['string', 'integer'])) {
$condition = in_array((string) $traitValue, explode(',', (string) $this->value));
$condition = in_array((string) $traitValue, explode(',', (string) $this->value), true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

[SegmentConditions::REGEX, 'foo', '[a-z]+', true],
[SegmentConditions::REGEX, 'FOO', '[a-z]+', false],
[SegmentConditions::REGEX, '1.2.3', '\\d', true],
[SegmentConditions::REGEX, 123, '^\\d*[13579]$', true],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test that check that an Integer trait value can be compared against a regex

@floranpagliai floranpagliai marked this pull request as ready for review May 21, 2024 18:10
@rolodato rolodato requested a review from khvn26 May 21, 2024 18:54
Copy link
Contributor

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @floranpagliai !

@matthewelwell matthewelwell merged commit 5e4b68f into Flagsmith:main May 24, 2024
@matthewelwell
Copy link
Contributor

@floranpagliai I have just released this fix in v4.3.1 .

@floranpagliai
Copy link
Contributor Author

Thank you so much @matthewelwell for the quick review and release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants