Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
When creating a view with a page display, the page view path does not accept an integer between 1 and 99999 as path.
Higher values are working.
Proposed resolution
Remaining tasks
User interface changes
API changes
Beta phase evaluation
Issue category | Bug because this is a regression from D7 |
---|---|
Issue priority | Normal because this is an isolated case. |
Prioritized changes | The main goal of this issue is to fix a bug. |
Disruption | None |
Comment | File | Size | Author |
---|---|---|---|
#22 | list-2015.png | 117.38 KB | koence |
#19 | interdiff-views_page_displays_16-19.txt | 1.69 KB | geertvd |
#19 | views_page_displays_do-2474471-19-complete.patch | 4.66 KB | geertvd |
#19 | views_page_displays_do-2474471-19-test.patch | 4.61 KB | geertvd |
#9 | views_page_displays_do-2474471-9.patch | 2.86 KB | pjbaert |
Comments
Comment #1
StryKaizerComment #2
geertvd CreditAttribution: geertvd at XIO commentedLet's see what the testbot says.
Note I removed the todo referencing #2351379: [meta] Define, then support exact use cases for link generation and storage since it looks like it was decided in #2416843: Decide if a Url::fromPath() method and/or a Url::fromUri() scheme other than 'base' or 'user-path' is wanted to support contrib/custom modules not to create the
Url::fromPath()
method.Comment #3
geertvd CreditAttribution: geertvd at XIO commentedWe'll probably need tests for this
Comment #4
dawehnerNice bug. I'm curious whether we should allow any integer values. Just curious what is the usecase for that?
Should we throw a validation error?
Comment #5
dawehnerNice bug. I'm curious whether we should allow any integer values. Just curious what is the usecase for that?
Should we throw a validation error?
Comment #6
StryKaizerTested patch in #2, works as expected.
Attached you can find a test for this issue, and a patch with test + fix from #2
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedThis seems a sane approach to me, but I don't think dawehner's question in #4 (on the usecase) was answered?
Tests were added, but we also need a beta evaluation here.
Some nitpicking:
This should be a single line.
Why should this be $view1 if it is the only view?
Comment #9
pjbaertI processed the remarks @pjonckiere mentioned in #8.
I'll add a beta evaluation later today.
Comment #11
pjbaertWell... I added an interdiff.patch file. Silly me.
I would still like a review of my latest patch.
Comment #12
pjbaertAdded the beta evaluation.
I certainly think there is a usecase for this.
For example; we should be able to add the date/year value in our path.
Comment #13
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedAgreed with #12. Also, like the beta evaluation now specifies, this is a regression (I tested with views 7.x-3.10).
The patch looks good now. IS looks a bit short but sufficient to me and we have a beta evaluation.
When committed, we should comment in #2423913: Leading slash in link fields and views has different UX that we added a todo in this patch.
Comment #14
dawehnerSorry here are just a couple if nitpicks...
It would be great to maybe also expand the test coverage in
\Drupal\views\Tests\Plugin\DisplayPageWebTest
Nitpick: Should be Contains \....
let's make it public
Let's use a space after 'foreach'
Comment #15
Anonymous (not verified) CreditAttribution: Anonymous at XIO commentedComment #16
geertvd CreditAttribution: geertvd at XIO commentedRewrote the test so we don't test the wizard but rather start from a yml file as suggested by dawehner via IRC.
This makes 1, 2 and 3 from #14 not applicable anymore since that file has been removed.
Comment #17
dawehnerLooks fine for me!
$path should be quickly documented :(
Comment #18
geertvd CreditAttribution: geertvd at XIO commentedha views_page_displays_do-2474471-16-test.patch should be failing though.
Comment #19
geertvd CreditAttribution: geertvd at XIO commentedFixed feedback in #17 and now we should have an actual failing test.
Comment #21
geertvd CreditAttribution: geertvd at XIO commentedbump
Comment #22
koence CreditAttribution: koence at XIO commentedList with integer between 1 and 99999 can be added.
Result of test added as attachment (screenshot).
Comment #23
webchickCommitted and pushed to 8.0.x. Thanks!
Comment #24
webchickSaving credit.