Ticket #160 (closed enhancement: fixed)

Opened 3 months ago

Last modified 4 weeks ago

Cruft-Free pagination

Reported by: anonymous Owned by: sidtheduck
Priority: low Milestone: 1.0
Component: General Version:
Severity: minor Keywords:
Cc:

Description

Currently, Plogger does not use the cruft-free option for pagination (see here). I think this should be utilized for consistency in URL generation.

Attachments

cruft_free_pagination.patch (13.9 kB) - added by sidtheduck 4 weeks ago.
updated_cruft_free_pagination.patch (16.7 kB) - added by sidtheduck 4 weeks ago.
another_updated_cruft_free_pagination.patch (17.5 kB) - added by sidtheduck 4 weeks ago.
final_cruft_free_pagination.patch (17.4 kB) - added by sidtheduck 4 weeks ago.

Change History

  Changed 3 months ago by sidtheduck

Same thing with search function.

I did think of something that may be a problem for cruft-free pagination and searches - What happens if a collection or album is named "page" or a collection named "search"? Should I worry about this? I assume the same thing would happen if you had a picture named "sorted" too.

  Changed 3 months ago by sidtheduck

  • milestone set to 1.0

  Changed 8 weeks ago by sidtheduck

Thinking back on this, I'm not sure that we need to have Cruft-Free URLs on search, but we should definitely provide it for paging.

  Changed 5 weeks ago by sidtheduck

  • keywords has-patch needs-testing added

I've attached a patch that should add cruft-free URLs for pagination (except at the "search" level which still uses the old style). I've tried testing on my end, but would like a second opinion for tests.

follow-ups: ↓ 6 ↓ 7 ↓ 8   Changed 5 weeks ago by kimparsell

I've applied the patch to my working copy, which also includes the patch for proper 404 pages.

In my test gallery, when I go into the Sunsets album and click on page 2, I'm getting a 404 error.

Question: Would it be better to drop the /plog_page/ and just have it read /sunsets/page2/ or sunsets/2/?

In the admin, I don't have enough collections or albums to warrant a second page on the Manage tab, but when I go to Feedback, I'm presented with the following errors at the top:

Warning: Missing argument 5 for generate_pagination(), called in /home/xxxxxx/public_html/plogger/gallery/plog-admin/plog-feedback.php on line 130 and defined in /home/xxxxxx/public_html/plogger/gallery/plog-includes/plog-functions.php on line 1127

Notice: Undefined variable: items_on_page in /home/xxxxxx/public_html/plogger/gallery/plog-includes/plog-functions.php on line 1139

Warning: Division by zero in /home/xxxxxx/public_html/plogger/gallery/plog-includes/plog-functions.php on line 1139

I change the number of entries/page in the dropdown to 5, and I still get the same errors, and no page numbers are listed to navigate to the next page.

Side note: There is no dropdown box on the Manage page to set the number of entries/page like there is on the Feedback page.

in reply to: ↑ 5   Changed 5 weeks ago by ryanduff

Replying to kimparsell:

Question: Would it be better to drop the /plog_page/ and just have it read /sunsets/page2/ or sunsets/2/?

I agree that it should be simple... something like /collection/gallery/ for the first page and /collection/album/2/ /collection/album/3/ etc for pages. No prefix should be required, but the structure should be able to be chosen by the user.

in reply to: ↑ 5   Changed 4 weeks ago by sidtheduck

Replying to kimparsell:

In my test gallery, when I go into the Sunsets album and click on page 2, I'm getting a 404 error.

I'll check this out. It was working on my installation with the 404 patch as well.

Replying to kimparsell:

Question: Would it be better to drop the /plog_page/ and just have it read /sunsets/page2/ or sunsets/2/?

I can definitely set it up this way. However, it means that people cannot name their collections, albums, or pictures with a pure number (i.e. 10.jpg or 3 for collection name). Should we add additional checks to make sure people can't name collections, albums, or pictures with a pure number to make sure there are no conflicts?

