[D7] Entityqueue Scheduler

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxjojonaloha2142361git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

In the module file as well as in the install file you have db_selects.
Instead of

  $query = db_select('entityqueue_subqueue', 'sq')
    ->fields('sq', array('name', 'queue'))
    ->condition('sq.module', 'entityqueue_scheduler');
  $query->addExpression($now_sql, 'is_past');
  $query->havingCondition('is_past', 1);
  $query->orderBy('sq.scheduler_date', 'ASC');
  $subqueues = $query->execute();

I don't see the reason to have further separate query-> statements as there are no dynamic parts in the query.
So instead you could just use

  $query = db_select('entityqueue_subqueue', 'sq')
    ->fields('sq', array('name', 'queue'))
    ->condition('sq.module', 'entityqueue_scheduler')
    ->addExpression($now_sql, 'is_past');
    ->havingCondition('is_past', 1)
    ->orderBy('sq.scheduler_date', 'ASC');
  $subqueues = $query->execute();

Also i'm not sure if the direct dependency to date_api is correct. date_api is part of the date module so i
would rather depend on both as it's not obvious for installing users that aren't familiar with the date module,
that date_api is part of the date module.

Pareview still gives some issues: http://pareview.sh/pareview/httpgitdrupalorgsandboxjojonaloha2142361git

function entityqueue_scheduler_schema_alter(&$schema) {
  if (isset($schema['entityqueue_subqueue'])) {
    $schema['entityqueue_subqueue']['fields']['scheduler_date'] = array(
      'description' => 'The date this subqueue is scheduled to be published.',
      'type' => 'varchar',
      'length' => 32,
      'not null' => FALSE,
      'default' => NULL,
    );
  }
}

This should use $t = get_t(); for the description. Have to use $t because this is used on install.
$t('description' => 'The date this subqueue is scheduled to be published.'),

function entityqueue_scheduler_install() {
  $spec = array(
    'description' => 'The date this subqueue is scheduled to be published.',
    'type' => 'varchar',
    'length' => 32,
    'not null' => FALSE,
    'default' => NULL,
    'initial' => NULL,
  );
  db_add_field('entityqueue_subqueue', 'scheduler_date', $spec);
  db_add_index('entityqueue_subqueue', 'scheduler_date', array('scheduler_date'));
}

This repeats code from the hook_schema_alter(). Better to do as:

function entityqueue_scheduler_install() {
  $schema['entityqueue_subqueue'] = array();
  entityqueue_scheduler_schema_alter($schema);
  db_add_field('entityqueue_subqueue', 'scheduler_date', $schema);
  db_add_index('entityqueue_subqueue', 'scheduler_date', array('scheduler_date'));
}

function entityqueue_scheduler_update_7000()
This repeats code from the hook_install. Either wrap the call for add field and index in a conditional db_index_exists and db_field_exists or don't repeat the logic.

Depending on how many queues there are to update, you might try using $sandbox for the entityqueue_scheduler_update_7000. See hook_update_N()

The .info file should include a version of 7.x-1.x

@heddn: descriptions in hook_schema must not run through t() since that only creates overhead for translators.

@klausi, good to know and it even makes sense.

While trying to grasp what entityqueue and entityqueue_scheduler actually do ^[1], I got this error on the page admin/structure/entityqueue/list/<QUEUE>/subqueues/2/edit:

Recoverable fatal error: Object of class DateTimeZone could not be converted to string in date_now() (line 1753 of /usr/local/share/drupal/7/modules/date/date_api/date_api.module).

The queue handler is of type Scheduled queue. It doesn't fail for the Multiple subqueue handler so I suspect a critical bug in this module.

I miss a note/paragraph about similar projects and what this solves compared to others. E.g. Scheduler. Is it a solution for publishing content at scheduled times in entityqueues only? As this module is rather specific, a more detailed explanation on the project page would help, maybe even with example usage.

Overall, the code quality seems good.

Notes

1. Funny thing, your blog post about Entityqueue vs Draggable Views gave me a better explanation and example usage than the one on entityqueue's project page.

It looks good to me.

No issues found, this looks good to me! It wasn't quite clear to me why someone would install this module, perhaps an example use case would be helpful.

Thanks for your contribution, jojonaloha!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.