🇮🇳India @mohit_aghera

larowlan → credited mohit_aghera → .

@Mingsong can you please update the PR with latest 11.x and change the destination for 11.x
I've pushed the fix on the branch.
There seems one test case failure, however this is failing on 11.x head as well (on my local).
So not sure if this issue's change is the root cause of this one.

Keeping this in needs work until we update the PR and pipeline is green.

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

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

Fixed the PR feedback.

Fixed the feedback.

@louis-cuny
Marking this as postponed since it needs more information.
Based on snippet given, it seems that cachable dependencies are not given.

Feel free to reopen if you feel it is valid.

Tests are green, back to needs review

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

@catch
I added that initially and my intention was to validate two things.

1. Validate the deprecation message being displayed
2. I should be able use and invoke the older plugins with different constructor argument
   

   $plugin = \Drupal::service('plugin.manager.views.argument')->createInstance('taxonomy_views_argument_test', []);
       $this->assertInstanceOf(TaxonomyViewsArgumentTest::class, $plugin);

Happy to remove the test case entirely or just expect deprecation only.
Let me know your thoughts.

I agree with @efrainh's comment in #13
This doesn't seems to be reproducible in latest 11.x.
I even manually tried in 10.2.x branch and doesn't seem to be reproducible there either.

To confirm that, I wrote one simple test case based on steps to reproduce.
Tests are coming green, so probably issue doesn't exist anymore or we need proper steps to reproduce the issue.

One can refer this PR https://git.drupalcode.org/project/drupal/-/merge_requests/7058 for the confirmation of test.

Updated branch with latest 11.x head since test case failures were related to 11.x HEAD only.
Keeping it to RTBC, will check if there are any failures.

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

Based on @Berdir's feedback in #29,
I've added a PR that adds permissions for `view label` operation.

I think we need some inputs from sub system maintainer about which is the correct approach.
I feel that `view label` access is more safe and restricted.

Updated the issue summary and mentioned that we need consensus about the solution.
Hiding other patches since #37 seems another valid implementation and passing test cases as well.

@adwivedi008, can you please be sure that we need to fix the issues?

Reason is comment in #100is saying that patch is no longer applied to the core.
In this case, you need to merge latest 11.x to the branch.
I tried that and it seemed to be working fine.

dan2k3k4 → credited mohit_aghera → .

Change record created. Back to RTBC

@alexpott do we need a change record for this one?

larowlan → credited mohit_aghera → .

@penyaskito
I'm not able to reproduce the issue.
I've attached one test-only patch which is passing on local against latest 11.x.
I haven't created new branch and fork since I'm not quite sure of this issue is still valid.

Can you please check the test-only patch and see if this is same as what has been given in #2

Hi @Anybody
I think this issue persists in 10.2.x release cycle.
Two issues are already fixed related to that in 10.3.x cycle.
https://www.drupal.org/project/drupal/issues/3416304 🐛 Javascript warning from content language and translation page Fixed
https://www.drupal.org/project/drupal/issues/3414415 🐛 Content language and translation AJAX expansion is backwards Fixed

I think we should cherry-pick those two commits in 10.2.x release cycle as well.
Since 10.2.x is still in active support, it should be easy to cherry pick those.

I even tested 10.3.x and 11.x and those seems works well.

@sakthi_dev, I think patch is already getting applied on 11.x
We probably don't need to re-roll the patch.
Can you please upload the re-roll diff, if possible.

I can confirm that MR #17 works for me as well.
Drupal core 10.1.x and editor_advanced_link: 2.2.4

I noticed the same observations mentionedin #25

Addressed the PR feedback.
I think https://git.drupalcode.org/project/drupal/-/merge_requests/6625#note_276326 would need some verification while doing review.
I'm somewhat confused about that.

Other point seems clear.

I noticed that both the changes has been addressed.
Moving back to RTBC since tests are green as well.

@smustgrave, I think we need to spin up a new branch since you've mentioned that new changes should go into. 2.0.x
We can update the destination for the PR once it is done.

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

Note: This is the different approach compared to all the uploaded patches. So be cautions if you are replacing this with your existing patches. This is based on comms in #80and #82

I've refactored the implementation based on discussion from #80 #82 and #83

Updates on #43
- Fixed point 1 and point 3.
Removed new test for validating both the permissions and removed changes in edit callback.
Reverted un-necessary changes made to the test callbacks.
- Reverted point 1.

I've ignored changes done in-between #50 to #69 since all are removing existing test cases and changing things in node handler.

#70
I'm somewhat confused about content owned by user 0.
I've added the changes and test case changes in commit. I think we can refactor it depending on the feedback.

mohit_aghera → changed the visibility of the branch 2845144-user-cant-reference to hidden.

Note: BC approach to handle annotations and attributes in MigratePluginManager class is being evaluated in https://www.drupal.org/project/drupal/issues/3424509 📌 Update MigratePluginManager to include both attribute and annotation class Fixed

We should revisit and rebase this PR after this is resolved.

Note: BC approach to handle annotations and attributes in MigratePluginManager class is being evaluated in https://www.drupal.org/project/drupal/issues/3424509 📌 Update MigratePluginManager to include both attribute and annotation class Fixed

We should revisit and rebase this PR after this is resolved.

Note: BC approach to handle annotations and attributes in MigratePluginManager class is being evaluated in https://www.drupal.org/project/drupal/issues/3424509 📌 Update MigratePluginManager to include both attribute and annotation class Fixed

We should revisit and rebase this PR after this is resolved.

