Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: conditional extenders #3759

Merged
merged 2 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions framework/core/src/Extend/Conditional.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/

namespace Flarum\Extend;

use Flarum\Extension\Extension;
use Flarum\Extension\ExtensionManager;
use Illuminate\Contracts\Container\Container;

class Conditional implements ExtenderInterface
{
/**
* @var array<array{condition: bool|callable, extenders: ExtenderInterface[]}>
*/
protected $conditions = [];

/**
* @param ExtenderInterface[] $extenders
*/
public function whenExtensionEnabled(string $extensionId, array $extenders): self
Comment on lines +23 to +26
Copy link
Member

Choose a reason for hiding this comment

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

The signature doesn't match the PR comment example. Your example uses a callable as the second argument, not an array. The array is the return response from the callable yes. I assume your example is wrong then..

Also the phpdoc is incorrect, there are two args.

Copy link
Member Author

Choose a reason for hiding this comment

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

yea updated the example thanks

phpdoc doesn't matter, we only add typings for what isn't typed, we can't vanilla type array as ExtenderInterface[], we've being doing this since 1.0 to avoid unnecessary duplication. And the phpdoc standard allows it.

{
return $this->when(function (ExtensionManager $extensions) use ($extensionId) {
return $extensions->isEnabled($extensionId);
}, $extenders);
}

/**
* @param bool|callable $condition
* @param ExtenderInterface[] $extenders
*/
public function when($condition, array $extenders): self
{
$this->conditions[] = [
'condition' => $condition,
'extenders' => $extenders,
];

return $this;
}

public function extend(Container $container, Extension $extension = null)
{
foreach ($this->conditions as $condition) {
if (is_callable($condition['condition'])) {
$condition['condition'] = $container->call($condition['condition']);
}

if ($condition['condition']) {
foreach ($condition['extenders'] as $extender) {
$extender->extend($container, $extension);
}
}
}
}
}
8 changes: 4 additions & 4 deletions framework/core/src/Extension/Extension.php
Original file line number Diff line number Diff line change
Expand Up @@ -414,19 +414,19 @@ public function getLinks()
$links['source'] = $sourceUrl;
}

