This is a follow-up of #2305919: Return 404 when trying to edit a non-existent feed..

A 404 should be returned for /import/%/log where "%" points to a non-existent importer. For this a Views argument validator plugin must be written. This plugin should extend "views_plugin_argument_validate".

CommentFileSizeAuthor
#2 feeds-views-validator-2333009-2.patch5.19 KBMegaChriz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MegaChriz’s picture

Issue summary: View changes

Updated issue summary.

MegaChriz’s picture

Status: Active » Needs review
Related issues: +#1393898: Order Log view by flid instead of log_time
FileSize
5.19 KB

It appears to be that the argument in the path "/import/%/log" is already validated. This happens in views/feeds_views_handler_argument_importer_id.inc. It is that only the wrong action is taken when the argument is not valid. It should be "Show page not found" and not "Display contents of no result found".

A validator for "/node/%/log" indeed is needed. Now a "Log" tab shows up for every node, even when that node is not a feed node.

The attached patch does the following:

  • It adds a Views validator for "feed_nid". This expects a node id and will validate if there is an importer attached to the node's type.
  • For "/import/%/log", the action to take when the argument validation fails is set to "Show page not found".
  • For "/node/%/log", the validator is set to "feed_nid" and the action to take when the argument validation fails is set to "Show page not found".
  • It adds translations for the View (automatically exported by Views).

Side node: the Log view could be improved more. See #1393898: Order Log view by flid instead of log_time.

  • twistor committed 83d7c2e on 7.x-2.x authored by MegaChriz
    Issue #2333009 by MegaChriz: Add importer validator for Views
    
twistor’s picture

Status: Needs review » Fixed

Awesome!

We could probably add some configuration options for importer type to the feed node validator at some point, but this looks great.

twistor’s picture

@MegaChriz, I've uploaded a patch for #1393898: Order Log view by flid instead of log_time. Simple enough. How did you get the translatables export?

MegaChriz’s picture

Awesome work on Feeds, twistor!

How did you get the translatables export?

Because I see these translatables always when I export a View, I thought they were added by default. Further exploration learns me that they are only exported when the core module "Locale" is enabled (or when Views' localization plugin gets overridden, see below), which I have enabled by default because I usually install my sites in Dutch.

From views/includes/view.inc, method export(), ± line 1917:
Views uses its localization plugin to export translatables.

// Give the localization system a chance to export translatables to code.
if ($this->init_localization()) {
  $this->export_locale_strings('export');
  $translatables = $this->localization_plugin->export_render($indent);
  if (!empty($translatables)) {
    $output .= $translatables;
  }
}

From views/views.module, ± line 1351:
The localization plugin can be overridden.

/**
 * Load the current enabled localization plugin.
 *
 * @return The name of the localization plugin.
 */
function views_get_localization_plugin() {
  $plugin = variable_get('views_localization_plugin', '');
  // Provide sane default values for the localization plugin.
  if (empty($plugin)) {
    if (module_exists('locale')) {
      $plugin = 'core';
    }
    else {
      $plugin = 'none';
    }
  }

  return $plugin;
}

Status: Fixed » Closed (fixed)

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