Skip to content
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

FIX: Feature-[DEV-10046] : Dashboard Filters - Parent #1820

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from

Conversation

Atash3000
Copy link
Collaborator

[Replace With Ticket Number]

Testing Steps

Set Up Filters in the Dashboard:

Create at least two filters with "Data" as the filter type.
Configure one as a Parent Filter and the other as a Child Filter (link the child to the parent using the Parents field).
Test Parent-Child Logic:

Select a value in the Parent Filter (e.g., Electronics).
Verify that the Child Filter updates to display only values related to the selected parent value (e.g., Sony, Samsung).
Screenshot 2025-01-08 at 14 22 45

Self Review

  • I have added testing steps for reviewers
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing unit tests are passing

Screenshots (if applicable)

Additional Notes

@Atash3000 Atash3000 added this to the 4.25.1 milestone Jan 8, 2025
@Atash3000 Atash3000 requested review from adamdoe and joshlacey January 8, 2025 19:27
@adamdoe
Copy link
Collaborator

adamdoe commented Jan 10, 2025

@Atash3000 - It looks like there are failing unit tests on this PR.

@Atash3000
Copy link
Collaborator Author

@adamdoe updated unit test. Should be good now.

import _ from 'lodash'

export const updateChildFilters = (newSharedFilters: SharedFilter[], data: Record<string, any>): SharedFilter[] => {
const dateSet = Object.values(data).flat()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spelling

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!


if (parentFilter) {
// Filter dataset based on parent's active value
const parentActiveValuesArr = dateSet.filter(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spelling

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

@Atash3000 Atash3000 requested a review from adamdoe January 15, 2025 22:43
@joshlacey
Copy link
Collaborator

@Atash3000 The parent filter selector should be a multi select.

import { SharedFilter } from '../types/SharedFilter'
import _ from 'lodash'

export const updateChildFilters = (newSharedFilters: SharedFilter[], data: Record<string, any>): SharedFilter[] => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add logic to this function to only run if the filter is a data filter? api filters handle parent relationships differently

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added logic to filter out all data filters

@Atash3000 Atash3000 requested a review from joshlacey January 17, 2025 16:10
}
})

return [...updatedFilters, ...urlFilters]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this reorder sharedFilter indexes? They should stay in the same place as the dashboardFilter visualizations are referencing those indexes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants