-
Notifications
You must be signed in to change notification settings - Fork 1
fix: enhance warehouse statistics to include downloads and maintainers #113
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 updates introduce dynamic calculation and display of package warehouse statistics, including total downloads and maintainers, by enhancing the Changes
Sequence Diagram(s)sequenceDiagram
participant UI (Warehouse.vue)
participant useWarehouse
participant API
UI->>useWarehouse: fetchPackages()
useWarehouse->>API: Fetch package data
API-->>useWarehouse: Return package list
useWarehouse->>useWarehouse: Calculate downloads, maintainers
useWarehouse-->>UI: Return packages, downloads, maintainers, etc.
UI->>UI: Compute stats, authors list
UI->>UI: Render CardStats and FilterBy with dynamic data
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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 (
|
396e67c to
9bb5499
Compare
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: 0
🧹 Nitpick comments (7)
packages/theme/composables/api/useWarehouse.ts (1)
22-23: Consider initializing downloads to 0The initial value of 20000 for downloads seems arbitrary and could be misleading if displayed before data is fetched. Since you're resetting it to 0 in the fetchPackages function anyway, consider initializing it to 0 for consistency.
- const downloads = ref<number>(20000); + const downloads = ref<number>(0);vitest.config.ts (1)
22-25: Consider adding tests for new functionalityThe coverage thresholds for statements and lines have been lowered from 59.72% to 58.73%. While this is a common practice when adding new features, consider adding tests for the new functionality to maintain or improve coverage.
packages/theme/molecules/card-stats/CardStats.vue (2)
7-7: Add default value for isLoading propYou've added an optional isLoading prop, but there's no default value specified in the withDefaults call. Consider adding a default value to ensure consistent behavior.
const props = withDefaults(defineProps<WarehouseStatsProps>(), { id: "card-stats-item", items: () => [], + isLoading: false });
20-20: Consider making the condition more explicitThe conditional logic works correctly but could be more explicit by separating the null/undefined check from the loading check.
- value: item.value && !props.isLoading ? formatNumber(item.value) : "-" + value: (item.value && !props.isLoading) ? formatNumber(item.value) : "-"Alternatively, for even better readability:
- value: item.value && !props.isLoading ? formatNumber(item.value) : "-" + value: (!props.isLoading && item.value) ? formatNumber(item.value) : "-"packages/theme/organisms/warehouse/Warehouse.stories.ts (2)
5-5: Remove unused importThe Button component is imported but not used in this file. Consider removing it to keep the code clean.
-import Button from "../../molecules/button/Button.vue";
28-30: Avoid using 'as any' type assertionType assertions with 'as any' should be avoided when possible as they bypass TypeScript's type checking. Consider defining proper types for the args.
- args: { - text: 'Discover our list of plugins to extend your Ts.ED project. Created by the Ts.ED team and community.' - } as any, + args: { + text: 'Discover our list of plugins to extend your Ts.ED project. Created by the Ts.ED team and community.' + },packages/theme/organisms/warehouse/Warehouse.vue (1)
80-92: Good implementation of author filtering.The implementation of the author filter follows the same pattern as other filters in the component, making it consistent and maintainable.
Consider simplifying the concat operation slightly:
- return [{ - label: "All", - value: "" - }].concat([...maintainers.value.values()] - .map((maintainer) => { - return { - label: maintainer, - value: maintainer - }; - })); + return [ + { + label: "All", + value: "" + }, + ...[...maintainers.value.values()].map((maintainer) => ({ + label: maintainer, + value: maintainer + })) + ];
📜 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 selected for processing (5)
packages/theme/composables/api/useWarehouse.ts(2 hunks)packages/theme/molecules/card-stats/CardStats.vue(2 hunks)packages/theme/organisms/warehouse/Warehouse.stories.ts(2 hunks)packages/theme/organisms/warehouse/Warehouse.vue(4 hunks)vitest.config.ts(1 hunks)
🔇 Additional comments (9)
packages/theme/composables/api/useWarehouse.ts (2)
34-43: LGTM! Good implementation for stats calculationThe implementation correctly resets the download counter and maintainers set before processing new data. The download count accumulation and collection of maintainer usernames (with lowercase conversion for case-insensitivity) are well implemented.
58-58: LGTM! Properly exposing new reactive referencesThe new reactive references (maintainers and downloads) are correctly included in the returned object from the composable.
packages/theme/organisms/warehouse/Warehouse.stories.ts (1)
15-21: LGTM! Clean implementation of custom render functionThe custom render function is well implemented, properly binding args to the component and using args.text as slot content.
packages/theme/organisms/warehouse/Warehouse.vue (6)
18-18: Good enhancement to useWarehouse destructuring.The destructuring now includes
maintainersanddownloadswhich will enable the dynamic statistics calculation that follows.
62-77: Well implemented reactive stats.Converting the static stats array to a computed property is a good approach, as it allows the statistics to update dynamically based on the reactive state. This follows Vue's best practices for derived state.
100-102: Effective author matching implementation.The author filtering logic is well-implemented, matching against maintainer usernames while properly handling the case when no author is selected.
128-128: Good use of slots for flexible content.Replacing the fixed banner content with a slot makes the component more flexible and reusable.
134-134: Proper loading state integration.Passing the
isActivestate to the CardStats component as a loading indicator ensures a consistent user experience during data fetching.
171-171: Good UI addition for author filtering.The new FilterBy component for authors follows the same pattern as the existing filtering controls, maintaining UI consistency.
9bb5499 to
7fa0f0f
Compare
|
🎉 This PR is included in version 1.5.6 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Improvements
Documentation