-
Notifications
You must be signed in to change notification settings - Fork 7
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
Implementar verificação de disponibilidade de artigos no site QA e public (opac_5) #462
Conversation
…_inline inutilizavel
…lity e ScieloSiteStatus
article/tasks.py
Outdated
f"https://www.scielo.br/scielo.php?script=sci_arttext&pid={pid_v3}&lng={lang}&nrm=iso", | ||
f"https://www.scielo.br/j/{journal_acron}/a/{pid_v3}/?lang={lang}" |
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 a parte www.scielo.br deve ser obtida da class Collection, no entanto, acho que ela está bem diferente da que está no Core. Será necessário atualizá-la
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 adicionar também os formatos pdf
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 acho que seria interessante colocar como melhoria o tempo de resposta
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.
@robertatakenaka Verdade. Acho que posso conseguir esse dado em:
scms-upload/collection/models.py
Line 65 in 91234dd
url = models.URLField(_("Website URL"), max_length=255, null=True, blank=True) |
article/tasks.py
Outdated
response = fetch_data(url, timeout=2, verify=True) | ||
CheckArticleAvailability.create_or_update( | ||
article=article, | ||
status=True, |
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 trocar True / False por HTTP ERROR CODE, ou pode ser um outro atributo
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 Muito bom!
- Modifica o paramentro Status para exemplicar melhor o error - Adiciona paramentro available
…process_article_availability - Melhora o registro das requisicoes em process_article_availability
pid_v2=article.sps_pkg.articleproc_set.first().pid, | ||
journal_acron=article.journal.journal_acron, | ||
lang=article_per_lang, | ||
domain=article.journal.journalproc_set.first().collection.websiteconfiguration_set.get(enabled=True).url, |
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 teste tem que ser feito para todas as coleções registradas
Modelo para armazenar o status de disponibilidade nos sites, | ||
tanto na nova versao, quanto na antiga, do scielo.br. | ||
""" | ||
article = models.ForeignKey( |
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 adicione a collection de modo que checará a disponibilidade em todas as coleções. Também deveria ser considerado a disponibilidade no site QA, inclusive saber se está disponível em ambos ou em somente um
@@ -124,3 +125,13 @@ | |||
(AS_SCHEDULED_TO_PUBLISH, _("Scheduled to publish")), | |||
(AS_PUBLISHED, _("Published")), | |||
) | |||
|
|||
VERIFY_ARTICLE_TYPE = [ |
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 nome não adequado, melhor FORMAT no lugar de ARTICLE_TYPE
@@ -29,6 +30,10 @@ | |||
User = get_user_model() | |||
|
|||
|
|||
def verify_type_of_url(type): |
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 qual é a necessidade desta função?
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.
Não há necessidade de verificar a URL do pdf do artigo?
@@ -314,3 +319,198 @@ def __str__(self) -> str: | |||
return f"{self.article or self.pid_v3} - {self.deadline}" | |||
|
|||
base_form_class = RequestArticleChangeForm | |||
|
|||
|
|||
class CheckArticleAvailability(CommonControlField): |
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 ArticleAvailability
no lugar de CheckArticleAvailability
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 mova este modelo para dentro de publication, pois dentro de publication podemos importar quaisquer outros modelos como JournalProc, IssueProc e outros minimizando problemas de dependências circulares
unique=True, | ||
) | ||
site_status = models.ManyToManyField( | ||
"ScieloSiteStatus" |
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 mude para url_status e "URLStatus
) | ||
|
||
class ScieloSiteStatus(CommonControlField): | ||
check_date = models.DateTimeField(null=True, blank=True) |
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 precisa de check_date se há updated?
|
||
class ScieloSiteStatus(CommonControlField): | ||
check_date = models.DateTimeField(null=True, blank=True) | ||
url_site_scielo = models.SlugField(max_length=500, unique=True) |
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 SlugField no lugar de URLField?
class ScieloSiteStatus(CommonControlField): | ||
check_date = models.DateTimeField(null=True, blank=True) | ||
url_site_scielo = models.SlugField(max_length=500, unique=True) | ||
status = models.CharField( |
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 HTTPCode (SmallPositiveInteger)
null=True, | ||
blank=True | ||
) | ||
type = models.CharField( |
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 precisa desta distinção?
null=True, | ||
blank=True, | ||
) | ||
available = models.BooleanField(default=False) |
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 qual é a diferença entre status e available?
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.
Status pode dar uma mensagem melhor para o usuário. Distingué entre uma URL errada ou instabilidade na URL. Talvez o nome do campo possa ser melhor.
verbose_name = "Scielo Site Status" | ||
verbose_name_plural = "Scielo Site Status" |
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.
URL status
@@ -0,0 +1,113 @@ | |||
import re |
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 mover este conteúdo para publication/tasks.py
@@ -14,7 +14,7 @@ | |||
from config.menu import get_menu_order | |||
|
|||
from .button_helper import ArticleButtonHelper, RequestArticleChangeButtonHelper | |||
from .models import Article, RelatedItem, RequestArticleChange, choices | |||
from .models import Article, RelatedItem, RequestArticleChange, choices, ScieloSiteStatus |
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 mover este conteúdo para publication/wagtail_hooks.py
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 separar em 2 PRs: refatoração do get_user e solução da verificação da disponibilidade do site
@robertatakenaka, vou fechar esta PR e abrir uma nova com o repositório atualizado, pois esta branch está bastante desatualizada. |
O que esse PR faz?
Este PR implementa uma funcionalidade automatizada que verifica a disponibilidade dos artigos tanto na versão nova quanto na versão antiga do SciELO. Após a verificação, os resultados são armazenados em modelos específicos e disponibilizados na interface administrativa para consulta.
Onde a revisão poderia começar?
Pelos commit b8901ab
Como este poderia ser testado manualmente?
python manage.py runscript load_check_article --script-args {username}
Obs: É importante registrar coleção e periódico e realizar relacionamento com um artigo.
Algum cenário de contexto que queira dar?
N/A
Screenshots
Quais são tickets relevantes?
#454
Referências
N/A