[WIP] Add update policy page functionality for Crossmark#1373
[WIP] Add update policy page functionality for Crossmark#1373robertatakenaka merged 3 commits intomainfrom
Conversation
…l, choices, and migration Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
journal/models.py
Outdated
| crossmark_policy_doi = models.CharField( | ||
| _("Crossmark Policy DOI"), | ||
| max_length=100, | ||
| blank=True, | ||
| null=True, | ||
| help_text=_( | ||
| "DOI of the crossmark update policy page for this journal" | ||
| ), | ||
| ) |
There was a problem hiding this comment.
@copilot este campo
crossmark_policy_doi = models.CharField(
_("Crossmark Policy DOI"),
max_length=100,
blank=True,
null=True,
help_text=_(
"DOI of the crossmark update policy page for this journal"
),
)deveria ser movido para dentro de UpdatePolicy
journal/models.py
Outdated
| crossmark_doi_is_active = models.BooleanField( | ||
| verbose_name=_("Crossmark DOI is active"), | ||
| default=False, | ||
| help_text=_( | ||
| "Indicates whether the crossmark policy DOI page is active" | ||
| ), | ||
| ) |
There was a problem hiding this comment.
@copilot mover este campo crossmark_doi_is_active para UpdatePolicy e aqui no lugar de campo criar property que retorna o resultado de UpdatePolicy.objects.filter(crossmark_doi_is_active=True).exists()
journal/models.py
Outdated
| and other update types registered with Crossmark. | ||
| """ | ||
|
|
||
| policy_type = models.CharField( |
There was a problem hiding this comment.
@copilot eliminar policy_type e códigos relacionados
journal/models.py
Outdated
| ) | ||
|
|
||
|
|
||
| class UpdatePolicy(Orderable, RichTextWithLanguage, CommonControlField): |
There was a problem hiding this comment.
@copilot prefiro adotar o nome CrossmarkPolicy no lugar de UpdatePolicy
|
@copilot resolva as questões |
…lização (crossmark) (#1381) * chore(journal): remove migração 0057 gerada para crossmark update policy Remove a migration 0057 que adicionava os campos crossmark_policy_doi e crossmark_doi_is_active diretamente no modelo Journal e criava o modelo UpdatePolicy. Esses campos e modelo serão substituídos pela nova abordagem com CrossmarkPolicy. * feat(journal): adiciona migração para criação do modelo CrossmarkPolicy Cria o modelo CrossmarkPolicy com os campos doi, is_active, url, language, journal (ParentalKey com CASCADE), creator e updated_by. Substitui a abordagem anterior que armazenava crossmark_policy_doi e crossmark_doi_is_active diretamente no modelo Journal. * refactor(journal): substitui UpdatePolicy por CrossmarkPolicy e converte campo para property - Renomeia modelo UpdatePolicy para CrossmarkPolicy com campos doi, is_active e url - Remove campos crossmark_policy_doi e crossmark_doi_is_active do modelo Journal - Adiciona property crossmark_doi_is_active em Journal que retorna True se houver alguma CrossmarkPolicy ativa (is_active=True) - Altera related_name de update_policy para crossmark_policy - Altera on_delete de SET_NULL para CASCADE e torna journal e url obrigatórios * refactor(journal): atualiza proxys para usar crossmark_policy no lugar de update_policy - Substitui InlinePanel update_policy por crossmark_policy em JournalProxyEditor e JournalProxyPanelPolicy - Remove FieldPanels crossmark_policy_doi e crossmark_doi_is_active, agora gerenciados via CrossmarkPolicy inline
There was a problem hiding this comment.
Pull request overview
Adds initial data model and admin UI hooks to manage a journal’s Crossmark “Update Policy” information (DOI/active flag/URL and rich text), as a foundation for later Crossref/Crossmark registration work.
Changes:
- Introduces
CrossmarkPolicymodel (with DOI, active flag, URL, language, rich text) and a DB migration. - Exposes
crossmark_policyas an InlinePanel in journal proxy editor/policy admin panels. - Adds
Journal.crossmark_doi_is_activehelper and a newUPDATE_POLICY_TYPEchoices list.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
journal/proxys.py |
Adds crossmark_policy InlinePanel to editor and policy panel configurations. |
journal/models.py |
Adds CrossmarkPolicy model and crossmark_doi_is_active property on Journal. |
journal/migrations/0057_crossmarkpolicy.py |
Creates the CrossmarkPolicy table and relations. |
journal/choices.py |
Adds UPDATE_POLICY_TYPE choice list (currently unused). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class CrossmarkPolicy(Orderable, RichTextWithLanguage, CommonControlField): | ||
| """ | ||
| Update Policy (Crossmark) - policies for correction, retraction, withdrawal | ||
| and other update types registered with Crossmark. | ||
| """ | ||
| doi = models.CharField( | ||
| max_length=255, | ||
| null=True, | ||
| blank=True, | ||
| ) | ||
| is_active = models.BooleanField( | ||
| verbose_name=_("Crossmark DOI is active"), | ||
| default=False, | ||
| help_text=_( | ||
| "Indicates whether the crossmark policy DOI page is active" | ||
| ), | ||
| ) | ||
| url = models.URLField( | ||
| _("Policy URL"), | ||
| null=False, | ||
| blank=False, | ||
| help_text=_( | ||
| "URL of the policy page describing how the journal handles this type of update" | ||
| ), | ||
| ) |
There was a problem hiding this comment.
CrossmarkPolicy is described as covering multiple update types (correction/retraction/withdrawal/etc.), and this PR also introduces UPDATE_POLICY_TYPE choices, but the model has no field to capture which update type a given policy entry represents. Without a type field, multiple CrossmarkPolicy rows for a journal will be indistinguishable beyond free-text. Consider adding a policy_type/update_type field (with choices=UPDATE_POLICY_TYPE) or removing the unused choices if the intent is a single generic policy page.
|
|
||
| @property | ||
| def crossmark_doi_is_active(self): | ||
| return self.crossmark_policy.filter(is_active=True).exists() |
There was a problem hiding this comment.
crossmark_doi_is_active currently only checks for any related CrossmarkPolicy with is_active=True, but it doesn’t verify that a DOI value is actually present. As a result, the property can return True even when the active record has doi empty/null, which seems inconsistent with the property name and could lead to incorrect downstream behavior. Consider also filtering for non-empty DOI (or rename the property to reflect that it’s only the flag).
| return self.crossmark_policy.filter(is_active=True).exists() | |
| return ( | |
| self.crossmark_policy.filter(is_active=True) | |
| .exclude(doi__isnull=True) | |
| .exclude(doi__exact="") | |
| .exists() | |
| ) |
| ), | ||
| ) | ||
| journal = ParentalKey( | ||
| Journal, on_delete=models.CASCADE, related_name="crossmark_policy", null=False |
There was a problem hiding this comment.
All other Orderable, RichTextWithLanguage, CommonControlField journal-policy models in this section use ParentalKey(Journal, on_delete=models.SET_NULL, null=True, ...), but CrossmarkPolicy is the only one using on_delete=models.CASCADE and null=False. This inconsistency can make lifecycle/cleanup behavior differ from the rest of the journal policy inlines. Consider aligning journal with the existing pattern (and updating the migration accordingly) unless there’s a strong reason for CrossmarkPolicy to behave differently.
| Journal, on_delete=models.CASCADE, related_name="crossmark_policy", null=False | |
| Journal, on_delete=models.SET_NULL, related_name="crossmark_policy", null=True |
Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.