[D8] Inline Image Attach

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

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.

Edit : the pareview.sh errors reported are not fixed

Testing

- Configuration : you should provide a link to the module’s configuration in the .info.yml
- All the content type are not listed in the configuration form (The basic page has a body field and a image field but is not listed in the configuration form)
- On a fresh drupal 8.0.3 install, I’ve got this error when configuring the content type article

Warning: Missing argument 1 for Drupal\Core\Form\FormState::getValue(), called in /srv/www/drupal8/modules/inline_image_attach/src/Form/InlineImageAttachForm.php on line 84 and defined in Drupal\Core\Form\FormState->getValue() (line988 of core/lib/Drupal/Core/Form/FormState.php).

When saving an article, after include an inline image

Warning: Missing argument 2 for inline_image_attach_entity_presave() in inline_image_attach_entity_presave() (line 6 of modules/inline_image_attach/inline_image_attach.module).

Otherwise the image was well injected in the field field_image.
But the alt attribute is not populated. As by default alt attributes is required, and it’s best practice to fill it, it would be nice to handle this behavior. You could use at least the filename.

Features

If you have specified a different folder for inline images then the drupal default, this will not work properly.

It's a common use case to customize folder which store images. Would be more relevant to use the path configured in the wysiwg settings instead of to hardcode this path (inline-image) in your module.

Manual review

1- inline_attach_image.routing.yml

The administration menu callback should probably use "administer inline attach image" - which implies the user can change something - rather than "access administration pages" which is about viewing but not changing configurations.

2- inline_image_attach.module

- PHPDocs not relevant (Implants hook_theme() wheres it’s hook_entity_presave)
- The correct implementation of hook_entity_presave is hook_entity_presave(Drupal\Core\Entity\EntityInterface $entity)
- For testing the entity type, you should use if $entity Instanceof …. instead of if $type_id == "node" 

3- InlineImageAttachForm.php

You use deprecated function :

• use NodeType::loadMultiple() instead of entity_get_bundle()
• use entity_type.manager service instead of entity.manager service

You should not call the service \Drupal::entityManager() inside your class but should use dependency injection if you want in the futur do automated tests
The $form_state->getValue() method require a $key parameter. you should use $form_state->getValues() instead.

The wrong permission is a security issue. And please don't remove the security tag, we keep that for statistics and to show examples of security problems.

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.

To the reviewers: Please change back the priority to Normal after doing a review.

Automated Review

pareview details see https://pareview.sh/pareview/https-git.drupal.org-project-inline_image_a...

Git errors:

• The following git branches do not match the release branch pattern, you should remove/rename them. See https://www.drupal.org/node/1015226

Review of the 8.x-1.x branch (commit 208f119):

• Your README.txt does not follow best practices (headings need to be uppercase).
• The inline_image_attach.module does not implement hook_help().
• Bad line endings were found, always use unix style terminators. See https://www.drupal.org/coding-standards#indenting
• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
• DrupalPractice has found some issues with your code, but could be false positives.
• No automated test cases were found, did you consider writing PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation. Similar https://www.drupal.org/project/inline_image_to_field but based on this module

Master Branch

No: Does not follow the guidelines for master branch. See pareview details.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

No: Does not follow the guidelines for in-project documentation and/or the README Template. See pareview details.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

None

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

If you are still working on this application, you should fix all known problems and set the status to Needs review. (See also the project application workflow.)
Please don't change status of this application if you aren't sure you have time to dedicate to this application, or it will be closed again as won't fix.

I am closing this application due to lack of activity.