Skip to content

MMT-4089: Fix inconsistencies with Collection Association #1418

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 17, 2025
Merged

MMT-4089: Fix inconsistencies with Collection Association #1418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e305ea
MMT-4089: Fix inconsistencies with Collection Association
mandyparson Oct 15, 2025
79ee029
MMT-4089: Editing based on CMR feedback
mandyparson Oct 16, 2025
20bc113
MMT-4089: Addressing PR Feedback
mandyparson Oct 16, 2025
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
327 changes: 164 additions & 163 deletions static/src/js/components/CollectionAssociationForm/CollectionAssociationForm.jsx
View file
Open in desktop

Large diffs are not rendered by default.

379 changes: 235 additions & 144 deletions .../src/js/components/CollectionAssociationForm/__tests__/CollectionAssociationForm.test.jsx
View file
Open in desktop

Large diffs are not rendered by default.

440 changes: 205 additions & 235 deletions .../components/CollectionAssociationForm/__tests__/__mocks__/CollectionAssociationResults.js
View file
Open in desktop

Large diffs are not rendered by default.

174 changes: 94 additions & 80 deletions static/src/js/components/ManageCollectionAssociation/ManageCollectionAssociation.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import React, { useCallback, useState } from 'react'
import pluralize from 'pluralize'

import Button from '@/js/components/Button/Button'
import ControlledPaginatedContent from '@/js/components/ControlledPaginatedContent/ControlledPaginatedContent'
import CustomModal from '@/js/components/CustomModal/CustomModal'
import EllipsisText from '@/js/components/EllipsisText/EllipsisText'
import Pagination from '@/js/components/Pagination/Pagination'
import Table from '@/js/components/Table/Table'

