Skip to content

refactor(journal): encapsular lógica de adição de instituições no mod… #1269

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.

Sign up for GitHub

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

Closed
robertatakenaka wants to merge 5 commits into from
Closed

refactor(journal): encapsular lógica de adição de instituições no mod… #1269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c446b7
refactor(journal): encapsular lógica de adição de instituições no mod…
robertatakenaka Feb 2, 2026
9718954
feat: permite vincular location em instituições do journal
robertatakenaka Feb 2, 2026
4342a63
feat(journal): implement OrganizationMixin and manual entry forms
robertatakenaka Feb 3, 2026
a194302
feat(journal): link history models to their respective custom form cl…
robertatakenaka Feb 3, 2026
46971ae
refactor(models): corrige persistência de organização e formatação
robertatakenaka Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
295 changes: 295 additions & 0 deletions journal/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,299 @@
from django import forms
from django.utils.translation import gettext_lazy as _

from core.forms import CoreAdminModelForm
from organization.models import Organization
from location.models import Country, Location


class OrganizationMixin:
"""
Mixin para adicionar campos de entrada manual de organização
que não estão presentes na lista padrão de Organization.
"""
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

# Adiciona campos para entrada manual de organização
self.fields['manual_org_name'] = forms.CharField(
label=_("Organization Name (Manual)"),
max_length=255,
required=False,
help_text=_("Enter organization name if not found in the list above"),
widget=forms.TextInput(attrs={'placeholder': _('Enter standardized organization name')}),
)

self.fields['manual_city'] = forms.CharField(
label=_("City"),
max_length=100,
required=False,
help_text=_("City where organization is located"),
)

self.fields['manual_state'] = forms.CharField(
label=_("State/Province"),
max_length=100,
required=False,
help_text=_("State or province where organization is located"),
)

self.fields['manual_country'] = forms.ModelChoiceField(
label=_("Country"),
queryset=Country.objects.all(),
required=False,
help_text=_("Country where organization is located"),
empty_label=_("Select country..."),
)

def clean(self):
cleaned_data = super().clean()
organization = cleaned_data.get('organization')
manual_org_name = cleaned_data.get('manual_org_name')

# Validação: deve ter organization OU manual_org_name
if not organization and not manual_org_name:
raise forms.ValidationError(
_("Please either select an organization from the list or enter manual organization data.")
)

# Se tem manual_org_name, deve ter pelo menos o país
if manual_org_name and not cleaned_data.get('manual_country'):
raise forms.ValidationError(
_("When entering manual organization data, country is required.")
)

return cleaned_data
Comment on lines +48 to +65
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The validation requires either organization or manual_org_name, but the context suggests that instances can have an 'institution' field as well (based on the model methods). The validation logic may be incomplete if it doesn't account for cases where the instance already has an institution set but neither organization nor manual data is provided. Consider checking if instance.institution exists before raising a validation error.

Copilot uses AI. Check for mistakes.

def save(self, commit=True):
instance = super().save(commit=False)

# Se não tem organization mas tem dados manuais, cria/busca organization
if not instance.organization and self.cleaned_data.get('manual_org_name'):
organization = self._create_or_get_organization()
if organization:
instance.organization = organization

if commit:
instance.save()
return instance

def _create_or_get_organization(self):
"""Cria ou busca Organization baseado nos dados manuais."""
manual_name = self.cleaned_data.get('manual_org_name')
manual_city = self.cleaned_data.get('manual_city')
manual_state = self.cleaned_data.get('manual_state')
Comment on lines +83 to +84
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The variables manual_city and manual_state are retrieved from cleaned_data but are never used in the organization creation logic. Either these should be used when creating the organization (perhaps stored in a related field or custom attributes), or they should be removed if not needed. This suggests incomplete implementation or unnecessary code.

Suggested change
manual_city = self.cleaned_data.get('manual_city')
manual_state = self.cleaned_data.get('manual_state')

Copilot uses AI. Check for mistakes.
manual_country = self.cleaned_data.get('manual_country')

if not manual_name:
return None

# Busca organization existente primeiro
existing_org = Organization.objects.filter(name=manual_name).first()
if existing_org:
return existing_org

