ODT Importer is a module for Drupal that imports OpenOffice/LibreOffice ODT files directly into drupal nodes (articles, pages...) or any custom entity. It produces HTML code from ODT files and places it to text field specified in configuration.

Configuration:

  1. Create a field type ".odt file importer" in desired node type or custom
    entity.
  2. Choose appropriate text field for "Destination field" to be populated
    with converted HTML code.

Project link

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

Git instructions

git clone --branch '1.0.x' https://git.drupalcode.org/project/odt_importer.git

Comments

dillix created an issue. See original summary.

avpaderno’s picture

Issue summary: View changes
Related issues: +#2501765: [D7] Super TOC

Thank you for applying! Reviewers will review the project files, describing what needs to be changed.

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 smother review.

To reviewers: Please read How to review security advisory coverage applications, What to cover in an application review, and Drupal.org security advisory coverage application workflow.

Since the project is being used for this application, for the time this application is open, only the user who created the application can commit code.

ivnish’s picture

Status: Needs review » Reviewed & tested by the community

I reviewed this module and I have no questions. Also phpcs has no warnings. The code is clean.

avpaderno’s picture

Status: Reviewed & tested by the community » Needs work
  • What follows is a quick review of the project; it doesn't mean to be complete
  • For each point, the review usually shows some lines that should be fixed (except in the case the point is about the full content of a file); it doesn't show all the lines that need to be changed for the same reason
  • A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; the single points aren't ordered, not even by importance

The LICENSE file isn't necessary, since every project hosted on drupal.org is licensed under the same license used by Drupal core. That file is also automatically added from the packaging script.

  if ($phase == 'install' || $phase == 'runtime') {
    if (!extension_loaded('xmlreader')) {
      $requirements['xmlreader'] = [
        'title' => 'XML reader',
        'value' => $t('Not installed'),
        'severity' => REQUIREMENT_ERROR,
        'description' => $t('The XMLReader extension for PHP is missing. Please check the <a href=":url">PHP XMLReader documentation</a> for information on how to correct this.', ['@url' => 'http://php.net/manual/en/book.xmlreader.php']),
      ];
    }

$t hasn't been initialized. In Drupal 8, there isn't any way to initialize it; t() is instead used.

The placeholder for the URL is correct, but the array passed as second argument doesn't contain any :url key.

  /**
   * {@inheritdoc}
   */
  public function __construct($config, $odt_file) {
    // Configuration settings.
    // @todo Add this settings to Drupal configuration GUI.
    $this->features['header'] = (bool) $config['features']['header'] ?? TRUE;
    $this->features['list'] = (bool) $config['features']['list'] ?? TRUE;
    $this->features['table'] = (bool) $config['features']['table'] ?? TRUE;
    $this->features['link'] = (bool) $config['features']['link'] ?? TRUE;
    $this->features['frame'] = (bool) $config['features']['frame'] ?? TRUE;
    $this->features['image'] = (bool) $config['features']['image'] ?? TRUE;
    $this->features['note'] = (bool) $config['features']['note'] ?? TRUE;
    $this->features['annotation'] = (bool) $config['features']['annotation'] ?? TRUE;
    $this->features['toc'] = (bool) $config['features']['toc'] ?? TRUE;
    $this->features['bookmark'] = (bool) $config['features']['bookmark'] ?? TRUE;

    $this->imagesFolder = $config['images_folder'] ?? 'images';

    $this->html = '';
    $this->footnotes = '';

    $odt_file_absolute_path = \Drupal::service('file_system')->realpath($odt_file);

    $this->prepare($odt_file_absolute_path);
  }

  /**
   * {@inheritdoc}
   */
  public function markup() {
    return $this->html . $this->footnotes;
  }

Since the class doesn't have a parent class nor does it implement an interface, the documentation comment cannot contain {@inheritdoc}.

      if (@$xml->xml($xml_string) === FALSE) {
        $this->messenger()->addError('Invalid file contents.');

        return FALSE;
      }

Messages shown with the messenger need to be translatable.

Why isn't OdtConverter used for a service?

dillix’s picture

Status: Needs work » Needs review

Hi @apaderno, thank you for your review! I fixed all items:

  1. Remove LICENSE.txt from git.
  2. Fix translation in odt_importer.install.
  3. Add translation for messenger errors.
  4. Fix phpdoc comments for class methods.

And pushed changes in 1.0.x current branch:
git clone --branch '1.0.x' https://git.drupalcode.org/project/odt_importer.git

Also I made some code cleanup for OdtImporter.php methods. I'm not plan to transform this class into a service now. Maybe later when I implement all todos in my list.

avpaderno’s picture

Priority: Normal » Major
avpaderno’s picture

Priority: Major » Normal
Status: Needs review » Fixed

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the Slack #contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

avpaderno’s picture

Assigned: Unassigned » avpaderno

Status: Fixed » Closed (fixed)

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