Skip default argument for view URL does not work and never has so it can be removed

Can you fix the codestyle, please?

+      if (!empty($this->argument)) foreach ($this->argument as $argument_id => $argument) {

It would be cool if some kind of code explains what the flag does etc.

It's hard to understand for someone not understand it totally.

This is a really tough one. I can see this going either way.

Particularly if the path is something like node/%/foo -- I might have a block that shows somewhere that links to a tab on the node. In that case, I would absolutely want the argument to appear in the more link.

And if I'm using a summary view? I almost certainly absolutely positively DO want that in there.

And in other cases, including one that's currently in the issue queue, I do not want that argument in there.

This needs some serious thought because I don't think there is a way to get this right automatically.

That's probably the only solution. It's a pretty difficult one to get the wording right for, but let's start by trying out a checkbox. It's okay if the checkbox is buried, this is a .5% use case.

Deleted 2 comments posted to wrong issue to prevent later confusion.

Ok, committed.

I think it's not fixed, is_default does not prevents for arguments that passes from page manager
For example, I wanna pass 1st argument from panel but 2nd from url - for now get_url always appends this 1st argument to action url

Patch removes is_default so devs can manually use 'default_argument_skip_url' checkbox for arguments that should not appear in url

This does the trick: if that checkbox is or is not checked the arguments get appended to the custom URL without the patch.

With the patch the arguments aren't appended to the end of the custom URL as expected.

We've recently switched our testing from the old qa.drupal.org to DrupalCI. Because of a bug in the new system, #2623840: Views (D7) patches not being tested, older patches must be re-uploaded. On re-uploading the patch, please set the status to "Needs Review" so that the test bot will add it to its queue.

If all tests pass, change the Status back to "Reviewed & tested by the community". We'll most likely commit the patch immediately without having to go through another round of peer review.

We apologize for the trouble, and appreciate your patience.

Re-uploaded. RTBC get's tested too I believe.

Thanks!

I'm sorry but doesn't this also introduces a BC break?

I suppose it's possible that this could break backward compatibility.

From dawehner on IRC:

Changing the way how those URLs are generated IMHO could lead to problems, when people kind of assume some of those.

Thoughts anyone?

This issue exists in Drupal 8.0 and above also.

The option 'Skip default argument for view URL' doesnt work at all in 8.0 and above!

Re-roll patch Attached against Drupal 10.1.x

Discussed this with @xjm and @longwave at Drupal Dev Days and we all agree this checkbox should be removed.

• It hasn't worked since Drupal 8.0.0
• the functionality is very edge case
• the description and title of the field are very unclear as to what a user could expect this to do

Here we go. Removed the setting. Update hook added to remove from existing and newly imported config. Removed from all existing core config.

Updated the IS

Applied the patch and searched for default_argument_skip_url.
Found 19 instances that were in the test, update hook, or fixture for the test.
Ran the update hook to make sure that ran and it did without issue.

Think this is good.

Adding credits for DDD discussion.

Unrelated fails

#3361121: [random test failure] InstallerExistingConfig[SyncDirectory]MultilingualTest::testConfigSync fixed some config install tests, looks like we need work here for that.

#3376563: Random test fail in Drupal\Tests\Component\Utility\RandomTest::testRandomMachineNamesUniqueness is a genuine random fail in the random component :)

Removed default_argument_skip_url from views.view.files and views.view.glossary from multilingual.tar.gz

No interdiff, because the only change is that tarball and interdiff on that is useless.

It really helps if you supply the correct tarball format _sigh_

Again, no interdiff, since the only change is in a tarball.

And maybe even not include the local test changes in an official patch.
#MustBeMondayMorning

Going to tentatively self back-to-RTBC.

- I didn't change any code.
- The only changes were made in a fixture, which wasn't testable before because of the re-enabling of the test in #3361121: [random test failure] InstallerExistingConfig[SyncDirectory]MultilingualTest::testConfigSync wasn't committed yet.

RTBC +1

To me this doesn't need a change record given we are just removing something that is entirely broken.

Committed f890b7f and pushed to 11.x. Thanks!

I would actually give this a change record.

Added small CR, if someone could review we can publish it, https://www.drupal.org/node/3382316

Looks good to me. Added version numbers and published the change record.

There is a regression a reporter thinks may be caused by this issue.

Via @berdir:

something that I'm seeing pop up a lot in contrib tests is the removal of default_argument_skip_url, I've had that in ~3 out of 5 modules that I've worked on so far. It's key in a yml file, not sure where exactly detection of that would go

[...]

well, config schema validation fails
so the test fails

https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22defau... finds 1500 instances of this

So it looks like the fallout from this is much bigger than expected.

I am working on #3450284: Support default_argument_skip_url views setting removal checking in Upgrade Status to side-add a deprecation for this in upgrade status at least. But based on that contrib search many more people will run into this.

This is another instance where we missed #3443942: Add proper deprecation notices in config entity presave bc layers.

If we'd done that at the time, then module tests with deprecation reporting enabled would have picked this up since the time it was committed.

We could still retrospectively add the deprecation support to 10.3.0 during beta maybe?

Opened #3450868: Add ViewsConfigUpdater deprecation support for default_argument_skip_url.

I released https://www.drupal.org/project/upgrade_status/releases/4.3.2 with support for checking for this one specific key deprecation. I don't think we have a generic way to statically detect config keys depreacted but would be happy to add support for that too there for a more general solution.