diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8e2f8abd..1923f608 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -58,3 +58,66 @@ jobs: - name: Run tests run: python manage.py test website --settings=makeabilitylab.settings_test --verbosity=2 + + # Browser end-to-end tests (member-page "Load more"/"Load all", bio toggle, + # section nav). Kept in a SEPARATE job from `test` so the fast unit/integration + # signal stays fast and isolated — a slow or flaky browser run doesn't muddy + # "did the logic break?". Like `test`, this is report-only (it doesn't block + # the push or the deploy). Playwright + its browser live in requirements-dev.txt + # only, never in the production image. + e2e: + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:16 + env: + POSTGRES_DB: makeability + POSTGRES_USER: admin + POSTGRES_PASSWORD: password + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U admin -d makeability" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + env: + DATABASE_HOST: localhost + DATABASE_PORT: 5432 + DJANGO_ENV: DEBUG + + steps: + - uses: actions/checkout@v4 + + # Same ImageMagick/Ghostscript deps as `test` — the Talk fixtures the e2e + # tests create run Artifact.save()'s PDF->thumbnail path. + - name: Install system dependencies + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends imagemagick ghostscript libpq-dev + sudo cp imagemagick-policy.xml /etc/ImageMagick-6/policy.xml + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.13" + cache: pip + + - name: Install Python dependencies (incl. Playwright) + run: pip install -r requirements-dev.txt + + # Cache the downloaded browser keyed on the pinned Playwright version, so + # only the first run (or a version bump) pays the ~120 MB download. + - name: Cache Playwright browser + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: playwright-${{ hashFiles('requirements-dev.txt') }} + + - name: Install Chromium for Playwright + run: python -m playwright install --with-deps chromium + + - name: Run end-to-end tests + run: python manage.py test website.tests.test_member_e2e --settings=makeabilitylab.settings_test --verbosity=2 diff --git a/makeabilitylab/settings.py b/makeabilitylab/settings.py index 6089dd7d..1d2e3d88 100644 --- a/makeabilitylab/settings.py +++ b/makeabilitylab/settings.py @@ -72,8 +72,8 @@ ALLOWED_HOSTS = ['*'] # Makeability Lab Global Variables, including Makeability Lab version -ML_WEBSITE_VERSION = "2.8.2" # Keep this updated with each release and also change the short description below -ML_WEBSITE_VERSION_DESCRIPTION = "SEO: add a dynamic sitemap.xml and advertise it in robots.txt (#1252). /sitemap.xml is generated on each request from our querysets via django.contrib.sitemaps — static listing pages (home, people, publications, projects, awards, news), visible projects, people with a position, and news items — so it stays current with no maintenance. It emits the correct per-environment domain via RequestSite (no django.contrib.sites, no DB migration) and pins URLs to https so the sitemap lists canonical links rather than http URLs that 302-redirect. Validated on the test server: all 189 sitemap URLs return 200. The top-level static robots.txt (served directly by Apache, not Django) now points crawlers at the sitemap. Remaining one-time step: submit the sitemap in Google Search Console once it is live on production." +ML_WEBSITE_VERSION = "2.9.0" # Keep this updated with each release and also change the short description below +ML_WEBSITE_VERSION_DESCRIPTION = "Member pages: dynamic per-section 'Load more'/'Load all' backed by a new member_artifacts AJAX endpoint that renders the same snippet templates, with higher initial counts (8 projects / 6 papers / 6 videos / 8 talks; 4/3/3/4 on mobile via a CSS cap). Fixes the recent-projects ordering (now sorted by most-recent-artifact date, descending, instead of a set re-sorted by project start_date). Prolific members' papers switch from the 3-up card grid to a scannable vertical list; over-long bios get a Show more/less toggle. Adds a sticky section nav with scroll-spy, the person's name revealed once the heading scrolls away, and live (loaded/total) counts shown in both the nav and the section headings (which drop the old 'Recent' wording); plus a floating Back to top button. Covered by unit/integration tests and a Playwright browser e2e suite wired into a new report-only CI job (#1110)." DATE_MAKEABILITYLAB_FORMED = datetime.date(2012, 1, 1) # Date Makeability Lab was formed MAX_BANNERS = 7 # Maximum number of banners on a page diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 00000000..a7bb3f98 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,17 @@ +# Development / test-only dependencies. +# +# These are NOT installed into the production (or test/prod server) image — the +# Dockerfile installs requirements.txt only — which keeps Playwright and its +# ~120 MB browser out of the deployed stack. This file is used by the Playwright +# end-to-end CI job (.github/workflows/test.yml) and for running the browser +# tests locally: +# +# pip install -r requirements-dev.txt +# python -m playwright install --with-deps chromium +# python manage.py test website.tests.test_member_e2e \ +# --settings=makeabilitylab.settings_test +# +# Pin Playwright so the pip package and the installed browser build stay in +# lockstep (a mismatch makes `playwright install` download the wrong revision). +-r requirements.txt +playwright==1.60.0 diff --git a/website/static/website/css/member.css b/website/static/website/css/member.css index c327885e..deb5f711 100644 --- a/website/static/website/css/member.css +++ b/website/static/website/css/member.css @@ -360,38 +360,342 @@ /* ============================================================================= - LOAD MORE BUTTON (Future Enhancement) + "LOAD MORE" / "LOAD ALL" ARTIFACT CONTROLS (#1110) + ----------------------------------------------------------------------------- + Per-section controls below each grid: a filled primary "Load N more " + button and an outline secondary "Load all " button, side by + side. member-load-more.js un-hides the block, sets the exact counts in the + labels, swaps in a spinner while loading, and removes the block once + everything is shown. Sentence-case, no chevron — reads as action buttons. ============================================================================= */ -.load-more-btn { - display: block; - width: fit-content; - margin: var(--space-6) auto 0 auto; - padding: var(--space-2) var(--space-6); - background-color: transparent; - border: 1px solid var(--color-border); +.see-more-controls { + display: flex; + flex-wrap: wrap; /* stack on very narrow phones */ + justify-content: center; + align-items: center; + gap: var(--space-3); + margin-top: var(--space-5); +} + +/* CRITICAL: a class selector's `display` beats the UA `[hidden]{display:none}` + rule, so without these explicit guards the `hidden` attribute that + member-load-more.js toggles would be IGNORED — the controls (and the + secondary "Load all") would stay on screen even when there's nothing more to + load. That was the "projects button shows with no count and does nothing" bug + (#1110): Jon has exactly 8 projects, so the projects controls SHOULD be + hidden, but display:flex kept them visible showing the un-numbered default + label. Keep these. */ +.see-more-controls[hidden], +.see-more-artifacts[hidden], +.see-more-all[hidden] { + display: none; +} + +/* Shared button shape for both controls. */ +.see-more-artifacts, +.see-more-all { + display: inline-flex; + align-items: center; + justify-content: center; + gap: var(--space-2); + padding: var(--space-3) var(--space-5); + min-height: 44px; /* WCAG 2.5.5 touch target */ border-radius: var(--border-radius-sm); - color: var(--color-link); + font-family: var(--font-family-secondary); font-size: var(--font-size-sm); - font-weight: var(--font-weight-medium); + font-weight: var(--font-weight-semibold); cursor: pointer; transition: background-color var(--transition-fast), border-color var(--transition-fast), color var(--transition-fast); } -.load-more-btn:hover { +/* Primary "Load more" — filled accent (the main action). */ +.see-more-artifacts { + background-color: var(--color-primary); + border: 1px solid var(--color-primary); + color: var(--color-text-on-dark); +} + +.see-more-artifacts:hover { + background-color: var(--color-primary-hover); + border-color: var(--color-primary-hover); +} + +/* Secondary "Load all" — outline. */ +.see-more-all { + background-color: transparent; + border: 1px solid var(--color-border); + color: var(--color-link); +} + +.see-more-all:hover { background-color: var(--color-bg-surface); border-color: var(--color-primary); color: var(--color-link-hover); } -.load-more-btn:focus { +.see-more-artifacts:focus, +.see-more-all:focus { + outline: var(--focus-ring-width) solid var(--focus-ring-color); + outline-offset: var(--focus-ring-offset); +} + +.see-more-artifacts[disabled], +.see-more-all[disabled] { + opacity: 0.65; + cursor: default; +} + +/* The loading spinner (member-load-more.js injects fa-spinner fa-spin). Don't + animate it for users who've asked to reduce motion — the static icon plus the + disabled "Loading…" label still communicates the busy state. */ +@media (prefers-reduced-motion: reduce) { + .see-more-artifacts .fa-spin, + .see-more-all .fa-spin { + animation: none; + } +} + + +/* ============================================================================= + MOBILE ARTIFACT CAP (#1110) + ----------------------------------------------------------------------------- + On phones (<=576px, where every grid is a single column) each section shows + only a reduced set: 4 projects, 3 papers, 3 videos, 4 talks. The FULL desktop + count is still in the DOM — these rules merely hide the overflow while the + grid carries the .is-collapsed class. member-load-more.js removes that class + on the first "See more" tap, so the rest appears instantly with no network + request; only loads BEYOND the desktop count hit the server. + + TRADEOFF: this cap is pure CSS, so it applies even when JavaScript is off. + A no-JS phone visitor therefore sees only the reduced set and cannot expand + (the "See more" button stays hidden without JS). That's an accepted + limitation — the site already depends on JS for the citation popover, + video-age, etc., and there's no per-member full-listing page to fall back to. + The alternative (JS-applied cap) would avoid this but flash the full set + before collapsing on every mobile load. + + Keep the :nth-child thresholds in sync with ARTIFACT_MOBILE_PAGE_SIZES in + website/views/member.py. + ============================================================================= */ + +@media (max-width: 576px) { + .person-project-grid.is-collapsed > *:nth-child(n+5), /* keep 4 projects */ + .person-publications-grid.is-collapsed > *:nth-child(n+4), /* keep 3 papers (card grid) */ + .person-publications-list.is-collapsed > *:nth-child(n+4), /* keep 3 papers (vertical >6) */ + .videos-grid.is-collapsed > *:nth-child(n+4), /* keep 3 videos */ + .talks-grid.is-collapsed > *:nth-child(n+5) { /* keep 4 talks */ + display: none; + } +} + + +/* ============================================================================= + BIO "SHOW MORE" / "SHOW LESS" (#1110) + ----------------------------------------------------------------------------- + bio-expand.js clamps an over-long bio (more than ~3 lines) and inserts the + .bio-toggle button. It adds .is-collapsible to the bio text element (and + .is-collapsed while clamped); it manages the inline max-height for the + animation. These rules supply the transition and the bottom fade. + ============================================================================= */ + +.person-bio-text.is-collapsible { + position: relative; + overflow: hidden; + transition: max-height var(--transition-slow); +} + +.person-bio-text.is-collapsible.is-collapsed::after { + content: ""; + position: absolute; + left: 0; + right: 0; + bottom: 0; + height: 2.5em; + background: linear-gradient(to bottom, transparent, var(--color-bg-page)); + pointer-events: none; +} + +@media (prefers-reduced-motion: reduce) { + .person-bio-text.is-collapsible { + transition: none; + } +} + +.bio-toggle { + display: inline-block; + margin-top: var(--space-2); + padding: var(--space-2) 0; + min-height: 44px; /* WCAG 2.5.5 touch target */ + background: none; + border: none; + font-family: var(--font-family-secondary); + font-size: var(--font-size-sm); + font-weight: var(--font-weight-semibold); + color: var(--color-link); + cursor: pointer; +} + +.bio-toggle:hover { + color: var(--color-link-hover); + text-decoration: underline; +} + +.bio-toggle:focus { outline: var(--focus-ring-width) solid var(--focus-ring-color); outline-offset: var(--focus-ring-offset); } +/* ============================================================================= + LONG-PAGE NAVIGATION: SECTION NAV + BACK TO TOP (#1110) + ----------------------------------------------------------------------------- + For prolific members whose page gets very long (especially after "Load all"). + The section nav sticks just below the fixed site navbar (height published as + --ml-navbar-height by member-nav.js); member-nav.js adds scroll-spy via the + .is-active class / aria-current. Anchor jumps use scroll-margin-top so a + section's heading lands below both the navbar and this nav. + ============================================================================= */ + +.member-section-nav { + position: sticky; + top: var(--ml-navbar-height, 0px); + z-index: 100; /* under the fixed navbar (~1030) */ + margin: 0 0 var(--space-5) 0; + background-color: var(--color-bg-page); + border-bottom: 1px solid var(--color-border); +} + +/* The person's name, revealed by member-nav.js once the page heading scrolls + away. Absolutely positioned so it never shifts the centered link list; tucked + away on narrow screens where there isn't room beside the links. */ +.member-section-nav-name { + position: absolute; + left: var(--space-3); + top: 50%; + transform: translateY(-50%); + max-width: 38%; + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + font-family: var(--font-family-secondary); + font-weight: var(--font-weight-semibold); + color: var(--color-text-primary); +} + +.member-section-nav-name[hidden] { + display: none; +} + +@media (max-width: 767px) { + .member-section-nav-name { + display: none; + } +} + +/* The "loaded/total" count, quieter than the section label. */ +.member-section-nav-count { + font-weight: var(--font-weight-normal); + font-size: 0.9em; + color: var(--color-text-muted); +} + +.member-section-nav-list a.is-active .member-section-nav-count { + color: var(--color-primary); +} + +/* The "loaded/total" count appended to each section heading (replaces the old + "Recent" prefix, #1110). Lighter and a touch smaller than the heading. */ +.person-section-count { + font-weight: var(--font-weight-normal); + font-size: 0.8em; + color: var(--color-text-muted); +} + +.member-section-nav-list { + display: flex; + flex-wrap: wrap; + justify-content: center; + gap: var(--space-1) var(--space-2); + margin: 0; + padding: var(--space-2) 0; + list-style: none; +} + +.member-section-nav-list a { + display: inline-block; + padding: var(--space-2) var(--space-3); + border-radius: var(--border-radius-sm); + font-family: var(--font-family-secondary); + font-size: var(--font-size-sm); + font-weight: var(--font-weight-semibold); + color: var(--color-link); + text-decoration: none; + border-bottom: 2px solid transparent; +} + +.member-section-nav-list a:hover { + color: var(--color-link-hover); + text-decoration: underline; +} + +.member-section-nav-list a.is-active { + color: var(--color-primary); + border-bottom-color: var(--color-primary); +} + +.member-section-nav-list a:focus { + outline: var(--focus-ring-width) solid var(--focus-ring-color); + outline-offset: var(--focus-ring-offset); +} + +/* Anchor jumps (and scroll-spy) should land the heading below the fixed navbar + AND this sticky nav, not underneath them. */ +.person-section { + scroll-margin-top: calc(var(--ml-navbar-height, 60px) + 56px); +} + +/* Floating "Back to top" — injected by member-nav.js, shown after scrolling. */ +.back-to-top { + position: fixed; + right: var(--space-5); + bottom: var(--space-5); + z-index: 100; + display: flex; + align-items: center; + justify-content: center; + width: 48px; + height: 48px; /* >= WCAG 2.5.5 touch target */ + border: 1px solid var(--color-primary); + border-radius: 50%; + background-color: var(--color-primary); + color: var(--color-text-on-dark); + font-size: var(--font-size-base); + cursor: pointer; + box-shadow: 0 2px 6px rgba(0, 0, 0, 0.25); + transition: background-color var(--transition-fast), + opacity var(--transition-fast); +} + +.back-to-top:hover { + background-color: var(--color-primary-hover); + border-color: var(--color-primary-hover); +} + +.back-to-top:focus { + outline: var(--focus-ring-width) solid var(--focus-ring-color); + outline-offset: var(--focus-ring-offset); +} + +/* As with .see-more-controls, a class `display` would otherwise defeat the + `hidden` attribute member-nav.js toggles — guard it explicitly. */ +.back-to-top[hidden] { + display: none; +} + + /* ============================================================================= UTILITY CLASSES ============================================================================= */ diff --git a/website/static/website/js/bio-expand.js b/website/static/website/js/bio-expand.js new file mode 100644 index 00000000..27457827 --- /dev/null +++ b/website/static/website/js/bio-expand.js @@ -0,0 +1,157 @@ +/** + * BioExpand — collapses an over-long member bio behind a "Show more" toggle + * (issue #1110). + * + * A bio that fits in a few lines is left completely alone. Only when the text + * exceeds COLLAPSED_LINES (~3 lines) do we clamp it with a max-height + bottom + * fade and insert an accessible toggle button. The measurement is done in JS + * (rather than a pure CSS line-clamp) so the clamp height tracks the element's + * actual computed line-height and so we only add the button when it's needed. + * + * Accessibility: the toggle is a real + + {% endif %} {% if publications %} -
-

