This module integrates the HIVO Digital Asset Management (DAM) platform directly into Drupal. It allows editors to browse, select, and import digital assets such as images, videos, and documents from HIVO into Drupal’s Media Library

Project link

https://www.drupal.org/project/hivo_connector

Comments

dannie_hivo created an issue. See original summary.

vishal.kadam’s picture

Issue summary: View changes
rushikesh raval’s picture

Thank you for applying!

Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

The important notes are the following.

  • If you have not done it yet, you should run phpcs --standard=Drupal,DrupalPractice on the project, which alone fixes most of what reviewers would report.
  • For the time this application is open, only your commits are allowed.
  • The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status won't be changed by this application and no other user will be able to opt projects into security advisory policy.
  • We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.

To the reviewers

Please read How to review security advisory coverage applications, Application workflow, What to cover in an application review, and Tools to use for reviews.

The important notes are the following.

  • It is preferable to wait for a Code Review Administrator before commenting on newly created applications. Code Review Administrators will do some preliminary checks that are necessary before any change on the project files is suggested.
  • Reviewers should show the output of a CLI tool only once per application.
  • It may be best to have the applicant fix things before further review.

For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues.

vishal.kadam’s picture

Status: Needs review » Needs work

1. FILE: composer.json

As a side note, it is not necessary to add the Drupal core requirements in the /composer.json/ file: The Drupal.org Composer Façade will add them.

2. FILE: hivo_connector.info.yml

package: Custom

This line is used by custom modules created for specific sites. It is not a package name used for projects hosted on drupal.org.

core_version_requirement: ^9 || ^10 || ^11

A new project should not declare itself compatible with a Drupal release that is no longer supported. No site should be using Drupal 8 nor Drupal 9, and people should not be encouraged to use those Drupal releases.

dannie_hivo’s picture

Status: Needs work » Needs review

Hello Vishal,

Sorry for the late reply. We just got back from the holiday.

We’ve fixed all the issues. Could you please take a look?

vishal.kadam’s picture

Status: Needs review » Needs work

1. FILE: src/Commands/HivoCleanupCommand.php

$this->logger()->error(" ✗ {$error_msg}");

$this->logger()->error(" Errors/Warnings: " . count($errors));

The $message parameter passed to the LoggerInterface methods must be a literal string that uses placeholders. It is not a translatable string returned from t()/$this->t(), a string concatenation, a value returned from a function/method, nor a variable containing an exception object.

2. FILE: src/Controller/HivoMediaApiController.php

  /**
   * The media storage service.
   *
   * @var \Drupal\media\MediaStorage
   */
  protected $mediaStorage;

  /**
   * The database connection service.
   *
   * @var \Drupal\Core\Database\Connection
   */
  protected $database;

  /**
   * The file URL generator.
   *
   * @var \Drupal\Core\File\FileUrlGeneratorInterface
   */
  protected $fileUrlGenerator;

  /**
   * The logger channel factory.
   *
   * @var \Drupal\Core\Logger\LoggerChannelFactoryInterface
   */
  protected $loggerFactory;

  /**
   * Constructs a new HivoMediaApiController.
   *
   * @param \Drupal\media\MediaStorage $media_storage
   *   The media storage service.
   * @param \Drupal\Core\Database\Connection $database
   *   The database connection service.
   * @param \Drupal\Core\File\FileUrlGeneratorInterface $file_url_generator
   *   The file URL generator.
   * @param \Drupal\Core\Logger\LoggerChannelFactoryInterface $logger_factory
   *   The logger channel factory.
   */
  public function __construct(
    MediaStorage $media_storage,
    Connection $database,
    FileUrlGeneratorInterface $file_url_generator,
    LoggerChannelFactoryInterface $logger_factory,
  ) {
    $this->mediaStorage = $media_storage;
    $this->database = $database;
    $this->fileUrlGenerator = $file_url_generator;
    $this->loggerFactory = $logger_factory;
  }

