Skip to content

Book on flex #1625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
May 14, 2025
8 changes: 5 additions & 3 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ jobs:

services:
postgres:
image: postgres:13
image: postgres:16
ports:
- 5432:5432
options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5
env:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: postgres
POSTGRES_DB: oscms_test

steps:
- uses: actions/checkout@v4
Expand All @@ -35,8 +37,8 @@ jobs:
cache-dependency-path: requirements/test.txt
- run: pip install -r requirements/test.txt

- name: Check migrations
run: python manage.py makemigrations --check
#- name: Check migrations
# run: python manage.py makemigrations --check

- name: Run tests and generate coverage reports
run: coverage run --source '.' manage.py test --settings=openstax.settings.test
Expand Down
88 changes: 52 additions & 36 deletions books/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,55 @@
return cleantext


def prefetch_book_resources(queryset):
"""Prefetch related faculty and student resources for a queryset of books."""
return queryset.prefetch_related(

Check warning on line 37 in books/models.py

View check run for this annotation

Codecov / codecov/patch

books/models.py#L37

Added line #L37 was not covered by tests
models.Prefetch('bookfacultyresources_set', queryset=BookFacultyResources.objects.all(), to_attr='prefetched_faculty_resources'),
models.Prefetch('bookstudentresources_set', queryset=BookStudentResources.objects.all(), to_attr='prefetched_student_resources')
)

def get_book_data(book):
has_faculty_resources = hasattr(book, 'prefetched_faculty_resources') and bool(book.prefetched_faculty_resources)
has_student_resources = hasattr(book, 'prefetched_student_resources') and bool(book.prefetched_student_resources)
try:
return {

Check warning on line 46 in books/models.py

View check run for this annotation

Codecov / codecov/patch

books/models.py#L43-L46

Added lines #L43 - L46 were not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is what's returned on the current subjects page - let me know if you need other fields (or don't need them anymore 🔥)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the book tiles, the FE only needs id, cover_url, title, and slug. It pulls the rest of the details from the main book query (which is used for the subjects page).
Book tiles are currently generated from the book lists included in the specific-subjects page, so that's the model of list to use. The query for that is

`pages/${slug}-books?type=pages.Subject`

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RoyEJohnson it doesn't make a separate query for every book tile does it?

Copy link
Member Author

@mwvolo mwvolo May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure it does, the console log was spitting out a constant stream of
Can't load Microbiology
Can't load A&P
...

or it might have been subjects... but it was a lot of stuff
i bet it is subjects, considering the endpoint being called

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok well id rather include extra data and have the option of changing it to like... Not do a separate query per tile if that's what it does

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a separate query to the CMS for each book. The main subjects query is pulled once, and is searched for the id of whatever book we're looking for.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But will that work if we're searching for arbitrary books and we're not in the context of a particular subject? The API endpoint that you quoted before looked like it needed to be about a particular subject?

Copy link
Member Author

@mwvolo mwvolo May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's a test page with the book block, it has nothing to do with subject (beyond it returning in the response), it's just returning book metadata
https://dev.openstax.org/apps/cms/api/v2/pages/562/
https://dev.openstax.org/admin/pages/562/edit/

'id': book.id,
'slug': f'books/{book.slug}',
'book_state': book.book_state,
'title': book.title,
'subjects': book.subjects(),
'subject_categories': book.subject_categories,
'k12subject': book.k12subjects(),
'is_ap': book.is_ap,
'is_hs': 'High School' in book.subjects(),
'cover_url': book.cover_url,
'cover_color': book.cover_color,
'high_resolution_pdf_url': book.high_resolution_pdf_url,
'low_resolution_pdf_url': book.low_resolution_pdf_url,
'ibook_link': book.ibook_link,
'ibook_link_volume_2': book.ibook_link_volume_2,
'webview_link': book.webview_link,
'webview_rex_link': book.webview_rex_link,
'bookshare_link': book.bookshare_link,
'kindle_link': book.kindle_link,
'amazon_coming_soon': book.amazon_coming_soon,
'amazon_link': book.amazon_link,
'bookstore_coming_soon': book.bookstore_coming_soon,
'comp_copy_available': book.comp_copy_available,
'salesforce_abbreviation': book.salesforce_abbreviation,
'salesforce_name': book.salesforce_name,
'urls': book.book_urls(),
'last_updated_pdf': book.last_updated_pdf,
'has_faculty_resources': has_faculty_resources,
'has_student_resources': has_student_resources,
'assignable_book': book.assignable_book,
'promote_tags': [snippet.value.name for snippet in book.promote_snippet],
}
except Exception as e:
capture_exception(e)
return None

Check warning on line 81 in books/models.py

View check run for this annotation

