Allow users to filter albums by contributors #48

2024-01-14T21:29:30+01:00

Aaron commented

2024-01-14 21:29:30 +01:00

Refactor ViewPhotoAlbum a little to clean up the code
Add a method to enumerate contributors
Add a method to safely filter by contributor
Add a UI for users to select an available filter
Carry filter over to photo page (e.g. ?by, analogous to ?in)
Take active filter into account when generating previous/next photo url
Carry filter over to download page
Test things thoroughly

- [x] Refactor ViewPhotoAlbum a little to clean up the code - [x] Add a method to enumerate contributors - [x] Add a method to safely filter by contributor - [x] Add a UI for users to select an available filter - [x] Carry filter over to photo page (e.g. `?by`, analogous to `?in`) - [x] Take active filter into account when generating previous/next photo url - [x] Carry filter over to download page - [x] Test things thoroughly

filter_active.png

49 KiB

filter_inactive.png

59 KiB

Aaron added 6 commits 2024-01-14 21:29:30 +01:00

ViewPhotoAlbum: move some logic into new prepareHeaderBox method 08cdbfe7b6

ViewPhotoAlbum: simplify tag handling in getAlbumButtons 25b9528628

ViewPhotoAlbum: use Tag::getUrl instead of fumbling with $_GET['tag'] 7cdcf8197c

Tag: add getContributorList method 16683d2f1f

ViewPhotoAlbum: add method to filter by id_user_uploaded 6a25ecec23

AlbumButtonBox: add way for users to select an album filter 99b867b241

Aaron marked the pull request as work in progress 2024-01-14 21:29:34 +01:00

Aaron added 1 commit 2024-01-14 22:17:40 +01:00

ViewPhotoAlbum: don't populate filter box if there are no album contributors 9c00248a7f

Aaron added 2 commits 2024-01-15 00:55:52 +01:00

Asset: combine getUrlFor{Next,Previous}InSet into one private method f33a7e397c

ViewPhoto: take filter into account for prev/next links d8c3e76df6

Aaron marked the pull request as ready for review 2024-01-15 00:56:50 +01:00

Aaron force-pushed refactor/viewalbum from 35cb6798c8 to 1f53689e4b

2024-01-15 01:01:08 +01:00

Compare

minnozz commented

2024-01-15 09:51:34 +01:00

Fixes #13

🚀 1

minnozz requested changes

2024-01-15 10:01:34 +01:00

controllers/ViewPhotoAlbum.php Outdated

						
				@@ -66,0 +50,4 @@

						$filters = [];

						if (!empty($contributors))

						{

							$filters[''] = ['id_user' => null, 'caption' => 'Show all photos', 'link' => $tag->getUrl()];

minnozz commented

2024-01-15 09:55:32 +01:00

Key label missing

Key `label` missing

Aaron commented

2024-01-15 11:27:23 +01:00

I initially didn't add it as it'd never be used. You're right, though, and it's better to just have it there for consistency.

Aaron marked this conversation as resolved

controllers/ViewPhotoAlbum.php Outdated

						
				@@ -74,3 +91,3 @@

						// Load a photo mosaic for the current tag.

						list($mosaic, $total_count) = $this->getPhotoMosaic($id_tag, $page, !isset($is_person));

						list($mosaic, $total_count) = $this->getPhotoMosaic($id_tag, $id_user_uploaded, $current_page, !isset($is_person));

minnozz commented

2024-01-15 09:56:38 +01:00

$is_person is never set

(At $employer we had the convention to always use !== null instead of isset when you expect the variable to exist to catch these kind of errors. empty() was also discouraged for this reason)

`$is_person` is never set (At $employer we had the convention to always use `!== null` instead of `isset` when you expect the variable to exist to catch these kind of errors. `empty()` was also discouraged for this reason)

Aaron commented

2024-01-15 11:28:27 +01:00

Yeah, I don't like this either. I've opted instead to explicitly declare it as a boolean before it's used.

Aaron marked this conversation as resolved

controllers/ViewPhotoAlbum.php

						
				@@ -201,0 +235,4 @@

						{

							$back_link = BASEURL . '/people/';

							$back_link_title = 'Back to &quot;People&quot;';

							$is_person = true;