CORE-2402 follow-up: table-block manual-entry redesign#1745
Open
mwvolo wants to merge 7 commits into
Open
Conversation
…ypedTableBlock shape
…plicating book_resources Test fixtures nest the table block inside a section (as real content does) — table is not a valid top-level RootPage.body block, so StreamBlock.to_python silently drops any raw block placed directly at the top level.
Task 2's data_source help-text edit changed TableBlock's deconstructed representation but was never captured in a migration. makemigrations --check caught the drift in news.PressIndex.faqs and 4 pages-app fields that nest the table block. No schema/data change — help text only.
FAQBlock.content (used by PressIndex.faqs) can nest the same 'table' block as RootPage.body, but 0200_migrate_table_block_manual_rows only walked RootPage. Extract _migrate_stream_field(pages, field_name) so the same walk/convert/save logic runs for both RootPage.body and PressIndex.faqs, and add a cross-app dependency on news.0058_alter_pressindex_faqs so this always runs after PressIndex.faqs is switched to the new manual/ TypedTableBlock shape.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| except KeyError: | ||
| return Response({'error': f'Unknown source type: {source_type}'}) | ||
| except Exception as e: | ||
| return Response({'error': str(e)}) |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the FlexPage table block to use Wagtail’s TypedTableBlock for manual authoring while keeping the renderer/API output contract ({caption, columns, rows, config}) stable. It also adds a staff-only DRF preview endpoint for dynamic table sources, a report-only management command to flag likely “book resources” manual duplicates, and migrations/tests to convert existing stored table data into the new manual shape (including nested occurrences like news.PressIndex.faqs).
Changes:
- Replace the table block’s manual entry shape with
TypedTableBlockand adapt API serialization back to the existing renderer contract. - Add a staff-only
/apps/cms/api/v2/pages/table-block/preview/endpoint to preview dynamic source output (rows capped at 5). - Add data migrations + tests to convert old
{columns, rows}manual tables to the new{manual: {columns, rows}}storage format, and add a dry-run finder command to flag likely book-resources duplicates.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pages/table_block.py | Switches manual table authoring to TypedTableBlock and reshapes its API representation to the renderer contract. |
| pages/table_sources.py | Updates module docstring to reflect the new manual-table API shape language. |
| pages/migrations/0200_migrate_table_block_manual_rows.py | RunPython migration to convert stored old-shape manual tables to the new manual field shape across StreamField content. |
| pages/migrations/0201_alter_assignable_faqs_alter_faq_questions_and_more.py | Auto-generated StreamField schema migration reflecting the new table block definition in page/FAQ-related StreamFields. |
| news/migrations/0058_alter_pressindex_faqs.py | Auto-generated schema migration reflecting the updated nested table block in PressIndex.faqs. |
| pages/management/commands/find_resource_tables.py | Adds a dry-run management command to detect manual tables that resemble book_resources output. |
| pages/admin_views.py | Adds DRF TablePreviewView to preview resolved dynamic table data sources. |
| pages/urls.py | Wires the table preview endpoint under preview/. |
| openstax/urls.py | Includes the new pages.urls under /apps/cms/api/v2/pages/table-block/. |
| openstax/settings/base.py | Adds wagtail.contrib.typed_table_block to INSTALLED_APPS. |
| pages/tests/test_table_block.py | Updates manual table tests for the new manual shape and adds coverage for date/rich-text/CTA serialization. |
| pages/tests/test_table_preview_view.py | Adds endpoint tests for authz, unknown source handling, and 5-row cap behavior. |
| pages/tests/test_migrate_table_block_manual_rows.py | Adds unit/integration-style tests for migration transforms and PressIndex dispatch coverage. |
| pages/tests/test_find_resource_tables.py | Tests the finder command output and non-flag cases (no CTA / has data_source). |
| pages/tests/test_faq_block.py | Updates FAQ content tests to use the new manual table storage shape. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
242
to
248
| rep = super().get_api_representation(value, context) | ||
| manual_rep = rep.pop('manual') | ||
| rep.pop('data_source', None) | ||
| stream = value.get('data_source') | ||
| if not stream: | ||
| rep.update(_manual_table_representation(manual_rep, table_sources.RENDERER_COLUMN_TYPES)) | ||
| return rep |
Comment on lines
+22
to
+28
| try: | ||
| data = table_sources.resolve_data_source(source_type, config) | ||
| except KeyError: | ||
| return Response({'error': f'Unknown source type: {source_type}'}) | ||
| except Exception as e: | ||
| return Response({'error': str(e)}) | ||
| return Response({'columns': data['columns'], 'rows': data['rows'][:5]}) |
This file contains hidden or 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
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.
What
Redesigns the
tableflex-page block's manual-entry experience, closes a real content-duplication gap found while scoping the work, and closes one gap the CORE-2402 acceptance criteria never got: admin-side preview of dynamic table sources.wagtail.contrib.typed_table_block.TypedTableBlock(Add row / Add column grid) instead of an accordion-of-StructBlocks, with five cell types: text, number, date, rich text, CTA. The public API contract ({caption, columns, rows, config}) consumed by the flex-pages renderer is unchanged — verified byte-for-byte against the pre-branch shape.columns/rowsshape to the new one, walking StreamField content recursively (handles arbitrary nesting inside sections/tabbed content). Also coversnews.PressIndex.faqs, which nests the sametableblock via FAQ content — found and closed during final review.find_resource_tablesmanagement command — dry-run only, finds manual tables that duplicate what the existingbook_resourcesdynamic source already generates (surfaced by auditing a real WIP page with 8 hand-duplicated tables). No auto-rewrite; flags candidates for human review./apps/cms/api/v2/pages/table-block/preview/) resolves a dynamic table source spec and returns up to 5 rows, so editors can see what a Books/Resources/etc. source will render before saving. Mounted outside the Wagtail admin urlconf (matching the existingauthoringapp's pattern), since Wagtail'sregister_admin_urlswraps everything in session-based auth that's incompatible with a token-authed DRF view.Design & plan
Full design spec and implementation plan are local-only (
docs/superpowers/, gitignored per repo convention) — summarizing here:columns/rowsListBlocks withTypedTableBlock, adapter preserves the renderer contract.text/rich_text/ctabased on existing cell content. Known, accepted limitation: oldnumber/datesort-hints are discarded (old cell content was never a real number/date to parse).find_resource_tables: report-only in v1, no--apply— book-name-to-Book-page matching is inherently fuzzy and needs a human in the loop.Verified
makemigrations --check --dry-run: clean.openstax-web-stack: since the renderer contract is unchanged, this requires noflex-pagesoros-webviewfollow-up work.Process note
Built with subagent-driven development — each task (manual field + migration, API adapter, data migration, finder command, preview endpoint, regression pass) went through an independent implementer + reviewer pass, plus a final whole-branch review. The whole-branch review caught the
news.PressIndex.faqsmigration gap, which is now closed (see the last commit).Test plan
python manage.py test --settings=openstax.settings.test— full suite passingpython manage.py makemigrations --check --dry-run— cleanfind_resource_tablesoutput against real dev/staging content, if convenient