added the platform parameter and changed to activityType#107
added the platform parameter and changed to activityType#107
Conversation
WalkthroughThe pull request updates several components to change the handling of the metric state. The metric value is modified from Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant ComputedProps
participant API
Component->>ComputedProps: Read metric ("all:all")
ComputedProps-->>Component: Return platform & activityType
Component->>API: Call useFetch(platform, activityType)
Possibly related PRs
Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📥 CommitsReviewing files that changed from the base of the PR and between 97578559907f5cabb710c60e1395d7fa6a2ce997 and bf6f97946e7171443080be566facc29733442167. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
🪧 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 using PR comments)
Other keywords and placeholders
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: 1
🧹 Nitpick comments (4)
frontend/app/components/modules/project/components/contributors/contributor-dependency.vue (1)
77-79: Update to metric format and introduction of platform parameterThe code now uses a combined format for metrics ('all:all') and splits it into platform and activityType components. This change supports the new parameter structure in the API while maintaining backward compatibility with the UI components.
Consider adding error handling in case the metric string doesn't contain a colon character. For example:
-const platform = computed(() => metric.value.split(':')[0]); -const activityType = computed(() => metric.value.split(':')[1]); +const platform = computed(() => { + const parts = metric.value.split(':'); + return parts.length > 0 ? parts[0] : 'all'; +}); +const activityType = computed(() => { + const parts = metric.value.split(':'); + return parts.length > 1 ? parts[1] : 'all'; +});frontend/app/components/modules/project/components/contributors/organization-dependency.vue (1)
77-79: Update to metric format and introduction of platform parameterSame changes as in contributor-dependency.vue - the metric format is now 'all:all' and computed properties have been added to split it into platform and activityType parameters.
Consider adding error handling for malformed metric strings, as mentioned in the previous file.
frontend/app/components/modules/project/components/contributors/organizations-leaderboard.vue (1)
51-53: Update to metric format and introduction of platform parameterThe metric format has been updated to 'all:all' with computed properties to extract platform and activityType values. This change is consistent with other files in this PR.
The same recommendation for error handling applies here too.
frontend/app/components/modules/project/components/contributors/contributors-leaderboard.vue (1)
50-53: Update to metric format and introduction of platform parameterThe metric format has been updated to 'all:all' with computed properties to extract platform and activityType values, consistent with the other files in this PR.
The same suggestion for error handling applies here as well.
Note: There's an extra empty line (53) that could be removed for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between d1f3efe and 93e206d338e77c94e6f8fca192676d63b11726e7.
📒 Files selected for processing (8)
frontend/app/components/modules/project/components/contributors/contributor-dependency.vue(1 hunks)frontend/app/components/modules/project/components/contributors/contributors-leaderboard.vue(1 hunks)frontend/app/components/modules/project/components/contributors/fragments/contributors-table.vue(1 hunks)frontend/app/components/modules/project/components/contributors/fragments/metric-dropdown.vue(1 hunks)frontend/app/components/modules/project/components/contributors/fragments/organizations-table.vue(1 hunks)frontend/app/components/modules/project/components/contributors/geographical-distribution.vue(1 hunks)frontend/app/components/modules/project/components/contributors/organization-dependency.vue(1 hunks)frontend/app/components/modules/project/components/contributors/organizations-leaderboard.vue(1 hunks)
🔇 Additional comments (13)
frontend/app/components/modules/project/components/contributors/fragments/contributors-table.vue (2)
47-47: Updated metric comparison to use the new formatThe condition has been updated to check for the new format "all:all" which aligns with the changes in the PR to handle platform and activity type separately.
53-53: Updated metric value processing to handle the new formatThe code now correctly splits the metric string by ":" and uses the second part (activityType) to find the corresponding label in the flattened metrics options.
frontend/app/components/modules/project/components/contributors/fragments/organizations-table.vue (2)
47-47: Updated metric comparison to use the new formatThe condition has been updated to check for the new format "all:all" which aligns with the changes in the PR to handle platform and activity type separately.
53-53: Updated metric value processing to handle the new formatThe code now correctly splits the metric string by ":" and uses the second part (activityType) to find the corresponding label in the flattened metrics options.
frontend/app/components/modules/project/components/contributors/geographical-distribution.vue (4)
97-103: Enhanced metric options with platform and activity typeThe metricOptions are now properly structured to include both platform and activity type in the format "platform:activityType". This refactoring provides better categorization and flexibility when selecting metrics from the dropdown.
106-106: Updated default metric value to match the new formatThe default metric value has been changed from 'all' to 'all:all' to align with the new format that includes both platform and activity type.
108-109: Added computed properties to extract platform and activity typeThese computed properties elegantly extract the platform and activityType from the metric value, allowing them to be used separately in API calls and other operations.
117-119: Updated API parameters to include platform and activity typeThe useFetch call has been updated to use the new computed properties (platform and activityType) instead of a single metric parameter, providing more granular control over the data being requested.
frontend/app/components/modules/project/components/contributors/fragments/metric-dropdown.vue (1)
18-18: Added type import for improved type safetyThe import of DropdownGroupOptions type enhances type safety when mapping through the metricsOptions.
frontend/app/components/modules/project/components/contributors/contributor-dependency.vue (1)
84-86: API parameters updated to match new structureThe parameters were correctly updated to use platform and activityType instead of the single metric parameter, aligning with the PR objectives.
frontend/app/components/modules/project/components/contributors/organization-dependency.vue (1)
84-86: API parameters updated to match new structureAPI parameters have been consistently updated to use the new platform and activityType parameters.
frontend/app/components/modules/project/components/contributors/organizations-leaderboard.vue (1)
58-60: API parameters updated to match new structureParameters in the API call have been correctly updated to use the new platform and activityType fields.
frontend/app/components/modules/project/components/contributors/contributors-leaderboard.vue (1)
58-60: API parameters updated to match new structureThe API parameters have been properly updated to use platform and activityType instead of metric.
There was a problem hiding this comment.
Enhanced metric options structure for dropdown
The metricOptions are now transformed with a more consistent structure that includes both platform and activity type in the format "platform:activityType". This change aligns with the PR objectives of improving platform handling within the dropdown select component.
However, there's an undefined type reference:
- items: option.items.map((item: DropdownItem) => ({
+ items: option.items.map((item: any) => ({You need to either import the DropdownItem type or use an appropriate type that's already available in the current scope.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const metricOptions = metricsOptions.map((option: DropdownGroupOptions) => ({ | |
| label: option.label, | |
| items: option.items.map((item: DropdownItem) => ({ | |
| label: item.label, | |
| value: option.label ? `${option.label.toLowerCase()}:${item.value}` : `all:${item.value}` | |
| })) | |
| })); | |
| const metricOptions = metricsOptions.map((option: DropdownGroupOptions) => ({ | |
| label: option.label, | |
| items: option.items.map((item: any) => ({ | |
| label: item.label, | |
| value: option.label ? `${option.label.toLowerCase()}:${item.value}` : `all:${item.value}` | |
| })) | |
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for DropdownItem type definition in the codebase
echo "Searching for DropdownItem type definition..."
rg "type\s+DropdownItem" --type ts
# Check the specific dropdown types file that's already being imported
echo "Checking the dropdown types file specifically..."
cat frontend/app/components/uikit/dropdown/types/dropdown.types.ts
# Check imports in the original file for any clues
echo "Checking imports in the metric-dropdown.vue file..."
grep "import" frontend/app/components/modules/project/components/contributors/fragments/metric-dropdown.vue
Length of output: 1704
Here's the fix:
import type { DropdownGroupOptions, DropdownOption } from '~/components/uikit/dropdown/types/dropdown.types';
const metricOptions = metricsOptions.map((option: DropdownGroupOptions) = > ({
label: option.label,
items: option.items.map((item: DropdownOption) => ({
label: item.label,
value: option.label ? `${option.label.toLowerCase()}:${item.value}` : `all:${item.value}`
}))
}));This will ensure type safety and prevent TypeScript compilation errors.
borfast
force-pushed
the
improvement/pass-activity-platform
branch
from
1b99907 to
98c9440
Compare
|
I mistakenly pushed this branch and somehow it seems to have closed this PR 🤔 Sorry Efren, my bad 🙏 I'll make sure the necessary changes are not forgotten. |
|
I figured out what happened: due to the |
2 participants
In this PR
Summary by CodeRabbit