Merge pull request #288 from bolt/fix/setcontent

Yet another setcontent fix
bolt · Jan 24, 2019 · 3949b04 · 3949b04
2 parents 343f485 + b78621e
commit 3949b04
Show file tree

Hide file tree

Showing 12 changed files with 73 additions and 62 deletions.
diff --git a/Makefile b/Makefile
@@ -99,7 +99,6 @@ docker-install:
 	docker-compose exec -T php sh -c "composer install"
 	docker-compose run node sh -c "npm install"
 	docker-compose run node sh -c "npm run build"
-	make docker-db-create
 
 docker-update:
 	docker-compose exec -T php sh -c "composer update"

diff --git a/src/Controller/Backend/ContentEditController.php b/src/Controller/Backend/ContentEditController.php
@@ -10,6 +10,7 @@
 use Bolt\Entity\Content;
 use Bolt\Entity\Field;
 use Bolt\Entity\Taxonomy;
+use Bolt\Enum\Statuses;
 use Bolt\Repository\TaxonomyRepository;
 use Carbon\Carbon;
 use Doctrine\Common\Persistence\ObjectManager;
@@ -106,7 +107,12 @@ private function contentFromPost(?Content $content, Request $request): Content
             $content->setDefinitionFromContentTypesConfig($this->config->get('contenttypes'));
         }
 
-        $content->setStatus(Json::findScalar($post['status']));
+        // @todo dumb status validation, to be replaced with Symfony Form validation
+        $status = Json::findScalar($post['status']);
+        if (in_array($status, Statuses::all(), true)) {
+            $content->setStatus($status);
+        }
+
         $content->setPublishedAt(new Carbon($post['publishedAt']));
         $content->setDepublishedAt(new Carbon($post['depublishedAt']));
 

diff --git a/src/Controller/Frontend/DetailController.php b/src/Controller/Frontend/DetailController.php
@@ -6,6 +6,7 @@
 
 use Bolt\Configuration\Config;
 use Bolt\Controller\BaseController;
+use Bolt\Enum\Statuses;
 use Bolt\Repository\ContentRepository;
 use Bolt\Repository\FieldRepository;
 use Bolt\TemplateChooser;
@@ -49,6 +50,10 @@ public function record(ContentRepository $contentRepository, FieldRepository $fi
             $record = $field->getContent();
         }
 
+        if ($record->getStatus() !== Statuses::PUBLISHED) {
+            throw new NotFoundHttpException('Content is not published');
+        }
+
         $recordSlug = $record->getDefinition()['singular_slug'];
 
         $context = [

diff --git a/src/DataFixtures/ContentFixtures.php b/src/DataFixtures/ContentFixtures.php
@@ -8,6 +8,7 @@
 use Bolt\Entity\Content;
 use Bolt\Entity\Field;
 use Bolt\Entity\User;
+use Bolt\Enum\Statuses;
 use Doctrine\Bundle\FixturesBundle\Fixture;
 use Doctrine\Common\DataFixtures\DependentFixtureInterface;
 use Doctrine\Common\Persistence\ObjectManager;
@@ -58,7 +59,7 @@ private function loadContent(ObjectManager $manager): void
                 $content = new Content();
                 $content->setContentType($contentType['slug']);
                 $content->setAuthor($author);
-                $content->setStatus($this->getRandomStatus());
+                $content->setStatus($i === 1 ? Statuses::PUBLISHED : $this->getRandomStatus());
                 $content->setCreatedAt($this->faker->dateTimeBetween('-1 year'));
                 $content->setModifiedAt($this->faker->dateTimeBetween('-1 year'));
                 $content->setPublishedAt($this->faker->dateTimeBetween('-1 year'));

diff --git a/src/Repository/FieldRepository.php b/src/Repository/FieldRepository.php
@@ -40,24 +40,15 @@ public function findOneBySlug($slug): ?Field
 //     * @return Field[] Returns an array of Field objects
 //     */
     /*
-    public function findByExampleField($value)
+    public function findByExampleField($value): ?Field
     {
         return $this->createQueryBuilder('field')
             ->andWhere('field.exampleField = :val')
             ->setParameter('val', $value)
             ->orderBy('field.id', 'ASC')
             ->setMaxResults(10)
             ->getQuery()
-            ->getResult()
-        ;
-    }
-     */
-    public function findOneBySomeField($value): ?Field
-    {
-        return $this->createQueryBuilder('field')
-            ->andWhere('field.exampleField = :val')
-            ->setParameter('val', $value)
-            ->getQuery()
             ->getOneOrNullResult();
     }
+     */
 }
