Account created on 27 March 2012, about 12 years ago
#

Merge Requests

More

Recent comments

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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:

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

Oh yeah, I am happy, thanks.

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

Provide my feedback on the MR.

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

I revert changes, because title was changed.

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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.
🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

@danflanagan8 thanks for your contribution.
Changes merged.

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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.

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

Thanks, please check my comments.

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

🇷🇺Russia zniki.ru

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

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

🇷🇺Russia zniki.ru

Tests failed, please check tests.

🇷🇺Russia zniki.ru

Thanks for MR, please check my feedback.

There are more changes in the MR, then we have in the phpcs report in the issue.
I don't want to fix issues that is reported by phpcs in this issue.
Can you please update report in the IS?

Production build 0.69.0 2024