Clone: git.drupal.org:project/widencollective.git widencollective

Project's page: https://www.drupal.org/project/widencollective

Synopsis
This module provides integration with Widen’s flagship digital asset management solution, the Widen Collective. Widen offers software solutions that help marketers connect their visual content – like graphics, logos, photos, videos, presentations, and more – for greater visibility and brand consistency.

Features
The Widen Collective Drupal module provides authentication setup through the Widen Collective API, along with the ability to search and insert asset embed codes.

  • Insert image and video embed codes from Widen Collective into your content
  • Integration with the WYSIWYG CKEditor
  • Permission on your user’s access level handled with user authentication through the Widen Collective API

Dependencies

  • CKEditor module. Enable the core module for CKEditor
  • iframedialog javascript library. The Widen Community module uses the external library iframedialog for ckeditor. Download the plugin from http://ckeditor.com/addon/iframedialog. Place the plugin in the root libraries folder (/libraries). Final path for the library should be /libraries/iframedialog/plugin.js

Testing the API

  • Configure one of your text format (/admin/config/content/formats) to use the widen module: select the text format to edit, drag and drop Widen Collective button to one of active toolbar.
  • Enter the following URL on the module's configuration (on /admin/config/content/widen):
    cloud.widencollective.com
  • On your testing user profile, authorization details (on editing your user's profile):
    username: drupalReview
    password: TodayisGr8t!

pareview.sh URL
https://pareview.sh/node/1832

Comments

Luukyb created an issue. See original summary.

luukyb’s picture

luukyb commented
Issue summary: View changes
luukyb’s picture

luukyb commented
luukyb’s picture

luukyb commented
Issue summary: View changes
luukyb’s picture

luukyb commented

On Pareview.sh comments:

\Drupal calls should be avoided in classes, use dependency injection instead

This is to get the configuration variables. From the official documentation (https://www.drupal.org/node/1667896): Grep the entire code base and replace all instances of variable_get() and variable_set() with \Drupal::config()->get() and \Drupal::configFactory()->getEditable->set().

Issues on FILE: ...w/drupal-7-pareview/pareview_temp/js/plugins/iframedialog/plugins.js

This is an external library.
Please advise if we need to use composer to include the library.

Issue on FILE: /var/www/drupal-7-pareview/pareview_temp/src/Form/AdminForm.php

This is on the comments line "{@inheritDoc}". This has been allowed at some point: https://www.drupal.org/node/1962592
Please advise.

Thanks,
Luc

luukyb’s picture

luukyb commented
Issue summary: View changes
PA robot’s picture

PA robot commented

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.

bkelly’s picture

bkelly commented
Status: Needs review » Needs work

The project looks great. I found a couple things for you to check on.

1. In widendam.module line 49 I believe y'all want to use the t() funcion:
'#prefix' => 'Currently authorized with Widen Collective as "' . $widen_username . '"
',

2. In WidendamAuthService.php you have 2 hard-coded strings that I believe should probably either be a configuration setting or constants.
private static $clientRegistration = '167d00b8e36e37dc47d4fb9876192c003f6d8e0a.app.widen.com';
private static $clientHash = 'cc8af29607993c0f9590f4ee0775046a4dfc4eeb';

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

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.

luukyb’s picture

luukyb commented
Assigned: Unassigned » luukyb
Status: Closed (won't fix) » Needs work
luukyb’s picture

luukyb commented
Issue summary: View changes
luukyb’s picture

luukyb commented
Assigned: luukyb » Unassigned
Status: Needs work » Needs review

Hi @bkelly,
Thanks a lot for the feedback.

I pushed the changes you suggested:

  1. Using the function t() on the prefix
  2. clientRegistration and clientHash are now part of the configuration system in the module

Many thanks,
Luc

luukyb’s picture

luukyb commented
Issue summary: View changes
luukyb’s picture

luukyb commented
Issue summary: View changes
dsdeiz’s picture

dsdeiz commented

Hi @Luukyb!

@timhtheos and I tested this and encountered a few issues. They are just minor issues though.

When an account was not authorized in Widen and the Widen button exists in the toolbar and this user uses the text format that has Widen Collective button enabled, we get this error in watchdog:

Notice: Undefined index: widen_token in Drupal\widendam\Controller\WidendamSearchController->getSearchUrl() (line 26 of /var/www/web/modules/custom/widen_collective/src/Controller/WidendamSearchController.php)

Another issue encountered is when entering a URL (that is valid) in the /admin/config/content/widen but is not reachable. The form gets a WSOD. This is the error we get from watchdog:

GuzzleHttp\Exception\ConnectException: cURL error 6: Could not resolve host: cloud.widencollective.co (see http://curl.haxx.se/libcurl/c/libcurl-errors.html) in GuzzleHttp\Handler\CurlFactory...

We are currently providing fixes for these. Other than that, we haven't encountered any issues.

dsdeiz’s picture

dsdeiz commented
Status: Needs review » Needs work
PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

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.

luukyb’s picture

luukyb commented
Status: Closed (won't fix) » Needs work

Hi @dsdeiz,
I confirm the two issues you have identified.

"When an account was not authorized in Widen and the Widen button exists in the toolbar and this user uses the text format that has Widen Collective button enabled"
Thanks for providing the fix. I tested and merged your updates.

"URL (that is valid) in the /admin/config/content/widen but is not reachable. The form gets a WSOD."
I think you are working on fixing this issue too. Let me know if you need anything.

Thanks,
Luc

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

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.

luukyb’s picture

luukyb commented
Assigned: Unassigned » luukyb
Status: Closed (won't fix) » Needs work

The module is called "Widen Community" but the machine name of all files is widendam. Widendam is the old name of the platform / module.
It will be changed from widendam to widencollective,

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

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.

luukyb’s picture

luukyb commented
Status: Closed (won't fix) » Needs work
luukyb’s picture

luukyb commented
Status: Needs work » Needs review

The module is now called Widen Collective and promoted as full project. https://www.drupal.org/project/widencollective
Sandbox link updated.

We could still use this review to be covered by the security advisory policy.

Many thanks,
Luc

luukyb’s picture

luukyb commented
Assigned: luukyb » Unassigned
luukyb’s picture

luukyb commented
Issue summary: View changes
luukyb’s picture

luukyb commented
Issue summary: View changes
PA robot’s picture

PA robot commented
Status: Needs review » Needs work

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

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

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

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.

Ravi Cmsminds’s picture

Ravi Cmsminds commented

Hi,

I checked your module and found some issues and recommendations that you may be interested in :

FILE: ...wsh/pareview_temp/src/Controller/WidencollectiveSearchController.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------
21 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
22 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
--------------------------------------------------------------------------

FILE: ...iewsh/pareview_temp/src/Controller/WidencollectiveAuthController.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
56 | WARNING | User::load calls should be avoided in classes, use
| | dependency injection instead
--------------------------------------------------------------------------

FILE: /root/repos/pareviewsh/pareview_temp/src/Form/AdminForm.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 6 WARNINGS AFFECTING 6 LINES
--------------------------------------------------------------------------
41 | WARNING | t() calls should be avoided in classes, use dependency
| | injection and $this->t() instead
43 | WARNING | t() calls should be avoided in classes, use dependency
| | injection and $this->t() instead
91 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
97 | WARNING | t() calls should be avoided in classes, use dependency
| | injection and $this->t() instead
103 | WARNING | t() calls should be avoided in classes, use dependency
| | injection and $this->t() instead
110 | WARNING | t() calls should be avoided in classes, use dependency
| | injection and $this->t() instead
--------------------------------------------------------------------------

FILE: /root/repos/pareviewsh/pareview_temp/widencollective.routing.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------
13 | WARNING | Open page callback found, please add a comment before the
| | line why there is no access restriction
19 | WARNING | Open page callback found, please add a comment before the
| | line why there is no access restriction
--------------------------------------------------------------------------

FILE: /root/repos/pareviewsh/pareview_temp/widencollective.info.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
7 | WARNING | All dependencies must be prefixed with the project name,
| | for example "drupal:"
--------------------------------------------------------------------------