# Cria location se tiver dados geográficos
location = None
if manual_country:
try:
# Busca ou cria location (simplificado)
location_data = {'country': manual_country}
location, created = Location.objects.get_or_create(**location_data)
except Exception:
location = None
Comment on lines +98 to +103
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

Silently catching all exceptions with bare 'except Exception' and returning None can hide legitimate errors during location creation. This makes debugging difficult. Consider logging the exception or allowing critical errors to propagate while only catching specific expected exceptions.

Copilot uses AI. Check for mistakes.

# Cria nova organization
try:
organization = Organization.objects.create(
name=manual_name,
location=location,
source='user', # Marca como fonte user
)
return organization
except Exception:
# Se não conseguir criar, retorna None - usuário terá que usar lista padrão
return None
Comment on lines +106 to +115
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

Silently catching all exceptions with bare 'except Exception' and returning None can hide legitimate errors during organization creation. This makes debugging difficult and could lead to data loss. Consider logging the exception or allowing critical errors to propagate while only catching specific expected exceptions.

Copilot uses AI. Check for mistakes.


class OwnerHistoryForm(OrganizationMixin, CoreAdminModelForm):
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

This assignment to 'OwnerHistoryForm' is unnecessary as it is redefined before this value is used.

Copilot uses AI. Check for mistakes.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Define o model dinamicamente para evitar importação circular
if not hasattr(self._meta, 'model') or self._meta.model is None:
from journal.models import OwnerHistory
self._meta.model = OwnerHistory


class PublisherHistoryForm(OrganizationMixin, CoreAdminModelForm):
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

This assignment to 'PublisherHistoryForm' is unnecessary as it is redefined before this value is used.

Copilot uses AI. Check for mistakes.
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Define o model dinamicamente para evitar importação circular
if not hasattr(self._meta, 'model') or self._meta.model is None:
from journal.models import PublisherHistory
self._meta.model = PublisherHistory


