Problem/Motivation

The module's code doesn't follow any PHP or Drupal coding standards, so the code all looks really messy.

https://www.drupal.org/docs/develop/standards/php/php-coding-standards

Proposed resolution

The maintainers of the module should set up PHP Codesniffer and Drupal Coder in their dev environments and fix all the coding standards violations. Tests should also be set up to check them.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork rest_api_authentication-3408256

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

solideogloria created an issue. See original summary.

ighosh’s picture

ighosh commented
Assigned: Unassigned » ighosh

Checked the module. There are a lot of files that need its coding standards fixed. I'll work on these.

IGhosh opened merge request !3

ighosh’s picture

ighosh commented

I have created the Merge Request for this. Please review and let me know if there are any issues with the changes.
https://git.drupalcode.org/project/rest_api_authentication/-/merge_reque...

solideogloria’s picture

solideogloria commented
Status: Active » Needs work

I noticed that you removed a use of exit() in RestAPI.php. Was that intentional? It might change the effects.

Overall, it looks like a good improvement. I don't use this module, however, so it's up to other users to verify the changes work.

ighosh’s picture

ighosh commented

@solideogloria Thanks for noticing that. It was not intentional. I'll quickly update the MR. Thanks!
Edit - It is now updated.

solideogloria’s picture

solideogloria commented
Status: Needs work » Needs review
nikolay shapovalov’s picture

nikolay shapovalov commented
Assigned: ighosh » Unassigned
solideogloria’s picture

solideogloria commented
Priority: Normal » Critical

Marking Critical, because the module's code looks absolutely terrible.

solideogloria’s picture

solideogloria commented

Also, the MR is targeting the wrong branch. It should be against 2.x

solideogloria’s picture

solideogloria commented
Status: Needs review » Needs work
ighosh’s picture

ighosh commented

Hi @solideogloria. Thanks for pointing that out. I'll add the updated MR link in this MR itself.

IGhosh opened merge request !5

IGhosh closed merge request !3

ighosh’s picture

ighosh commented

Hi All. Updated MR - https://git.drupalcode.org/project/rest_api_authentication/-/merge_reque...
Please review.

ighosh’s picture

ighosh commented
Status: Needs work » Needs review
roberttabigue’s picture

roberttabigue commented
Status: Needs review » Needs work

Hi @IGhosh,

After applying your latest MR !5 to the Drupal REST & JSON API module against 2.0.12 on Drupal 10, not all of the PHPCS errors were resolved.
See below:

FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/rest_api_authentication.info.yml
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-----------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/Form/miniorangeAPIAuth.php
---------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 3 WARNINGS AFFECTING 4 LINES
---------------------------------------------------------------------------------------------------------------------------------------
  23 | ERROR   | Class name doesn't match filename; expected "class miniorangeAPIAuth"
  81 | WARNING | Unused global variable $base_url.
  82 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 295 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/setupAuthenticationForm.php
----------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------
 10 | ERROR | Class name doesn't match filename; expected "class setupAuthenticationForm"
----------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/customerSetupForm.php
----------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------
 10 | ERROR | Class name doesn't match filename; expected "class customerSetupForm"
----------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/upgradePlansForm.php
---------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
---------------------------------------------------------------------------------------------------------------------------------
 10 | ERROR   | Class name doesn't match filename; expected "class upgradePlansForm"
 24 | WARNING | Unused global variable $base_url.
---------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/advancedSettingsForm.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 10 | ERROR | Class name doesn't match filename; expected "class advancedSettingsForm"
-------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/Utilities.php
--------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------------------
 86 | WARNING | Unused variable $ajax_form.
 95 | WARNING | Unused variable $ajax_form.
--------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/MiniorangeApiAuthSupport.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------
 170 | WARNING | Unused variable $response.
-----------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/Routing/RouteSubscriber.php
----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------
 35 | WARNING | Unused private method restLoginRount()
----------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/requestForDemoForm.php
-----------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------
 10 | ERROR | Class name doesn't match filename; expected "class requestForDemoForm"
-----------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/roberttabigue/Project/DrupalOrg/drupalorgissues/web/modules/contrib/rest_api_authentication/src/Authentication/Provider/restAPI.php
------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------------
 18 | ERROR | Class name doesn't match filename; expected "class restAPI"
------------------------------------------------------------------------------------------------------------------------------------------------

I ran this command on the module:
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml rest_api_authentication

I'm moving this to ‘Needs work’ for now.

Thank you!

ighosh’s picture

ighosh commented

Hi @roberttabigue, In my system the only phpcs issues that are still coming up are the ones that I am not sure whether to remove or not as those changes/files are created by @arsh244, one of the maintainers of this module.
I updated the names of a few files because those are classes that had names starting with a small letter, but it should've been a capital letter.
I have not removed a few unused variables as I believe a confirmation from one of the maintainers might be required to do so.
Other than these, I have fixed most of the phpcs issues.
BTW, do let me know if there has been any mistake from my end.

purva_shende’s picture

purva_shende commented
Status: Needs work » Fixed
purva_shende’s picture

purva_shende commented
Status: Fixed » Closed (fixed)