fix(marketing): corrige participantes ausentes no Meeting Showcase - #478
Conversation
📝 WalkthroughWalkthroughMeeting Showcase participant loading now includes messages through the selected minute’s final second. External identities include soft-deleted records and linked users. Discord participant extraction supports root metadata, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app-modules/panel-admin/tests/Feature/Marketing/MeetingShowcasePageTest.php (1)
135-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the exclusion side of the boundary.
The test proves inclusion at 23:40:59. It does not prove exclusion at 23:41:00. A future change to
endOfMinute()(for exampleendOfHour()) would still pass.♻️ Proposed additional case
test('it excludes messages sent after the selected end minute', function (): void { $identity = ExternalIdentity::factory()->create([ 'provider' => IdentityProvider::Discord, 'external_account_id' => '445', 'metadata' => ['username' => 'too-late-user'], ]); Message::factory()->create([ 'external_identity_id' => $identity->id, 'channel_id' => 'meeting-channel', 'sent_at' => Date::parse('2026-08-03 23:41:00', 'America/Sao_Paulo')->utc(), ]); livewire(MeetingShowcasePage::class) ->set('channelId', 'meeting-channel') ->set('startDate', '2026-08-03T22:00') ->set('endDate', '2026-08-03T23:40') ->call('loadParticipants') ->assertSet('participants', []); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/tests/Feature/Marketing/MeetingShowcasePageTest.php` around lines 135 - 155, Add a complementary test near “it includes messages sent during the selected end minute” that creates a message at 23:41:00, uses the same selected end time of 23:40, calls loadParticipants, and asserts participants is empty, verifying messages after the selected minute are excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app-modules/panel-admin/tests/Feature/Marketing/MeetingShowcasePageTest.php`:
- Around line 135-155: Add a complementary test near “it includes messages sent
during the selected end minute” that creates a message at 23:41:00, uses the
same selected end time of 23:40, calls loadParticipants, and asserts
participants is empty, verifying messages after the selected minute are
excluded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5919169c-5521-4c90-8ada-41b0e78cbb57
📒 Files selected for processing (2)
app-modules/panel-admin/src/Marketing/Pages/MeetingShowcasePage.phpapp-modules/panel-admin/tests/Feature/Marketing/MeetingShowcasePageTest.php
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app-modules/panel-admin/tests/Feature/Marketing/MeetingShowcasePageTest.php (1)
109-132: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAssert aggregated message totals.
This test creates one message per identity and never checks
total_messages. It would pass if repeated messages were counted incorrectly. Add a second message for one identity and assert itstotal_messagesvalue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/panel-admin/tests/Feature/Marketing/MeetingShowcasePageTest.php` around lines 109 - 132, Update the participant aggregation assertions in MeetingShowcasePageTest so the fixture includes a second message for one identity, then assert that identity’s total_messages equals 2 while retaining the existing participant count and identity assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app-modules/panel-admin/tests/Feature/Marketing/MeetingShowcasePageTest.php`:
- Around line 109-132: Update the participant aggregation assertions in
MeetingShowcasePageTest so the fixture includes a second message for one
identity, then assert that identity’s total_messages equals 2 while retaining
the existing participant count and identity assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: dd763ae2-8d97-441e-a839-9eacb6e1d9b5
📒 Files selected for processing (1)
app-modules/panel-admin/tests/Feature/Marketing/MeetingShowcasePageTest.php
guisaliba
left a comment
guisaliba
left a comment
There was a problem hiding this comment.
LGTM! mas tive a mesma dúvida do Nadachi.
|
@Luisnadachi @guisaliba fui atrás dessa do author vs user, porque a resposta muda o que a linha 107 significa. são o mesmo objeto: os dois são o User da API do Discord, mesmo schema (id, username, global_name, avatar). o que muda é de qual payload cada um foi arrancado. são três caminhos criando ExternalIdentity e cada um salva o payload cru do jeito que recebeu, o OAuth do site achata username/email/avatar na raiz do metadata, o import de perfil salva o $profile inteiro (e no profile o usuário vem embrulhado numa chave user), e o import de mensagem salva e o que acho que tava por trás da dúvida: os dois nunca aparecem juntos no mesmo registro. o ImportDiscordProfileAction usa updateOrCreate com o metadata no array de update, então substitui o metadata inteiro toda vez que roda. o import de mensagem só escreve metadata no createIdentity, quando a identidade ainda nem existe. quem toca por último manda, e o perfil sempre ganha porque é o único que dá update. se a identidade nasceu de uma mensagem e o perfil for importado depois, o author some e vira user. por isso o user ?? author não desempata nada. não existe registro com os dois, não tem risco de pegar a fonte errada. o ?? aqui não é defensivo, é a união de três produtores. fora do escopo desse PR, mas esse sobrescrito me incomodou: quem logou com Discord no site tem email no metadata e perde ele se o perfil for importado depois. vou abrir issue pra normalizar na ingestão, aí a tela lê um formato só em vez de adivinhar três. |
|
LGTM |
7 participants
Contexto
O Meeting Showcase podia encontrar a mensagem no período informado e ainda assim não identificar corretamente o participante. Isso acontecia porque os dados do Discord chegam em formatos diferentes e a tela só tratava parte deles.
Também existia um corte no último minuto escolhido. Ao informar 23:40 como horário final, por exemplo, mensagens enviadas depois de 23:40:00 ficavam fora da consulta.
Este PR corrige a leitura e exibição dos participantes que já foram ingeridos. O fluxo de ingestão das mensagens não foi alterado.
Alterações
metadata.usere emmetadata.authorPlano de Testes
metadata.authorcom 1 teste e 4 assertionsA suíte completa local executou 933 testes. Foram 931 aprovados e 2 falhas preexistentes em
MergeDuplicateDiscordProfilesTest, causadas pelo tratamento de caminho absoluto do Windows no parâmetro--pairs-file. Esses testes ficam fora dos arquivos e do fluxo alterado neste PR. O CI em Linux fará a validação completa novamente.Issues Relacionadas
Closes #477