if (($discussUrl = $this->composerJsonAttribute('support.forum'))) {
if ($discussUrl = $this->composerJsonAttribute('support.forum')) {
$links['discuss'] = $discussUrl;
}

if (($documentationUrl = $this->composerJsonAttribute('support.docs'))) {
if ($documentationUrl = $this->composerJsonAttribute('support.docs')) {
$links['documentation'] = $documentationUrl;
}

if (($websiteUrl = $this->composerJsonAttribute('homepage'))) {
if ($websiteUrl = $this->composerJsonAttribute('homepage')) {
$links['website'] = $websiteUrl;
}

if (($supportEmail = $this->composerJsonAttribute('support.email'))) {
if ($supportEmail = $this->composerJsonAttribute('support.email')) {
$links['support'] = "mailto:$supportEmail";
}

Expand Down
4 changes: 2 additions & 2 deletions framework/core/src/User/Access/Gate.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public function allows(User $actor, string $ability, $model): bool
$appliedPolicies = [];

if ($model) {
$modelClasses = is_string($model) ? [$model] : array_merge(class_parents(($model)), [get_class($model)]);
$modelClasses = is_string($model) ? [$model] : array_merge(class_parents($model), [get_class($model)]);

foreach ($modelClasses as $class) {
$appliedPolicies = array_merge($appliedPolicies, $this->getPolicies($class));
Expand All @@ -87,7 +87,7 @@ public function allows(User $actor, string $ability, $model): bool
// If no policy covered this permission query, we will only grant
// the permission if the actor's groups have it. Otherwise, we will
// not allow the user to perform this action.
if ($actor->isAdmin() || ($actor->hasPermission($ability))) {
if ($actor->isAdmin() || $actor->hasPermission($ability)) {
return true;
}

Expand Down
162 changes: 162 additions & 0 deletions framework/core/tests/integration/extenders/ConditionalTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
<?php

/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/

namespace Flarum\Tests\integration\extenders;

use Exception;
use Flarum\Api\Serializer\ForumSerializer;
use Flarum\Extend;
use Flarum\Extension\ExtensionManager;
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;

class ConditionalTest extends TestCase
{
use RetrievesAuthorizedUsers;

/** @test */
public function conditional_works_if_condition_is_primitive_true()
{
$this->extend(
(new Extend\Conditional())
->when(true, [
(new Extend\ApiSerializer(ForumSerializer::class))
->attributes(function () {
return [
'customConditionalAttribute' => true
];
})
])
);

$this->app();

$response = $this->send(
$this->request('GET', '/api', [
'authenticatedAs' => 1,
])
);

$payload = json_decode($response->getBody()->getContents(), true);

$this->assertArrayHasKey('customConditionalAttribute', $payload['data']['attributes']);
}

/** @test */
public function conditional_does_not_work_if_condition_is_primitive_false()
{
$this->extend(
(new Extend\Conditional())
->when(false, [
(new Extend\ApiSerializer(ForumSerializer::class))
->attributes(function () {
return [
'customConditionalAttribute' => true
];
})
])
);

$this->app();

$response = $this->send(
$this->request('GET', '/api', [
'authenticatedAs' => 1,
])
);

$payload = json_decode($response->getBody()->getContents(), true);

$this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']);
}

/** @test */
public function conditional_works_if_condition_is_callable_true()
{
$this->extend(
(new Extend\Conditional())
->when(function () {
return true;
}, [
(new Extend\ApiSerializer(ForumSerializer::class))
->attributes(function () {
return [
'customConditionalAttribute' => true
];
})
])
);

$this->app();

$response = $this->send(
$this->request('GET', '/api', [
'authenticatedAs' => 1,
])
);

$payload = json_decode($response->getBody()->getContents(), true);

$this->assertArrayHasKey('customConditionalAttribute', $payload['data']['attributes']);
}

/** @test */
public function conditional_does_not_work_if_condition_is_callable_false()
{
$this->extend(
(new Extend\Conditional())
->when(function () {
return false;
}, [
(new Extend\ApiSerializer(ForumSerializer::class))
->attributes(function () {
return [
'customConditionalAttribute' => true
];
})
])
);

$this->app();

$response = $this->send(
$this->request('GET', '/api', [
'authenticatedAs' => 1,
])
);

$payload = json_decode($response->getBody()->getContents(), true);

$this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']);
}

/** @test */
public function conditional_injects_dependencies_to_condition_callable()
{
$this->expectNotToPerformAssertions();

$this->extend(
(new Extend\Conditional())
->when(function (?ExtensionManager $extensions) {
if (! $extensions) {
throw new Exception('ExtensionManager not injected');
}
}, [
(new Extend\ApiSerializer(ForumSerializer::class))
->attributes(function () {
return [
'customConditionalAttribute' => true
];
})
])
);

$this->app();
}
}
8 changes: 4 additions & 4 deletions framework/core/tests/integration/extenders/LocalesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function custom_translation_does_not_exist_by_default()
public function custom_translation_exists_if_added()
{
$this->extend(
(new Extend\Locales(dirname(__FILE__, 3).'/fixtures/locales'))
new Extend\Locales(dirname(__FILE__, 3).'/fixtures/locales')
);

$this->app()->getContainer()->make('flarum.locales');
Expand All @@ -57,7 +57,7 @@ public function custom_translation_exists_if_added()
public function custom_translation_exists_if_added_with_intl_suffix()
{
$this->extend(
(new Extend\Locales(dirname(__FILE__, 3).'/fixtures/locales'))
new Extend\Locales(dirname(__FILE__, 3).'/fixtures/locales')
);

$this->app()->getContainer()->make('flarum.locales');
Expand All @@ -72,7 +72,7 @@ public function custom_translation_exists_if_added_with_intl_suffix()
public function messageformat_works_in_translations()
{
$this->extend(
(new Extend\Locales(dirname(__FILE__, 3).'/fixtures/locales'))
new Extend\Locales(dirname(__FILE__, 3).'/fixtures/locales')
);

$this->app()->getContainer()->make('flarum.locales');
Expand All @@ -87,7 +87,7 @@ public function messageformat_works_in_translations()
public function laravel_interface_methods_work()
{
$this->extend(
(new Extend\Locales(dirname(__FILE__, 3).'/fixtures/locales'))
new Extend\Locales(dirname(__FILE__, 3).'/fixtures/locales')
);

$this->app()->getContainer()->make('flarum.locales');
Expand Down