Exclude profiles from the People landing page listing #139
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While a landing page with _all_ the staff was an extreme case, it was also making a dozen-or-so queries per person, which is un-good
Profiles _could_ be managed as tree-items, below categories, but I don't think the categories are mutually-exclusive, so it seems to make more sense to manage all the profiles in one place, and just exclude them from the user-facing listing
gdelavil
reviewed
Jul 16, 2024
There was a problem hiding this comment.
@haydngreatnews I'm missing the ticket and context, I think. Why are you excluding profiles?
gdelavil
reviewed
Jul 16, 2024
|
Unit tests are almost entirely for the original pre-SL implementation. We left them around for the ideal case of us having time to update them |
|
Description updated to add context |
gdelavil
approved these changes
Jul 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a migration step, we have moved the existing people pages, so that we're relying on parent-child relationships for category pages, rather than views mounted over the top of page paths.
However, people can belong to more than one category, for example Brian Kernighan belongs on both the Executive Committee and the (Past) Staff pages, so we can't have the profiles existing under the specific categories.
As such, the person-profiles are set up to be homed directly underneath the PeopleLandingPage, but should not be listed alongside the PeopleCategoryPages that do belong there. While we could restrict the child listing by type, it seems safer (in case we eventually add another allowed child-type for example) that we exclude the profiles from that listing, as they already have a suitable display location as grandchildren
Also do some prefetching to optimise pages with a larger number of people