diff --git a/src/Storage/Query/Handler/SelectQueryHandler.php b/src/Storage/Query/Handler/SelectQueryHandler.php
@@ -20,48 +20,51 @@ class SelectQueryHandler
     public function __invoke(ContentQueryParser $contentQuery)
     {
         $set = new QueryResultset();
-        /** @var SelectQuery $query */
-        $query = $contentQuery->getService('select');
-        $query->setSingleFetchMode(false);
+        /** @var SelectQuery $selectQuery */
+        $selectQuery = $contentQuery->getService('select');
+        $selectQuery->setSingleFetchMode(false);
 
         foreach ($contentQuery->getContentTypes() as $contentType) {
             $contentType = str_replace('-', '_', $contentType);
 
             $repo = $contentQuery->getContentRepository();
             $qb = $repo->getQueryBuilder();
-            $query->setQueryBuilder($qb);
-            $query->setContentType('content');
+            $selectQuery->setQueryBuilder($qb);
+            $selectQuery->setContentType('content');
             // $query->setAlias('content')
 
-            $query->setParameters($contentQuery->getParameters());
-            $contentQuery->runScopes($query);
-            $contentQuery->runDirectives($query);
+            $selectQuery->setParameters($contentQuery->getParameters());
+            $contentQuery->runScopes($selectQuery);
+            $contentQuery->runDirectives($selectQuery);
 
             // This is required. Not entirely sure why.
-            $query->build();
+            $selectQuery->build();
 
             // Bolt4 introduces an extra table for field values, so additional
             // joins are required.
-            $query->doReferenceJoins();
-            $query->doFieldJoins();
+            $selectQuery->doReferenceJoins();
+            $selectQuery->doFieldJoins();
 
-            $query
+            $selectQuery
                 ->getQueryBuilder()
                 ->andWhere('content.contentType = :ct')
                 ->setParameter('ct', $contentType);
 
-            $result = $query
+            $query = $selectQuery
                 ->getQueryBuilder()
-                ->getQuery()
+                ->getQuery();
+
+            $set->setOriginalQuery($contentType, $query);
+
+            $result = $query
                 ->getResult();
 
             if ($result) {
-                $set->setOriginalQuery($contentType, $query);
                 $set->add($result, $contentType);
             }
         }
 
-        if ($query->getSingleFetchMode()) {
+        if ($selectQuery->getSingleFetchMode()) {
             if ($set->count() === 0) {
                 return null;
             }

diff --git a/src/Storage/Query/Query.php b/src/Storage/Query/Query.php
@@ -70,14 +70,11 @@ public function getContentByScope(string $scopeName, string $textQuery, array $p
     /**
      * Helper to be called from Twig that is passed via a TwigRecordsView rather than the raw records.
      *
-     * @param string $textQuery       The base part like `pages` or `pages/1`
-     * @param array  $parameters      Parameters like `printquery` and `paging`
-     * @param array  $whereParameters Actual `where` parameters taken from `... where { foo: bar } ...`
+     * @param string $textQuery  The base part like `pages` or `pages/1`
+     * @param array  $parameters Parameters like `printquery` and `paging`, but also `where` parameters taken from `... where { foo: bar } ...`
      */
-    public function getContentForTwig(string $textQuery, array $parameters = [], array $whereParameters = [])
+    public function getContentForTwig(string $textQuery, array $parameters = [])
     {
-        $parameters = array_merge($parameters, $whereParameters);
-
         return $this->getContentByScope('frontend', $textQuery, $parameters);
     }
 }
diff --git a/src/Storage/Query/QueryParameterParser.php b/src/Storage/Query/QueryParameterParser.php
@@ -126,9 +126,10 @@ public function getFilter(string $key, $value = null): ?Filter
      */
     public function incorrectQueryHandler(string $key, $value, Expr $expr)
     {
-        if (! is_string($value)) {
+        if (is_string($value) === false) {
             return null;
         }
+
         if (mb_strpos($value, '&&') && mb_strpos($value, '||')) {
             throw new \Exception('Mixed && and || operators are not supported', 1);
         }
@@ -141,6 +142,10 @@ public function incorrectQueryHandler(string $key, $value, Expr $expr)
      */
     public function multipleKeyAndValueHandler(string $key, $value, Expr $expr): ?Filter
     {
+        if (is_string($value) === false) {
+            return null;
+        }
+
         if (! mb_strpos($key, '|||')) {
             return null;
         }
@@ -161,7 +166,7 @@ public function multipleKeyAndValueHandler(string $key, $value, Expr $expr): ?Fi
                 $filterParams += $multipleValue->getParameters();
             } else {
                 $val = $this->parseValue($val);
-                // todo: check what type of field $key is
+                // @todo check what type of field $key is
                 $placeholder = $key . '_' . $count;
                 $filterParams[$placeholder] = $val['value'];
                 $exprMethod = $val['operator'];
@@ -190,9 +195,10 @@ public function multipleKeyAndValueHandler(string $key, $value, Expr $expr): ?Fi
      */
     public function multipleValueHandler(string $key, $value, Expr $expr): ?Filter
     {
-        if (! is_string($value)) {
+        if (is_string($value) === false) {
             return null;
         }
+
         if (mb_strpos($value, '&&') === false && mb_strpos($value, '||') === false) {
             return null;
         }
@@ -232,7 +238,7 @@ public function multipleValueHandler(string $key, $value, Expr $expr): ?Filter
     }
 
     /**
-     * The default handler is the last to be run and handler simple value parsing.
+     * The default handler is the last to be run and handles simple value parsing.
      *
      * @param string|array $value
      */
@@ -247,7 +253,7 @@ public function defaultFilterHandler(string $key, $value, Expr $expr): Filter
             $composite = $expr->andX();
 
             foreach ($value as $paramName => $valueItem) {
-                $val = $this->parseValue($valueItem);
+                $val = $this->parseValue((string) $valueItem);
                 $placeholder = sprintf('%s_%s_%s', $key, $paramName, $count);
                 $exprMethod = $val['operator'];
                 $composite->add($expr->{$exprMethod}($this->alias . $key, ':' . $placeholder));
@@ -260,7 +266,7 @@ public function defaultFilterHandler(string $key, $value, Expr $expr): Filter
             return $filter;
         }
 
-        $val = $this->parseValue($value);
+        $val = $this->parseValue((string) $value);
         $placeholder = $key . '_1';
         $exprMethod = $val['operator'];
 
@@ -279,17 +285,13 @@ public function defaultFilterHandler(string $key, $value, Expr $expr): Filter
      *     'operator' => <the operator that should be used>
      *     'matched'  => <the pattern that the value matched>
      * ]
-     *
-     * @param mixed $value Value to process
-     *
-     * @return array Parsed values
      */
-    public function parseValue($value): array
+    public function parseValue(string $value): array
     {
         foreach ($this->valueMatchers as $matcher) {
             $regex = sprintf('/%s/u', $matcher['token']);
             $values = $matcher['params'];
-            if (preg_match($regex, (string) $value)) {
+            if (preg_match($regex, $value)) {
                 if (is_callable($values['value'])) {
                     preg_match($regex, $value, $output);
                     $values['value'] = $values['value']($output[1]);

diff --git a/src/Storage/Query/QueryResultset.php b/src/Storage/Query/QueryResultset.php
@@ -7,7 +7,7 @@
 use AppendIterator;
 use ArrayIterator;
 use Countable;
-use Doctrine\ORM\QueryBuilder;
+use Doctrine\ORM\Query;
 
 /**
  * This class is a wrapper that handles single or multiple
@@ -20,7 +20,7 @@ class QueryResultset extends AppendIterator implements Countable
     /** @var array */
     protected $results = [];
 
-    /** @var QueryBuilder[] */
+    /** @var Query[] */
     protected $originalQueries = [];
 
     /**
@@ -62,20 +62,17 @@ public function get($label = null): array
         return $results;
     }
 
-    /**
-     * Returns the total count.
-     */
     public function count(): int
     {
         return count($this->get());
     }
 
-    public function setOriginalQuery($type, $originalQuery): void
+    public function setOriginalQuery(string $type, Query $originalQuery): void
     {
         $this->originalQueries[$type] = $originalQuery;
     }
 
-    public function getOriginalQuery($type = null): QueryBuilder
+    public function getOriginalQuery($type = null): ?Query
     {
         if ($type !== null) {
             return $this->originalQueries[$type];
@@ -85,7 +82,7 @@ public function getOriginalQuery($type = null): QueryBuilder
     }
 
     /**
-     * @return QueryBuilder[]
+     * @return Query[]
      */
     public function getOriginalQueries(): array
     {

diff --git a/src/Twig/Node/SetcontentNode.php b/src/Twig/Node/SetcontentNode.php
@@ -52,15 +52,20 @@ public function compile(Compiler $compiler): void
             ->raw("'] = ")
             ->raw("\$this->env->getExtension('" . SetcontentExtension::class . "')->getQueryEngine()->getContentForTwig(")
             ->subcompile($this->getAttribute('contentType'))
-            ->raw(', ')
-            ->subcompile($arguments);
+            ->raw(', ');
 
         if ($this->hasNode('wherearguments')) {
             $compiler
+                ->raw('array_merge(')
+                ->subcompile($arguments)
                 ->raw(', ')
-                ->subcompile($this->getNode('wherearguments'));
+                ->subcompile($this->getNode('wherearguments'))
+                ->raw(')');
+        } else {
+            $compiler
+                ->subcompile($arguments);
         }
 
-        $compiler->raw(" );\n");
+        $compiler->raw(");\n");
     }
 }
diff --git a/tests/e2e/features/edit_record.feature b/tests/e2e/features/edit_record.feature
@@ -1,15 +1,18 @@
 Feature: Edit record
 
+    @wip
     Scenario: As an Admin I want to change title of a record
         Given I am logged in as "admin"
 
         When I visit the "edit_record" page with parameters:
-            | id | 2 |
+            | id | 5 |
         Then I wait for "title_field" element to appear
 
         When I fill the "title_field" field with "Changed title"
+        And I click the "status_select" element
+        And I click the "status_published" element
         And I click the "save_button" element
 
         When I visit the "single_record" page with parameters:
-            | id | 2 |
+            | id | 5 |
         Then there is element "title" with text "Changed title"
diff --git a/tests/e2e/pages/edit_record.js b/tests/e2e/pages/edit_record.js
@@ -7,6 +7,8 @@ class EditRecordPage extends BasePage {
     this.url = '/bolt/edit/:id';
 
     this.title_field = $('#field-title');
+    this.status_select = $('#metadata > form > div:nth-child(2) > div > div:nth-child(1) > div > div > div.multiselect__select');
+    this.status_published = $('#metadata > form > div:nth-child(2) > div > div:nth-child(1) > div > div > div.multiselect__content-wrapper > ul > li:nth-child(1) > span');
     this.save_button = $('button[type="submit"]');
   }
 }