Note: BC approach to handle annotations and attributes in MigratePluginManager class is being evaluated in https://www.drupal.org/project/drupal/issues/3424509 📌 Update MigratePluginManager to include both attribute and annotation class Fixed

We should revisit and rebase this PR after this is resolved.

- Added the test case for the another ViewsArgument plugin.
- Tests are passing on local as well as CI
- Unusual failure form nightwatch tests, triggered the job again.

Working on the test coverage for the second plugin.

- Tests are green now.
There was one deprecated config change in view. Sorted that out.

- Updated issue summary and moving it to Needs Review
- Removed Needs test tag since it is not required anymore.

I think we can move this to `tour` module's issue queue by changing the project.
I'm not quite sure if this is the right approach?

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

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

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

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

dan2k3k4 → credited mohit_aghera → .

Changes looks good. Since we are removing docblock text, I don't anticipate regression.

@dineshkumarbollu
This issue is for Novice contributors.
Based on your profile it seems you've enough experience.

Please don't pick Novice issues, keep it for *real novice* users.

Also, there are phpcs failures. Can you please address those.

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

mohit_aghera → created an issue.

Fixed one minor thing in above commit. No impact on functionality as such.
Keeping in RTBC only.

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

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

Thanks everyone for the inputs and feedback.
I've merged this to 2.x and tagged for the releaes 2.0.2-rc1 https://www.drupal.org/project/redirect_metrics/releases/2.0.2-rc1 →

Pushed the changes in latest 2.0.2 rc release https://www.drupal.org/project/redirect_metrics/releases/2.0.2-rc1 →

PR and the implementation approach looks good to me, if possible can we address the phpcs related issues?
I understand that those are not related to the changes we did, however it would be good to address since those came across.
I can do it before merge otherwise.

Thanks for the fixes @xeniksp
I think test run should be green now.

Apart from your changes in the MR, I've addressed following thigns.

• Fixing test case failures.
• Fix the deprecation message formatting.
• Add the necessary trait for expectDeprecation method.
• Add additional test case to validate the sort order when sort happens on non translatable field. This case was raised in comment #54

Failing tests are passing on local now.

I'll move issue to Needs review after test run is successful.

mohit_aghera → changed the visibility of the branch 3186834-sorting-nodes-by to hidden.

mohit_aghera → changed the visibility of the branch 3186834-sorting-nodes-by--reroll to hidden.

- Fixed the test case failures in the patch.
- Added new test case to filter the system path.
- Hiding all the patches except images for further confusion since we are following MR approach now.

@joseph.olstad:
Regarding test-only patch.
I feel it might not need because we are adding new option to filter form.
Since filter form element isn't available on the 11.x branch, it will always fail.
In that case, can we skip adding `test-only.patch`?

I think we can close this issue.
All the changes from this issue are added in #3388400 ✨ CKEditor 5 Support Fixed

The other issue has all the changes related to Drupal 10 and contains CKEditor5 related compatibility fixes as well.

dan2k3k4 → credited mohit_aghera → .

I think we need to do some work on the solution.
The option that is used in the patch #23 is removed now.
Issue that removed the option: https://www.drupal.org/node/981870 →

I am able to reproduce the issue. However slightly confused whether I took right steps to reproduce the issue.
I'm going to upload the issue along with the test-only patch and a fix.

larowlan → credited mohit_aghera → .

Pipeline is green now.
Good to get the initial review.
Updated the issue summary with the necessary details.

Hiding all the patches in favour of PR approach.
Hidden a couple of old branches as well as I couldn't update those with latest 11.x due to merge conflicts.
Created a new branch instead.

mohit_aghera → changed the visibility of the branch 2645922-image-field-generates to hidden.

mohit_aghera → changed the visibility of the branch 11.x to hidden.

@larowlan Addressed all 3 issues.
Tests are green now.

- Tests are added now and CI job is green.
- Did a quick check on issue summary and it looks good now.

I think ready for initial review now.

Created a new 2.0.0-beta release for D10

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

Hi @gg24
I think we might not need to add the index on the table anymore.
In 2.x branch, we are using dynamic_entity_reference field to store the entities that preview link wants to unlock.
DER already provides necessary integration and changes.
We can close the issue because only 2.x branch has support for D10+

mohit_aghera → created an issue.

yeah, working on that.

mohit_aghera → created an issue.

We are joining with comma here, default argument value in the join() method is comma.
As soon as we keep on selecting multiple media items, all the media item ids are joined with comma and stored in input element field called `current_selection`

`current_selection` hidden input field is retaining all the correct values despite any ajax call.
So in our fix, I am reading the value from the `current_selection` input field and storing values in another hidden input field called `media-library-modal-selection`.

There is a typo in patch #32
Resolving that error and few tests are passing on local

I am able to reproduce the issue by following exact steps in the issue summary.
Uploaded the patch for the same.
Not creating MR for now until we have conclusion about the solution.

Hence, keeping issue in needs work and triggering test manually to see how many failures we end up with.

https://www.drupal.org/project/webform_scheduled_tasks/releases/3.0.0-rc1 → is available for download.

Thanks for the testing @spuky @devil2005
3.0.0 release candidate is available to download.

https://www.drupal.org/project/webform_scheduled_tasks/releases/3.0.0-rc1 →

Thanks for testing @spuky
I'll create a new rc release today.
So folks can use it if they need

I am planning to fix few more php8 goodies in 3.x release.
So will keep rc release until we don't make those tweaks.

Meanwhile, we've already a dev release here https://www.drupal.org/project/webform_scheduled_tasks/releases/3.x-dev →
Feel free to test out in your existing projects.

Hiding patches in favor of MR