Skip to content

LEAN-3843 #346

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

Open
fwaisi wants to merge 1 commit into
base: master
Choose a base branch
from
Open

LEAN-3843 #346

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion src/Controller/V2/Document/DocumentRoute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ export class DocumentRoute extends BaseRoute {
payload,
user
)

Copilot AI Jan 14, 2026

Copy link

Choose a reason for hiding this comment

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

Removing the broadcastSteps call may break real-time synchronization for collaborative editing. If this broadcast is no longer needed, ensure that step broadcasting is handled elsewhere in the application flow, or verify that real-time updates are intentionally being disabled.

Suggested change
)
)
await this.documentController.broadcastSteps(projectID, manuscriptID, payload, user)

Copilot uses AI. Check for mistakes.
this.documentController.broadcastSteps(manuscriptID, result)
res.status(StatusCodes.OK).send(result)
}
Comment on lines -166 to 168

@mbetamony mbetamony Jan 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we getting rid of this?

@fwaisi fwaisi Jan 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

because the worker will be responsible to broadcast the steps.

@mbetamony mbetamony Jan 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Even locally? And on dev environments? We currently have the worker only on dev2.

@fwaisi fwaisi Jan 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don’t think we need this locally. I think we should enable it across all environments.

As a fallback, we could implement an alternative approach, but I need to figure out how to support that in the API repo.

@mbetamony mbetamony Jan 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was the fallback, so we don't have to run the worker locally. We do need a way to to send steps back to the FE from the BE, no?

@fwaisi fwaisi Jan 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The new implementation keeps the POST request as a fallback, allowing the FE to send steps to the BE and receive them back without broadcasting.

However, it’s not clear how this acts as a fallback. With the current implementation, it seems to introduce a redundant step.

@mbetamony mbetamony Jan 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We send steps back to the FE. This is needed if multiple users are editing the same document.

@fwaisi fwaisi Jan 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, now the worker will be responsible to handle this.

But I think we can keep it as a fallback and make it working only when the user send the steps by post request.

here is the worker pr: https://github.com/atypon/manuscripts-cf-worker/pull/5


Expand Down