Skip to content

Commit

Permalink
Add Critera api to exit a single group() level (#147)
Browse files Browse the repository at this point in the history
It was decided that 306f813 was too risky a change that could be
considered API breaking and thus a major version jump. To reduce this we
can add a new method for explicitly escaping a single group level
instead of making Criteria::end() do two different things depending on
context.

This adds the ability for authors to add multiple groups to their
display Criteria, without interrupting people with nested group() rules
that rely on end() to return the form field the rules apply to.
  • Loading branch information
NightJar committed Dec 11, 2022
1 parent f124dc8 commit 2c069d3
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 6 deletions.
29 changes: 23 additions & 6 deletions src/Criteria.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@

namespace UncleCheese\DisplayLogic;

use SilverStripe\Core\Injector\Injectable;
use SilverStripe\Core\Extensible;
use SilverStripe\Core\Config\Configurable;
use BadMethodCallException;
use OutOfRangeException;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Config\Configurable;
use SilverStripe\Core\Extensible;
use SilverStripe\Core\Injector\Injectable;
use SilverStripe\Forms\FormField;
use BadMethodCallException;

class Criteria
{
Expand Down Expand Up @@ -252,8 +253,9 @@ public function getAnimation()
}

/**
* Ends the chaining and returns the parent object, either {@link Criteria} or {@link FormField}
* @return FormField/Criteria
* Ends the chaining and returns the parent {@link FormField}.
* Works recursively if in a group {@link Criteria} context
* @return FormField
*/
public function end()
{
Expand All @@ -264,6 +266,21 @@ public function end()
return $this->slave;
}

/**
* Escape a group {@link Criteria}
* @return Criteria the parent Criteria
* @throws LogicException if there is no parent Criteria
*/
public function endGroup()
{
if (!$this->parent) {
$message = __FUNCTION__ . 'called on Criteria with no parent (not in a group).';
$message .= ' Call group() before endGroup(), or perhaps you\'re looking for end() instead.';
throw new OutOfRangeException($message);
}
return $this->parent;
}

/**
* Creates a JavaScript readable representation of the logic
* @return string
Expand Down
52 changes: 52 additions & 0 deletions tests/php/CriteriaTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

namespace UncleCheese\DisplayLogic\Tests;

use OutOfRangeException;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\Forms\TextField;
use UncleCheese\DisplayLogic\Criteria;

class CriteriaTest extends SapphireTest
{
public function testEndIsRecursiveAndReturnsTheFormField()
{
$field = new TextField('test');
$displayRulesEnd = $field->displayIf('one')->isEmpty()
->andIf()
->group()
->orIf('two')->isEqualTo('no')
->orIf('two')->isEqualTo('non')
->orIf()
->group() // nested group
->andIf('three')->isEqualTo('yes')
->andIf('four')->isEqualTo('oui')
->end();
$this->assertInstanceOf(TextField::class, $displayRulesEnd);
}

public function testEndGroupOnlyReturnsTheParentCriteria()
{
$field = new TextField('test');
$displayRulesEnd = $field->displayIf('one')->isEmpty()
->andIf()
->group()
->orIf('two')->isEqualTo('no')
->orIf('two')->isEqualTo('non')
->endGroup()
->andIf()
->group()
->orIf('three')->isEqualTo('yes')
->orIf('three')->isEqualTo('oui')
->endGroup();
$this->assertInstanceOf(Criteria::class, $displayRulesEnd);
}

public function testEndGroupThrowsExceptionWhenThereIsNoParentCriteria()
{
$this->expectException(OutOfRangeException::class);

$field = new TextField('test');
$displayRulesEnd = $field->displayIf('one')->isEmpty()->endGroup();
}
}

0 comments on commit 2c069d3

Please sign in to comment.