- {% if publications|length > 3 %}Recent Papers{% else %}Papers{% endif %} - ({{ publications|length }}/{{ publications_total }}) +

- - {% if publications|length > 3 %} + + {% comment %} + Publications have three layouts, chosen by TOTAL count (never the rendered + slice, so the layout can't flip as items load — #1110): + - > page size (many): the scannable VERTICAL list (thumbnail-left rows, + like /publications) plus the "Load more"/"Load all" controls. A 3-up + card grid of 100+ papers is overwhelming, so prolific members get rows. + The member_artifacts endpoint renders vertical rows to match. + - 4..page size: the compact horizontal 3-up CARD grid (no paging needed). + - <= 3: the vertical list (a grid of 1-3 cards looks sparse). + {% endcomment %} + {% if publications_total > page_sizes.publications %} + + + + {% elif publications_total > 3 %}
- {% for pub in publications|slice:":3" %} + {% for pub in publications %} {% include "snippets/display_pub_snippet.html" with orientation="horizontal" %} {% endfor %}
@@ -289,47 +420,75 @@

{% if videos %} -
-

- {% if videos|length > 3 %}Recent Videos{% else %}Videos{% endif %} - ({{ videos|length }}/{{ videos_total }}) +

- -
- {% for video in videos|slice:":3" %} + + + +
{% endif %} {% if talks %} -
-

- {% if talks|length > 4 %}Recent Talks{% else %}Talks{% endif %} - ({{ talks|length }}/{{ talks_total }}) +

- -
- {% for talk in talks|slice:":4" %} + + + +
{% endif %} diff --git a/website/tests/base.py b/website/tests/base.py index 99af5e72..8b494c62 100644 --- a/website/tests/base.py +++ b/website/tests/base.py @@ -110,6 +110,19 @@ def make_talk(self, title="A Test Talk", year=2024, **kwargs): ) return Talk.objects.create(title=title, **kwargs) + def make_video(self, title="A Test Video", year=2024, **kwargs): + """ + Create and return a Video. video_url defaults to a YouTube URL because + Video.get_video_host_str() does a substring check on it (a None url + would raise), and the video snippet embeds it. date is set so + get_most_recent_artifact_date() has something to sort on. + """ + from datetime import date as _date + from website.models import Video + kwargs.setdefault("date", _date(year, 1, 1)) + kwargs.setdefault("video_url", "https://www.youtube.com/watch?v=dQw4w9WgXcQ") + return Video.objects.create(title=title, **kwargs) + def make_news_item(self, title="Test News", author=None, **kwargs): """ Create and return a News item. `author` is intentionally optional diff --git a/website/tests/test_member_artifacts.py b/website/tests/test_member_artifacts.py new file mode 100644 index 00000000..071d53a2 --- /dev/null +++ b/website/tests/test_member_artifacts.py @@ -0,0 +1,334 @@ +""" +Regression tests for the member page's artifact ordering and the AJAX +"See more" endpoint (issue #1110). + +Covers: + - get_member_projects(): visible projects ordered purely by most-recent + artifact date (descending), no active/ended grouping, artifact-less projects + last, invisible projects excluded. This pins the fix for the old bug where + projects were ordered by the project's own start_date after being routed + through an unordered set. + - member_artifacts view: correct slicing, has_more / next_offset at the page + boundaries, offset paging, defensive offset parsing, and 404s. +""" + +from datetime import date + +from django.urls import reverse + +from website.models import ProjectRole +from website.views.member import ( + get_member_projects, + ARTIFACT_PAGE_SIZES, +) + +from .base import DatabaseTestCase + + +class MemberProjectOrderingTests(DatabaseTestCase): + """get_member_projects() ordering and visibility filtering.""" + + def _add_role(self, person, project): + ProjectRole.objects.create( + person=person, project=project, start_date=date(2024, 1, 1) + ) + + def test_projects_ordered_by_most_recent_artifact_descending(self): + person = self.make_person() + + # An ACTIVE project (no end_date) whose newest artifact is old (2020). + proj_active_old = self.make_project(name="Active Old", is_visible=True) + pub_old = self.make_publication(title="Old Pub", year=2020) + pub_old.projects.add(proj_active_old) + + # An ENDED project whose newest artifact is recent (2025). + proj_ended_new = self.make_project( + name="Ended New", is_visible=True, end_date=date(2024, 12, 31) + ) + pub_new = self.make_publication(title="New Pub", year=2025) + pub_new.projects.add(proj_ended_new) + + # A visible project with no artifacts at all -> sorts last. + proj_no_artifacts = self.make_project(name="No Artifacts", is_visible=True) + + for proj in (proj_active_old, proj_ended_new, proj_no_artifacts): + self._add_role(person, proj) + + ordered = get_member_projects(person) + + # Pure date order: the recently-active (ended) project outranks the + # long-running active one because its artifact is newer. No active-first + # grouping. Artifact-less project is last. + self.assertEqual( + ordered, [proj_ended_new, proj_active_old, proj_no_artifacts] + ) + + def test_invisible_projects_excluded(self): + person = self.make_person() + visible = self.make_project(name="Visible", is_visible=True) + hidden = self.make_project(name="Hidden", is_visible=False) + # Give the hidden project the newest artifact to prove visibility wins + # over recency. + hidden_pub = self.make_publication(title="Hidden Pub", year=2030) + hidden_pub.projects.add(hidden) + self._add_role(person, visible) + self._add_role(person, hidden) + + ordered = get_member_projects(person) + + self.assertIn(visible, ordered) + self.assertNotIn(hidden, ordered) + + +class MemberArtifactsEndpointTests(DatabaseTestCase): + """The member_artifacts AJAX "See more" endpoint.""" + + def _url(self, person, artifact_type, offset=None): + url = reverse( + "website:member_artifacts", + kwargs={"member_id": person.id, "artifact_type": artifact_type}, + ) + if offset is not None: + url += f"?offset={offset}" + return url + + def setUp(self): + super().setUp() + self.person = self.make_person(first_name="Pagey", last_name="McTest") + # 8 publications authored by the person; pubs page size is 6. + self.pubs = [] + for i in range(8): + pub = self.make_publication(title=f"Paper {i}", year=2024) + pub.authors.add(self.person) + self.pubs.append(pub) + + def test_first_page_reports_more(self): + page_size = ARTIFACT_PAGE_SIZES["publications"] # 6 + resp = self.client.get(self._url(self.person, "publications")) + self.assertEqual(resp.status_code, 200) + data = resp.json() + self.assertTrue(data["has_more"]) + self.assertEqual(data["next_offset"], page_size) + # The "Load more" path only exists for prolific members, who get the + # VERTICAL list — so appended papers are vertical rows, not card-grid + # cells (#1110). + self.assertEqual(data["html"].count("pub-row-vert-layout"), page_size) + self.assertNotIn("pub-column-horiz-layout", data["html"]) + + def test_second_page_is_last(self): + resp = self.client.get(self._url(self.person, "publications", offset=6)) + self.assertEqual(resp.status_code, 200) + data = resp.json() + self.assertFalse(data["has_more"]) + self.assertEqual(data["next_offset"], 8) + self.assertEqual(data["html"].count("pub-row-vert-layout"), 2) + + def test_offset_past_end_returns_empty(self): + resp = self.client.get(self._url(self.person, "publications", offset=99)) + self.assertEqual(resp.status_code, 200) + data = resp.json() + self.assertFalse(data["has_more"]) + self.assertEqual(data["html"], "") + + def test_load_all_returns_everything_from_offset(self): + # ?all=1 returns every remaining item in one response (backs "Load all"). + resp = self.client.get(self._url(self.person, "publications") + "?all=1") + self.assertEqual(resp.status_code, 200) + data = resp.json() + self.assertFalse(data["has_more"]) + self.assertEqual(data["next_offset"], 8) + self.assertEqual(data["html"].count("pub-row-vert-layout"), 8) + + def test_load_all_from_nonzero_offset(self): + resp = self.client.get(self._url(self.person, "publications", offset=6) + "&all=1") + data = resp.json() + self.assertFalse(data["has_more"]) + self.assertEqual(data["html"].count("pub-row-vert-layout"), 2) + + def test_invalid_offset_treated_as_zero(self): + resp = self.client.get(self._url(self.person, "publications", offset="abc")) + self.assertEqual(resp.status_code, 200) + data = resp.json() + # Falls back to offset 0 -> first full page. + self.assertEqual(data["next_offset"], ARTIFACT_PAGE_SIZES["publications"]) + + def test_unknown_artifact_type_404(self): + resp = self.client.get(self._url(self.person, "widgets")) + self.assertEqual(resp.status_code, 404) + + def test_unknown_member_404(self): + url = reverse( + "website:member_artifacts", + kwargs={"member_id": 999999, "artifact_type": "publications"}, + ) + self.assertEqual(self.client.get(url).status_code, 404) + + def test_talks_endpoint_renders_and_pages(self): + person = self.make_person(first_name="Talky", last_name="One") + for i in range(9): # talks page size is 8 + talk = self.make_talk(title=f"Talk {i}", year=2024) + talk.authors.add(person) + + first = self.client.get(self._url(person, "talks")).json() + self.assertTrue(first["has_more"]) + self.assertEqual(first["next_offset"], 8) + self.assertEqual(first["html"].count("talk-card"), 8) + + second = self.client.get(self._url(person, "talks", offset=8)).json() + self.assertFalse(second["has_more"]) + self.assertEqual(second["html"].count("talk-card"), 1) + + def test_videos_endpoint_renders_and_pages(self): + # A person's videos surface through an authored talk (or publication), + # so wire each video to a talk the person gave. videos page size is 6. + person = self.make_person(first_name="Viddy", last_name="One") + for i in range(7): + video = self.make_video(title=f"Video {i}", year=2024) + talk = self.make_talk(title=f"Video Talk {i}", year=2024) + talk.video = video + talk.save() + talk.authors.add(person) + + first = self.client.get(self._url(person, "videos")).json() + self.assertTrue(first["has_more"]) + self.assertEqual(first["next_offset"], 6) + self.assertEqual(first["html"].count("video-card"), 6) + + second = self.client.get(self._url(person, "videos", offset=6)).json() + self.assertFalse(second["has_more"]) + self.assertEqual(second["html"].count("video-card"), 1) + + def test_projects_endpoint_pages_list_path(self): + # Projects go through the Python-list code path (len()/slice) rather than + # a queryset, so exercise it separately. 9 visible projects, page size 8. + person = self.make_person(first_name="Proj", last_name="Owner") + # Letters only: the project URL pattern (website:project) rejects digits + # in short_name, and display_project_snippet.html reverses that URL. + for i in range(9): + letter = chr(ord("A") + i) + proj = self.make_project(name=f"Project {letter}", is_visible=True) + ProjectRole.objects.create( + person=person, project=proj, start_date=date(2024, 1, 1) + ) + + first = self.client.get(self._url(person, "projects")).json() + self.assertTrue(first["has_more"]) + self.assertEqual(first["next_offset"], 8) + self.assertEqual(first["html"].count("project-card"), 8) + + second = self.client.get(self._url(person, "projects", offset=8)).json() + self.assertFalse(second["has_more"]) + self.assertEqual(second["html"].count("project-card"), 1) + + +class MemberPageRenderTests(DatabaseTestCase): + """End-to-end render of the member page with the #1110 changes wired in.""" + + def test_page_renders_see_more_controls(self): + person = self.make_person(first_name="Talky", last_name="McTest") + # 9 talks > the talks page size (8): collapsible grid + See More controls. + for i in range(9): + talk = self.make_talk(title=f"Talk {i}", year=2024) + talk.authors.add(person) + + resp = self.client.get( + reverse("website:member_by_name", kwargs={"member_name": person.url_name}) + ) + self.assertEqual(resp.status_code, 200) + body = resp.content.decode() + # The talks grid is collapsible and the load-more controls are present + # (rendered hidden; JS un-hides them and sets exact counts). + self.assertIn('id="person-talks-grid"', body) + self.assertIn("is-collapsed", body) + self.assertIn("see-more-controls", body) + self.assertIn('data-artifact-type="talks"', body) + self.assertIn("Load more talks", body) + self.assertIn("Load all talks", body) + # Headings never say "Recent" any more; they carry a loaded/total count + # (8 of 9 loaded) instead. (Check the old heading string specifically — + # the page footer has an unrelated "Recent News" block.) + self.assertNotIn("Recent Talks", body) + self.assertIn("person-section-count", body) + self.assertIn("(8/9)", body) + # The bio-expand / load-more scripts are wired up. + self.assertIn("member-load-more.js", body) + self.assertIn("bio-expand.js", body) + + def _get_member_body(self, person): + resp = self.client.get( + reverse("website:member_by_name", kwargs={"member_name": person.url_name}) + ) + self.assertEqual(resp.status_code, 200) + return resp.content.decode() + + def _make_pubs(self, person, n): + for i in range(n): + pub = self.make_publication(title=f"Paper {i}", year=2024) + pub.authors.add(person) + + def test_section_nav_lists_only_present_sections(self): + # The sticky section nav links only to sections that exist (#1110). + person = self.make_person(first_name="Nav", last_name="Tester") + for i in range(2): + talk = self.make_talk(title=f"Talk {i}", year=2024) + talk.authors.add(person) + body = self._get_member_body(person) + self.assertIn("member-section-nav", body) + self.assertIn("member-nav.js", body) + self.assertIn('data-section-link="person-talks"', body) + self.assertNotIn('data-section-link="person-videos"', body) + self.assertNotIn('data-section-link="person-publications"', body) + self.assertNotIn('data-section-link="person-projects"', body) + # The nav carries the person's name (revealed on scroll by JS) and a + # loaded/total count per section (2 talks shown of 2 total). + self.assertIn("member-section-nav-name", body) + self.assertIn(person.get_full_name(), body) + self.assertIn("member-section-nav-count", body) + self.assertIn("(2/2)", body) + + def test_nav_count_reflects_loaded_slice_over_total(self): + # 9 talks: the desktop slice (8) is loaded, of 9 total -> "(8/9)". + person = self.make_person(first_name="Count", last_name="Tester") + for i in range(9): + talk = self.make_talk(title=f"Talk {i}", year=2024) + talk.authors.add(person) + body = self._get_member_body(person) + self.assertIn("(8/9)", body) + + def test_few_publications_use_vertical_layout_without_controls(self): + # <= 3 papers: vertical list, no controls (a grid of 1-3 looks sparse). + person = self.make_person(first_name="Fewpubs", last_name="McTest") + self._make_pubs(person, 2) + body = self._get_member_body(person) + self.assertEqual(body.count("pub-row-vert-layout"), 2) + self.assertNotIn("pub-column-horiz-layout", body) + self.assertNotIn('data-artifact-type="publications"', body) + self.assertNotIn("Recent Papers", body) + + def test_mid_count_publications_use_card_grid_without_controls(self): + # 4..page_size papers: compact horizontal card grid, still no paging. + person = self.make_person(first_name="Midpubs", last_name="McTest") + self._make_pubs(person, 5) # 3 < 5 <= 6 + body = self._get_member_body(person) + self.assertEqual(body.count("pub-column-horiz-layout"), 5) + self.assertNotIn("pub-row-vert-layout", body) + self.assertIn("person-publications-grid", body) + self.assertNotIn('data-artifact-type="publications"', body) + self.assertNotIn("Recent Papers", body) + + def test_many_publications_use_vertical_list_with_controls(self): + # > page_size papers: scannable vertical list + Load more/all controls. + person = self.make_person(first_name="Manypubs", last_name="McTest") + self._make_pubs(person, 7) # > 6 + body = self._get_member_body(person) + self.assertIn('id="person-publications-grid"', body) + self.assertIn("person-publications-list", body) # vertical, not card grid + self.assertEqual(body.count("pub-row-vert-layout"), 6) # desktop page size + self.assertNotIn("pub-column-horiz-layout", body) + self.assertIn("see-more-controls", body) + self.assertIn('data-artifact-type="publications"', body) + self.assertIn("Load more papers", body) + self.assertIn("Load all papers", body) + # No "Recent" prefix; the heading carries a loaded/total count (6 of 7). + self.assertNotIn("Recent Papers", body) + self.assertIn("(6/7)", body) diff --git a/website/tests/test_member_e2e.py b/website/tests/test_member_e2e.py new file mode 100644 index 00000000..36f6d87a --- /dev/null +++ b/website/tests/test_member_e2e.py @@ -0,0 +1,278 @@ +""" +End-to-end browser smoke tests for the member page's interactive bits (#1110): +the "Load more" / "Load all" controls, the no-overflow hidden state, the +live loaded/total section counts, the section nav, and the bio toggle. + +These exercise the JavaScript (member-load-more.js, bio-expand.js) that the +Django test client can't run. They use Playwright against a live server. + +WHY SKIP-GUARDED: Playwright and its browser aren't part of the normal test +image, so this module SKIPS itself (rather than erroring) when either is +missing. `python manage.py test website` stays green without them. + +To run locally / in CI: + pip install playwright + python -m playwright install --with-deps chromium + python manage.py test website.tests.test_member_e2e \ + --settings=makeabilitylab.settings_test + +Notes: + - StaticLiveServerTestCase serves collected/​app static files, so the JS/CSS + under test are actually loaded by the browser. + - It subclasses TransactionTestCase, so fixtures are created in setUp and are + visible to the live-server thread (unlike TestCase's per-test transaction). +""" + +import os +import unittest + +from django.contrib.staticfiles.testing import StaticLiveServerTestCase + +try: + from playwright.sync_api import sync_playwright, TimeoutError as PWTimeout + _PLAYWRIGHT_IMPORT_ERROR = None +except Exception as exc: # pragma: no cover - environment dependent + sync_playwright = None + PWTimeout = Exception + _PLAYWRIGHT_IMPORT_ERROR = exc + +# Playwright's sync API spins up its own subprocess; allow it inside Django's +# test thread. +os.environ.setdefault("DJANGO_ALLOW_ASYNC_UNSAFE", "1") + +_LONG_BIO = ( + "

