[INS-216] Fix sending activity type and platform in requests to the backend API#109
[INS-216] Fix sending activity type and platform in requests to the backend API#109
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request updates several Vue components related to contributor metrics handling. The default metric value is changed from Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Vue Component
participant API as API Server
U->>C: Update or select metric
C->>C: Compute platform & activityType from "all:all"
C->>API: Call useFetch with platform & activityType
API-->>C: Return data
C->>U: Render updated view
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 1f0aea60cf69d1129231c996e65eb080b6d5805e and 1e8a693. 📒 Files selected for processing (8)
🪧 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 (
|
eac639b to
9090721
Compare
...tend/app/components/modules/project/components/contributors/fragments/contributors-table.vue
Outdated
Show resolved
Hide resolved
...end/app/components/modules/project/components/contributors/fragments/organizations-table.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/app/components/modules/project/components/contributors/geographical-distribution.vue (1)
126-127: Good computed properties implementation with potential edge caseThe computed properties cleanly extract platform and activityType from the metric value. Consider adding a fallback for the split operation in case the format is unexpected.
- const platform = computed(() => metric.value.split(':')[0]); - const activityType = computed(() => metric.value.split(':')[1]); + const platform = computed(() => metric.value.split(':')[0] || 'all'); + const activityType = computed(() => metric.value.split(':')[1] || 'all');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 90907212582e8ea5470ddd9aadc40431dfacff06 and fc20b31d52c2b84886dcbf58886ee8751b5a9dca.
📒 Files selected for processing (1)
frontend/app/components/modules/project/components/contributors/geographical-distribution.vue(1 hunks)
🔇 Additional comments (4)
frontend/app/components/modules/project/components/contributors/geographical-distribution.vue (4)
113-113: Appropriate type imports addedThe addition of the dropdown type imports enhances type safety and ensures proper TypeScript compatibility for the dropdown options implementation.
115-121: Good implementation of dropdown options with platform:activityType structureThe implementation correctly maps the metrics options to include both platform and activity type in the value field, following the format
platform:activityType. This aligns with the PR objective of handling platform in dropdown selections.
124-124: Appropriate default value updateChanged metric default from 'all' to 'all:all' to match the new format where the first part represents platform and the second part represents activity type.
135-137: Fix correctly implemented for API parametersThe API call now correctly sends platform and activityType as separate parameters instead of a single metric parameter, addressing the main issue described in the PR objectives.
892a83b to
1f0aea6
Compare
This is meant to redo the work Efren did, and that I somehow mistakenly overwrote. I probably misused the git town tool and that messed up Efren's PR. Signed-off-by: Raúl Santos <4837+borfast@users.noreply.github.com>
Signed-off-by: Raúl Santos <4837+borfast@users.noreply.github.com>
1f0aea6 to
1e8a693
Compare
This is meant to redo the work @emlimlf did in #107, and that I somehow mistakenly overwrote. I probably misused git town and it messed up Efren's PR. Sorry Efren!
As per the original PR:
ℹ️ This PR is part of a stack
Please review them in order, as some of the first ones set the foundation for the later ones.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes