🇳🇱Netherlands @Lendude

Agree with @acbramley, the Media Library sounds like the wrong widget/field type here, it should just use a file field. Media is about reusing.

And the described results seem inline with what the permission set dictates. Sure, they give confusing results for the end user, but I don't see how we would change the outcome short of trying to guess the intent based on a certain combination of permissions chosen. So "works as designed" I'd say.

Since it seems to break on \Drupal\views\Plugin\views\query\Sql::create() maybe a patch that was updated (but might be broken) during the upgrade?

Does the problem go away if you remove all patches from the project?

Didn't try, but even if we would fix the access check we would then probably run into caching issues, since the validation cache contexts wouldn't bubble currently due to #3091671: Validate user has access option in contextual filter does not bubble access cacheability for views page →

This behaviour is not specific to trimming, rewriting the result to a link acts the same way, it works on individual results, not the end result.

The way to work around this is by hiding the multi value field and then using it as a token in a second field (based in an ID field for example) and then trimming the value of the field where you are using the token.

I am not in favour of adding more settings to the multiple value form. If we want to fix this in a way that doesn't require a workaround I'd be looking more to a single checkbox that does something like 'Apply all Rewrites to the concatenated values' and then we don't need to change this one functionality at a time.
Not saying that would be easy, because I don't think it is, but that would not make the Views UI any more crowded than it needs to be.
My 2cts.

Steps to reproduce:
* Create a View of nodes
* Show fields
* Show the nid and title field
* Add a contextual filter 'Content: Has taxonomy term ID' and set default value to ' Display all results for the specified field'
* For the title field set rewrite results: Output this field as a custom link to /node/{{ nid }}?term_id={{ arguments.tid }}
* Create a page display
* Go to the page and see that 'term_id' is added to the link URL on the title, bt with no value

Didn't try the patch yet

As pointed out in #3, 'Content ID from URL' won't work with products, because it will look for a node ID, not a product ID (naming things is hard)

The hidden selector 'Profile ID' is not something that comes from core, no idea what puts it in your form, but it is probably something from contrib or custom code. So best start looking there.

larowlan → credited Lendude → .

Screenshot of the failed test page: https://git.drupalcode.org/issue/drupal-3439427/-/jobs/1271159/artifacts...

PoC to show we can do this, needs clean up

Lendude → created an issue.

Hide patches

Colleague just ran into this, lets fix it. Moved #30 to a MR

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

Since we no longer need the needs review status to trigger test running I'm resurrecting this.

The latest patch and the MR patches posted in #113 still break the media library search

Sounds good. The code weight is indeed low, it has some test coverage and the number of open issues seems manageable. Plus, it's a very useful feature to have!

On a View with about 40 displays but not that many fields I have 8791 input vars, so hitting 10000 doesn't seem impossible, for me it also needed more than 256M of memory to actually load the form in the first place....

If you add something like

    if (!empty($input)) {
      var_dump(count($input, COUNT_RECURSIVE));
      die();
    }

to \Drupal\Core\Form\FormBuilder::buildForm after $input has been build and then submit the form you can get an indication if this is the problem.

Either way, we need better steps to reproduce this, because for most Views it works just fine. So for this to be an issue other than #1565704: Core interfaces can go over max_input_vars → , please provide steps to reproduce this with a View that has less than the max_input_vars but still won't save and doesn't give an error message.

Applied the patch in #37, unfortunately still runs out of memory. No time to test further I'm afraid.

Updated the IS, code looks good.

We were using 📌 Use 'From' email address for the sendmail POST user ID RTBC before, hence the working setup before updating, but that got reverted, so this is a new attempt at that but using the new helper method

So redid that patch to use the new helper method. Also redid the current logic for the default from address since the ?? doesn't do what the code hopes it does since default_mail will get set to an empty string.

Added logic for handling a given 'from' address

NB. this seemed to work previously, but looking at the old code, it was never supported, so I might be missing something

Lendude → created an issue.

We updated to 10.2.12 and one map displaying ~600 pins stopped loading, giving out-of-memory errors where previously it was loading fine. Tracked it down to this change. Something here makes the caching go crazy and loopy until \Drupal\Core\Cache\CacheTagsChecksumTrait::calculateChecksum runs out of memory. Not sure if this is the fault of core caching, search API caching or how this patch changed things but here is a patch reverting this change for people running into similar problems and don't need this functionality

Yeah that is more readable, changed it, and added a small comment

I see plenty of coverage for this method in \Drupal\Tests\path_alias\Unit\AliasManagerTest but none that tests the exception being thrown (as far as I can tell), so probably good to add a small test method there that tests this

All other 'delete' operations in core trigger a dialog as far as a little clicking around could show me, so cancel there just leaves you on the current page. This is also what the delete option in the Views list does. Wouldn't it make more sense/be more consistent to have the delete option in the View Edit screen also trigger a dialog?

Managed to create MR now, updated the IS a bit

This would show something like this in the UI:

I'm not sure if using $plugin->pluginTitle() in the selector is great, because there are no further explanations about what the options in the list are, it's probably best to be verbose there.

I would like to see it used in the admin summary, so that you can see in the Views UI which (if any) default plugin has been selected without having to open the edit dialog.

A use case I can see: If you have multiple fields in a row and only want to show the wrapper for that field if it has a value?

Since it would be up to the plugin rendering the row to do something with the information from the fields, it would be very hard to bubble this setting up to the row consistently.
That would need to be implemented in all Style and/or Row plugins individually, sort of like what the Table Style does with the empty_column setting.

So to me this is 'Works as designed'. The setting in the field should control that which it has control over: only that field.

But yeah, better wording might help, although the description is already pretty extensive: "Enable to hide this field if it is empty. Note that the field label or rewritten output may still be displayed. To hide labels, check the style or row style settings for empty fields. To hide rewritten content, check the "Hide rewriting if empty" checkbox."

larowlan → credited Lendude → .

Not 100% clear what you would want/expect to see but it sort of sounds like you want the exposed filter to do a POST request and not a GET request.

You can probably do that through a form alter hook, not sure how well it will play along with other parts of the View if you do that.

Tried to reproduce, and this seems to be working as designed. The option that is enabled in the screenshot and the steps is about hiding an empty field, which it is (the row is empty in the screenshot). What is not hidden is the empty row. There is nothing saying that this setting should bubble up and hide the whole row (if all fields are empty and all fields have this option enabled?). Right?

If you want to hide rows that don't have content in a certain field, that would need a filter.

Am I reading this correctly or is there something here that I'm overlooking?

I agree with #2, feel free to reopen this is if you feel this is a different issue 💬 Views UI - Parameter "view" for route "entity.view.edit_form" must match "[^/]++ Closed: works as designed

It's not available when the currently active display is an Entity Reference display, I do see the option if I use it on a Page display. Right? Or am I misreading?

Test-only job fails as expected https://git.drupalcode.org/issue/drupal-3420230/-/jobs/915521

Added some test coverage and simplified the merge statement

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

Sounds a little edge case to me, don't think it makes the 80% criterium, especially with token support thrown in. Might be more user friendly with a select element showing available languages

It would need some things I think:

• should only be shown on multi-lingual sites
• yeah validating if we have valid input sounds good, just hoping it falls back sounds bad
• tests
• a schema update
• an upgrade path for the schema change
• a test for the upgrade path

Lendude → created an issue.

larowlan → credited Lendude → .

Please search the Views and Views UI modules (and maybe others?) for 'Link display' and update all occurrences everywhere. Please make sure the changed sentences still make sense, not sure a find/replace is going to work everywhere

Did anybody ever look into moving this out of the pager section? I agree with @dawehner in #1 that this feel like a weird place for this.

Do we need to think about updating the machine name of the setting too? We now have a big difference between the name of the field in code and the name in the UI which can lead to more confusion but now from a DX perspective

