[8.x-1.x] Butler AI

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 → .

While this application is open, only the user who opened the application can make commits to the project used for the application.


Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review.

It seems you have missed working on the coding standards. You can use the PHPCS tool for checking and resolving issues.

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml butler_ai/

FILE: butler_ai/butler_ai.install
-------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------
 4 | ERROR | [x] The second line in the file doc comment must be "@file"
-------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------


FILE: butler_ai/modules/butler_ai_ckeditor/config/install/butler_ai_ckeditor.settings.yml
-------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------
 2 | ERROR | [x] Expected 1 newline at end of file; 0 found
-------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------


FILE: butler_ai/modules/butler_ai_ckeditor/src/Form/ButlerAiCKEditorSettings.php
----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------
 113 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------------------------


FILE: butler_ai/modules/butler_ai_ckeditor/src/Plugin/CKEditorPlugin/OpenAIPlugin.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------
 53 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-----------------------------------------------------------------------------------------------------------------------------------------


FILE: butler_ai/README.md
------------------------------------------------------------------------
FOUND 1 ERROR AND 2 WARNINGS AFFECTING 3 LINES
------------------------------------------------------------------------
  2 | WARNING | [ ] Line exceeds 80 characters; contains 158 characters
  4 | WARNING | [ ] Line exceeds 80 characters; contains 107 characters
 32 | ERROR   | [x] Expected 1 newline at end of file; 0 found
------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------


FILE: butler_ai/src/ButleraiOpenaiConnection.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------
  98 | WARNING | Unused variable $package.
 106 | WARNING | Unused variable $term.
--------------------------------------------------------------------------------

Hello @pgrandeg
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.

@pgrandeg Drupal core version requirement is not consistent.

butler_ai.info.yml

core_version_requirement: ^9 || ^10

composer.json

  "require": {
    "drupal/core": "^8.8 || ^9"
  }

Hello @pgrandeg
butler_ai/src/Form/ButlerOpenAiConnectionSettings.php

   $form['openai_url'] = [
      '#type' => 'textfield',
      '#title' => $this->t('OpenAI URL'),
      '#description' => $this->t('OpenAI API endpoint.'),
      '#maxlength' => 255,
      '#size' => 100,
      '#default_value' => $config->get('openai_url'),
    ];

    $form['openai_token'] = [
      '#type' => 'textfield',
      '#title' => $this->t('OpenAI Token'),
      '#description' => $this->t('OpenAI API personal token string.'),
      '#maxlength' => 255,
      '#size' => 100,
      '#default_value' => $config->get('openai_token'),
    ];

    $form['butler_ckeditor'] = [
      '#type' => 'fieldset',
      '#title' => $this->t('CKEditor Contextual Autocomplete'),
    ];

    $form['butler_ckeditor']['ckeditor_engine'] = [
      '#type' => 'select',
      '#title' => $this->t('OpenAI CKEditor Engine'),
      '#description' => $this->t('OpenAI favourite engine to be used in API calls. This engine may impact on your API costs, see more information in openai.com.'),
      '#options' => $this->getEnginesList(),
      '#default_value' => $config->get('ckeditor_engine'),
    ];

    $form['butler_ckeditor']['ckeditor_tokens'] = [
      '#type' => 'textfield',
      '#title' => $this->t('Tokens'),
      '#description' => $this->t('OpenAI CKEditor tokens per call.'),
      '#maxlength' => 255,
      '#size' => 100,
      '#default_value' => $config->get('ckeditor_tokens'),
    ];

    $form['butler_ckeditor']['ckeditor_type'] = [
      '#type' => 'textfield',
      '#title' => $this->t('Type'),
      '#description' => $this->t("OpenAI CKEditor call type. Don't change if you are not sure what you are doing."),
      '#maxlength' => 255,
      '#size' => 100,
      '#default_value' => $config->get('ckeditor_type'),
    ];

As a side note, we do not use capital case for all the words in a form element title or description.

Thank you very much in advance for your review and feedback.

Please, find below the commit where I have fixed all the topics you mentioned above:
· Codersniffer
· Remove LICENSE.txt file
· Consistency between composer.json and .info.yml file
· Remove unnecessary capital letters
Git commit: https://git.drupalcode.org/project/butler_ai/-/commit/33c7003e1c9415e296...

@pgrandeg If you changed what has been reported, please change the status to Needs review. In this way, reviewers will know everything has been changed and can be reviewed again.
If you want to take some time to check what you changed, that is fine. Just remember to change status back when ready for the next review.

Thank you @apaderno for the follow up, I forgot to change it actually.

Hello @pgrandeg,

I have reviewed the changes, and they look fine to me.

Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.

Thanks

Hi @pgrandeg

I've done a smoke test of your module and here is what I found:

• Your module works for CkEditor 4 (which is deprecated) and not CkEditor 5. You need to focus on version 5, not 4 because "tomorrow", your module won't be helpful. As for CKEditor 4, it works perfectly!
• The file: modules/butler_ai_ckeditor/butler_ai_ckeditor.module miss hook_help. Also, the hook butler_ai_ckeditor_preprocess_html has a setting that is loaded on any page $variables['#attached']['drupalSettings']['butlerCKEditorEndpoint'] I am not sure if this is ok.
• There is a 5xx error when there is a bad token for open AI You need to add coverage for it and return a readable API message back once there is no connection to the API due to the wrong token/key or whatever.
• You probably need to change the form config field type from string to integer Now, I can insert any string.
• In the controller ButlerAiContextualAutocomplete add the missing protected variable $openaiConnection as DI.
• Same for the form ButlerAiCKEditorSettings, add the missing protected variables $openaiConnection and $entityTypeManager
• Add '-- Any --' in t() function

Regards,
Andrei

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