Skip to content
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

Update - removendo limite de supplies na listagem de abrigos #136

Merged

Conversation

xlucaix
Copy link
Contributor

@xlucaix xlucaix commented May 18, 2024

Melhorar a descoberta de que existem mais itens depois que o limite de 10 é excedido no card

Conforme solicitado na issue #257 foi removido o limite no retorno dos supplies de abrigo.
PR do Frontend: SOS-RS/frontend#249

Copy link

@vinny-silveira vinny-silveira left a comment

Choose a reason for hiding this comment

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

O nome do método getQuery foi alterado mas a chamada dele no service não foi ajustada, está dando erro logo ao iniciar o projeto:

image

@xlucaix
Copy link
Contributor Author

xlucaix commented May 21, 2024

O nome do método getQuery foi alterado mas a chamada dele no service não foi ajustada, está dando erro logo ao iniciar o projeto:

image

Opa @vinny-silveira, acabei de atualizar com a develop

Copy link

@vinny-silveira vinny-silveira left a comment

Choose a reason for hiding this comment

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

Bom dia @xlucaix, você está rodando o backend local? Pois ainda segue sem funcionar:

image

Atualizei minha branch local com as suas atualizações e mesmo assim o erro persiste:

image

Antes de submeter a PR ou uma nova revisão, peço que minimamente rode o projeto localmente e garanta o funcionamento básico do projeto.

@xlucaix
Copy link
Contributor Author

xlucaix commented May 22, 2024

@vinny-silveira aqui tinha rodado localmente tranquilo, o que foi estranho kkkk
Depois de comparar o arquivo shelterSearch notei que a classe foi alterada, acredito ter sido erro aqui de alguma extensão local, vou revisar depois qual pode ter feito isso.
Subi um novo commit removendo as alterações da classe e já rodando o projeto

Copy link

@vinny-silveira vinny-silveira left a comment

Choose a reason for hiding this comment

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

Certo, o problema de execução foi resolvido, porém, as tags não retornam corretamente do BFF, deixando o front dessa forma:

image

Você removeu o defaultTagsData, e pelo objeto tags da linha 176 ficar vazio, nada é tagueado. Para manter o funcionamento, sugiro que volte o objeto defaultTagsData e faça o spread dele como era feito antes:

  const tags: ShelterTagInfo = {
    ...defaultTagsData,
    ...(tagProps?.tags ?? {}),
  };

Novamente, peço que teste o backend integrado com o front sempre, e para evitar caches, sempre faça um clear no Chrome pelo Developer Tools:

image

Ou sempre teste em abas anônimas.

@xlucaix
Copy link
Contributor Author

xlucaix commented May 22, 2024

Certo, o problema de execução foi resolvido, porém, as tags não retornam corretamente do BFF, deixando o front dessa forma:

image

Você removeu o defaultTagsData, e pelo objeto tags da linha 176 ficar vazio, nada é tagueado. Para manter o funcionamento, sugiro que volte o objeto defaultTagsData e faça o spread dele como era feito antes:

  const tags: ShelterTagInfo = {
    ...defaultTagsData,
    ...(tagProps?.tags ?? {}),
  };

Novamente, peço que teste o backend integrado com o front sempre, e para evitar caches, sempre faça um clear no Chrome pelo Developer Tools:

image

Ou sempre teste em abas anônimas.

Talvez por cache comigo exibia, alterei o sche
ma da tag para boolean, já que o objetivo náo é lidar mais limitar quantidades, acho que agora ta ok, testei várias vezes removendo cache.

Screenshot 2024-05-22 at 18 19 47

Copy link

@vinny-silveira vinny-silveira left a comment

Choose a reason for hiding this comment

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

PR apto ao merge

@vinny-silveira
Copy link

@SOS-RS/backenders , preciso de mais um review aqui.

Copy link

@larissapissurno larissapissurno left a comment

Choose a reason for hiding this comment

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

lgtm

@larissapissurno larissapissurno merged commit b5ba7b3 into SOS-RS:develop Jun 1, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants