fix(frontend): subscription page refactor #197
Conversation
Resumo do PR
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Flags with carried forward coverage won't be shown. Click here to find out more. |
Yudi
changed the title
|
Warning Rate limit exceeded@Yudi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 43 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAs mudanças implementadas aprimoram a funcionalidade de inscrição para eventos, introduzindo uma estrutura modular que promove a reutilização e a clareza no código. Novos componentes foram adicionados para gerenciar a exibição de informações dos eventos e a seleção do tipo de inscrição. Além disso, melhorias na validação de horários e no gerenciamento de dados em tempo real utilizando o Firestore foram realizadas, aumentando a robustez da aplicação. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (13)
- frontend/src/app/app.component.scss (1 hunks)
- frontend/src/app/restricted-area/manage-events/add-event/add-event.page.ts (1 hunks)
- frontend/src/app/shared/services/major-event.service.ts (1 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/confirm-subscription-modal/confirm-subscription-modal.component.html (3 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/confirm-subscription-modal/confirm-subscription-modal.component.ts (3 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.html (1 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.scss (1 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.ts (1 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/major-event-info-subscription/major-event-info-subscription.component.html (1 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/major-event-info-subscription/major-event-info-subscription.component.ts (1 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/subscribe.page.html (4 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/subscribe.page.scss (1 hunks)
- frontend/src/app/tabs/major-events-display/subscribe/subscribe.page.ts (12 hunks)
Additional context used
Biome
frontend/src/app/tabs/major-events-display/subscribe/confirm-subscription-modal/confirm-subscription-modal.component.ts
[error] 100-100: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
frontend/src/app/tabs/major-events-display/subscribe/subscribe.page.ts
[error] 369-369: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 458-458: Reassigning a function parameter is confusing.
The parameter is declared here:
Use a local variable instead.
(lint/style/noParameterAssign)
[error] 461-461: Reassigning a function parameter is confusing.
The parameter is declared here:
Use a local variable instead.
(lint/style/noParameterAssign)
frontend/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.ts
[error] 88-88: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 89-89: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 283-283: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 295-295: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 333-333: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
Additional comments not posted (36)
frontend/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.scss (1)
1-6: Estilos CSS aprovados para.twemoji.Os estilos definidos para a classe
.twemojisão claros e concisos, adequados para o uso pretendido. Não há problemas aparentes.frontend/src/app/tabs/major-events-display/subscribe/subscribe.page.scss (1)
Line range hint
1-44:
Estilos CSS aprovados com a remoção de.twemoji.A remoção da classe
.twemojisimplifica o arquivo de estilos. As definições restantes são bem estruturadas e fazem uso eficaz de técnicas modernas de CSS, como flexbox e-webkit-line-clamp.frontend/src/app/tabs/major-events-display/subscribe/major-event-info-subscription/major-event-info-subscription.component.ts (1)
1-22: Componente Angular aprovado.O componente
MajorEventInfoSubscriptionComponentestá bem estruturado, utilizando componentes independentes do Angular e injeção de dependência. O uso do decorador@Inputcom a propriedaderequiredgarante a integridade dos dados, o que é uma boa prática.frontend/src/app/shared/services/major-event.service.ts (1)
76-76: Considere o impacto do novo campomandatoryEvents.A adição do campo opcional
mandatoryEventsna interfaceMajorEventItempode impactar a lógica de negócios relacionada aos eventos obrigatórios. Certifique-se de que todas as funcionalidades que interagem comMajorEventItemlidem corretamente com a presença ou ausência deste campo.frontend/src/app/tabs/major-events-display/subscribe/confirm-subscription-modal/confirm-subscription-modal.component.html (2)
69-69: Correção de texto na mensagem de erro.A correção na mensagem de erro melhora a clareza na interface do usuário. Verifique se todas as mensagens de erro seguem um padrão consistente.
13-13: Verifique a integração do componente<app-major-event-info-subscription>.A integração do componente
<app-major-event-info-subscription>promove a modularidade e a reutilização do código. Certifique-se de que o componente está devidamente testado e documentado.frontend/src/app/app.component.scss (1)
147-149: A adição da classe.opacity-50está aprovada.A classe
.opacity-50oferece uma maneira simples de aplicar transparência a elementos, melhorando a flexibilidade de design.frontend/src/app/tabs/major-events-display/subscribe/major-event-info-subscription/major-event-info-subscription.component.html (1)
1-74: A estrutura HTML está bem organizada.O uso de diretivas e bindings do Angular parece correto. Verifique se todos os bindings Angular estão funcionando conforme esperado, especialmente com o uso de
@if.frontend/src/app/tabs/major-events-display/subscribe/confirm-subscription-modal/confirm-subscription-modal.component.ts (1)
71-102: A abordagem reativa e verificações de entrada estão bem implementadas.O uso de
BehaviorSubjectecombineLatestmelhora a reatividade do componente. As verificações de entrada no construtor aumentam a robustez.Tools
Biome
[error] 100-100: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
frontend/src/app/tabs/major-events-display/subscribe/subscribe.page.html (2)
14-15: Integração do componente<app-major-event-info-subscription>aprovada.O componente está corretamente recebendo e exibindo os detalhes do evento principal.
64-73: Integração do componente<app-event-list-form>aprovada.O componente está corretamente configurado com os inputs necessários para gerenciar a lógica de seleção e inscrição de eventos.
frontend/src/app/restricted-area/manage-events/add-event/add-event.page.ts (1)
321-321: Uso do operador de igualdade estrita===aprovado.A alteração melhora a segurança de tipo e está alinhada com as melhores práticas do TypeScript.
frontend/src/app/tabs/major-events-display/subscribe/subscribe.page.ts (7)
155-175: Integração com Firestore para lógica de inscrição de usuário aprovada.A integração é eficiente e lida corretamente com a recuperação e atualização de dados.
176-199: Integração com Firestore para dados de eventos principais aprovada.A integração está corretamente recuperando e processando os dados dos eventos, incluindo eventos obrigatórios.
119-120: Uso doEventListFormComponentaprovado.O componente está corretamente integrado e funcional dentro do
SubscribePage.
72-73: Uso doMajorEventInfoSubscriptionComponentaprovado.O componente está corretamente integrado e funcional dentro do
SubscribePage.
Line range hint
319-365: Melhorias na lógica de manipulação de formulários aprovadas.As melhorias na lógica estão alinhadas com os objetivos e aprimoram a experiência do usuário.
Line range hint
300-313: Melhorias nos mecanismos de feedback do usuário aprovadas.As notificações e diálogos fornecem informações claras e úteis aos usuários.
461-461: Evite a reatribuição de parâmetros de função.Reatribuir parâmetros pode ser confuso; use uma variável local em vez disso.
Tools
Biome
[error] 461-461: Reassigning a function parameter is confusing.
The parameter is declared here:
Use a local variable instead.
(lint/style/noParameterAssign)
frontend/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.ts (17)
1-28: Importações parecem corretas.As importações incluem módulos necessários para o funcionamento do componente e parecem estar em ordem.
29-51: Metadados do componente estão corretos.Os metadados do componente estão configurados corretamente para uso standalone e as importações são apropriadas.
72-93: Construtor inicializa corretamente o estado do componente.O construtor configura os serviços e inicializa o
FormGroupcorretamente, além de subscrever aoevents$para popular a lista de eventos.Tools
Biome
[error] 88-88: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 89-89: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
95-115: ngOnInit está configurado corretamente.O método
ngOnInitlida corretamente com a inicialização dos eventos obrigatórios e já inscritos.
117-153: Método selectAlreadySubscribed processa corretamente as inscrições.O método lida adequadamente com os dados de inscrição do usuário e atualiza os controles do formulário.
156-171: Método incrementAmountOfEventsSelected atualiza contadores corretamente.A lógica do método está correta para incrementar os contadores de eventos com base no tipo de evento.
173-190: Método autoSelectMandatory processa eventos obrigatórios corretamente.A lógica do método está correta para selecionar automaticamente eventos obrigatórios e verificar conflitos.
193-202: Método showEventInfo apresenta modal corretamente.A lógica do método está correta para criar e apresentar o modal de informações do evento.
204-257: Método selectFromGroup gerencia seleção de grupos corretamente.A lógica do método está correta para aplicar limites de seleção e gerenciar seleções de grupos de eventos.
260-273: Método checkConflicts verifica conflitos corretamente.A lógica do método está correta para identificar conflitos de agendamento entre eventos.
275-285: Método blockEventGroup bloqueia eventos em conflito corretamente.A lógica do método está correta para desabilitar eventos que estão em conflito.
Tools
Biome
[error] 283-283: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
288-297: Método unblockEventGroup desbloqueia eventos corretamente.A lógica do método está correta para habilitar eventos que não estão mais em conflito.
Tools
Biome
[error] 295-295: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
300-318: Método isConflict determina conflitos de horário corretamente.A lógica do método está correta para identificar conflitos de horário entre eventos.
320-325: Método formatDate formata datas corretamente.A lógica do método está correta para formatar datas de acordo com o local especificado.
332-345: Método itemClick lida com cliques corretamente.A lógica do método está correta para alternar a seleção e gerenciar estados desabilitados.
Tools
Biome
[error] 333-333: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
347-364: Método presentLimitReachedToast apresenta toast corretamente.A lógica do método está correta para criar e apresentar a notificação de toast quando os limites de seleção são atingidos.
366-383: Método presentDisabledToast apresenta toast corretamente.A lógica do método está correta para criar e apresentar a notificação de toast para eventos indisponíveis.
...d/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.html
Show resolved
Hide resolved
...-events-display/subscribe/confirm-subscription-modal/confirm-subscription-modal.component.ts
Outdated
Show resolved
Hide resolved
...vents-display/subscribe/confirm-subscription-modal/confirm-subscription-modal.component.html
Show resolved
Hide resolved
frontend/src/app/tabs/major-events-display/subscribe/subscribe.page.ts
Outdated
Show resolved
Hide resolved
frontend/src/app/tabs/major-events-display/subscribe/subscribe.page.ts
Outdated
Show resolved
Hide resolved
| // For every event after eventIndex | ||
| for (let i = eventIndex + 1; i < this.eventList.length; i++) { | ||
| const eventIterationStartDate = this.dateService.getDateFromTimestamp(this.eventList[i].eventStartDate); | ||
| const eventIterationEndDate = this.dateService.getDateFromTimestamp(this.eventList[i].eventEndDate); | ||
| // If event doesn't overlap or if it's itself, break | ||
| if ( | ||
| eventItemStartDate >= eventIterationEndDate || | ||
| eventItemEndDate <= eventIterationStartDate || | ||
| eventItem.id === this.eventList[i].id | ||
| ) { | ||
| break; | ||
| } | ||
| // If event overlaps, disable it | ||
|
|
||
| if (this.eventList[i].eventGroup?.groupEventIDs) { | ||
| this.eventList[i].eventGroup?.groupEventIDs.forEach((event) => { | ||
| this.dataForm.get(event).disable(); | ||
| }); | ||
| } else { | ||
| this.dataForm.get(this.eventList[i].id).disable(); | ||
| this.dataForm.get(this.eventList[i].id).setValue(null); | ||
| } | ||
| } | ||
|
|
||
| // For every event before eventIdex | ||
| for (let i = eventIndex - 1; i >= 0; i--) { | ||
| const eventIterationStartDate = this.dateService.getDateFromTimestamp(this.eventList[i].eventStartDate); | ||
| const eventIterationEndDate = this.dateService.getDateFromTimestamp(this.eventList[i].eventEndDate); | ||
|
|
||
| // If event doesn't overlap or if it's itself, break | ||
| if ( | ||
| eventItemStartDate >= eventIterationEndDate || | ||
| eventItemEndDate <= eventIterationStartDate || | ||
| eventItem.id === this.eventList[i].id | ||
| ) { | ||
| break; | ||
| } | ||
|
|
||
| // If event overlaps, disable it | ||
|
|
||
| if (this.eventList[i].eventGroup?.groupEventIDs) { | ||
| this.eventList[i].eventGroup.groupEventIDs.forEach((event) => { | ||
| this.dataForm.get(event).disable(); | ||
| }); | ||
| } else { | ||
| this.dataForm.get(this.eventList[i].id).disable(); | ||
| this.dataForm.get(this.eventList[i].id).setValue(null); | ||
| } | ||
| } | ||
| } else { | ||
| // For every event after eventIndex | ||
| for (let i = eventIndex + 1; i < this.eventList.length; i++) { | ||
| const eventIterationStartDate = this.dateService.getDateFromTimestamp(this.eventList[i].eventStartDate); | ||
|
|
||
| // If event doesn't overlap, break | ||
| if (eventIterationStartDate >= eventItemEndDate) { | ||
| break; | ||
| } | ||
|
|
||
| // If event overlaps, enable it | ||
|
|
||
| // Keeps event disabled if it's a palestra. | ||
| // Used during SECOMPP22 where palestras were mandatory | ||
| if (this.eventList[i].eventType !== 'palestra') { | ||
| if (this.eventList[i].slotsAvailable > 0) { | ||
| if (this.eventList[i].eventGroup?.groupEventIDs) { | ||
| this.eventList[i].eventGroup.groupEventIDs.forEach((event) => { | ||
| this.dataForm.get(event).enable(); | ||
| }); | ||
| } else { | ||
| this.dataForm.get(this.eventList[i].id).enable(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // For every event before eventIdex | ||
| for (let i = eventIndex - 1; i >= 0; i--) { | ||
| const eventIterationEndDate = this.dateService.getDateFromTimestamp(this.eventList[i].eventEndDate); | ||
|
|
||
| // If event doesn't overlap, break | ||
| if (eventIterationEndDate <= eventItemStartDate) { | ||
| break; | ||
| } | ||
|
|
||
| // If event overlaps, enable it | ||
|
|
||
| if (this.eventList[i].eventType !== 'palestra') { | ||
| if (this.eventList[i].eventGroup?.groupEventIDs) { | ||
| this.eventList[i].eventGroup.groupEventIDs.forEach((event) => { | ||
| this.dataForm.get(event).enable(); | ||
| }); | ||
| } else { | ||
| this.dataForm.get(this.eventList[i].id).enable(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| this.isEventScheduleBeingChecked = false; | ||
| }*/ |
There was a problem hiding this comment.
Considere remover o código comentado.
Existem blocos de código comentado que podem não ser mais necessários. Considere removê-los para manter o código limpo.
...end/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.ts
Outdated
Show resolved
Hide resolved
...end/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.ts
Outdated
Show resolved
Hide resolved
...end/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.ts
Outdated
Show resolved
Hide resolved
...end/src/app/tabs/major-events-display/subscribe/event-list-form/event-list-form.component.ts
Outdated
Show resolved
Hide resolved
A página de inscrição foi feita às pressas, com diversos usos incorretos.
Esse pull request corrige esses anti-patterns e garante o funcionamento da página.
Closes #114
Closes #145
Closes #146
Summary by CodeRabbit
Novos Recursos
.opacity-50para permitir a aplicação de opacidade 0.5 em elementos.EventListFormComponentpara gerenciar inscrições de eventos.MajorEventInfoSubscriptionComponentpara exibir detalhes de eventos principais.Correções de Bugs
Melhorias na Experiência do Usuário
Documentação