[D7] RAMP

ramp.info

files[] = plugins/ramp-mrss-body.tpl.php
files[] = plugins/ramp-mrss-footer.tpl.php
files[] = plugins/ramp-mrss-header.tpl.php
files[] = plugins/ramp.tpl.php

these lines are not need in fact.

ramp.module
LINE:109

'export feed icon' => 'sites/all/modules/views_data_export/images/xml.png',

the icon path should be dynamic.

Image that if we install views_data_export in different directory...

Properly, should like below:

'export feed icon' => drupal_get_path('module','views_data_export') . '/images/xml.png',

also it is suggested that move below function from ramp.module to ramp.views.inc

function ramp_views_plugins_alter(&$plugins)

function ramp_node_presave($node) {
  $node->nid = (int)$node->nid;
  $duplicate = db_query("SELECT * FROM {ramp} WHERE nid = :nid", array(':nid' => $node->nid))->fetchAssoc();
  if (empty($duplicate)) {
      db_insert('ramp')
        ->fields(array(
            'nid' => $node->nid,
            'type' => $node->type,
            'r_index' => $node->index,
      ))->execute();
  }
  else {
    db_update('ramp')
      ->fields(array(
        'r_index' => $node->index,
    ))->condition('nid', $node->nid, '=')
      ->execute();
  }
}

actually, you don't need to write so many code for this condition.
try db_merge, it is more interesting:)

an example here,
http://drupalcode.org/project/recently_read.git/blob/refs/heads/7.x-3.x:...

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and 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.

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

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

RP121, make sure you change the status, so people know it's ready for review.

Any other feedback here?

I want to make sure this is still being reviewed. Any errors that are reported by the automated review tools are associated with views class names not using camel case. To my knowledge this isn't something I can change.

Hi rp121,

Comments

It is an very interesting module, but it is really hard to review without an API key (and they do not look easy to get) making impossible to see how your module works.

On the other hand, I don't understand what exactly your ramp_search sub module does, but it seems that it index some part of your content and also add some search auto-complementation forms. So, I think your module can benefit from search.api module, have you consider it?

Manual Review

You need to include to your project applications the url to your project page (i.e https://drupal.org/sandbox/plinthify/1884568).

On views_handler_filter_term_node_tid.inc, line 90 you call taxonomy_vocabulary_machine_name_load() so, you need to add taxonomy module as a requirement on the corresponding .info file. Or else, you will have a fatal error.

On ramp_search.admin.inc line 19, you have '#default_value' => RAMP_DEFAULT_API_URL, instead of #default_value' => variable_get('ramp_api_url', RAMP_DEFAULT_API_URL),.

Thanks for the feedback jrviorato. I have made the code changes you've requested and committed to latest working copy.

As for using search api... I'm certainly in favor of leveraging any module that can limit functional redundancy, however, when looking into search api I noticed that it says it only works for Drupal entities. One of the pieces to this RAMP module is that it can pull content from other areas outside of your Drupal instance, which is an approach that some companies use. The RAMP service uses various feeds from a handful of sources (sometimes from multiple systems) and aggregates them into search results, among other things.

I'll see if I can grab a demo api key for users to test with and add it to the module description.

I'll look at this now in the Project applications sprint

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

Review of the 7.x-1.x branch:

• The "?>" PHP delimiter at the end of files is discouraged, see http://drupal.org/node/318#phptags
  

  ./xmlSchema.inc
  

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• DrupalPractice has found some issues with your code, but could be false positives. See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. You can ignore the warnigns about Views integration classes nd some undefined variables after extract() in the automated review, but the other stuff ist relevant.
2. Project page is too short, what are you still planning? This should be finished before promoting to a full project, see https://drupal.org/node/997024
3. Remove the 7.x branch, so that it is not confused with 7.x-1.x
4. ramp_rss_schema(): wrong doc block, this is not hook_schema()?
5. xmlSchema.inc: wrong doc block, what is this?
6. ramp_node_presave(): use db_merge() instead?
7. "'#default_value' => variable_get('ramp_api_key', ''),": all variables defined by a module need to be removed in hook_uninstall().
8. ramp_views_data(): contains a lot of example documentation which has nothing to do with your specific code. Please remove and adapt it.
9. ramp.views.inc: @file doc block is wrong. Please fix all your @file doc blocks.
10. ramp_related_request(): if you make an API call yoiu have to use drupal_http_request(), as remote file inclusion will be disabled for security reasons on most Drupal installations.
11. ramp_related_content_contents(): this looks vulnerable to XSS exploits, where is the untrusted third party data sanitized before printing it in the template? Make sure to read https://drupal.org/node/28984 again. And please don't remove the security tag, we keep that for statistics and to show examples of security problems.

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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

added sandbox link