Codecov / codecov/patch

books/models.py#L79-L81

Added lines #L79 - L81 were not covered by tests


class VideoFacultyResource(models.Model):
resource_heading = models.CharField(max_length=255)
resource_description = RichTextField(blank=True, null=True)
Expand Down Expand Up @@ -1101,42 +1150,9 @@
books = Book.objects.live().filter(locale=self.locale).exclude(book_state='unlisted').order_by('title')
book_data = []
for book in books:
has_faculty_resources = BookFacultyResources.objects.filter(book_faculty_resource=book).exists()
has_student_resources = BookStudentResources.objects.filter(book_student_resource=book).exists()
try:
book_data.append({
'id': book.id,
'cnx_id': book.cnx_id,
'slug': 'books/{}'.format(book.slug),
'book_state': book.book_state,
'title': book.title,
'subjects': book.subjects(),
'is_ap': book.is_ap,
'cover_url': book.cover_url,
'cover_color': book.cover_color,
'high_resolution_pdf_url': book.high_resolution_pdf_url,
'low_resolution_pdf_url': book.low_resolution_pdf_url,
'ibook_link': book.ibook_link,
'ibook_link_volume_2': book.ibook_link_volume_2,
'webview_link': book.webview_link,
'webview_rex_link': book.webview_rex_link,
'bookshare_link': book.bookshare_link,
'kindle_link': book.kindle_link,
'amazon_coming_soon': book.amazon_coming_soon,
'amazon_link': book.amazon_link,
'bookstore_coming_soon': book.bookstore_coming_soon,
'comp_copy_available': book.comp_copy_available,
'salesforce_abbreviation': book.salesforce_abbreviation,
'salesforce_name': book.salesforce_name,
'urls': book.book_urls(),
'last_updated_pdf': book.last_updated_pdf,
'has_faculty_resources': has_faculty_resources,
'has_student_resources': has_student_resources,
'assignable_book': book.assignable_book,
'promote_tags': [snippet.value.name for snippet in book.promote_snippet],
})
except Exception as e:
capture_exception(e)
data = get_book_data(book)
if data:
book_data.append(data)

Check warning on line 1155 in books/models.py

View check run for this annotation

Codecov / codecov/patch

books/models.py#L1153-L1155

Added lines #L1153 - L1155 were not covered by tests
return book_data

content_panels = Page.content_panels + [
Expand Down
4 changes: 4 additions & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,7 @@ ignore:
- '*/settings/*'
- '*settings.py'
- '*wsgi.py'
- '*/__init__.py'
- '*/urls.py'
- 'versions/*' # this is internal only, and not mission critical
- 'wagtailimportexport/*' # this doesn't work, candidate for removal / offloading / rewriting
11 changes: 11 additions & 0 deletions openstax/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@
'host': 'test',
}

DATABASES = {
'default': {
'ENGINE': "django.db.backends.postgresql",
'NAME': os.getenv('DATABASE_NAME', 'oscms_test'),
'USER': os.getenv('DATABASE_USER', 'postgres'),
'PASSWORD': os.getenv('DATABASE_PASSWORD', 'postgres'),
'HOST': os.getenv('DATABASE_HOST', 'localhost'),
'PORT': os.getenv('DATABASE_PORT', '5432'),
}
}

# silence whitenoise warnings for CI
import warnings
warnings.filterwarnings("ignore", message="No directory at", module="whitenoise.base")
13 changes: 13 additions & 0 deletions pages/custom_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from api.serializers import ImageSerializer
from openstax.functions import build_image_url, build_document_url
from wagtail.rich_text import expand_db_html
from books.models import get_book_data


class APIRichTextBlock(blocks.RichTextBlock):
Expand Down Expand Up @@ -144,6 +145,7 @@
name = blocks.CharBlock(help_text="The name of the person or entity to attribute the quote to.")
title = blocks.CharBlock(requred=False, help_text="Additional title or label about the quotee.")


class DividerBlock(StructBlock):
image = APIImageChooserBlock()
config = blocks.StreamBlock([
Expand Down Expand Up @@ -323,3 +325,14 @@
'cover': build_document_url(value['cover'].url),
'title': value['title'],
}

class BookListBlock(blocks.StreamBlock):
books = blocks.PageChooserBlock(page_type=['books.Book'], required=False)

class Meta:
icon = 'placeholder'
label = "Book List"

def get_api_representation(self, value, context=None):
# value is a StreamValue of blocks, each with .value as a Book page
return [book_data for book in value if (book_data := get_book_data(book.value))]

Check warning on line 338 in pages/custom_blocks.py

View check run for this annotation

Codecov / codecov/patch

pages/custom_blocks.py#L338

Added line #L338 was not covered by tests
Loading