Currently, sites must be verified manually after any of their variables are set to get the new values.

Let's do this automatically after any variable changes.

This should be as easy as adding a line at the end of hosting_variables_set_site_variables():

hosting_add_task($site->nid, 'verify');

We can then remove the TODO in the README.

CommentFileSizeAuthor
#5 hosting_variables-remove_readme_todo-2844027-5.patch1.03 KBGrimreaper
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

colan created an issue. See original summary.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented

Hello,

I am currently testing commit from #2796619-3: Add support for Drupal 8 configuration

and when I submit the variables form, a verify task is triggered.

In hosting_variables/hosting_variables.module, hosting_variables_site_settings_submit():

if (!empty($form['#site'])) {
    drupal_set_message(t('Variables saved.'));
    hosting_variables_set_site_variables($form['#site']->nid, $variables);

    node_save($form['#site']);
  }

There is a node save on the site node which then triggers in hosting/site/hosting_site.nodeapi.inc:

/**
 * Implements hook_node_update().
 */
function hosting_site_node_update($node) {
  if ($node->type == "site") {
    if ((isset($node->no_verify) && $node->no_verify) == FALSE) {
      hosting_add_task($node->nid, 'verify');
    }
  }
}

It seems that there is already a site verify.

Do you still want to force one in hosting_variables_set_site_variables()?

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
CreditAttribution: colan at Colan Schwartz Consulting commented
Title: Run a verify task after variables are set » Remove README TODO "Run a verify task after variables are set"
Component: Code » Documentation
Category: Feature request » Task

I can confirm this as well, as I discovered yesterday. I just assumed this didn't happen as it was listed as a TODO in the README.

I was thinking it may be good to also have it run during a save for #2796625: Allow remote variable setting via Aegir Services, as we're not saving a node there, but let's have the client initiate a Verify task if he/she wants.

So I can't think of anything else to do here other than update the README.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
Assigned: Unassigned » Grimreaper

Ok.

I will make the patch tomorrow.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
Status: Active » Needs review
FileSize
hosting_variables-remove_readme_todo-2844027-5.patch1.03 KB

Hello,

Here is the patch. It also does a bit of formatting in the README file.

Thanks for the review.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
Assigned: Grimreaper » Unassigned
krina.addweb’s picture

krina.addweb CreditAttribution: krina.addweb at AddWeb Solution Pvt. Ltd. commented
Status: Needs review » Reviewed & tested by the community

Hi Grimreaper, Your patch regarding README changes works for me. Thanks!

  • helmo committed db9aa9b on 7.x-1.x authored by Grimreaper 
    Issue #2844027 by Grimreaper: Remove README TODO "Run a verify task...
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions for Aegir Cooperative commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

krina.addweb’s picture

krina.addweb CreditAttribution: krina.addweb at AddWeb Solution Pvt. Ltd. commented

Hi helmo,

I guess you forgot to provide me RTBC credit, can you please do the needful.

helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions for Aegir Cooperative commented
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions for Aegir Cooperative commented

Status: Fixed » Closed (fixed)

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