Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

TCP-100c Authority #110

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open

TCP-100c Authority #110

wants to merge 40 commits into from

Conversation

mbetamony
Copy link

@mbetamony mbetamony commented Sep 5, 2023

@mbartenev-atypon, I think this is ready for testing. If you can provide me with example requests or if there's a version of the editor that should be using the new API, that would be great.

@mbetamony
Copy link
Author

mbetamony commented Sep 18, 2023

Something to keep in mind @mbartenev-atypon is that since requests will now be sent directly to the Manuscripts API, all parameters with an 'id' postfix will be in capital letters. For example, 'documentID', 'clientID' and 'clientIDs' will all be in capital letters.
This and the front-end code will probably require an update after we move it to manuscripts-api, I'll keep you updated.

@mbartenev-atypon
Copy link
Contributor

mbartenev-atypon commented Sep 18, 2023

@mbartenev-atypon, I think this is ready for testing. If you can provide me with example requests or if there's a version of the editor that should be using the new API, that would be great.

Here is the PR for FE Implementation:
Atypon-OpenSource/manuscripts-article-editor#252

To test we need to edit content and see that the quarterback works correctly - changes are applied correctly. Also a page should be reloaded and the changes from quarterback should be preserved. Applying and rejecting changes should also work fine.

@mbartenev-atypon mbartenev-atypon changed the title Authority TCP-100c Authority Sep 18, 2023
@mbetamony
Copy link
Author

@mbartenev-atypon, I think this is ready for testing. If you can provide me with example requests or if there's a version of the editor that should be using the new API, that would be great.

Here is the PR for FE Implementation: Atypon-OpenSource/manuscripts-article-editor#252

To test we need to edit content and see that the quarterback works correctly - changes are applied correctly. Also a page should be reloaded and the changes from quarterback should be preserved. Applying and rejecting changes should also work fine.

I'm trying to run the PR locally, I also added this PR for the body-editor dependency, but I'm getting lots of errors when building the article-editor. Is it working for you?

@mbartenev-atypon
Copy link
Contributor

@mbartenev-atypon, I think this is ready for testing. If you can provide me with example requests or if there's a version of the editor that should be using the new API, that would be great.

Here is the PR for FE Implementation: Atypon-OpenSource/manuscripts-article-editor#252
To test we need to edit content and see that the quarterback works correctly - changes are applied correctly. Also a page should be reloaded and the changes from quarterback should be preserved. Applying and rejecting changes should also work fine.

I'm trying to run the PR locally, I also added this PR for the body-editor dependency, but I'm getting lots of errors when building the article-editor. Is it working for you?

Yes, that should work without any errors.

@mbetamony
Copy link
Author

mbetamony commented Oct 6, 2023

This PR has the following changes:

Changing Quarterback-API Build/Dev process:

  • Removed the previous Rollup configuration and replaced it with one from Pressroom and Mikhail's demo.

New API based on Mikhail's demo and Prosemirror's collaborative processing:

  • Primary logic can be found in collaboration.svc.ts, doc.svc.ts, doc.ctrl.ts, doc.schema.ts, other changes are results of changing the build process.

Some notes:

  • @mbartenev-atypon, I'd prefer that we change all clientID and clientIDs to strings since we're now saving them to the database. A regular integer wasn't enough to hold their values. Additionally, I believe it's better to have the versionID in the request body for the getStepsOfVersion/getVersion route. Do you agree?
  • Also, since this is going to move to Manuscripts-api, we need to choose between ID(used by manuscripts-api) or Id(used by quarterback-api).
  • History is cleared after saving the snapshot, this can be found in snap.svc.ts.

Testing the API:

@mbetamony mbetamony requested a review from mnatsheh October 6, 2023 15:05
@mbetamony mbetamony requested a review from hhusban October 8, 2023 09:23
@mbetamony
Copy link
Author

mbetamony commented Oct 22, 2023

@mbartenev-atypon, @mnatsheh, and @hhusban, can you please review this?

Also since we're moving the routes to manuscripts-api, I reconfigured the routes here.

We have 3 routes:

  1. Get initial history (previously listen).
  2. Receive steps.
  3. Get steps from a certain version.

The open connection will now be between manuscript-api and the front-end. I'm keeping the client's data (ID and response) in manuscripts-api instead of quarterback. Whenever a change happens, and we want to notify listeners, we get the data from QB and notify the clients registered in manuscripts-api.

Instead of saving a memory version for each document, there's a request queue now. Throttling the requests might also solve the memory issue, if it did we can remove the queue.

I'm also deleting the document history, and version becomes 0.

I'll also push my manuscripts-api PR once this PR is merged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants