feat(website): add count to reference selector#5907
feat(website): add count to reference selector#5907anna-parker wants to merge 20 commits intoimprove-nextclade-metadatafrom
Conversation
a87b261 to
7a54fe6
Compare
b38307a to
d4b3143
Compare
| expect(new URL(page.url()).searchParams.size).toBe(0); | ||
| }); | ||
|
|
||
| test('multi-reference mutation filter can be added and removed', async ({ page }) => { |
There was a problem hiding this comment.
I had to move this as until there are submitted sequences no options will be shown in the references dropdown
|
Claude finished @anna-parker's task in 2m 33s —— View job Code Review CompleteI've completed a comprehensive review of this PR. The feature successfully adds sequence counts to reference selector dropdown options, which is a valuable UX improvement. SummaryStrengths:
Issues Found:
No Critical Issues:
Recommendations
The core functionality is solid and the changes are well-structured. The issues noted are minor performance optimizations rather than correctness problems. |
integration-tests/tests/specs/features/singleseg-multiref-submission-flow.spec.ts
Outdated
Show resolved
Hide resolved
2641a12 to
d189ec4
Compare
7d66310 to
a431ec1
Compare
a431ec1 to
4f315f5
Compare
cd7c474 to
d950ab8
Compare
4f315f5 to
c2976e7
Compare
integration-tests/tests/specs/features/singleseg-multiref-submission-flow.spec.ts
Show resolved
Hide resolved
…ission-flow.spec.ts
|
@claude figure out why the integration tests are failing - EV-A71 is not showing up as an option in integration tests (but is on previews) |
|
Claude finished @anna-parker's task in 1m 45s —— View job Integration Test Fix CompleteI've identified and fixed the issue causing the integration test failure. Root CauseThe problem was in line 41 of await releasedPage.selectReference('Genotype', 'EV-A71 (1)');The In the ReferenceSelector component (ReferenceSelector.tsx:69-76):
The test was trying to select using the display text Fix AppliedI've changed the test to use the correct value: await releasedPage.selectReference('Genotype', 'EV-A71');Note on PushingI've committed the fix locally, but I don't have push permissions to the repository. The commit is ready and staged with the message: You'll need to either:
|
|
@claude no that is not the problem, on the website when I submit 1 sequence with refence EV-A71 I see EV-A71 (1) in the options. The integration test finds no options although one segment has been submitted |
|
Claude finished @anna-parker's task in 2m 17s —— View job Root Cause Identified and FixedRoot CauseI found the issue! The problem is a fundamental change in how the ReferenceSelector works: Before this PR (on main):
After this PR (current code):
The Problem: Fix AppliedI've updated the
I've also fixed the test to use Changes MadeFile:
File:
Note on PushingI've committed the fix locally, but I don't have push permissions. The commit is ready with message: You'll need to either:
|
resolves #5699
Screenshot
PR Checklist
🚀 Preview: https://add-count.loculus.org