Reorganizar abas do formulário de Journal: separar abas administrativas#1311
Reorganizar abas do formulário de Journal: separar abas administrativas#1311robertatakenaka merged 4 commits intomainfrom
Conversation
…and create separate admin viewset Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
…on naming Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the Wagtail admin editing experience for Journals by reorganizing the main tab layout, splitting “Legacy Compatibility” and “Notes” into a new admin-only section, and adding regression tests for tab structure.
Changes:
- Removed “Legacy Compatibility” and “Notes” tabs from the main
Journaledit handler. - Added a new proxy model (
JournalProxyAdminOnly) and a corresponding snippet viewset entry to surface admin-only tabs. - Added tests to validate tab presence/order across the main model and proxy models.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
journal/models.py |
Removes admin-only tabs from the primary Journal tabbed interface. |
journal/proxys.py |
Introduces an admin-only proxy model containing Legacy Compatibility + Notes panels. |
journal/wagtail_hooks.py |
Registers a new snippet viewset and permissions hook for the admin-only proxy. |
journal/tests.py |
Adds tests asserting the expected tab structure and ordering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def get_queryset(self, request): | ||
| # Only allow superusers to access this viewset | ||
| user = request.user | ||
| if not user.is_authenticated or not user.is_superuser: | ||
| return models.Journal.objects.none() | ||
|
|
||
| # For superusers, return all journals with optimizations | ||
| return super().get_queryset(request) |
There was a problem hiding this comment.
This only filters the queryset for non-superusers, but does not actually enforce the "superuser-only" access requirement: non-superusers can still hit the endpoints and get a 200 with an empty list, and the create view may still be reachable if permissions are granted (it doesn't rely on get_queryset). Enforce superuser-only access at the view/permission level (e.g., raising PermissionDenied for any non-superuser request across list/add/edit/inspect and hiding the menu item accordingly), instead of relying on returning an empty queryset.
| panels_legacy_compatibility_fields = [ | ||
| FieldPanel("alphabet"), | ||
| FieldPanel("classification"), | ||
| FieldPanel("national_code"), | ||
| FieldPanel("type_of_literature"), | ||
| FieldPanel("treatment_level"), | ||
| FieldPanel("level_of_publication"), | ||
| FieldPanel("center_code"), | ||
| FieldPanel("identification_number"), | ||
| FieldPanel("ftp"), | ||
| FieldPanel("user_subscription"), | ||
| FieldPanel("subtitle"), | ||
| FieldPanel("section"), | ||
| FieldPanel("has_supplement"), | ||
| FieldPanel("is_supplement"), | ||
| FieldPanel("acronym_letters"), | ||
| ] | ||
|
|
||
| panels_notes = [InlinePanel("annotation", label=_("Notes"))] | ||
|
|
There was a problem hiding this comment.
panels_legacy_compatibility_fields / panels_notes duplicate panel definitions that already exist on journal.models.Journal. Duplicating these lists increases the chance of the admin-only form drifting from the canonical field set over time; consider reusing the base model’s panel lists (or extracting shared constants) so there is a single source of truth.
| panels_legacy_compatibility_fields = [ | |
| FieldPanel("alphabet"), | |
| FieldPanel("classification"), | |
| FieldPanel("national_code"), | |
| FieldPanel("type_of_literature"), | |
| FieldPanel("treatment_level"), | |
| FieldPanel("level_of_publication"), | |
| FieldPanel("center_code"), | |
| FieldPanel("identification_number"), | |
| FieldPanel("ftp"), | |
| FieldPanel("user_subscription"), | |
| FieldPanel("subtitle"), | |
| FieldPanel("section"), | |
| FieldPanel("has_supplement"), | |
| FieldPanel("is_supplement"), | |
| FieldPanel("acronym_letters"), | |
| ] | |
| panels_notes = [InlinePanel("annotation", label=_("Notes"))] | |
| panels_legacy_compatibility_fields = Journal.panels_legacy_compatibility_fields | |
| panels_notes = Journal.panels_notes |
| # Get the tabs from edit_handler | ||
| tabs = Journal.edit_handler.children | ||
| tab_headings = [tab.heading for tab in tabs] | ||
|
|
||
| # Verify expected tabs are present | ||
| self.assertIn("Title", tab_headings) | ||
| self.assertIn("Institutions", tab_headings) | ||
| self.assertIn("Website", tab_headings) | ||
| self.assertIn("Focus and Scope", tab_headings) | ||
| self.assertIn("Open Science", tab_headings) | ||
|
|
||
| # Verify admin-only tabs are NOT present | ||
| self.assertNotIn("Legacy Compatibility", tab_headings) | ||
| self.assertNotIn("Notes", tab_headings) |
There was a problem hiding this comment.
tab.heading values come from gettext_lazy and will be lazy translation objects; with LANGUAGE_CODE = pt-br these assertions against hard-coded English strings are likely to fail (and may also fail due to lazy-object equality semantics). Convert headings to str(...) and build expected values using the same translation mechanism (e.g., str(_("Title")), or run the test under translation.override(...)) so the test is locale- and lazy-safe.
| # Get the tabs from edit_handler | ||
| tabs = JournalProxyEditor.edit_handler.children | ||
| tab_headings = [tab.heading for tab in tabs] | ||
|
|
||
| # Verify tab order | ||
| expected_order = [ | ||
| "Title", | ||
| "Institutions", | ||
| "Website", | ||
| "Focus and Scope", | ||
| "Open Science", | ||
| "Editorial Policy", | ||
| "Instructions for Authors", | ||
| "Editorial Board", | ||
| ] | ||
|
|
||
| self.assertEqual(tab_headings, expected_order) |
There was a problem hiding this comment.
Same issue here: tab_headings will likely be a list of lazy translation objects, so comparing directly to a list of English strings can fail under non-English locales. Consider normalizing with str(tab.heading) (and generating expected_order via str(_(...)) or within a language override) before asserting exact order.
O que esse PR faz?
Reestrutura as abas do formulário de Journal, movendo "Legacy Compatibility" e "Notas" para um menu exclusivo de administradores (superusers), mantendo apenas abas essenciais no modelo principal.
Mudanças principais:
journal/models.py: Removidas abasLegacy CompatibilityeNotesdo modelo baseJournaljournal/proxys.py: CriadoJournalProxyAdminOnlycom apenas as duas abas administrativasjournal/wagtail_hooks.py: ImplementadoJournalAdminOnlySnippetViewSetcom restriçãois_superusernoget_queryset()journal/tests.py: Adicionados testes unitários para validar estrutura de abasOrdem final das abas:
Onde a revisão poderia começar?
journal/models.pylinha 710 (remoção das abas do edit_handler), seguido porjournal/proxys.pylinha 350 (novo proxy model) ejournal/wagtail_hooks.pylinha 331 (viewset com restrição de acesso).Como este poderia ser testado manualmente?
python manage.py test journal.tests.JournalTabStructureTestCaseAlgum cenário de contexto que queira dar?
A separação de abas administrativas otimiza o carregamento do formulário para editores regulares, reduzindo de 7 para 5 abas no modelo principal. O acesso às configurações legadas permanece disponível para administradores via proxy model dedicado com controle de acesso no nível de queryset.
Screenshots
N/A - Mudanças estruturais no admin backend
Quais são tickets relevantes?
#1244 (parent issue)
Referências
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.