import conceptTypeQueries from '@/js/constants/conceptTypeQueries'
Expand All @@ -41,14 +41,17 @@ const ManageCollectionAssociation = () => {

const { addNotification } = useNotificationsContext()

const [searchParams, setSearchParams] = useSearchParams()
const derivedConceptType = getConceptTypeByConceptId(conceptId)

// Variables for deletion mutation
const [collectionConceptIds, setCollectionConceptIds] = useState([])
const [showDeleteModal, setShowDeleteModal] = useState(false)
const [isDeleting, setIsDeleting] = useState(false)

const derivedConceptType = getConceptTypeByConceptId(conceptId)

// Variables for pagination
const [searchParams, setSearchParams] = useSearchParams()
const [activePage, setActivePage] = useState(1)
const limit = 20
const activePage = parseInt(searchParams.get('page'), 10) || 1
const offset = (activePage - 1) * limit

let params = {
Expand Down Expand Up @@ -79,10 +82,7 @@ const ManageCollectionAssociation = () => {
})

const [deleteAssociationMutation] = useMutation(DELETE_ASSOCIATION, {
refetchQueries: [{
query: conceptTypeQueries[derivedConceptType],
variables: params
}],

onCompleted: () => {
setShowDeleteModal(false)

Expand All @@ -93,8 +93,14 @@ const ManageCollectionAssociation = () => {
})

setCollectionConceptIds([])

// Gives time for CMR to update data
setTimeout(() => {
Copy link
Collaborator

@cgokey cgokey Oct 16, 2025

Choose a reason for hiding this comment

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

One pattern I found for handling this that could be implemented is below:

Suggested change
setTimeout(() => {
setTimeout(() => { 
const pollForChanges = (attempts = 0) => {
  if (attempts > 5) {
    addNotification({
      message: 'Unable to confirm update. Please refresh manually.',
      variant: 'warning'
    })
    return
  }

  refetch().then((result) => {
    if (/* check if data is updated */) {
      // Data updated successfully
    } else {
      setTimeout(() => pollForChanges(attempts + 1), 1000)
    }
  })
}

onCompleted: () => {
  setShowDeleteModal(false)
  addNotification({
    message: 'Collection Associations Deleted Successfully!',
    variant: 'success'
  })
  setCollectionConceptIds([])
  pollForChanges()
},

Copy link
Contributor Author

@mandyparson mandyparson Oct 17, 2025

Choose a reason for hiding this comment

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

In 10 attempts, the setTimeout appears to work each time. Though I do think we should consider polling for future CMR lag issues.

Copy link
Collaborator

@cgokey cgokey Oct 17, 2025

Choose a reason for hiding this comment

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

Yeah I think we can probably come up with a generic component too

refetch()
}, 250)
},
onError: () => {
setShowDeleteModal(false)
addNotification({
message: 'Error disassociating collection',
variant: 'danger'
Expand All @@ -105,13 +111,15 @@ const ManageCollectionAssociation = () => {
})

// Handles deleting selected collection
// if no collections selected, returns an error notification
const handleDeleteAssociation = () => {
Copy link
Collaborator

@cgokey cgokey Oct 16, 2025

Choose a reason for hiding this comment

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

May want to surface an error if can't delete -- const handleDeleteAssociation = () => {

Suggested change
const handleDeleteAssociation = () => {
const handleDeleteAssociation = () => {
setIsDeleting(true);
deleteAssociationMutation({
variables: {
conceptId,
associatedConceptIds: collectionConceptIds
}
}).catch((error) => {
// Handle error
console.error('Failed to delete association:', error);
addNotification({
message: 'Failed to delete association',
variant: 'danger'
});
}).finally(() => {
setIsDeleting(false);
});
};```

Copy link
Contributor Author

@mandyparson mandyparson Oct 17, 2025

Choose a reason for hiding this comment

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

This is done in deleteAssociationMutation's onError

setIsDeleting(true)
deleteAssociationMutation({
variables: {
conceptId,
associatedConceptIds: collectionConceptIds
}
}).finally(() => {
setIsDeleting(false)
})
}

Expand All @@ -124,14 +132,13 @@ const ManageCollectionAssociation = () => {
if (order === 'ascending') nextSortKey = `-${key}`
if (order === 'descending') nextSortKey = key

// Reset the page parameter
currentParams.delete('page')

// Set the sort key
currentParams.set('sortKey', nextSortKey)

return Object.fromEntries(currentParams)
})

setActivePage(1) // Reset to first page when sorting
}, [])

const buildEllipsisTextCell = useCallback((cellData) => (
Expand All @@ -140,8 +147,8 @@ const ManageCollectionAssociation = () => {
</EllipsisText>
), [])

// Handles checkbox selections, if checked add the conceptId to the state variable
// and pops the added conceptId from the array.
// Adds or removes checked collections from collectionConceptIds array
// which is provided to the deleteMutation
const handleCheckbox = (event) => {
const { target } = event
const { value } = target
Expand Down Expand Up @@ -200,11 +207,7 @@ const ManageCollectionAssociation = () => {
]

const setPage = (nextPage) => {
setSearchParams((currentParams) => {
currentParams.set('page', nextPage)

return Object.fromEntries(currentParams)
})
setActivePage(nextPage)
}

const toggleShowDeleteModal = (nextState) => {
Expand All @@ -216,91 +219,101 @@ const ManageCollectionAssociation = () => {
toggleShowDeleteModal(true)
})

// Handle refresh, calls getMetadata to get the list of association
// TODO: MMT-4089 See if we can get rid of this refresh button.
const handleRefreshPage = () => {
refetch()
}

const refreshAccessibleEventProps = useAccessibleEvent(() => {
handleRefreshPage()
})

const { [camelCase(derivedConceptType)]: concept } = data

const { collections: associatedCollections } = concept

const { items = [], count } = associatedCollections

const totalPages = Math.ceil(count / limit)

const currentPageIndex = Math.floor(offset / limit)
const firstResultIndex = currentPageIndex * limit
const isLastPage = totalPages === activePage
const lastResultIndex = firstResultIndex + (isLastPage ? count % limit : limit)

const paginationMessage = count > 0
? `Showing ${totalPages > 1 ? `Collection Associations ${firstResultIndex + 1}-${lastResultIndex} of ${count}` : `${count} ${pluralize('Collection Association', count)}`}`
: 'No Collection Associations found'
const { items, count } = associatedCollections

return (
<div className="mt-4">
<div>
<Alert className="fst-italic fs-6" variant="warning">
<i className="eui-icon eui-fa-info-circle" />
{' '}
Association operations may take some time. If you are not seeing what you expect below,
please
{' '}
<span
className="text-decoration-underline"
<button
className="btn btn-link p-0 text-decoration-underline"
style={
{
color: 'blue',
cursor: 'pointer'
}
}
// eslint-disable-next-line react/jsx-props-no-spreading
{...refreshAccessibleEventProps}
onClick={() => refetch()}
aria-label="Refresh the page"
type="button"
>
refresh the page
</span>
<i>refresh the page</i>
</button>
</Alert>
<Row className="d-flex justify-content-between align-items-center mb-4 mt-5">
<Col className="mb-4 flex-grow-1" xs="auto">
{
(!!count) && (
<span className="text-secondary fw-bolder">{paginationMessage}</span>
<ControlledPaginatedContent
activePage={activePage}
count={count}
limit={limit}
setPage={setPage}
>
{
({
totalPages,
pagination,
firstResultPosition,
lastResultPosition
}) => {
const paginationMessage = count > 0
? `Showing ${totalPages > 1 ? `${firstResultPosition}-${lastResultPosition} of` : ''} ${count} Collection ${pluralize('Association', count)}`
: 'No collection associations found'

return (
<>
<Row className="d-flex justify-content-between align-items-center mb-4">
<Col className="mb-4 flex-grow-1" xs="auto">
{
(!!count) && (
<span className="text-secondary fw-bolder">{paginationMessage}</span>
)
}
</Col>
<Col className="mb-4 flex-grow-1" xs="auto" />
{
totalPages > 1 && (
<Col xs="auto">
{pagination}
</Col>
)
}
</Row>
<Table
className="m-5"
columns={columns}
data={items}
generateCellKey={({ conceptId: conceptIdCell }, dataKey) => `column_${dataKey}_${conceptIdCell}`}
generateRowKey={({ conceptId: conceptIdRow }) => `row_${conceptIdRow}`}
id="associated-collections"
limit={count}
noDataMessage="No collection associations found."
/>
{
totalPages > 1 && (
<Row>
<Col xs="12" className="pt-4 d-flex align-items-center justify-content-center">
<div>
{pagination}
</div>
</Col>
</Row>
)
}
</>
)
}
</Col>
{
totalPages > 1 && (
<Col xs="auto">
<Pagination
setPage={setPage}
activePage={activePage}
totalPages={totalPages}
/>
</Col>
)
}
</Row>
<Table
className="m-5"
columns={columns}
data={items}
generateCellKey={({ conceptId: conceptIdCell }, dataKey) => `column_${dataKey}_${conceptIdCell}`}
generateRowKey={({ conceptId: conceptIdRow }) => `row_${conceptIdRow}`}
id="associated-collections"
limit={count}
noDataMessage="No collection associations found."
offset={offset}
/>
</ControlledPaginatedContent>
<Button
className="mt-4"
variant="danger"
disabled={collectionConceptIds.length === 0}
// eslint-disable-next-line react/jsx-props-no-spreading
disabled={collectionConceptIds.length === 0 || isDeleting}
{...accessibleEventProps}
>
Delete Selected Associations
Expand All @@ -319,7 +332,8 @@ const ManageCollectionAssociation = () => {
{
label: 'Yes',
variant: 'primary',
onClick: handleDeleteAssociation
onClick: handleDeleteAssociation,
disabled: isDeleting
}
]
}
Expand Down
Loading