Do we need to run the new label past some UI experts and make sure we all agree this is the best we can come up with, 'Destination' feels a bit vague to me.

📌 Increase or remove default textfield #maxlength=128 Needs review might solve this too

Feels similar to 🐛 Views RSS Feed Fields adds tag. Active and probably also caused by 🐛 Allow TimestampFormatter to show as a fully cacheable time difference with JS Fixed

Thanks for opening this, it reads like a duplicate of 🐛 Ellipsis in pager template fails accessibility tests Needs work so closing for new, feel free to reopen this if you feel this is addressing a different issue.

Well if there is a use for it, we can always undeprecate it (or in this case, never deprecate it in the first place), or if it needs a string version of this we should add a string version and put it in the pgsql module where this seems to belong (is PostgreSQL the exception here or is MySQL? How does SQLite handle this?)

Also, I think the general consensus is that int joins are more performant than string joins, so what performance impact does casting everything to a string have? We should check.

How is this handled outside Views? It's not like Views is the only place you can make joins. Is there maybe some method we could/should add to the database drivers that handles this?

I would like to see test coverage added that actually shows we need this, so something that would fail without this fix. All the modified test coverage changes only show that the ':text' is added and this doesn't break existing queries.

Apparently doing a merge to resolve conflicts makes spellcheck croak.... 🐛 Spell-checking job fails with "Argument list too long" when too many files are changed Active , so needs a rebase

Did some clean up, no idea what spellcheck is complaining about.

I think we still need an upgrade path test for this

Rereading this, feels like a duplicate of 🐛 Switching on aggregation generates fatal "Column not found: 1054 Unknown column" SQL error when using multi-column Fields Needs work can somebody check if the work there helps fix their issues?

Nice find, fix looks good, but we should probably add a test for this, shouldn't be too hard to add something to \Drupal\Tests\views\FunctionalJavascript\ClickSortingAJAXTest I think?

Tried to reproduce:
* Clean Umami install
* Enabled Comment module
* Checked key_value table, comment entry was there
* Uninstall comment module
* Checked key_value table, comment entry was gone

Can you reproduce this on a clean Drupal install?

Added some more modules

This test printed output: int(37)

Checked all modules that implement hook_views_data, think this get all of the ones that don't need extra handling or just reuse existing plugins...I think

Forgot to untag

The "Views Default Argument: Entity Field Values" module already provides this functionality and much much more, see https://www.drupal.org/project/views_arg_entity_field →

I don't think we should be adding something so Node specific to core. The problem with these default arguments is that they are not filtered and all of them always show up on all fields even when totally inappropriate (filtering a date field by current node type when viewing a taxonomy term anyone?), so I'd prefer the list we have by default in core to stay as small as possible. Yes default arguments are great tools but having a generic solution like the module provides is, I think, a better way forward than adding individual fields to core.

This checkbox comes from \Drupal\Core\Field\Plugin\Field\FieldFormatter\EntityReferenceLabelFormatter, Views just reuses the field formatter, so not sure if this is something Views should be handling or the FieldFormatter itself.

@larowlan is 📌 Fall back to 'edit-form' as default $rel in EntityBase::toUrl() Fixed the change you are referring to? The formatter seems to catch the new exception thrown there so not sure.

@daffie the visual representation leaves a little to be desired to make it clear, but if you look at the top level Postgres build you can see that the normal test runs pass and only the test-only run fails, as we would hope

https://git.drupalcode.org/issue/drupal-3415118/-/pipelines/79102

Hiding patches

PostgreSQL test-only fails as expected: https://git.drupalcode.org/issue/drupal-3415118/-/jobs/653536

So this looks good to me

Nice, moved to a MR and lets see if the test run better there, works fine locally

Nice find, we need a test for this, I've already written one in 🐛 A number of Views combined fields filter operators are currently pointless to an end user Needs work that just hasn't landed yet.

Probably easiest to grab the REGEX test from that and add it here and see if it indeed fails on PostGreSQL

Can we use \Drupal\views\Plugin\views\join\CastedIntFieldJoin for this and avoid adding pgsql specific code to base plugins?

