[2.0.x] JSON:API Reference

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

Fix PHPCS Issue.

vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,php  web/modules/contrib/jsonapi_reference/


FILE: //web/modules/contrib/jsonapi_reference/jsonapi_reference.post_update.php
------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------
  1 | ERROR   | [x] End of line character is invalid; expected "\n" but found "\r\n"
 26 | WARNING | [ ] Unused variable $article_node_type.
------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------

Thank you @shashank5563 ❤️

I have run PHPCS and only receive one warning:

jsonapi_reference on  2.0.x via 🐘 v8.1.13 on ☁️  (eu-west-1)
❯ ddev exec vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,php  web/modules/contrib/jsonapi_reference/

FILE: /var/www/html/web/modules/contrib/jsonapi_reference/jsonapi_reference.post_update.php
-------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------
 26 | WARNING | Unused variable $article_node_type.
-------------------------------------------------------------------------------------------

Have you definitely reviewed 2.0.x? Is there any chance you may have introduced that change of line ending locally?

I have checked out a fresh version of that branch that has not been opened in an IDE (so could not have been auto-corrected) and have verified the line endings are correct and identical:

jsonapi_reference on  2.0.x via 🐘 v8.1.13 on ☁️  (eu-west-1)
❯ git ls-files --eol
i/lf    w/lf    attr/                 	.gitignore
i/lf    w/lf    attr/                 	README.txt
i/lf    w/lf    attr/                 	composer.json
i/lf    w/lf    attr/                 	config/schema/jsonapi_reference.schema.yml
i/lf    w/lf    attr/                 	jsonapi_reference.info.yml
i/lf    w/lf    attr/                 	jsonapi_reference.links.menu.yml
i/lf    w/lf    attr/                 	jsonapi_reference.module
i/lf    w/lf    attr/                 	jsonapi_reference.permissions.yml
i/lf    w/lf    attr/                 	jsonapi_reference.post_update.php
i/lf    w/lf    attr/                 	jsonapi_reference.routing.yml
i/lf    w/lf    attr/                 	jsonapi_reference.services.yml
i/lf    w/lf    attr/                 	src/Controller/TypedResourceController.php
i/lf    w/lf    attr/                 	src/Form/JsonApiReferenceConfigForm.php
i/lf    w/lf    attr/                 	src/JsonApiClient.php
i/lf    w/lf    attr/                 	src/JsonApiClientInterface.php
i/lf    w/lf    attr/                 	src/Plugin/Field/FieldFormatter/TypedResourceObjectStringFormatter.php
i/lf    w/lf    attr/                 	src/Plugin/Field/FieldType/TypedResourceObjectItem.php
i/lf    w/lf    attr/                 	src/Plugin/Field/FieldWidget/TypedResourceObjectAutocompleteWidget.php
i/lf    w/lf    attr/                 	tests/src/Functional/JsonApiReferenceFieldBase.php
i/lf    w/lf    attr/                 	tests/src/Functional/JsonApiReferenceFieldFormatterTest.php
i/lf    w/lf    attr/                 	tests/src/Functional/JsonApiReferenceTest.php
i/lf    w/lf    attr/                 	tests/src/Unit/UuidTest.php

@Eli-T, The new line doesn't have matter because, this is my PHPC settings. Have you fixed the second point?

Unused variable $article_node_type

@shashank5563 I have just fixed that on commit c6a6d4b3fab9ef16df1a2daf4a825043fe93f695 just pushed to HEAD of 2.0.x.

Ran PHPCS and no issues were found. Also went through the code and I have to say the code looks very clean and well-written. I have a few comments.

First of all, my IDE (PHPStorm in this case) does not like consecutive dots in comments in the TypedResourceObjectAutocompleteWidget class. I don't necessarily think it's an issue, but might be worth changing.

In the class doc of TypedResourceObjectStringFormatter, I think you want to change the following;

Plugin implementation of a 'basic_string' formatter.

to

Plugin implementation of a 'typed_resource_object_string' formatter.

I also see that you define your return types in every class except for the TypedResourceObjectStringFormatter class. I recommend adding those for consistency.

You are also missing the return type of the create function in the TypedResourceObjectStringFormatter class.

Thanks @kevin.brocatus!

I have added return types to TypedResourceObjectStringFormatter as suggested and updated the class doc of the same, addressing two of your points in #3375840-9: [2.0.x] JSON:API Reference →

I have not removed the consecutive dots in the comments. I don't know what specific issue they cause you in PHPStorm, but I also use PHPStorm and such style of comments does not cause any problems in my instance. If I'm the outlier then maybe it would be worth changing, but I feel this style of comment useful, for example when you want to use a two comments to explain a condition and action with two comments that gramatically consist of a single sentence.

These changes are in commit 648a03bdc6858d1b585d5e9bd8435cd1d7e356b6 on 2.0.x

Putting back to needs review as comments are addressed

@Eli-T 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.

I am changing priority as per Issue priorities.

No issues found
Changing status to RTBC!

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

I thank all the reviewers.

Thank you @apaderno, @vinaymahale, @shashank5563, and @kevin.brocatus for your help 💙

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