[D7] Trustpilot

Your module fails automated review:

http://pareview.sh/pareview/httpgitdrupalorgsandboxxaviemirmon2302739git

the appropriate place for hook_enable is not in the .module file.

you should not use HTML in t() on line 21, and I'm not sure why you are assigning it to a variable in drupal_set_message.

I see that you are using t('') for many of the form element descriptions. This is not necessary. If there is no description, just omit #description.

On line 185 and 199, inline javascript makes for an unreadable mess. It's bad form.

Consider rewriting your html output to be more Drupaly.

Those are my big issues, so far.

I don't really care for the way that you're loading your javascript asynchronously, but since that's the way Asynchronous JavaScript module does it, I guess it's okay.

As far as your output, I'd prefer to see something like this (but that's just me)

$data-tp-settings = array();
$data-tp-settings[] = 'domainId:' . variable_get('trustpilot_domain_id');
...

$attributes = array(
'class' => 'tp_-_category-badge',
'data-tp-settings' => implode(', ', $data-tp-settings),
);

$output = '<div ' . drupal_attributes($attributes) . '>';

  // Anchor tag for trustpilot link.

  $base_url = 'https://www.trustpilot.co.uk/review/';
  $output .= l(t('@site_name Reviews', array('@site_name' => variable_get('site_name')), $base_url . variable_get('trustpilot_domain'));


  // End settings div.
  $output .= '</div>';

Because that's The Drupal Way. But I believe it is largely a matter of style.

Because I cannot test this in a working environment, I don't feel comfortable changing it to RTBC

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.

Have validated this module and it works well for my use case :)

Good work! Thanks!

Hi xaviemirmon,

I have installed the module and tested with a valid trustpilot account. The blocks display and with the expected information.

Some issues I have found:

Line 44 of README.txt missing "> blocks >" in navigation path
Line 45 of README.txt, "Trustpilot reviews" should be "Trustpilot Trustbox 2.0"

The badge widget settings are misleading as changing the size does not alter the dimensions of the badge icon itself. Also, when setting a width smaller than the icon some clipping occurs.
I understand these styles are inherited from the iframe source so I question the need for the badge size settings.

The height settings for the trustbox seem to be overridden by the number of reviews to display. This could be made clear by renaming the title to "Min height" and updating the description.

The 2 javascript files seem to be identical, are they both necessary?

Good luck with your submission,

Louis

Hi xaviermirmon,

Thanks for looking into the suggestions, I have tested the changes and don't have anything further to add. :)

Louis.

Blocking Issues:

1. All variables should be removed in hook_uninstall(), there's a bunch in use in trustpilot_block_configure()
2. trustpilot.min.js seems to exist simply to call another js library, why not just include the library you want in the first place?
3. In _trustpilot_box(), all user-facing text should be passed through the t() function to allow for multilingual sites.

Non-blocking issues:

• You README is good, but the project page is very sparse and could be improved
• Remove .gitignore file from repo
• You can put your configuration url (admin/config/system/trustpilot) in the .info file and it will show up nicely on the module page
• In _trustpilot_box(), your initial switch statement can be simplified to $show_header = variable_get('trustpilot_show_header', 1) ? 'true' : 'false'
• Did you mean to use both url domains https://www.trustpilot.com and https://www.trustpilot.co.uk?

If you can resolve the first 3, this issue is RTBC from me.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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