-
-
Notifications
You must be signed in to change notification settings - Fork 10
✨ Add EDPC and TDPC to contest table (#1956) #2286
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
Conversation
WalkthroughThe changes introduce support for displaying EDPC and TDPC contests as separate tables without headers or round labels, grouped under a single button. This involves refactoring contest table provider logic, adding new provider classes, updating metadata structures, and extending test coverage to accommodate the new DP contest tables. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TaskTableComponent
participant ProviderGroup
participant Provider (EDPC/TDPC/ABC)
participant Store
User->>TaskTableComponent: Selects contest type group (e.g., "DP")
TaskTableComponent->>ProviderGroup: Get all providers in group
loop For each provider (EDPC, TDPC)
TaskTableComponent->>Provider: Prepare contest table data
Provider->>TaskTableComponent: Returns table data (no header/round label for DP)
TaskTableComponent->>TaskTableComponent: Render table per provider
end
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Pull Request Overview
This PR adds support for Educational DP Contest (EDPC) and Typical DP Contest (TDPC) to the contest table, refactors provider registration to use grouped providers, and updates related tests and the Svelte component accordingly.
- Introduce
EDPCProviderandTDPCProvider, register them inContestProviderBuilderandcontestTableProviderGroups - Rename
contestTableProviders→contestTableProviderGroupsand split metadata types intoContestTableMetaDatavs.ContestTablesMetaData - Update tests and
TaskTable.svelteto iterate over provider groups, add display configuration, and introduce new utility classes for dynamic column widths
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/utils/contest_table_provider.ts | Added EDPC/TDPC providers, renamed provider exports, new group class and builder |
| src/lib/types/contest_table_provider.ts | Updated metadata and added display config types |
| src/lib/stores/active_contest_type.svelte.ts | Switched to use ContestTableProviderGroups type |
| src/lib/components/TaskTables/TaskTable.svelte | Refactored to render multiple provider tables, added dynamic layout helpers |
| src/app.css | Added custom .w-1/7 and .w-1/8 utility classes |
| src/test/lib/utils/contest_table_provider.test.ts | Extended tests for EDPC/TDPC providers and display config |
| src/test/lib/stores/active_contest_type.svelte.test.ts | Updated tests to use new provider group key type |
| package.json | Added @types/jsdom |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/test/lib/utils/contest_table_provider.test.ts (2)
67-67: Use explicit boolean assertion for clarity.Replace
toBeTruthy()withtoBe(true)for more explicit boolean assertions.- expect(filtered?.every((task) => task.contest_id.startsWith('abc'))).toBeTruthy(); + expect(filtered?.every((task) => task.contest_id.startsWith('abc'))).toBe(true);
252-252: Pass a valid contest ID for consistency.Although the TDPC provider returns an empty string regardless of input, it would be more consistent with other tests to pass a valid contest ID.
- const label = provider.getContestRoundLabel(''); + const label = provider.getContestRoundLabel('tdpc');src/lib/utils/contest_table_provider.ts (1)
374-421: Consider refactoring to a simple module export to address static analysis warning.The static analysis tool correctly identifies that
ContestProviderBuildercontains only static members. While the current implementation is valid, you could refactor it to a simpler module export pattern.-/** - * Builder class for easily creating ContestProviderGroups - */ -export class ContestProviderBuilder { - /** - * Create predefined groups - * Easily create groups with commonly used combinations - */ - static createPresets() { - return { +/** + * Create predefined provider groups + * Easily create groups with commonly used combinations + */ +export const createContestProviderPresets = () => { + return { /** * Single ABC group (latest 20 rounds) */ ABCLatest20Rounds: () => new ContestTableProviderGroup(`ABC Latest 20 Rounds`, { buttonLabel: 'ABC 最新 20 回', ariaLabel: 'Filter ABC latest 20 rounds', }).addProvider(ContestType.ABC, new ABCLatest20RoundsProvider(ContestType.ABC)), // ... rest of the presets - }; - } -} + }; +};Then update the usage:
export const contestTableProviderGroups = { - abcLatest20Rounds: ContestProviderBuilder.createPresets().ABCLatest20Rounds(), - abc319Onwards: ContestProviderBuilder.createPresets().ABC319Onwards(), - fromAbc212ToAbc318: ContestProviderBuilder.createPresets().ABC212ToABC318(), - dps: ContestProviderBuilder.createPresets().dps(), + abcLatest20Rounds: createContestProviderPresets().ABCLatest20Rounds(), + abc319Onwards: createContestProviderPresets().ABC319Onwards(), + fromAbc212ToAbc318: createContestProviderPresets().ABC212ToABC318(), + dps: createContestProviderPresets().dps(), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
package.json(1 hunks)src/app.css(1 hunks)src/lib/components/TaskTables/TaskTable.svelte(4 hunks)src/lib/stores/active_contest_type.svelte.ts(6 hunks)src/lib/types/contest_table_provider.ts(2 hunks)src/lib/utils/contest_table_provider.ts(8 hunks)src/test/lib/stores/active_contest_type.svelte.test.ts(2 hunks)src/test/lib/utils/contest_table_provider.test.ts(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/test/lib/stores/active_contest_type.svelte.test.ts (2)
src/lib/stores/active_contest_type.svelte.ts (1)
ActiveContestTypeStore(15-67)src/lib/utils/contest_table_provider.ts (1)
ContestTableProviderGroups(430-430)
src/lib/stores/active_contest_type.svelte.ts (2)
src/lib/stores/local_storage_helper.svelte.ts (1)
useLocalStorage(5-7)src/lib/utils/contest_table_provider.ts (1)
ContestTableProviderGroups(430-430)
src/test/lib/utils/contest_table_provider.test.ts (1)
src/lib/utils/contest_table_provider.ts (7)
ABCLatest20RoundsProvider(107-140)ABC319OnwardsProvider(144-167)ABC212ToABC318Provider(174-197)EDPCProvider(210-238)TDPCProvider(240-268)ContestTableProviderGroup(275-369)ContestProviderBuilder(374-421)
src/lib/utils/contest_table_provider.ts (3)
src/lib/types/contest_table_provider.ts (3)
ContestTableDisplayConfig(129-132)ContestTableMetaData(105-108)ContestTablesMetaData(117-120)src/lib/types/task.ts (1)
TaskResult(62-70)src/lib/utils/contest.ts (1)
classifyContest(5-93)
🪛 Biome (1.9.4)
src/lib/utils/contest_table_provider.ts
[error] 375-424: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: preview
🔇 Additional comments (14)
package.json (1)
43-43: LGTM: Good addition of TypeScript types for jsdom.Adding
@types/jsdomprovides proper TypeScript support for the existing jsdom dependency, which will improve type safety in tests and DOM manipulation utilities.src/app.css (1)
15-22: LGTM: Well-implemented custom width utilities.The new
.w-1/7and.w-1/8utility classes are correctly implemented with proper slash escaping and accurate percentage calculations. The use of@layer utilitiesfollows Tailwind CSS best practices.src/test/lib/stores/active_contest_type.svelte.test.ts (2)
3-3: LGTM: Correct type import update.The import has been properly updated from
ContestTableProviderstoContestTableProviderGroupsto align with the refactoring to grouped providers.
46-96: Test values correspond to valid provider group keysAll hardcoded values used in the tests—
'abcLatest20Rounds','abc319Onwards', and'fromAbc212ToAbc318'—are present as keys incontestTableProviderGroups(src/lib/utils/contest_table_provider.ts). No changes required.src/lib/types/contest_table_provider.ts (3)
51-54: LGTM: Good addition of display configuration method.The new
getDisplayConfig()method provides a clean way to control table display behavior, promoting better separation of concerns between data and presentation logic.
105-108: LGTM: Improved metadata structure.The refactoring of
ContestTableMetaDatato focus on core metadata (title, abbreviationName) while moving UI-specific properties toContestTablesMetaDatais a good design decision that improves separation of concerns.
110-132: LGTM: Well-designed new types for UI configuration.The new
ContestTablesMetaDataandContestTableDisplayConfigtypes provide clear, focused interfaces for UI configuration. The comprehensive JSDoc documentation makes the purpose and usage clear.src/lib/stores/active_contest_type.svelte.ts (3)
2-2: LGTM: Correct type import update.The import has been properly updated to use
ContestTableProviderGroupsinstead ofContestTableProviders, aligning with the refactoring to grouped providers.
27-67: LGTM: Consistent type updates throughout the class.All method signatures have been properly updated to use
ContestTableProviderGroupsinstead ofContestTableProviders. The type safety and functionality are maintained while adapting to the new grouped provider architecture.
16-19: Default key ‘abcLatest20Rounds’ is valid in ContestTableProviderGroupsThe default value
abcLatest20Roundsis already defined insrc/lib/utils/contest_table_provider.ts:
- contestTableProviderGroups includes
abcLatest20Rounds: ContestProviderBuilder.createPresets().ABCLatest20Rounds()No further changes needed.
src/test/lib/utils/contest_table_provider.test.ts (1)
6-14: Excellent test coverage for new provider functionality!The test suite has been comprehensively extended to cover:
- New EDPC and TDPC providers with proper metadata and display configuration tests
- ContestTableProviderGroup functionality including group creation, provider management, and statistics
- ContestProviderBuilder presets with verification of independent instances
The tests follow consistent patterns and provide good coverage of the new features.
Also applies to: 208-451
src/lib/utils/contest_table_provider.ts (2)
14-29: Excellent documentation for extensibility!The step-by-step guide for adding new contest table providers is clear and helpful. This will make it easy for future contributors to extend the system.
210-268: Well-implemented DP contest providers!The EDPC and TDPC providers are correctly implemented with:
- Appropriate filtering for their specific contest IDs
- Display configuration that hides headers and round labels (suitable for single-contest providers)
- Clear metadata with both English and Japanese titles
The empty contest round labels are appropriate since these providers represent single contests rather than contest series.
src/lib/components/TaskTables/TaskTable.svelte (1)
47-93: Excellent refactoring to support provider groups!The component has been successfully refactored to:
- Handle multiple providers within a group
- Respect display configuration for headers and round labels
- Render separate tables for each provider with appropriate titles
- Maintain reactive updates through proper use of Svelte 5's
$derivedruneThe implementation is clean and maintains backward compatibility while adding the new functionality.
Also applies to: 185-256
KATO-Hiro
left a comment
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.
LGTM
close #1956
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Enhancements
Style
Documentation
Tests
Chores