Moved code to a MR to provide some feedback

This seems to have been fixed in #2899248: Don't reimplement validation rules for workflow state add/edit forms in ::copyFormValuesToEntity → where this was added

if (!$form_state->isValidationComplete()) {
      // Only do something once form validation is complete.
      return;
}

Am I reading that right or is there still something broken here?

#3 and #9 outline nicely why we can't just use the field's labels, because the same field can have multiple sets of labels. So the goal set out in the title I don't think we can do.
The option set out in #9 where we provide fields in the boolean filter where you can configure this could work, but does that really offer much more value over just using a grouped filter for this? I don't think it does, so "Won't fix" for me, unless we can think of way to determine the values we should be using here

Did a check:
Investigation: See for example https://www.drupal.org/project/drupal/issues/2404603#comment-9576291 → for the test in an earlier form, where the two parameters made sense (they contained different things)
Conclusion: leftover code from previous iterations of the test that never got committed in that form and was missed in review

Added to the IS

larowlan → credited Lendude → .

Custom Date field

What does that mean? Is it a date field added through the UI or a base field or some type of custom date field you've build?

Per #4, please add some steps to reproduce this on a clean Drupal install, that would really help

Check all your Views that have a taxonomy term filter and try to resave them manually. Does that work? Are there any broken handlers on them? After resaving manually, does the update run?
All the update does if resave the View if it has such a taxonomy term filter, and apparently that is failing, usually this is due to a pre-existing problem in the View and has very little to do with the actual update. The update just surfaces the issue.

Can you provide steps to reproduce this with just Drupal core? The stack trace is using paragraphs

We actually have a Views test that uses a batching action: \Drupal\Tests\views\Functional\UserBatchActionTest::testUserAction

Not sure if this issue can be replicated with just core, but if it can, it would be good to add a test at some point.
I agree that we don't need to add coverage in this issue, fixing the regression is more important, but I would suggest making a follow up issue to expand the coverage in that test a bit, it's very minimal as it stands.

We really need some steps to reproduce this if we are going to keep this open. Is this still relevant? If so, when?

Can we track down what changed here in 10.2? Neither files touched by the current fix were changed recently, it would be nice to track down what caused this so we can maybe prevent other side effect and make sure we are on the right track with fixing this here and not just band-aiding a much bigger issue

I tried to rebase some stuff into 11.x but no luck ¯\_(ツ)_/¯

Lendude → changed the visibility of the branch 2829178-11.x to hidden.

Lendude → changed the visibility of the branch 10.1.x to hidden.

The current fix changes way more than just the row plugins, as pointed out in the IS, this also affects things like pagers. This isn't necessarily a bad thing but we do need to check if we are not hiding things we don't want hidden and maybe expand the test coverage to also cover other plugin types.

For #39, I don't think plugins that have display_types = {"normal"} set need to change anything, it just becomes redundant, because we are adding display_types = {"normal"} for all plugins that don't set anything. I think....

But yeah changing this in ViewsPluginManager feels a little bit more invasive than it did when this was opened, I feel we do need a clearer list of all affected plugin types if we do it in ViewsPluginManager

Pushed the cleanup

Checked and indeed, as @longwave pointed out in #3222251: [November 8, 2021] Replace all isset constructs with the null coalescing operator → , $link_html is never used other than for setting $final_html, so we might as well just use $final_html always.

Not a bug, just a bit of cleanup really.

Fixed the tests, did some manual testing, updated the IS

Moved to MR, lets see what tests fail now and fix them

I'm not sure the name is even available in the database for the sake of sorting

It is not, it's config after all. So moving to a feature request. Would be nice to be able to do......

Checking old issues for the Bug Smash initiative.

Tested this, and indeed, with the new email permission this seems to work, without you get a 404 is you try to show the email.

If there was another issue here, please feel free to reopen this

Going through old issues for the Bug smash Initiative.

