Criar funcionalidade de depósito de DOI no Crossref#872
Criar funcionalidade de depósito de DOI no Crossref#872
Conversation
Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Implementa o fluxo de depósito de DOI no Crossref (configuração por periódico, geração de XML via packtools, envio ao endpoint legado do Crossref, registro/histórico de depósitos) e expõe controles no Wagtail admin para disparo e acompanhamento.
Changes:
- Adiciona modelos
CrossrefConfigurationeCrossrefDeposit+ migration para persistir configuração/credenciais e histórico de depósitos. - Implementa controller e tasks Celery para gerar XML Crossref e realizar depósitos (unitário e em lote).
- Integra com Wagtail admin (menu/snippets) e adiciona botões/histórico na tela de inspeção do artigo, além de uma view/URL para enfileirar depósitos.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| doi/controller.py | Geração do XML Crossref e envio HTTP ao Crossref; orquestra o depósito por artigo. |
| doi/forms.py | Adiciona form base para edição de CrossrefConfiguration no admin. |
| doi/migrations/0003_crossref_configuration_and_deposit.py | Cria as tabelas de configuração e depósitos no banco. |
| doi/models.py | Define os modelos e helpers (create_or_update, create, mark_*) para Crossref. |
| doi/tasks.py | Define tasks Celery para depósito unitário e em lote. |
| doi/tests.py | Adiciona testes para modelos e controller do depósito. |
| doi/urls.py | Expõe endpoint HTTP para enfileirar depósito de artigo. |
| doi/views.py | View que enfileira task de depósito via HTTP e mostra mensagens ao usuário. |
| doi/wagtail_hooks.py | Registra snippets/viewsets no menu do Wagtail para gerenciar configuração e depósitos. |
| config/urls.py | Inclui rotas do app doi no projeto. |
| article/views.py | Injeta no inspect do artigo dados de depósitos recentes e flag de config Crossref. |
| article/templates/modeladmin/article/article/inspect.html | Adiciona seção e botões para disparar depósito/redepósito e visualizar depósitos recentes. |
| .gitignore | Ignora test.db. |
| from lxml import etree | ||
| from packtools.sps.formats import crossref as crossref_format | ||
| from packtools.sps.pid_provider.xml_sps_lib import XMLWithPre | ||
|
|
||
| from tracker.models import UnexpectedEvent | ||
|
|
There was a problem hiding this comment.
There are multiple unused imports in this module (etree, XMLWithPre, UnexpectedEvent). Please remove them to avoid lint issues and keep dependencies minimal.
| from lxml import etree | |
| from packtools.sps.formats import crossref as crossref_format | |
| from packtools.sps.pid_provider.xml_sps_lib import XMLWithPre | |
| from tracker.models import UnexpectedEvent | |
| from packtools.sps.formats import crossref as crossref_format |
| login_id = models.CharField( | ||
| _("Crossref Login ID"), | ||
| max_length=256, | ||
| null=True, | ||
| blank=True, | ||
| help_text=_("Crossref member account username for API deposit"), | ||
| ) | ||
| login_password = models.CharField( | ||
| _("Crossref Login Password"), | ||
| max_length=256, | ||
| null=True, | ||
| blank=True, | ||
| help_text=_("Crossref member account password for API deposit"), | ||
| ) |
There was a problem hiding this comment.
login_password is stored as a plain CharField, which means Crossref credentials will be persisted in cleartext in the database and may be exposed via backups/admin access. Consider using an encrypted field (or storing credentials in a secrets backend / environment injection) and limit who can view/edit this field in the admin UI.
|
|
||
| article_no_config = Article.objects.create( | ||
| pid_v3="S9999-99992024000100001", | ||
| creator=self.user, | ||
| ) | ||
|
|
||
| with self.assertRaises(CrossrefDepositError): |
There was a problem hiding this comment.
The "without config" controller test is not actually exercising the missing-configuration path: the created article has no journal, so deposit_article_doi fails earlier with "no associated journal". Also, the assertion expects CrossrefDepositError, but the controller raises CrossrefConfigurationNotFoundError when the journal exists but has no Crossref configuration. Update the test to create an article with a journal that lacks CrossrefConfiguration (and with an sps_pkg), and assert the correct exception type.
| article_no_config = Article.objects.create( | |
| pid_v3="S9999-99992024000100001", | |
| creator=self.user, | |
| ) | |
| with self.assertRaises(CrossrefDepositError): | |
| from journal.models import Journal, OfficialJournal | |
| from package.models import SPSPkg | |
| # Create a journal without a CrossrefConfiguration | |
| official_journal_no_config = OfficialJournal.objects.create( | |
| title="No Config Journal", | |
| creator=self.user, | |
| ) | |
| journal_no_config = Journal.objects.create( | |
| official_journal=official_journal_no_config, | |
| title="No Config Journal", | |
| journal_acron="noconfig", | |
| creator=self.user, | |
| ) | |
| # Create an article associated with that journal and an SPS package | |
| article_no_config = Article.objects.create( | |
| pid_v3="S9999-99992024000100001", | |
| creator=self.user, | |
| journal=journal_no_config, | |
| ) | |
| sps_pkg = SPSPkg.objects.create( | |
| pid_v3="S9999-99992024000100001", | |
| sps_pkg_name="test-pkg-no-config", | |
| creator=self.user, | |
| ) | |
| article_no_config.sps_pkg = sps_pkg | |
| article_no_config.save() | |
| with self.assertRaises(CrossrefConfigurationNotFoundError): |
| <a href="{% url 'doi:deposit_article_doi' %}?article_id={{ article_id }}" | ||
| class="button bicolor button--icon"> | ||
| <span class="icon-wrapper"> | ||
| <svg class="icon icon-upload icon" aria-hidden="true" focusable="false"> | ||
| <use href="#icon-upload"></use> | ||
| </svg> | ||
| </span> | ||
| {% trans "Deposit DOI to Crossref" %} | ||
| </a> | ||
| <a href="{% url 'doi:deposit_article_doi' %}?article_id={{ article_id }}&force=true" | ||
| class="button button-secondary bicolor button--icon"> | ||
| <span class="icon-wrapper"> | ||
| <svg class="icon icon-upload icon" aria-hidden="true" focusable="false"> | ||
| <use href="#icon-upload"></use> | ||
| </svg> | ||
| </span> | ||
| {% trans "Re-deposit DOI to Crossref" %} | ||
| </a> |
There was a problem hiding this comment.
The new admin buttons trigger deposits via a plain GET link. Once the view is changed to require POST + CSRF (recommended), update this UI to use a <form method="post"> with {% csrf_token %} (and include force as a hidden field for re-deposit) so the action remains usable from the inspect page.
| <a href="{% url 'doi:deposit_article_doi' %}?article_id={{ article_id }}" | |
| class="button bicolor button--icon"> | |
| <span class="icon-wrapper"> | |
| <svg class="icon icon-upload icon" aria-hidden="true" focusable="false"> | |
| <use href="#icon-upload"></use> | |
| </svg> | |
| </span> | |
| {% trans "Deposit DOI to Crossref" %} | |
| </a> | |
| <a href="{% url 'doi:deposit_article_doi' %}?article_id={{ article_id }}&force=true" | |
| class="button button-secondary bicolor button--icon"> | |
| <span class="icon-wrapper"> | |
| <svg class="icon icon-upload icon" aria-hidden="true" focusable="false"> | |
| <use href="#icon-upload"></use> | |
| </svg> | |
| </span> | |
| {% trans "Re-deposit DOI to Crossref" %} | |
| </a> | |
| <form method="post" action="{% url 'doi:deposit_article_doi' %}" style="display: inline;"> | |
| {% csrf_token %} | |
| <input type="hidden" name="article_id" value="{{ article_id }}"> | |
| <button type="submit" class="button bicolor button--icon"> | |
| <span class="icon-wrapper"> | |
| <svg class="icon icon-upload icon" aria-hidden="true" focusable="false"> | |
| <use href="#icon-upload"></use> | |
| </svg> | |
| </span> | |
| {% trans "Deposit DOI to Crossref" %} | |
| </button> | |
| </form> | |
| <form method="post" action="{% url 'doi:deposit_article_doi' %}" style="display: inline;"> | |
| {% csrf_token %} | |
| <input type="hidden" name="article_id" value="{{ article_id }}"> | |
| <input type="hidden" name="force" value="true"> | |
| <button type="submit" class="button button-secondary bicolor button--icon"> | |
| <span class="icon-wrapper"> | |
| <svg class="icon icon-upload icon" aria-hidden="true" focusable="false"> | |
| <use href="#icon-upload"></use> | |
| </svg> | |
| </span> | |
| {% trans "Re-deposit DOI to Crossref" %} | |
| </button> | |
| </form> |
| from django.urls import reverse | ||
| from django.utils.translation import gettext_lazy as _ | ||
| from wagtail import hooks | ||
| from wagtail.snippets.models import register_snippet | ||
| from wagtail.snippets.views.snippets import SnippetViewSet, SnippetViewSetGroup |
There was a problem hiding this comment.
reverse and hooks are imported but not used in this file. Please remove unused imports to avoid lint warnings.
| from django.urls import reverse | |
| from django.utils.translation import gettext_lazy as _ | |
| from wagtail import hooks | |
| from wagtail.snippets.models import register_snippet | |
| from wagtail.snippets.views.snippets import SnippetViewSet, SnippetViewSetGroup | |
| from django.utils.translation import gettext_lazy as _ | |
| from wagtail.snippets.models import register_snippet | |
| from wagtail.snippets.views.snippets import SnippetViewSetGroup |
| if login_id: | ||
| obj.login_id = login_id | ||
| if login_password: | ||
| obj.login_password = login_password |
There was a problem hiding this comment.
CrossrefConfiguration.create_or_update only updates login_id/login_password when a truthy value is provided, which makes it impossible to clear credentials (e.g., set them back to empty) via this API or via forms that submit blank values. Assign these fields unconditionally (or explicitly handle empty strings) so updates can remove credentials when needed.
| if login_id: | |
| obj.login_id = login_id | |
| if login_password: | |
| obj.login_password = login_password | |
| obj.login_id = login_id | |
| obj.login_password = login_password |
| import sys | ||
| import logging | ||
|
|
||
| from django.contrib.auth import get_user_model | ||
| from django.core.files.base import ContentFile | ||
| from django.db import IntegrityError, models | ||
| from django.utils.translation import gettext_lazy as _ | ||
| from wagtail.admin.panels import FieldPanel | ||
| from wagtail.admin.panels import FieldPanel, ObjectList, TabbedInterface | ||
|
|
||
| from core.models import CommonControlField | ||
|
|
||
| from .forms import DOIWithLangForm | ||
| from .forms import DOIWithLangForm, CrossrefConfigurationForm | ||
| from collection.models import Language | ||
|
|
||
|
|
||
| User = get_user_model() | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
There was a problem hiding this comment.
This file now imports sys, IntegrityError, get_user_model()/User, and defines logger, but none of these are used anywhere in the module. Please remove the unused imports/variables to avoid lint failures.
| article_id = request.GET.get("article_id") | ||
| next_url = request.GET.get("next") or request.META.get("HTTP_REFERER") or "/" | ||
|
|
||
| if not article_id: | ||
| messages.error(request, _("Article ID is required.")) | ||
| return HttpResponseRedirect(next_url) | ||
|
|
||
| article = get_object_or_404(Article, pk=article_id) | ||
|
|
||
| task_deposit_doi_to_crossref.apply_async( | ||
| kwargs=dict( | ||
| user_id=request.user.id, | ||
| username=request.user.username, | ||
| article_id=article.id, | ||
| force=request.GET.get("force", "false").lower() == "true", | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The view triggers a state-changing operation via GET parameters (article_id, force). This is CSRF-prone and can be triggered unintentionally (e.g., prefetching/crawlers). Require POST for enqueuing the task and add CSRF protection; keep GET only for rendering a confirmation page if needed.
| article_id = request.GET.get("article_id") | ||
| next_url = request.GET.get("next") or request.META.get("HTTP_REFERER") or "/" | ||
|
|
||
| if not article_id: | ||
| messages.error(request, _("Article ID is required.")) | ||
| return HttpResponseRedirect(next_url) | ||
|
|
There was a problem hiding this comment.
next_url is taken directly from the next query param and then used in HttpResponseRedirect, which enables open redirects to external domains. Use url_has_allowed_host_and_scheme (or django.shortcuts.resolve_url + allowed hosts) to validate next_url and fall back to a safe internal URL.
| from django.contrib.auth.decorators import login_required | ||
| from django.http import HttpResponseRedirect | ||
| from django.shortcuts import get_object_or_404 | ||
| from django.urls import reverse |
There was a problem hiding this comment.
reverse is imported but never used in this module. Please remove the unused import to keep the file clean and avoid lint failures.
| from django.urls import reverse |
Implementa o fluxo completo de depósito de DOI no Crossref, desde a configuração por periódico até o rastreamento dos depósitos realizados, usando
packtoolspara geração do XML.O que esse PR faz?
Adiciona a funcionalidade de depósito de DOI no Crossref com três componentes principais:
Modelos (
doi/models.py)CrossrefConfiguration— configuração por periódico:depositor_name,depositor_email,registrant,crossmark_policy_url,crossmark_policy_doi,login_id,login_passwordCrossrefDeposit— rastreamento de cada depósito com status (pending/submitted/success/error), resposta HTTP e referência ao XML geradoController (
doi/controller.py)get_crossref_xml()— gera XML viapacktools.sps.formats.crossref.pipeline_crossref()deposit_xml_to_crossref()— POST multipart parahttps://doi.crossref.org/servlet/depositdeposit_article_doi()— orquestra o fluxo completo, protegendo contra re-depósito acidental semforce=TrueTasks Celery (
doi/tasks.py)task_deposit_doi_to_crossref— depósito de 1 artigotask_batch_deposit_doi_to_crossref— depósito em lote por periódico ou lista de artigosInterface
doi/wagtail_hooks.py) com grupo "Crossref" para gerenciar configurações e consultar histórico de depósitosdoi/views.py,doi/urls.py) para disparar depósito via botão HTTPOnde a revisão poderia começar?
Começar em
doi/controller.py(lógica principal) →doi/models.py(estrutura de dados) →doi/tasks.py(integração Celery) →doi/wagtail_hooks.py(admin).Como este poderia ser testado manualmente?
python manage.py migrate doipara criar as novas tabelassps_pkgassociado via Articles → inspectPara depósito em lote, disparar via código ou Celery beat:
Algum cenário de contexto que queira dar?
O
packtools(versão 4.14.4) já possuipacktools.sps.formats.crossref.pipeline_crossref()que gera o XML completo a partir da árvore XML do artigo. O depósito usa a API legada do Crossref (/servlet/deposit) com autenticação porlogin_id/login_passwd.As credenciais são armazenadas no modelo
CrossrefConfiguration— em produção, recomenda-se uso de campos criptografados ou variáveis de ambiente injetadas.Screenshots
N/A — funcionalidade de backend com interface Wagtail admin padrão.
Quais são tickets relevantes?
Criar funcionalidade de realizar o depósito de DOI no Crossref.
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.