FILE: src/Controller/HivoSyncApiController.php

  /**
   * The media storage service.
   *
   * @var \Drupal\media\MediaStorage
   */
  protected $mediaStorage;

  /**
   * The file system service.
   *
   * @var \Drupal\Core\File\FileSystemInterface
   */
  protected $fileSystem;

  /**
   * Constructs a new HivoSyncApiController.
   *
   * @param \Drupal\media\MediaStorage $media_storage
   *   The media storage service.
   * @param \Drupal\Core\File\FileSystemInterface $file_system
   *   The file system service.
   */
  public function __construct(MediaStorage $media_storage, FileSystemInterface $file_system) {
    $this->mediaStorage = $media_storage;
    $this->fileSystem = $file_system;
  }

FILE: src/Controller/HivoUploadApiController.php

  /**
   * The file system service.
   *
   * @var \Drupal\Core\File\FileSystemInterface
   */
  protected $fileSystem;

  /**
   * The file repository service.
   *
   * @var \Drupal\file\FileRepository
   */
  protected $fileRepository;

  /**
   * The media storage service.
   *
   * @var \Drupal\media\MediaStorage
   */
  protected $mediaStorage;

  /**
   * The entity type manager.
   *
   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
   */
  protected $entityTypeManager;

  /**
   * The current user.
   *
   * @var \Drupal\Core\Session\AccountProxyInterface
   */
  protected $currentUser;

  /**
   * Constructs a new HivoUploadApiController.
   *
   * @param \Drupal\Core\File\FileSystemInterface $file_system
   *   The file system service.
   * @param \Drupal\file\FileRepository $file_repository
   *   The file repository service.
   * @param \Drupal\media\MediaStorage $media_storage
   *   The media storage service.
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager.
   * @param \Drupal\Core\Session\AccountProxyInterface $current_user
   *   The current user.
   */
  public function __construct(
    FileSystemInterface $file_system,
    FileRepository $file_repository,
    MediaStorage $media_storage,
    EntityTypeManagerInterface $entity_type_manager,
    AccountProxyInterface $current_user,
  ) {
    $this->fileSystem = $file_system;
    $this->fileRepository = $file_repository;
    $this->mediaStorage = $media_storage;
    $this->entityTypeManager = $entity_type_manager;
    $this->currentUser = $current_user;
  }

New modules, which are compatible with Drupal 10 and higher versions are expected to include type declarations in property definitions, and use constructor property promotion.

dannie_hivo’s picture

Status: Needs work » Needs review

Hello

We fixed all issues, please check it.

Thank you

vishal.kadam’s picture

Rest seems fine to me.

Please wait for other reviewers and Project Moderator to take a look and if everything goes fine, you will get the role.

bbu23’s picture

Assigned: Unassigned » bbu23
bbu23’s picture

Assigned: bbu23 » Unassigned
Status: Needs review » Needs work

Hello,

Below you have my feedback:
- There is no dev release for your project. While not mandatory, it's a highly encouraged practice in the Drupal community. You should consider creating one.
- The composer.json file needs to be reviewed as it contains sections that don't belong there. See Add a composer.json file for more info.
- The Drush command definition and the PHP plugin files in src/Plugin folder should use PHP Attributes instead of Annotations for plugin definitions as described in Attribute-based plugins.
- MetadataWidget.php: Line 96: It is incorrect to ignore the PHPCS warning about unused variable. The correct approach is to delete the actual unused variable with the funny name $_.
- There are still unsolved PHPCS warnings/errors
- The Helpers directory relies on static classes, which bypasses Drupal's dependency injection container. Classes like HivoApiClient, S3ChunkedUploadHelper etc. should be registered as services in hivo_connector.services.yml and injected where needed — this makes them testable, swappable, and consistent with Drupal's architecture. See https://www.drupal.org/docs/drupal-apis/services-and-dependency-injectio...

...and that's all I have time for now.

dannie_hivo’s picture

Status: Needs work » Needs review

Hello

Thank you for your feedbacks.

I updated the repo.

Please check

dannie_hivo’s picture

Hello
Anyone can check this please?

dannie_hivo’s picture

Hello
Anyone can check this please?

vishal.kadam’s picture

Priority: Normal » Major

I am changing priority as per Issue priorities.