🇷🇺Russia @zniki.ru

Resolve Merge conflict and update some comments. Ready for review.

During review please take a closer look at order of @hook, @throws, @annotation in src/Commands/PathautoCommands.php not sure, that this is correct order, welcome for suggestion.

And I don't know how to fix "Missing short description in doc comment" at pathauto.api.php:10. This is last one from Drupal.Commenting in code base for now,

CS Report updated in the IS.

@akshaydalvi212 thanks for your contribution, this issue is only about to fix Drupal.Commenting violations.
I will revert commit b043409c, because it contains changes not related to these cs rules.
And I will resolve merge conflict.

@paramnida, thanks a lot for your feedback. Sorry that this change affects you in this way. I also don't know if it possible to update CR.

Nikolay Shapovalov → created an issue.

Nikolay Shapovalov → changed the visibility of the branch 3442756-add-composer.json to hidden.

Ready for review.

Nikolay Shapovalov → created an issue.

Nikolay Shapovalov → created an issue.

Nikolay Shapovalov → created an issue.

Thanks. Patch works for me as well.

Merged.

I will close this issue for now. Please feel free to open if you will find way to replicate it.

Nikolay Shapovalov → created an issue.

I was searching how to improve solution and I found that there is a module with just exact functionality:
Views user field value contextual filter [user_field_value] →

And there are more modules that also introduce some nice features, like token support for views default value:

• https://www.drupal.org/project/user_field_value →
• https://www.drupal.org/project/views_argument_token →
• https://www.drupal.org/project/views_extras →
• https://www.drupal.org/project/views_arg_entity_field →

Maybe we can combine these modules?
Let's have discussion before we continue to work on this feature request.

Thanks for your effort, it's better to separate phpcs fix and gitlab ci integration.

MR 4160 is mergeable, update status back to needs review.

DamienMcKenna, thanks for fixing tests.
Changes in the MR looks good.
I am not sure about changes in method MetatagTagTypesTest::todoTestUrl(), but I leave it up to you.
I would completely remove this method.

Mark this as RTBC.

Thanks, I left feedback in the MR, please check.

Removed changes from PathautoWidget.php because there is separate task 📌 Remove leftover from src/PathautoWidget.php Active .

Oh yeah, I am happy, thanks.

Remove full report from IS description, add file with full report.

There are too many violations, for single issue

A TOTAL OF 304 SNIFF VIOLATIONS WERE FOUND IN 61 SOURCES

Let's create child issues for different rules to fix, it will be easy to review.
Here is the list of violations sorted by name.