class SponsorHistoryForm(OrganizationMixin, CoreAdminModelForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Define o model dinamicamente para evitar importação circular
if not hasattr(self._meta, 'model') or self._meta.model is None:
from journal.models import SponsorHistory
self._meta.model = SponsorHistory


Comment on lines +136 to +144
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

This assignment to 'SponsorHistoryForm' is unnecessary as it is redefined before this value is used.

Suggested change
class SponsorHistoryForm(OrganizationMixin, CoreAdminModelForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Define o model dinamicamente para evitar importação circular
if not hasattr(self._meta, 'model') or self._meta.model is None:
from journal.models import SponsorHistory
self._meta.model = SponsorHistory

Copilot uses AI. Check for mistakes.
class CopyrightHolderHistoryForm(OrganizationMixin, CoreAdminModelForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Define o model dinamicamente para evitar importação circular
if not hasattr(self._meta, 'model') or self._meta.model is None:
from journal.models import CopyrightHolderHistory
self._meta.model = CopyrightHolderHistory


class SciELOJournalModelForm(CoreAdminModelForm):
"""
Mixin para adicionar campos de entrada manual de organização
que não estão presentes na lista padrão de Organization.
Comment on lines +156 to +157
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The docstring incorrectly describes this class as a "Mixin". SciELOJournalModelForm is a form class, not a mixin. The docstring should be updated to accurately describe the purpose of this form class.

Suggested change
Mixin para adicionar campos de entrada manual de organização
que não estão presentes na lista padrão de Organization.
Formulário base para modelos SciELO Journal.
Estende CoreAdminModelForm para adicionar campos de entrada manual de
organização (nome, cidade, estado e país) quando a organização não
estiver presente na lista padrão de Organization.

Copilot uses AI. Check for mistakes.
"""
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

# Adiciona campos para entrada manual de organização
self.fields['manual_org_name'] = forms.CharField(
label=_("Organization Name (Manual)"),
max_length=255,
required=False,
help_text=_("Enter organization name if not found in the list above"),
widget=forms.TextInput(attrs={'placeholder': _('Enter standardized organization name')}),
)

self.fields['manual_city'] = forms.CharField(
label=_("City"),
max_length=100,
required=False,
help_text=_("City where organization is located"),
)

self.fields['manual_state'] = forms.CharField(
label=_("State/Province"),
max_length=100,
required=False,
help_text=_("State or province where organization is located"),
)

self.fields['manual_country'] = forms.ModelChoiceField(
label=_("Country"),
queryset=Country.objects.all(),
required=False,
help_text=_("Country where organization is located"),
empty_label=_("Select country..."),
)

def clean(self):
cleaned_data = super().clean()
organization = cleaned_data.get('organization')
manual_org_name = cleaned_data.get('manual_org_name')

# Validação: deve ter organization OU manual_org_name
if not organization and not manual_org_name:
raise forms.ValidationError(
_("Please either select an organization from the list or enter manual organization data.")
)

# Se tem manual_org_name, deve ter pelo menos o país
if manual_org_name and not cleaned_data.get('manual_country'):
raise forms.ValidationError(
_("When entering manual organization data, country is required.")
)

return cleaned_data

def save(self, commit=True):
instance = super().save(commit=False)

# Se não tem organization mas tem dados manuais, cria/busca organization
if not instance.organization and self.cleaned_data.get('manual_org_name'):
organization = self._create_or_get_organization()
if organization:
instance.organization = organization

if commit:
instance.save()
return instance

def _create_or_get_organization(self):
"""Cria ou busca Organization baseado nos dados manuais."""
manual_name = self.cleaned_data.get('manual_org_name')
manual_city = self.cleaned_data.get('manual_city')
manual_state = self.cleaned_data.get('manual_state')
Comment on lines +228 to +229
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

Variable manual_city is not used.

Suggested change
manual_city = self.cleaned_data.get('manual_city')
manual_state = self.cleaned_data.get('manual_state')

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

Variable manual_state is not used.

Suggested change
manual_state = self.cleaned_data.get('manual_state')

Copilot uses AI. Check for mistakes.
manual_country = self.cleaned_data.get('manual_country')

if not manual_name:
return None

# Busca organization existente primeiro
existing_org = Organization.objects.filter(name=manual_name).first()
if existing_org:
return existing_org

# Cria location se tiver dados geográficos
location = None
if manual_country:
try:
# Busca ou cria location (simplificado)
location_data = {'country': manual_country}
location, created = Location.objects.get_or_create(**location_data)
except Exception:
location = None

# Cria nova organization
try:
organization = Organization.objects.create(
name=manual_name,
location=location,
source='user', # Marca como fonte user
)
return organization
except Exception:
# Se não conseguir criar, retorna None - usuário terá que usar lista padrão
return None


class OwnerHistoryForm(OrganizationMixin, CoreAdminModelForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Define o model dinamicamente para evitar importação circular
if not hasattr(self._meta, 'model') or self._meta.model is None:
from journal.models import OwnerHistory
self._meta.model = OwnerHistory


class PublisherHistoryForm(OrganizationMixin, CoreAdminModelForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Define o model dinamicamente para evitar importação circular
if not hasattr(self._meta, 'model') or self._meta.model is None:
from journal.models import PublisherHistory
self._meta.model = PublisherHistory


class SponsorHistoryForm(OrganizationMixin, CoreAdminModelForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Define o model dinamicamente para evitar importação circular
if not hasattr(self._meta, 'model') or self._meta.model is None:
from journal.models import SponsorHistory
self._meta.model = SponsorHistory


class CopyrightHolderHistoryForm(OrganizationMixin, CoreAdminModelForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Define o model dinamicamente para evitar importação circular
if not hasattr(self._meta, 'model') or self._meta.model is None:
from journal.models import CopyrightHolderHistory
self._meta.model = CopyrightHolderHistory
Comment on lines +154 to +296
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The entire SciELOJournalModelForm class (lines 154-260) and the four form classes (OwnerHistoryForm, PublisherHistoryForm, SponsorHistoryForm, CopyrightHolderHistoryForm at lines 263-296) are completely duplicated. This is the exact same code that appears at lines 9-151, creating unnecessary code duplication. These duplicate class definitions should be removed to maintain code cleanliness and prevent confusion.

Copilot uses AI. Check for mistakes.


class SciELOJournalModelForm(CoreAdminModelForm):
Expand Down
Loading
Loading