refactor(editor): Enable collaboration features only in NodeView v2 (no-changelog) #10756
Conversation
n8n-assistant
bot
added
n8n team
n8n
|
||||||||||||||||||||||||||||
| Project |
n8n
|
| Branch Review |
remove-collaboration-from-v1-nodeview
|
| Run status |
|
| Run duration | 04m 43s |
| Commit |
|
| Committer | कारतोफ्फेलस्क्रिप्ट™ |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
427
|
| View all changes introduced in this branch ↗︎ | |
|
✅ All Cypress E2E specs passed |
netroy
force-pushed
the
remove-collaboration-from-v1-nodeview
branch
from
b249fa4 to
acdc168
Compare
netroy
force-pushed
the
remove-collaboration-from-v1-nodeview
branch
from
acdc168 to
5b4ba90
Compare
| const msgData: IActiveWorkflowUsersChanged = { | ||
| const users = await this.userRepository.getByIds(this.userRepository.manager, userIds); | ||
| const activeCollaborators = users.map((user) => ({ | ||
| user: user.toIUser(), |
There was a problem hiding this comment.
What's the difference between IUser and Public user? Which one should be used going forward?
There was a problem hiding this comment.
IUser is the just id, email, firstName, and lastName. We should be returning that to the frontend everywhere, unless any other field is explicitly needed.
PublicUser was originally meant to be this, but has now instead become too loaded, and leaks too much data about other users.
There was a problem hiding this comment.
Maybe we could mark it as @deprecated
| @@ -17,42 +15,40 @@ import { useWorkflowsStore } from '@/stores/workflows.store'; | |||
| export function useBeforeUnload({ route }: { route: ReturnType<typeof useRoute> }) { | |||
There was a problem hiding this comment.
It would actually make more sense to rename this into useCanvasBeforeUnload since this contains business logic and split out a generic useBeforeUnload into a separate composable. No need to do here, I can also do it as a follow up.
There was a problem hiding this comment.
this is mostly generic now, and can be used by other views as well.
We do have uiStore.stateIsDirty that's currently only used in canvas. but we could reuse that to mark when any view has unsaved changes.
|
|
||
| const heartbeatTimer = ref<number | null>(null); | ||
|
|
||
| const startHeartbeat = () => { |
There was a problem hiding this comment.
Good idea to bring this into this store 👍
| return; | ||
| notifyWorkflowClosed(); | ||
| stopHeartbeat(); | ||
| pushStore.clearQueue(); |
There was a problem hiding this comment.
I'm not sure if clearing the queue should be collaborationStores responsibility, as the queue should be private implementation detail of pushStore. Sure it works now, as this is the only one sending messages, but in a general case there could be items from other producers.
There was a problem hiding this comment.
I think we need to move the queue in this store. but I'd like to do that refactor in another PR.
|
✅ All Cypress E2E specs passed |
|
Got released with |
* master: refactor(core): Revamp subworkflow policy checker errors (#10752) ci(benchmark): Add missing license cert (#10773) feat(editor): Show Collaboration pane only when there are multiple active users (#10772) 🚀 Release 1.59.0 (#10775) feat: Add credential help to Assistant (no-changelog) (#10736) fix(editor): Fix error rendering and indexing of LLM sub-node outputs (#10688) feat(OpenAI Node): Add Max Tools Iteration parameter and prevent tool calling after execution is aborted (#10735) fix(editor): Revert remove tooltip from info tip (no-changelog) (#10771) refactor(editor): Enable collaboration features only in NodeView v2 (no-changelog) (#10756) fix(If Node): Update copy for type conversion parameter (#10769) build(core): Allow adding tests side by side with the code (#10760) # Conflicts: # packages/editor-ui/src/stores/assistant.store.ts
Summary
collaboration features aren't working on NodeView v1 as expected. Instead of trying to fix it, in this PR, we disable these features altogether for v1.
Review / Merge checklist