Skip to content

Commit 7a52981

Browse files
authored
Serve student resources from books/resources/ and match table markers by id (#1779)
FacultyResourcesSerializer omitted book_student_resources, so table cells with resource_type 'Student' could never resolve client-side against the resources API — os-webview had faculty resources but no student ones to match against. Add it with the same hidden-filtering and ?x=y redaction as the faculty rows, but flattened to resource_heading/resource_description/ resource_unlocked to match the shape os-webview already reads from the Book page API for student resources. Also give the table's resource_ref marker a resource_id (the through-row pk, equal to the id books/resources/ now serializes for the same row). Heading-only matching is fragile — punctuation differences between the CMS and the resources API can silently miss a match — so the frontend can now match by id first and fall back to heading only for already-cached table JSON (up to 30 days old) that predates resource_id.
1 parent b276179 commit 7a52981

5 files changed

Lines changed: 144 additions & 9 deletions

File tree

CLAUDE.md

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,15 @@ its file URL. The per-request `?x=y` redaction in `books/serializers.py` cannot
173173
work here: `get_api_representation` runs with no user and its output is cached and
174174
served to every visitor.
175175

176-
The cell also carries a `resource_ref` marker (book slug/id, heading, resource
177-
type) that os-webview uses to swap in the real per-user resource box client-side.
178-
Renaming those keys breaks that override — and note os-webview camelCases every
179-
key of a page payload before a block sees it, so it reads them as
180-
`bookSlug`/`bookId`/`resourceType`.
176+
The cell also carries a `resource_ref` marker (book slug/id, resource id, heading,
177+
resource type) that os-webview uses to swap in the real per-user resource box
178+
client-side. It matches the marker back to a `books/resources/` row by
179+
`resource_id` (the through-row pk) first, falling back to heading matching when
180+
`resource_id` is absent — either a synthetic row (e.g. Web PDF) or already-cached
181+
table JSON (up to 30 days old) from before `resource_id` existed. Renaming those
182+
keys breaks that override — and note os-webview camelCases every key of a page
183+
payload before a block sees it, so it reads them as
184+
`bookSlug`/`bookId`/`resourceId`/`resourceType`.
181185

182186
**Adding config to a nested StreamField block requires a migration.** Wagtail
183187
bakes each StreamField's full block-tree deconstruction into migration state, so

books/serializers.py

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,36 @@ def to_representation(self, instance):
4646
for resource in book_video_faculty_resources:
4747
# remove listing of linked book data
4848
resource['book_video_faculty_resource'] = {}
49+
50+
ret['book_student_resources'] = [r for r in ret['book_student_resources'] if not r.get('hidden')]
51+
52+
book_student_resources = ret['book_student_resources']
53+
for resource in book_student_resources:
54+
# remove listing of linked book data
55+
resource['book_student_resource'] = {}
56+
# the resource snippet is SET_NULL, so a deleted snippet leaves this None
57+
snippet = resource['resource'] or {}
58+
# os-webview resolves student rows against the flat shape the Book
59+
# page API emits (StudentResources.resource_heading/_description/
60+
# _unlocked), not the nested `resource` dict depth=2 produces here.
61+
resource['resource_heading'] = snippet.get('heading')
62+
resource['resource_description'] = snippet.get('description')
63+
resource['resource_unlocked'] = snippet.get('unlocked_resource')
64+
# if parameter sent, clear links to locked student resources
65+
if x_param and x_param == 'y':
66+
if not snippet.get('unlocked_resource'):
67+
if resource['link_document'] is not None:
68+
resource['link_document']['file'] = ''
69+
if resource['link_page'] is not None:
70+
resource['link_page']['url_path'] = ''
71+
if resource['link_external'] is not None:
72+
resource['link_external'] = ''
4973
return ret
5074

5175
class Meta:
5276
model = Book
53-
fields = ('book_video_faculty_resources','book_orientation_faculty_resources','book_faculty_resources','audiobook_link')
54-
read_only_fields = ('book_video_faculty_resources','book_orientation_faculty_resources','book_faculty_resources','audiobook_link')
77+
fields = ('book_video_faculty_resources','book_orientation_faculty_resources','book_faculty_resources',
78+
'audiobook_link','book_student_resources')
79+
read_only_fields = ('book_video_faculty_resources','book_orientation_faculty_resources','book_faculty_resources',
80+
'audiobook_link','book_student_resources')
5581
depth=2

books/tests.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,68 @@ def test_hidden_faculty_resources_filtered_from_resources_api(self):
608608
self.assertEqual(len(response.data['book_faculty_resources']), 1)
609609
self.assertEqual(response.data['book_faculty_resources'][0]['link_text'], 'Visible')
610610

611+
def test_student_resources_available_from_resources_api(self):
612+
"""book_student_resources must be served with the same flat shape
613+
(resource_heading/resource_description/resource_unlocked) os-webview
614+
already reads from the Book page API, plus its own id, hidden filtered
615+
out, and locked links redacted only when verified (?x=y)."""
616+
with vcr.use_cassette('fixtures/vcr_cassettes/books_univ_physics.yaml'):
617+
book_index = BookIndex.objects.all()[0]
618+
root_page = Page.objects.get(title="Root")
619+
book = Book(title="University Physics",
620+
slug="university-physics-student",
621+
cnx_id='031da8d3-b525-429c-80cf-6c8ed997733a',
622+
salesforce_book_id='a0ZU0000008pyvQMAQ',
623+
description="Test Book",
624+
cover=self.test_doc,
625+
title_image=self.test_doc,
626+
publish_date=datetime.date.today(),
627+
locale=root_page.locale
628+
)
629+
book_index.add_child(instance=book)
630+
631+
locked_resource = snippets.models.StudentResource(heading="Study Guide",
632+
description="Solutions",
633+
unlocked_resource=False)
634+
locked_resource.save()
635+
636+
hidden_resource = snippets.models.StudentResource(heading="Hidden Guide",
637+
description="Hidden",
638+
unlocked_resource=True)
639+
hidden_resource.save()
640+
641+
row = BookStudentResources.objects.create(link_external="https://openstax.org/solutions",
642+
link_text="Get it", resource=locked_resource,
643+
book_student_resource=book)
644+
BookStudentResources.objects.create(link_external="https://openstax.org/hidden",
645+
link_text="Hidden", resource=hidden_resource,
646+
book_student_resource=book, hidden=True)
647+
648+
# hidden rows filtered out, flat fields present, id present
649+
response = self.client.get('/apps/cms/api/books/resources/?slug=university-physics-student')
650+
student_resources = response.data['book_student_resources']
651+
self.assertEqual(len(student_resources), 1)
652+
resource = student_resources[0]
653+
self.assertEqual(resource['id'], row.pk)
654+
self.assertEqual(resource['resource_heading'], 'Study Guide')
655+
self.assertEqual(resource['resource_description'], 'Solutions')
656+
self.assertFalse(resource['resource_unlocked'])
657+
# book back-reference cleared, matching the faculty resources shape
658+
self.assertEqual(resource['book_student_resource'], {})
659+
# unverified: locked resource's link stays intact
660+
self.assertEqual(resource['link_external'], 'https://openstax.org/solutions')
661+
662+
# verified flag on a locked resource: link redacted
663+
response = self.client.get('/apps/cms/api/books/resources/?slug=university-physics-student&x=y')
664+
self.assertEqual(response.data['book_student_resources'][0]['link_external'], '')
665+
666+
# verified flag on an unlocked resource: link stays intact
667+
locked_resource.unlocked_resource = True
668+
locked_resource.save()
669+
response = self.client.get('/apps/cms/api/books/resources/?slug=university-physics-student&x=y')
670+
self.assertEqual(response.data['book_student_resources'][0]['link_external'],
671+
'https://openstax.org/solutions')
672+
611673
def test_hidden_faculty_resources_filtered_from_pages_api(self):
612674
"""HiddenFilterChildRelationField excludes hidden faculty resources from Wagtail Pages API."""
613675
with vcr.use_cassette('fixtures/vcr_cassettes/books_univ_physics.yaml'):

pages/table_sources.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,10 @@ def _resource_link_cell(r):
258258
return {'text': '', 'url': ''}
259259
# Marker for os-webview's progressive-enhancement override: it resolves
260260
# the real per-user link client-side (where verified-instructor status
261-
# is visible) and matches this cell back to a resource by heading.
261+
# is visible) and matches this cell back to a resource, by id first
262+
# (equal to the books/resources/ row's own id) with heading as a
263+
# fallback for already-cached table JSON (up to 30 days old) that
264+
# predates resource_id.
262265
# book_slug is computed the same way as the fallback url above, so the
263266
# two can never drift apart.
264267
return {
@@ -272,6 +275,12 @@ def _resource_link_cell(r):
272275
# download-tracking record; it is absent from the
273276
# books/resources/ payload, so it has to ride along here.
274277
'book_id': next(iter(getattr(r, '_book_ids', [])), None),
278+
# r is the through-model row (BookFacultyResources/
279+
# BookStudentResources); its pk equals the `id` the
280+
# resources API serializes for the same row. A synthetic
281+
# row (no real pk) yields None here, and the frontend
282+
# falls back to heading matching.
283+
'resource_id': getattr(r, 'pk', None),
275284
'heading': r.resource_heading if r.resource else '',
276285
'resource_type': getattr(r, '_resource_type', ''),
277286
},

pages/tests/test_table_sources.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,9 @@ def test_locked_resource_link_points_at_book_page_no_real_url_leaks(self):
931931
marker = next(c for c in cta['config'] if c['type'] == 'resource_ref')
932932
# trackLink() needs the numeric book id; it is not in the resources API.
933933
self.assertEqual(marker['value']['book_id'], book.pk)
934+
# resource_id is the through-row pk, matching the id the
935+
# books/resources/ endpoint serializes for the same row.
936+
self.assertEqual(marker['value']['resource_id'], row.pk)
934937
serialized = json.dumps(cell)
935938
self.assertNotIn(real_url, serialized)
936939
self.assertNotIn('Download', serialized) # original CTA text must not leak either
@@ -939,6 +942,7 @@ def test_locked_resource_link_points_at_book_page_no_real_url_leaks(self):
939942
self.assertEqual(cta['config'], [{
940943
'type': 'resource_ref',
941944
'value': {'book_slug': book.slug, 'book_id': book.pk,
945+
'resource_id': row.pk,
942946
'heading': 'Locked Guide',
943947
'resource_type': 'Instructor'},
944948
}])
@@ -957,7 +961,7 @@ def test_locked_student_resource_links_to_student_tab(self):
957961
locked = StudentResource.objects.create(
958962
heading='Locked Student Guide', description='<p>x</p>',
959963
locale=book.locale, unlocked_resource=False)
960-
BookStudentResources.objects.create(
964+
row = BookStudentResources.objects.create(
961965
book_student_resource=book, resource=locked,
962966
link_external='https://example.com/secret.pdf')
963967
result = resolve_book_resources({
@@ -971,10 +975,40 @@ def test_locked_student_resource_links_to_student_tab(self):
971975
self.assertEqual(cta['config'], [{
972976
'type': 'resource_ref',
973977
'value': {'book_slug': book.slug, 'book_id': book.pk,
978+
'resource_id': row.pk,
974979
'heading': 'Locked Student Guide',
975980
'resource_type': 'Student'},
976981
}])
977982

983+
def test_resource_ref_marker_resource_id_matches_resources_api_row_id(self):
984+
# The frontend matches a marker back to a books/resources/ row by id
985+
# first (heading is only a fallback for pre-resource_id cached JSON),
986+
# so the two ids must agree.
987+
from books.models import BookFacultyResources
988+
from snippets.models import FacultyResource
989+
from pages.table_sources import resolve_book_resources
990+
from books.serializers import FacultyResourcesSerializer
991+
from django.test import RequestFactory
992+
with vcr.use_cassette('fixtures/vcr_cassettes/books_univ_physics.yaml'):
993+
book = self._make_book()
994+
locked_snippet = FacultyResource.objects.create(
995+
heading='Cross-checked Guide', description='<p>x</p>', locale=book.locale)
996+
row = BookFacultyResources.objects.create(
997+
book_faculty_resource=book, resource=locked_snippet,
998+
link_external='https://example.com/g.pdf', link_text='Go')
999+
result = resolve_book_resources({
1000+
'books': [book], 'resource_type': 'instructor', 'audience': '',
1001+
'columns': [{'field': 'link', 'header': '', 'type': ''}],
1002+
})
1003+
marker = result['rows'][0]['cells'][0]['cta'][0]['config'][0]['value']
1004+
1005+
request = RequestFactory().get('/apps/cms/api/books/resources/', {'slug': book.slug})
1006+
serializer = FacultyResourcesSerializer(book, context={'request': request})
1007+
api_row = serializer.data['book_faculty_resources'][0]
1008+
1009+
self.assertEqual(marker['resource_id'], row.pk)
1010+
self.assertEqual(marker['resource_id'], api_row['id'])
1011+
9781012
def test_book_slugs_stay_aligned_with_titles_when_row_shared(self):
9791013
# A resource shared across books merges into one row listing both books;
9801014
# _book_slugs must track _book_titles so the link targets a real book.

0 commit comments

Comments
 (0)