From 5e7c5608eb8a8f797475eb3f2f58f8f0eb0c40e9 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 2 Feb 2023 15:57:10 -0400 Subject: [PATCH 1/6] Break reusable bit out to trait. --- src/Access/QueryConjunctionTrait.php | 49 ++++++++++++++++++++++++++++ src/Access/QueryTagger.php | 42 +++--------------------- 2 files changed, 53 insertions(+), 38 deletions(-) create mode 100644 src/Access/QueryConjunctionTrait.php diff --git a/src/Access/QueryConjunctionTrait.php b/src/Access/QueryConjunctionTrait.php new file mode 100644 index 0000000..05dc84c --- /dev/null +++ b/src/Access/QueryConjunctionTrait.php @@ -0,0 +1,49 @@ +conditions(); + if ($original_conditions['#conjunction'] === 'AND') { + // Nothing to do... + return $query; + } + + $new_or = $query->orConditionGroup(); + + $original_conditions_copy = $original_conditions; + unset($original_conditions_copy['#conjunction']); + foreach ($original_conditions_copy as $orig_cond) { + $new_or->condition($orig_cond['field'], $orig_cond['value'] ?? NULL, + $orig_cond['operator'] ?? '='); + } + + $new_and = $query->andConditionGroup() + ->condition($new_or); + + $original_conditions = $new_and->conditions(); + + return $query; + } +} diff --git a/src/Access/QueryTagger.php b/src/Access/QueryTagger.php index 748a7fb..1592f1a 100644 --- a/src/Access/QueryTagger.php +++ b/src/Access/QueryTagger.php @@ -13,6 +13,8 @@ */ class QueryTagger { + use QueryConjunctionTrait; + /** * The database connection service. * @@ -94,7 +96,7 @@ public function tagFile(SelectInterface $query) : void { return; } - $this->andifyQuery($query); + static::conjunctionQuery($query); $file_tables = $this->entityTypeManager->getStorage('file') ->getTableMapping() @@ -181,43 +183,7 @@ protected function getTaggedMediaQuery(): SelectInterface { return $this->taggedMediaQuery; } - /** - * Ensure the given query represents an "AND" to which we can attach filters. - * - * Queries can select either "OR" or "AND" as their base conjunction when they - * are created; however, constraining results is much easier with "AND"... so - * let's rework the query object to make it so. - * - * @param \Drupal\Core\Database\Query\SelectInterface $query - * The query to be tagged. - * - * @return \Drupal\Core\Database\Query\SelectInterface - * The query which has been dealt with... should be the same query, just - * returning for (potential) convenience. - */ - protected function andifyQuery(SelectInterface $query): SelectInterface { - $original_conditions =& $query->conditions(); - if ($original_conditions['#conjunction'] === 'AND') { - // Nothing to do... - return $query; - } - - $new_or = $query->orConditionGroup(); - - $original_conditions_copy = $original_conditions; - unset($original_conditions_copy['#conjunction']); - foreach ($original_conditions_copy as $orig_cond) { - $new_or->condition($orig_cond['field'], $orig_cond['value'] ?? NULL, - $orig_cond['operator'] ?? '='); - } - $new_and = $query->andConditionGroup() - ->condition($new_or); - - $original_conditions = $new_and->conditions(); - - return $query; - } /** * Tag media_access queries. @@ -231,7 +197,7 @@ public function tagMedia(SelectInterface $query) : void { return; } - $this->andifyQuery($query); + $this->conjunctionQuery($query); $media_tables = $this->entityTypeManager->getStorage('media') ->getTableMapping() From 6b7761700a2a58a826927d042a2ad836e48e1464 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 2 Feb 2023 16:19:48 -0400 Subject: [PATCH 2/6] Coding standards... ... and on second thought, let's roll a deprecation pass for the method, as there's other stuff in flight that removing it now might complicate. --- src/Access/QueryConjunctionTrait.php | 16 ++++++++++++++++ src/Access/QueryTagger.php | 2 -- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Access/QueryConjunctionTrait.php b/src/Access/QueryConjunctionTrait.php index 05dc84c..6892a4b 100644 --- a/src/Access/QueryConjunctionTrait.php +++ b/src/Access/QueryConjunctionTrait.php @@ -46,4 +46,20 @@ protected static function conjunctionQuery(SelectInterface $query): SelectInterf return $query; } + + /** + * Deprecated method name. + * + * @pbpcs:ignore Drupal.Commenting.DocComment.SpacingBeforeTags - Additional tags cause whatever sniffs to break. + * @phpcs:disable Drupal.Commenting.Deprecated.DeprecatedWrongSeeUrlFormat + * We are not a project on d.o, so there's no applicable URL. + * @deprecated in project:1.2.0 and is removed from project:2.0.0. Deprecated in + * favor of the static and better-named ::conjunctionQuery() method. + * @see \Drupal\islandora_hierarchical_access\Access\QueryConjunctionTrait::conjunctionQuery() + * @phpcs:enable Drupal.Commenting.Deprecated.DeprecatedWrongSeeUrlFormat + */ + protected function andifyQuery(SelectInterface $query) : SelectInterface { + return static::conjunctionQuery($query); + } + } diff --git a/src/Access/QueryTagger.php b/src/Access/QueryTagger.php index 1592f1a..6ca55fe 100644 --- a/src/Access/QueryTagger.php +++ b/src/Access/QueryTagger.php @@ -183,8 +183,6 @@ protected function getTaggedMediaQuery(): SelectInterface { return $this->taggedMediaQuery; } - - /** * Tag media_access queries. * From f8a4b32c4d44aed345d95004420637950b3f5011 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 2 Feb 2023 16:23:11 -0400 Subject: [PATCH 3/6] Swap the call over to target statically. --- src/Access/QueryTagger.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Access/QueryTagger.php b/src/Access/QueryTagger.php index 6ca55fe..81a012b 100644 --- a/src/Access/QueryTagger.php +++ b/src/Access/QueryTagger.php @@ -195,7 +195,7 @@ public function tagMedia(SelectInterface $query) : void { return; } - $this->conjunctionQuery($query); + static::conjunctionQuery($query); $media_tables = $this->entityTypeManager->getStorage('media') ->getTableMapping() From fcca7ab87ae8d96d2e7d18d66501cc9ef279b85f Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 2 Feb 2023 19:23:02 -0400 Subject: [PATCH 4/6] Attempt to switch over to the `pull_request_target`... ... given this needs the Packagist secrets, to run? --- .github/workflows/auto_test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/auto_test.yml b/.github/workflows/auto_test.yml index 06a1e11..2df9431 100644 --- a/.github/workflows/auto_test.yml +++ b/.github/workflows/auto_test.yml @@ -2,7 +2,7 @@ name: PHPUnit on: - pull_request: + pull_request_target: branches: - main From 50ad68f2c7182da555ac8f2c701d69a22f3d5efb Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 2 Feb 2023 19:40:45 -0400 Subject: [PATCH 5/6] Revert "Attempt to switch over to the `pull_request_target`..." This reverts commit fcca7ab87ae8d96d2e7d18d66501cc9ef279b85f. Don't think this is what we actually want here... or not quite all of it, anyway? Seems to get skipped entirely. --- .github/workflows/auto_test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/auto_test.yml b/.github/workflows/auto_test.yml index 2df9431..06a1e11 100644 --- a/.github/workflows/auto_test.yml +++ b/.github/workflows/auto_test.yml @@ -2,7 +2,7 @@ name: PHPUnit on: - pull_request_target: + pull_request: branches: - main From 2ce0a2db21f2a39d616c4860402f748ee0593950 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 2 Feb 2023 19:44:52 -0400 Subject: [PATCH 6/6] Attempt using the hash dealio? Not sure that it will work, since it seems like the sha isn't actually on the branch? Hurray, detached commits? --- .github/workflows/auto_test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/auto_test.yml b/.github/workflows/auto_test.yml index 06a1e11..c758fb1 100644 --- a/.github/workflows/auto_test.yml +++ b/.github/workflows/auto_test.yml @@ -121,7 +121,7 @@ jobs: composer config minimum-stability dev composer require ${{ env.COMPOSER_PACKAGE_PREREQUISITES }} \ - "${{ env.COMPOSER_PACKAGE_NAME }}:dev-${{ github.sha }}" + "${{ env.COMPOSER_PACKAGE_NAME }}:dev-main#${{ github.sha }}" # Install the module. drush --uri=http://localhost --yes -v en ${{ env.DRUPAL_EXTENSION_NAME }}