Drupal Coding Standard Issue

Hi @Rakhi,

I will review your patch.

Thanks

Hey @Rakhi,

I reviewed your patch. It applied cleanly.

Thanks for the patch.

But,

Still there are certain issues, so moving this to "Needs work".

urvasi@urvasi-Inspiron-15-3552:/var/www/html/contribution/d8_cont/web/modules/contrib$ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig organigrams-3301295/

FILE: ...b/modules/contrib/organigrams-3301295/src/Form/OrganigramsImportD7Form.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 79 | WARNING | \Drupal calls should be avoided in classes, use dependency
    |         | injection instead
--------------------------------------------------------------------------------


FILE: ...odules/contrib/organigrams-3301295/src/Form/OrganigramsImportItemsForm.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 150 | WARNING | \Drupal calls should be avoided in classes, use dependency
     |         | injection instead
--------------------------------------------------------------------------------


FILE: ...n/d8_cont/web/modules/contrib/organigrams-3301295/src/TaxonomyTermTree.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
--------------------------------------------------------------------------------
 133 | WARNING | \Drupal calls should be avoided in classes, use dependency
     |         | injection instead
 186 | WARNING | \Drupal calls should be avoided in classes, use dependency
     |         | injection instead
 214 | WARNING | \Drupal calls should be avoided in classes, use dependency
     |         | injection instead
 223 | WARNING | \Drupal calls should be avoided in classes, use dependency
     |         | injection instead
--------------------------------------------------------------------------------

Time: 1.32 secs; Memory: 10MB

urvasi@urvasi-Inspiron-15-3552:/var/www/html/contribution/d8_cont/web/modules/contrib$ 

I am working on them.

Hi,

I worked on them. Providing a patch, please review.

Thanks

Thank you for your work! I applied your patch on the dev branch in a clean Drupal 9 environment and ran into the following issues:

1. Go to /admin/structure/taxonomy
2. Click on "Add organigram".
3. The following error shows: ArgumentCountError: Too few arguments to function Drupal\organigrams\TaxonomyTermTree::__construct(), 2 passed in /drupal9_data/web/core/lib/Drupal/Component/DependencyInjection/Container.php on line 262 and exactly 3 expected in Drupal\organigrams\TaxonomyTermTree->__construct() (line 52 of modules/contrib/organigrams/src/TaxonomyTermTree.php).

This can easily be fixed by adding @module_handler as argument to organigrams.services.yml.

Then, the next issue arose:

1. After fixing the missing argument, go to /admin/structure/taxonomy/add/organigram
2. Click on "Import Drupal 7 organigram".
3. The following error shows: Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "entity.query". Did you mean one of these: "entity.query.config", "entity.query.sql", "pgsql.entity.query.sql", "entity.query.null", "entity.query.keyvalue"? in Drupal\Component\DependencyInjection\Container->get() (line 156 of core/lib/Drupal/Component/DependencyInjection/Container.php).

I'm not sure what to do with this one.

After applying the patch #7, i got the following phpcs warnings-

FILE: /organigrams/src/TaxonomyTermTree.php
-------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-------------------------------------------------------------------------------
 199 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 227 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------

So, I have fixed the above warnings along with @PatrickScheffer's first points after reviewing module in D8 and created a new patch.

and @PatrickScheffer, about your 2nd concern, it is because entity.query got deprecated in D9, see this - entity.query service deprecated in favor of EntityStorageInterface::getQuery()

So, attached patch fixes the phpcs warnings and fatal error of ArgumentCountError: Too few arguments to function Drupal\organigrams\TaxonomyTermTree and You have requested a non-existent service "entity.query"

here is the updated patch -

Hi, I reviewed patch #10 and all the PHPCS issues have been fixed. Moving it to RTBC, Good job =)

Thank you all for your contributions! I have applied patch #10 and also added access checks to the entity queries because that gave some errors as well. Can someone do a final test on the 8.x-2.x branch before I create a new release?

Hi Patrick, I've tested the 8.x-2.x branch and the issues related to acces to the entity queries have been fixed. Hence I move it to RTBC.

Thanks to you all! I released version 8.x-2.3.