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 summary case card issues. (PT-188001359) #1526

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions v3/cypress/e2e/case-card.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ context("case card", () => {
table.toggleCaseView()
cy.wait(500)
cy.get('[data-testid="case-card-view"]').should("have.length", 3)
cy.get('[data-testid="case-card-view-next-button"]').eq(0).click()
cy.log("Add new case to 'middle' collection.")
cy.get('[data-testid="case-card-view"]').eq(1).find('[data-testid="case-card-view-index"]')
.eq(0).should("have.text", "4 cases")
Expand All @@ -264,7 +265,7 @@ context("case card", () => {
toolbar.getUndoTool().click()
toolbar.getUndoTool().click()
cy.get('[data-testid="case-card-view"]').eq(1).find('[data-testid="case-card-view-index"]')
.eq(0).should("have.text", "4 cases")
.eq(0).should("have.text", "1 of 4")
toolbar.getRedoTool().click()
toolbar.getRedoTool().click()
cy.get('[data-testid="case-card-view"]').eq(1).find('[data-testid="case-card-view-index"]')
Expand Down Expand Up @@ -328,16 +329,16 @@ context("case card", () => {
cy.wait(500)
cy.get('[data-testid="trash-menu-list"]').should("be.visible")
card.getDeleteSelectedCasesButton().click()
cy.get('[data-testid="case-card-view-index"]').should("have.text", "26 cases")
cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "26 values")
cy.get('[data-testid="case-card-view-next-button"]').click()
cy.get('[data-testid="case-card-view-index"]').should("have.text", "1 of 26")
cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Asian Elephant")
cy.get('[data-testid="case-card-view-next-button"]').click()
cy.get('[data-testid="case-card-view-index"]').should("have.text", "2 of 26")
cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Big Brown Bat")
card.getDeleteCasesButton().click()
cy.wait(500)
card.getDeleteUnselectedCasesButton().click()
cy.get('[data-testid="case-card-view-index"]').should("have.text", "1 of 1")
cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Big Brown Bat")
cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Asian Elephant")
})
it("allows user to set aside and restore cases from inspector panel", () => {
table.toggleCaseView()
Expand All @@ -353,8 +354,8 @@ context("case card", () => {
card.getRestoreSetAsideCasesButton().should("be.disabled").and("have.text", "Restore 0 Set Aside Cases")
// resorting to {force: true} because this is failing in CI, though it passes locally
card.getSetAsideSelectedCasesButton().click({force: true})
cy.get('[data-testid="case-card-view-index"]').should("have.text", "1 of 26")
cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "Asian Elephant")
cy.get('[data-testid="case-card-view-index"]').should("have.text", "26 cases")
cy.get('[data-testid="case-card-attr-value"]').first().should("have.text", "26 values")
card.getHideShowButton().click()
cy.wait(500)
card.getRestoreSetAsideCasesButton().should("not.be.disabled").and("have.text", "Restore 1 Set Aside Cases")
Expand Down
2 changes: 1 addition & 1 deletion v3/src/components/case-card/card-view.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
color: #0592af;
cursor: pointer;
display: block;
font-size: 10px;
font-size: 12px;
font-weight: bold;
margin: 0 auto;
padding: 6px 8px;
Expand Down
43 changes: 17 additions & 26 deletions v3/src/components/case-card/card-view.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { useEffect, useRef } from "react"
import React, { useRef, useState } from "react"
import { observer } from "mobx-react-lite"
import { mstReaction } from "../../utilities/mst-reaction"
import { CollectionContext } from "../../hooks/use-collection-context"
import { AttributeHeaderDividerContext } from "../case-tile-common/use-attribute-header-divider-context"
import { CaseView } from "./case-view"
Expand All @@ -18,40 +17,32 @@
const cardModel = useCaseCardModel()
const data = cardModel?.data
const collections = data?.collections
const areAllCollectionsSummarized = !!collections?.every(c => cardModel?.summarizedCollections.includes(c.id))
const rootCollection = collections?.[0]
const selectedItems = data?.selection
const selectedItemId = selectedItems && Array.from(selectedItems)[0]
const selectedItemLineage = cardModel?.caseLineage(selectedItemId)
const contentRef = useRef<HTMLDivElement>(null)
const summarizedCollections = cardModel?.summarizedCollections || []
const [isInSummaryMode, setIsInSummaryMode] = useState(summarizedCollections.length !== 0)

const handleSelectCases = (caseIds: string[], collectionId: string) => {
cardModel?.setShowSummary(caseIds.length > 1, collectionId)
const handleSelectCases = (caseIds: string[]) => {
data?.setSelectedCases(caseIds)
}

const handleSummaryButtonClick = () => {
cardModel?.setShowSummary(!areAllCollectionsSummarized)
}

// The first time the card is rendered, summarize all collections unless there is a selection.
useEffect(function startWithAllCollectionsSummarized() {
if (data?.selection.size === 0) {
cardModel?.summarizeAllCollections()
if (isInSummaryMode) {
// select the first child-most case
const firstItemId = data?.itemIds[0]
const firstItemLineage = cardModel?.caseLineage(firstItemId)
if (firstItemLineage) {
data?.setSelectedCases([firstItemLineage[firstItemLineage.length - 1]])
}
setIsInSummaryMode(false)
} else {

Check warning on line 41 in v3/src/components/case-card/card-view.tsx

View check run for this annotation

Codecov / codecov/patch

v3/src/components/case-card/card-view.tsx#L41

Added line #L41 was not covered by tests
data?.setSelectedCases([])
setIsInSummaryMode(true)
}
}, [cardModel, data])

// When all cases are selected, show summary.
useEffect(function showSummaryOnAllCasesSelection() {
return mstReaction(
() => selectedItems?.size,
() => {
if (selectedItems?.size === data?.items.length) {
cardModel?.summarizeAllCollections()
}
}, {name: "CardView.showSummaryOnAllCasesSelection"}, data
)
}, [cardModel, data, selectedItems])
}

return (
<div ref={contentRef} className="case-card-content" data-testid="case-card-content">
Expand All @@ -72,7 +63,7 @@
onClick={handleSummaryButtonClick}
disabled={data?.items.length === 0}
>
{ areAllCollectionsSummarized
{ isInSummaryMode
? t("V3.caseCard.summaryButton.showIndividualCases")
: t("V3.caseCard.summaryButton.showSummary")
}
Expand Down
120 changes: 73 additions & 47 deletions v3/src/components/case-card/case-card-model.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Instance, SnapshotIn, types } from "mobx-state-tree"
import { getTileCaseMetadata, getTileDataSet } from "../../models/shared/shared-data-utils"
import { ISharedModel } from "../../models/shared/shared-model"
import { ITileContentModel, TileContentModel } from "../../models/tiles/tile-content"
import { kCaseCardTileType } from "./case-card-defs"
import { ICollectionModel } from "../../models/data/collection"
Expand All @@ -13,7 +12,6 @@
type: types.optional(types.literal(kCaseCardTileType), kCaseCardTileType),
// key is collection id; value is width
attributeColumnWidths: types.map(types.number),
summarizedCollections: types.optional(types.array(types.string), [])
})
.views(self => ({
get data() {
Expand All @@ -26,6 +24,40 @@
return self.attributeColumnWidths.get(collectionId)
}
}))
.views(self => ({
get summarizedCollections() {
const selectedItems = self.data?.selection
const items = self.data?.items
const collections = self.data?.collections

if (!collections || !items || selectedItems?.size === 1) {
return []
} else if (!selectedItems || selectedItems.size === 0 || selectedItems.size === items.length) {
return collections?.map(c => c.id) ?? []
} else {

Check warning on line 37 in v3/src/components/case-card/case-card-model.ts

View check run for this annotation

Codecov / codecov/patch

v3/src/components/case-card/case-card-model.ts#L37

Added line #L37 was not covered by tests
const collectionIdsToSummarize: string[] = []
const summarizeCollectionsRecursively = (index = 0) => {
if (index >= collections.length - 1) return
const collection = collections[index]
const collectionCases = collection.cases
const selectedChildrenPerCase = collectionCases.map(collectionCase => {
const caseGroup = self.data?.caseInfoMap.get(collectionCase.__id__)
if (!caseGroup?.childCaseIds) return 0
return caseGroup.childCaseIds.filter(childCaseId => self.data?.isCaseSelected(childCaseId)).length
})
if (selectedChildrenPerCase.filter(v => v > 0).length > 1) {
collectionIdsToSummarize.push(collection.id)
}
summarizeCollectionsRecursively(index + 1)
}

summarizeCollectionsRecursively()
// always summarize the last collection
collectionIdsToSummarize.push(collections[collections.length - 1].id)
return collectionIdsToSummarize
}
}
}))
.views(self => ({
caseLineage(itemId?: string) {
if (!itemId) return undefined
Expand All @@ -35,19 +67,19 @@
const parentCaseInfo = self.data?.caseInfoMap.get(parentCaseId)
if (!parentCaseInfo?.childCaseIds) return undefined
return parentCaseInfo.childCaseIds
.map(childCaseId => self.data?.caseInfoMap.get(childCaseId)?.groupedCase)
.filter(groupedCase => !!groupedCase)
.map(childCaseId => self.data?.caseInfoMap.get(childCaseId)?.groupedCase)
.filter(groupedCase => !!groupedCase)
},
displayValues(collection: ICollectionModel, caseItem: IGroupedCase) {

const getNumericSummary = (numericValues: number[], attrUnits: string): string => {
const minValue = Math.min(...numericValues)
const maxValue = Math.max(...numericValues)
return minValue === maxValue
? `${minValue}${attrUnits ? ` ${attrUnits}` : ""}`
: `${minValue}-${maxValue}${attrUnits ? ` ${attrUnits}` : ""}`
? `${minValue}${attrUnits ? ` ${attrUnits}` : ""}`
: `${minValue}-${maxValue}${attrUnits ? ` ${attrUnits}` : ""}`
}

const getCategoricalSummary = (uniqueValues: Set<IValueType>): string => {
const uniqueValuesArray = Array.from(uniqueValues)
if (uniqueValuesArray.length === 1) {
Expand All @@ -58,15 +90,15 @@
return `${uniqueValuesArray.length} values`
}
}

if (self.summarizedCollections.includes(collection.id)) {
const summaryMap = collection?.attributes.reduce((acc: Record<string, string>, attr) => {
if (!attr || !attr.id) return acc

const selectedCases = self.data?.selection
const casesToUse = selectedCases && selectedCases.size >= 1
? Array.from(selectedCases).map((id) => ({ __id__: id }))
: collection.cases
? Array.from(selectedCases).map((id) => ({ __id__: id }))
: collection.cases
const allValues = casesToUse.map(c => self.data?.getValue(c.__id__, attr.id))
const uniqueValues = new Set(allValues)
const isNumeric = attr.numValues?.some((v, i) => attr.isNumeric(i))
Expand All @@ -78,7 +110,7 @@
summary = getNumericSummary(numericValues, attrUnits)
} else {
summary = getCategoricalSummary(uniqueValues)
}
}
return { ...acc, [attr.id]: summary }
}, {})
return collection?.attributes.map(attr => attr?.id && summaryMap[attr.id]) ?? []
Expand All @@ -87,11 +119,6 @@
}
}
}))
.actions(self => ({
setSummarizedCollections(collections: string[]) {
self.summarizedCollections.replace(collections)
}
}))
.actions(self => ({
setAttributeColumnWidth(collectionId: string, width?: number) {
if (width) {
Expand All @@ -101,48 +128,47 @@
self.attributeColumnWidths.delete(collectionId)
}
},
addNewCase(cases: IGroupedCase[], collection: ICollectionModel, displayedCaseId: string) {
addNewCase(collection: ICollectionModel) {
const newCase: ICaseCreation = {}
const selectedCases = self.data?.selection

collection.allParentDataAttrs.forEach(attr => {
if (attr?.id) {
const value = self.data?.getValue(displayedCaseId, attr.id)
newCase[attr.id] = value
function findCommonCases(lineages: (readonly string[])[]) {
if (lineages.length === 0) return []
let commonValues = lineages[0]
for (let i = 1; i < lineages.length; i++) {
commonValues = commonValues.filter(value => lineages[i].includes(value))
if (commonValues.length === 0) {
return []

Check warning on line 141 in v3/src/components/case-card/case-card-model.ts

View check run for this annotation

Codecov / codecov/patch

v3/src/components/case-card/case-card-model.ts#L141

Added line #L141 was not covered by tests
}
}
})
return commonValues
}

if (selectedCases) {
const caseLineages = Array.from(selectedCases).map(caseId => self.caseLineage(caseId) || [])
const commonCaseIds = findCommonCases(caseLineages)
if (commonCaseIds.length > 0) {
const nearestCommonParentCaseId = commonCaseIds[commonCaseIds.length - 1]
const parentAttrs = collection.allParentAttrs
parentAttrs.forEach(attr => {
const attrId = attr.id
const parentValue = nearestCommonParentCaseId && self.data?.getValue(nearestCommonParentCaseId, attrId)
newCase[attrId] = parentValue
})
}
}

const [newCaseId] = self.data?.addCases([newCase]) ?? []

// TODO: Figure out why this call to validateCases is necessary. The Cypress test for adding a case
// fails if it isn't here, though adding a case seems to work fine when tested manually.
self.data?.validateCases()

return newCaseId
},
setShowSummary(show: boolean, collectionId?: string) {
if (show) {
self.data?.setSelectedCases([])
}

const updatedSummarizedCollections = show
? collectionId
? [...self.summarizedCollections, collectionId]
: self.data?.collections.map(c => c.id) ?? []
: collectionId
? self.summarizedCollections.filter(cid => cid !== collectionId)
: []

self.setSummarizedCollections(updatedSummarizedCollections)
},
summarizeAllCollections() {
self.setSummarizedCollections(self.data?.collections.map(c => c.id) ?? [])
},
updateAfterSharedModelChanges(sharedModel?: ISharedModel) {
// TODO
},
return newCaseId
}
}))
export interface ICaseCardModel extends Instance<typeof CaseCardModel> {}
export interface ICaseCardSnapshot extends SnapshotIn<typeof CaseCardModel> {}
export interface ICaseCardModel extends Instance<typeof CaseCardModel> { }
export interface ICaseCardSnapshot extends SnapshotIn<typeof CaseCardModel> { }

export function isCaseCardModel(model?: ITileContentModel): model is ICaseCardModel {
return model?.type === kCaseCardTileType
Expand Down
12 changes: 5 additions & 7 deletions v3/src/components/case-card/case-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import "./case-view.scss"
interface ICaseViewProps {
cases: IGroupedCase[]
level: number
onSelectCases: (caseIds: string[], collection: string) => void
onSelectCases: (caseIds: string[]) => void
displayedCaseLineage?: readonly string[]
onNewCollectionDrop: (dataSet: IDataSet, attrId: string, beforeCollectionId: string) => void
}
Expand Down Expand Up @@ -69,17 +69,16 @@ export const CaseView = observer(function CaseView(props: ICaseViewProps) {
: displayedCaseIndex + delta
const newCase = cases[selectedCaseIndex]
if (!newCase.__id__) return

onSelectCases([newCase.__id__], collectionId)
}, [isCollectionSummarized, cases, displayedCaseIndex, onSelectCases, collectionId])
onSelectCases([newCase.__id__])
}, [isCollectionSummarized, cases, displayedCaseIndex, onSelectCases])

const handleAddNewCase = () => {
if (collection) {
let newCaseId: string | undefined
data?.applyModelChange(() => {
const newItemId = cardModel?.addNewCase(cases, collection, displayedCaseId)
const newItemId = cardModel?.addNewCase(collection)
newCaseId = newItemId && data?.getItemCaseIds(newItemId)[level]
newCaseId && onSelectCases([newCaseId], collectionId)
newCaseId && onSelectCases([newCaseId])
}, {
notify: () => {
if (newCaseId) {
Expand All @@ -89,7 +88,6 @@ export const CaseView = observer(function CaseView(props: ICaseViewProps) {
undoStringKey: "DG.Undo.caseTable.createNewCase",
redoStringKey: "DG.Redo.caseTable.createNewCase"
})
cardModel?.setShowSummary(false)
}
}

Expand Down
Loading