DT-1017 - support unique workflow table columns per namespace #1455
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
This reverts commit 18ce887.
|
This is awesome. I think I only saw one instance when I was testing that the columns got reset to original default state. It was when I first pulled down, was on default, didn't change anything, went to canary, changed columns, went back to default, changed columns, then went back to canary (columns weren't preserved in change). But I'll keep testing to make sure I can repro. |
rossedfort
force-pushed
the
DT-1017-table-columns-per-ns
branch
from
01cbb1c
to
2d9254d
Compare
else fall back to default columns
| @@ -48,8 +48,8 @@ | |||
| "test:coverage": "TZ=UTC vitest run --coverage", | |||
| "test:e2e": "PW_MODE=e2e playwright test tests/e2e", | |||
| "test:e2e:run": "pnpm test:e2e --ui", | |||
| "test:integration": "PW_MODE=integration playwright test tests/integration", | |||
| "test:integration:run": "pnpm test:integration --ui", | |||
| "test:integration:run": "PW_MODE=integration playwright test tests/integration", | |||
There was a problem hiding this comment.
Was this meant to be switched? Is :run the UI mode or no?
There was a problem hiding this comment.
Yeah I switched it, this is the way our cypress tests used to be. So pnpm test:integration does UI mode just like pnpm cypress used to, and pnpm test:integration:run does headless like pnpm cypress:run used to.
| const workflowTableColumns = persistStore<WorkflowHeader[]>( | ||
| 'workflow-table-columns', | ||
| DEFAULT_COLUMNS, | ||
| const getDefaultColumns = (): WorkflowHeader[] => { |
| @@ -26,9 +60,66 @@ export const mockWorkflowsApis = (page: Page) => { | |||
| mockGlobalApis(page), | |||
| mockWorkflowsApi(page), | |||
| mockSearchAttributesApi(page), | |||
| mockWorkflowsCountApi(page), | |||
| ]); | |||
| }; | |||
|
|
|||
| export const mockNamespaceApis = (page: Page) => { | |||
There was a problem hiding this comment.
These are some solid utils.
There was a problem hiding this comment.
Thanks, I'm hoping we can continue to follow this pattern where we just compose a bunch of mock calls together for each page we need to test. Then you can still import a mock API fn and call it individually if you need to override something.
| @@ -1,6 +1,8 @@ | |||
| import type { Page } from '@playwright/test'; | |||
| import type { GetClusterInfoResponse as Cluster } from '$src/lib/types'; | |||
|
|
|||
| export const CLUSTER_API = '**/api/v1/cluster?'; | |||
|
|
|||
| const MOCK_CLUSTER: Cluster = { | |||
There was a problem hiding this comment.
One thing that is going to be a struggle is remembering the default cluster server version (1.19.3) vs. what you need to pass in for the version for mockClusterApi. Like do we keep updating the default cluster to most recent or most stable (1.20 vs. 1.21) or do we keep it here forever and explicitly pass in newer versions. I think the latter, but it might be a bit of a headache down the road. Just something to think about.
There was a problem hiding this comment.
Yeah that's a good point. It's possible we'd even want to run the same tests against diff cluster versions, so passing in an override will probably be the way to go.
|
Not sure if this is expected, but when I run Playwright locally all the workflows-list.spect.ts tests have the Server Failed to Respond error in them. One test failed on the initial run, then they all passed after that. But seems like those should be getting back mocked workflows. 
|
Description & motivation 💭
Previously on the workflows list page, we would only fetch global search attributes to render the available table columns for the workflows summary table. With the change introduced in #1414, we fetch namespace specific search attributes any time the namespace path parameter changes. This allows us to let users configure their table columns on a per-namespace basis.
Screenshots (if applicable) 📸
Screen.Recording.2023-06-22.at.10.01.19.AM.mov
Design Considerations 🎨
N/a
Testing 🧪
How was this tested 👻
This is covered by existing e2e and unit tests, as well as manual testing
Steps for others to test: 🚶🏽♂️🚶🏽♀️
Checklists
N/a
Draft Checklist
Merge Checklist
Issue(s) closed
Docs
N/a
Any docs updates needed?