" + ("This is a deliberately long bio sentence that wraps several " + "times so it exceeds the three-line collapse threshold. ") * 12 + + "

" +) + + +@unittest.skipIf(sync_playwright is None, + f"Playwright not installed ({_PLAYWRIGHT_IMPORT_ERROR})") +class MemberPageE2ETests(StaticLiveServerTestCase): + + @classmethod + def setUpClass(cls): + super().setUpClass() + try: + cls._pw = sync_playwright().start() + cls._browser = cls._pw.chromium.launch() + except Exception as exc: # browser binary missing, etc. + super().tearDownClass() + raise unittest.SkipTest(f"Chromium unavailable for Playwright: {exc}") + + @classmethod + def tearDownClass(cls): + try: + cls._browser.close() + cls._pw.stop() + finally: + super().tearDownClass() + + # ---- fixtures ----------------------------------------------------------- + + def _make_person(self, **kwargs): + from django.core.files.uploadedfile import SimpleUploadedFile + from website.models import Person + gif = (b"GIF89a\x01\x00\x01\x00\x80\x00\x00\xff\xff\xff\x00\x00\x00" + b"!\xf9\x04\x01\x00\x00\x00\x00,\x00\x00\x00\x00\x01\x00\x01" + b"\x00\x00\x02\x02D\x01\x00;") + kwargs.setdefault("first_name", "E2E") + kwargs.setdefault("last_name", "Tester") + kwargs.setdefault("image", SimpleUploadedFile("a.gif", gif, content_type="image/gif")) + kwargs.setdefault("easter_egg", SimpleUploadedFile("b.gif", gif, content_type="image/gif")) + return Person.objects.create(**kwargs) + + def _make_talks(self, person, n): + from datetime import date + from django.core.files.uploadedfile import SimpleUploadedFile + from website.models import Talk + from website.models.talk import TalkType + for i in range(n): + talk = Talk.objects.create( + title=f"Talk number {i}", + date=date(2024, 1, 1), + forum_name="CHI", + talk_type=TalkType.CONFERENCE_TALK, + pdf_file=SimpleUploadedFile(f"t{i}.pdf", b"%PDF-1.4 test", + content_type="application/pdf"), + ) + talk.authors.add(person) + + def _make_papers(self, person, n): + from datetime import date + from django.core.files.uploadedfile import SimpleUploadedFile + from website.models import Publication + from website.models.publication import PubType + for i in range(n): + pub = Publication.objects.create( + title=f"Paper number {i}", date=date(2024, 1, 1), forum_name="CHI", + pub_venue_type=PubType.CONFERENCE, + pdf_file=SimpleUploadedFile(f"p{i}.pdf", b"%PDF-1.4 test", + content_type="application/pdf"), + ) + pub.authors.add(person) + + def _make_projects(self, person, n): + from datetime import date + from website.models import Project, ProjectRole + for i in range(n): + letter = chr(ord("A") + i) + proj = Project.objects.create( + name=f"Project {letter}", short_name=f"project{letter.lower()}", + is_visible=True, + ) + ProjectRole.objects.create(person=person, project=proj, + start_date=date(2024, 1, 1)) + + def _goto(self, person): + page = self._browser.new_page(viewport={"width": 1280, "height": 900}) + page.goto(f"{self.live_server_url}/member/{person.url_name}/") + return page + + # ---- tests -------------------------------------------------------------- + + def _talk_card_count(self, page): + return page.locator("#person-talks-grid .talk-card").count() + + def test_load_more_appends_until_complete(self): + person = self._make_person() + self._make_talks(person, 20) # > talks page size (8) + page = self._goto(person) + try: + page.wait_for_selector("#person-talks-grid .talk-card") + self.assertEqual(self._talk_card_count(page), 8) + # Heading carries a live loaded/total count (no "Recent" wording). + self.assertIn("(8/20)", page.inner_text("#talks-heading")) + + more = page.locator('[data-artifact-type="talks"] [data-load-more]') + self.assertRegex(more.inner_text(), r"Load \d+ more talks") + + more.click() # 8 -> 16 + page.wait_for_function( + "document.querySelectorAll('#person-talks-grid .talk-card').length === 16" + ) + + more.click() # 16 -> 20 (last batch) + page.wait_for_function( + "document.querySelectorAll('#person-talks-grid .talk-card').length === 20" + ) + # Everything shown: controls removed and the heading count is full. + self.assertEqual( + page.locator('[data-artifact-type="talks"]').count(), 0) + self.assertIn("(20/20)", page.inner_text("#talks-heading")) + finally: + page.close() + + def test_load_all_loads_everything_and_updates_count(self): + person = self._make_person() + self._make_talks(person, 20) + page = self._goto(person) + try: + page.wait_for_selector("#person-talks-grid .talk-card") + self.assertEqual(self._talk_card_count(page), 8) + + all_btn = page.locator('[data-artifact-type="talks"] [data-load-all]') + self.assertRegex(all_btn.inner_text(), r"Load all \d+ talks") + all_btn.click() + page.wait_for_function( + "document.querySelectorAll('#person-talks-grid .talk-card').length === 20" + ) + self.assertEqual( + page.locator('[data-artifact-type="talks"]').count(), 0) + # The heading's loaded/total count reaches full. + self.assertIn("(20/20)", page.inner_text("#talks-heading")) + finally: + page.close() + + def test_projects_controls_hidden_when_no_overflow(self): + # Exactly page-size (8) projects -> nothing more to load -> the projects + # controls must NOT be visible on desktop. This is the regression guard + # for the display:flex-overriding-[hidden] bug (#1110). + person = self._make_person() + self._make_projects(person, 8) + page = self._goto(person) + try: + page.wait_for_selector("#person-projects-grid .project-card") + controls = page.locator('[data-artifact-type="projects"]') + self.assertEqual(controls.count(), 1) + self.assertFalse(controls.is_visible()) + finally: + page.close() + + def test_section_nav_highlights_and_jumps(self): + person = self._make_person() + self._make_papers(person, 7) # -> Papers section + self._make_talks(person, 9) # -> Talks section + page = self._goto(person) + try: + nav = page.locator("[data-member-section-nav]") + page.wait_for_selector("[data-member-section-nav]") + self.assertEqual(nav.locator("a").count(), 2) # Papers + Talks + nav.locator('[data-section-link="person-talks"]').click() + page.wait_for_function( + "document.querySelector('[data-section-link=\"person-talks\"]')" + ".getAttribute('aria-current') === 'location'" + ) + finally: + page.close() + + def test_nav_name_reveals_on_scroll(self): + person = self._make_person(first_name="Ada", last_name="Lovelace") + self._make_talks(person, 20) # long enough to scroll the