Yes, this is messy. The underlying problem is #2917814: Views table style info never gets updated when changing/adding fields → so going to close this as a duplicate of that. If you feel this is talking about a different issue, please feel free to reopen this.

Going through old issues for the bug smash Initiative

Tried to reproduce this and can't anymore. This seems to have gotten fixed. If this is still a problem for anybody, please feel free to reopen this issue

Looking at this again, this is a "works as designed" for me. The combined field filter is not meant to work for all possible use cases. More complex search questions should be handled by using something like Search API and Facets.

Going through old issues for the Bug Smash Initiative

I still see this happening, but it's not blocking anything as far as I can tell, only throwing some errors on the Preview. Once you save the View, everything is fine (until you switch between types again).

Going through old bugs for the Bug Smash Initiative.

#4 Sounds like this might have been solved by other issues.

If this is still an issue please feel free to reopen this

Going through old issues for the Bug Smash Initiative

I would expect you to always get just 50 results, so the 200 sound strange. Should this still be a problem, please provide some steps to reproduce your issue.

Going through old issues for the Bug Smash Initiative.

🐛 Database reserved keywords need to be quoted as per the ANSI standard Fixed will have fixed this hopefully, if this is still an issue for somebody please feel free to reopen this.

Going through old issues for the Bug Smash Initiative.

I'd say this is by design, the field is hidden, which in Views lingo means it shouldn't be rendered, so it's not passed on to the rendering layer.

If you feel that this is something that is needed we could reopen this as a feature request, but not sure this would fit the 80% case.

Going through old issues for the Bug Smash Initiative.

Now you get a fairly clear message: "No valid values found on filter: Content: Something." on the Views UI and the View stop displaying results until you fix it.

We could add a config dependency to this field, but as always, the problem then becomes, what is the correct action to take? Just stop filtering on that value? That might lead to a information disclosure. If this was the only value filtered on, it's not doing anything, should we remove it?

So, I think, what happens here is right, make the user fix the View and don't show anything until they do.

Other thoughts?

Going through old issues for the Bug Smash Initiative.

Search API seems to have found a way to work without this, so currently I don't think we should change this until we have a clear idea of what/if it is blocking.

Going through old issues for the Bug Smash Initiative.

Some work was done for checkboxes here #2651102: Using checkboxes for exposed filters results in zero rows displaying → also still some work to be done for dates in 🐛 Views exposed group filter (with option allow multiple selections) generates notices when using date and date range fields Needs work but not getting this error anymore so that part seems to have been fixed

Going through old bugs for the Bug Smash Initiative

The rearrange works now, as far as I can tell, the styling isn't great, but if you switch off javascript in Claro and then view the Views UI, this is not the first thing you trip over by far.

So if we start caring about the layout of nojs, I think that should be a whole new thing.

Closing this for now

It's a deprecation message.

@trigger_error('The extension.list.module service must be passed to ' . __NAMESPACE__ . '\SystemController::__construct. It was added in Drupal 8.9.0 and will be required before Drupal 10.0.0.', E_USER_DEPRECATED);

Not passing extension.list.module to the constructor is deprecated, so like the message says, before to mark your module ready for D10, you need to pass this to the constructor if you are extending SystemController, or it will break....which it did

So that module probably extends SystemController (I didn't check) and they need to update their code, so let's move the issue to that queue

Going through old issues for the Bug Smash Initiative.

This still happens, closing this as a duplicate of 🐛 Views doesn't parse twig when there are no tokens to replace Needs work . Technically that issue should be a duplicate of this, but more activity there so lets close this one.

Tried to reproduce this on a clean Umami install but not problems there as far as I can tell

Steps done:
* Add a new Vocabulary
* Add permissions for this Vocabulary to the Editor and Author roles
* Check the config for these roles in config sync (/admin/config/development/configuration/single/export) and make sure the permissions are there
* Delete the Vocabulary
* Check the config for these roles in config sync (/admin/config/development/configuration/single/export) and make sure the permissions are gone

Am I missing steps? Or does this only break when using Drush?