Problem/Motivation

Currently the plugin is not consulted with regards to whether a build should occur or not.

Proposed resolution

In \Drupal\build_hooks\Trigger::triggerBuildHookForEnvironment add the current deployment to the value object
In \Drupal\build_hooks\Trigger::triggerBuildHook ask the plugin if the trigger should fire.
Whilst there also dispatch an event so other modules can intervene too.
Add a 'should fire' property to the BuildHookDetails object that allows modules/plugins to cancel sending.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Issue summary: View changes
larowlan’s picture

Status: Active » Needs review
StatusFileSize
new31.36 KB
larowlan’s picture

StatusFileSize
new426 bytes
new31.38 KB
larowlan’s picture

+++ b/src/Plugin/FrontendEnvironmentInterface.php
@@ -45,4 +45,12 @@ interface FrontendEnvironmentInterface extends ConfigurableInterface, DependentP
+  public function shouldDeploymentTrigger(BuildTrigger $trigger) : void;

Renaming this method to preDeploymentTrigger because at this point you can still modify the build hook details but have more context

larowlan’s picture

StatusFileSize
new2.52 KB
new31.37 KB
larowlan’s picture

+++ b/src/BuildHookDetails.php
@@ -131,4 +131,27 @@ class BuildHookDetails {
+  /**
+   * Sets deployment.
+   *
+   * @param \Drupal\build_hooks\Entity\DeploymentInterface $deployment
+   *   Deployment.
+   *
+   * @return $this
+   */
+  public function setDeployment(DeploymentInterface $deployment) : BuildHookDetails {
+    $this->deployment = $deployment;
+    return $this;
+  }
+
+  /**
+   * Gets value of Deployment.
+   *
+   * @return \Drupal\build_hooks\Entity\DeploymentInterface
+   *   Value of Deployment.
+   */
+  public function getDeployment(): DeploymentInterface {
+    return $this->deployment;
+  }

In retrospect I think this should go on the BuildTrigger class, not here

larowlan’s picture

StatusFileSize
new4.56 KB
new30.99 KB

Status: Needs review » Needs work

The last submitted patch, 8: 3205552-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

larowlan’s picture

Status: Needs work » Needs review
StatusFileSize
new2.93 KB
new31.66 KB
acbramley’s picture

Status: Needs review » Reviewed & tested by the community

Looking great, love the idea of the event and the test coverage is looking good too 💪

larowlan’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 12: 3205552-12.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

larowlan’s picture

Status: Needs work » Reviewed & tested by the community
StatusFileSize
new445 bytes
new31.83 KB

Missed the new test method signature in re-roll

  • larowlan committed a3bebf2 on 3.x
    Issue #3205552 by larowlan, acbramley: Allow plugins to determine if a...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

This will go out as 3.1.0

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.