Problem/Motivation

If there are multiple contextual filters and we do not provide a value for all of them then the view is not processed correctly

Steps to reproduce

Proposed resolution

If no value has been provided for an argument use the default value for displaying all results.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#13 3382310-13.patch5.12 KBhchonov
#3 3382310-3.patch1.78 KBmajid.ali
#2 3382310-2.patch1.82 KBhchonov

Issue fork viewsreference-3382310

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

hchonov created an issue. See original summary.

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
Status: Needs work » Needs review
StatusFileSize
new 3382310-2.patch1.82 KB
majid.ali’s picture

majid.ali commented
StatusFileSize
new 3382310-3.patch1.78 KB

Re-rolling #2 for updated 8.x-2.x-dev branch.

scott_euser’s picture

scott_euser commented
Status: Needs review » Needs work

Thanks for the work on this! A few things:

  1. Can this be converted to a merge request to make it easier to review and comment please?
  2. The last unset($argument) is actually deleting the argument is it not? Since it is passed by reference.
  3. This needs test coverage, which can be added to ViewsReferenceSettingsTest.php (there is an existing method ::testArgument())

sleitner made their first commit to this issue’s fork.

sleitner opened merge request !61

sleitner’s picture

sleitner commented
Status: Needs work » Needs review

PHPStan issues are resolved, please review

scott_euser’s picture

scott_euser commented
Status: Needs review » Needs work

Great thank you for the progress on this! Just needs the tests adding now as per my comment in #4. Thanks!

sleitner’s picture

sleitner commented
Status: Needs work » Needs review

Test extended

sleitner’s picture

sleitner commented

Any news on this issue?

scott_euser’s picture

scott_euser commented

This looks good for me, thank you for adding the test coverage! I have not actively used arguments in any of my projects, so I am hoping one of the other followers of this issue can confirm they are successfully using this MR patch and RTBC it.

scott_euser’s picture

scott_euser commented

If it gets stuck as a result, I'll set a reminder to try to do more thorough manual tests eventually myself but I have a bit of a backlog at the moment.

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
StatusFileSize
new 3382310-13.patch5.12 KB

Somehow during the development the usage of the unserialized data broke. I fixed this now with https://git.drupalcode.org/project/viewsreference/-/merge_requests/61/di...

I am also attaching a static patch of the MR.

scott_euser’s picture

scott_euser commented
Status: Needs review » Needs work

Thanks for the work on this! Test coverage is failing though it seems.