Replying to kimparsell:

In the admin, I don't have enough collections or albums to warrant a second page on the Manage tab, but when I go to Feedback, I'm presented with the following errors at the top:

I noticed this too after I added my patch. I've already started to work on the Feedback section and will update the patch when I finish the code.

Replying to kimparsell:

Side note: There is no dropdown box on the Manage page to set the number of entries/page like there is on the Feedback page.

I agree, the Manage section should have a pulldown like the Feedback section. I'm also wanting to work on Manage and Feedback to get away from the forms being sent using $_GET variables and instead use $_POST variables. If you're deleting or moving a large number of images, some people may hit the maximum character count of a URL for their server if we're passing large arrays of form information using $_GET URL strings. But this could wait until 1.1 as well.

Replying to ryanduff:

. . . but the structure should be able to be chosen by the user.

Do you mean have an option for the user to have a "plog_page" structure or a non-plog_page structure? Maybe this option could be for 1.1 and may be the workaround needed if people want to name their collection, albums, or images using pure numbers?

Thanks for testing!

Changed 4 weeks ago by sidtheduck

in reply to: ↑ 5   Changed 4 weeks ago by sidtheduck

Replying to kimparsell:

I've applied the patch to my working copy, which also includes the patch for proper 404 pages. In my test gallery, when I go into the Sunsets album and click on page 2, I'm getting a 404 error.

I checked the code and realized I left out the most important part (because it was within the resolve_path function that also had 404 code within it). I added it to the patch and re-uploaded so you can test functionality. I'm still working on the simpler paging style and the feedback paging section. I'll upload the new code when I've had a chance to finish and test myself.

Changed 4 weeks ago by sidtheduck

  Changed 4 weeks ago by sidtheduck

I've uploaded an updated_cruft_free_pagination.patch that removes the 'plog_page' from the cruft-free URLs and just uses the page number. It also fixes the issue with the Admin -> Feedback section (but still does not have the finalized pulldown for items per page in the Admin -> Manage section). Test it out and let me know.

It still should allow collections and albums like 10.28 or 2008.01.05 without having problems, but if you have real round numbers like 2008 it will have issues with the cruft-free pagination.

Changed 4 weeks ago by sidtheduck

  Changed 4 weeks ago by sidtheduck

I uploaded another_updated_cruft_free_pagination.patch that attempts to work better with pagination and albums. This patch checks the name first (of collection, album, etc.) before checking for pagination, so if there is a conflict, it defaults to the actual named item before it checks for pagination. Again, the only issue is if a collection name, album name, picture name, or picture caption are a real, round number that is also the same number as a page of your collection or album, then the pagination link will direct the user to the named item instead of the page.

So, named items like 2008 should be safe (since most galleries will not have 2008 pages to them), but named items like 3 may have issues. I see this issue could conflict if people add single numbers to their picture captions in order to manually force an order to display the images (instead of date-uploaded or filename, etc.)

Changed 4 weeks ago by sidtheduck

  Changed 4 weeks ago by sidtheduck

Added final_cruft_free_pagination.patch file after discussing options with Ryan Duff. The only way to achieve a non-conflicting paging system is to use the format: /collection/album/page/3/

It is working fine on my server with this final patch, but I would like at least another pair of eyes on it.

  Changed 4 weeks ago by kimparsell

I've just applied this patch and the 404 patch to a clean SVN checkout (562), and uploaded the pages to my server. I've checked the one album that can be split into 2 pages, and the cruft-free pagination is working fine now. The errors are gone on the Manage Feedback page in the admin section, and the pagination is working properly there was well.

I also double-checked the 404 function with these changes, and there was no impact on it whatsoever.

  Changed 4 weeks ago by sidtheduck

  • keywords has-patch needs-testing removed

Fixed with final patch committed in r563.

  Changed 4 weeks ago by sidtheduck

  • status changed from new to closed
  • resolution set to fixed
Note: See TracTickets for help on using tickets.