[ ] Drupal.Arrays.Array.LongLineDeclaration                                                           13
[ ] Drupal.Commenting.DocComment.MissingShort                                                         27
[ ] Drupal.Commenting.DocComment.ShortSingleLine                                                      1
[ ] Drupal.Commenting.FunctionComment.MissingParamComment                                             2
[ ] Drupal.Commenting.FunctionComment.MissingParamName                                                2
[ ] Drupal.Commenting.FunctionComment.MissingParamType                                                19
[ ] Drupal.Commenting.FunctionComment.MissingReturnComment                                            11
[ ] Drupal.Commenting.FunctionComment.MissingReturnType                                               2
[ ] Drupal.Commenting.FunctionComment.ParamMissingDefinition                                          4
[ ] Drupal.Commenting.HookComment.HookCommentFormat                                                   1
[ ] Drupal.Commenting.InlineComment.SpacingBefore                                                     6
[ ] Drupal.Commenting.VariableComment.Missing                                                         1
[ ] Drupal.Files.LineLength.TooLong                                                                   24
[ ] Drupal.NamingConventions.ValidFunctionName.InvalidPrefix                                          1
[ ] Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps                                      2
[ ] Drupal.Semantics.FunctionT.BackslashSingleQuote                                                   1
[ ] DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable                                       5
[ ] DrupalPractice.Commenting.CommentEmptyLine.SpacingAfter                                           2
[ ] DrupalPractice.FunctionCalls.InsecureUnserialize.InsecureUnserialize                              5
[ ] DrupalPractice.Objects.GlobalDrupal.GlobalDrupal                                                  10
[ ] Squiz.PHP.NonExecutableCode.Unreachable                                                           1
[x] Drupal.Arrays.Array.CommaLastItem                                                                 8
[x] Drupal.Classes.ClassDeclaration.CloseBraceAfterBody                                               10
[x] Drupal.Commenting.ClassComment.Missing                                                            4
[x] Drupal.Commenting.DataTypeNamespace.DataTypeNamespace                                             9
[x] Drupal.Commenting.DocComment.ContentAfterOpen                                                     1
[x] Drupal.Commenting.DocComment.SpacingAfter                                                         1
[x] Drupal.Commenting.DocCommentAlignment.SpaceBeforeStar                                             4
[x] Drupal.Commenting.FileComment.Missing                                                             1
[x] Drupal.Commenting.FileComment.NamespaceNoFileDoc                                                  6
[x] Drupal.Commenting.FileComment.SpacingAfterComment                                                 1
[x] Drupal.Commenting.FunctionComment.Missing                                                         10
[x] Drupal.Commenting.InlineComment.InvalidEndChar                                                    2
[x] Drupal.Commenting.InlineComment.NoSpaceBefore                                                     6
[x] Drupal.Commenting.InlineComment.SpacingAfter                                                      2
[x] Drupal.Commenting.InlineVariableComment.VarInlineOrder                                            1
[x] Drupal.Commenting.TodoComment.TodoFormat                                                          2
[x] Drupal.ControlStructures.ControlSignature.NewlineAfterCloseBrace                                  1
[x] Drupal.ControlStructures.ControlSignature.SpaceAfterCloseParenthesis                              1
[x] Drupal.Files.EndFileNewline.NoneFound                                                             1
[x] Drupal.Files.EndFileNewline.TooMany                                                               1
[x] Drupal.Formatting.MultipleStatementAlignment.Incorrect                                            1
[x] Drupal.Formatting.MultipleStatementAlignment.NotSame                                              1
[x] Drupal.Scope.MethodScope.Missing                                                                  4
[x] Drupal.WhiteSpace.Comma.NoSpace                                                                   2
[x] Drupal.WhiteSpace.OpenBracketSpacing.OpeningWhitespace                                            1
[x] Drupal.WhiteSpace.OpenTagNewline.BlankLine                                                        3
[x] Drupal.WhiteSpace.ScopeIndent.IncorrectExact                                                      36
[x] Generic.PHP.UpperCaseConstant.Found                                                               1
[x] PSR2.Namespaces.UseDeclaration.SpaceAfterLastUse                                                  2
[x] SlevomatCodingStandard.ControlStructures.RequireNullCoalesceOperator.NullCoalesceOperatorNotUsed  5
[x] SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses.IncorrectlyOrderedUses                 15
[x] Squiz.Arrays.ArrayDeclaration.CloseBraceNewLine                                                   1
[x] Squiz.ControlStructures.SwitchDeclaration.SpacingAfterBreak                                       2
[x] Squiz.Strings.ConcatenationSpacing.PaddingFound                                                   2
[x] Squiz.WhiteSpace.FunctionSpacing.After                                                            2
[x] Squiz.WhiteSpace.FunctionSpacing.AfterLast                                                        10
[x] Squiz.WhiteSpace.FunctionSpacing.Before                                                           1
[x] Squiz.WhiteSpace.OperatorSpacing.NoSpaceBefore                                                    1
[x] Squiz.WhiteSpace.ScopeKeywordSpacing.Incorrect                                                    1
[x] Squiz.WhiteSpace.SuperfluousWhitespace.EmptyLines                                                 1

We need to think how to group rules.

I think this is ready for review.

Update phpcs report.

Removed phpcs.xml file from MR, as suggested by @berdir https://git.drupalcode.org/project/pathauto/-/merge_requests/62#note_256660.
Remove changes that is not part of phpcs report.

Thanks for the feedback.
Removed phpcs.xml.
Created follow up issue 📌 Remove leftover from src/PathautoWidget.php Active .

Nikolay Shapovalov → created an issue.

No progress so far.

Will put some time to fix it.

Thanks, RTBC.

MR is ready for review.
Validation steps can be fixed in separate issues.

Create MR with auto fix from phpcbf, have no time to continue on this issue now.

Nikolay Shapovalov → created an issue.

Nikolay Shapovalov → created an issue.

Thanks, Great work. RTBC!

Thanks. Merged.

Nikolay Shapovalov → created an issue.

+1 for #20.

Nikolay Shapovalov → changed the visibility of the branch 3416385-youtubefieldtesttestyoutubefield-undefined-array to hidden.

Please check feedback in the MR 7.
Merci.

@Mohd Sahzad thanks for your patch.
But changes you provided doesn't make sense.
The name of the module is "Y Layout Builder - Donate", and you can split it into to sentences.
You can try to use module machine name.

The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, show which command has been used, which arguments have been used, and which report that command shown.

Thanks a lot, for you changes.
I provide a review, please check my feedback.

Suggestion about adding comment is optional, it's up to you to decide.

Do you think title "Adopt GitlabCi" will be better fit this issue?
I wasn't able to find "starter phpstan configuration" in the MR.

I was able to find solution, not sure if I will merge it to code base.
But you can use it as patch.

