-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
Cria Processamento Para Logos dos Periódicos e Adiciona o campo url_logo na API de Journal #838
Conversation
…cessing-for-logo-journal
journal/models.py
Outdated
@@ -2516,6 +2516,10 @@ class JournalLogo(CommonControlField): | |||
null=True, | |||
blank=True, | |||
) | |||
url_logo = models.URLField( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel no lugar de criar um novo campo, vc poderia usar o link para a imagem (campo logo, linha 2512)
journal/api/v1/serializers.py
Outdated
issn_print = serializers.CharField(source="official.issn_print") | ||
issn_electronic = serializers.CharField(source="official.issn_electronic") | ||
next_journal_title = serializers.CharField(source="official.next_journal_title") | ||
previous_journal_titles = serializers.CharField(source="official.previous_journal_titles") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel next_journal_title e previous_journal_titles deveriam ter além dos títulos, também outros atributos, como issns e acrônimos, pois eles serão links no site para outros periódicos. Imagina que o título C tem um título anterior B. No site haverá um link de B para C, então terá que ter todos os mesmos elementos necessário para montar os links.
journal/api/v1/serializers.py
Outdated
other_titles = serializers.SerializerMethodField() | ||
sponsor = serializers.SerializerMethodField() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel por que other_titles e sponsor não é similar a Mission?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel neste PR não encontrei a parte "Cria Processamento Para Logos "
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other_titles só há um campo 'title', então achei mais fácil fazer desta forma do que criar uma class serializer para other_ttitles.
Ao acessar os atributos de sponsor, owner, copyright e publisher, estou utilizando métodos que verificam a existência desses atributos nas classes relacionadas, evitando erros de AttibuteError caso algum dos atributos seja None.
Já em Mission não é preciso fazer isto, já que não há classes relacionadas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
journal/api/v1/serializers.py
Outdated
return obj.title | ||
if obj.logo: | ||
domain = Site.objects.get(is_default_site=True).hostname | ||
if not domain.startswith('https'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel alguns sites clássicos não tem https, só http, vai funcionar a coleta?
institution/models.py
Outdated
if self.institution.institution: | ||
return self.institution.institution.institution_identification.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel tem que retornar str sempre. Também teria que testar a existência de self.institution.institution.institution_identification
try:
return self.institution.institution.institution_identification.name
except AttributeError:
return ''
journal/api/v1/serializers.py
Outdated
"issn_print": obj.official.new_title.issn_print, | ||
"issn_electronic": obj.official.new_title.issn_electronic, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel o next_journal_title pode estar preenchido, mas o new_title não.
journal/api/v1/serializers.py
Outdated
def get_previous_journal_titles(self, obj): | ||
if obj.official.previous_journal_titles: | ||
try: | ||
old_issn_print = obj.official.old_title.get(title__icontains=obj.official.previous_journal_titles).issn_print |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel faça a consulta obj.official.old_title.get(title__icontains=obj.official.previous_journal_titles)
apenas 1 vez
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel verificar os comentários
journal/api/v1/serializers.py
Outdated
@@ -163,25 +163,25 @@ def get_other_titles(self, obj): | |||
|
|||
def get_next_journal_title(self, obj): | |||
if obj.official.next_journal_title: | |||
journal_new_title = obj.filter(title__icontains=obj.official.next_journal_title) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel filter retornará um queryset, então ocorrerá erro nas linhas 169 e 170. Além disso, tem que considerar que pode não existir.
journal/api/v1/serializers.py
Outdated
} | ||
|
||
def get_previous_journal_titles(self, obj): | ||
if obj.official.previous_journal_titles: | ||
try: | ||
old_issn_print = obj.official.old_title.get( | ||
old_journal = obj.official.old_title.get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel este trecho está estranho
institution/models.py
Outdated
@@ -432,6 +432,9 @@ def get_or_create(cls, institution, initial_date=None, final_date=None, user=Non | |||
class Meta: | |||
abstract = True | |||
|
|||
def __str__(self): | |||
if self.institution.institution: | |||
return getattr(self.institution.institution.institution_identification, 'name', '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel
Use:
try:
return self.institution.institution.institution_identification.name
except AttributeError:
return ''
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelveigarangel parece ok
O que esse PR faz?
Cria Processamento Para Logos dos Periódicos e Adiciona o campo url_logo na API de Journal
Onde a revisão poderia começar?
pelos commits
Como este poderia ser testado manualmente?
Acessar interface administrativa e cria a task fetch_and_process_journal_logos_in_collection
Algum cenário de contexto que queira dar?
N/A
Screenshots
N/A
Quais são tickets relevantes?
N/A
Referências
Indique as referências utilizadas para a elaboração do pull request.