Skip to content

Commit

Permalink
refactor(next): fix Drupal coding standards
Browse files Browse the repository at this point in the history
* refactor(next): lint warnings in Drupal PHP coding standards
* build(next): simplify phpcs task in CI
* build(next): add DrupalPractice coding standards to CI

Fixes #551
  • Loading branch information
JohnAlbin authored Oct 19, 2023
1 parent 050429c commit 6fb7718
Show file tree
Hide file tree
Showing 35 changed files with 70 additions and 77 deletions.
8 changes: 3 additions & 5 deletions .github/workflows/next.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,13 @@ jobs:
cd ~/drupal
COMPOSER_MEMORY_LIMIT=-1 composer require --dev phpspec/prophecy-phpunit:^2 -W
if: ${{ startsWith(matrix.drupal, '9') }}
- name: Run phpcs
run: |
~/drupal/vendor/bin/phpcs -p -s --colors --standard=modules/next/phpcs.xml modules/next
- name: Copy module
run: cp -R ../next-drupal/modules/next ~/drupal/web/modules/next
- name: Run php built-in server
run: php -S 127.0.0.1:8080 -t ~/drupal/web &
- name: Run phpcs
run: |
cd ~/drupal/web
cp modules/next/phpcs.xml.dist .
../vendor/bin/phpcs --standard=./phpcs.xml.dist modules/next -p -s -n --colors
- name: Run PHPUnit
run: |
cd ~/drupal/web
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use Drupal\Core\TypedData\ComputedItemListTrait;