Please test it and provide feedback.

I was able to replicate this on Drupal 10.2.
Thanks for detailed explanation, I want to add that you need to enable "Allow each content item to have its layout customized." option on Article manage display.

This happens when you move block, and request to layout_builder.move_block route is made.

  public function getArgument() {
    $value = NULL;
    // $this->options['entity_type_id'] is "node" in my case.
    if ($entity = $this->routeMatch->getParameter($this->options['entity_type_id'])) {
    }
   
    return $value;
  }

As you can see from the code for route layout_builder.move_block getParameter('node') will be null.
And if clause if FALSE, and getArgument returns NULL. That is the reason of this issue.

In my case I have:
Node URL: /node/149/layout
When I move blocks
AJAX requestUri: /layout_builder/move/block/overrides/node.149/0/0/content/9e02ecfc-2654-4e95-b554-db1808cebb1e?_wrapper_format=drupal_ajax

So looks like section_storage is node.149.
I think you can get desired entity, but only as customization.
I don't know how this can be implemented in a contrib module as universal solution.

Maybe someone have any ideas?
We need to find the way how to get original url, to get entity object.

Not sure what status to set. Set it to Needs review for now.

Provide my feedback on the MR.

Thank you, left a feedback.
During review found issue in README.md, issue create 📌 [novice] Change layout_fields to layout_paragraphs in README.md Active

Nikolay Shapovalov → created an issue.

I revert changes, because title was changed.

Added branch 3364511-refactor just as prototype. Not finished.

I checked MR, and changes is not so big.
But there are a lot places with code duplication, so I will set status to NW.

I have several ideas how to refactor the code:
1) Add "Current user" for $form['entity_type_id'].
2) Extends CurrentUserFieldValue from CurrentEntityFieldValue.
3) Create common abstract class for CurrentEntityFieldValue, CurrentUserFieldValue.

Only thing I am not sure is views caching.

I was not able to replicate the issue on my localhost.
I create a block and add set User contextual filter.
But when I visit page with the block by anonym user, view was not presented.

Can you please provide more information how to replicate the issue.

Needs to provide change record for this change.
Need IS update:

• provide information about UI changes, screenshots before and after.
• provide information about data mode changes.

As this is going to be data model fix, need to create Change Record.
Also it would be great to have tests.

@danflanagan8 thanks for your contribution.
Changes merged.

Thanks @apaderno, changes merged.

Add schema as separate commit.

Thanks everyone, let's continue with MR workflow.
I see there are tons of violations in MR 2, please run phpcs and fix all the violations.

Thanks for the update.
MR looks good.
The only thing I am not sure, is about renaming _customFieldDefinitions method. But let's see what will maintainer will tell.
Set it to RTBC.
Great job.

Please provide interdiff → .

+++ b/colorbox_media_video.info.yml
@@ -6,4 +6,4 @@ core_version_requirement: ^8.8 || ^9
+  - drupal/colorbox

My bad, I was wrong. This should be

 - colorbox/colorbox

because colorbox is not part of drupal core.

Thank you @realsp, one more note from my side, please check MR.

Great job, thanks for your effort.

Thanks, please check my comments.

Great work, thanks. RTBC.

https://www.drupal.org/node/2535454#s-example-2-updating-views-field-han... →
Looks like update function needs to be refactor.

Update MR. Looks good for me.
Waiting for feedback.

MR is ready for review.

MR is ready for review.

Nikolay Shapovalov → created an issue.

Please check my feedback.

@realsp Thanks for the update.
Let's move Dependecy injection changes to separate issue.
It's hard to make review.

Thanks, looks good.

Thanks, please update phpcs report in the IS, and check my feedback at the MR.

Thanks, suggested changes doesn't fix the issue.
Tests failed because there is no module youtube.

Thanks, I left comment.
Looks like IS phpcs report is outdated, can you please renew it for current 8.x-1.x branch?

@realsp thanks for your changes, tests still failing.
We need to fix tests first.
I create issue 🐛 YoutubeFieldTest::testYoutubeField Undefined array key "youtube" Active .

Nikolay Shapovalov → created an issue.

@danflanagan8 thanks a lot for you contribution.
It would be awesome to cover code with tests.
I created MR, and test failed.
New issues to phpstan was introduced by suggested changes.
Set to NW.

Nikolay Shapovalov → made their first commit to this issue’s fork.

@apaderno thanks for provided commits, phpcs reports new violation. Set to NW.

Thanks for a great module.

Fixed.

It will be 10 years this year.

Ready for review.

Nikolay Shapovalov → created an issue.

Taras, thanks a lot, changes looks good. Please check my question.

Can you please update IS, and provide fresh phpcs report?