Needs review
Project:
Views Alpha Pagination
Version:
8.x-2.x-dev
Component:
Miscellaneous
Priority:
Normal
Category:
Plan
Assigned:
Unassigned
Reporter:
Created:
23 Oct 2016 at 15:53 UTC
Updated:
3 Jun 2019 at 22:23 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
elusivemind commentedThe short answer is yes. There is no timeframe, but one is definitely planned. I would say maybe sometime by the first of the year.
Comment #3
elusivemind commentedComment #4
markhalliwellNot really sure why this is postponed.
Anyone could technically start working on a port of this since views is in core now.
However, a 8.x-2.x branch should probably be created to help commit changes as needed.
I'll also create a related contrib_tracker issue that points here.
Comment #5
markhalliwellComment #6
biswajeetparida commentedAny Update ?
Comment #7
f2boot commentedHere is a first attempt of porting alpha_pagination to D8 (patch against initial 8.x-2.x)
Please review thoroughly
- I am not sure we need so many dependency injections
- I wonder if the list of fields for index is correct
- Cache uses "default" table, should-it use the "cache-data" table ?
Comment #8
matsbla commentedI, just just wanted to make a quick check if it worked!
I tried to filter on a letter and got this error:
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '***CURRENT_USER*** AND ***CURRENT_USER*** <> 0 AND ***VIEW_OWN_UNPUBLISHED_NODES' at line 2: SELECT node_field_data.langcode AS node_field_data_langcode, users_field_data_node_field_data.langcode AS users_field_data_node_field_data_langcode, node_field_data.nid AS nid, users_field_data_node_field_data.uid AS users_field_data_node_field_data_uid FROM {node_field_data} node_field_data INNER JOIN {users_field_data} users_field_data_node_field_data ON node_field_data.uid = users_field_data_node_field_data.uid WHERE (node_field_data.status = 1 OR (node_field_data.uid = ***CURRENT_USER*** AND ***CURRENT_USER*** <> 0 AND ***VIEW_OWN_UNPUBLISHED_NODES*** = 1) OR ***BYPASS_NODE_ACCESS*** = 1) ORDER BY node_field_data.changed DESC; Array ( ) i Drupal\alpha_pagination\Plugin\views\area\AlphaPagination->getEntityIds() (linje 752 i /var/www/html/modules/alpha_pagination/src/Plugin/views/area/AlphaPagination.php).
And I came to a page not found.
Comment #9
f2boot commentedI am not sure problem is related to D8 port.
This part of the code, if I get it right, does 3 things:
- bypass "SUBSTRING" by adding " 1 OR " before it
// If we are dealing with a substring, then short circuit it as we are most
// likely dealing with a glossary contextual filter.
- remove "LIMIT" by removing last line of the query if it starts with "LIMIT"
// Evaluate the last line looking for anything which may limit the result
// set as we need results against the entire set of data and not just what
// is configured in the view.
- decode some html leftover (& > < )
// Construct the query from the array and change the single quotes from
// HTML special characters back into single quotes.
Does showing SQL request (see admin/structure/views/settings) helps to guess where the problem can come from ?
Thanks
Comment #10
markhalliwellThis doesn't appear to be based off the 7.x-2.x code (which fixed some of the above errors mentioned above).
Comment #11
f2boot commentedHere is a patch based off the 7.x-2.x code.
I must say I have been strugling to reach this and I would be very surprised to have it all right (althought it is working on my dev env)
I have tried to stick to the global structure but I am new to D8 and symphony so I have probably missed a few things about services and dependencies injection.
Comment #12
markhalliwellWow, impressive work @f2boot, thanks!!
I'll have to take a look at this later when I can actually spin up a site instance as it's a lot to get through.
Comment #13
f2boot commentedThank you for considering this porting patch.
This one should be better:
- two bugs less
- added relationship so we can get fields from entity_reference (needs testing, this is a first attempt)
Comment #14
matsbla commentedI tested #13, thanks for this great work!
It seem to work pretty good for node/content, however I do get this warning on the log:
Notice: Undefined property: stdClass::$base_field in Drupal\alpha_pagination\AlphaPagination->getEntityIds() (line 539 of /var/www/html/modules/alpha_pagination/src/AlphaPagination.php)
However for terms I can choose a "title" in the config option "View field to paginate against", even though terms doesn't have the field.
I tried to choose "name", however the result is not pagianated.
Then I tried to add "All" item position to "before".
When I visit my view I get "The website encountered an unexpected error. Please try again later."
In my log I got this error:
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal.taxonomy_term__name' doesn't exist
And this warning:
User warning: setDisplay() called with invalid display ID "page". in Drupal\views\ViewExecutable->setDisplay() (line 802 of /var/www/html/core/modules/views/src/ViewExecutable.php)
Comment #15
f2boot commentedHi
Nice to know someone is testing this.
Don't know how much these bugs are related.
Here is a new patch (and interdiff) that should solve the latest warning you get and may be other ones too.
This list of fields you can chose of as "View field to paginate against" is not generated very cleanly. It isn't either in the D7 version if I remember well. So I guess this module is not ready for everyone yet.
I haven't tried this module with taxonomy terms. Must be something to tweak somewhere since term name is rather in taxonomy_term_field_data table.
Comment #16
markhalliwellSorry @f2boot, I've just had a lot on my plate lately.
I will try to get to this soon. Feel free to ping once a week until I do though (it helps keep this on my d.o dashboard).
Comment #17
matsbla commented@f2boot
It did solve the warnings.
As views is now and core and more entities use views to display lists, I think it is quite important that D8 version of this module does not make assumptions about fields and entities.
However I think it would be great to get a D8-dev version soon so that we could start dig into other issues in separate issues
Comment #18
f2boot commented@matsbla,
I do share your two points.
Straightening the list of compatible fields is a bit out of my scope of competence though. I could contribute to dig it in if I know there are other interested users (cf #15)
By the way, I haven't gone into benchmarking for alternatives to alpha pagination since Views has moved to core and caching improved.
Might be worth checking before putting more time and brainware on this module.
Comment #19
daniel kulbeI could not apply the latest patch against the 8.x-2.x branch, so here is an version which is.
I left out the demo view. If you want to provide one, this should go into
config/optionalas alpha_pagination.view.yml ... may be disabled by default...Comment #20
markhalliwellThe patch in #19 is half the size and mostly just due to massive deletes. I don't see how that is a "version" of the work that has been done prior.
@f2boot, I just tried applying your patches as well and I'm not entirely sure what's going on here.
Can you please rebase your patches off the 8.x-2.x branch.
Once there is an applicable patch, I think I'll just go ahead and commit it so newer (smaller) issues/patches can be created to clean up the rest.
Comment #21
markhalliwellComment #22
daniel kulbeOhh, sorry ... copying files from on dir to an other messed up. Updated the patch
Comment #23
markhalliwellOh, I figured out why it wasn't applying. It's because the 8.x-2.x branch is technically based on 7.x-1.x, not 7.x-2.x... doh!
Fixing that now, sorry @Daniel Kulbe. I'd rather just merge 7.x-2.x into 8.x-2.x first to keep git history rather than a standalone patch that squashes everything together. FWIW, you did help, thank you! :D
Comment #25
markhalliwellOk, I've gone ahead and committed #15 to 8.x-2.x.
Setting this issue to CNR so people can review.
If there are bugs, please create new issues against this branch.
Once we're satisfied that it's relatively stable, we can close this issue and create a full release.
Comment #26
matsbla commentedOkay great!
I've opened two:
#3058296: Broken/missing handler
#3058297: Keep styling to a minimum
Comment #27
matsbla commentedAlso created this one #3059234: Alpha pager title field creates error or leads to a page not found