away + page = self._goto(person) + try: + name = page.locator("[data-nav-name]") + page.wait_for_selector("[data-member-section-nav]") + self.assertFalse(name.is_visible()) # hidden while the

shows + page.evaluate("window.scrollTo(0, document.body.scrollHeight)") + name.wait_for(state="visible") + self.assertIn("Ada Lovelace", name.inner_text()) + finally: + page.close() + + def test_nav_count_updates_on_load_more(self): + person = self._make_person() + self._make_talks(person, 20) + page = self._goto(person) + try: + count = page.locator( + '[data-section-link="person-talks"] .member-section-nav-count') + page.wait_for_selector("[data-member-section-nav]") + self.assertEqual(count.inner_text(), "(8/20)") + page.locator('[data-artifact-type="talks"] [data-load-more]').click() + page.wait_for_function( + "document.querySelector('[data-section-link=\"person-talks\"] " + ".member-section-nav-count').textContent === '(16/20)'" + ) + finally: + page.close() + + def test_back_to_top_appears_and_scrolls(self): + person = self._make_person() + self._make_talks(person, 20) # long page + page = self._goto(person) + try: + btn = page.locator(".back-to-top") + self.assertFalse(btn.is_visible()) # hidden at the top + page.evaluate("window.scrollTo(0, document.body.scrollHeight)") + btn.wait_for(state="visible") + btn.click() + page.wait_for_function("window.scrollY === 0") + finally: + page.close() + + def test_bio_show_more_then_less_stays_collapsed(self): + person = self._make_person(bio=_LONG_BIO) + page = self._goto(person) + try: + toggle = page.locator(".bio-toggle") + page.wait_for_selector(".bio-toggle") + self.assertEqual(toggle.get_attribute("aria-expanded"), "false") + toggle.click() + self.assertEqual(toggle.get_attribute("aria-expanded"), "true") + toggle.click() + # The reported bug: it bounced back open. It must stay collapsed. + self.assertEqual(toggle.get_attribute("aria-expanded"), "false") + finally: + page.close() diff --git a/website/urls.py b/website/urls.py index b73f9a5f..76c814b7 100644 --- a/website/urls.py +++ b/website/urls.py @@ -27,11 +27,19 @@ # re_path(r'^member/(?P[0-9]+)/$', views.member, name='member'), path('member//', views.member, name='member_by_id'), - # Matches URLs like "member/john-doe/" where "john-doe" is a member ID consisting of + # Matches URLs like "member/john-doe/" where "john-doe" is a member ID consisting of # lowercase letters and hyphens, and routes it to the `member` view. # re_path(r'^member/(?P[-a-z]+)/$', views.member, name='member'), path('member//', views.member, name='member_by_name'), + # AJAX endpoint backing the per-section "See more" controls on a member page + # (#1110). Keyed by numeric pk (the page already knows person.id), so it does + # not collide with the single-segment member_by_name route above. Returns the + # next batch of the given artifact_type (projects/publications/videos/talks) + # as rendered HTML. See website/views/member.py::member_artifacts. + path('member//artifacts//', + views.member_artifacts, name='member_artifacts'), + # Matches the URL "publications/" and routes it to the `publications` view. re_path(r'^publications/$', views.publications, name='publications'), diff --git a/website/views/member.py b/website/views/member.py index 8cec6f74..c438001e 100644 --- a/website/views/member.py +++ b/website/views/member.py @@ -5,15 +5,46 @@ from django.shortcuts import render, get_object_or_404, redirect from django.db.models import Q from django.core.exceptions import MultipleObjectsReturned +from django.http import Http404, JsonResponse +from django.template.loader import render_to_string + +from datetime import date # For logging import time import logging -from django.http import Http404 # This retrieves a Python logging instance (or creates it) _logger = logging.getLogger(__name__) +# How many of each artifact type the member page renders server-side on first +# paint (the *desktop* count) and, equivalently, the batch size fetched by each +# subsequent AJAX "See more" click (#1110). +# +# Mobile shows fewer than this (see ARTIFACT_MOBILE_PAGE_SIZES). Importantly, +# the full desktop count is ALWAYS in the initial HTML regardless of viewport; +# a CSS rule (member.css) merely hides the overflow on narrow screens. That's +# why the first "See more" tap on a phone is an instant CSS reveal with no +# network request, and only loads beyond the desktop count go to the server. +ARTIFACT_PAGE_SIZES = { + 'projects': 8, + 'publications': 6, + 'videos': 6, + 'talks': 8, +} + +# How many of each artifact type are visible on small screens (<=576px, the +# point where every grid collapses to a single column). Enforced purely in CSS +# (member.css) via an :nth-child cap; passed to the template only so the +# load-more JS can decide whether a section needs a "See more" control and can +# announce the right counts. See the no-JS tradeoff note in member.css. +ARTIFACT_MOBILE_PAGE_SIZES = { + 'projects': 4, + 'publications': 3, + 'videos': 3, + 'talks': 4, +} + def member(request, member_name=None, member_id=None): func_start_time = time.perf_counter() _logger.debug(f"Starting views/member member_id={member_id} and member_name={member_name} at {func_start_time:0.4f}") @@ -63,49 +94,53 @@ def member(request, member_name=None, member_id=None): # the first 4 objects. news = News.objects.filter(people=person).order_by('-date')[:4] latest_position = person.get_latest_position - publications = (person.publication_set - .prefetch_related('authors', 'projects', 'keywords') - .order_by('-date')) - talks = (person.talk_set - .select_related('video') - .prefetch_related('authors', 'publication_set', 'projects') - .order_by('-date')) + + # Full, ordered sequences for each artifact type. These same helpers back + # the member_artifacts AJAX endpoint, so the initial render and every "See + # more" batch share one ordering (and one definition of "this person's + # artifacts"). We slice them below for the first paint and only count() the + # totals — we never materialize the whole list of (sometimes 100+) papers. + publications = get_member_publications(person) + talks = get_member_talks(person) videos = get_videos_by_author(person) + projects = get_member_projects(person) # a list (sorted in Python), not a queryset project_roles = person.projectrole_set.order_by('-start_date') - projects = person.get_projects - - # Sort projects: active first (not ended), then by most recent start_date - projects = sorted( - projects, - key=lambda proj: ( - # First sort key: active projects first - # has_ended() returns False for active, True for ended - # Since False < True, active projects come first - proj.has_ended(), - # Second sort key: most recent start_date first (descending) - -(proj.start_date.toordinal() if proj.start_date else 0) - ) - ) - left_align_headers = (len(projects) <= 4 and len(publications) <= 3 and - len(talks) <= 3 and len(videos) <= 3) - + publications_total = publications.count() + talks_total = talks.count() + videos_total = videos.count() + projects_total = len(projects) + + # Left-align section headers only when every section is short enough to fit + # in its first (desktop) row — i.e. nothing is truncated. Thresholds track + # the desktop page sizes above. + left_align_headers = (projects_total <= ARTIFACT_PAGE_SIZES['projects'] and + publications_total <= ARTIFACT_PAGE_SIZES['publications'] and + talks_total <= ARTIFACT_PAGE_SIZES['talks'] and + videos_total <= ARTIFACT_PAGE_SIZES['videos']) + auto_generated_bio = "" if not person.bio: auto_generated_bio = auto_generate_bio(person) - # Show only projects marked visible (#1300). Visibility is governed solely - # by the is_visible flag, replacing the old thumbnail+publication heuristic. - projects = [proj for proj in projects if proj.is_visible] - context = {'person': person, 'auto_generated_bio': auto_generated_bio, 'news': news, - 'talks': talks, - 'videos': videos, - 'publications': publications, + # Slice to the desktop page size for first paint. list()/[:n] + # forces evaluation of just that slice; the *_total values above + # carry the real counts the template needs for the "See more" + # buttons and the "Recent" headings. + 'talks': list(talks[:ARTIFACT_PAGE_SIZES['talks']]), + 'videos': list(videos[:ARTIFACT_PAGE_SIZES['videos']]), + 'publications': list(publications[:ARTIFACT_PAGE_SIZES['publications']]), + 'projects': projects[:ARTIFACT_PAGE_SIZES['projects']], + 'talks_total': talks_total, + 'videos_total': videos_total, + 'publications_total': publications_total, + 'projects_total': projects_total, + 'page_sizes': ARTIFACT_PAGE_SIZES, + 'mobile_page_sizes': ARTIFACT_MOBILE_PAGE_SIZES, 'project_roles': project_roles, - 'projects' : projects, 'position' : latest_position, # 'banners': displayed_banners, 'left_align_headers': left_align_headers, @@ -127,14 +162,137 @@ def member(request, member_name=None, member_id=None): return render_response +def get_member_publications(person): + """Publications authored by ``person``, newest first. + + The ``-id`` secondary sort is a deterministic tiebreaker: many papers share + a single ``date``, and the "See more" control pages with ``[offset:offset+n]`` + slices across separate requests. Without a stable total order, equal-date + rows could shuffle between pages and be dropped or duplicated. + """ + return (person.publication_set + .prefetch_related('authors', 'projects', 'keywords') + .order_by('-date', '-id')) + + +def get_member_talks(person): + """Talks given by ``person``, newest first (``-id`` tiebreaker; see + :func:`get_member_publications` for why).""" + return (person.talk_set + .select_related('video') + .prefetch_related('authors', 'publication_set', 'projects') + .order_by('-date', '-id')) + + def get_videos_by_author(person): - """Returns a queryset of videos that the given person is an author on""" + """Videos that ``person`` is an author on (via an associated publication or + talk), newest first (``-id`` tiebreaker; see + :func:`get_member_publications`).""" return (Video.objects .select_related('publication') .prefetch_related('projects') .filter(Q(publication__authors=person) | Q(talk__authors=person)) .distinct() - .order_by('-date')) + .order_by('-date', '-id')) + + +def get_member_projects(person): + """Visible projects ``person`` has worked on, ordered by each project's most + recent activity (newest publication/talk/video) first. + + Ordering rationale (#1110): a member page should foreground the projects + with the most recent *activity*, not the project's own ``start_date`` — a + person can join a long-running project recently, and the old code sorted by + ``start_date`` (after dropping order entirely by routing through a ``set``), + which surfaced stale projects. We sort purely by most-recent-artifact date, + descending — no active/ended grouping — with projects that have no artifacts + sorting last (``date.min``). + + The ``pk`` tiebreaker makes the order fully deterministic across requests + (``person.get_projects`` is an unordered ``set``), which the offset-based + "See more" pagination in :func:`member_artifacts` relies on. + + Returns a list (the artifact-date key is computed in Python, so this can't + stay a queryset). + """ + projects = [proj for proj in person.get_projects if proj.is_visible] + projects.sort( + key=lambda proj: (proj.get_most_recent_artifact_date() or date.min, proj.pk), + reverse=True, + ) + return projects + + +# Maps an artifact_type URL segment to (ordered-sequence builder, snippet +# template, snippet context key, extra snippet context). Shared by the member +# page (indirectly, via the helpers above) and the member_artifacts endpoint so +# both render the identical markup. +_ARTIFACT_CONFIG = { + 'projects': (get_member_projects, 'snippets/display_project_snippet.html', 'project', {}), + # Publications render VERTICAL here. The "Load more" control (hence this + # endpoint) only appears when a member has more papers than the page size, + # and that "many papers" case uses the scannable vertical list rather than + # the compact 3-up card grid (#1110) — so appended papers are always vertical + # rows. Members with few papers (<= page size) never reach this endpoint. + 'publications': (get_member_publications, 'snippets/display_pub_snippet.html', 'pub', + {'orientation': 'vertical'}), + 'videos': (get_videos_by_author, 'snippets/display_video_snippet.html', 'video', {}), + 'talks': (get_member_talks, 'snippets/display_talk_snippet.html', 'talk', {}), +} + + +def member_artifacts(request, member_id, artifact_type): + """AJAX endpoint returning the next batch of a member's artifacts as HTML, + for the per-section "See more" controls on the member page (#1110). + + Why HTML rather than JSON data: we render the very same snippet templates + the page uses on first paint, so appended cards are byte-for-byte identical + and stay accessible — there is no parallel client-side renderer to keep in + sync. ``render_to_string(..., request=request)`` supplies ``MEDIA_URL`` and + ``static`` via the normal context processors, which the talk/video snippets + need. + + Keyed by primary key (not ``url_name``) because the page already knows + ``person.id``; this deliberately sidesteps the fuzzy-name-match/redirect + logic in :func:`member`. + + Query params: + offset (int): how many items of this type the client already shows. + all (str): when "1", return every remaining item from ``offset`` in one + response (backs the "Load all" control) instead of a single batch. + + Returns JSON ``{html, has_more, next_offset}``. The server fixes the batch + size (``ARTIFACT_PAGE_SIZES``) — the client cannot request an arbitrary page + size (only "one batch" or "all the rest"). + """ + person = get_object_or_404(Person, id=member_id) + + if artifact_type not in _ARTIFACT_CONFIG: + raise Http404(f"Unknown artifact type: {artifact_type}") + builder, template_name, ctx_key, extra_ctx = _ARTIFACT_CONFIG[artifact_type] + + try: + offset = max(int(request.GET.get('offset', 0)), 0) + except (TypeError, ValueError): + offset = 0 + + page_size = ARTIFACT_PAGE_SIZES[artifact_type] + load_all = request.GET.get('all') == '1' + + items = builder(person) + total = len(items) if isinstance(items, list) else items.count() + batch = items[offset:] if load_all else items[offset:offset + page_size] + + html = ''.join( + render_to_string(template_name, {ctx_key: obj, **extra_ctx}, request=request) + for obj in batch + ) + next_offset = offset + len(batch) + return JsonResponse({ + 'html': html, + 'has_more': next_offset < total, + 'next_offset': next_offset, + }) def get_closest_urlname_in_database(query_urlname, cutoff=0.8): """