/**
* Class ContentTranslationsFieldItemList.
* A list of field item objects with translations.
*/
class ContentTranslationsFieldItemList extends FieldItemList {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
* Class JwtEventSubscriber.
* Subscribes to JWT authentication events.
*/
class JwtEventSubscriber implements EventSubscriberInterface {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\Core\Url;
use Drupal\jwt\Authentication\Provider\JwtAuth;
use Drupal\next\Annotation\PreviewUrlGenerator;
use Drupal\next\Entity\NextSiteInterface;
use Drupal\next\Exception\InvalidPreviewUrlRequest;
use Drupal\next\Plugin\ConfigurablePreviewUrlGeneratorBase;
Expand Down Expand Up @@ -142,7 +141,8 @@ public function generate(NextSiteInterface $next_site, EntityInterface $entity,
$query['timestamp'] = $timestamp = $this->time->getRequestTime();
$query['secret'] = $secret = $this->previewSecretGenerator->generate($timestamp . $slug . $resource_version . $this->user->uuid());

// Generate a JWT and store it temporarily so that we can retrieve it on validate.
// Generate a JWT and store it temporarily so that we can retrieve it on
// validate.
$jwt = $this->jwtAuth->generateToken();
$this->keyValue->get('next_jwt')
->setWithExpire($secret, $jwt, $this->configuration['access_token_expiration']);
Expand Down
10 changes: 10 additions & 0 deletions modules/next/phpcs.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="DrupalProjectNext">
<!-- Ignore files -->
<exclude-pattern>*/config/*</exclude-pattern>
<exclude-pattern>*/css/*</exclude-pattern>
<exclude-pattern>*/js/*</exclude-pattern>

<rule ref="Drupal"/>
<rule ref="DrupalPractice"/>
</ruleset>
12 changes: 0 additions & 12 deletions modules/next/phpcs.xml.dist

This file was deleted.

2 changes: 1 addition & 1 deletion modules/next/src/Controller/NextSiteEntityController.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public function environmentVariables(NextSiteInterface $next_site) {
'#context' => [
'name' => $name,
'value' => $value,
]
],
];
}

Expand Down
1 change: 0 additions & 1 deletion modules/next/src/Entity/NextEntityTypeConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace Drupal\next\Entity;

use Drupal\Core\Config\Entity\ConfigEntityBase;
use Drupal\Core\Entity\EntityInterface;
use Drupal\next\Plugin\RevalidatorInterface;
use Drupal\next\Plugin\SiteResolverInterface;
use Drupal\next\RevalidatorPluginCollection;
Expand Down
3 changes: 0 additions & 3 deletions modules/next/src/Entity/NextEntityTypeConfigInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@
namespace Drupal\next\Entity;

use Drupal\Core\Config\Entity\ConfigEntityInterface;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\EntityWithPluginCollectionInterface;
use Drupal\next\Plugin\RevalidatorInterface;
use Drupal\next\Plugin\SiteResolverInterface;
use Drupal\next\RevalidatorPluginCollection;
use Drupal\next\SiteResolverPluginCollection;

/**
* Provides an interface for next_entity_type_config entity definitions.
Expand Down
29 changes: 15 additions & 14 deletions modules/next/src/Entity/NextSite.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ public function setRevalidateSecret(string $revalidate_secret): NextSiteInterfac
* {@inheritdoc}
*/
public function getPreviewUrlForEntity(EntityInterface $entity): Url {
// Anonymous users do not have access to the preview url.
// Same for authenticated users with no additional roles, since we assume no scope.
// Anonymous users do not have access to the preview url. Same for
// authenticated users with no additional roles, since we assume no scope.
if (\Drupal::currentUser()->isAnonymous() || (!count(\Drupal::currentUser()->getRoles(TRUE)) && \Drupal::currentUser()->id() !== "1")) {
return $this->getLiveUrlForEntity($entity);
}
Expand All @@ -190,28 +190,29 @@ public function getPreviewUrlForEntity(EntityInterface $entity): Url {
/** @var \Drupal\Core\Entity\RevisionableInterface $entity */
$rel = NULL;

// In Drupal terms, a "working copy" is the latest revision. It may or may not
// be a "default" revision. This revision is the working copy because it is
// the revision to which new work will be applied. In other words, it denotes
// the most recent revision which might be considered a work-in-progress.
// In Drupal terms, a "working copy" is the latest revision. It may or may
// not be a "default" revision. This revision is the working copy because
// it is the revision to which new work will be applied. In other words,
// it denotes the most recent revision which might be considered a
// work-in-progress.
// @see \Drupal\jsonapi\Revisions\VersionByRel
if ($entity->isLatestRevision()) {
$rel = 'working-copy';
}

// In Drupal terms, the "latest version" is the latest "default" revision. It
// may or may not have later revisions after it, as long as none of them are
// "default" revisions. This revision is the latest version because it is the
// last revision where work was considered finished. Typically, this means
// that it is the most recent "published" revision.
// In Drupal terms, the "latest version" is the latest "default" revision.
// It may or may not have later revisions after it, as long as none of
// them are "default" revisions. This revision is the latest version
// because it is the last revision where work was considered finished.
// Typically, this means that it is the most recent "published" revision.
// @see \Drupal\jsonapi\Revisions\VersionByRel
if ($entity->isDefaultRevision()) {
$rel = 'latest-version';
}
$resource_version = $rel ? "rel:$rel" : "id:{$entity->getRevisionId()}";
}

// TODO: Extract this to a service.
// @todo Extract this to a service.
/** @var \Drupal\next\NextSettingsManagerInterface $next_settings */
$next_settings = \Drupal::service('next.settings.manager');
$preview_url_generator = $next_settings->getPreviewUrlGenerator();
Expand All @@ -228,8 +229,8 @@ public function getPreviewUrlForEntity(EntityInterface $entity): Url {

$query = $preview_url->getOption('query');

// Add the plugin to the query.
// This allows next.js app to determine the preview strategy based on the plugin.
// Add the plugin to the query. This allows next.js app to determine the
// preview strategy based on the plugin.
$query['plugin'] = $preview_url_generator->getId();

// Add the locale to the query.
Expand Down
4 changes: 3 additions & 1 deletion modules/next/src/Event/EntityActionEventInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ interface EntityActionEventInterface {
public const UPDATE_ACTION = 'update';

/**
* The entity delete action. We use predelete because we need access to the entity for revalidating.
* The entity delete action.
*
* We use predelete because we need access to the entity for revalidating.
*/
public const DELETE_ACTION = 'delete';

Expand Down
2 changes: 0 additions & 2 deletions modules/next/src/Event/EntityRevalidatedEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Drupal\next\Event;

use Drupal\Core\Entity\EntityInterface;

/**
* Defines an entity revalidated event.
*
Expand Down
2 changes: 1 addition & 1 deletion modules/next/src/Form/NextEntityTypeConfigDeleteForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use Drupal\Core\Url;

/**
* Provides a deletion confirmation form for the next_entity_type_config deletion form.
* Provides a deletion confirmation form for next_entity_type_config.
*/
class NextEntityTypeConfigDeleteForm extends EntityDeleteForm {

Expand Down
4 changes: 3 additions & 1 deletion modules/next/src/Form/NextEntityTypeConfigForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@ class NextEntityTypeConfigForm extends EntityForm {
*/
public function __construct(EntityTypeManagerInterface $entity_type_manager, EntityTypeBundleInfoInterface $entity_type_bundle_info, SiteResolverManagerInterface $site_resolver_manager, RevalidatorManagerInterface $revalidator_manager = NULL) {
if (!$revalidator_manager) {
@trigger_error('Calling NextEntityTypeConfigForm::__construct() without the $revalidator_manager argument is deprecated in next:1.4.0. The $revalidator_manager argument will be required in next:2.0.0. See https://www.drupal.org/project/next/releases/1.4.0', E_USER_DEPRECATED);
@trigger_error('Calling NextEntityTypeConfigForm::__construct() without the $revalidator_manager argument is deprecated in next:1.4.0 and will be required in next:2.0.0. See https://www.drupal.org/node/3325622', E_USER_DEPRECATED);
// @codingStandardsIgnoreStart
$revalidator_manager = \Drupal::service('plugin.manager.next.revalidator');
// @codingStandardsIgnoreEnd
}

$this->entityTypeManager = $entity_type_manager;
Expand Down
4 changes: 3 additions & 1 deletion modules/next/src/Form/NextSettingsForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ class NextSettingsForm extends ConfigFormBase {
*/
public function __construct(ConfigFactoryInterface $config_factory, SitePreviewerManagerInterface $site_previewer_manager, PreviewUrlGeneratorManagerInterface $preview_url_generator_manager = NULL) {
if (!$preview_url_generator_manager) {
@trigger_error('Calling NextSettingsForm::__construct() without the $preview_url_generator_manager argument is deprecated in next:1.3.0. The $preview_url_generator_manager argument will be required in next:2.0.0. See https://www.drupal.org/project/next/releases/1.3.0', E_USER_DEPRECATED);
@trigger_error('Calling NextSettingsForm::__construct() without the $preview_url_generator_manager argument is deprecated in next:1.3.0 and will be required in next:2.0.0. See https://www.drupal.org/node/3308330', E_USER_DEPRECATED);
// @codingStandardsIgnoreStart
$preview_url_generator_manager = \Drupal::service('plugin.manager.next.preview_url_generator');
// @codingStandardsIgnoreEnd
}

parent::__construct($config_factory);
Expand Down
7 changes: 5 additions & 2 deletions modules/next/src/NextEntityTypeManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public function getConfigEntityTypeIds() {
public function getEntityFromRouteMatch(RouteMatchInterface $route_match): ?EntityInterface {
$entity_types_ids = $this->getConfigEntityTypeIds();

// TODO: Handle all revisionable entity types.
// @todo Handle all revisionable entity types.
$revision_routes = ['entity.node.revision', 'entity.node.latest_version'];
if (in_array($route_match->getRouteName(), $revision_routes) && in_array('node', $entity_types_ids)) {
$node_revision = $route_match->getParameter('node_revision');
Expand Down Expand Up @@ -93,9 +93,12 @@ public function getEntityFromRouteMatch(RouteMatchInterface $route_match): ?Enti
* {@inheritdoc}
*/
public function isEntityRevisionable(EntityInterface $entity): bool {
// @todo How do you do dependency injection on optional dependencies?
// @codingStandardsIgnoreStart
if (\Drupal::hasService('jsonapi.resource_type.repository')) {
/* @var \Drupal\jsonapi\ResourceType\ResourceTypeRepositoryInterface $resource_type_repository */
/** @var \Drupal\jsonapi\ResourceType\ResourceTypeRepositoryInterface $resource_type_repository */
$resource_type_repository = \Drupal::service('jsonapi.resource_type.repository');
// @codingStandardsIgnoreEnd
$resource = $resource_type_repository->get($entity->getEntityTypeId(), $entity->bundle());
return $resource->isVersionable() && $entity->getEntityType()->isRevisionable();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use Drupal\Core\Messenger\MessengerInterface;
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\Core\Url;
use Drupal\next\Annotation\PreviewUrlGenerator;
use Drupal\next\Entity\NextSiteInterface;
use Drupal\next\Exception\InvalidPreviewUrlRequest;
use Drupal\next\Plugin\ConfigurablePreviewUrlGeneratorBase;
Expand Down
1 change: 0 additions & 1 deletion modules/next/src/Plugin/Next/Revalidator/Path.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace Drupal\next\Plugin\Next\Revalidator;

use Drupal\Core\Form\FormStateInterface;
use Drupal\next\Annotation\Revalidator;
use Drupal\next\Event\EntityActionEvent;
use Drupal\next\Plugin\ConfigurableRevalidatorBase;
use Drupal\next\Plugin\RevalidatorInterface;
Expand Down
7 changes: 3 additions & 4 deletions modules/next/src/Plugin/Next/SitePreviewer/Iframe.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use Drupal\Core\Form\FormBuilderInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\Core\Url;
use Drupal\next\Form\IframeSitePreviewerSwitcherForm;
use Drupal\next\Plugin\ConfigurableSitePreviewerBase;
use Symfony\Component\DependencyInjection\ContainerInterface;
Expand Down Expand Up @@ -191,7 +190,7 @@ public function render(EntityInterface $entity, array $sites) {
'class' => [
$entity->isPublished() ? 'published' : '',
],
]
],
];
}

Expand Down Expand Up @@ -243,8 +242,8 @@ public function render(EntityInterface $entity, array $sites) {
'iframe_preview' => [
'sync_route' => $this->configuration['sync_route'],
'skip_routes' => array_map('trim', explode("\n", mb_strtolower($this->configuration['sync_route_skip_routes']))),
]
]
],
],
],
'library' => [
'next/site_preview.iframe',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
* @SiteResolver(
* id = "entity_reference_field",
* label = "Entity reference field",
* description = "This plugin allows you to select an entity reference field from which to resolve the Next.js site."
* description = "This plugin allows you to select an entity reference field
* from which to resolve the Next.js site."
* )
*/
class EntityReferenceField extends ConfigurableSiteResolverBase implements ContainerFactoryPluginInterface {
Expand Down
3 changes: 2 additions & 1 deletion modules/next/src/Plugin/Next/SiteResolver/SiteSelector.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
* @SiteResolver(
* id = "site_selector",
* label = "Site selector",
* description = "The site selector plugin allows you to manually select the Next.js sites for the entity type."
* description = "The site selector plugin allows you to manually select the
* Next.js sites for the entity type."
* )
*/
class SiteSelector extends ConfigurableSiteResolverBase implements ContainerFactoryPluginInterface {
Expand Down
1 change: 0 additions & 1 deletion modules/next/src/Plugin/RevalidatorBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\Core\Plugin\PluginBase;
use Drupal\next\NextSettingsManagerInterface;
use GuzzleHttp\Client;
use GuzzleHttp\ClientInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;

Expand Down
1 change: 0 additions & 1 deletion modules/next/src/Plugin/RevalidatorInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Drupal\next\Plugin;

use Drupal\Core\Entity\EntityInterface;
use Drupal\next\Event\EntityActionEvent;

/**
Expand Down
1 change: 0 additions & 1 deletion modules/next/src/Plugin/SitePreviewerManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\Plugin\DefaultPluginManager;
use Drupal\next\Annotation\SitePreviewer;
use Drupal\next\Annotation\SiteResolver;

/**
* Plugin manager for site previewers.
Expand Down
4 changes: 2 additions & 2 deletions modules/next/src/Render/MainContent/HtmlRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ protected function prepare(array $main_content, Request $request, RouteMatchInte
return $build;
}

// TODO: Make this configurable?
// @todo Make this configurable?
$entity_type_id = $entity->getEntityTypeId();
$revision_routes = $entity->getEntityType()->isRevisionable() ? [
"entity.$entity_type_id.revision",
"entity.$entity_type_id.latest_version"
"entity.$entity_type_id.latest_version",
] : [];
$routes = array_merge(["entity.$entity_type_id.canonical"], $revision_routes);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ protected function setUp(): void {
'id' => 'blog',
'base_url' => 'https://blog.com',
'preview_url' => 'https://blog.com/api/preview',
'preview_secret' => 'one'
'preview_secret' => 'one',
]);
$this->nextSite->save();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public function testEnvironmentVariables() {
*/
public function testOverriddenEnvironmentVariables() {
$GLOBALS['config']['next.next_site.' . $this->nextSite->id()] = [
'preview_secret' => 'overridden'
'preview_secret' => 'overridden',
];
$overridden_entity = NextSite::load($this->nextSite->id());
$controller = NextSiteEntityController::create($this->container);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected function setUp(): void {
*/
public function testSiteResolver() {
$blog_site = NextSite::create([
'id' => 'blog'
'id' => 'blog',
]);
$blog_site->save();

Expand Down Expand Up @@ -116,7 +116,7 @@ public function testSiteResolver() {
*/
public function testRevalidator() {
$blog_site = NextSite::create([
'id' => 'blog'
'id' => 'blog',
]);
$blog_site->save();

Expand Down
2 changes: 1 addition & 1 deletion modules/next/tests/src/Kernel/Entity/NextSiteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ protected function setUp(): void {
'id' => 'blog',
'base_url' => 'https://blog.com',
'preview_url' => 'https://blog.com/api/preview',
'preview_secret' => 'one'
'preview_secret' => 'one',
]);
$this->nextSite->save();

Expand Down
Loading

0 comments on commit 6fb7718

Please sign in to comment.