feat(releases): adding correct confirmation dialog to archive release flow #7827
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
No changes to documentation |
|
Component Testing Report Updated Nov 18, 2024 4:28 PM (UTC) ✅ All Tests Passed -- expand for details
|
⚡️ Editor Performance ReportUpdated Mon, 18 Nov 2024 16:29:53 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
…/sanity into corel-255-archive-confirmation
| @@ -0,0 +1,17 @@ | |||
| import {type ReleaseDocument} from '../store/types' | |||
|
|
|||
| export const activeScheduledRelease: ReleaseDocument = { | |||
There was a problem hiding this comment.
Thinking that we'd continue to create others here eg activeASAPRelease as are necessary for test cases across our suite. All fixtures would be correct and valid states, eg activeASAPRelease would not have an intendedPublishAt since that is not a valid state machine value
| vi.mock('sanity', () => ({ | ||
| SANITY_VERSION: '0.0.0', | ||
| useTranslation: vi.fn().mockReturnValue({t: vi.fn()}), | ||
| vi.mock('../../../../store/useReleaseOperations', () => ({ |
There was a problem hiding this comment.
Tried in vain to simplify the way this could be mocked, but a mixture of issues an nuance with how vi.mock is hoisted and executed mean we still need these mock statements explicitly in our tests rather than in generalised mock files
There was a problem hiding this comment.
I rogue thought I had when I was reviewing this related to where wea re placing the fixtures.
I think it might make better sense to keep them somewhere along like we do with the TestProvider in the test directory?
Right now we are keeping in the core/releases but I feel it will come a point where we might want to use releases in other places for one reason or another. Especially since this does kinda touch more parts than this self contained part (the first thing that comes to mind right now are some components in structure).
I'm not married to the idea but I think findability might be better :)
| </Text> | ||
| ), | ||
| }) | ||
| console.error(archivingError) |
There was a problem hiding this comment.
Rogue console log or meant to be here? :)
There was a problem hiding this comment.
I followed similar approach to what we do in Publish and schedule, so this is intended
| @@ -67,6 +150,8 @@ export const ReleaseMenuButton = ({disabled, release}: ReleaseMenuButtonProps) = | |||
| {!release?.state || release.state === 'archived' ? ( | |||
| <MenuItem | |||
| onClick={handleUnarchive} | |||
| // TODO: disabled as CL action not yet impl | |||
There was a problem hiding this comment.
Do we have a linear story for this? 👀
packages/sanity/src/core/releases/store/createReleaseOperationStore.ts
Outdated
Show resolved
Hide resolved
| telemetry.log(UnarchivedRelease) | ||
| setIsPerformingOperation(false) | ||
| // noop | ||
| // TODO: similar to handleArchive - complete once server action exists |
There was a problem hiding this comment.
what's the reason for deleting this? Do we expect unarchive to change significantly?
There was a problem hiding this comment.
It will change from what it currently is in corel (will be updated to to be similar to archive). I considered implementing it, but feel it's better to complete the full task once the action can be test alongside. There's a separate task I'm monitoring and will complete once ready
packages/sanity/src/core/releases/tool/components/ReleaseMenuButton/ReleaseMenuButton.tsx
Show resolved
Hide resolved
|
|
||
| import {type ReleaseOperationsStore} from '../../createReleaseOperationStore' | ||
|
|
||
| export const useReleaseOperationsMock: Mocked<ReleaseOperationsStore> = { |
There was a problem hiding this comment.
This looks like a great pattern to me!
Description
The current flow for archiving, although correctly using the server action, was not implementing the correct UX.
Now, archiving brings up the correct confirmation dialog.
Unarchiving has also been no-op-ed as the action doesn't exist right now
What to review
Specifically, looking for thoughts on:
useReleaseOperations- https://github.com/sanity-io/sanity/pull/7827/files#diff-4aa171c7928a5161b60dde60a11cd53762cc08c16e27a114558f6da103cacb03ReleaseMenuButton.testsuite - https://github.com/sanity-io/sanity/pull/7827/files#diff-49a11b1a7f4b6c7f99fadf3b5161e16b71d6aea1af3afe1d1e5d25a58513aa54Interested to know thoughts both on directory structure for these mocks and fixtures, and the naming convention for these files also. Further thoughts:
vi.mockusage for export mocks) and fixtures (objects and other primitives that can be used consistently across all our tests)mockssit alongside the files they are mocking the shape of; fixtures sit inside the domain space they are related tomockscan still be overridden on test by test to customiseTesting
Note testing thoughts above
Notes for release
N/A