fix(models): clicking Archive on a disabled model had no effect#1535
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the functionality to archive and restore models, including UI updates for row actions, a new confirmation dialog, a GraphQL mutation hook for status updates, and localized strings. Feedback was provided to improve the implementation of the status change handler by using the mutate method instead of mutateAsync with an empty catch block. Additionally, it is recommended to make the success and error messages in the useUpdateModelStatus hook more dynamic to accurately reflect different status transitions.
Greptile SummaryThis PR fixes a broken Archive action on the models page — The previous error-context issue (hardcoded "Archive Model" string on restore errors) is correctly addressed: Confidence Score: 5/5Safe to merge — the fix is minimal, well-scoped, and follows existing patterns throughout the codebase. All changes are isolated to the models UI and locale files. The prior error-context concern from the previous review thread has been resolved. No new logic paths touch auth, data integrity, or shared state outside the models feature. No P0/P1 findings. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
actor User
participant RowActions as DataTableRowActions
participant Context as ModelsContext
participant Dialogs as ModelsDialogs
participant Dialog as ModelsArchiveDialog
participant Hook as useUpdateModelStatus
participant API as GraphQL API
User->>RowActions: Click Archive / Restore
RowActions->>Context: setCurrentRow(model)
RowActions->>Context: setOpen('archive')
Context-->>Dialogs: open === 'archive'
Dialogs->>Dialog: render ModelsArchiveDialog
Dialog->>Context: read currentRow.status
Note over Dialog: isArchived = status === 'archived'
Dialog-->>User: Show confirm dialog (Archive or Restore)
User->>Dialog: Confirm
Dialog->>Hook: mutate({ id, status: isArchived ? 'enabled' : 'archived' })
Hook->>API: updateModelStatus(id, status)
API-->>Hook: { updateModelStatus: true }
Hook->>Hook: invalidateQueries(['models'])
Hook->>Hook: toast.success(archiveSuccess | restoreSuccess)
Hook->>Dialog: onSuccess callback
Dialog->>Context: setOpen(null)
Reviews (3): Last reviewed commit: "fix(models): narrow useUpdateModelStatus..." | Re-trigger Greptile |
The dropdown menu called setOpen('archive') but ModelsDialogs never
rendered a dialog for open === 'archive', so clicking Archive did nothing.
- Add useUpdateModelStatus mutation hook calling updateModelStatus GraphQL
- Add ModelsArchiveDialog with archive/restore confirmation
- Wire ModelsArchiveDialog into ModelsDialogs for open === 'archive'
- Add Restore menu item for archived models (previously unreachable)
- Add i18n keys for archive/restore dialogs and success toasts (en, zh-CN)
e0b6f20 to
7a800c0
Compare
The hook only handles archive/restore, not disable. Narrowing the status union prevents callers from accidentally passing 'disabled'.
* fix(models): wire archive dialog so clicking Archive on a model works
The dropdown menu called setOpen('archive') but ModelsDialogs never
rendered a dialog for open === 'archive', so clicking Archive did nothing.
- Add useUpdateModelStatus mutation hook calling updateModelStatus GraphQL
- Add ModelsArchiveDialog with archive/restore confirmation
- Wire ModelsArchiveDialog into ModelsDialogs for open === 'archive'
- Add Restore menu item for archived models (previously unreachable)
- Add i18n keys for archive/restore dialogs and success toasts (en, zh-CN)
* fix(models): narrow useUpdateModelStatus type to enabled|archived
The hook only handles archive/restore, not disable. Narrowing the
status union prevents callers from accidentally passing 'disabled'.
Summary
The dropdown menu called setOpen('archive') but ModelsDialogs never rendered a dialog for open === 'archive', so clicking Archive did nothing.
Spirit/Intent
Fix the broken archive action on the models page so users can archive/restore models.
Key Changes
Risks
None identified - changes are isolated to models UI components and locales.