[1.1.x] Unique Alias Checker

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

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

FILE: unique_alias_checker/src/Form/UniqueAliasCheckerSettingsForm.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 52 | WARNING | Only string literals should be passed to t() where possible
--------------------------------------------------------------------------------


FILE: unique_alias_checker/src/UniqueAliasChecker.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 15 | ERROR | Missing short description in doc comment
--------------------------------------------------------------------------------


FILE: unique_alias_checker/unique_alias_checker.info.yml
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 7 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"
--------------------------------------------------------------------------------


FILE: unique_alias_checker/unique_alias_checker.module
--------------------------------------------------------------------------------
FOUND 20 ERRORS AND 3 WARNINGS AFFECTING 19 LINES
--------------------------------------------------------------------------------
  1 | ERROR   | [x] The PHP open tag must be followed by exactly one blank line
  4 | ERROR   | [x] Doc comment short description must end with a full stop
  7 | WARNING | [x] Unused use statement
  7 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 1
  8 | WARNING | [x] Unused use statement
  8 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 1
  9 | WARNING | [x] Unused use statement
  9 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 1
 11 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 1
 11 | ERROR   | [x] Missing function doc comment
 12 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 3
 14 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 5
 15 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 3
 16 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 1
 18 | ERROR   | [x] Line indented incorrectly; expected 0 spaces, found 1
 19 | ERROR   | [x] Expected 2 space(s) before asterisk; 1 found
 20 | ERROR   | [x] Expected 2 space(s) before asterisk; 1 found
 21 | ERROR   | [x] Expected 2 space(s) before asterisk; 1 found
 22 | ERROR   | [x] Expected 2 space(s) before asterisk; 1 found
 23 | ERROR   | [x] Expected 2 space(s) before asterisk; 1 found
 24 | ERROR   | [x] Expected 2 space(s) before asterisk; 1 found
 25 | ERROR   | [x] Expected 2 space(s) before asterisk; 1 found
 35 | ERROR   | [x] Expected newline after closing brace
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 23 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Hello Vishal, all errors have been fixed on newly tagged release 1.1.1.

1 warning remains, intend to leave as is:

FILE: /Users/mikdrano/dev/drupal-testing-two/web/modules/contrib/unique_alias_checker/src/Form/UniqueAliasCheckerSettingsForm.php
---------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------
 38 | WARNING | Only string literals should be passed to t() where possible

1. Remove the LICENSE.txt file. The LICENSE file isn't necessary. It will be added automatically by the packaging script.

2. Replace README.txt with README.md file. Please take a moment to make your README.md follow the guidelines.

1. Remove the LICENSE.txt file. The LICENSE file isn't necessary. It will be added automatically by the packaging script.

2. Replace README.txt with README.md file. Please take a moment to make your README.md follow the guidelines.

Tagged and released v1.1.2 with the following:
1. Removed license.txt
2. Replaced Readme.txt with Readme.md based on drupal guidelines.

@mdranove,

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

It is a great idea to notice content managers about this behaviour.

The following are not actually a security but general advices:

1. You can simply use MODULENAME_form_node_form_alter() here
2. I'm pretty sure that entity can be retrived with $form_state->getFormObject()->getEntity() here
3. If you use setErrorByName() method you should provide the actual form element key for the first parameter
4. In case when $original_alias == $alias your method returns nothing (void). You might consider the approach when your method ends with return $original_alias != $alias; so you won't need to use isset() in the validation function
5. I belive you would like to change the class description :)

1. You can simply use MODULENAME_form_node_form_alter() here.
2. I'm pretty sure that entity can be retrived with $form_state->getFormObject()->getEntity() here.
3. If you use setErrorByName() method you should provide the actual form element key for the first parameter.
4. In case when $original_alias == $alias your method returns nothing (void). You might consider the approach when your method ends with return $original_alias != $alias; so you won't need to use isset() in the validation function.
5. I belive you would like to change the class description :)

Thanks for the feedback @a.kovrigin. I have incorporated some of your suggestions on release 1.1.3 → , but not all. Response below:

1. I don't think this will work as form IDs change per content type.
2. Tried this, but it did not work consistently.
3. I have updated setErrorByName()
4. I have updated this logic based on your suggestion
5. Yes :). Thank you.

Manual Review

Hello, thank you for the feedback.

Feedback has been incorporated into release 1.1.4 → .

Validation for taxonomy terms is not supported yet, but will take it into consideration for the future.

@mdranove As a side note, it is not necessary to create a new tag nor a new release every time that what reported has been changed. Commit the changes in the branch without creating tags or releases.

A correct branch ends with the literal .x. (x is not a placeholder for a number.)

Please commit the last changes in that branch. The other branches, like 1.1.4 are not correct.

As side note, please check Git on your computer is set to use the same email associated to your drupal.org account, or the commits are not associated to your account.

@apaderno,

Thank you for the feedback, I have gone ahead and merged the most recent changes into branch 1.1.x.

class UniqueAliasCheckerSettingsForm extends ConfigFormBase implements ContainerInjectionInterface {

That interface is already implemented by the parent class. There is no need to repeat implements ContainerInjectionInterface.

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.

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