[D8] MotionPoint Translation Provider

We only review a single branch for a single project. Please choose the branch you want to be reviewed and edit the issue summary accordingly.

Is the account used to post this application a shared account?

Anyway, there aren't commits from the account used to create this application; we cannot accept an application that uses a project for which the user who created the application hasn't done any commits.

See comment #3: A user who hasn't committed code in a project cannot apply using that project. These applications are for users, not for projects.

That's just one commit. There is a user who made more commits.

I also asked: Is the account used to post this application a shared account?

There is no need to create a new project: sugumarant can commit code in the existing project and follow this application.

As side note, shared accounts cannot be used to:

• Commit code to Git repositories
• Create any node except for organization, case study or project nodes
• Comment on nodes

• What follows is a quick review of the project; it doesn't mean to be complete
• For every point, the review doesn't make a complete list of lines that should be fixed, but an example of what is wrong in the code
• A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; if a point isn't about that, it makes it clear

8.0-x and 9.0-x are wrong tag names. The correct ones would be 8.0.x and 9.0.x. New tags needs to be created, the default one changed, and the old tags deleted.

Since the project machine name is motionpointtranslation, the following filenames are wrong.

• tmgmt_motionpoint.info.yml
• tmgmt_motionpoint.module
• tmgmt_motionpoint.routing.yml

There are other files/directory with the wrong name, including src/Plugin/tmgmt/Translator. The same is true for the paths used in the route definitions.

The .module file isn't necessary, if it doesn't contain code.

    /** @var \Drupal\tmgmt_file\Format\FormatInterface $xliff_converter */
    $xliff_converter = \Drupal::service('plugin.manager.tmgmt_file.format')->createInstance('xlf');
    $xliff_content = $xliff_converter->export($job);

A class that implements create() from which it receives the dependency container object doesn't use \Drupal to access a service or other dependencies.

      if (is_bool($received_translation) === true) { throw new Exception(); };

The code isn't following the Drupal coding standards.

 $api_base_url = UrlHelper::isValid($translator->getSetting('motionpoint_api_url')) ? $translator->getSetting('motionpoint_api_url') : 'https://api.motionpoint.com';

UrlHelper::isValid() returns TRUE or FALSE; that code is setting a URL to a Boolean value (which is surely not what it should do).

@MotionPoint It's not necessary to delete the project. The developer can continue with this application and change the project code as reported here.

Shared accounts can be used to create project. They cannot be used to commit code, but a single commit isn't going to be a big issue.

Also, I think projects cannot be deleted.

Please remember that only sugumarant can make commits for the duration of this application.

    return AvailableResult::no(t('@translator is not available. Please make sure it is properly <a href=:configured>configured</a>.', [

:configured needs to be surrounded by double quotes, as described in FormattableMarkup::placeholderFormat().

:variable: Return value is escaped with \Drupal\Component\Utility\Html::escape() and filtered for dangerous protocols using UrlHelper::stripDangerousProtocols(). Use this when using the "href" attribute, ensuring the attribute value is always wrapped in quotes

// Secure (with quotes):
$this->placeholderFormat('<a href=":url">@variable</a>', [
  ':url' => $url,
  '@variable' => $variable,
]);

// Insecure (without quotes):
$this->placeholderFormat('<a href=:url>@variable</a>', [
  ':url' => $url,
  '@variable' => $variable,
]);

The code doesn't follow the Drupal coding standards in some points, as:

• PHP constants must be written all capital case (including TRUE, FALSE, NULL)
• else and elseif branches must be written on a new line
• The indentation is expected to be two spaces
• There should not be empty lines between the line containing function and the first function/method code line

\Drupal::logger('tmgmt_motionpoint')->warning('API Request Exception, status code is: ' . print_r();

Instead of concatenating string, the message should use placeholders.

    catch (Exception $e) {
      if (method_exists($e, 'getCode')) {
        \Drupal::logger('tmgmt_motionpoint')->warning('API Request Exception, status code is: ' . print_r() . 'omitted string' );
      }
      return $e;
    }

Why is the code returning an exception instead of re-throwing it? in PHP, exceptions aren't returned.

    } catch (Exception $e) {
      \Drupal::logger('tmgmt_motionpoint')->warning('Translation cannot be retrieved. If situation persists please contact support@motionpoint.com.');
    }

Classes that implement ContainerFactoryPluginInterface don't use \Drupal, but inject their dependencies in create().

  private $submitSettings = [
    0 => 'Single Job - all pages are combined into one MotionPoint job',
    1 => 'Multiple Jobs - an individual MotionPoint job is created for each page',
  ];

Strings shown in the user interface must be translatable.

    $out = [];
    foreach ($this->submitSettings as $value) {
        $out[] = t($value);
    }

The first parameter of t() must be a literal string, not a variable. Passing a variable is considered a potential security issue.

  requirements:
    _access: 'TRUE'

The README.txt isn't complete and it's not formatted as the Drupal coding standards say.

Fixed all the review comments. Please review and let me know your feedback.

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

These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find more contributors chatting on the IRC #drupal-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.

Thank you apaderno for you time and valuable feedback